[libcamera-devel] [PATCH v4 4/4] ipa: ipu3: awb: Clamp gain values

[Kieran Bingham]

Quoting Umang Jain (2022-02-28 17:02:43)
> Hi JM
> 
> On 2/24/22 20:41, Jean-Michel Hautbois wrote:
> > The gain values are coded as u3.13 fixed point values, ie they can not
> > be more than 8. Clampt the values in order to avoid any off limits value
> 
> 
> s/Clampt/Clamp
> 
> > which could make the IPU3 behave weirdly.
> >
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > ---
> >   src/ipa/ipu3/algorithms/awb.cpp | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> > index 1dc27fc9..dc25be81 100644
> > --- a/src/ipa/ipu3/algorithms/awb.cpp
> > +++ b/src/ipa/ipu3/algorithms/awb.cpp
> > @@ -353,6 +353,14 @@ void Awb::awbGreyWorld()
> >   
> >       /* Color temperature is not relevant in Grey world but still useful to estimate it :-) */
> >       asyncResults_.temperatureK = estimateCCT(sumRed.R, sumRed.G, sumBlue.B);
> > +
> > +     /*
> > +      * Gain values are unsigned integer value, range 0 to 8 with 13 bit

Techincally, we can't store '8' in this representation though can we.
It looks like the max is something like 7.99987792969 ... so maybe this
should be range 0 to < 8 ?

Unless there's a better notation to say up to but not including 8?
 
> I am not able to decipher "integer value" with "fractional part". The 
> redGain and blueGain are floating valuesright..?

Perhaps stating that they are stored as unsigned integer 3.13 fixed
point might be more clear.

I would love to see a proper fixed-point class representing that, but
that could be a big task, so I think this is fine until we get something
like that.

With Clampt/Clamp, and this updated if needed/desired.

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

> 
> > +      * fractional part.
> > +      */
> > +     redGain = std::clamp(redGain, 0.0, 65535.0 / 8192);
> > +     blueGain = std::clamp(blueGain, 0.0, 65535.0 / 8192);
> > +
> >       asyncResults_.redGain = redGain;
> >       /* Hardcode the green gain to 1.0. */
> >       asyncResults_.greenGain = 1.0;