[PATCH] ipa: libipa: Fix bug in ExposureModeHelper that leads to oscillations in AEGC

[Paul Elder]

On Thu, Feb 27, 2025 at 08:58:10PM +0100, Stefan Klug wrote:
> The ExposureModeHelper::splitExposures() runs through the configured
> stages to find the best gain/exposure time pair. It first raises the
> exposure time until it reaches the limit of the current stage. Then it
> raises the gain until that also reaches the limit of the current stage.
> After that it continues with the next stage until a match is found.
> 
> Due to a slight mistake in the initial code, the second step doesn't
> work as expected because the exposure time gets divided by the gain of
> the current stage, effectively leading to a jump of the gain value from
> the maximum gain of the last stage to the maximum gain of the current
> stage instead of gradually increasing the gain value.
> 
> Depending on the tuning file this leads to very visible oscillations and
> jumps in the brightness.
> 
> Fix by clamping the exposure time in the second step to the maximum
> exposure time of the current stage.
> 
> While at it, add two comments for easier understanding.
> 
> Fixes: 34c9ab62827b ("ipa: libipa: Add ExposureModeHelper")
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>

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

> ---
>  src/ipa/libipa/exposure_mode_helper.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index f235316d3539..0c1e99e31a47 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -182,6 +182,7 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  		 * the stage limits are initialised.
>  		 */
>  
> +		/* Clamp the gain to lastStageGain and regulate exposureTime. */
>  		if (stageExposureTime * lastStageGain >= exposure) {
>  			exposureTime = clampExposureTime(exposure / clampGain(lastStageGain));
>  			gain = clampGain(exposure / exposureTime);
> @@ -189,8 +190,9 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  			return { exposureTime, gain, exposure / (exposureTime * gain) };
>  		}
>  
> +		/* Clamp the exposureTime to stageExposureTime and regulate gain. */
>  		if (stageExposureTime * stageGain >= exposure) {
> -			exposureTime = clampExposureTime(exposure / clampGain(stageGain));
> +			exposureTime = clampExposureTime(stageExposureTime);
>  			gain = clampGain(exposure / exposureTime);
>  
>  			return { exposureTime, gain, exposure / (exposureTime * gain) };
> -- 
> 2.43.0
>