[PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour temperature reporting

[Laurent Pinchart]

On Fri, May 02, 2025 at 09:08:46AM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-04-03 16:49:15)
> > In commit b60bd37b1a49 ("ipa: rkisp1: Move calculation of RGB means into
> > own function") the output of the current measured colour temperature as
> > metadata was incorrectly added. Remove it.
> > 
> 
> Ooops, it even says "Commit doesn't contain any functional changes." But
> it did!
> 
> Is the colour temperature set elsewhere now?

It's set earlier in the process() function, to
frameContext.awb.temperatureK instead of activeState.awb.temperatureK.

There was a discussion on whether or not we should add a metadata
control to report the colour temperature estimated for the frame, in
addition to the one that was used to process the frame. Using
activeState.awb.temperatureK switches to the latter. Regardless of the
answer to that question, the aforementioned commit introduced a
functional change, so I think this fix is right.

> > Fixes: b60bd37b1a49 ("ipa: rkisp1: Move calculation of RGB means into own function")
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

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

> > ---
> > 
> > Changes in v3:
> > - Added this patch
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index a9759e53f593..79c4c658406d 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -311,9 +311,6 @@ void Awb::process(IPAContext &context,
> >  
> >         activeState.awb.temperatureK = awbResult.colourTemperature;
> >  
> > -       /* Metadata shall contain the up to date measurement */
> > -       metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> > -
> >         /*
> >          * Clamp the gain values to the hardware, which expresses gains as Q2.8
> >          * unsigned integer values. Set the minimum just above zero to avoid

-- 
Regards,

Laurent Pinchart