[8/9] libipa: agc_mean_luminance: Add exposure compensation support
diff mbox series

Message ID 20250331144352.736700-9-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Wdr preparations
Related show

Commit Message

Stefan Klug March 31, 2025, 2:43 p.m. UTC
Exposure compensation allows to over- or under-expose an
image by a given value (typically provided in EV). Add support
for that in agc_mean_luminance.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/libipa/agc_mean_luminance.cpp | 23 +++++++++++++++++++++--
 src/ipa/libipa/agc_mean_luminance.h   |  6 ++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

Comments

Stefan Klug March 31, 2025, 4:22 p.m. UTC | #1
Hi all,

I made a mistake in last-minute-variable-sorting and forgot to compile
:-/. Will be added in v2.

This patch needs a fixup:
diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index a498b646bdd5..f6cb7144b31a 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -143,7 +143,7 @@ static constexpr double kMaxRelativeLuminanceTarget = 0.95;
  */

 AgcMeanLuminance::AgcMeanLuminance()
-       : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0)
+       : exposureCompensation_(1.0), frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
 {
 }

Best regards,
Stefan


On Mon, Mar 31, 2025 at 04:43:47PM +0200, Stefan Klug wrote:
> Exposure compensation allows to over- or under-expose an
> image by a given value (typically provided in EV). Add support
> for that in agc_mean_luminance.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  src/ipa/libipa/agc_mean_luminance.cpp | 23 +++++++++++++++++++++--
>  src/ipa/libipa/agc_mean_luminance.h   |  6 ++++++
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 9154f083a510..a498b646bdd5 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -44,6 +44,15 @@ static constexpr uint32_t kNumStartupFrames = 10;
>   */
>  static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>  
> +/*
> + * Maximum relative luminance target
> + *
> + * This value limits the relative luminance target after applying the exposure
> + * compensation. Targeting a value above this limit results in saturation
> + * and the inability to regulate properly.
> + */
> +static constexpr double kMaxRelativeLuminanceTarget = 0.95;
> +
>  /**
>   * \struct AgcMeanLuminance::AgcConstraint
>   * \brief The boundaries and target for an AeConstraintMode constraint
> @@ -134,7 +143,7 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>   */
>  
>  AgcMeanLuminance::AgcMeanLuminance()
> -	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
> +	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0)
>  {
>  }
>  
> @@ -369,6 +378,15 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)
>  	return parseExposureModes(tuningData);
>  }
>  
> +/**
> + * \fn AgcMeanLuminance::setExposureCompensation()
> + * \brief Set the exposure compensation value
> + * \param[in] gain The exposure compensation gain
> + *
> + * This function sets the exposure compensation value to be used in the
> + * AGC calculations. It is expressed as gain instead of EV.
> + */
> +
>  /**
>   * \brief Set the ExposureModeHelper limits for this class
>   * \param[in] minExposureTime Minimum exposure time to allow
> @@ -425,7 +443,8 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
>   */
>  double AgcMeanLuminance::estimateInitialGain() const
>  {
> -	double yTarget = relativeLuminanceTarget_;
> +	double yTarget = std::min(relativeLuminanceTarget_ * exposureCompensation_,
> +				  kMaxRelativeLuminanceTarget);
>  	double yGain = 1.0;
>  
>  	/*
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index c41391cb0b73..cad7ef845487 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -44,6 +44,11 @@ public:
>  
>  	int parseTuningData(const YamlObject &tuningData);
>  
> +	void setExposureCompensation(double gain)
> +	{
> +		exposureCompensation_ = gain;
> +	}
> +
>  	void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
>  		       double minGain, double maxGain);
>  
> @@ -84,6 +89,7 @@ private:
>  				   double gain);
>  	utils::Duration filterExposure(utils::Duration exposureValue);
>  
> +	double exposureCompensation_;
>  	uint64_t frameCount_;
>  	utils::Duration filteredExposure_;
>  	double relativeLuminanceTarget_;
> -- 
> 2.43.0
>
Dan Scally March 31, 2025, 10:08 p.m. UTC | #2
Hi Stefan

On 31/03/2025 15:43, Stefan Klug wrote:
> Exposure compensation allows to over- or under-expose an
> image by a given value (typically provided in EV). Add support


Given you're implementing it as gain instead, I would either drop "typically provided in EV" or 
explain that you're not doing so.

> for that in agc_mean_luminance.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>   src/ipa/libipa/agc_mean_luminance.cpp | 23 +++++++++++++++++++++--
>   src/ipa/libipa/agc_mean_luminance.h   |  6 ++++++
>   2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 9154f083a510..a498b646bdd5 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -44,6 +44,15 @@ static constexpr uint32_t kNumStartupFrames = 10;
>    */
>   static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>   
> +/*
> + * Maximum relative luminance target
> + *
> + * This value limits the relative luminance target after applying the exposure
> + * compensation. Targeting a value above this limit results in saturation
> + * and the inability to regulate properly.
> + */
> +static constexpr double kMaxRelativeLuminanceTarget = 0.95;
> +
>   /**
>    * \struct AgcMeanLuminance::AgcConstraint
>    * \brief The boundaries and target for an AeConstraintMode constraint
> @@ -134,7 +143,7 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>    */
>   
>   AgcMeanLuminance::AgcMeanLuminance()
> -	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
> +	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0)
>   {
>   }
>   
> @@ -369,6 +378,15 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)
>   	return parseExposureModes(tuningData);
>   }
>   
> +/**
> + * \fn AgcMeanLuminance::setExposureCompensation()
> + * \brief Set the exposure compensation value
> + * \param[in] gain The exposure compensation gain
> + *
> + * This function sets the exposure compensation value to be used in the
> + * AGC calculations. It is expressed as gain instead of EV.
> + */
> +
>   /**
>    * \brief Set the ExposureModeHelper limits for this class
>    * \param[in] minExposureTime Minimum exposure time to allow
> @@ -425,7 +443,8 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
>    */
>   double AgcMeanLuminance::estimateInitialGain() const
>   {
> -	double yTarget = relativeLuminanceTarget_;
> +	double yTarget = std::min(relativeLuminanceTarget_ * exposureCompensation_,
> +				  kMaxRelativeLuminanceTarget);


The capping is really a separate change to the one described in the commit message...I would 
personally split them into separate commits, but failing that it ought to be described in the commit 
message. Either way (and if you split, keep the tag for both):


Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   	double yGain = 1.0;
>   
>   	/*
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index c41391cb0b73..cad7ef845487 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -44,6 +44,11 @@ public:
>   
>   	int parseTuningData(const YamlObject &tuningData);
>   
> +	void setExposureCompensation(double gain)
> +	{
> +		exposureCompensation_ = gain;
> +	}
> +
>   	void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
>   		       double minGain, double maxGain);
>   
> @@ -84,6 +89,7 @@ private:
>   				   double gain);
>   	utils::Duration filterExposure(utils::Duration exposureValue);
>   
> +	double exposureCompensation_;
>   	uint64_t frameCount_;
>   	utils::Duration filteredExposure_;
>   	double relativeLuminanceTarget_;

Patch
diff mbox series

diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index 9154f083a510..a498b646bdd5 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -44,6 +44,15 @@  static constexpr uint32_t kNumStartupFrames = 10;
  */
 static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
 
+/*
+ * Maximum relative luminance target
+ *
+ * This value limits the relative luminance target after applying the exposure
+ * compensation. Targeting a value above this limit results in saturation
+ * and the inability to regulate properly.
+ */
+static constexpr double kMaxRelativeLuminanceTarget = 0.95;
+
 /**
  * \struct AgcMeanLuminance::AgcConstraint
  * \brief The boundaries and target for an AeConstraintMode constraint
@@ -134,7 +143,7 @@  static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
  */
 
 AgcMeanLuminance::AgcMeanLuminance()
-	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
+	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0)
 {
 }
 
@@ -369,6 +378,15 @@  int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)
 	return parseExposureModes(tuningData);
 }
 
+/**
+ * \fn AgcMeanLuminance::setExposureCompensation()
+ * \brief Set the exposure compensation value
+ * \param[in] gain The exposure compensation gain
+ *
+ * This function sets the exposure compensation value to be used in the
+ * AGC calculations. It is expressed as gain instead of EV.
+ */
+
 /**
  * \brief Set the ExposureModeHelper limits for this class
  * \param[in] minExposureTime Minimum exposure time to allow
@@ -425,7 +443,8 @@  void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
  */
 double AgcMeanLuminance::estimateInitialGain() const
 {
-	double yTarget = relativeLuminanceTarget_;
+	double yTarget = std::min(relativeLuminanceTarget_ * exposureCompensation_,
+				  kMaxRelativeLuminanceTarget);
 	double yGain = 1.0;
 
 	/*
diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
index c41391cb0b73..cad7ef845487 100644
--- a/src/ipa/libipa/agc_mean_luminance.h
+++ b/src/ipa/libipa/agc_mean_luminance.h
@@ -44,6 +44,11 @@  public:
 
 	int parseTuningData(const YamlObject &tuningData);
 
+	void setExposureCompensation(double gain)
+	{
+		exposureCompensation_ = gain;
+	}
+
 	void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
 		       double minGain, double maxGain);
 
@@ -84,6 +89,7 @@  private:
 				   double gain);
 	utils::Duration filterExposure(utils::Duration exposureValue);
 
+	double exposureCompensation_;
 	uint64_t frameCount_;
 	utils::Duration filteredExposure_;
 	double relativeLuminanceTarget_;