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

[Kieran Bingham]

Quoting Stefan Klug (2024-08-06 07:37:54)
> 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.

So - perhaps we should be instantiating this control at the top level
and not inside the awb. If someone disables the awb module
in the tuning file, it would impact CCM ?


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