[libcamera-devel] [PATCH] ipa: raspberrypi: awb: Update colour temperature whenever manual gains change

[Kieran Bingham]

Quoting Naushir Patuck via libcamera-devel (2022-10-28 11:53:00)
> Hi David,
> 
> Thank you for this work.
> 
> On Thu, 27 Oct 2022 at 12:40, David Plowman via libcamera-devel <
> libcamera-devel at lists.libcamera.org> wrote:
> 
> > Previously we only did this when the system starts (on the first
> > switch_mode). Now we do it whenever the manual colour gains are
> > updated. To facilitate this, this R/B vs. colour temperature inverse
> > functions are stored persistently in the AwbConfig.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> >
> 
> It's good to be consistent with this behavior.
> 
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> > ---
> >  src/ipa/raspberrypi/controller/rpi/awb.cpp | 24 +++++++++-------------
> >  src/ipa/raspberrypi/controller/rpi/awb.h   |  3 ++-
> >  2 files changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > index 2b88c3b0..8d8ddf09 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > @@ -104,6 +104,9 @@ int AwbConfig::read(const libcamera::YamlObject
> > &params)
> >                 ret = readCtCurve(ctR, ctB, params["ct_curve"]);
> >                 if (ret)
> >                         return ret;
> > +               /* We will want the inverse functions of these too. */
> > +               ctRInverse = ctR.inverse();
> > +               ctBInverse = ctB.inverse();
> >         }
> >
> >         if (params.contains("priors")) {
> > @@ -174,7 +177,6 @@ Awb::Awb(Controller *controller)
> >         asyncAbort_ = asyncStart_ = asyncStarted_ = asyncFinished_ = false;
> >         mode_ = nullptr;
> >         manualR_ = manualB_ = 0.0;
> > -       firstSwitchMode_ = true;
> >         asyncThread_ = std::thread(std::bind(&Awb::asyncFunc, this));
> >  }
> >
> > @@ -270,27 +272,21 @@ void Awb::setManualGains(double manualR, double
> > manualB)
> >                 syncResults_.gainR = prevSyncResults_.gainR = manualR_;
> >                 syncResults_.gainG = prevSyncResults_.gainG = 1.0;
> >                 syncResults_.gainB = prevSyncResults_.gainB = manualB_;
> > +               if (config_.bayes) {
> > +                       /* Also estimate the best corresponding colour
> > temperature from the curves. */
> > +                       double ctR =
> > config_.ctRInverse.eval(config_.ctRInverse.domain().clip(1 / manualR_));
> > +                       double ctB =
> > config_.ctBInverse.eval(config_.ctBInverse.domain().clip(1 / manualB_));
> > +                       prevSyncResults_.temperatureK = (ctR + ctB) / 2;
> > +                       syncResults_.temperatureK =
> > prevSyncResults_.temperatureK;
> > +               }
> >         }
> >  }
> >
> >  void Awb::switchMode([[maybe_unused]] CameraMode const &cameraMode,
> >                      Metadata *metadata)
> >  {
> > -       /*
> > -        * On the first mode switch we'll have no meaningful colour
> > -        * temperature, so try to dead reckon one if in manual mode.
> > -        */
> > -       if (!isAutoEnabled() && firstSwitchMode_ && config_.bayes) {
> > -               Pwl ctRInverse = config_.ctR.inverse();
> > -               Pwl ctBInverse = config_.ctB.inverse();
> > -               double ctR = ctRInverse.eval(ctRInverse.domain().clip(1 /
> > manualR_));
> > -               double ctB = ctBInverse.eval(ctBInverse.domain().clip(1 /
> > manualB_));
> > -               prevSyncResults_.temperatureK = (ctR + ctB) / 2;
> > -               syncResults_.temperatureK = prevSyncResults_.temperatureK;
> > -       }
> >         /* Let other algorithms know the current white balance values. */
> >         metadata->set("awb.status", prevSyncResults_);
> > -       firstSwitchMode_ = false;
> >  }
> >
> >  bool Awb::isAutoEnabled() const
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h
> > b/src/ipa/raspberrypi/controller/rpi/awb.h
> > index cb4cfd1b..30acd89d 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.h
> > @@ -42,6 +42,8 @@ struct AwbConfig {
> >         bool fast; /* "fast" mode uses a 16x16 rather than 32x32 grid */
> >         Pwl ctR; /* function maps CT to r (= R/G) */
> >         Pwl ctB; /* function maps CT to b (= B/G) */
> > +       Pwl ctRInverse; /* inverse of ctR */
> > +       Pwl ctBInverse; /* inverse of ctB */
> >         /* table of illuminant priors at different lux levels */
> >         std::vector<AwbPrior> priors;
> >         /* AWB "modes" (determines the search range) */
> > @@ -168,7 +170,6 @@ private:
> >         double manualR_;
> >         /* manual b setting */
> >         double manualB_;
> > -       bool firstSwitchMode_; /* is this the first call to SwitchMode? */
> >  };
> >
> >  static inline Awb::RGB operator+(Awb::RGB const &a, Awb::RGB const &b)
> > --
> > 2.30.2
> >
> >