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

[David Plowman]

On Tue, 1 Oct 2024 at 11:28, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi David,
>
> On Mon, 30 Sept 2024 at 16:14, David Plowman
> <david.plowman at raspberrypi.com> wrote:
> >
> > 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?
>
> Sure, will do, but see below...
>
> >
> > > +       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... :)
>
> I'm half wondering whether we change things so we use the proportion
> of the total region pixel area rather than the proportion of the
> counted pixels?  This would fix the problem on vc4 where
> region.counted might be 0, and probably give us enough to not need to
> bother with the hard cut-off?

Hmm, yes. I guess on Pi 5 "counted" happens to give us everything,
whereas on a Pi 4 it probably doesn't. Do we have a way to get the
total number of all pixels in a portable way? My vague recollection is
that vc4 has an "uncounted" field, but without looking it up I'm not
too sure...

David

>
> Naush
>
>
> >
> > 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
> > >