[libcamera-devel] [PATCH v2 12/13] ipa: ipu3: agc: Remove unused variables

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Wed, Oct 20, 2021 at 05:46:06PM +0200, Jean-Michel Hautbois wrote:
> We currently control the exposure value by the shutter speed and the
> analogue gain. We can't use the digital gain to have more than the
> maximum exposure value calculated because we are not controlling it.
> 
> Remove unused code associated with this digital gain.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 19 +++----------------
>  src/ipa/ipu3/algorithms/agc.h   |  2 --
>  2 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 0417dc99..fc37eca4 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -42,8 +42,7 @@ static constexpr double kEvGainTarget = 0.5;
>  Agc::Agc()
>  	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
>  	  minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s),
> -	  filteredExposureNoDg_(0s), currentExposure_(0s),
> -	  currentExposureNoDg_(0s), prevExposureValue_(0s)
> +	  currentExposure_(0s), prevExposureValue_(0s)
>  {
>  }
>  
> @@ -102,7 +101,6 @@ void Agc::filterExposure()
>  	if (filteredExposure_ == 0s) {
>  		/* DG stands for digital gain.*/
>  		filteredExposure_ = currentExposure_;
> -		filteredExposureNoDg_ = currentExposureNoDg_;
>  	} else {
>  		/*
>  		 * If we are close to the desired result, go faster to avoid making
> @@ -115,18 +113,8 @@ void Agc::filterExposure()
>  
>  		filteredExposure_ = speed * currentExposure_ +
>  				filteredExposure_ * (1.0 - speed);
> -		filteredExposureNoDg_ = speed * currentExposureNoDg_ +
> -				filteredExposureNoDg_ * (1.0 - speed);
>  	}
> -	/*
> -	 * We can't let the no_dg exposure deviate too far below the
> -	 * total exposure, as there might not be enough digital gain available
> -	 * in the ISP to hide it (which will cause nasty oscillation).
> -	 */
> -	double fastReduceThreshold = 0.4;
> -	if (filteredExposureNoDg_ <
> -	    filteredExposure_ * fastReduceThreshold)
> -		filteredExposureNoDg_ = filteredExposure_ * fastReduceThreshold;
> +
>  	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << filteredExposure_;
>  }
>  
> @@ -142,8 +130,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>  
>  	/* extracted from Rpi::Agc::computeTargetExposure */
>  	utils::Duration currentShutter = exposure * lineDuration_;
> -	currentExposureNoDg_ = currentShutter * analogueGain;
> -	LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
> +	LOG(IPU3Agc, Debug) << "Actual total exposure " << currentShutter * analogueGain
>  			    << " Shutter speed " << currentShutter
>  			    << " Gain " << analogueGain
>  			    << " Needed ev gain " << evGain;
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 2ae88e9f..ad133b98 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -46,9 +46,7 @@ private:
>  	uint32_t maxExposureLines_;
>  
>  	utils::Duration filteredExposure_;
> -	utils::Duration filteredExposureNoDg_;
>  	utils::Duration currentExposure_;
> -	utils::Duration currentExposureNoDg_;
>  	utils::Duration prevExposureValue_;
>  
>  	uint32_t stride_;

-- 
Regards,

Laurent Pinchart