[libcamera-devel] [PATCH 1/2] ipa: rkisp1: Drop private exposure and gain limits

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Tue, Mar 08, 2022 at 02:45:19PM +0530, Umang Jain wrote:
> The private members of exposure and gain limits can be dropped
> from IPARkISP1 since they are not used class-wide and can be easily
> replaced by local counter-parts.
> 
> In case they are required to be widely available, these should then
> sit in IPASessionConfiguration.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>

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

> ---
>  src/ipa/rkisp1/rkisp1.cpp | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 2d79f15f..129afddd 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -69,10 +69,6 @@ private:
>  
>  	/* Camera sensor controls. */
>  	bool autoExposure_;
> -	uint32_t minExposure_;
> -	uint32_t maxExposure_;
> -	uint32_t minGain_;
> -	uint32_t maxGain_;
>  
>  	/* revision-specific data */
>  	rkisp1_cif_isp_version hwRevision_;
> @@ -166,15 +162,15 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  
>  	autoExposure_ = true;
>  
> -	minExposure_ = itExp->second.min().get<int32_t>();
> -	maxExposure_ = itExp->second.max().get<int32_t>();
> +	int32_t minExposure = itExp->second.min().get<int32_t>();
> +	int32_t maxExposure = itExp->second.max().get<int32_t>();
>  
> -	minGain_ = itGain->second.min().get<int32_t>();
> -	maxGain_ = itGain->second.max().get<int32_t>();
> +	int32_t minGain = itGain->second.min().get<int32_t>();
> +	int32_t maxGain = itGain->second.max().get<int32_t>();
>  
>  	LOG(IPARkISP1, Info)
> -		<< "Exposure: " << minExposure_ << "-" << maxExposure_
> -		<< " Gain: " << minGain_ << "-" << maxGain_;
> +		<< "Exposure: " << minExposure << "-" << maxExposure
> +		<< " Gain: " << minGain << "-" << maxGain;
>  
>  	/* Clean context at configuration */
>  	context_ = {};
> @@ -191,10 +187,10 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  	 *
>  	 * \todo take VBLANK into account for maximum shutter speed
>  	 */
> -	context_.configuration.agc.minShutterSpeed = minExposure_ * context_.configuration.sensor.lineDuration;
> -	context_.configuration.agc.maxShutterSpeed = maxExposure_ * context_.configuration.sensor.lineDuration;
> -	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);
> -	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);
> +	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
> +	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
> +	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> +	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
>  
>  	for (auto const &algo : algorithms_) {
>  		int ret = algo->configure(context_, info);

-- 
Regards,

Laurent Pinchart