[PATCH v1 2/6] ipa: rkisp1: awb: Load white balance gains from tuning file

[Stefan Klug]

On Mon, Aug 05, 2024 at 02:43:39PM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2024-08-05 13:05:03)
> > For the implementation of a manual colour temperature setting, it is
> > necessary to read predefined colour gains per colour temperature from
> > the tuning file. Implement this in a backwards compatible way. If no
> > gains are contained in the tuning file, loading just continues as
> > before.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/awb.h   |  6 ++++++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index 4ccafd48dedd..957d24fe3425 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -39,6 +39,24 @@ Awb::Awb()
> >  {
> >  }
> >  
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::init
> > + */
> > +int Awb::init(IPAContext &context, const YamlObject &tuningData)
> > +{
> > +       MatrixInterpolator<double, 2, 1> gains;
> > +       int ret = gains.readYaml(tuningData["gains"], "ct", "gains");
> 
> Do we have a way to check if there are gains set at all? If there are no
> gains set in the yaml - we wouldn't report a warning at all I'd expect?

If there are no gains in the yaml, readYaml returns an error. We could
add a check for that specific case and silence the warning. But do we
really want to silence that? It basically says "You have an old tuning
file, some features are limited but I'll continue". So I think it's
reasonable - up to you.

> 
> (And presumably we wouldn't expose/report a manual color temperature
> control as setable either?)

That's a bit difficult here as the temperature also controls the ccm.
So we would need to check the existence of ccms also. As the ccms are
necessary for a properly tuned sensor, I think it is ok to
unconditionally add the control. 

> 
> > +       if (ret < 0)
> > +               LOG(RkISP1Awb, Warning)
> > +                       << "Failed to parse 'gains' "
> > +                       << "parameter from tuning file; "
> > +                       << "rgb gains will not be set based on colour temperature";
> > +       else
> > +               gains_ = gains;
> > +
> > +       return 0;
> > +}
> > +
> >  /**
> >   * \copydoc libcamera::ipa::Algorithm::configure
> >   */
> > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h
> > index 06c92896e2dc..a010e6d1cb3c 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.h
> > +++ b/src/ipa/rkisp1/algorithms/awb.h
> > @@ -7,6 +7,10 @@
> >  
> >  #pragma once
> >  
> > +#include <optional>
> > +
> > +#include "libipa/matrix_interpolator.h"
> > +
> >  #include "algorithm.h"
> >  
> >  namespace libcamera {
> > @@ -19,6 +23,7 @@ public:
> >         Awb();
> >         ~Awb() = default;
> >  
> > +       int init(IPAContext &context, const YamlObject &tuningData) override;
> >         int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> >         void queueRequest(IPAContext &context, const uint32_t frame,
> >                           IPAFrameContext &frameContext,
> > @@ -34,6 +39,7 @@ public:
> >  private:
> >         uint32_t estimateCCT(double red, double green, double blue);
> >  
> > +       std::optional<MatrixInterpolator<double, 2, 1>> gains_;
> >         bool rgbMode_;
> >  };
> >  
> > -- 
> > 2.43.0
> >