[03/10] ipa: ipu3: awb: Port to the new libipa AwbAlgorithm

26894 diff mbox series

Port the IPU3 Awb algorithm to use the new libipa implementation
of AwbAlgorithm.

In this implementation the awbAlgo_ class member is initialised as
UQ<3, 13> following the IPU3 params format documentation.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp |  12 ++-
 src/ipa/ipu3/algorithms/awb.cpp | 194 ++++++++++++++++++----------------------
 src/ipa/ipu3/algorithms/awb.h   |  21 ++---
 src/ipa/ipu3/ipa_context.cpp    |  35 ++++----
 src/ipa/ipu3/ipa_context.h      |  15 ++--
 5 files changed, 128 insertions(+), 149 deletions(-)

Quoting Daniel Scally (2026-06-16 07:41:37)
> Port the IPU3 Awb algorithm to use the new libipa implementation
> of AwbAlgorithm.
> 
> In this implementation the awbAlgo_ class member is initialised as
> UQ<3, 13> following the IPU3 params format documentation.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp |  12 ++-
>  src/ipa/ipu3/algorithms/awb.cpp | 194 ++++++++++++++++++----------------------
>  src/ipa/ipu3/algorithms/awb.h   |  21 ++---
>  src/ipa/ipu3/ipa_context.cpp    |  35 ++++----
>  src/ipa/ipu3/ipa_context.h      |  15 ++--
>  5 files changed, 128 insertions(+), 149 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index d6a7036c6504acb106f5c773b529ad80b8349f85..5120b3bcb177c75ebd61d82c9684779f73305499 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -208,9 +208,15 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>                   ControlList &metadata)
>  {
>         Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
> -       rGain_ = context.activeState.awb.gains.red;
> -       gGain_ = context.activeState.awb.gains.blue;
> -       bGain_ = context.activeState.awb.gains.green;
> +       rGain_ = context.activeState.awb.autoEnabled ?
> +                context.activeState.awb.automatic.gains[0] :
> +                context.activeState.awb.manual.gains[0];
> +       gGain_ = context.activeState.awb.autoEnabled ?
> +                context.activeState.awb.automatic.gains[1] :
> +                context.activeState.awb.manual.gains[1];
> +       bGain_ = context.activeState.awb.autoEnabled ?
> +                context.activeState.awb.automatic.gains[2] :
> +                context.activeState.awb.manual.gains[2];

Can we drop rGain_, gGain_ and bGain_ and change that into an RGB type?
Perhaps as a patch beforehand.


Then we can simply do
	if (context.activeState.awb.autoEnabled)
		gains = context.activeState.awb.automatic.gains;
	else
		gains = context.activeState.awb.manual.gains;

But maybe we need to check what those local class members are being
used for - now I see we're in agc.cpp!

>  
>         /*
>          * The Agc algorithm needs to know the effective exposure value that was
> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> index 613bdbbf51cd127e03906571813260a6857067dc..fa1c70856a3647b188531473e9d9f8b77c747230 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -26,11 +26,40 @@ namespace ipa::ipu3::algorithms {
>  LOG_DEFINE_CATEGORY(IPU3Awb)
>  
>  /*
> - * When zones are used for the grey world algorithm, they are only considered if
> - * their average green value is at least 16/255 (after black level subtraction)
> - * to exclude zones that are too dark and don't provide relevant colour
> - * information (on the opposite side of the spectrum, saturated regions are
> - * excluded by the ImgU statistics engine).
> + * \todo IPU3 doesn't support the Lux algorithm.
> + */

That's really easy to add ;-) (can be later)

> +static constexpr unsigned int kDefaultLux = 500;
> +
> +/**
> + * \brief The IPU3 implementation of AwbStats
> + */
> +class Ipu3AwbStats final : public AwbStats
> +{
> +public:
> +       Ipu3AwbStats(){};
> +       Ipu3AwbStats(const RGB<double> means)
> +               : AwbStats(means)
> +       {
> +       }
> +
> +       double minColourValue() const override
> +       {
> +               return 0.0;

is 0.0 a valid number here? I saw Jacopo's series put 0.2 which I
thought was to prevent some divide by 0. But I'm curious what this would
be used for anyway, or if any platform would use the same numbers?

> +       }
> +};
> +
> +/**
> + * \fn Ipu3AwbStats::Ipu3AwbStats(const RGB<double> means)
> + * \brief Construct an instance of the class with RGB means
> + * \param[in] means The mean R, G and B values from the statistics
> + */
> +
> +/*
> + * Zones are only considered if their average green value is at least
> + * kMinGreenLevelInZone/255 (after black level subtraction) to exclude zones
> + * that are too dark and don't provide relevant colour information (on the
> + * opposite side of the spectrum, saturated regions are excluded by the ImgU
> + * statistics engine).
>   */
>  static constexpr uint32_t kMinGreenLevelInZone = 16;
>  
> @@ -74,26 +103,6 @@ static constexpr uint32_t kMinCellsPerZoneRatio = 255 * 90 / 100;
>   * \brief Sum of the average blue values of each unsaturated cell in the zone
>   */
>  
> -/**
> - * \struct Awb::AwbStatus
> - * \brief AWB parameters calculated
> - *
> - * The AwbStatus structure is intended to store the AWB
> - * parameters calculated by the algorithm
> - *
> - * \var AwbStatus::temperatureK
> - * \brief Color temperature calculated
> - *
> - * \var AwbStatus::redGain
> - * \brief Gain calculated for the red channel
> - *
> - * \var AwbStatus::greenGain
> - * \brief Gain calculated for the green channel
> - *
> - * \var AwbStatus::blueGain
> - * \brief Gain calculated for the blue channel
> - */
> -
>  /* Default settings for Bayer noise reduction replicated from the Kernel */
>  static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>         .wb_gains = { 16, 16, 16, 16 },
> @@ -114,11 +123,7 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>  
>  /**
>   * \class Awb
> - * \brief A Grey world white balance correction algorithm
> - *
> - * The Grey World algorithm assumes that the scene, in average, is neutral grey.
> - * Reference: Lam, Edmund & Fung, George. (2008). Automatic White Balancing in
> - * Digital Photography. 10.1201/9781420054538.ch10.
> + * \brief The IPU3 white balance correction algorithm implementation
>   *
>   * The IPU3 generates statistics from the Bayer Down Scaler output into a grid
>   * defined in the ipu3_uapi_awb_config_s structure.
> @@ -168,26 +173,26 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>   * cells are ignored. The grid configuration is computed by
>   * IPAIPU3::calculateBdsGrid().
>   *
> - * Before calculating the gains, the algorithm aggregates the cell averages for
> - * each zone in generateAwbStats(). Cells that have a too high ratio of
> - * saturated pixels are ignored, and only zones that contain enough
> - * non-saturated cells are then used by the algorithm.
> - *
> - * The Grey World algorithm will then estimate the red and blue gains to apply, and
> - * store the results in the metadata. The green gain is always set to 1.
> + * Before running the AWB algorithm, we aggregate the cell averages for each
> + * zone in generateAwbStats(). Cells that have a too high ratio of saturated
> + * pixels are ignored, and only zones that contain enough non-saturated cells
> + * are then used by the algorithm.
>   */
>  
>  Awb::Awb()
>         : Algorithm()
>  {
> -       asyncResults_.blueGain = 1.0;
> -       asyncResults_.greenGain = 1.0;
> -       asyncResults_.redGain = 1.0;
> -       asyncResults_.temperatureK = 4500;
> -
>         zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
>  }
>  
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Awb::init(IPAContext &context, const ValueNode &tuningData)
> +{
> +       return awbAlgo_.init(tuningData, context.ctrlMap);
> +}
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::configure
>   */
> @@ -197,12 +202,14 @@ int Awb::configure(IPAContext &context,
>         const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
>         stride_ = context.configuration.grid.stride;
>  
> +       awbAlgo_.configure(context.activeState.awb, context.configuration.awb);
> +
>         cellsPerZoneX_ = std::round(grid.width / static_cast<double>(kAwbStatsSizeX));
>         cellsPerZoneY_ = std::round(grid.height / static_cast<double>(kAwbStatsSizeY));
>  
>         /*
>          * Configure the minimum proportion of cells counted within a zone
> -        * for it to be relevant for the grey world algorithm.
> +        * for it to be used.
>          * \todo This proportion could be configured.
>          */
>         cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * kMaxCellSaturationRatio;
> @@ -211,6 +218,17 @@ int Awb::configure(IPAContext &context,
>         return 0;
>  }
>  
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Awb::queueRequest(IPAContext &context, const uint32_t frame,
> +                      IPAFrameContext &frameContext,
> +                      const ControlList &controls)
> +{
> +       awbAlgo_.queueRequest(context.activeState.awb, frame, frameContext.awb,
> +                             controls);
> +}
> +
>  constexpr uint16_t Awb::threshold(float value)
>  {
>         /* AWB thresholds are in the range [0, 8191] */
> @@ -237,11 +255,12 @@ constexpr uint16_t Awb::gainValue(double gain)
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
> -void Awb::prepare(IPAContext &context,
> -                 [[maybe_unused]] const uint32_t frame,
> -                 [[maybe_unused]] IPAFrameContext &frameContext,
> -                 ipu3_uapi_params *params)
> +void Awb::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +                 IPAFrameContext &frameContext,
> +                 [[maybe_unused]] ipu3_uapi_params *params)
>  {
> +       awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
> +
>         /*
>          * Green saturation thresholds are reduced because we are using the
>          * green channel only in the exposure computation.
> @@ -279,13 +298,11 @@ void Awb::prepare(IPAContext &context,
>         params->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset
>                                                         * params->acc_param.bnr.opt_center.y_reset;
>  
> -       params->acc_param.bnr.wb_gains.gr = gainValue(context.activeState.awb.gains.green);
> -       params->acc_param.bnr.wb_gains.r  = gainValue(context.activeState.awb.gains.red);
> -       params->acc_param.bnr.wb_gains.b  = gainValue(context.activeState.awb.gains.blue);
> -       params->acc_param.bnr.wb_gains.gb = gainValue(context.activeState.awb.gains.green);
> -
> -       LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK;
>  
> +       params->acc_param.bnr.wb_gains.gr = gainValue(frameContext.awb.gains.g());
> +       params->acc_param.bnr.wb_gains.r = gainValue(frameContext.awb.gains.r());
> +       params->acc_param.bnr.wb_gains.b = gainValue(frameContext.awb.gains.b());
> +       params->acc_param.bnr.wb_gains.gb = gainValue(frameContext.awb.gains.g());

Coool, now we're doing AWB correctly here!

A quick look at Awb::gainValue looks like it could be converted to a
UQ<3,13> I guess, but it would have to be checked and verified, so lets
keep that as a separate topic, as ther'es that +/- 1.0 which I suspect
isn't a straight translation...

>  
>         params->use.acc_awb = 1;
>         params->use.acc_bnr = 1;
> @@ -366,9 +383,17 @@ void Awb::clearAwbStats()
>         }
>  }
>  
> -void Awb::awbGreyWorld()
> +Ipu3AwbStats Awb::calculateRgbMeans(const ipu3_uapi_stats_3a *stats)
>  {
> -       LOG(IPU3Awb, Debug) << "Grey world AWB";
> +       ASSERT(stats->stats_3a_status.awb_en);
> +
> +       clearAwbStats();
> +       generateAwbStats(stats);
> +       generateZones();
> +
> +       if (zones_.size() <= 10)
> +               return {};
> +
>         /*
>          * Make a separate list of the derivatives for each of red and blue, so
>          * that we can sort them to exclude the extreme gains. We could
> @@ -399,66 +424,21 @@ void Awb::awbGreyWorld()
>         double redGain = sumRed.g() / (sumRed.r() + 1),
>                blueGain = sumBlue.g() / (sumBlue.b() + 1);
>  
> -       /* Color temperature is not relevant in Grey world but still useful to estimate it :-) */
> -       asyncResults_.temperatureK = estimateCCT({{ sumRed.r(), sumRed.g(), sumBlue.b() }});
> -
> -       /*
> -        * Gain values are unsigned integer value ranging [0, 8) with 13 bit
> -        * fractional part.
> -        */
> -       redGain = std::clamp(redGain, 0.0, 65535.0 / 8192);
> -       blueGain = std::clamp(blueGain, 0.0, 65535.0 / 8192);
> -
> -       asyncResults_.redGain = redGain;
> -       /* Hardcode the green gain to 1.0. */
> -       asyncResults_.greenGain = 1.0;
> -       asyncResults_.blueGain = blueGain;
> -}
> -
> -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> -{
> -       ASSERT(stats->stats_3a_status.awb_en);
> -
> -       clearAwbStats();
> -       generateAwbStats(stats);
> -       generateZones();
> -
> -       LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
> -
> -       if (zones_.size() > 10) {
> -               awbGreyWorld();
> -               LOG(IPU3Awb, Debug) << "Gain found for red: " << asyncResults_.redGain
> -                                   << " and for blue: " << asyncResults_.blueGain;
> -       }
> +       return Ipu3AwbStats({ { 1.0 / redGain, 1.0, 1.0 / blueGain } });
>  }
>  
>  /**
>   * \copydoc libcamera::ipa::Algorithm::process
>   */
>  void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> -                 [[maybe_unused]] IPAFrameContext &frameContext,
> -                 const ipu3_uapi_stats_3a *stats,
> -                 [[maybe_unused]] ControlList &metadata)
> +                 IPAFrameContext &frameContext,
> +                 [[maybe_unused]] const ipu3_uapi_stats_3a *stats,
> +                 ControlList &metadata)
>  {
> -       calculateWBGains(stats);
> +       Ipu3AwbStats awbStats = calculateRgbMeans(stats);
>  
> -       /*
> -        * Gains are only recalculated if enough zones were detected.
> -        * The results are cached, so if no results were calculated, we set the
> -        * cached values from asyncResults_ here.
> -        */
> -       context.activeState.awb.gains.blue = asyncResults_.blueGain;
> -       context.activeState.awb.gains.green = asyncResults_.greenGain;
> -       context.activeState.awb.gains.red = asyncResults_.redGain;
> -       context.activeState.awb.temperatureK = asyncResults_.temperatureK;
> -
> -       metadata.set(controls::AwbEnable, true);
> -       metadata.set(controls::ColourGains, {
> -                       static_cast<float>(context.activeState.awb.gains.red),
> -                       static_cast<float>(context.activeState.awb.gains.blue)
> -               });
> -       metadata.set(controls::ColourTemperature,
> -                    context.activeState.awb.temperatureK);
> +       awbAlgo_.process(context.activeState.awb, frameContext.awb, awbStats,
> +                        kDefaultLux, metadata);
>  }
>  
>  REGISTER_IPA_ALGORITHM(Awb, "Awb")
> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> index 7ec322318dab54ae7c8a647a67a0cf5815a36eb6..3f3996e6c846558acd3902c9fb3cb804cb5b65d4 100644
> --- a/src/ipa/ipu3/algorithms/awb.h
> +++ b/src/ipa/ipu3/algorithms/awb.h
> @@ -13,6 +13,9 @@
>  
>  #include <libcamera/geometry.h>
>  
> +#include "libipa/awb.h"
> +#include "libipa/fixedpoint.h"
> +
>  #include "libcamera/internal/vector.h"
>  
>  #include "algorithm.h"
> @@ -21,6 +24,8 @@ namespace libcamera {
>  
>  namespace ipa::ipu3::algorithms {
>  
> +class Ipu3AwbStats;
> +
>  /* Region size for the statistics generation algorithm */
>  static constexpr uint32_t kAwbStatsSizeX = 16;
>  static constexpr uint32_t kAwbStatsSizeY = 12;
> @@ -39,7 +44,11 @@ class Awb : public Algorithm
>  public:
>         Awb();
>  
> +       int init(IPAContext &context, const ValueNode &tuningData) override;
>         int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> +       void queueRequest(IPAContext &context, const uint32_t frame,
> +                         IPAFrameContext &frameContext,
> +                         const ControlList &controls) override;
>         void prepare(IPAContext &context, const uint32_t frame,
>                      IPAFrameContext &frameContext,
>                      ipu3_uapi_params *params) override;
> @@ -49,15 +58,7 @@ public:
>                      ControlList &metadata) override;
>  
>  private:
> -       struct AwbStatus {
> -               double temperatureK;
> -               double redGain;
> -               double greenGain;
> -               double blueGain;
> -       };
> -
> -private:
> -       void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> +       Ipu3AwbStats calculateRgbMeans(const ipu3_uapi_stats_3a *stats);
>         void generateZones();
>         void generateAwbStats(const ipu3_uapi_stats_3a *stats);
>         void clearAwbStats();
> @@ -67,7 +68,7 @@ private:
>  
>         std::vector<RGB<double>> zones_;
>         Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
> -       AwbStatus asyncResults_;
> +       AwbAlgorithm<UQ<3, 13>> awbAlgo_;
>  
>         uint32_t stride_;
>         uint32_t cellsPerZoneX_;
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 3b22f7917650d9e400d5368c2f890d6b2dc846a0..b35c925d959027c540257e47944047c238f85571 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -109,6 +109,11 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Maximum analogue gain supported with the configured sensor
>   */
>  
> +/**
> + * \var IPAActiveState::awb
> + * \brief Active auto-white balance parameters for the IPA
> + */
> +
>  /**
>   * \var IPASessionConfiguration::sensor
>   * \brief Sensor-specific configuration of the IPA
> @@ -123,6 +128,11 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Sensor output resolution
>   */
>  
> +/**
> + * \var IPASessionConfiguration::awb
> + * \brief Auto-white balance specific session configuration data
> + */
> +
>  /**
>   * \var IPAActiveState::agc
>   * \brief Context for the Automatic Gain Control algorithm
> @@ -139,26 +149,6 @@ namespace libcamera::ipa::ipu3 {
>   * The gain should be adapted to the sensor specific gain code before applying.
>   */
>  
> -/**
> - * \var IPAActiveState::awb
> - * \brief Context for the Automatic White Balance algorithm
> - *
> - * \var IPAActiveState::awb.gains
> - * \brief White balance gains
> - *
> - * \var IPAActiveState::awb.gains.red
> - * \brief White balance gain for R channel
> - *
> - * \var IPAActiveState::awb.gains.green
> - * \brief White balance gain for G channel
> - *
> - * \var IPAActiveState::awb.gains.blue
> - * \brief White balance gain for B channel
> - *
> - * \var IPAActiveState::awb.temperatureK
> - * \brief Estimated color temperature
> - */
> -
>  /**
>   * \var IPAActiveState::toneMapping
>   * \brief Context for ToneMapping and Gamma control
> @@ -187,4 +177,9 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Analogue gain multiplier
>   */
>  
> +/**
> + * \var IPAFrameContext::awb
> + * \brief Per-frame auto-white balance parameters for the IPA
> + */
> +
>  } /* namespace libcamera::ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 97fcf06cd4ac9ac6d64c4933fcea80ace0e572df..245cf8b50b270a61df863e314128bede40d30541 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -15,6 +15,7 @@
>  #include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
>  
> +#include <libipa/awb.h>
>  #include <libipa/fc_queue.h>
>  
>  namespace libcamera {
> @@ -44,6 +45,8 @@ struct IPASessionConfiguration {
>                 utils::Duration lineDuration;
>                 Size size;
>         } sensor;
> +
> +       ipa::awb::Session awb;
>  };
>  
>  struct IPAActiveState {
> @@ -60,15 +63,7 @@ struct IPAActiveState {
>                 uint32_t exposureMode;
>         } agc;
>  
> -       struct {
> -               struct {
> -                       double red;
> -                       double green;
> -                       double blue;
> -               } gains;
> -
> -               double temperatureK;
> -       } awb;
> +       ipa::awb::ActiveState awb;

I love all this simplification ;-)

Overall - I think we're going in the right direction so:


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

>  
>         struct {
>                 double gamma;
> @@ -81,6 +76,8 @@ struct IPAFrameContext : public FrameContext {
>                 uint32_t exposure;
>                 double gain;
>         } sensor;
> +
> +       ipa::awb::FrameContext awb;
>  };
>  
>  struct IPAContext {
> 
> -- 
> 2.43.0
>

Hi Dan

On Tue, Jun 16, 2026 at 07:41:37AM +0100, Daniel Scally wrote:
> Port the IPU3 Awb algorithm to use the new libipa implementation
> of AwbAlgorithm.
>
> In this implementation the awbAlgo_ class member is initialised as
> UQ<3, 13> following the IPU3 params format documentation.
>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp |  12 ++-
>  src/ipa/ipu3/algorithms/awb.cpp | 194 ++++++++++++++++++----------------------
>  src/ipa/ipu3/algorithms/awb.h   |  21 ++---
>  src/ipa/ipu3/ipa_context.cpp    |  35 ++++----
>  src/ipa/ipu3/ipa_context.h      |  15 ++--
>  5 files changed, 128 insertions(+), 149 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index d6a7036c6504acb106f5c773b529ad80b8349f85..5120b3bcb177c75ebd61d82c9684779f73305499 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -208,9 +208,15 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  		  ControlList &metadata)
>  {
>  	Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
> -	rGain_ = context.activeState.awb.gains.red;
> -	gGain_ = context.activeState.awb.gains.blue;
> -	bGain_ = context.activeState.awb.gains.green;
> +	rGain_ = context.activeState.awb.autoEnabled ?
> +		 context.activeState.awb.automatic.gains[0] :
> +		 context.activeState.awb.manual.gains[0];
> +	gGain_ = context.activeState.awb.autoEnabled ?
> +		 context.activeState.awb.automatic.gains[1] :
> +		 context.activeState.awb.manual.gains[1];
> +	bGain_ = context.activeState.awb.autoEnabled ?
> +		 context.activeState.awb.automatic.gains[2] :
> +		 context.activeState.awb.manual.gains[2];
>
>  	/*
>  	 * The Agc algorithm needs to know the effective exposure value that was
> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> index 613bdbbf51cd127e03906571813260a6857067dc..fa1c70856a3647b188531473e9d9f8b77c747230 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -26,11 +26,40 @@ namespace ipa::ipu3::algorithms {
>  LOG_DEFINE_CATEGORY(IPU3Awb)
>
>  /*
> - * When zones are used for the grey world algorithm, they are only considered if
> - * their average green value is at least 16/255 (after black level subtraction)
> - * to exclude zones that are too dark and don't provide relevant colour
> - * information (on the opposite side of the spectrum, saturated regions are
> - * excluded by the ImgU statistics engine).
> + * \todo IPU3 doesn't support the Lux algorithm.
> + */
> +static constexpr unsigned int kDefaultLux = 500;
> +
> +/**
> + * \brief The IPU3 implementation of AwbStats
> + */
> +class Ipu3AwbStats final : public AwbStats
> +{
> +public:
> +	Ipu3AwbStats(){};
> +	Ipu3AwbStats(const RGB<double> means)
> +		: AwbStats(means)
> +	{
> +	}
> +
> +	double minColourValue() const override
> +	{
> +		return 0.0;
> +	}
> +};
> +
> +/**
> + * \fn Ipu3AwbStats::Ipu3AwbStats(const RGB<double> means)
> + * \brief Construct an instance of the class with RGB means
> + * \param[in] means The mean R, G and B values from the statistics
> + */
> +
> +/*
> + * Zones are only considered if their average green value is at least
> + * kMinGreenLevelInZone/255 (after black level subtraction) to exclude zones
> + * that are too dark and don't provide relevant colour information (on the
> + * opposite side of the spectrum, saturated regions are excluded by the ImgU
> + * statistics engine).
>   */
>  static constexpr uint32_t kMinGreenLevelInZone = 16;
>
> @@ -74,26 +103,6 @@ static constexpr uint32_t kMinCellsPerZoneRatio = 255 * 90 / 100;
>   * \brief Sum of the average blue values of each unsaturated cell in the zone
>   */
>
> -/**
> - * \struct Awb::AwbStatus
> - * \brief AWB parameters calculated
> - *
> - * The AwbStatus structure is intended to store the AWB
> - * parameters calculated by the algorithm
> - *
> - * \var AwbStatus::temperatureK
> - * \brief Color temperature calculated
> - *
> - * \var AwbStatus::redGain
> - * \brief Gain calculated for the red channel
> - *
> - * \var AwbStatus::greenGain
> - * \brief Gain calculated for the green channel
> - *
> - * \var AwbStatus::blueGain
> - * \brief Gain calculated for the blue channel
> - */
> -
>  /* Default settings for Bayer noise reduction replicated from the Kernel */
>  static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>  	.wb_gains = { 16, 16, 16, 16 },
> @@ -114,11 +123,7 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>
>  /**
>   * \class Awb
> - * \brief A Grey world white balance correction algorithm
> - *
> - * The Grey World algorithm assumes that the scene, in average, is neutral grey.
> - * Reference: Lam, Edmund & Fung, George. (2008). Automatic White Balancing in
> - * Digital Photography. 10.1201/9781420054538.ch10.
> + * \brief The IPU3 white balance correction algorithm implementation
>   *
>   * The IPU3 generates statistics from the Bayer Down Scaler output into a grid
>   * defined in the ipu3_uapi_awb_config_s structure.
> @@ -168,26 +173,26 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>   * cells are ignored. The grid configuration is computed by
>   * IPAIPU3::calculateBdsGrid().
>   *
> - * Before calculating the gains, the algorithm aggregates the cell averages for
> - * each zone in generateAwbStats(). Cells that have a too high ratio of
> - * saturated pixels are ignored, and only zones that contain enough
> - * non-saturated cells are then used by the algorithm.
> - *
> - * The Grey World algorithm will then estimate the red and blue gains to apply, and
> - * store the results in the metadata. The green gain is always set to 1.
> + * Before running the AWB algorithm, we aggregate the cell averages for each
> + * zone in generateAwbStats(). Cells that have a too high ratio of saturated
> + * pixels are ignored, and only zones that contain enough non-saturated cells
> + * are then used by the algorithm.

I guess this was the intended purpose of minColourValue() in AwbStats,
but it seems IPU3 does a per-zone filtering instead of looking at the
per-channel mean values.

Can you add a note on minColourValue() as it feels a bit weird it
returns 0.0 ?

>   */
>
>  Awb::Awb()
>  	: Algorithm()
>  {
> -	asyncResults_.blueGain = 1.0;
> -	asyncResults_.greenGain = 1.0;
> -	asyncResults_.redGain = 1.0;
> -	asyncResults_.temperatureK = 4500;
> -
>  	zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
>  }
>
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Awb::init(IPAContext &context, const ValueNode &tuningData)
> +{
> +	return awbAlgo_.init(tuningData, context.ctrlMap);
> +}
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::configure
>   */
> @@ -197,12 +202,14 @@ int Awb::configure(IPAContext &context,
>  	const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
>  	stride_ = context.configuration.grid.stride;
>
> +	awbAlgo_.configure(context.activeState.awb, context.configuration.awb);
> +
>  	cellsPerZoneX_ = std::round(grid.width / static_cast<double>(kAwbStatsSizeX));
>  	cellsPerZoneY_ = std::round(grid.height / static_cast<double>(kAwbStatsSizeY));
>
>  	/*
>  	 * Configure the minimum proportion of cells counted within a zone
> -	 * for it to be relevant for the grey world algorithm.
> +	 * for it to be used.
>  	 * \todo This proportion could be configured.
>  	 */
>  	cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * kMaxCellSaturationRatio;
> @@ -211,6 +218,17 @@ int Awb::configure(IPAContext &context,
>  	return 0;
>  }
>
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Awb::queueRequest(IPAContext &context, const uint32_t frame,
> +		       IPAFrameContext &frameContext,
> +		       const ControlList &controls)
> +{
> +	awbAlgo_.queueRequest(context.activeState.awb, frame, frameContext.awb,
> +			      controls);
> +}
> +
>  constexpr uint16_t Awb::threshold(float value)
>  {
>  	/* AWB thresholds are in the range [0, 8191] */
> @@ -237,11 +255,12 @@ constexpr uint16_t Awb::gainValue(double gain)
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
> -void Awb::prepare(IPAContext &context,
> -		  [[maybe_unused]] const uint32_t frame,
> -		  [[maybe_unused]] IPAFrameContext &frameContext,
> -		  ipu3_uapi_params *params)
> +void Awb::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +		  IPAFrameContext &frameContext,
> +		  [[maybe_unused]] ipu3_uapi_params *params)
>  {
> +	awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
> +
>  	/*
>  	 * Green saturation thresholds are reduced because we are using the
>  	 * green channel only in the exposure computation.
> @@ -279,13 +298,11 @@ void Awb::prepare(IPAContext &context,
>  	params->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset
>  							* params->acc_param.bnr.opt_center.y_reset;
>
> -	params->acc_param.bnr.wb_gains.gr = gainValue(context.activeState.awb.gains.green);
> -	params->acc_param.bnr.wb_gains.r  = gainValue(context.activeState.awb.gains.red);
> -	params->acc_param.bnr.wb_gains.b  = gainValue(context.activeState.awb.gains.blue);
> -	params->acc_param.bnr.wb_gains.gb = gainValue(context.activeState.awb.gains.green);
> -
> -	LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK;
>
> +	params->acc_param.bnr.wb_gains.gr = gainValue(frameContext.awb.gains.g());
> +	params->acc_param.bnr.wb_gains.r = gainValue(frameContext.awb.gains.r());
> +	params->acc_param.bnr.wb_gains.b = gainValue(frameContext.awb.gains.b());
> +	params->acc_param.bnr.wb_gains.gb = gainValue(frameContext.awb.gains.g());
>
>  	params->use.acc_awb = 1;
>  	params->use.acc_bnr = 1;
> @@ -366,9 +383,17 @@ void Awb::clearAwbStats()
>  	}
>  }
>
> -void Awb::awbGreyWorld()
> +Ipu3AwbStats Awb::calculateRgbMeans(const ipu3_uapi_stats_3a *stats)
>  {
> -	LOG(IPU3Awb, Debug) << "Grey world AWB";
> +	ASSERT(stats->stats_3a_status.awb_en);
> +
> +	clearAwbStats();
> +	generateAwbStats(stats);
> +	generateZones();
> +
> +	if (zones_.size() <= 10)
> +		return {};
> +

You know already I think it's very weird the IPU3 awb keeps stats as
global variables. But this is not about reworking the algo, so let's
not dwell on it.

>  	/*
>  	 * Make a separate list of the derivatives for each of red and blue, so
>  	 * that we can sort them to exclude the extreme gains. We could
> @@ -399,66 +424,21 @@ void Awb::awbGreyWorld()
>  	double redGain = sumRed.g() / (sumRed.r() + 1),
>  	       blueGain = sumBlue.g() / (sumBlue.b() + 1);
>
> -	/* Color temperature is not relevant in Grey world but still useful to estimate it :-) */
> -	asyncResults_.temperatureK = estimateCCT({{ sumRed.r(), sumRed.g(), sumBlue.b() }});

Could you check your includes, not what you're not calling estimateCCT
you should be able to remove

#include "libipa/colours.h"

> -
> -	/*
> -	 * Gain values are unsigned integer value ranging [0, 8) with 13 bit
> -	 * fractional part.
> -	 */
> -	redGain = std::clamp(redGain, 0.0, 65535.0 / 8192);
> -	blueGain = std::clamp(blueGain, 0.0, 65535.0 / 8192);
> -
> -	asyncResults_.redGain = redGain;
> -	/* Hardcode the green gain to 1.0. */
> -	asyncResults_.greenGain = 1.0;
> -	asyncResults_.blueGain = blueGain;
> -}
> -
> -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> -{
> -	ASSERT(stats->stats_3a_status.awb_en);
> -
> -	clearAwbStats();
> -	generateAwbStats(stats);
> -	generateZones();
> -
> -	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
> -
> -	if (zones_.size() > 10) {
> -		awbGreyWorld();
> -		LOG(IPU3Awb, Debug) << "Gain found for red: " << asyncResults_.redGain
> -				    << " and for blue: " << asyncResults_.blueGain;
> -	}
> +	return Ipu3AwbStats({ { 1.0 / redGain, 1.0, 1.0 / blueGain } });
>  }
>
>  /**
>   * \copydoc libcamera::ipa::Algorithm::process
>   */
>  void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> -		  [[maybe_unused]] IPAFrameContext &frameContext,
> -		  const ipu3_uapi_stats_3a *stats,
> -		  [[maybe_unused]] ControlList &metadata)
> +		  IPAFrameContext &frameContext,
> +		  [[maybe_unused]] const ipu3_uapi_stats_3a *stats,
> +		  ControlList &metadata)
>  {
> -	calculateWBGains(stats);
> +	Ipu3AwbStats awbStats = calculateRgbMeans(stats);
>
> -	/*
> -	 * Gains are only recalculated if enough zones were detected.
> -	 * The results are cached, so if no results were calculated, we set the
> -	 * cached values from asyncResults_ here.
> -	 */
> -	context.activeState.awb.gains.blue = asyncResults_.blueGain;
> -	context.activeState.awb.gains.green = asyncResults_.greenGain;
> -	context.activeState.awb.gains.red = asyncResults_.redGain;
> -	context.activeState.awb.temperatureK = asyncResults_.temperatureK;
> -
> -	metadata.set(controls::AwbEnable, true);
> -	metadata.set(controls::ColourGains, {
> -			static_cast<float>(context.activeState.awb.gains.red),
> -			static_cast<float>(context.activeState.awb.gains.blue)
> -		});
> -	metadata.set(controls::ColourTemperature,
> -		     context.activeState.awb.temperatureK);
> +	awbAlgo_.process(context.activeState.awb, frameContext.awb, awbStats,
> +			 kDefaultLux, metadata);
>  }
>
>  REGISTER_IPA_ALGORITHM(Awb, "Awb")
> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> index 7ec322318dab54ae7c8a647a67a0cf5815a36eb6..3f3996e6c846558acd3902c9fb3cb804cb5b65d4 100644
> --- a/src/ipa/ipu3/algorithms/awb.h
> +++ b/src/ipa/ipu3/algorithms/awb.h
> @@ -13,6 +13,9 @@
>
>  #include <libcamera/geometry.h>
>
> +#include "libipa/awb.h"
> +#include "libipa/fixedpoint.h"
> +
>  #include "libcamera/internal/vector.h"

Is this still needed here ?

>
>  #include "algorithm.h"
> @@ -21,6 +24,8 @@ namespace libcamera {
>
>  namespace ipa::ipu3::algorithms {
>
> +class Ipu3AwbStats;
> +
>  /* Region size for the statistics generation algorithm */
>  static constexpr uint32_t kAwbStatsSizeX = 16;
>  static constexpr uint32_t kAwbStatsSizeY = 12;
> @@ -39,7 +44,11 @@ class Awb : public Algorithm
>  public:
>  	Awb();
>
> +	int init(IPAContext &context, const ValueNode &tuningData) override;
>  	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> +	void queueRequest(IPAContext &context, const uint32_t frame,
> +			  IPAFrameContext &frameContext,
> +			  const ControlList &controls) override;
>  	void prepare(IPAContext &context, const uint32_t frame,
>  		     IPAFrameContext &frameContext,
>  		     ipu3_uapi_params *params) override;
> @@ -49,15 +58,7 @@ public:
>  		     ControlList &metadata) override;
>
>  private:
> -	struct AwbStatus {
> -		double temperatureK;
> -		double redGain;
> -		double greenGain;
> -		double blueGain;
> -	};
> -
> -private:
> -	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> +	Ipu3AwbStats calculateRgbMeans(const ipu3_uapi_stats_3a *stats);
>  	void generateZones();
>  	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
>  	void clearAwbStats();
> @@ -67,7 +68,7 @@ private:
>
>  	std::vector<RGB<double>> zones_;
>  	Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
> -	AwbStatus asyncResults_;
> +	AwbAlgorithm<UQ<3, 13>> awbAlgo_;
>
>  	uint32_t stride_;
>  	uint32_t cellsPerZoneX_;
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 3b22f7917650d9e400d5368c2f890d6b2dc846a0..b35c925d959027c540257e47944047c238f85571 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -109,6 +109,11 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Maximum analogue gain supported with the configured sensor
>   */
>
> +/**
> + * \var IPAActiveState::awb
> + * \brief Active auto-white balance parameters for the IPA
> + */
> +
>  /**
>   * \var IPASessionConfiguration::sensor
>   * \brief Sensor-specific configuration of the IPA
> @@ -123,6 +128,11 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Sensor output resolution
>   */
>
> +/**
> + * \var IPASessionConfiguration::awb
> + * \brief Auto-white balance specific session configuration data
> + */
> +

This will have to be rebased, as I've now dropped this

>  /**
>   * \var IPAActiveState::agc
>   * \brief Context for the Automatic Gain Control algorithm
> @@ -139,26 +149,6 @@ namespace libcamera::ipa::ipu3 {
>   * The gain should be adapted to the sensor specific gain code before applying.
>   */
>
> -/**
> - * \var IPAActiveState::awb
> - * \brief Context for the Automatic White Balance algorithm
> - *
> - * \var IPAActiveState::awb.gains
> - * \brief White balance gains
> - *
> - * \var IPAActiveState::awb.gains.red
> - * \brief White balance gain for R channel
> - *
> - * \var IPAActiveState::awb.gains.green
> - * \brief White balance gain for G channel
> - *
> - * \var IPAActiveState::awb.gains.blue
> - * \brief White balance gain for B channel
> - *
> - * \var IPAActiveState::awb.temperatureK
> - * \brief Estimated color temperature
> - */
> -
>  /**
>   * \var IPAActiveState::toneMapping
>   * \brief Context for ToneMapping and Gamma control
> @@ -187,4 +177,9 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Analogue gain multiplier
>   */
>
> +/**
> + * \var IPAFrameContext::awb
> + * \brief Per-frame auto-white balance parameters for the IPA
> + */
> +
>  } /* namespace libcamera::ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 97fcf06cd4ac9ac6d64c4933fcea80ace0e572df..245cf8b50b270a61df863e314128bede40d30541 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -15,6 +15,7 @@
>  #include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
>
> +#include <libipa/awb.h>
>  #include <libipa/fc_queue.h>
>
>  namespace libcamera {
> @@ -44,6 +45,8 @@ struct IPASessionConfiguration {
>  		utils::Duration lineDuration;
>  		Size size;
>  	} sensor;
> +
> +	ipa::awb::Session awb;
>  };
>
>  struct IPAActiveState {
> @@ -60,15 +63,7 @@ struct IPAActiveState {
>  		uint32_t exposureMode;
>  	} agc;
>
> -	struct {
> -		struct {
> -			double red;
> -			double green;
> -			double blue;
> -		} gains;
> -
> -		double temperatureK;
> -	} awb;
> +	ipa::awb::ActiveState awb;
>
>  	struct {
>  		double gamma;
> @@ -81,6 +76,8 @@ struct IPAFrameContext : public FrameContext {
>  		uint32_t exposure;
>  		double gain;
>  	} sensor;
> +
> +	ipa::awb::FrameContext awb;

All minors, thanks for the work!
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>  };
>
>  struct IPAContext {
>
> --
> 2.43.0
>

Hi Kieran

On 16/06/2026 19:25, Kieran Bingham wrote:
> Quoting Daniel Scally (2026-06-16 07:41:37)
>> Port the IPU3 Awb algorithm to use the new libipa implementation
>> of AwbAlgorithm.
>>
>> In this implementation the awbAlgo_ class member is initialised as
>> UQ<3, 13> following the IPU3 params format documentation.
>>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>>   src/ipa/ipu3/algorithms/agc.cpp |  12 ++-
>>   src/ipa/ipu3/algorithms/awb.cpp | 194 ++++++++++++++++++----------------------
>>   src/ipa/ipu3/algorithms/awb.h   |  21 ++---
>>   src/ipa/ipu3/ipa_context.cpp    |  35 ++++----
>>   src/ipa/ipu3/ipa_context.h      |  15 ++--
>>   5 files changed, 128 insertions(+), 149 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index d6a7036c6504acb106f5c773b529ad80b8349f85..5120b3bcb177c75ebd61d82c9684779f73305499 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -208,9 +208,15 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>>                    ControlList &metadata)
>>   {
>>          Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
>> -       rGain_ = context.activeState.awb.gains.red;
>> -       gGain_ = context.activeState.awb.gains.blue;
>> -       bGain_ = context.activeState.awb.gains.green;
>> +       rGain_ = context.activeState.awb.autoEnabled ?
>> +                context.activeState.awb.automatic.gains[0] :
>> +                context.activeState.awb.manual.gains[0];
>> +       gGain_ = context.activeState.awb.autoEnabled ?
>> +                context.activeState.awb.automatic.gains[1] :
>> +                context.activeState.awb.manual.gains[1];
>> +       bGain_ = context.activeState.awb.autoEnabled ?
>> +                context.activeState.awb.automatic.gains[2] :
>> +                context.activeState.awb.manual.gains[2];
> 
> Can we drop rGain_, gGain_ and bGain_ and change that into an RGB type?
> Perhaps as a patch beforehand.
> 
> 
> Then we can simply do
> 	if (context.activeState.awb.autoEnabled)
> 		gains = context.activeState.awb.automatic.gains;
> 	else
> 		gains = context.activeState.awb.manual.gains;
> 
> But maybe we need to check what those local class members are being
> used for - now I see we're in agc.cpp!
> 
They're actually just used as...

RGB<double> gains{{ rGain_, gGain_, bGain_ }};

So yeah that's probably quite viable.

>>   
>>          /*
>>           * The Agc algorithm needs to know the effective exposure value that was
>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
>> index 613bdbbf51cd127e03906571813260a6857067dc..fa1c70856a3647b188531473e9d9f8b77c747230 100644
>> --- a/src/ipa/ipu3/algorithms/awb.cpp
>> +++ b/src/ipa/ipu3/algorithms/awb.cpp
>> @@ -26,11 +26,40 @@ namespace ipa::ipu3::algorithms {
>>   LOG_DEFINE_CATEGORY(IPU3Awb)
>>   
>>   /*
>> - * When zones are used for the grey world algorithm, they are only considered if
>> - * their average green value is at least 16/255 (after black level subtraction)
>> - * to exclude zones that are too dark and don't provide relevant colour
>> - * information (on the opposite side of the spectrum, saturated regions are
>> - * excluded by the ImgU statistics engine).
>> + * \todo IPU3 doesn't support the Lux algorithm.
>> + */
> 
> That's really easy to add ;-) (can be later)
> 
>> +static constexpr unsigned int kDefaultLux = 500;
>> +
>> +/**
>> + * \brief The IPU3 implementation of AwbStats
>> + */
>> +class Ipu3AwbStats final : public AwbStats
>> +{
>> +public:
>> +       Ipu3AwbStats(){};
>> +       Ipu3AwbStats(const RGB<double> means)
>> +               : AwbStats(means)
>> +       {
>> +       }
>> +
>> +       double minColourValue() const override
>> +       {
>> +               return 0.0;
> 
> is 0.0 a valid number here? I saw Jacopo's series put 0.2 which I
> thought was to prevent some divide by 0. But I'm curious what this would
> be used for anyway, or if any platform would use the same numbers?

It's the minimum average colour for a channel that needs to be met for the statistics to be 
considered valid, and so probably platform specific...but the existing implementation instead 
filters out any stats zones that don't have a green channel level above a particular value (set to 
16)...
> 
>> +       }
>> +};
>> +
>> +/**
>> + * \fn Ipu3AwbStats::Ipu3AwbStats(const RGB<double> means)
>> + * \brief Construct an instance of the class with RGB means
>> + * \param[in] means The mean R, G and B values from the statistics
>> + */
>> +
>> +/*
>> + * Zones are only considered if their average green value is at least
>> + * kMinGreenLevelInZone/255 (after black level subtraction) to exclude zones
>> + * that are too dark and don't provide relevant colour information (on the
>> + * opposite side of the spectrum, saturated regions are excluded by the ImgU
>> + * statistics engine).
>>    */
>>   static constexpr uint32_t kMinGreenLevelInZone = 16;

...which is the purpose of this constant here. So with that filtering, I think that the statistics 
should always be considered valid, since we already dropped zones that are too dim to be useful. I 
think  it's probably better kept as-is, as it should allow the algorithm to keep going even if some 
sectors were so dark that they would otherwise drag the average below the threshold.

Thanks
Dan

>>   
>> @@ -74,26 +103,6 @@ static constexpr uint32_t kMinCellsPerZoneRatio = 255 * 90 / 100;
>>    * \brief Sum of the average blue values of each unsaturated cell in the zone
>>    */
>>   
>> -/**
>> - * \struct Awb::AwbStatus
>> - * \brief AWB parameters calculated
>> - *
>> - * The AwbStatus structure is intended to store the AWB
>> - * parameters calculated by the algorithm
>> - *
>> - * \var AwbStatus::temperatureK
>> - * \brief Color temperature calculated
>> - *
>> - * \var AwbStatus::redGain
>> - * \brief Gain calculated for the red channel
>> - *
>> - * \var AwbStatus::greenGain
>> - * \brief Gain calculated for the green channel
>> - *
>> - * \var AwbStatus::blueGain
>> - * \brief Gain calculated for the blue channel
>> - */
>> -
>>   /* Default settings for Bayer noise reduction replicated from the Kernel */
>>   static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>>          .wb_gains = { 16, 16, 16, 16 },
>> @@ -114,11 +123,7 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>>   
>>   /**
>>    * \class Awb
>> - * \brief A Grey world white balance correction algorithm
>> - *
>> - * The Grey World algorithm assumes that the scene, in average, is neutral grey.
>> - * Reference: Lam, Edmund & Fung, George. (2008). Automatic White Balancing in
>> - * Digital Photography. 10.1201/9781420054538.ch10.
>> + * \brief The IPU3 white balance correction algorithm implementation
>>    *
>>    * The IPU3 generates statistics from the Bayer Down Scaler output into a grid
>>    * defined in the ipu3_uapi_awb_config_s structure.
>> @@ -168,26 +173,26 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
>>    * cells are ignored. The grid configuration is computed by
>>    * IPAIPU3::calculateBdsGrid().
>>    *
>> - * Before calculating the gains, the algorithm aggregates the cell averages for
>> - * each zone in generateAwbStats(). Cells that have a too high ratio of
>> - * saturated pixels are ignored, and only zones that contain enough
>> - * non-saturated cells are then used by the algorithm.
>> - *
>> - * The Grey World algorithm will then estimate the red and blue gains to apply, and
>> - * store the results in the metadata. The green gain is always set to 1.
>> + * Before running the AWB algorithm, we aggregate the cell averages for each
>> + * zone in generateAwbStats(). Cells that have a too high ratio of saturated
>> + * pixels are ignored, and only zones that contain enough non-saturated cells
>> + * are then used by the algorithm.
>>    */
>>   
>>   Awb::Awb()
>>          : Algorithm()
>>   {
>> -       asyncResults_.blueGain = 1.0;
>> -       asyncResults_.greenGain = 1.0;
>> -       asyncResults_.redGain = 1.0;
>> -       asyncResults_.temperatureK = 4500;
>> -
>>          zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);
>>   }
>>   
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::init
>> + */
>> +int Awb::init(IPAContext &context, const ValueNode &tuningData)
>> +{
>> +       return awbAlgo_.init(tuningData, context.ctrlMap);
>> +}
>> +
>>   /**
>>    * \copydoc libcamera::ipa::Algorithm::configure
>>    */
>> @@ -197,12 +202,14 @@ int Awb::configure(IPAContext &context,
>>          const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
>>          stride_ = context.configuration.grid.stride;
>>   
>> +       awbAlgo_.configure(context.activeState.awb, context.configuration.awb);
>> +
>>          cellsPerZoneX_ = std::round(grid.width / static_cast<double>(kAwbStatsSizeX));
>>          cellsPerZoneY_ = std::round(grid.height / static_cast<double>(kAwbStatsSizeY));
>>   
>>          /*
>>           * Configure the minimum proportion of cells counted within a zone
>> -        * for it to be relevant for the grey world algorithm.
>> +        * for it to be used.
>>           * \todo This proportion could be configured.
>>           */
>>          cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * kMaxCellSaturationRatio;
>> @@ -211,6 +218,17 @@ int Awb::configure(IPAContext &context,
>>          return 0;
>>   }
>>   
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::queueRequest
>> + */
>> +void Awb::queueRequest(IPAContext &context, const uint32_t frame,
>> +                      IPAFrameContext &frameContext,
>> +                      const ControlList &controls)
>> +{
>> +       awbAlgo_.queueRequest(context.activeState.awb, frame, frameContext.awb,
>> +                             controls);
>> +}
>> +
>>   constexpr uint16_t Awb::threshold(float value)
>>   {
>>          /* AWB thresholds are in the range [0, 8191] */
>> @@ -237,11 +255,12 @@ constexpr uint16_t Awb::gainValue(double gain)
>>   /**
>>    * \copydoc libcamera::ipa::Algorithm::prepare
>>    */
>> -void Awb::prepare(IPAContext &context,
>> -                 [[maybe_unused]] const uint32_t frame,
>> -                 [[maybe_unused]] IPAFrameContext &frameContext,
>> -                 ipu3_uapi_params *params)
>> +void Awb::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>> +                 IPAFrameContext &frameContext,
>> +                 [[maybe_unused]] ipu3_uapi_params *params)
>>   {
>> +       awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
>> +
>>          /*
>>           * Green saturation thresholds are reduced because we are using the
>>           * green channel only in the exposure computation.
>> @@ -279,13 +298,11 @@ void Awb::prepare(IPAContext &context,
>>          params->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset
>>                                                          * params->acc_param.bnr.opt_center.y_reset;
>>   
>> -       params->acc_param.bnr.wb_gains.gr = gainValue(context.activeState.awb.gains.green);
>> -       params->acc_param.bnr.wb_gains.r  = gainValue(context.activeState.awb.gains.red);
>> -       params->acc_param.bnr.wb_gains.b  = gainValue(context.activeState.awb.gains.blue);
>> -       params->acc_param.bnr.wb_gains.gb = gainValue(context.activeState.awb.gains.green);
>> -
>> -       LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK;
>>   
>> +       params->acc_param.bnr.wb_gains.gr = gainValue(frameContext.awb.gains.g());
>> +       params->acc_param.bnr.wb_gains.r = gainValue(frameContext.awb.gains.r());
>> +       params->acc_param.bnr.wb_gains.b = gainValue(frameContext.awb.gains.b());
>> +       params->acc_param.bnr.wb_gains.gb = gainValue(frameContext.awb.gains.g());
> 
> Coool, now we're doing AWB correctly here!
> 
> A quick look at Awb::gainValue looks like it could be converted to a
> UQ<3,13> I guess, but it would have to be checked and verified, so lets
> keep that as a separate topic, as ther'es that +/- 1.0 which I suspect
> isn't a straight translation...
> 
>>   
>>          params->use.acc_awb = 1;
>>          params->use.acc_bnr = 1;
>> @@ -366,9 +383,17 @@ void Awb::clearAwbStats()
>>          }
>>   }
>>   
>> -void Awb::awbGreyWorld()
>> +Ipu3AwbStats Awb::calculateRgbMeans(const ipu3_uapi_stats_3a *stats)
>>   {
>> -       LOG(IPU3Awb, Debug) << "Grey world AWB";
>> +       ASSERT(stats->stats_3a_status.awb_en);
>> +
>> +       clearAwbStats();
>> +       generateAwbStats(stats);
>> +       generateZones();
>> +
>> +       if (zones_.size() <= 10)
>> +               return {};
>> +
>>          /*
>>           * Make a separate list of the derivatives for each of red and blue, so
>>           * that we can sort them to exclude the extreme gains. We could
>> @@ -399,66 +424,21 @@ void Awb::awbGreyWorld()
>>          double redGain = sumRed.g() / (sumRed.r() + 1),
>>                 blueGain = sumBlue.g() / (sumBlue.b() + 1);
>>   
>> -       /* Color temperature is not relevant in Grey world but still useful to estimate it :-) */
>> -       asyncResults_.temperatureK = estimateCCT({{ sumRed.r(), sumRed.g(), sumBlue.b() }});
>> -
>> -       /*
>> -        * Gain values are unsigned integer value ranging [0, 8) with 13 bit
>> -        * fractional part.
>> -        */
>> -       redGain = std::clamp(redGain, 0.0, 65535.0 / 8192);
>> -       blueGain = std::clamp(blueGain, 0.0, 65535.0 / 8192);
>> -
>> -       asyncResults_.redGain = redGain;
>> -       /* Hardcode the green gain to 1.0. */
>> -       asyncResults_.greenGain = 1.0;
>> -       asyncResults_.blueGain = blueGain;
>> -}
>> -
>> -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
>> -{
>> -       ASSERT(stats->stats_3a_status.awb_en);
>> -
>> -       clearAwbStats();
>> -       generateAwbStats(stats);
>> -       generateZones();
>> -
>> -       LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
>> -
>> -       if (zones_.size() > 10) {
>> -               awbGreyWorld();
>> -               LOG(IPU3Awb, Debug) << "Gain found for red: " << asyncResults_.redGain
>> -                                   << " and for blue: " << asyncResults_.blueGain;
>> -       }
>> +       return Ipu3AwbStats({ { 1.0 / redGain, 1.0, 1.0 / blueGain } });
>>   }
>>   
>>   /**
>>    * \copydoc libcamera::ipa::Algorithm::process
>>    */
>>   void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>> -                 [[maybe_unused]] IPAFrameContext &frameContext,
>> -                 const ipu3_uapi_stats_3a *stats,
>> -                 [[maybe_unused]] ControlList &metadata)
>> +                 IPAFrameContext &frameContext,
>> +                 [[maybe_unused]] const ipu3_uapi_stats_3a *stats,
>> +                 ControlList &metadata)
>>   {
>> -       calculateWBGains(stats);
>> +       Ipu3AwbStats awbStats = calculateRgbMeans(stats);
>>   
>> -       /*
>> -        * Gains are only recalculated if enough zones were detected.
>> -        * The results are cached, so if no results were calculated, we set the
>> -        * cached values from asyncResults_ here.
>> -        */
>> -       context.activeState.awb.gains.blue = asyncResults_.blueGain;
>> -       context.activeState.awb.gains.green = asyncResults_.greenGain;
>> -       context.activeState.awb.gains.red = asyncResults_.redGain;
>> -       context.activeState.awb.temperatureK = asyncResults_.temperatureK;
>> -
>> -       metadata.set(controls::AwbEnable, true);
>> -       metadata.set(controls::ColourGains, {
>> -                       static_cast<float>(context.activeState.awb.gains.red),
>> -                       static_cast<float>(context.activeState.awb.gains.blue)
>> -               });
>> -       metadata.set(controls::ColourTemperature,
>> -                    context.activeState.awb.temperatureK);
>> +       awbAlgo_.process(context.activeState.awb, frameContext.awb, awbStats,
>> +                        kDefaultLux, metadata);
>>   }
>>   
>>   REGISTER_IPA_ALGORITHM(Awb, "Awb")
>> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
>> index 7ec322318dab54ae7c8a647a67a0cf5815a36eb6..3f3996e6c846558acd3902c9fb3cb804cb5b65d4 100644
>> --- a/src/ipa/ipu3/algorithms/awb.h
>> +++ b/src/ipa/ipu3/algorithms/awb.h
>> @@ -13,6 +13,9 @@
>>   
>>   #include <libcamera/geometry.h>
>>   
>> +#include "libipa/awb.h"
>> +#include "libipa/fixedpoint.h"
>> +
>>   #include "libcamera/internal/vector.h"
>>   
>>   #include "algorithm.h"
>> @@ -21,6 +24,8 @@ namespace libcamera {
>>   
>>   namespace ipa::ipu3::algorithms {
>>   
>> +class Ipu3AwbStats;
>> +
>>   /* Region size for the statistics generation algorithm */
>>   static constexpr uint32_t kAwbStatsSizeX = 16;
>>   static constexpr uint32_t kAwbStatsSizeY = 12;
>> @@ -39,7 +44,11 @@ class Awb : public Algorithm
>>   public:
>>          Awb();
>>   
>> +       int init(IPAContext &context, const ValueNode &tuningData) override;
>>          int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>> +       void queueRequest(IPAContext &context, const uint32_t frame,
>> +                         IPAFrameContext &frameContext,
>> +                         const ControlList &controls) override;
>>          void prepare(IPAContext &context, const uint32_t frame,
>>                       IPAFrameContext &frameContext,
>>                       ipu3_uapi_params *params) override;
>> @@ -49,15 +58,7 @@ public:
>>                       ControlList &metadata) override;
>>   
>>   private:
>> -       struct AwbStatus {
>> -               double temperatureK;
>> -               double redGain;
>> -               double greenGain;
>> -               double blueGain;
>> -       };
>> -
>> -private:
>> -       void calculateWBGains(const ipu3_uapi_stats_3a *stats);
>> +       Ipu3AwbStats calculateRgbMeans(const ipu3_uapi_stats_3a *stats);
>>          void generateZones();
>>          void generateAwbStats(const ipu3_uapi_stats_3a *stats);
>>          void clearAwbStats();
>> @@ -67,7 +68,7 @@ private:
>>   
>>          std::vector<RGB<double>> zones_;
>>          Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
>> -       AwbStatus asyncResults_;
>> +       AwbAlgorithm<UQ<3, 13>> awbAlgo_;
>>   
>>          uint32_t stride_;
>>          uint32_t cellsPerZoneX_;
>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>> index 3b22f7917650d9e400d5368c2f890d6b2dc846a0..b35c925d959027c540257e47944047c238f85571 100644
>> --- a/src/ipa/ipu3/ipa_context.cpp
>> +++ b/src/ipa/ipu3/ipa_context.cpp
>> @@ -109,6 +109,11 @@ namespace libcamera::ipa::ipu3 {
>>    * \brief Maximum analogue gain supported with the configured sensor
>>    */
>>   
>> +/**
>> + * \var IPAActiveState::awb
>> + * \brief Active auto-white balance parameters for the IPA
>> + */
>> +
>>   /**
>>    * \var IPASessionConfiguration::sensor
>>    * \brief Sensor-specific configuration of the IPA
>> @@ -123,6 +128,11 @@ namespace libcamera::ipa::ipu3 {
>>    * \brief Sensor output resolution
>>    */
>>   
>> +/**
>> + * \var IPASessionConfiguration::awb
>> + * \brief Auto-white balance specific session configuration data
>> + */
>> +
>>   /**
>>    * \var IPAActiveState::agc
>>    * \brief Context for the Automatic Gain Control algorithm
>> @@ -139,26 +149,6 @@ namespace libcamera::ipa::ipu3 {
>>    * The gain should be adapted to the sensor specific gain code before applying.
>>    */
>>   
>> -/**
>> - * \var IPAActiveState::awb
>> - * \brief Context for the Automatic White Balance algorithm
>> - *
>> - * \var IPAActiveState::awb.gains
>> - * \brief White balance gains
>> - *
>> - * \var IPAActiveState::awb.gains.red
>> - * \brief White balance gain for R channel
>> - *
>> - * \var IPAActiveState::awb.gains.green
>> - * \brief White balance gain for G channel
>> - *
>> - * \var IPAActiveState::awb.gains.blue
>> - * \brief White balance gain for B channel
>> - *
>> - * \var IPAActiveState::awb.temperatureK
>> - * \brief Estimated color temperature
>> - */
>> -
>>   /**
>>    * \var IPAActiveState::toneMapping
>>    * \brief Context for ToneMapping and Gamma control
>> @@ -187,4 +177,9 @@ namespace libcamera::ipa::ipu3 {
>>    * \brief Analogue gain multiplier
>>    */
>>   
>> +/**
>> + * \var IPAFrameContext::awb
>> + * \brief Per-frame auto-white balance parameters for the IPA
>> + */
>> +
>>   } /* namespace libcamera::ipa::ipu3 */
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index 97fcf06cd4ac9ac6d64c4933fcea80ace0e572df..245cf8b50b270a61df863e314128bede40d30541 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -15,6 +15,7 @@
>>   #include <libcamera/controls.h>
>>   #include <libcamera/geometry.h>
>>   
>> +#include <libipa/awb.h>
>>   #include <libipa/fc_queue.h>
>>   
>>   namespace libcamera {
>> @@ -44,6 +45,8 @@ struct IPASessionConfiguration {
>>                  utils::Duration lineDuration;
>>                  Size size;
>>          } sensor;
>> +
>> +       ipa::awb::Session awb;
>>   };
>>   
>>   struct IPAActiveState {
>> @@ -60,15 +63,7 @@ struct IPAActiveState {
>>                  uint32_t exposureMode;
>>          } agc;
>>   
>> -       struct {
>> -               struct {
>> -                       double red;
>> -                       double green;
>> -                       double blue;
>> -               } gains;
>> -
>> -               double temperatureK;
>> -       } awb;
>> +       ipa::awb::ActiveState awb;
> 
> I love all this simplification ;-)
> 
> Overall - I think we're going in the right direction so:
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>>   
>>          struct {
>>                  double gamma;
>> @@ -81,6 +76,8 @@ struct IPAFrameContext : public FrameContext {
>>                  uint32_t exposure;
>>                  double gain;
>>          } sensor;
>> +
>> +       ipa::awb::FrameContext awb;
>>   };
>>   
>>   struct IPAContext {
>>
>> -- 
>> 2.43.0
>>