[libcamera-devel,v4,09/14] ipa: ipu3: agc: Use exposure in time for storage
diff mbox series

Message ID 20211111140928.136111-10-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • IPA: IPU3: Introduce per-frame controls
Related show

Commit Message

Jean-Michel Hautbois Nov. 11, 2021, 2:09 p.m. UTC 
The minimum and maximum exposure are stored in lines. Replace it by
values in time to simplify the calculations.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp | 21 +++++++++------------
 src/ipa/ipu3/algorithms/agc.h   |  4 ++--
 2 files changed, 11 insertions(+), 14 deletions(-)

Comments

Umang Jain Nov. 12, 2021, 9:40 a.m. UTC | #1 
Hi JM,

On 11/11/21 7:39 PM, Jean-Michel Hautbois wrote:
> The minimum and maximum exposure are stored in lines. Replace it by
> values in time to simplify the calculations.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   src/ipa/ipu3/algorithms/agc.cpp | 21 +++++++++------------
>   src/ipa/ipu3/algorithms/agc.h   |  4 ++--
>   2 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index d736f21c..86b71886 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -79,8 +79,8 @@ static constexpr uint32_t kMaxLuminance = 255;
>   static constexpr double kNormalizedLumaTarget = 0.16;
>   
>   Agc::Agc()
> -	: frameCount_(0), iqMean_(0.0), lineDuration_(0s), minExposureLines_(0),
> -	  maxExposureLines_(0), filteredExposure_(0s), currentExposure_(0s)
> +	: frameCount_(0), iqMean_(0.0), lineDuration_(0s), minShutterSpeed_(0s),
> +	  maxShutterSpeed_(0s), filteredExposure_(0s), currentExposure_(0s)
>   {
>   }
>   
> @@ -99,17 +99,16 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>   	lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
>   		      / configInfo.sensorInfo.pixelRate;
>   
> -	/* \todo replace the exposure in lines storage with time based ones. */
> -	minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_;
> -	maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_,
> -				     kMaxShutterSpeed / lineDuration_);
> +	minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
> +	maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
> +				    kMaxShutterSpeed);
>   
>   	minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
>   	maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>   
>   	/* Configure the default exposure and gain. */
>   	context.frameContext.agc.gain = minAnalogueGain_;
> -	context.frameContext.agc.exposure = minExposureLines_;
> +	context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
>   
>   	return 0;
>   }
> @@ -242,11 +241,9 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double currentYGain)
>   	 * exposure value applied multiplied by the new estimated gain.
>   	 */
>   	currentExposure_ = effectiveExposureValue * evGain;
> -	utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;
> -	utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;
>   
>   	/* Clamp the exposure value to the min and max authorized */
> -	utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;
> +	utils::Duration maxTotalExposure = maxShutterSpeed_ * maxAnalogueGain_;
>   	currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>   	LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
>   			    << ", maximum is " << maxTotalExposure;
> @@ -256,14 +253,14 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double currentYGain)
>   
>   	/* Divide the exposure value as new exposure and gain values */
>   	utils::Duration exposureValue = filteredExposure_;
> -	utils::Duration shutterTime = minShutterSpeed;
> +	utils::Duration shutterTime;
>   
>   	/*
>   	* Push the shutter time up to the maximum first, and only then
>   	* increase the gain.
>   	*/
>   	shutterTime = std::clamp<utils::Duration>(exposureValue / minAnalogueGain_,
> -						  minShutterSpeed, maxShutterSpeed);
> +						  minShutterSpeed_, maxShutterSpeed_);
>   	double stepGain = std::clamp(exposureValue / shutterTime,
>   				     minAnalogueGain_, maxAnalogueGain_);
>   	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 1d085b40..f14d56b7 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -46,8 +46,8 @@ private:
>   	double iqMean_;
>   
>   	utils::Duration lineDuration_;
> -	uint32_t minExposureLines_;
> -	uint32_t maxExposureLines_;
> +	utils::Duration minShutterSpeed_;
> +	utils::Duration maxShutterSpeed_;
>   
>   	double minAnalogueGain_;
>   	double maxAnalogueGain_;
Paul Elder Nov. 12, 2021, 11:18 p.m. UTC | #2 
Hi Jean-Michel,

imo the subject could be a bit more clear:

"Store exposure in units of time"

On Thu, Nov 11, 2021 at 03:09:23PM +0100, Jean-Michel Hautbois wrote:
> The minimum and maximum exposure are stored in lines. Replace it by
> values in time to simplify the calculations.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 21 +++++++++------------
>  src/ipa/ipu3/algorithms/agc.h   |  4 ++--
>  2 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index d736f21c..86b71886 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -79,8 +79,8 @@ static constexpr uint32_t kMaxLuminance = 255;
>  static constexpr double kNormalizedLumaTarget = 0.16;
>  
>  Agc::Agc()
> -	: frameCount_(0), iqMean_(0.0), lineDuration_(0s), minExposureLines_(0),
> -	  maxExposureLines_(0), filteredExposure_(0s), currentExposure_(0s)
> +	: frameCount_(0), iqMean_(0.0), lineDuration_(0s), minShutterSpeed_(0s),
> +	  maxShutterSpeed_(0s), filteredExposure_(0s), currentExposure_(0s)

To me it sounds a bit strange that speed is measured in seconds, but oh
well :p

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

>  {
>  }
>  
> @@ -99,17 +99,16 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>  	lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
>  		      / configInfo.sensorInfo.pixelRate;
>  
> -	/* \todo replace the exposure in lines storage with time based ones. */
> -	minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_;
> -	maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_,
> -				     kMaxShutterSpeed / lineDuration_);
> +	minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
> +	maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
> +				    kMaxShutterSpeed);
>  
>  	minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
>  	maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>  
>  	/* Configure the default exposure and gain. */
>  	context.frameContext.agc.gain = minAnalogueGain_;
> -	context.frameContext.agc.exposure = minExposureLines_;
> +	context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
>  
>  	return 0;
>  }
> @@ -242,11 +241,9 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double currentYGain)
>  	 * exposure value applied multiplied by the new estimated gain.
>  	 */
>  	currentExposure_ = effectiveExposureValue * evGain;
> -	utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;
> -	utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;
>  
>  	/* Clamp the exposure value to the min and max authorized */
> -	utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;
> +	utils::Duration maxTotalExposure = maxShutterSpeed_ * maxAnalogueGain_;
>  	currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>  	LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
>  			    << ", maximum is " << maxTotalExposure;
> @@ -256,14 +253,14 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double currentYGain)
>  
>  	/* Divide the exposure value as new exposure and gain values */
>  	utils::Duration exposureValue = filteredExposure_;
> -	utils::Duration shutterTime = minShutterSpeed;
> +	utils::Duration shutterTime;
>  
>  	/*
>  	* Push the shutter time up to the maximum first, and only then
>  	* increase the gain.
>  	*/
>  	shutterTime = std::clamp<utils::Duration>(exposureValue / minAnalogueGain_,
> -						  minShutterSpeed, maxShutterSpeed);
> +						  minShutterSpeed_, maxShutterSpeed_);
>  	double stepGain = std::clamp(exposureValue / shutterTime,
>  				     minAnalogueGain_, maxAnalogueGain_);
>  	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 1d085b40..f14d56b7 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -46,8 +46,8 @@ private:
>  	double iqMean_;
>  
>  	utils::Duration lineDuration_;
> -	uint32_t minExposureLines_;
> -	uint32_t maxExposureLines_;
> +	utils::Duration minShutterSpeed_;
> +	utils::Duration maxShutterSpeed_;
>  
>  	double minAnalogueGain_;
>  	double maxAnalogueGain_;
> -- 
> 2.32.0
>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index d736f21c..86b71886 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -79,8 +79,8 @@  static constexpr uint32_t kMaxLuminance = 255;
 static constexpr double kNormalizedLumaTarget = 0.16;
 
 Agc::Agc()
-	: frameCount_(0), iqMean_(0.0), lineDuration_(0s), minExposureLines_(0),
-	  maxExposureLines_(0), filteredExposure_(0s), currentExposure_(0s)
+	: frameCount_(0), iqMean_(0.0), lineDuration_(0s), minShutterSpeed_(0s),
+	  maxShutterSpeed_(0s), filteredExposure_(0s), currentExposure_(0s)
 {
 }
 
@@ -99,17 +99,16 @@  int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
 	lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
 		      / configInfo.sensorInfo.pixelRate;
 
-	/* \todo replace the exposure in lines storage with time based ones. */
-	minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_;
-	maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_,
-				     kMaxShutterSpeed / lineDuration_);
+	minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
+	maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
+				    kMaxShutterSpeed);
 
 	minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
 	maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
 
 	/* Configure the default exposure and gain. */
 	context.frameContext.agc.gain = minAnalogueGain_;
-	context.frameContext.agc.exposure = minExposureLines_;
+	context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
 
 	return 0;
 }
@@ -242,11 +241,9 @@  void Agc::computeExposure(IPAFrameContext &frameContext, double currentYGain)
 	 * exposure value applied multiplied by the new estimated gain.
 	 */
 	currentExposure_ = effectiveExposureValue * evGain;
-	utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;
-	utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;
 
 	/* Clamp the exposure value to the min and max authorized */
-	utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;
+	utils::Duration maxTotalExposure = maxShutterSpeed_ * maxAnalogueGain_;
 	currentExposure_ = std::min(currentExposure_, maxTotalExposure);
 	LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
 			    << ", maximum is " << maxTotalExposure;
@@ -256,14 +253,14 @@  void Agc::computeExposure(IPAFrameContext &frameContext, double currentYGain)
 
 	/* Divide the exposure value as new exposure and gain values */
 	utils::Duration exposureValue = filteredExposure_;
-	utils::Duration shutterTime = minShutterSpeed;
+	utils::Duration shutterTime;
 
 	/*
 	* Push the shutter time up to the maximum first, and only then
 	* increase the gain.
 	*/
 	shutterTime = std::clamp<utils::Duration>(exposureValue / minAnalogueGain_,
-						  minShutterSpeed, maxShutterSpeed);
+						  minShutterSpeed_, maxShutterSpeed_);
 	double stepGain = std::clamp(exposureValue / shutterTime,
 				     minAnalogueGain_, maxAnalogueGain_);
 	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
index 1d085b40..f14d56b7 100644
--- a/src/ipa/ipu3/algorithms/agc.h
+++ b/src/ipa/ipu3/algorithms/agc.h
@@ -46,8 +46,8 @@  private:
 	double iqMean_;
 
 	utils::Duration lineDuration_;
-	uint32_t minExposureLines_;
-	uint32_t maxExposureLines_;
+	utils::Duration minShutterSpeed_;
+	utils::Duration maxShutterSpeed_;
 
 	double minAnalogueGain_;
 	double maxAnalogueGain_;