Patch Detail
Show a patch.
GET /api/patches/26238/?format=api
{ "id": 26238, "url": "https://patchwork.libcamera.org/api/patches/26238/?format=api", "web_url": "https://patchwork.libcamera.org/patch/26238/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20260225221859.600869-2-floss@jetm.me>", "date": "2026-02-25T22:18:58", "name": "[1/2] ipa: simple: agc: Replace bang-bang controller with proportional", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "0596f7aea8e8e4d80bca998883aa5fb13a67be8a", "submitter": { "id": 261, "url": "https://patchwork.libcamera.org/api/people/261/?format=api", "name": "Javier Tia", "email": "floss@jetm.me" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/26238/mbox/", "series": [ { "id": 5812, "url": "https://patchwork.libcamera.org/api/series/5812/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5812", "date": "2026-02-25T22:18:57", "name": "ipa: simple: Proportional AGC and OV2740 tuning", "version": 1, "mbox": "https://patchwork.libcamera.org/series/5812/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/26238/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/26238/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 DA724C32C8\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 25 Feb 2026 22:20:18 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8CE9622B2;\n\tWed, 25 Feb 2026 23:20:16 +0100 (CET)", "from fhigh-a2-smtp.messagingengine.com\n\t(fhigh-a2-smtp.messagingengine.com [103.168.172.153])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65FCD61FA0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 25 Feb 2026 23:20:15 +0100 (CET)", "from phl-compute-05.internal (phl-compute-05.internal\n\t[10.202.2.45])\n\tby mailfhigh.phl.internal (Postfix) with ESMTP id B2B981400215;\n\tWed, 25 Feb 2026 17:20:14 -0500 (EST)", "from phl-frontend-04 ([10.202.2.163])\n\tby phl-compute-05.internal (MEProxy); Wed, 25 Feb 2026 17:20:14 -0500", "by mail.messagingengine.com (Postfix) with ESMTPA; Wed,\n\t25 Feb 2026 17:20:14 -0500 (EST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=jetm.me header.i=@jetm.me header.b=\"N5awxEPK\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"pyyjpXbq\"; \n\tdkim-atps=neutral", "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/relaxed; d=jetm.me; h=cc:cc\n\t:content-transfer-encoding:content-type:date:date:from:from\n\t:in-reply-to:in-reply-to:message-id:mime-version:references\n\t:reply-to:subject:subject:to:to; s=fm2; t=1772058014; x=\n\t1772144414; bh=OaeDzIOakd+ONgmodunrnm7nSLAmRXKVvpXL1/Le0Aw=; b=N\n\t5awxEPKoLqklOzqg/odFBLmaPhPh1Jf927sH/oNdpdko+HHlelOPmRzVx62vDFP+\n\tqZsM+wV+32FxIdM/UD6Ftbrw06Fwfk1E1wR7MRS7E/95qVt/rP3Nhty5ONG6RVtm\n\tPa0EkQ7DB9wnq2FWEX3AXV+yUlHi2mLDwLh62JpELURBAouvVXbn6In9d9k3t9pe\n\tDweEX3njnCtGF899P64LtbS2+6kC8YYow4rCxNXFrztuAtLxIWiVZ/vxBMUpGGBB\n\tzQ6SxD7sGymtgD/dgwQMclBmJeHpBFAzS6HlHM9M88BAf0wkIlzngvQUaVQbts6c\n\tP/A6J3pG4O00TkGnzshBw==", "v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:cc:content-transfer-encoding\n\t:content-type:date:date:feedback-id:feedback-id:from:from\n\t:in-reply-to:in-reply-to:message-id:mime-version:references\n\t:reply-to:subject:subject:to:to:x-me-proxy:x-me-sender\n\t:x-me-sender:x-sasl-enc; s=fm3; t=1772058014; x=1772144414; bh=O\n\taeDzIOakd+ONgmodunrnm7nSLAmRXKVvpXL1/Le0Aw=; b=pyyjpXbq8lLt77nIR\n\trjgvRi2MRwNW1S6hMlORxKmljR9gM0jgE9S4Fg7OKy+KhrsSdw+cle1MlN7NAknC\n\tr8Ha1vuTlrkV9BmuvgYhbvNGWATws45vE9XjQ7LTw+7fX29x8hs+jZGY04QxyYRK\n\tzaCtdJ1gbkqXqP/cMCTzZjzaWWU4gP810iZz91yCJruLoq59VvPzfkDiRRorx1EI\n\tLA6Za8SXSLlJohGaT1q6ubl19KsL1Nek/hirxYtAuUIti4SavPbRwhAGPv2MwQVJ\n\t6xx2g352AgJZsecZTkcEHUAZn6Yy7IxUFLlAMSnqtTjHAwIN/g50VV8EDXxMHEkE\n\tDBrpw==" ], "X-ME-Sender": "<xms:nnWfaceWRuZygZoAOpGGGHCPWfgaBzlRvPFeuveViDLSje8iRbnInw>\n\t<xme:nnWfaUOcjOEyVXMn-yRSIHmjxtOi3FlUx8LJWmqaMDLySt9KXyGtNEhPCjbKVyM3p\n\ttzvRVhOnBF9gxU3pX7ou5BIonwmsBO1laxLElDtO_eunkCZFNpGoy8>", "X-ME-Received": "<xmr:nnWfabKwT7lovPJOJQaj4AA3KSZGjpL-TyOwmtLAkB4IPwTw0__iOGlpyb2tc2zcPFuMyh5vR0UHEJTh1TmvjQ>", "X-ME-Proxy-Cause": "gggruggvucftvghtrhhoucdtuddrgeefgedrtddtgddvgeegfedtucetufdoteggodetrf\n\tdotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu\n\trghilhhouhhtmecufedttdenucenucfjughrpefhvfevufffkffojghfggfgsedtkeertd\n\tertddtnecuhfhrohhmpeflrghvihgvrhcuvfhirgcuoehflhhoshhssehjvghtmhdrmhgv\n\tqeenucggtffrrghtthgvrhhnpeeiueehgeevudeiheeuheevgffhtdevheeuffeiieduff\n\tffvdeftdejfefftdekheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgr\n\tihhlfhhrohhmpehflhhoshhssehjvghtmhdrmhgvpdhnsggprhgtphhtthhopedvpdhmoh\n\tguvgepshhmthhpohhuthdprhgtphhtthhopehlihgstggrmhgvrhgrqdguvghvvghlsehl\n\tihhsthhsrdhlihgstggrmhgvrhgrrdhorhhgpdhrtghpthhtohepfhhlohhsshesjhgvth\n\thmrdhmvg", "X-ME-Proxy": "<xmx:nnWfadFwoOfi1WVz1EqqL0qQzt3QhXCqu_PNXZegD3p38Ro_8Eze6w>\n\t<xmx:nnWfabRMHu2Oja8YiYVxBZsZ-XDWV6eK2iStyaiQ_EQHwz02IgAmNg>\n\t<xmx:nnWfaRFMWJrKgpUPP1fnNrNY4Ph6fUEVAs57groB5KofSjHrJ1tkdg>\n\t<xmx:nnWfaf9bMMUnWbnBOOUwpCW75B5M7YjyifkJdBmJn0mdu8mgK6W88g>\n\t<xmx:nnWfaSD3rWKYSkHSjRgpdjZEvPR6RVaFVDrVeoixXLC6hFSD8ww0a6Fm>", "Feedback-ID": "i9dde48b3:Fastmail", "From": "Javier Tia <floss@jetm.me>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Javier Tia <floss@jetm.me>", "Subject": "[PATCH 1/2] ipa: simple: agc: Replace bang-bang controller with\n\tproportional", "Date": "Wed, 25 Feb 2026 16:18:58 -0600", "Message-ID": "<20260225221859.600869-2-floss@jetm.me>", "X-Mailer": "git-send-email 2.53.0", "In-Reply-To": "<20260225221859.600869-1-floss@jetm.me>", "References": "<20260225221859.600869-1-floss@jetm.me>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The AGC's updateExposure() uses a fixed ~10% step per frame regardless\nof how far the current exposure is from optimal. With a hysteresis dead\nband of only +/-4%, the controller overshoots when the correct value\nfalls within one step, causing visible brightness oscillation (flicker).\n\nReplace the fixed-step bang-bang controller with a proportional one\nwhere the correction factor scales linearly with the MSV error:\n\n factor = 1.0 + clamp(error * 0.04, -0.15, +0.15)\n\nAt maximum error (~2.5), this gives the same ~10% step as before. Near\nthe target, steps shrink to <1%, eliminating overshoot. The existing\nhysteresis (kExposureSatisfactory) still prevents hunting on noise.\n\nTested on OV2740 behind Intel IPU6 ISYS (ThinkPad X1 Carbon Gen 10)\nwhere the old controller produced continuous brightness flicker. The\nproportional controller converges in ~3 seconds from cold start with\nno visible oscillation.\n\nSigned-off-by: Javier Tia <floss@jetm.me>\n---\n src/ipa/simple/algorithms/agc.cpp | 73 +++++++++++++++++++++----------\n 1 file changed, 49 insertions(+), 24 deletions(-)", "diff": "diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\nindex 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 <algorithm>\n+#include <cmath>\n #include <stdint.h>\n \n #include <libcamera/base/log.h>\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<float>(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<int32_t>(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<int32_t>(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 \n", "prefixes": [ "1/2" ] }