[{"id":35997,"web_url":"https://patchwork.libcamera.org/comment/35997/","msgid":"<175888879678.198859.17335401633157977600@isaac-ThinkPad-T16-Gen-2>","date":"2025-09-26T12:13:16","subject":"Re: [PATCH v2 2/6] ipa: software_isp: AGC: Do not lower gain below\n\t1.0","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Hans,\n\nThank you for the patch!\n\nQuoting Hans de Goede (2025-09-25 23:17:04)\n> At the moment when the overall image brightness is considered too high\n> the AGC code will lower the gain all the way down to againMin before\n> considering lowering the exposure.\n> \n> What should happen instead is lower the gain no lower than 1.0 and after\n> that lower the exposure instead of lowering the gain.\n> \n> Otherwise there might be a heavily overexposed image (e.g. all white)\n> which then is made less white by a gain < 1.0 which is no good.\n> \n> When there is no sensor-helper, assume the driver reported default-gain\n> value is close to a gain of 1.0 .\n\nReviewed-by: Isaac Scott \n\nBest wishes,\nIsaac\n\n> \n> While at it also remove the weird limitation to only lower the gain\n> when exposure is set to the maximum. As long as the gain is higher\n> than the default gain, the gain should be lowered first.\n> \n> Reviewed-by: Milan Zamazal \n> Tested-by: Milan Zamazal \n> Tested-by: Kieran Bingham \n> Signed-off-by: Hans de Goede \n> ---\n> Changes in v2:\n> - Use a gain of 1.0 as low-limit instead of always using the default-gain,\n> falling back to the default if there is no sensor-helper for the sensor.\n> ---\n> src/ipa/simple/algorithms/agc.cpp | 3 +--\n> src/ipa/simple/ipa_context.h | 2 +-\n> src/ipa/simple/soft_simple.cpp | 3 +++\n> 3 files changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> index c46bb0eb..1fc8d7f4 100644\n> --- a/src/ipa/simple/algorithms/agc.cpp\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -71,8 +71,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\n> }\n> \n> if (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n> - if (exposure == context.configuration.agc.exposureMax &&\n> - again > context.configuration.agc.againMin) {\n> + if (again > context.configuration.agc.again10) {\n> next = again * kExpNumeratorDown / kExpDenominator;\n> if (again - next < context.configuration.agc.againMinStep)\n> again -= context.configuration.agc.againMinStep;\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index a471b80a..468fccab 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -28,7 +28,7 @@ struct IPASessionConfiguration {\n> float gamma;\n> struct {\n> int32_t exposureMin, exposureMax;\n> - double againMin, againMax, againMinStep;\n> + double againMin, againMax, again10, againMinStep;\n> utils::Duration lineDuration;\n> } agc;\n> struct {\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index e70439ee..b147aca2 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -216,10 +216,12 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> \n> int32_t againMin = gainInfo.min().get();\n> int32_t againMax = gainInfo.max().get();\n> + int32_t againDef = gainInfo.def().get();\n> \n> if (camHelper_) {\n> context_.configuration.agc.againMin = camHelper_->gain(againMin);\n> context_.configuration.agc.againMax = camHelper_->gain(againMax);\n> + context_.configuration.agc.again10 = camHelper_->gain(1.0);\n> context_.configuration.agc.againMinStep =\n> (context_.configuration.agc.againMax -\n> context_.configuration.agc.againMin) /\n> @@ -246,6 +248,7 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> * other) we limit the range of the gain values used.\n> */\n> context_.configuration.agc.againMax = againMax;\n> + context_.configuration.agc.again10 = againDef;\n> if (againMin) {\n> context_.configuration.agc.againMin = againMin;\n> } else {\n> -- \n> 2.51.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C178EBDB1C\n\tfor ;\n\tFri, 26 Sep 2025 12:13:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42AC76B5F3;\n\tFri, 26 Sep 2025 14:13:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8DB5E613AB\n\tfor ;\n\tFri, 26 Sep 2025 14:13:21 +0200 (CEST)","from thinkpad.ideasonboard.com\n\t(cpc90716-aztw32-2-0-cust408.18-1.cable.virginm.net [86.26.101.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 093E2152D;\n\tFri, 26 Sep 2025 14:11:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"szM0LMGz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758888716;\n\tbh=VE+DxcSV+aL7KJsHt1c48D82Ju1K7quIIdb+beljnHE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=szM0LMGzAq65s6xwFfpjK5of+xSx9zRoPvGjVC52rdiLpf/SU4QvvPE3O6qG+Q02E\n\tlU7MDVePUjaH+ZX+b7o2TjFeOFeukC7VYhAZJIC8txkBg01lB4WoqbROe6SRQ1pY5W\n\tHfFqC9okFRWwtJdnH6rn2B+CECN/gr7Una3R0/Cc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250925221708.7471-3-hansg@kernel.org>","References":"<20250925221708.7471-1-hansg@kernel.org>\n\t<20250925221708.7471-3-hansg@kernel.org>","Subject":"Re: [PATCH v2 2/6] ipa: software_isp: AGC: Do not lower gain below\n\t1.0","From":"Isaac Scott ","Cc":"Hans de Goede , Milan Zamazal ,\n\tKieran Bingham ","To":"Hans de Goede , libcamera-devel@lists.libcamera.org","Date":"Fri, 26 Sep 2025 13:13:16 +0100","Message-ID":"<175888879678.198859.17335401633157977600@isaac-ThinkPad-T16-Gen-2>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]