[PATCH 4/4] ipa: rkisp1: Initialize FrameContext.agc.meteringMode

[Paul Elder]

On Wed, Oct 16, 2024 at 07:03:45PM +0200, Jacopo Mondi wrote:
> The RkISP1 AGC algorithms assumes the metering mode to be
> "MeteringMatrix" and pre-fill an array of weights associated
> with it stored in meteringModes_[MeteringMatrix] when intializing
> the algorithm in parseMeteringModes().
> 
> It laters fetches the weights when computing parameters using the
> FrameContext.agc.meteringMode as index of the meteringModes_ array.
> 
> When a Request gets queued to the algorithm, the
> FrameContext.agc.meteringMode index is populated from the algorithm's
> active state, and optionally updated if the application has specified
> a different metering mode.
> 
> In case of Request underrun however, a non-initialized FrameContext
> gets provided to the algorithm, causing an (unhandled) exception when
> accessing the meteringModes_ array.
> 
> Fix this by intializing the AGC metering mode to a supported value
> coming from the ActiveState in IPAFrameContext::init().
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 2 ++
>  src/ipa/rkisp1/ipa_context.cpp    | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 17d074d9c03e..dd7e9468741e 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -175,6 +175,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>  		static_cast<controls::AeConstraintModeEnum>(constraintModes().begin()->first);
>  	context.activeState.agc.exposureMode =
>  		static_cast<controls::AeExposureModeEnum>(exposureModeHelpers().begin()->first);
> +
> +	/* Use the metering matrix mode by default. */

Strictly speaking, this isn't where the metering mode is defaulted to
matrix; it's in parseMeteringModes().

>  	context.activeState.agc.meteringMode =
>  		static_cast<controls::AeMeteringModeEnum>(meteringModes_.begin()->first);
>  
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 2dad42b3154f..e88609137345 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -421,6 +421,11 @@ void IPAFrameContext::init(const uint32_t frameNum,
>  			   const ActiveState &activeState)
>  {
>  	FrameContext::init(frameNum, activeState);
> +
> +	const IPAActiveState *rkisp1ActiveState =
> +		reinterpret_cast<const IPAActiveState *>(&activeState);
> +
> +	agc.meteringMode = rkisp1ActiveState->agc.meteringMode;

But yes this looks like the right solution.

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

>  }
>  
>  /**
> -- 
> 2.47.0
>