[PATCH v3] ipa: libipa: Change constraint Y target to piecewise linear function

[Paul Elder]

On Mon, Jun 10, 2024 at 11:31:32AM +0100, Kieran Bingham wrote:
> Quoting Paul Elder (2024-06-07 08:59:14)
> > Change the constraint luminance target from a scalar value to a
> > piecewise linear function that needs to be sampled by the estimated lux
> > value.
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > 
> > ---
> > This patch depends on v5 of "ipa:: Move Pwl from Raspberry Pi"
> > 
> > Changes in v3:
> > - use new PointF that's a typedef of Vector
> > 
> > Changes in v2:
> > - s/FPoint/PointF/
> > - construct default Pwl with *two* points so that it actually constructs
> >   properly
> > ---
> >  src/ipa/libipa/agc_mean_luminance.cpp | 15 +++++++++------
> >  src/ipa/libipa/agc_mean_luminance.h   |  4 ++--
> >  2 files changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> > index 10c16850d..575143610 100644
> > --- a/src/ipa/libipa/agc_mean_luminance.cpp
> > +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> > @@ -168,8 +168,10 @@ void AgcMeanLuminance::parseConstraint(const YamlObject &modeDict, int32_t id)
> >                 AgcConstraint::Bound bound = static_cast<AgcConstraint::Bound>(idx);
> >                 double qLo = content["qLo"].get<double>().value_or(0.98);
> >                 double qHi = content["qHi"].get<double>().value_or(1.0);
> > -               double yTarget =
> > -                       content["yTarget"].getList<double>().value_or(std::vector<double>{ 0.5 }).at(0);
> > +               Pwl yTarget;
> > +               int ret = yTarget.readYaml(content["yTarget"]);
> > +               if (ret < 0)
> > +                       yTarget = Pwl({ Pwl::PointF({ 0, 0.5 }), Pwl::PointF({ 1000, 0.5 }) });
> >  
> >                 AgcConstraint constraint = { bound, qLo, qHi, yTarget };
> >  
> > @@ -221,7 +223,7 @@ int AgcMeanLuminance::parseConstraintModes(const YamlObject &tuningData)
> >                         AgcConstraint::Bound::lower,
> >                         0.98,
> >                         1.0,
> > -                       0.5
> > +                       Pwl({ Pwl::PointF({ 0, 0.5 }), Pwl::PointF({ 1000, 0.5 }) })
> 
> In V2, I asked:
> 
> > Should this be some pre-constructed global const if it's going to be
> > used in multiple places?
>     
> And you replied:
> 
> Probably? It's used three times; twice here and once more for
> relativeLuminance.
> 
> But I don't see that action taken for v3?

It's split between two patches and I don't really want to introduce
another depencency at this stage...


Paul

> 
> If you do so, it would be a good opportunity to define and document the
> value choices for 0,0.5 -> 1000,0.5 and state clearly that this is a
> single horizontal linear default Pwl used to express a mid-point
> brightness level regardless of the input colour temperature?
> 
> 
> 
> 
> >                 };
> >  
> >                 constraintModes_[controls::ConstraintNormal].insert(
> > @@ -471,16 +473,17 @@ double AgcMeanLuminance::estimateInitialGain(double lux) const
> >   * \param[in] constraintModeIndex The index of the constraint to adhere to
> >   * \param[in] hist A histogram over which to calculate inter-quantile means
> >   * \param[in] gain The gain to clamp
> > + * \param[in] lux The lux value at which to sample the constraint luminance target pwl
> >   *
> >   * \return The gain clamped within the constraint bounds
> >   */
> >  double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex,
> >                                              const Histogram &hist,
> > -                                            double gain)
> > +                                            double gain, double lux)
> >  {
> >         std::vector<AgcConstraint> &constraints = constraintModes_[constraintModeIndex];
> >         for (const AgcConstraint &constraint : constraints) {
> > -               double newGain = constraint.yTarget * hist.bins() /
> > +               double newGain = constraint.yTarget.eval(constraint.yTarget.domain().clamp(lux)) * hist.bins() /
> >                                  hist.interQuantileMean(constraint.qLo, constraint.qHi);
> >  
> >                 if (constraint.bound == AgcConstraint::Bound::lower &&
> > @@ -559,7 +562,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,
> >                 exposureModeHelpers_.at(exposureModeIndex);
> >  
> >         double gain = estimateInitialGain(lux);
> > -       gain = constraintClampGain(constraintModeIndex, yHist, gain);
> > +       gain = constraintClampGain(constraintModeIndex, yHist, gain, lux);
> >  
> >         /*
> >          * We don't check whether we're already close to the target, because
> > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> > index 6ec2a0dc9..d43f673dd 100644
> > --- a/src/ipa/libipa/agc_mean_luminance.h
> > +++ b/src/ipa/libipa/agc_mean_luminance.h
> > @@ -38,7 +38,7 @@ public:
> >                 Bound bound;
> >                 double qLo;
> >                 double qHi;
> > -               double yTarget;
> > +               Pwl yTarget;
> >         };
> >  
> >         int parseTuningData(const YamlObject &tuningData);
> > @@ -80,7 +80,7 @@ private:
> >         double estimateInitialGain(double lux) const;
> >         double constraintClampGain(uint32_t constraintModeIndex,
> >                                    const Histogram &hist,
> > -                                  double gain);
> > +                                  double gain, double lux);
> >         utils::Duration filterExposure(utils::Duration exposureValue);
> >  
> >         uint64_t frameCount_;
> > -- 
> > 2.39.2
> >