[v3,22/41] libipa: agc: Populate frameContext in queueRequest() in auto mode

28266 diff mbox series

In case of a param buffer underrun computeParams() is not called on a
frame. At the moment this causes a

"ERROR AgcMeanLuminance agc_mean_luminance.cpp:689 Effective exposure
value is 0. This is a bug in AGC and must be fixed for proper
operation."

log message, when processStats() is called on such a frame because
frameContext.gain is 0. Fix that by initializing all the values in
queueRequest().

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v3:
- Manually rebased as agc was moved to libipa
- Changed commit message to better reflect the issue that this fixes

Changes in v2:
- Collected tag
---
 src/ipa/libipa/agc.cpp | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

2026. 09. 14. 16:02 keltezéssel, Stefan Klug írta:
> In case of a param buffer underrun computeParams() is not called on a
> frame. At the moment this causes a
> 
> "ERROR AgcMeanLuminance agc_mean_luminance.cpp:689 Effective exposure
> value is 0. This is a bug in AGC and must be fixed for proper
> operation."
> 
> log message, when processStats() is called on such a frame because
> frameContext.gain is 0. Fix that by initializing all the values in
> queueRequest().
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

Could you adjust the title to says "... in auto mode *as well*" and
mention that it is already done in manual mode?

I also feel like with the current implementation, `prepare()` should
be a no-op since the algorithm only touches sensor controls. But I
see that this keeps everything in `prepare()` as is, which seems a bit
confusing to me. Is that because a later `process()` between `queueRequest()`
and `prepare()` should make it so that those new parameters are used
for the (not yet processed but queued) frame?


> 
> ---
> Changes in v3:
> - Manually rebased as agc was moved to libipa
> - Changed commit message to better reflect the issue that this fixes
> 
> Changes in v2:
> - Collected tag
> ---
>   src/ipa/libipa/agc.cpp | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index 9016e5f68ab7..a2438cb49d1b 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -608,12 +608,18 @@ void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
>   	frameContext.autoExposureEnabled = state.autoExposureEnabled;
>   	frameContext.autoGainEnabled = state.autoGainEnabled;
>   
> -	if (!frameContext.autoExposureEnabled)
> +	if (frameContext.autoExposureEnabled)
> +		frameContext.exposure = state.automatic.exposure;
> +	else
>   		frameContext.exposure = state.manual.exposure;
> -	if (!frameContext.autoGainEnabled)
> +	if (frameContext.autoGainEnabled)
> +		frameContext.gain = state.automatic.gain;
> +	else
>   		frameContext.gain = state.manual.gain;
>   
> -	if (!frameContext.autoExposureEnabled && !frameContext.autoGainEnabled)
> +	if (frameContext.autoExposureEnabled || frameContext.autoGainEnabled)
> +		frameContext.quantizationGain = state.automatic.quantizationGain;
> +	else
>   		frameContext.quantizationGain = 1.0;
>   
>   	const auto &exposureMode = controls.get(controls::AeExposureMode);