From 950ca85e8aa5a894278893e20eb029e1dcecf8d7 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 30 Sep 2025 17:04:24 +0200 Subject: [PATCH] ipa: software_isp: AGC: Do not lower gain below 1.0 At the moment when the overall image brightness is considered too high the AGC code will lower the gain all the way down to againMin before considering lowering the exposure. What should happen instead is lower the gain no lower than 1.0 and after that lower the exposure instead of lowering the gain. Otherwise there might be a heavily overexposed image (e.g. all white) which then is made less white by a gain < 1.0 which is no good. When there is no sensor-helper, assume the driver reported default-gain value is close to a gain of 1.0 . While at it also remove the weird limitation to only lower the gain when exposure is set to the maximum. As long as the gain is higher than the default gain, the gain should be lowered first. Reviewed-by: Milan Zamazal Tested-by: Milan Zamazal Tested-by: Kieran Bingham Reviewed-by: Kieran Bingham Reviewed-by: Isaac Scott Signed-off-by: Hans de Goede Signed-off-by: Kieran Bingham --- src/ipa/simple/algorithms/agc.cpp | 3 +-- src/ipa/simple/ipa_context.h | 2 +- src/ipa/simple/soft_simple.cpp | 3 +++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp index c46bb0eb..1fc8d7f4 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -71,8 +71,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou } if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { - if (exposure == context.configuration.agc.exposureMax && - again > context.configuration.agc.againMin) { + if (again > context.configuration.agc.again10) { next = again * kExpNumeratorDown / kExpDenominator; if (again - next < context.configuration.agc.againMinStep) again -= context.configuration.agc.againMinStep; diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index a471b80a..468fccab 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -28,7 +28,7 @@ struct IPASessionConfiguration { float gamma; struct { int32_t exposureMin, exposureMax; - double againMin, againMax, againMinStep; + double againMin, againMax, again10, againMinStep; utils::Duration lineDuration; } agc; struct { diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index e70439ee..b147aca2 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -216,10 +216,12 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) int32_t againMin = gainInfo.min().get(); int32_t againMax = gainInfo.max().get(); + int32_t againDef = gainInfo.def().get(); if (camHelper_) { context_.configuration.agc.againMin = camHelper_->gain(againMin); context_.configuration.agc.againMax = camHelper_->gain(againMax); + context_.configuration.agc.again10 = camHelper_->gain(1.0); context_.configuration.agc.againMinStep = (context_.configuration.agc.againMax - context_.configuration.agc.againMin) / @@ -246,6 +248,7 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) * other) we limit the range of the gain values used. */ context_.configuration.agc.againMax = againMax; + context_.configuration.agc.again10 = againDef; if (againMin) { context_.configuration.agc.againMin = againMin; } else {