[PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when effectiveExposureValue is zero

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Fri, Feb 28, 2025 at 01:55:54PM +0100, Stefan Klug wrote:
> In a proper system it never happens that the effectiveExposureValue
> drops to zero. If that still happens due to a bug outside of
> agc_mean_luminance, the calculated gain goes towards infinity but the
> newExposureValue is still 0 because it is the result of multiplying the
> effectiveExposureTime with the gain, leading to wild oscillations.
> 
> Catch that condition, print an error message and set the new effective
> exposure value to an arbitrary 10ms.
> 
> Note that in any case the underlying problem must be fixed. The
> important change is the added error message to be able to detect such a
> situation.

I'm tempted to then return all 0's. If this indicates a bug somewhere
else, the bug should be fixed, is there a value in working around it ?

> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/libipa/agc_mean_luminance.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 02555a44d271..a7343c18f5aa 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -541,6 +541,13 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,
>  	std::shared_ptr<ExposureModeHelper> exposureModeHelper =
>  		exposureModeHelpers_.at(exposureModeIndex);
>  
> +	if (effectiveExposureValue == 0s) {
> +		LOG(AgcMeanLuminance, Error) << "Effective exposure value is 0."
> +					     << " Resetting exposure time and"
> +					     << " gain to arbitrary defaults.";

You can write

		LOG(AgcMeanLuminance, Error)
			<< "Effective exposure value is 0. Resetting exposure "
			   "time and gain to arbitrary defaults.";

which will split the string on the lines, but won't call the << operator
multiple times.

I'd focus the error message on reporting the underlying problem. As it
is, I don't think it would be clear for users what to do.

		LOG(AgcMeanLuminance, Error)
			<< "Effective exposure value is 0. This is a bug in AGC "
			   "and must be fixed for proper operation."

or something like that ?

> +		return exposureModeHelper->splitExposure(10ms);
> +	}
> +
>  	double gain = estimateInitialGain();
>  	gain = constraintClampGain(constraintModeIndex, yHist, gain);
>  

-- 
Regards,

Laurent Pinchart