Patch Detail
Show a patch.
GET /api/patches/26257/?format=api
{ "id": 26257, "url": "https://patchwork.libcamera.org/api/patches/26257/?format=api", "web_url": "https://patchwork.libcamera.org/patch/26257/", "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": "<20260304232526.E38A91EA006B@mailuser.phl.internal>", "date": "2026-03-04T23:01:29", "name": "[v2,1/4] 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/26257/mbox/", "series": [ { "id": 5820, "url": "https://patchwork.libcamera.org/api/series/5820/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5820", "date": "2026-03-04T23:00:56", "name": "Simple pipeline: proportional AGC, AWB stats fix, OV2740 tuning", "version": 2, "mbox": "https://patchwork.libcamera.org/series/5820/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/26257/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/26257/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 C9D6ABE086\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 4 Mar 2026 23:25:29 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8103C625C1;\n\tThu, 5 Mar 2026 00:25:29 +0100 (CET)", "from fhigh-b2-smtp.messagingengine.com\n\t(fhigh-b2-smtp.messagingengine.com [202.12.124.153])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 196CC623A8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 5 Mar 2026 00:25:28 +0100 (CET)", "from phl-compute-02.internal (phl-compute-02.internal\n\t[10.202.2.42])\n\tby mailfhigh.stl.internal (Postfix) with ESMTP id 371FD7A008A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 4 Mar 2026 18:25:27 -0500 (EST)", "from phl-imap-07 ([10.202.2.97])\n\tby phl-compute-02.internal (MEProxy); Wed, 04 Mar 2026 18:25:27 -0500", "by mailuser.phl.internal (Postfix, from userid 501)\n\tid E38A91EA006B; Wed, 4 Mar 2026 18:25:26 -0500 (EST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=jetm.me header.i=@jetm.me header.b=\"LsR79zdG\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"YHJLDMAm\"; \n\tdkim-atps=neutral", "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/relaxed; d=jetm.me; h=cc\n\t:content-transfer-encoding:content-type:content-type:date:date\n\t:from:from:in-reply-to:in-reply-to:message-id:mime-version\n\t:references:reply-to:subject:subject:to:to; s=fm3; t=1772666727;\n\tx=1772753127; bh=OaeDzIOakd+ONgmodunrnm7nSLAmRXKVvpXL1/Le0Aw=; b=\n\tLsR79zdGYULr5aJM8o9xRcVUdOjNNj5eKz9G8FfigwdYqDgV3DNkrqNNIRkTASnb\n\tsY5nm3KCw6aoFcNbrmPtiJAoD/l01qopYCEFboaGU/K5/G5xbtRtKjXt/Z1Ye4pj\n\tqUrGzcjbein/Sf3AOxUV1WT0GZ5lMWx759elfL46TTvz0poCgS4somkvLF1gDjG1\n\tcaIdNNHbg6bi+j7JI54Ub7782kZ9SXasyhQ1Ll/Irs1IlHTX099lvhc1yvKcJz8Q\n\txlkQX4WLYuRnNV8jCoNvt4xtFdoM7ey9KZGDjglhrRuatmBiNP21O0NLB4vVt7R/\n\t9u+WeaGf3f1XvycfO4eicw==", "v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:content-transfer-encoding:content-type\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=fm1; t=1772666727; x=1772753127; bh=O\n\taeDzIOakd+ONgmodunrnm7nSLAmRXKVvpXL1/Le0Aw=; b=YHJLDMAmYJezYYXls\n\t4qJkPYquRWK641Zh0UrLowpCYTUh09G/WnlJQfMnuRJKGfBZZgdUnkcoRDuGv7Jt\n\txqg2U3bISYvHfHjwcfos82Rq+jFzK9Qqr79nqf8pzqdmOzdNu99JFJjDL+ipwkWo\n\tBYBOJbhhzq1AJ+60gl0ODKWSuwNLfzs/Amb5U4MDGjQPur0tn6NMNYHjZwG3ua5L\n\tzmMFAChoI4A5UzpOwl3e70VH2PFR0hs3qXJpb0kjvqdEEryEubWCXZIZQR4OoXeN\n\td5mVI8r13JYMDe9omnPqHf6aibWTyNTKqD+ALXu9qFU0jycCl0luCkZv78M9wQIm\n\tgXU+A==" ], "X-ME-Sender": "<xms:Z7-oaYAORhMqZZyA3DqF-B5OMe2xG6T90GFqnI0e7rxWfaqMNwQ25A>\n\t<xme:Z7-oaVUAal_RWpmxmhguAA19KCDwXSguJoNSuMblksbN9WtBbbnwEi3jHkB7JYh1j\n\tIujoQArb5FWliQBWqhgoE5HWE3eMCHIjgONFNagz9ffsRXHH3lW4g>", "X-ME-Proxy-Cause": "gggruggvucftvghtrhhoucdtuddrgeefgedrtddtgddvieegkeduucetufdoteggodetrf\n\tdotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu\n\trghilhhouhhtmecufedttdenucgopfhokfffucdluddtmdenucfjughrpefotggggffhvf\n\tffufgjfhesthektddtredtjeenucfhrhhomheplfgrvhhivghrucfvihgruceofhhlohhs\n\tshesjhgvthhmrdhmvgeqnecuggftrfgrthhtvghrnhepteetvdeklefgledtjefhhfdtle\n\tdvhfffgfeuieelgeeigfekgffhleeghfefhfejnecuvehluhhsthgvrhfuihiivgepuden\n\tucfrrghrrghmpehmrghilhhfrhhomhepfhhlohhsshesjhgvthhmrdhmvgdpnhgspghrtg\n\thpthhtohepuddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhisggtrghmvghr\n\trgdquggvvhgvlheslhhishhtshdrlhhisggtrghmvghrrgdrohhrgh", "X-ME-Proxy": "<xmx:Z7-oaVTjznwwxCMFph82WN_B2urqzK1PqdABSWwTq0DqSGgo-vXWew>\n\t<xmx:Z7-oaevDOdYdzGftN8NSzkPaMvAQpm2kFgaJ678qq5yfgyk8WjKJWg>\n\t<xmx:Z7-oaXuCZiTmCevHMa797cqhFMrqeTTEYTvbrPWVW78WIlnAzluB0g>\n\t<xmx:Z7-oaUz97k0SNsjb0eyO5QYr2sSsiBbo1DWI_YUr28d7CZsoFTRkIg>\n\t<xmx:Z7-oaZkdxV9enA7IkwWcFQx_TWfVkgCSyimOxrj1Hyv76rHsYFL8iYrH>", "Feedback-ID": "i9dde48b3:Fastmail", "X-Mailer": "MessagingEngine.com Webmail Interface", "Content-Type": "text/plain; charset=\"utf-8\"", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "From": "Javier Tia <floss@jetm.me>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 04 Mar 2026 17:01:29 -0600", "Subject": "[PATCH v2 1/4] ipa: simple: agc: Replace bang-bang controller with\n\tproportional", "In-Reply-To": "20260225221859.600869-1-floss@jetm.me", "References": "20260225221859.600869-1-floss@jetm.me", "Message-Id": "<20260304232526.E38A91EA006B@mailuser.phl.internal>", "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": [ "v2", "1/4" ] }