[libcamera-devel,0/2] Raspberry Pi: handle sensors more flexibly
mbox series

Message ID 20210324114415.19866-1-david.plowman@raspberrypi.com
Headers show
Series
  • Raspberry Pi: handle sensors more flexibly
Related show

Message

David Plowman March 24, 2021, 11:44 a.m. UTC
Hi everyone

Here are a couple more patches aimed at allowing us to deal more
flexibly with "interesting" sensors.

The first patch simply makes the CamHelper's exposure-related methods
virtual. This gives greater flexibility on how we deal with sensors
where different modes have different signal levels. Differences
between modes can be accounted for with the gain methods (already
virtual), in the exposure (with this change), or via a combination of
the two.

Note that another solution to this involves fiddling around with the
AGC. It would have to know that modes can have different "base signal
levels", and account for the difference. I'm still in several minds
about whether I want to do this, so this change gives us options in
the meantime, and doesn't preclude messing with the AGC later if we
wish.

The second commit takes the view that if sensors seem to be asking for
the red/blue colour gains, then we should tell them. I guess there's a
risk that some sensors might seem to want these numbers, but we find
that we don't want to tell them. If that happens maybe the CamHelper
would have to tell us what to do, but this approach seems reasonable
to me at this point.

I also apply a fixed 256x multiplier to the colour gains before
passing them to the control. I don't think V4L2 docs mandate any
particular scale here? Maybe there's an argument for another
CamHelper function "ColourGainCode", or something like that.

Thoughts and suggestions welcome as always. Thanks!

David

David Plowman (2):
  ipa: raspberrypi: Make CamHelper exposure methods virtual
  ipa: raspberrypi: Update sensor's RED/BLUE balance controls when
    present

 src/ipa/raspberrypi/cam_helper.hpp              |  4 ++--
 src/ipa/raspberrypi/raspberrypi.cpp             | 17 ++++++++++++++---
 .../pipeline/raspberrypi/raspberrypi.cpp        |  9 +++++++++
 3 files changed, 25 insertions(+), 5 deletions(-)

Comments

David Plowman April 7, 2021, 9:13 a.m. UTC | #1
Hi everyone

I was wondering if I might give this little pair of changes a teeny
nudge, don't think there's been any feedback as yet...

Thanks!

David


On Wed, 24 Mar 2021 at 11:44, David Plowman
<david.plowman@raspberrypi.com> wrote:
>
> Hi everyone
>
> Here are a couple more patches aimed at allowing us to deal more
> flexibly with "interesting" sensors.
>
> The first patch simply makes the CamHelper's exposure-related methods
> virtual. This gives greater flexibility on how we deal with sensors
> where different modes have different signal levels. Differences
> between modes can be accounted for with the gain methods (already
> virtual), in the exposure (with this change), or via a combination of
> the two.
>
> Note that another solution to this involves fiddling around with the
> AGC. It would have to know that modes can have different "base signal
> levels", and account for the difference. I'm still in several minds
> about whether I want to do this, so this change gives us options in
> the meantime, and doesn't preclude messing with the AGC later if we
> wish.
>
> The second commit takes the view that if sensors seem to be asking for
> the red/blue colour gains, then we should tell them. I guess there's a
> risk that some sensors might seem to want these numbers, but we find
> that we don't want to tell them. If that happens maybe the CamHelper
> would have to tell us what to do, but this approach seems reasonable
> to me at this point.
>
> I also apply a fixed 256x multiplier to the colour gains before
> passing them to the control. I don't think V4L2 docs mandate any
> particular scale here? Maybe there's an argument for another
> CamHelper function "ColourGainCode", or something like that.
>
> Thoughts and suggestions welcome as always. Thanks!
>
> David
>
> David Plowman (2):
>   ipa: raspberrypi: Make CamHelper exposure methods virtual
>   ipa: raspberrypi: Update sensor's RED/BLUE balance controls when
>     present
>
>  src/ipa/raspberrypi/cam_helper.hpp              |  4 ++--
>  src/ipa/raspberrypi/raspberrypi.cpp             | 17 ++++++++++++++---
>  .../pipeline/raspberrypi/raspberrypi.cpp        |  9 +++++++++
>  3 files changed, 25 insertions(+), 5 deletions(-)
>
> --
> 2.20.1
>
Laurent Pinchart April 7, 2021, 9:17 a.m. UTC | #2
Hi David,

On Wed, Apr 07, 2021 at 10:13:13AM +0100, David Plowman wrote:
> Hi everyone
> 
> I was wondering if I might give this little pair of changes a teeny
> nudge, don't think there's been any feedback as yet...

Easter got a bit in the way, with its overdoses of chocolate (as I get
older I'm strangely more tempted to cook the easter bunny instead). I'll
get back to reviewing your pending series.

> On Wed, 24 Mar 2021 at 11:44, David Plowman wrote:
> >
> > Hi everyone
> >
> > Here are a couple more patches aimed at allowing us to deal more
> > flexibly with "interesting" sensors.
> >
> > The first patch simply makes the CamHelper's exposure-related methods
> > virtual. This gives greater flexibility on how we deal with sensors
> > where different modes have different signal levels. Differences
> > between modes can be accounted for with the gain methods (already
> > virtual), in the exposure (with this change), or via a combination of
> > the two.
> >
> > Note that another solution to this involves fiddling around with the
> > AGC. It would have to know that modes can have different "base signal
> > levels", and account for the difference. I'm still in several minds
> > about whether I want to do this, so this change gives us options in
> > the meantime, and doesn't preclude messing with the AGC later if we
> > wish.
> >
> > The second commit takes the view that if sensors seem to be asking for
> > the red/blue colour gains, then we should tell them. I guess there's a
> > risk that some sensors might seem to want these numbers, but we find
> > that we don't want to tell them. If that happens maybe the CamHelper
> > would have to tell us what to do, but this approach seems reasonable
> > to me at this point.
> >
> > I also apply a fixed 256x multiplier to the colour gains before
> > passing them to the control. I don't think V4L2 docs mandate any
> > particular scale here? Maybe there's an argument for another
> > CamHelper function "ColourGainCode", or something like that.
> >
> > Thoughts and suggestions welcome as always. Thanks!
> >
> > David
> >
> > David Plowman (2):
> >   ipa: raspberrypi: Make CamHelper exposure methods virtual
> >   ipa: raspberrypi: Update sensor's RED/BLUE balance controls when
> >     present
> >
> >  src/ipa/raspberrypi/cam_helper.hpp              |  4 ++--
> >  src/ipa/raspberrypi/raspberrypi.cpp             | 17 ++++++++++++++---
> >  .../pipeline/raspberrypi/raspberrypi.cpp        |  9 +++++++++
> >  3 files changed, 25 insertions(+), 5 deletions(-)
> >