[PATCH v1 3/6] ipa: rkisp1: awb: Implement ColourTemperature control

[Stefan Klug]

Hi Kieran,

Thanks for the review.

On Mon, Aug 05, 2024 at 02:46:00PM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2024-08-05 13:05:04)
> > There are many use-cases (tuning-validation, working in static
> > environments) where a manual ColourTemperature control is helpful.
> > Implement that by interpolating and applying the white balance gains
> > from the tuning file according to the requested colour temperature. If
> > colour gains are provided on the same request, they take precedence. As
> > the colour temperature reported in the metadata is always based on the
> > measurements, we don't have to touch that.
> > 
> > Note that in the automatic case, the colour gains are still based on the
> > gray world model and the ones from the tuning file get ignored.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index 957d24fe3425..d482eda5b541 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -31,6 +31,10 @@ namespace ipa::rkisp1::algorithms {
> >  
> >  LOG_DEFINE_CATEGORY(RkISP1Awb)
> >  
> > +constexpr int32_t kMinColourTemperature = 2500;
> > +constexpr int32_t kMaxColourTemperature = 10000;
> > +constexpr int32_t kDefaultColourTemperature = 6500;
> > +
> >  /* Minimum mean value below which AWB can't operate. */
> >  constexpr double kMeanMinThreshold = 2.0;
> >  
> > @@ -44,6 +48,11 @@ Awb::Awb()
> >   */
> >  int Awb::init(IPAContext &context, const YamlObject &tuningData)
> >  {
> > +       auto &cmap = context.ctrlMap;
> > +       cmap[&controls::ColourTemperature] = ControlInfo(kMinColourTemperature,
> > +                                                        kMaxColourTemperature,
> > +                                                        kDefaultColourTemperature);
> > +
> 
> Should this control only be exposed if there are gains available in the
> tuning file?

As noted in the previous mail I think unconditional is ok as it also
applies to the ccms which should be available.

> 
> Can/Should the minColourTemperature/maxColourTemperature be based on the
> values that are available in the Tuning file? Or does the interpolator
> make this possible to extend beyond the limits calibrated if it can
> extrapolate for wider values?

The interpolator extends the values if the temperature out of range.
Here we would also need to check the range of ccms and gains. I don't
know if it's worth the effort. On the other hand the min/max values are
arbitrary. A quick google indicates that we could even lower the min to
1000.

> 
> 
> 
> >         MatrixInterpolator<double, 2, 1> gains;
> >         int ret = gains.readYaml(tuningData["gains"], "ct", "gains");
> >         if (ret < 0)
> > @@ -113,6 +122,17 @@ void Awb::queueRequest(IPAContext &context,
> >                         << ", blue: " << awb.gains.manual.blue;
> >         }
> >  
> > +       const auto &colourTemperature = controls.get(controls::ColourTemperature);
> > +       if (colourTemperature && !awb.autoEnabled && gains_ && !colourGains) {
> > +               Matrix<double, 2, 1> gains = gains_->get(*colourTemperature);
> > +               awb.gains.manual.red = gains[0][0];
> > +               awb.gains.manual.blue = gains[1][0];
> > +
> > +               LOG(RkISP1Awb, Debug)
> > +                       << "Set colour gains to red: " << awb.gains.manual.red
> > +                       << ", blue: " << awb.gains.manual.blue;
> > +       }
> > +
> >         frameContext.awb.autoEnabled = awb.autoEnabled;
> >  
> >         if (!awb.autoEnabled) {
> > -- 
> > 2.43.0
> >