[PATCH 07/10] ipa: rkisp1: Damp color temperature regulation

[Stefan Klug]

Hi Kieran,

Thank you for the review. 

On Thu, Feb 20, 2025 at 12:20:18PM +0000, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-02-17 10:01:48)
> > Damp the regulation of the color temperature with the same factor as the
> > gains.  Not damping the color temperature leads to visible flicker, as
> > the CCM changes too much.
> 
> Interesting, I thought we had something on the CCM interplotator so it
> wouldn't change too much on small movements of the colour temperature,
> but I guess smoothing out the colour temperature is reasonable anyway -
> especially as it's an estimate. I wonder if we should have some sort of
> IIR instances to make these a bit more specific, but the speed is fine
> now.

The interpolator has support for quantizations. On lsc we use that.  But
I'm not really confident if that is the best strategy. The damping is
merely to damp larger spikes, which wouldn't get caught by quantization.
With the IIR instance you are completely right. I even had some
prototype somewhere. But properly tuning an IIR filter can also be a
pain :-)

> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> Or maybe we should report the real calculated value to metadata, but
> filter it on the input to the CCM interpolator ?

I'm quite undecided here. The filtered data might be better than the
unfiltered one? I think I'd stick to the filtered one, as it is in sync
with the gains.

Regards,
Stefan

> 
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index 9244a1e64f41..347d38d226b4 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -320,8 +320,6 @@ void Awb::process(IPAContext &context,
> >             rgbMeans.b() < kMeanMinThreshold)
> >                 return;
> >  
> > -       activeState.awb.automatic.temperatureK = estimateCCT(rgbMeans);
> > -
> >         /*
> >          * Estimate the red and blue gains to apply in a grey world. The green
> >          * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
> > @@ -343,8 +341,11 @@ void Awb::process(IPAContext &context,
> >  
> >         /* Filter the values to avoid oscillations. */
> >         double speed = 0.2;
> > +       double ct = estimateCCT(rgbMeans);
> > +       ct = ct * speed + activeState.awb.automatic.temperatureK * (1 - speed);
> >         gains = gains * speed + activeState.awb.automatic.gains * (1 - speed);
> >  
> > +       activeState.awb.automatic.temperatureK = static_cast<unsigned int>(ct);
> >         activeState.awb.automatic.gains = gains;
> >  
> >         LOG(RkISP1Awb, Debug)
> > -- 
> > 2.43.0
> >