[v2] ipa: libipa: Change constraint Y target to piecewise linear function
diff mbox series

Message ID 20240529193322.835594-1-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • [v2] ipa: libipa: Change constraint Y target to piecewise linear function
Related show

Commit Message

Paul Elder May 29, 2024, 7:33 p.m. UTC 
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@ideasonboard.com>

---
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(-)

Comments

Paul Elder May 29, 2024, 7:41 p.m. UTC | #1 
On Thu, May 30, 2024 at 04:33:22AM +0900, Paul Elder wrote:
> 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@ideasonboard.com>
> 
> ---

I forgot to mention that this patch also depends on v3 of the series
"ipa: Move Pwl from Raspberry Pi"


Paul

> 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 5b79d5d59..5eaa86c82 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({ PointF(0, 0.5), 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({ PointF(0, 0.5), PointF(1000, 0.5) })
>  		};
>  
>  		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
>
Kieran Bingham June 3, 2024, 12:24 p.m. UTC | #2 
Quoting Paul Elder (2024-05-29 20:33:22)
> 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@ideasonboard.com>
> 
> ---
> 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 5b79d5d59..5eaa86c82 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({ PointF(0, 0.5), PointF(1000, 0.5) });

Is the 'length' of the Pwl arbitrary? Does it extend if we go above
'1000' here? From this snipped/hunk - I don't know what units/reference
0 and 1000 are? Is it the lux value?

Can any reasoning for 0...1000 be documented? I assume 0.5 is just 'half
brightness target' if that's sufficiently self explanatory then fine -
but even that might warrant a couple of words in why it's chosen?

Googling lux levels tells me:

- Direct Sunlight	32,000 to 100,000
- Ambient Daylight	10,000 to 25,000
- Overcast Daylight	1000
- Sunset & Sunrise	400

Does that impact the choice of '1000' here? As the two points make a
linear that will always return 0.5 - I assume it doesn't matter here,
and

  Pwl({ PointF(0, 0.5), PointF(1, 0.5) });

would also have been equivalent ?

>  
>                 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({ PointF(0, 0.5), PointF(1000, 0.5) })

Should this be some pre-constructed global const if it's going to be
used in multiple places?


>                 };
>  
>                 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
>
Paul Elder June 5, 2024, 9:59 a.m. UTC | #3 
On Mon, Jun 03, 2024 at 01:24:18PM +0100, Kieran Bingham wrote:
> Quoting Paul Elder (2024-05-29 20:33:22)
> > 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@ideasonboard.com>
> > 
> > ---
> > 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 5b79d5d59..5eaa86c82 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({ PointF(0, 0.5), PointF(1000, 0.5) });
> 
> Is the 'length' of the Pwl arbitrary? Does it extend if we go above
> '1000' here? From this snipped/hunk - I don't know what units/reference
> 0 and 1000 are? Is it the lux value?
> 
> Can any reasoning for 0...1000 be documented? I assume 0.5 is just 'half
> brightness target' if that's sufficiently self explanatory then fine -
> but even that might warrant a couple of words in why it's chosen?
> 
> Googling lux levels tells me:
> 
> - Direct Sunlight	32,000 to 100,000
> - Ambient Daylight	10,000 to 25,000
> - Overcast Daylight	1000
> - Sunset & Sunrise	400
> 
> Does that impact the choice of '1000' here? As the two points make a
> linear that will always return 0.5 - I assume it doesn't matter here,
> and
> 
>   Pwl({ PointF(0, 0.5), PointF(1, 0.5) });
> 
> would also have been equivalent ?

Yes it would be. Since this is a constant function, the input doesn't
matter and extrapolation and clamping are equivalent.

> 
> >  
> >                 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({ PointF(0, 0.5), PointF(1000, 0.5) })
> 
> Should this be some pre-constructed global const if it's going to be
> used in multiple places?

Probably? It's used three times; twice here and once more for
relativeLuminance.


Paul

> 
> 
> >                 };
> >  
> >                 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
> >

Patch
diff mbox series 

diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index 5b79d5d59..5eaa86c82 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({ PointF(0, 0.5), 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({ PointF(0, 0.5), PointF(1000, 0.5) })
 		};
 
 		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_;