[PATCH 2/2] ipa: rpi: awb: Add a bias to the AWB search

[David Plowman]

Hi Naush

Thanks for this! I think it's an elegant solution, where failure cases
tend in the limit to the "default", rather than having hard cut-offs
that are then difficult to manage.

On Mon, 30 Sept 2024 at 09:40, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> In the case of an AWB search failure, the current algorithm logic will
> return a point on the CT curve closest to where the search finisned.
> This can be quite undesirable. Instead, add some bias params to the AWB
> algorithm which will direct the search to a set CT value in the case
> where statistics become unreliable causing the search to fail.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/ipa/rpi/controller/rpi/awb.cpp | 17 +++++++++++++++--
>  src/ipa/rpi/controller/rpi/awb.h   |  4 ++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
> index 24f296fc66fa..8f46dae0a961 100644
> --- a/src/ipa/rpi/controller/rpi/awb.cpp
> +++ b/src/ipa/rpi/controller/rpi/awb.cpp
> @@ -169,6 +169,8 @@ int AwbConfig::read(const libcamera::YamlObject &params)
>         whitepointB = params["whitepoint_b"].get<double>(0.0);
>         if (bayes == false)
>                 sensitivityR = sensitivityB = 1.0; /* nor do sensitivities make any sense */
> +       biasProportion = params["bias_proportion"].get<double>(0.0);

I wonder if some comment somewhere that suggests a reasonable bias
proportion might be helpful? I know it should go in our official
documentation, which is probably long overdue an update, but it might
not hurt to put something here in the meantime?

> +       biasCT = params["bias_ct"].get<double>(DefaultCT);
>         return 0;
>  }
>
> @@ -409,7 +411,8 @@ void Awb::asyncFunc()
>
>  static void generateStats(std::vector<Awb::RGB> &zones,
>                           StatisticsPtr &stats, double minPixels,
> -                         double minG, Metadata &globalMetadata)
> +                         double minG, Metadata &globalMetadata,
> +                         double biasProportion, double biasCtR, double biasCtB)
>  {
>         std::scoped_lock<RPiController::Metadata> l(globalMetadata);
>
> @@ -422,6 +425,14 @@ static void generateStats(std::vector<Awb::RGB> &zones,
>                                 continue;
>                         zone.R = region.val.rSum / region.counted;
>                         zone.B = region.val.bSum / region.counted;
> +                       /*
> +                        * Add some bias samples to allow the search to tend to a
> +                        * bias CT in failure cases.
> +                        */
> +                       const unsigned int proportion = biasProportion * region.counted;

Being perhaps overly paranoid, is there a risk (probably on a vc4
platform rather than a PiSP one), that region.counted could be zero?
In which case the failure avoidance going on here would itself fail.
Maybe ensure proportion is at least 1?

But these minor quibbles notwithstanding:

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Now all we need to do is think about the other hard cut-off for the
number of valid zones... :)

David

> +                       zone.R += proportion * biasCtR;
> +                       zone.B += proportion * biasCtB;
> +                       zone.G += proportion * 1.0;
>                         /* Factor in the ALSC applied colour shading correction if required. */
>                         const AlscStatus *alscStatus = globalMetadata.getLocked<AlscStatus>("alsc.status");
>                         if (stats->colourStatsPos == Statistics::ColourStatsPos::PreLsc && alscStatus) {
> @@ -442,7 +453,9 @@ void Awb::prepareStats()
>          * any LSC compensation.  We also ignore config_.fast in this version.
>          */
>         generateStats(zones_, statistics_, config_.minPixels,
> -                     config_.minG, getGlobalMetadata());
> +                     config_.minG, getGlobalMetadata(),
> +                     config_.biasProportion, config_.ctR.eval(config_.biasCT),
> +                     config_.ctB.eval(config_.biasCT));
>         /*
>          * apply sensitivities, so values appear to come from our "canonical"
>          * sensor.
> diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
> index ab30f4fa51c1..5d628b47c8a6 100644
> --- a/src/ipa/rpi/controller/rpi/awb.h
> +++ b/src/ipa/rpi/controller/rpi/awb.h
> @@ -87,6 +87,10 @@ struct AwbConfig {
>         double whitepointR;
>         double whitepointB;
>         bool bayes; /* use Bayesian algorithm */
> +       /* proportion of counted samples to add for the search bias */
> +       double biasProportion;
> +       /* CT target for the search bias */
> +       double biasCT;
>  };
>
>  class Awb : public AwbAlgorithm
> --
> 2.34.1
>