[PATCH v2 16/17] ipa: rkisp1: awb: Take the CCM into account for the AWB gains calculation

[Laurent Pinchart]

On Mon, Mar 31, 2025 at 06:56:27PM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-03-19 16:11:21)
> > The AWB measurements are taken after the CCM. This can be seen by
> > enabling debug logging on AWB, disabling AWB (stats will still be
> > processed) and manually chaning the CCM.
> > 
> > This means that the estimated colour temperature and the corresponding
> > CCM also lead to changed rgbMeans which in turn leads to oscillations.
> > Fix that by applying the inverse transform on the rgbMeans.
> 
> I'm happy to see this one!
> 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > 
> > ---
> > 
> > Changes in v2:
> > - Improved commit message
> > - Added comment in the code
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index 324bc14b42a0..e8b04d03748d 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -412,6 +412,12 @@ RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rk
> >                 rgbMeans = rgbMeans.max(0.0);
> >         }
> >  
> > +       /*
> > +        * The ISP computes the AWB means after applying the ccm. Apply the

s/ccm/CCM/

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > +        * inverse as we want to get the raw means before the colour gains.
> > +        */
> > +       rgbMeans = frameContext.ccm.ccm.inverse() * rgbMeans;
> > +
> 
> So 'simple' yet so powerful thanks to the helpers before.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> Sounds silly, but I would have written this as :
> 
> 	rgbMeans = rgbMeans * frameContext.ccm.ccm.inverse();
> 
> But I (very much) hope that's identical,

It's not :-)

> but reads as 'multiply the rgb
> by the inverse ccm' to me, (as opposed to 'multiply the inverse CCM by
> the rgbMeans).
> 
> Anyway, it shouldn't make a difference so it's fine either way to me.
> 
> >         /*
> >          * The ISP computes the AWB means after applying the colour gains,
> >          * divide by the gains that were used to get the raw means from the

-- 
Regards,

Laurent Pinchart