[PATCH 09/10] ipa: rkisp1: awb: Take the CCM into account for the AWB gains calculation

[Stefan Klug]

Hi Kieran,

Thank you for the review. 

On Mon, Feb 17, 2025 at 12:26:36PM +0000, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-02-17 10:01:50)
> > Apparently AWB measurements are taken after the CCM. So the estimated
> 
> Are we more sure than 'apparantly' ? What do we need to do to be more
> confident or certain of this ?

You are right. I will explain how to test that in v2. Basically, you can
comment the gains from the manual colour temperture code path and enable
AWB debug logs. Then you can see the measurements change, when you
modify the colour temperature. (I just realize that we can't manually
set the CCM on rkisp1. That would have been an even easier test)

> 
> > colour temperature and the corresponding CCM also lead to changed
> > rgbMeans. This is another source of oscillations. Fix that by applying
> > the inverse transform on the rgbMeans.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index 12934771c69c..66e6aecedc4c 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -305,6 +305,8 @@ void Awb::process(IPAContext &context,
> >                 rgbMeans = rgbMeans.max(0.0);
> >         }
> >  
> 
> I would definitely put a block comment here describing /why/ we are
> doing this - likely matching closely to what you have in the commit
> message.

Right. Will add one.

Best regards,
Stefan

> 
> > +       rgbMeans = frameContext.ccm.ccm.cast<double>().inverse() * rgbMeans;
> > +
> >         /*
> >          * 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
> > -- 
> > 2.43.0
> >