[{"id":38773,"web_url":"https://patchwork.libcamera.org/comment/38773/","msgid":"<3063853c-b927-4b63-b305-4ec843ffb9e8@oss.qualcomm.com>","date":"2026-05-07T13:16:00","subject":"Re: [PATCH v5 1/3] ipa: simple: agc: Replace bang-bang controller\n\twith proportional","submitter":{"id":242,"url":"https://patchwork.libcamera.org/api/people/242/","name":"Hans de Goede","email":"johannes.goede@oss.qualcomm.com"},"content":"Hi,\n\nOn 6-May-26 23:46, Javier Tia wrote:\n> The AGC's updateExposure() uses a fixed ~10% step per frame regardless\n> of how far the current exposure is from optimal. With a hysteresis dead\n> band of only +/-4%, the controller overshoots when the correct value\n> falls within one step, causing visible brightness oscillation (flicker).\n> \n> Replace the fixed-step bang-bang controller with a proportional one\n> where the correction factor scales linearly with the MSV error:\n> \n> factor = 1.0 + clamp(error * 0.04, -0.15, +0.15)\n> \n> At maximum error (~2.5), this gives the same ~10% step as before. Near\n> the target, steps shrink to <1%, eliminating overshoot. The step is\n> clamped to +/-15% to bound corrections when the scene changes\n> dramatically. The existing hysteresis (kExposureSatisfactory) still\n> prevents hunting on noise.\n> \n> Tested on OV2740 behind Intel IPU6 ISYS (ThinkPad X1 Carbon Gen 10)\n> where the old controller produced continuous brightness flicker. The\n> proportional controller converges in ~3 seconds from cold start with\n> no visible oscillation.\n> \n> Signed-off-by: Javier Tia \n> Reviewed-by: Milan Zamazal \n> Tested-by: Barnabás Pőcze \n\nThanks, patch looks good to me:\n\nReviewed-by: Hans de Goede \n\nRegards,\n\nHans\n\n\n\n> ---\n> src/ipa/simple/algorithms/agc.cpp | 73 +++++++++++++++++++++----------\n> 1 file changed, 49 insertions(+), 24 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> index 2f7e040c..a13a7552 100644\n> --- a/src/ipa/simple/algorithms/agc.cpp\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \"agc.h\"\n> \n> +#include \n> +#include \n> #include \n> \n> #include \n> @@ -37,52 +39,74 @@ static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n> */\n> static constexpr float kExposureSatisfactory = 0.2;\n> \n> +/*\n> + * Proportional gain for exposure/gain adjustment. Maps the MSV error to a\n> + * multiplicative correction factor:\n> + *\n> + * factor = 1.0 + kExpProportionalGain * error\n> + *\n> + * With kExpProportionalGain = 0.04:\n> + * - max error ~2.5 -> factor 1.10 (~10% step, same as before)\n> + * - error 1.0 -> factor 1.04 (~4% step)\n> + * - error 0.3 -> factor 1.012 (~1.2% step)\n> + *\n> + * This replaces the fixed 10% bang-bang step with a proportional correction\n> + * that converges smoothly and avoids overshooting near the target.\n> + */\n> +static constexpr float kExpProportionalGain = 0.04;\n> +\n> +/*\n> + * Maximum multiplicative step per frame, to bound the correction when the\n> + * scene changes dramatically.\n> + */\n> +static constexpr float kExpMaxStep = 0.15;\n> +\n> Agc::Agc()\n> {\n> }\n> \n> void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)\n> {\n> -\t/*\n> -\t * kExpDenominator of 10 gives ~10% increment/decrement;\n> -\t * kExpDenominator of 5 - about ~20%\n> -\t */\n> -\tstatic constexpr uint8_t kExpDenominator = 10;\n> -\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n> -\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> -\n> \tint32_t &exposure = frameContext.sensor.exposure;\n> \tdouble &again = frameContext.sensor.gain;\n> \n> -\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> +\tdouble error = kExposureOptimal - exposureMSV;\n> +\n> +\tif (std::abs(error) <= kExposureSatisfactory)\n> +\t\treturn;\n> +\n> +\t/*\n> +\t * Compute a proportional correction factor. The sign of the error\n> +\t * determines the direction: positive error means too dark (increase),\n> +\t * negative means too bright (decrease).\n> +\t */\n> +\tfloat step = std::clamp(static_cast(error) * kExpProportionalGain,\n> +\t\t\t\t-kExpMaxStep, kExpMaxStep);\n> +\tfloat factor = 1.0f + step;\n> +\n> +\tif (factor > 1.0f) {\n> +\t\t/* Scene too dark: increase exposure first, then gain. */\n> \t\tif (exposure < context.configuration.agc.exposureMax) {\n> -\t\t\tint32_t next = exposure * kExpNumeratorUp / kExpDenominator;\n> -\t\t\tif (next - exposure < 1)\n> -\t\t\t\texposure += 1;\n> -\t\t\telse\n> -\t\t\t\texposure = next;\n> +\t\t\tint32_t next = static_cast(exposure * factor);\n> +\t\t\texposure = std::max(next, exposure + 1);\n> \t\t} else {\n> -\t\t\tdouble next = again * kExpNumeratorUp / kExpDenominator;\n> +\t\t\tdouble next = again * factor;\n> \t\t\tif (next - again < context.configuration.agc.againMinStep)\n> \t\t\t\tagain += context.configuration.agc.againMinStep;\n> \t\t\telse\n> \t\t\t\tagain = next;\n> \t\t}\n> -\t}\n> -\n> -\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n> +\t} else {\n> +\t\t/* Scene too bright: decrease gain first, then exposure. */\n> \t\tif (again > context.configuration.agc.again10) {\n> -\t\t\tdouble next = again * kExpNumeratorDown / kExpDenominator;\n> +\t\t\tdouble next = again * factor;\n> \t\t\tif (again - next < context.configuration.agc.againMinStep)\n> \t\t\t\tagain -= context.configuration.agc.againMinStep;\n> \t\t\telse\n> \t\t\t\tagain = next;\n> \t\t} else {\n> -\t\t\tint32_t next = exposure * kExpNumeratorDown / kExpDenominator;\n> -\t\t\tif (exposure - next < 1)\n> -\t\t\t\texposure -= 1;\n> -\t\t\telse\n> -\t\t\t\texposure = next;\n> +\t\t\tint32_t next = static_cast(exposure * factor);\n> +\t\t\texposure = std::min(next, exposure - 1);\n> \t\t}\n> \t}\n> \n> @@ -96,6 +120,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\n> \n> \tLOG(IPASoftExposure, Debug)\n> \t\t<< \"exposureMSV \" << exposureMSV\n> +\t\t<< \" error \" << error << \" factor \" << factor\n> \t\t<< \" exp \" << exposure << \" again \" << again;\n> }\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 563CCBDCB5\n\tfor ;\n\tThu, 7 May 2026 13:16:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3FEE463025;\n\tThu, 7 May 2026 15:16:08 +0200 (CEST)","from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com\n\t[205.220.168.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A67E062010\n\tfor ;\n\tThu, 7 May 2026 15:16:06 +0200 (CEST)","from pps.filterd (m0279863.ppops.net [127.0.0.1])\n\tby mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id\n\t6479FxBu258729 for ;\n\tThu, 7 May 2026 13:16:04 GMT","from mail-ot1-f71.google.com (mail-ot1-f71.google.com\n\t[209.85.210.71])\n\tby mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4e0hr8t9ac-1\n\t(version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT)\n\tfor ;\n\tThu, 07 May 2026 13:16:04 +0000 (GMT)","by mail-ot1-f71.google.com with SMTP id\n\t46e09a7af769-7dcc8f071b3so1373932a34.2\n\tfor ;\n\tThu, 07 May 2026 06:16:04 -0700 (PDT)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-bc81cd34d24sm81485666b.3.2026.05.07.06.16.01\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 07 May 2026 06:16:01 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=qualcomm.com header.i=@qualcomm.com\n\theader.b=\"KpZYH7yY\"; dkim=pass (2048-bit key;\n\tunprotected) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com\n\theader.b=\"TyOp0r4f\"; dkim-atps=neutral","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h=\n\tcc:content-transfer-encoding:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to; s=qcppdkim1; bh=\n\tYsTjm8RpJkcqJoG300ZT+ZlGg9M0duYJP/iVljY7ToQ=; b=KpZYH7yYehD4nD8f\n\tPofe/g4wG9hQTQohbH/vGOqLXUBDjPGP6Cjt3r+gb/6C5mA6McbnfkF9yxnDV6rx\n\t4MYkb78EI2ZEorDxGTcMKjNWVbVJmmsqe3VSDe2o3pDvwm1WbUOREKUMVFVpSV7F\n\tT+ymjg20XmuHWbqP+tcUqQL8wvDyG24R/L8KnWVGJtT3heXNFGItAfbRW6qUfSbj\n\t/q96docqTquXc3JNVFycNr82ZGYkdyURc4/dZEpGV5Ow1ILvJ0KPig9Y1RDBiDsA\n\t8BlCSyj10bW8Lkg6KC866n9DkbtJvpRiP6UNMcJzX0ZUEZP8MDvsgaB35KFn4XE4\n\tnuVlKw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=oss.qualcomm.com; s=google; t=1778159764; x=1778764564;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:from:to\n\t:cc:subject:date:message-id:reply-to;\n\tbh=YsTjm8RpJkcqJoG300ZT+ZlGg9M0duYJP/iVljY7ToQ=;\n\tb=TyOp0r4f79v4M+muJ9WcFngnNylqbg6LE01UFxv3hCsDrxrRK0mtg2EYYKBGMX0OXm\n\tGEaMSTtW1ce5pifrBdY0DEtoCk2DboJvUHrSCMOY3kTj0BnoXt3+99MarIxEhH51Gwfz\n\t9cw7ecSQdlF/ZffX2HtJ2nnMaKKqjwXCcEhpc6aHnyILV/uVP4pe/b63ENbfWvAIMkFP\n\tP0iQLmliFNAdkXAmsHYuJX+0UGVhrIqFRMWkz45wzRz3Sm4tM7bO99hN080DPeZXvjVa\n\tNL0SwfblpuEFWhh3fE8F35r/mXPKpUT5AwebZuie+fJzmTP6aMSJU8Vk/dTUxdA9wxYR\n\tasMg=="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1778159764; x=1778764564;\n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=YsTjm8RpJkcqJoG300ZT+ZlGg9M0duYJP/iVljY7ToQ=;\n\tb=CyJuytjBLwgDeG+T1Zf2sle9YDUO6ohKmBKI/mCHRnlyOkZ6c4UVN9U69KmX452nQQ\n\ttqEWxUq/J4cbcDefV4Q6mvjcdKqN2E28SaQ4LycavQi2Qbdb187deEKAQ1DSZt/biTD9\n\t2lfnTda+NJOX+G0Sua3cQ9bjrNKJDGAE2J8kbNv2z45RVy0SFvCIcuwxN4ZkHfrErsnC\n\tQVG8F3d64QmnHmEW2emJFTlLu7v7lXDhu+w+37FudF60LZ1F3SG6e0b4zKgnmxu+l4Lw\n\tXk3Jv/Adn2uEX09kgdixDXSs89l2u2Ow5KE8hivDuXNWchnEjluWbczmlVLv+gJCBicO\n\tnFgg==","X-Forwarded-Encrypted":"i=1;\n\tAFNElJ9KEiVkilsKPzcdPy/fxY0nrnaBlSRMOLpF2zQz8eLY4rp4/tmKk4naBDNpDy63Kvr385czOiNkehrpKtmY1PE=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YwyMn/ClWLdwWsXjeq+/n/S7oGPPjfGMGBEsfqvqWObUufzvyuc\n\tQF1XjL8/9fkTo6oc5959QtzykKTdNEVKIcqx2zoPDw0IyEZsKI6/I4qpqv9ROB80gngmueoebEi\n\tfu7hxF4syve0IVwz7nVgd2z8qgNswMnJq07FgT4EA+pVj3cN2E0jZCWll73/iT02G1IoAwXwWaH\n\ttJ","X-Gm-Gg":"AeBDievaTG0XSw8CktcHoShTA3zZfWPO9rqK8jAny/2YIV8+AAjdKhWiInnEoOVVJNO\n\tou1k2kB5fk0SXq9aNwAviZiBjy/Vkb6BshiIGRWD+4YxKqxyTZUhvD0bttrkROeGHhz/a81z/rO\n\tq6AnPfWJyjYXkEoBE4RNOQZcIt88sF5FFORuTdMUgVI5+3eSNYP8JbNTM9Q2BP14dj84vtb+x4N\n\ts5yJvBGCMaCSamiTuWqJA3XeOKXr61So4v6fmWaQOfs+TBBoeBcq8kKFAHl2CnPY0EH9rBKZnFw\n\tuoeq+jZqu9KXrz9+u8d21sl5SZccC+QJjBso26+CKSTJVu0SHESqJd8ZY83/kQSLqPnzxBbt7ja\n\tk4JiCZJazUemLakRgzHexwRK7bb4+bMXubZB9fltjYlb71I9PvD169QDqru4BhZ+dZn+dHq6MSq\n\tKRTvtaW/M7VMMfzb6hstlLuPB8DvB8TPTph0gLCtBlc5wF8UjYJLvX8w29X/LevqyjvW2nNNJrt\n\tTpgSNfGBy1nx1nNVkGkO2jZvYo=","X-Received":["by 2002:a05:6830:67c6:b0:7dd:e032:3cdb with SMTP id\n\t46e09a7af769-7e1df0d2a88mr5106251a34.18.1778159763589; \n\tThu, 07 May 2026 06:16:03 -0700 (PDT)","by 2002:a05:6830:67c6:b0:7dd:e032:3cdb with SMTP id\n\t46e09a7af769-7e1df0d2a88mr5106216a34.18.1778159763170; \n\tThu, 07 May 2026 06:16:03 -0700 (PDT)"],"Message-ID":"<3063853c-b927-4b63-b305-4ec843ffb9e8@oss.qualcomm.com>","Date":"Thu, 7 May 2026 15:16:00 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","From":"johannes.goede@oss.qualcomm.com","Subject":"Re: [PATCH v5 1/3] ipa: simple: agc: Replace bang-bang controller\n\twith proportional","To":"Javier Tia , libcamera-devel@lists.libcamera.org","Cc":"mzamazal@redhat.com, barnabas.pocze@ideasonboard.com,\n\tkieran.bingham@ideasonboard.com","References":"<177810597783.688418.1631246733707368646@jetm.me>\n\t<20260506221942.55C4F1EA006B@mailuser.phl.internal>","Content-Language":"en-US, nl","In-Reply-To":"<20260506221942.55C4F1EA006B@mailuser.phl.internal>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-Proofpoint-ORIG-GUID":"s1_3UQOnm7JfRuYfYUFD2TAfXkR7FQSU","X-Authority-Analysis":"v=2.4 cv=caHiaHDM c=1 sm=1 tr=0 ts=69fc9094 cx=c_pps\n\ta=OI0sxtj7PyCX9F1bxD/puw==:117 a=xqWC_Br6kY4A:10 a=IkcTkHD0fZMA:10\n\ta=NGcC8JguVDcA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22\n\ta=u7WPNUs3qKkmUXheDGA7:22 a=yOCtJkima9RkubShWh1s:22 a=20KFwNOVAAAA:8\n\ta=P1BnusSwAAAA:8 a=EUspDBNiAAAA:8 a=fQUxIICLtdSvMgObMfMA:9\n\ta=3ZKOabzyN94A:10\n\ta=QEXdDO2ut3YA:10 a=Z1Yy7GAxqfX1iEi80vsk:22 a=D0XLA9XvdZm18NrgonBM:22","X-Proofpoint-GUID":"s1_3UQOnm7JfRuYfYUFD2TAfXkR7FQSU","X-Proofpoint-Spam-Details-Enc":"AW1haW4tMjYwNTA3MDEzMiBTYWx0ZWRfX3YeN4zhazrP7\n\tgol+uqa3Ogm5JcKF6ImhGrlzYKakLWJyb7L+sWQ0X3BDj2CyBH1n3QjAZEZ7PJ4DQEX6O4kihAg\n\tpUCuG61+vIV3PChhI9Rob3RpOBT8WoguGE6oNvTqX7saLt9OnVqoPjfVsixY3RSyAIBXC2uRdaQ\n\t8Km5283xSpmBM0d7Aqr1nE3aV2QDy3bkVNIwvfqgFDn7JRXvvrVIu3Z7a1EDPrNg/Bb24EGYrnm\n\t9TYyiqydgWFwQSzvMOerdUbrAphPpA0I4P7cE4v8okAy+0SOz6I5Yysv9l+1c6ezyluvxjpgAjn\n\tuGmu27jtNDPquiniN7EA5QqX+gK3MmUdgAICTrWprus0ydGa/ereQKhBsBZlhzx5H8GwYBimEDl\n\tkKYVQqMsjtAy8Hv5DGymaDXrERlMYThNLxSmjximvwr9P6ChmfNIjeKsQG2cDTp88VOrNOXn63L\n\t3Y1CFnUMf7rEmL7nr2g==","X-Proofpoint-Virus-Version":"vendor=baseguard\n\tengine=ICAP:2.0.293, Aquarius:18.0.1143, Hydra:6.1.51,\n\tFMLib:17.12.100.49\n\tdefinitions=2026-05-07_01,2026-05-06_01,2025-10-01_01","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tmalwarescore=0 clxscore=1015 suspectscore=0 bulkscore=0\n\tpriorityscore=1501\n\tadultscore=0 lowpriorityscore=0 impostorscore=0 spamscore=0\n\tphishscore=0\n\tclassifier=typeunknown authscore=0 authtc= authcc= route=outbound\n\tadjust=0\n\treason=mlx scancount=1 engine=8.22.0-2604200000\n\tdefinitions=main-2605070132","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\" "}},{"id":38785,"web_url":"https://patchwork.libcamera.org/comment/38785/","msgid":"<20260507143312.GM1938994@killaraus.ideasonboard.com>","date":"2026-05-07T14:33:12","subject":"Re: [PATCH v5 1/3] ipa: simple: agc: Replace bang-bang controller\n\twith proportional","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Javier,\n\nThank you for the patch.\n\nOn Wed, May 06, 2026 at 03:46:01PM -0600, Javier Tia wrote:\n> The AGC's updateExposure() uses a fixed ~10% step per frame regardless\n> of how far the current exposure is from optimal. With a hysteresis dead\n> band of only +/-4%, the controller overshoots when the correct value\n> falls within one step, causing visible brightness oscillation (flicker).\n> \n> Replace the fixed-step bang-bang controller with a proportional one\n> where the correction factor scales linearly with the MSV error:\n> \n> factor = 1.0 + clamp(error * 0.04, -0.15, +0.15)\n> \n> At maximum error (~2.5), this gives the same ~10% step as before. Near\n> the target, steps shrink to <1%, eliminating overshoot. The step is\n> clamped to +/-15% to bound corrections when the scene changes\n> dramatically. The existing hysteresis (kExposureSatisfactory) still\n> prevents hunting on noise.\n> \n> Tested on OV2740 behind Intel IPU6 ISYS (ThinkPad X1 Carbon Gen 10)\n> where the old controller produced continuous brightness flicker. The\n> proportional controller converges in ~3 seconds from cold start with\n> no visible oscillation.\n\nI'm fine with this, but I'd like to reiterate that Kieran is working on\nmoving the soft ISP IPA to use the AGC algorithm from libipa. That may\nthen cause a regression when this code will be replaced.\n\nKieran, could you try and remember to CC Javier when you will post that\nseries, to ensure it gets tested properly ?\n\n> Signed-off-by: Javier Tia \n> Reviewed-by: Milan Zamazal \n> Tested-by: Barnabás Pőcze \n> ---\n> src/ipa/simple/algorithms/agc.cpp | 73 +++++++++++++++++++++----------\n> 1 file changed, 49 insertions(+), 24 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> index 2f7e040c..a13a7552 100644\n> --- a/src/ipa/simple/algorithms/agc.cpp\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \"agc.h\"\n> \n> +#include \n> +#include \n> #include \n> \n> #include \n> @@ -37,52 +39,74 @@ static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n> */\n> static constexpr float kExposureSatisfactory = 0.2;\n> \n> +/*\n> + * Proportional gain for exposure/gain adjustment. Maps the MSV error to a\n> + * multiplicative correction factor:\n> + *\n> + * factor = 1.0 + kExpProportionalGain * error\n> + *\n> + * With kExpProportionalGain = 0.04:\n> + * - max error ~2.5 -> factor 1.10 (~10% step, same as before)\n> + * - error 1.0 -> factor 1.04 (~4% step)\n> + * - error 0.3 -> factor 1.012 (~1.2% step)\n> + *\n> + * This replaces the fixed 10% bang-bang step with a proportional correction\n> + * that converges smoothly and avoids overshooting near the target.\n> + */\n> +static constexpr float kExpProportionalGain = 0.04;\n> +\n> +/*\n> + * Maximum multiplicative step per frame, to bound the correction when the\n> + * scene changes dramatically.\n> + */\n> +static constexpr float kExpMaxStep = 0.15;\n> +\n> Agc::Agc()\n> {\n> }\n> \n> void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)\n> {\n> -\t/*\n> -\t * kExpDenominator of 10 gives ~10% increment/decrement;\n> -\t * kExpDenominator of 5 - about ~20%\n> -\t */\n> -\tstatic constexpr uint8_t kExpDenominator = 10;\n> -\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n> -\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> -\n> \tint32_t &exposure = frameContext.sensor.exposure;\n> \tdouble &again = frameContext.sensor.gain;\n> \n> -\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> +\tdouble error = kExposureOptimal - exposureMSV;\n> +\n> +\tif (std::abs(error) <= kExposureSatisfactory)\n> +\t\treturn;\n> +\n> +\t/*\n> +\t * Compute a proportional correction factor. The sign of the error\n> +\t * determines the direction: positive error means too dark (increase),\n> +\t * negative means too bright (decrease).\n> +\t */\n> +\tfloat step = std::clamp(static_cast(error) * kExpProportionalGain,\n> +\t\t\t\t-kExpMaxStep, kExpMaxStep);\n> +\tfloat factor = 1.0f + step;\n> +\n> +\tif (factor > 1.0f) {\n> +\t\t/* Scene too dark: increase exposure first, then gain. */\n> \t\tif (exposure < context.configuration.agc.exposureMax) {\n> -\t\t\tint32_t next = exposure * kExpNumeratorUp / kExpDenominator;\n> -\t\t\tif (next - exposure < 1)\n> -\t\t\t\texposure += 1;\n> -\t\t\telse\n> -\t\t\t\texposure = next;\n> +\t\t\tint32_t next = static_cast(exposure * factor);\n> +\t\t\texposure = std::max(next, exposure + 1);\n> \t\t} else {\n> -\t\t\tdouble next = again * kExpNumeratorUp / kExpDenominator;\n> +\t\t\tdouble next = again * factor;\n> \t\t\tif (next - again < context.configuration.agc.againMinStep)\n> \t\t\t\tagain += context.configuration.agc.againMinStep;\n> \t\t\telse\n> \t\t\t\tagain = next;\n> \t\t}\n> -\t}\n> -\n> -\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n> +\t} else {\n> +\t\t/* Scene too bright: decrease gain first, then exposure. */\n> \t\tif (again > context.configuration.agc.again10) {\n> -\t\t\tdouble next = again * kExpNumeratorDown / kExpDenominator;\n> +\t\t\tdouble next = again * factor;\n> \t\t\tif (again - next < context.configuration.agc.againMinStep)\n> \t\t\t\tagain -= context.configuration.agc.againMinStep;\n> \t\t\telse\n> \t\t\t\tagain = next;\n> \t\t} else {\n> -\t\t\tint32_t next = exposure * kExpNumeratorDown / kExpDenominator;\n> -\t\t\tif (exposure - next < 1)\n> -\t\t\t\texposure -= 1;\n> -\t\t\telse\n> -\t\t\t\texposure = next;\n> +\t\t\tint32_t next = static_cast(exposure * factor);\n> +\t\t\texposure = std::min(next, exposure - 1);\n> \t\t}\n> \t}\n> \n> @@ -96,6 +120,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\n> \n> \tLOG(IPASoftExposure, Debug)\n> \t\t<< \"exposureMSV \" << exposureMSV\n> +\t\t<< \" error \" << error << \" factor \" << factor\n> \t\t<< \" exp \" << exposure << \" again \" << again;\n> }\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 4DFDDBDCB5\n\tfor ;\n\tThu, 7 May 2026 14:33:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7BBE663024;\n\tThu, 7 May 2026 16:33:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2286862FEC\n\tfor ;\n\tThu, 7 May 2026 16:33:14 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-70f3-e800--a06.rev.dnainternet.fi\n\t[IPv6:2001:14ba:70f3:e800::a06])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C4A30664;\n\tThu, 7 May 2026 16:33:09 +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=\"QyQr9ZrN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778164390;\n\tbh=NcWa5c0n92UmFwsDObRfeJWXcXSzZ4x5VUOqbJIw/4g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QyQr9ZrN8mgFcRrVlK0AUxpf/OmBUZFtqbjccZMwHLwQ7WAOP2crLueTMJSwW9w9n\n\tXJ8Z7C/O1x+G6aFFfB6LsR3oRLPXpD79f9RllgDC9fjVnd2vJnzuNmBa5eNiIDGS1F\n\tZ7a/Qq8ifOhMxi6cJL9dwAZhJRD6dtt7z+ZvWJj0=","Date":"Thu, 7 May 2026 17:33:12 +0300","From":"Laurent Pinchart ","To":"Javier Tia ","Cc":"libcamera-devel@lists.libcamera.org, mzamazal@redhat.com,\n\tbarnabas.pocze@ideasonboard.com, kieran.bingham@ideasonboard.com,\n\tjohannes.goede@oss.qualcomm.com","Subject":"Re: [PATCH v5 1/3] ipa: simple: agc: Replace bang-bang controller\n\twith proportional","Message-ID":"<20260507143312.GM1938994@killaraus.ideasonboard.com>","References":"<177810597783.688418.1631246733707368646@jetm.me>\n\t<20260506221942.55C4F1EA006B@mailuser.phl.internal>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20260506221942.55C4F1EA006B@mailuser.phl.internal>","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\" "}}]