Patch Detail
Show a patch.
GET /api/1.1/patches/24538/?format=api
{ "id": 24538, "url": "https://patchwork.libcamera.org/api/1.1/patches/24538/?format=api", "web_url": "https://patchwork.libcamera.org/patch/24538/", "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": "<20250930150428.11101-6-hansg@kernel.org>", "date": "2025-09-30T15:04:27", "name": "[v4,5/6] libcamera: software_isp: Add valid flag to struct SwIspStats", "commit_ref": "9b441cf198ab7344885620de0291978c26b03a5e", "pull_url": null, "state": "accepted", "archived": false, "hash": "8fa777c555ca8810a9cfe1986fdc9d28334aac1e", "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/24538/mbox/", "series": [ { "id": 5470, "url": "https://patchwork.libcamera.org/api/1.1/series/5470/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5470", "date": "2025-09-30T15:04:22", "name": "ipa: software_isp: AGC: Fix AGC oscillation bug", "version": 4, "mbox": "https://patchwork.libcamera.org/series/5470/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/24538/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/24538/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 F3B35C328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 30 Sep 2025 15:04:48 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 618176B619;\n\tTue, 30 Sep 2025 17:04:48 +0200 (CEST)", "from sea.source.kernel.org (sea.source.kernel.org\n\t[IPv6:2600:3c0a:e001:78e:0:1991:8:25])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 320376B601\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 30 Sep 2025 17:04:43 +0200 (CEST)", "from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby sea.source.kernel.org (Postfix) with ESMTP id BE9F243CBF;\n\tTue, 30 Sep 2025 15:04:41 +0000 (UTC)", "by smtp.kernel.org (Postfix) with ESMTPSA id 7AFBDC4CEF0;\n\tTue, 30 Sep 2025 15:04:40 +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=\"fKCte6Oc\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1759244681;\n\tbh=FeUG4GKlhVLhUviw9aOdli4GM+DFNBO80MuynjHWCZQ=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=fKCte6OcQxYmCLSX8su0BkqRcNdNk1fsXF66EChSN+/A093o439c4/mKdxvWkvp1k\n\tLR8A+tmZhSPFeEg8SMv/8Lr3I8B1ASZb8tHWnWzDE6/6EkjTjccENVL3eigg683DZw\n\t/2anyFJpc4BsbA6XnFicwJlEVc+Cgt5YP4+PNSRZtf1jtUdS46JVCyEiQppZQ9iJNn\n\t5DIcAkS5Bs7zJ4ox7cZStEJWhAREElNoXweuTaC3xbCmAcWDxZDun/vBeTKyn2zETo\n\tV7C0epkLNeSRdSN3ZjUGNn6ZH6LoSO4A7BQKCxCZC+Zk1OCpntLKIKOLpMdPnQpIbw\n\trTAYMQwo5Y/FQ==", "From": "Hans de Goede <hansg@kernel.org>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Hans de Goede <hansg@kernel.org>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tMilan Zamazal <mzamazal@redhat.com>", "Subject": "[PATCH v4 5/6] libcamera: software_isp: Add valid flag to struct\n\tSwIspStats", "Date": "Tue, 30 Sep 2025 17:04:27 +0200", "Message-ID": "<20250930150428.11101-6-hansg@kernel.org>", "X-Mailer": "git-send-email 2.51.0", "In-Reply-To": "<20250930150428.11101-1-hansg@kernel.org>", "References": "<20250930150428.11101-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": "Generating statistics for every single frame is not really necessary.\n\nHowever a roundtrip through ipa_->processStats() still need to be done\nevery frame, even if there are no stats to make the IPA generate metadata\nfor every frame.\n\nAdd a valid flag to the statistics struct to let the IPA know when there\nare no statistics for the frame being processed and modify the IPA to\nonly generate metadata for frames without valid statistics.\n\nThis is a preparation patch for skipping statistics generation for some\nframes.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Milan Zamazal <mzamazal@redhat.com>\nTested-by: Milan Zamazal <mzamazal@redhat.com>\nTested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nSigned-off-by: Hans de Goede <hansg@kernel.org>\n---\nChanges in v4:\n- Add an empty line after the agc substruct declaration\n- Init active-state agc gain and exposue from sensor values in case\n updateExposure() does not run for the first frame\n\nChanges in v3:\n- Drop if (!stats_->valid) early exit from IPASoftSimple::processStats()\n- Save last AGC calculated new gain and exposure values and re-use these\n for frames without statistics, this avoids delayedCtrl history underruns\n- Improve SwIspStats.valid documentation\n\nChanges in v2:\n- This is a new patch in v2 of this series\n---\n .../internal/software_isp/swisp_stats.h | 5 ++++\n src/ipa/simple/algorithms/agc.cpp | 24 ++++++++++++++++++-\n src/ipa/simple/algorithms/awb.cpp | 3 +++\n src/ipa/simple/algorithms/blc.cpp | 3 +++\n src/ipa/simple/ipa_context.h | 5 ++++\n src/libcamera/software_isp/swstats_cpu.cpp | 1 +\n 6 files changed, 40 insertions(+), 1 deletion(-)", "diff": "diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h\nindex ae11f112e..3c9860185 100644\n--- a/include/libcamera/internal/software_isp/swisp_stats.h\n+++ b/include/libcamera/internal/software_isp/swisp_stats.h\n@@ -20,6 +20,11 @@ namespace libcamera {\n * wrap around.\n */\n struct SwIspStats {\n+\t/**\n+\t * \\brief True if the statistics buffer contains valid data, false if\n+\t * no statistics were generated for this frame\n+\t */\n+\tbool valid;\n \t/**\n \t * \\brief Holds the sum of all sampled red pixels\n \t */\ndiff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\nindex 3471cc881..e0447cbd6 100644\n--- a/src/ipa/simple/algorithms/agc.cpp\n+++ b/src/ipa/simple/algorithms/agc.cpp\n@@ -91,13 +91,16 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou\n \tagain = std::clamp(again, context.configuration.agc.againMin,\n \t\t\t context.configuration.agc.againMax);\n \n+\tcontext.activeState.agc.exposure = exposure;\n+\tcontext.activeState.agc.again = again;\n+\n \tLOG(IPASoftExposure, Debug)\n \t\t<< \"exposureMSV \" << exposureMSV\n \t\t<< \" exp \" << exposure << \" again \" << again;\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@@ -107,6 +110,25 @@ void Agc::process(IPAContext &context,\n \tmetadata.set(controls::ExposureTime, exposureTime.get<std::micro>());\n \tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n \n+\tif (frame == 0) {\n+\t\t/*\n+\t\t * Init active-state from sensor values in case updateExposure()\n+\t\t * does not run for the first frame.\n+\t\t */\n+\t\tcontext.activeState.agc.exposure = frameContext.sensor.exposure;\n+\t\tcontext.activeState.agc.again = frameContext.sensor.gain;\n+\t}\n+\n+\tif (!stats->valid) {\n+\t\t/*\n+\t\t * Use the new exposure and gain values calculated the last time\n+\t\t * there were valid stats.\n+\t\t */\n+\t\tframeContext.sensor.exposure = context.activeState.agc.exposure;\n+\t\tframeContext.sensor.gain = context.activeState.agc.again;\n+\t\treturn;\n+\t}\n+\n \t/*\n \t * Calculate Mean Sample Value (MSV) according to formula from:\n \t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\ndiff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\nindex cf567e894..ddd0b7914 100644\n--- a/src/ipa/simple/algorithms/awb.cpp\n+++ b/src/ipa/simple/algorithms/awb.cpp\n@@ -61,6 +61,9 @@ void Awb::process(IPAContext &context,\n \t};\n \tmetadata.set(controls::ColourGains, mdGains);\n \n+\tif (!stats->valid)\n+\t\treturn;\n+\n \t/*\n \t * Black level must be subtracted to get the correct AWB ratios, they\n \t * would be off if they were computed from the whole brightness range\ndiff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\nindex 8c1e9ed08..616da0ee7 100644\n--- a/src/ipa/simple/algorithms/blc.cpp\n+++ b/src/ipa/simple/algorithms/blc.cpp\n@@ -60,6 +60,9 @@ void BlackLevel::process(IPAContext &context,\n \t};\n \tmetadata.set(controls::SensorBlackLevels, blackLevels);\n \n+\tif (!stats->valid)\n+\t\treturn;\n+\n \tif (context.configuration.black.level.has_value())\n \t\treturn;\n \ndiff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\nindex 468fccabb..c3081e306 100644\n--- a/src/ipa/simple/ipa_context.h\n+++ b/src/ipa/simple/ipa_context.h\n@@ -37,6 +37,11 @@ struct IPASessionConfiguration {\n };\n \n struct IPAActiveState {\n+\tstruct {\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/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\nindex 4b77b3600..eb416dfdc 100644\n--- a/src/libcamera/software_isp/swstats_cpu.cpp\n+++ b/src/libcamera/software_isp/swstats_cpu.cpp\n@@ -318,6 +318,7 @@ void SwStatsCpu::startFrame(void)\n */\n void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)\n {\n+\tstats_.valid = true;\n \t*sharedStats_ = stats_;\n \tstatsReady.emit(frame, bufferId);\n }\n", "prefixes": [ "v4", "5/6" ] }