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

[Kieran Bingham]

Quoting Stefan Klug (2025-03-19 15:33:48)
> 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 :-)

Yes, indeed! they're difficult to get right for sure.
No need to complicate things here now.

> > 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.

I think that makes a lot of sense! If we report a colour temperature, we
should make sure the gains we use correspond to it!

> 
> 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
> > >