Patch Detail
Show a patch.
GET /api/1.1/patches/24444/?format=api
{ "id": 24444, "url": "https://patchwork.libcamera.org/api/1.1/patches/24444/?format=api", "web_url": "https://patchwork.libcamera.org/patch/24444/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20250923190657.21453-6-hansg@kernel.org>", "date": "2025-09-23T19:06:57", "name": "[5/5] ipa: software_isp: AGC: Stop using delayed control for previous values", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "020a673318b80b3f50b1ad7655a2ad278721edda", "submitter": { "id": 239, "url": "https://patchwork.libcamera.org/api/1.1/people/239/?format=api", "name": "Hans de Goede", "email": "hansg@kernel.org" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/24444/mbox/", "series": [ { "id": 5455, "url": "https://patchwork.libcamera.org/api/1.1/series/5455/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5455", "date": "2025-09-23T19:06:52", "name": "ipa: software_isp: AGC: Fox AGC oscillation bug", "version": 1, "mbox": "https://patchwork.libcamera.org/series/5455/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/24444/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/24444/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 31361C32A9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Sep 2025 19:07:19 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B3A66B61A;\n\tTue, 23 Sep 2025 21:07:18 +0200 (CEST)", "from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 336CC6B608\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Sep 2025 21:07:08 +0200 (CEST)", "from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby tor.source.kernel.org (Postfix) with ESMTP id 6693C60278\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Sep 2025 19:07:07 +0000 (UTC)", "by smtp.kernel.org (Postfix) with ESMTPSA id 616BDC113D0;\n\tTue, 23 Sep 2025 19:07:06 +0000 (UTC)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=kernel.org header.i=@kernel.org\n\theader.b=\"ZLwO21v8\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1758654427;\n\tbh=CCZwD+CJ73bAxaZILpNf1Z+g+VTllZSU3Kfzx+PICJo=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=ZLwO21v8l2nuSKTSwlU0e0HrtbJyTcgYoTmRGq01ImSZWGWgnjd93fRlk6HDp5fk+\n\tGtZl7I+0n+Cripchh3UwOXgrTfIIaakb3Snxm1rmjGSSVGjO++pFPAMTTLJlqDXcBA\n\tiKsSulbjwy078SMWeAdREZ7ch5x40iJ65+PFTrgxPXAGNLmfCypz+ueoGc3x519h9s\n\tmg6Z2IIf1Wp9dPhViwl+xWvDbepS4GcDCd6Cj9zPpXSsCAVJ//VzDXzuiTdmSb0huG\n\tmV/3JLhYTf/hIhZ7gJorxnf9kviBkfzESIN9SzOaVInGbXbpE6qKT+ahTk50zXDOAM\n\trcNP4QO++YhFA==", "From": "Hans de Goede <hansg@kernel.org>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Hans de Goede <hansg@kernel.org>", "Subject": "[PATCH 5/5] ipa: software_isp: AGC: Stop using delayed control for\n\tprevious values", "Date": "Tue, 23 Sep 2025 21:06:57 +0200", "Message-ID": "<20250923190657.21453-6-hansg@kernel.org>", "X-Mailer": "git-send-email 2.51.0", "In-Reply-To": "<20250923190657.21453-1-hansg@kernel.org>", "References": "<20250923190657.21453-1-hansg@kernel.org>", "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": "Due to a combination of not having correct control-delay information for\nvarious sensors as well as the generic nature of the simple-pipeline +\nsoftware-ISP meaning that any pipeline delays are unknown it is impossible\nto get reliable control-delay values.\n\nWrong control-delay values can lead to pretty bad oscilation. Since atm\nit is not possible to fix the wrong control-delay values, stop using\nthe old sensor values reported by the delayed-controlled mechanism.\n\nInstead remember the gain and exposures set the last time the algorithm\nruns (initializing the cached values with the actual sensor values on\nthe first frame), combined with skipping a fixed number of frames after\nchanging values.\n\nSigned-off-by: Hans de Goede <hansg@kernel.org>\n---\n src/ipa/simple/algorithms/agc.cpp | 30 +++++++++++++++++++++++++-----\n src/ipa/simple/algorithms/agc.h | 3 ++-\n src/ipa/simple/ipa_context.h | 6 ++++++\n src/ipa/simple/soft_simple.cpp | 4 ++--\n 4 files changed, 35 insertions(+), 8 deletions(-)", "diff": "diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\nindex cdde56ba2..3c0e20ddc 100644\n--- a/src/ipa/simple/algorithms/agc.cpp\n+++ b/src/ipa/simple/algorithms/agc.cpp\n@@ -41,7 +41,15 @@ Agc::Agc()\n {\n }\n \n-void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)\n+int Agc::configure(IPAContext &context,\n+\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n+{\n+\tcontext.activeState.agc.skipFrames = 0;\n+\n+\treturn 0;\n+}\n+\n+void Agc::updateExposure(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, double exposureMSV)\n {\n \t/*\n \t * kExpDenominator of 10 gives ~10% increment/decrement;\n@@ -51,8 +59,18 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\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+\tint32_t &skipFrames = context.activeState.agc.skipFrames;\n+\tint32_t &exposure = context.activeState.agc.exposure;\n+\tdouble &again = context.activeState.agc.again;\n+\n+\t/* Set initial-gain values from sensor on first frame */\n+\tif (frame == 0) {\n+\t\texposure = frameContext.sensor.exposure;\n+\t\tagain = frameContext.sensor.gain;\n+\t}\n+\n+\tif (skipFrames && --skipFrames)\n+\t\treturn;\n \n \tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n \t\tif (exposure < context.configuration.agc.exposureMax) {\n@@ -68,6 +86,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\n \t\t\telse\n \t\t\t\tagain = next;\n \t\t}\n+\t\tskipFrames = 3;\n \t}\n \n \tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n@@ -84,6 +103,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\n \t\t\telse\n \t\t\t\texposure = next;\n \t\t}\n+\t\tskipFrames = 3;\n \t}\n \n \texposure = std::clamp(exposure, context.configuration.agc.exposureMin,\n@@ -97,7 +117,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\n }\n \n void Agc::process(IPAContext &context,\n-\t\t [[maybe_unused]] const uint32_t frame,\n+\t\t const uint32_t frame,\n \t\t IPAFrameContext &frameContext,\n \t\t const SwIspStats *stats,\n \t\t ControlList &metadata)\n@@ -135,7 +155,7 @@ void Agc::process(IPAContext &context,\n \t}\n \n \tfloat exposureMSV = (denom == 0 ? 0 : static_cast<float>(num) / denom);\n-\tupdateExposure(context, frameContext, exposureMSV);\n+\tupdateExposure(context, frame, frameContext, exposureMSV);\n }\n \n REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\ndiff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\nindex 112d9f5a1..ef387664f 100644\n--- a/src/ipa/simple/algorithms/agc.h\n+++ b/src/ipa/simple/algorithms/agc.h\n@@ -19,13 +19,14 @@ public:\n \tAgc();\n \t~Agc() = default;\n \n+\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n \tvoid process(IPAContext &context, const uint32_t frame,\n \t\t IPAFrameContext &frameContext,\n \t\t const SwIspStats *stats,\n \t\t ControlList &metadata) override;\n \n private:\n-\tvoid updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV);\n+\tvoid updateExposure(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, double exposureMSV);\n };\n \n } /* namespace ipa::soft::algorithms */\ndiff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\nindex ba525a881..cdab980a1 100644\n--- a/src/ipa/simple/ipa_context.h\n+++ b/src/ipa/simple/ipa_context.h\n@@ -37,6 +37,12 @@ struct IPASessionConfiguration {\n };\n \n struct IPAActiveState {\n+\tstruct {\n+\t\tint32_t skipFrames;\n+\t\tint32_t exposure;\n+\t\tdouble again;\n+\t} agc;\n+\n \tstruct {\n \t\tuint8_t level;\n \t\tint32_t lastExposure;\ndiff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex f0764ef46..f8c6291e2 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -327,8 +327,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n \n \tControlList ctrls(sensorInfoMap_);\n \n-\tauto &againNew = frameContext.sensor.gain;\n-\tctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure);\n+\tauto &againNew = context_.activeState.agc.again;\n+\tctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n \tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n \t\t static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n \n", "prefixes": [ "5/5" ] }