[PATCH v5 3/8] ipa: rkisp1: awb: Implement ColourTemperature control

[Kieran Bingham]

Quoting Stefan Klug (2024-12-06 14:52:23)
> 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>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 35 ++++++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 23a81e75d3d3..41f260e7089c 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -33,6 +33,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,8 +48,13 @@ Awb::Awb()
>  /**
>   * \copydoc libcamera::ipa::Algorithm::init
>   */
> -int Awb::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> +int Awb::init(IPAContext &context, const YamlObject &tuningData)
>  {
> +       auto &cmap = context.ctrlMap;
> +       cmap[&controls::ColourTemperature] = ControlInfo(kMinColourTemperature,
> +                                                        kMaxColourTemperature,
> +                                                        kDefaultColourTemperature);
> +
>         Interpolator<Vector<double, 2>> gains;
>         int ret = gains.readYaml(tuningData["gains"], "ct", "gains");
>         if (ret < 0)
> @@ -101,19 +110,31 @@ void Awb::queueRequest(IPAContext &context,
>                         << (*awbEnable ? "Enabling" : "Disabling") << " AWB";
>         }
>  
> +       frameContext.awb.autoEnabled = awb.autoEnabled;
> +
>         const auto &colourGains = controls.get(controls::ColourGains);
> -       if (colourGains && !awb.autoEnabled) {
> +       const auto &colourTemperature = controls.get(controls::ColourTemperature);

Why do you get the colourTemperature here

> +
> +       if (awb.autoEnabled)

And then potentially return without using it here ?

> +               return;

Perhaps we should only get it here ?

> +
> +       bool update = false;
> +       if (colourGains) {
>                 awb.gains.manual.r() = (*colourGains)[0];
>                 awb.gains.manual.b() = (*colourGains)[1];
> +               update = true;
> +       } else if (colourTemperature && gains_) {
> +               auto gains = gains_->getInterpolated(*colourTemperature);
> +               awb.gains.manual.r() = gains[0];
> +               awb.gains.manual.b() = gains[1];
> +               update = true;
> +       }

I'm getting more convinced that queueRequest for each algorithm that
uses libipa should be using a common path... This would need to be
implemented in the same way for Mali, IPU3, RKISP1 ... (and Pi
separately)

(I had the same comment earlier on AGC for Mali ... but I don't think it
blocks getting these merged - but I wouldn't want to see all the libipa
implementations having a different response to control handling in the
end).

With the adjustment for when you read the
controls.get(controls::ColourTemperature); to be after the
awb.autoEnabled()


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


>  
> +       if (update)
>                 LOG(RkISP1Awb, Debug)
>                         << "Set colour gains to " << awb.gains.manual;
> -       }
> -
> -       frameContext.awb.autoEnabled = awb.autoEnabled;
>  
> -       if (!awb.autoEnabled)
> -               frameContext.awb.gains = awb.gains.manual;
> +       frameContext.awb.gains = awb.gains.manual;
>  }
>  
>  /**
> -- 
> 2.43.0
>