[libcamera-devel] [PATCH v4 30/32] ipa: rkisp1: awb: Clamp gains to prevent divisions by zero

[Jacopo Mondi]

Hi

On Wed, Sep 21, 2022 at 12:41:24AM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:58)
> > The gain values are currently clamped to the range [0.0, 3.996] used by
> > the hardware. A zero value makes little sense, as it would completely
> > remove the contribution of the corresponding color channel from the AWB
> > accumulators, but worse, would lead to divisions by zero when
> > calculating the raw means in subsequent iterations. Prevent this by
> > setting the minimum gain value to 1/256.
> >
> > While at it, clamp the gain values before filtering them, to improve the
> > stability of the control loop.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 22 +++++++++++++++-------
> >  1 file changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index de54c4d24650..5f2535688c93 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -265,21 +265,29 @@ void Awb::process(IPAContext &context,
> >
> >         frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
> >
> > -       /* Estimate the red and blue gains to apply in a grey world. */
> > +       /*
> > +        * Estimate the red and blue gains to apply in a grey world. The green
> > +        * gain is hardcoded to 0.
>
> 0 or 1?

I guess a gain of 0 means multiplying the color channel by 1 :) ?

>
> With that
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>
> > +        */
> >         double redGain = greenMean / (redMean + 1);
> >         double blueGain = greenMean / (blueMean + 1);
> >
> > +       /*
> > +        * Clamp the gain values to the hardware, which expresses gains as Q2.8
> > +        * unsigned integer values. Set the minimum just above zero to avoid
> > +        * divisions by zero when computing the raw means in subsequent
> > +        * iterations.
> > +        */
> > +       redGain = std::clamp(redGain, 1.0 / 256, 1023.0 / 256);
> > +       blueGain = std::clamp(blueGain, 1.0 / 256, 1023.0 / 256);
> > +
> >         /* Filter the values to avoid oscillations. */
> >         double speed = 0.2;
> >         redGain = speed * redGain + (1 - speed) * activeState.awb.gains.automatic.red;
> >         blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.automatic.blue;
> >
> > -       /*
> > -        * Gain values are unsigned integer value, range 0 to 4 with 8 bit
> > -        * fractional part. Hardcode the green gain to 1.0.
> > -        */
> > -       activeState.awb.gains.automatic.red = std::clamp(redGain, 0.0, 1023.0 / 256);
> > -       activeState.awb.gains.automatic.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);
> > +       activeState.awb.gains.automatic.red = redGain;
> > +       activeState.awb.gains.automatic.blue = blueGain;
> >         activeState.awb.gains.automatic.green = 1.0;
> >
> >         LOG(RkISP1Awb, Debug) << std::showpoint
> > --
> > Regards,
> >
> > Laurent Pinchart
> >