[libcamera-devel] [PATCH] ipa: rpi: awb: Make is possible to set the colour temperature directly

[Laurent Pinchart]

On Thu, Nov 23, 2023 at 12:54:43PM +0000, David Plowman wrote:
> On Thu, 23 Nov 2023 at 12:27, Laurent Pinchart wrote:
> > On Fri, Nov 10, 2023 at 04:14:33PM +0000, David Plowman via libcamera-devel wrote:
> > > ColourTemperature is now exported as a writable control so that
> > > algorithms can set it directly.
> >
> > I'm puzzled here. You mention algorithms, but the patch exposes the
> > control as being writable from applications. Is it a typo in the commit
> > message ? If so, what's the use case ? control_ids.yaml documents the
> > control as
> 
> Yes, I'll change "algorithms" to "applications".
> 
> >
> >   - ColourTemperature:
> >       type: int32_t
> >       description: Report the current estimate of the colour temperature, in
> >         kelvin, for this frame. The ColourTemperature control can only be
> >         returned in metadata.
> >
> > so this would need to be updated as a prerequisite for this patch.
> 
> I'll add a patch before this one to change that description.

OK, let's discuss the use case there.

> > > The AWB algorithm class now requires a
> > > method to be provided to perform this operation. The method should
> > > clamp the passed value to the calibrated range known to the algorithm.
> > >
> > > The default range is set very wide to cover all conceivable future AWB
> > > calibrations. It will always be clamped before use.
> > >
> > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > > ---
> > >  src/ipa/rpi/common/ipa_base.cpp        | 20 ++++++++++++++++++++
> > >  src/ipa/rpi/controller/awb_algorithm.h |  1 +
> > >  src/ipa/rpi/controller/rpi/awb.cpp     | 18 ++++++++++++++++++
> > >  src/ipa/rpi/controller/rpi/awb.h       |  1 +
> > >  4 files changed, 40 insertions(+)
> > >
> > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> > > index 97a32522..cd1d6e3d 100644
> > > --- a/src/ipa/rpi/common/ipa_base.cpp
> > > +++ b/src/ipa/rpi/common/ipa_base.cpp
> > > @@ -80,6 +80,7 @@ const ControlInfoMap::Map ipaColourControls{
> > >       { &controls::AwbEnable, ControlInfo(false, true) },
> > >       { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> > >       { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > > +     { &controls::ColourTemperature, ControlInfo(100, 100000) },
> > >       { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
> > >  };
> > >
> > > @@ -972,6 +973,25 @@ void IpaBase::applyControls(const ControlList &controls)
> > >                       break;
> > >               }
> > >
> > > +             case controls::COLOUR_TEMPERATURE: {
> > > +                     /* Silently ignore this control for a mono sensor. */
> > > +                     if (monoSensor_)
> > > +                             break;
> > > +
> > > +                     auto temperatureK = ctrl.second.get<int32_t>();
> > > +                     RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
> > > +                             controller_.getAlgorithm("awb"));
> > > +                     if (!awb) {
> > > +                             LOG(IPARPI, Warning)
> > > +                                     << "Could not set COLOUR_TEMPERATURE - no AWB algorithm";
> > > +                             break;
> > > +                     }
> > > +
> > > +                     awb->setColourTemperature(temperatureK);
> > > +                     /* This metadata will get reported back automatically. */
> > > +                     break;
> > > +             }
> > > +
> > >               case controls::BRIGHTNESS: {
> > >                       RPiController::ContrastAlgorithm *contrast = dynamic_cast<RPiController::ContrastAlgorithm *>(
> > >                               controller_.getAlgorithm("contrast"));
> > > diff --git a/src/ipa/rpi/controller/awb_algorithm.h b/src/ipa/rpi/controller/awb_algorithm.h
> > > index 8462c4db..d966dfa8 100644
> > > --- a/src/ipa/rpi/controller/awb_algorithm.h
> > > +++ b/src/ipa/rpi/controller/awb_algorithm.h
> > > @@ -18,6 +18,7 @@ public:
> > >       virtual unsigned int getConvergenceFrames() const = 0;
> > >       virtual void setMode(std::string const &modeName) = 0;
> > >       virtual void setManualGains(double manualR, double manualB) = 0;
> > > +     virtual void setColourTemperature(double temperatureK) = 0;
> > >       virtual void enableAuto() = 0;
> > >       virtual void disableAuto() = 0;
> > >  };
> > > diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
> > > index 5ae0c2fa..0918e20d 100644
> > > --- a/src/ipa/rpi/controller/rpi/awb.cpp
> > > +++ b/src/ipa/rpi/controller/rpi/awb.cpp
> > > @@ -275,6 +275,24 @@ void Awb::setManualGains(double manualR, double manualB)
> > >       }
> > >  }
> > >
> > > +void Awb::setColourTemperature(double temperatureK)
> > > +{
> > > +     if (!config_.bayes) {
> > > +             LOG(RPiAwb, Warning) << "AWB uncalibrated - cannot set colour temperature";
> > > +             return;
> > > +     }
> > > +
> > > +     temperatureK = config_.ctR.domain().clip(temperatureK);
> > > +     manualR_ = 1 / config_.ctR.eval(temperatureK);
> > > +     manualB_ = 1 / config_.ctB.eval(temperatureK);
> > > +
> > > +     syncResults_.temperatureK = temperatureK;
> > > +     syncResults_.gainR = manualR_;
> > > +     syncResults_.gainG = 1.0;
> > > +     syncResults_.gainB = manualB_;
> > > +     prevSyncResults_ = syncResults_;
> > > +}
> > > +
> > >  void Awb::switchMode([[maybe_unused]] CameraMode const &cameraMode,
> > >                    Metadata *metadata)
> > >  {
> > > diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
> > > index e7d49cd8..dbd79eda 100644
> > > --- a/src/ipa/rpi/controller/rpi/awb.h
> > > +++ b/src/ipa/rpi/controller/rpi/awb.h
> > > @@ -97,6 +97,7 @@ public:
> > >       unsigned int getConvergenceFrames() const override;
> > >       void setMode(std::string const &name) override;
> > >       void setManualGains(double manualR, double manualB) override;
> > > +     void setColourTemperature(double temperatureK) override;
> > >       void enableAuto() override;
> > >       void disableAuto() override;
> > >       void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;

-- 
Regards,

Laurent Pinchart