[PATCH 1/3] ipa: rkisp1: Initialise AGC from FrameDurationLimits controls

[Laurent Pinchart]

On Mon, Oct 14, 2024 at 04:47:45PM +0100, Kieran Bingham wrote:
> The IPA calculates and reports the FrameDurationLimits to applications
> by configuring the ControlInfo accordingly during
> IPARkISP1::updateControls()
> 
> We later need to know these limits during Agc::configure() for
> initialising the ActiveState of the AGC implementation with the limits.
> 
> Store the FrameDurationLimits ControlInfo in the ControlInfoMap which is
> now present in the IPAContext so that it is commonly available for the
> AGC algorithm, removing the 'todo' accordingly.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 9 +++------
>  src/ipa/rkisp1/rkisp1.cpp         | 5 ++---
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 17d074d9c03e..33f902862c4a 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -178,12 +178,9 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>  	context.activeState.agc.meteringMode =
>  		static_cast<controls::AeMeteringModeEnum>(meteringModes_.begin()->first);
>  
> -	/*
> -	 * \todo This should probably come from FrameDurationLimits instead,
> -	 * except it's computed in the IPA and not here so we'd have to
> -	 * recompute it.
> -	 */
> -	context.activeState.agc.maxFrameDuration = context.configuration.sensor.maxShutterSpeed;
> +	/* Limit the frame duration to match current initialisation */
> +	ControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];

const

> +	context.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());

That's long, maybe

	context.activeState.agc.maxFrameDuration =
		std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());

or

	int64_t maxFrameDuration = frameDurationLimits.max().get<int64_t>();
	context.activeState.agc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);
>  
>  	/*
>  	 * Define the measurement window for AGC as a centered rectangle
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 9e161cabdea4..47777ece783f 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -432,9 +432,8 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,
>  		frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
>  	}
>  
> -	ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
> -							      frameDurations[1],
> -							      frameDurations[2]);
> +	context_.ctrlMap[&controls::FrameDurationLimits] =
> +		ControlInfo(frameDurations[0], frameDurations[1], frameDurations[2]);

I wonder if all this should be moved to the AGC algorithm. There are
cross-dependencies between the IPARkISP1 class and the AGC algorithm
that make the code fragile. Cleanups would be nice.

>  
>  	ctrlMap.insert(context_.ctrlMap.begin(), context_.ctrlMap.end());
>  	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);

-- 
Regards,

Laurent Pinchart