[libcamera-devel] [PATCH v2 0/2] Digital gain control

[david.plowman]

Hi Laurent

On Tue, 15 Dec 2020 at 04:58, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> On Mon, Dec 14, 2020 at 10:34:48AM +0000, David Plowman wrote:
> > Hi everyone
> >
> > Thanks for all the reviews and merging that happened towards the end
> > of last week. I think we're getting really close now! So I wondered if
> > I might be able to give this little patch a final nudge over the
> > line...?
>
> Absolutely. Now that the other patches have gone in, it's on top of my
> review todo list, with Naush's frame rate series. Feel free to let me
> know which one I should handle first, otherwise I'll start with this
> one.

Great, thank you very much. These are the final two that I think we
feel we need before letting users try our applications. I don't think
I have any preference on the order, the digital gain one first is
fine!

Thanks and best regards
David

>
> > On Mon, 7 Dec 2020 at 08:58, David Plowman wrote:
> > >
> > > Hi everyone
> > >
> > > I think you can guess what I might be about to say.... :)
> > >
> > > So this is just my little weekly nudge to see where we are with this.
> > > We were having some discussions about the pipeline model as a whole,
> > > as I recall, though I don't think we were wanting to block these
> > > patches. But please let me know what you think!
> > >
> > > Thanks and sorry for being a nuisance...
> > > David
> > >
> > > On Mon, 30 Nov 2020 at 10:30, Jacopo Mondi wrote:
> > > > On Mon, Nov 30, 2020 at 09:59:21AM +0000, David Plowman wrote:
> > > > > Hi everyone
> > > > >
> > > > > I was wondering if I could give this one its customary Monday morning nudge?
> > > > >
> > > > > I notice that there was still some question last week over whether
> > > > > metadata (that's not allowed as a control) should be able to signal
> > > > > its maximum/minimum values. It doesn't really feel like an issue to me
> > > > > - I just report what the pipeline did, I'm not really seeing a need to
> > > > > report what the pipeline *might* have done! Other than that, it seems
> > > > > we were largely agreed...?
> > > >
> > > > My question on the metadata min-max was mostly to plan ahead if we
> > > > ever need to expose the metadata limits from the Camera API, something
> > > > we currently don't have.
> > > >
> > > > Not something to block this patch on.
> > > >
> > > > I see the patch itself has my tags only. I would wait for an explicit
> > > > ack from Laurent then merge it.
> > > >
> > > > > On Thu, 26 Nov 2020 at 14:50, David Plowman wrote:
> > > > > >
> > > > > > Hi everyone
> > > > > >
> > > > > > Here (after much talking ourselves in circles!) is a second version of
> > > > > > the digital gain control. I've taken Jacopo's text for the control
> > > > > > description, and added a second patch that makes use of the control in
> > > > > > the Raspberry Pi IPAs to add the digital gain to the metadata.
> > > > > >
> > > > > > This second patch doesn't really tell you very much. In our libcamera
> > > > > > version of raspistill (still image capture app) there will be a line
> > > > > > like this:
> > > > > >
> > > > > >     iso *= metadata.get<float>(controls::DigitalGain);
> > > > > >
> > > > > > and that's about it!
> > > > > >
> > > > > > Best regards
> > > > > > David
> > > > > >
> > > > > > David Plowman (2):
> > > > > >   libcamera: controls: Add DigitalGain control
> > > > > >   src: ipa: raspberrypi: Add digital gain to libcamera metadata
> > > > > >
> > > > > >  src/ipa/raspberrypi/raspberrypi.cpp |  4 +++-
> > > > > >  src/libcamera/control_ids.yaml      | 17 +++++++++++++++++
> > > > > >  2 files changed, 20 insertions(+), 1 deletion(-)
>
> --
> Regards,
>
> Laurent Pinchart