[{"id":38336,"web_url":"https://patchwork.libcamera.org/comment/38336/","msgid":"<85a4wmdttq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-03-05T18:17:37","subject":"Re: [PATCH v2 1/4] ipa: simple: agc: Replace bang-bang controller\n\twith proportional","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Javier Tia writes:\n\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 existing\n> hysteresis (kExposureSatisfactory) still 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\nReviewed-by: Milan Zamazal \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> }","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 EA1C6BE086\n\tfor ;\n\tThu, 5 Mar 2026 18:17:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 13C9A625D1;\n\tThu, 5 Mar 2026 19:17:45 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E4FB2622AE\n\tfor ;\n\tThu, 5 Mar 2026 19:17:43 +0100 (CET)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-392-3s544D3_NZSW6glsorDedQ-1; Thu, 05 Mar 2026 13:17:41 -0500","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-48378c4a79fso74183085e9.0\n\tfor ;\n\tThu, 05 Mar 2026 10:17:41 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-439ab6ebe56sm42157150f8f.15.2026.03.05.10.17.38\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 05 Mar 2026 10:17:38 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"dXVp7lkp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1772734662;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=pTmEGrDXjq1ek5xrF69asPZq3MF5Nd/6vA6QBC3PEYk=;\n\tb=dXVp7lkpJoZGxb4oxQeS11LCu7/7poFrhec2rt+HunB2sbcyBbqZHzQqd8a1EqAY80aFp1\n\t1CCXGHKTDeBY15WN257/ab04BKnpIzsIByY+8JUnrDvjS9YjD6TrsJq+GJDhGKuZIR+guM\n\t0xR5EQZVb7eGBY0GMIT9W+wxZ2FcTzM=","X-MC-Unique":"3s544D3_NZSW6glsorDedQ-1","X-Mimecast-MFC-AGG-ID":"3s544D3_NZSW6glsorDedQ_1772734660","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1772734660; x=1773339460;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=pTmEGrDXjq1ek5xrF69asPZq3MF5Nd/6vA6QBC3PEYk=;\n\tb=j77gvh3CLaMOjgRALKYVJpANZxyi7KaUYdhIJKXG8LatZxxUn7b3QLI06dN2Rfxjkk\n\tcfB821CeG3/c7+XGksd0pY6MINxA2+y4OHQeqBPbjU3P4nEFS4d5o30FZCdOmfJBO1KA\n\tCB2T1EzqbQ7g9nGpOUfKo8fZ5c+BjfUpPq1SxMMVWNuSFQPzgwTA/4bARBVvtg3cRXx3\n\tZ4I90Cv81SliLkk1BAT3EccijxaUoQHqj2ALHCwy3Wk3LK0QsJUR0SUTYFhVJsyj+BpH\n\tgmzwqfHxyxdhRugalFO3QiIad4RTDdFirVfhyBIoZNjfTL7utWQRXcE8TczpYjjl1m7t\n\tYKoA==","X-Gm-Message-State":"AOJu0YzQiTNXHywHJJINzdw/V6v6MKXbCyhZItB6kP+eLkoXBWiTlEgh\n\tgy4WqxvcHBOoi3ObzNi2KSaMp74Vr4XSyOWcKf0uz9utd9NIxTpnDrm41rJQDUZeC7pQMaoRmlf\n\tW39/ME88kIgImoqAIVjtmGshjz5NRL5XsvKr8OsDnG2CuPQbho0VqyRkUPgn95sIRw8M+/i/nZN\n\tgDAFlx5+RA0JuJqGdEMeBRLeeKg6KUAjBY5CwfZIMtahCU9dba42KXkZs+aso=","X-Gm-Gg":"ATEYQzxIOa7aGNwkbZh97bR6zarWDxYLKmqbUAaOAvfo7+zyqrg/vHZt5WRGu6X82RZ\n\tX4fPiAH8tnfjFA1/GYBmkoSLIgONp0zF2fW/G4Dm04bBTqOxRjruUOVCgZU4l8Usnyo66w3/G4M\n\tpCZn4pTnT3WhhwlNlx8LAOc4HHfW7oKCh/R69hMgBuVAJxvOKs6LmjDLd3xJ4/lpK5brs1m4z3Z\n\tAfYyGa/TEKH7zHCq2iNlZiTBDsVHWta/Fi4bLCuC2t+Og9cR0DZtpSuAd39MYPs00kZ+iFfjrEz\n\t7bbytrExfWCvmssDJe47U5/CiWmuUsLe50vO2o+2OU0NceA622I1gw9VPkPzmhrc3bMxGzle2cX\n\tSe7lzIgd9PaATQyzeEYRiEnyqNswcO7mNg5plmHnyKXF170OgyW+NOOsEBPsK+44jSdIJJseayG\n\tg=","X-Received":["by 2002:a05:600c:a03:b0:47d:264e:b35a with SMTP id\n\t5b1f17b1804b1-48519854e97mr131039855e9.13.1772734659658; \n\tThu, 05 Mar 2026 10:17:39 -0800 (PST)","by 2002:a05:600c:a03:b0:47d:264e:b35a with SMTP id\n\t5b1f17b1804b1-48519854e97mr131039025e9.13.1772734659007; \n\tThu, 05 Mar 2026 10:17:39 -0800 (PST)"],"From":"Milan Zamazal ","To":"Javier Tia ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 1/4] ipa: simple: agc: Replace bang-bang controller\n\twith proportional","In-Reply-To":"<20260304232526.E38A91EA006B@mailuser.phl.internal> (Javier\n\tTia's message of \"Wed, 04 Mar 2026 17:01:29 -0600\")","References":"<20260304232526.E38A91EA006B@mailuser.phl.internal>","Date":"Thu, 05 Mar 2026 19:17:37 +0100","Message-ID":"<85a4wmdttq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"p7EDtI7RLmnQKnno_GcLGOEKNtpRgHHY1uNFFtliKgY_1772734660","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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\" "}}]