{"id":26648,"url":"https://patchwork.libcamera.org/api/patches/26648/?format=json","web_url":"https://patchwork.libcamera.org/patch/26648/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20260506221942.55C4F1EA006B@mailuser.phl.internal>","date":"2026-05-06T21:46:01","name":"[v5,1/3] ipa: simple: agc: Replace bang-bang controller with proportional","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"0596f7aea8e8e4d80bca998883aa5fb13a67be8a","submitter":{"id":261,"url":"https://patchwork.libcamera.org/api/people/261/?format=json","name":"Javier Tia","email":"floss@jetm.me"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/26648/mbox/","series":[{"id":5913,"url":"https://patchwork.libcamera.org/api/series/5913/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5913","date":"2026-05-06T21:45:28","name":"ipa: simple: Proportional AGC and statistics normalization","version":5,"mbox":"https://patchwork.libcamera.org/series/5913/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/26648/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/26648/checks/","tags":{},"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 C7069C32F6\n\tfor ;\n\tWed, 6 May 2026 22:19:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1EF266302E;\n\tThu, 7 May 2026 00:19:46 +0200 (CEST)","from fhigh-a6-smtp.messagingengine.com\n\t(fhigh-a6-smtp.messagingengine.com [103.168.172.157])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 310876301A\n\tfor ;\n\tThu, 7 May 2026 00:19:43 +0200 (CEST)","from phl-compute-02.internal (phl-compute-02.internal\n\t[10.202.2.42])\n\tby mailfhigh.phl.internal (Postfix) with ESMTP id 74A5714000FF;\n\tWed, 6 May 2026 18:19:42 -0400 (EDT)","from phl-imap-07 ([10.202.2.97])\n\tby phl-compute-02.internal (MEProxy); Wed, 06 May 2026 18:19:42 -0400","by mailuser.phl.internal (Postfix, from userid 501)\n\tid 55C4F1EA006B; Wed, 6 May 2026 18:19:42 -0400 (EDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=jetm.me header.i=@jetm.me header.b=\"CmGqEsCA\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"R98w9VOA\"; \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: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=fm2; t=1778105982;\n\tx=1778192382; bh=+uCojCTKCMOFVIV59W7fSxyYCD3fDWoJOIKPEBSOamQ=; b=\n\tCmGqEsCAEW/7L4YCkQ99iPcFk2FUmHP1qcFrYZjDf2ps7enCm1Zl21wvqlqlHPFn\n\t8NPE6cRhQGpmLegWOWXG7D3eYQ+lr1GFIGa/UOKimI4CxZjNLpTOxmgyIAz4AB3l\n\tD/0mIAufA61ixWJblF7IMUQTBaWZtEUvpifRUrgwYVJuv9o/6ZMyHIiFEZQkyrf6\n\tMbmAVIpTZYZV0/vIfY2VlEDV0OxsKur/XnvE20TmsrG7auVQP5k9EfYKfrk02I7/\n\tIFF4kCO7s2Uf6+reCnaG5fpFYzZHVUen4tRuNwLs+/lifIRUjCONazhudMu3tjlv\n\t1R88wPVrdrHejx39lJ1exw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:cc:content-transfer-encoding\n\t:content-type:content-type:date:date:feedback-id:feedback-id\n\t:from:from:in-reply-to:in-reply-to:message-id:mime-version\n\t:references:reply-to:subject:subject:to:to:x-me-proxy\n\t:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1778105982; x=\n\t1778192382; bh=+uCojCTKCMOFVIV59W7fSxyYCD3fDWoJOIKPEBSOamQ=; b=R\n\t98w9VOARBcxVEGmiXkiPnIBsBldJHOUwe1RZ7XQDk6Y6s03qLHZTLNSXJNQRoTan\n\tiG/YAu493ACmUIy4VRP2L3HneyOBiSgyGVQaQmMIIwsPCmLRxTSfAFnJ1E+diIUA\n\ts/nYD76n/uwitCK2si/fLSoNRe9VWiY/SoPaIpmt43ImdZwn1XcoAItTHOl04g76\n\taoxIC4WWPFEJQcs5iIkMnueAHpKID5oHDckVlxBkETOKZs3Q2JFIxqoNTy26xFor\n\t3t7i3GRfsdmMxZoJRo12uGGlCsk7T+Uy4/OeM4r78C6eNrnYRNymULer66E6QgnU\n\tIq0R6MQKN/4SGtHiiwSpg=="],"X-ME-Sender":"\n\t","X-ME-Proxy-Cause":"gggruggvucftvghtrhhoucdtuddrgeefhedrtddtgddutdehjeejucetufdoteggodetrf\n\tdotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu\n\trghilhhouhhtmecufedttdenucgopfhokfffucdluddtmdenucfjughrpefotggggffhvf\n\tffufevjghfsehtkedttdertdejnecuhfhrohhmpeflrghvihgvrhcuvfhirgcuoehflhho\n\tshhssehjvghtmhdrmhgvqeenucggtffrrghtthgvrhhnpedtudejffejkeekteelueefvd\n\tejvdeuhfefteehkeevtddvleduteekleetvdelhfenucevlhhushhtvghrufhiiigvpedt\n\tnecurfgrrhgrmhepmhgrihhlfhhrohhmpehflhhoshhssehjvghtmhdrmhgvpdhnsggprh\n\tgtphhtthhopeehpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopegsrghrnhgrsggr\n\tshdrphhotgiivgesihguvggrshhonhgsohgrrhgurdgtohhmpdhrtghpthhtohepkhhivg\n\thrrghnrdgsihhnghhhrghmsehiuggvrghsohhnsghorghrugdrtghomhdprhgtphhtthho\n\tpehlihgstggrmhgvrhgrqdguvghvvghlsehlihhsthhsrdhlihgstggrmhgvrhgrrdhorh\n\thgpdhrtghpthhtohepjhhohhgrnhhnvghsrdhgohgvuggvsehoshhsrdhquhgrlhgtohhm\n\tmhdrtghomhdprhgtphhtthhopehmiigrmhgriigrlhesrhgvughhrghtrdgtohhm","X-ME-Proxy":"\n\t\n\t\n\t\n\t","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 ","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 06 May 2026 15:46:01 -0600","Subject":"[PATCH v5 1/3] ipa: simple: agc: Replace bang-bang controller with\n\tproportional","Cc":"mzamazal@redhat.com, barnabas.pocze@ideasonboard.com,\n\tkieran.bingham@ideasonboard.com, johannes.goede@oss.qualcomm.com","In-Reply-To":"<177810597783.688418.1631246733707368646@jetm.me>","References":"<177810597783.688418.1631246733707368646@jetm.me>","Message-Id":"<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\" "},"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 step is\nclamped to +/-15% to bound corrections when the scene changes\ndramatically. The existing hysteresis (kExposureSatisfactory) still\nprevents 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 \nReviewed-by: Milan Zamazal \nTested-by: Barnabás Pőcze \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 \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 \n","prefixes":["v5","1/3"]}