Patch Detail
Show a patch.
GET /api/1.1/patches/24539/?format=api
{ "id": 24539, "url": "https://patchwork.libcamera.org/api/1.1/patches/24539/?format=api", "web_url": "https://patchwork.libcamera.org/patch/24539/", "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-7-hansg@kernel.org>", "date": "2025-09-30T15:04:28", "name": "[v4,6/6] libcamera: software_isp: Run sw-statistics once every 4th frame", "commit_ref": "c28bb6a6a48e83be91bdc2a22d8d5ecf602d978e", "pull_url": null, "state": "accepted", "archived": false, "hash": "f0360422a0d480056ac94a43746ba3ee84e911a7", "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/24539/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/24539/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/24539/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 9B2A5C328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 30 Sep 2025 15:04:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C00166B608;\n\tTue, 30 Sep 2025 17:04:49 +0200 (CEST)", "from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6AAEC6B615\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 30 Sep 2025 17:04:44 +0200 (CEST)", "from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby tor.source.kernel.org (Postfix) with ESMTP id 97B2562890;\n\tTue, 30 Sep 2025 15:04:43 +0000 (UTC)", "by smtp.kernel.org (Postfix) with ESMTPSA id 23544C4CEF0;\n\tTue, 30 Sep 2025 15:04:41 +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=\"f2/2ryl2\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1759244683;\n\tbh=RnbEW+JJJj2WQrZC2bqN7fhdjO5jAqhOiCDeEX+v1u4=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=f2/2ryl2CgEXHlXHkdnAqdS3R3eA4aT09Wzyodb8A7qSFrMJDWLP4LXLpJjn/F4Y6\n\txKWZLt9Ef6PlufwpZm1ai0fwPP8MuSGrDg4uFu6C5pROXhY3ZX0eCsySBsgyCi0gqm\n\tIlDOy07gKqpjNtXZFLA5WhzB2kQZBn+TYN007DJcrDZYu7LccKDWrI0btvkLbYC3l4\n\tTxwvxsjV9eV1SKm8WMD1gRgYEe2uf2h4IE29iBK/E1+hdO0nNMUcuMrAIhKg+wPBC3\n\tZerL2nN4Vf5wuLKpszdYkEHzecjtTBMlSFwrceT7OOV8c1bDUYft9SW0lYM4b5XgYh\n\tS8k9TW69btqtw==", "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 6/6] libcamera: software_isp: Run sw-statistics once every\n\t4th frame", "Date": "Tue, 30 Sep 2025 17:04:28 +0200", "Message-ID": "<20250930150428.11101-7-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": "Run sw-statistics once every 4th frame, instead of every frame. There are\n2 reasons for this:\n\n1. There really is no need to have statistics for every frame and only\ndoing this every 4th frame helps save some CPU time.\n\n2. The generic nature of the simple pipeline-handler, so no information\nabout possible CSI receiver frame-delays. In combination with the software\nISP often being used with sensors without sensor info in the sensor-helper\ncode, so no reliable control-delay information means that the software ISP\nis prone to AGC oscillation. Skipping statistics gathering also means\nskipping running the AGC algorithm slowing it down, avoiding this\noscillation.\n\nNote ideally the AGC oscillation problem would be fixed by adding sensor\nmetadata support all through the stack so that the exact gain and exposure\nused for a specific frame are reliably provided by the sensor metadata.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.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- Document why to skip 3 frames / why once every 4 frames\n- Pass frame number to SwStatsCpu::startFrame() SwStatsCpu::processLine?()\n and move all skipping handling to inside the SwStatsCpu class\n---\n src/libcamera/software_isp/debayer_cpu.cpp | 18 +++++++++---------\n src/libcamera/software_isp/debayer_cpu.h | 4 ++--\n src/libcamera/software_isp/swstats_cpu.cpp | 19 +++++++++++++++----\n src/libcamera/software_isp/swstats_cpu.h | 20 +++++++++++++++++---\n 4 files changed, 43 insertions(+), 18 deletions(-)", "diff": "diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\nindex 2dc85e5e0..d5fd0ae73 100644\n--- a/src/libcamera/software_isp/debayer_cpu.cpp\n+++ b/src/libcamera/software_isp/debayer_cpu.cpp\n@@ -655,7 +655,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n }\n \n-void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n+void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n {\n \tunsigned int yEnd = window_.y + window_.height;\n \t/* Holds [0] previous- [1] current- [2] next-line */\n@@ -681,7 +681,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n \tfor (unsigned int y = window_.y; y < yEnd; y += 2) {\n \t\tshiftLinePointers(linePointers, src);\n \t\tmemcpyNextLine(linePointers);\n-\t\tstats_->processLine0(y, linePointers);\n+\t\tstats_->processLine0(frame, y, linePointers);\n \t\t(this->*debayer0_)(dst, linePointers);\n \t\tsrc += inputConfig_.stride;\n \t\tdst += outputConfig_.stride;\n@@ -696,7 +696,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n \tif (window_.y == 0) {\n \t\tshiftLinePointers(linePointers, src);\n \t\tmemcpyNextLine(linePointers);\n-\t\tstats_->processLine0(yEnd, linePointers);\n+\t\tstats_->processLine0(frame, yEnd, linePointers);\n \t\t(this->*debayer0_)(dst, linePointers);\n \t\tsrc += inputConfig_.stride;\n \t\tdst += outputConfig_.stride;\n@@ -710,7 +710,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n \t}\n }\n \n-void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n+void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n {\n \tconst unsigned int yEnd = window_.y + window_.height;\n \t/*\n@@ -733,7 +733,7 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n \tfor (unsigned int y = window_.y; y < yEnd; y += 4) {\n \t\tshiftLinePointers(linePointers, src);\n \t\tmemcpyNextLine(linePointers);\n-\t\tstats_->processLine0(y, linePointers);\n+\t\tstats_->processLine0(frame, y, linePointers);\n \t\t(this->*debayer0_)(dst, linePointers);\n \t\tsrc += inputConfig_.stride;\n \t\tdst += outputConfig_.stride;\n@@ -746,7 +746,7 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n \n \t\tshiftLinePointers(linePointers, src);\n \t\tmemcpyNextLine(linePointers);\n-\t\tstats_->processLine2(y, linePointers);\n+\t\tstats_->processLine2(frame, y, linePointers);\n \t\t(this->*debayer2_)(dst, linePointers);\n \t\tsrc += inputConfig_.stride;\n \t\tdst += outputConfig_.stride;\n@@ -821,12 +821,12 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n \t\treturn;\n \t}\n \n-\tstats_->startFrame();\n+\tstats_->startFrame(frame);\n \n \tif (inputConfig_.patternSize.height == 2)\n-\t\tprocess2(in.planes()[0].data(), out.planes()[0].data());\n+\t\tprocess2(frame, in.planes()[0].data(), out.planes()[0].data());\n \telse\n-\t\tprocess4(in.planes()[0].data(), out.planes()[0].data());\n+\t\tprocess4(frame, in.planes()[0].data(), out.planes()[0].data());\n \n \tmetadata.planes()[0].bytesused = out.planes()[0].size();\n \ndiff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\nindex 9d343e464..03e0d7843 100644\n--- a/src/libcamera/software_isp/debayer_cpu.h\n+++ b/src/libcamera/software_isp/debayer_cpu.h\n@@ -133,8 +133,8 @@ private:\n \tvoid setupInputMemcpy(const uint8_t *linePointers[]);\n \tvoid shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n \tvoid memcpyNextLine(const uint8_t *linePointers[]);\n-\tvoid process2(const uint8_t *src, uint8_t *dst);\n-\tvoid process4(const uint8_t *src, uint8_t *dst);\n+\tvoid process2(uint32_t frame, const uint8_t *src, uint8_t *dst);\n+\tvoid process4(uint32_t frame, const uint8_t *src, uint8_t *dst);\n \n \t/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */\n \tstatic constexpr unsigned int kMaxLineBuffers = 5;\ndiff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\nindex eb416dfdc..634ebfc3c 100644\n--- a/src/libcamera/software_isp/swstats_cpu.cpp\n+++ b/src/libcamera/software_isp/swstats_cpu.cpp\n@@ -62,8 +62,9 @@ namespace libcamera {\n */\n \n /**\n- * \\fn void SwStatsCpu::processLine0(unsigned int y, const uint8_t *src[])\n+ * \\fn void SwStatsCpu::processLine0(uint32_t frame, unsigned int y, const uint8_t *src[])\n * \\brief Process line 0\n+ * \\param[in] frame The frame number\n * \\param[in] y The y coordinate.\n * \\param[in] src The input data.\n *\n@@ -74,8 +75,9 @@ namespace libcamera {\n */\n \n /**\n- * \\fn void SwStatsCpu::processLine2(unsigned int y, const uint8_t *src[])\n+ * \\fn void SwStatsCpu::processLine2(uint32_t frame, unsigned int y, const uint8_t *src[])\n * \\brief Process line 2 and 3\n+ * \\param[in] frame The frame number\n * \\param[in] y The y coordinate.\n * \\param[in] src The input data.\n *\n@@ -89,6 +91,11 @@ namespace libcamera {\n * \\brief Signals that the statistics are ready\n */\n \n+/**\n+ * \\var SwStatsCpu::kStatPerNumFrames\n+ * \\brief Run stats once every kStatPerNumFrames frames\n+ */\n+\n /**\n * \\typedef SwStatsCpu::statsProcessFn\n * \\brief Called when there is data to get statistics from\n@@ -295,11 +302,15 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n \n /**\n * \\brief Reset state to start statistics gathering for a new frame\n+ * \\param[in] frame The frame number\n *\n * This may only be called after a successful setWindow() call.\n */\n-void SwStatsCpu::startFrame(void)\n+void SwStatsCpu::startFrame(uint32_t frame)\n {\n+\tif (frame % kStatPerNumFrames)\n+\t\treturn;\n+\n \tif (window_.width == 0)\n \t\tLOG(SwStatsCpu, Error) << \"Calling startFrame() without setWindow()\";\n \n@@ -318,7 +329,7 @@ void SwStatsCpu::startFrame(void)\n */\n void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)\n {\n-\tstats_.valid = true;\n+\tstats_.valid = frame % kStatPerNumFrames == 0;\n \t*sharedStats_ = stats_;\n \tstatsReady.emit(frame, bufferId);\n }\ndiff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\nindex 26a2f462e..fae575f85 100644\n--- a/src/libcamera/software_isp/swstats_cpu.h\n+++ b/src/libcamera/software_isp/swstats_cpu.h\n@@ -32,6 +32,14 @@ public:\n \tSwStatsCpu();\n \t~SwStatsCpu() = default;\n \n+\t/*\n+\t * The combination of pipeline + sensor delays means that\n+\t * exposure changes can take up to 3 frames to get applied,\n+\t * Run stats once every 4 frames to ensure any previous\n+\t * exposure changes have been applied.\n+\t */\n+\tstatic constexpr uint32_t kStatPerNumFrames = 4;\n+\n \tbool isValid() const { return sharedStats_.fd().isValid(); }\n \n \tconst SharedFD &getStatsFD() { return sharedStats_.fd(); }\n@@ -40,11 +48,14 @@ public:\n \n \tint configure(const StreamConfiguration &inputCfg);\n \tvoid setWindow(const Rectangle &window);\n-\tvoid startFrame();\n+\tvoid startFrame(uint32_t frame);\n \tvoid finishFrame(uint32_t frame, uint32_t bufferId);\n \n-\tvoid processLine0(unsigned int y, const uint8_t *src[])\n+\tvoid processLine0(uint32_t frame, unsigned int y, const uint8_t *src[])\n \t{\n+\t\tif (frame % kStatPerNumFrames)\n+\t\t\treturn;\n+\n \t\tif ((y & ySkipMask_) || y < static_cast<unsigned int>(window_.y) ||\n \t\t y >= (window_.y + window_.height))\n \t\t\treturn;\n@@ -52,8 +63,11 @@ public:\n \t\t(this->*stats0_)(src);\n \t}\n \n-\tvoid processLine2(unsigned int y, const uint8_t *src[])\n+\tvoid processLine2(uint32_t frame, unsigned int y, const uint8_t *src[])\n \t{\n+\t\tif (frame % kStatPerNumFrames)\n+\t\t\treturn;\n+\n \t\tif ((y & ySkipMask_) || y < static_cast<unsigned int>(window_.y) ||\n \t\t y >= (window_.y + window_.height))\n \t\t\treturn;\n", "prefixes": [ "v4", "6/6" ] }