[libcamera-devel] [PATCH v3 1/3] ipa: raspberrypi: Make CamHelper exposure methods virtual

[David Plowman]

Hi Laurent

Thanks for the comments and questions!

On Fri, 16 Apr 2021 at 22:56, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Wed, Apr 14, 2021 at 11:29:53AM +0100, David Plowman wrote:
> > This allows derived classes to override them if they have any special
> > behaviours to implement. For instance if a particular camera mode
> > produces a different signal level to other modes, you might choose to
> > address that in the gain or exposure methods.
>
> For my information, would you be able to give an example of how this
> would be used ? What kind of particular camera mode would produce a
> different signal level, is this related to binning, or something else ?

Yes, so the sensor I'm looking at produces twice the signal level in
the binning modes that it does in the full resolution mode. I think
this difference would be awkward for applications which means we have
to hide it somewhere.

One of my earlier emails talked a little bit about hiding this in the
AGC. You'd have to know a "base gain" for every mode (so 1 for full
res, 2 for binned), and then have some process where we have public
exposure/gain numbers (which are the same for all modes) and there's a
process of conversion into and out of the AGC and IPA which deal with
real exposure/gain values. But I'm really not sure I want to go there,
at least right now!

I think this is a pragmatic alternative. You can already hide the
difference by changing the CamHelper's Gain and GainCode functions, to
apply the magic extra factor of 2 when necessary. This change here
makes it possible to apply that factor in the exposure instead - you
could imagine someone wanting the lowest noise possible and accepting
that the real exposure will be double (I wouldn't want to double
exposures without people explicitly making that choice). And finally,
this change doesn't make it any more difficult to do the compensation
in the AGC at a later date, should we wish.

In the CamHelper for my new sensor, I even make it easy to mix the
factor of 2 across both gain and exposure, so you could, for example,
have sqrt(2) in each.

That got rather long... I hope it was understandable!

David

>
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > ---
> >  src/ipa/raspberrypi/cam_helper.hpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp
> > index 1b2d6eec..4053a870 100644
> > --- a/src/ipa/raspberrypi/cam_helper.hpp
> > +++ b/src/ipa/raspberrypi/cam_helper.hpp
> > @@ -66,8 +66,8 @@ public:
> >       virtual ~CamHelper();
> >       void SetCameraMode(const CameraMode &mode);
> >       MdParser &Parser() const { return *parser_; }
> > -     uint32_t ExposureLines(double exposure_us) const;
> > -     double Exposure(uint32_t exposure_lines) const; // in us
> > +     virtual uint32_t ExposureLines(double exposure_us) const;
> > +     virtual double Exposure(uint32_t exposure_lines) const; // in us
> >       virtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,
> >                                     double maxFrameDuration) const;
> >       virtual uint32_t GainCode(double gain) const = 0;
>
> --
> Regards,
>
> Laurent Pinchart