Patch Detail
Show a patch.
GET /api/1.1/patches/22817/?format=api
{ "id": 22817, "url": "https://patchwork.libcamera.org/api/1.1/patches/22817/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22817/", "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": "<20250221092045.3896021-4-paul.elder@ideasonboard.com>", "date": "2025-02-21T09:20:45", "name": "[v2,3/3] rkisp1: Honor the FrameDurationLimits control", "commit_ref": "f72c76eb6e06a41d2aaff8c8c4002dea21a7774d", "pull_url": null, "state": "accepted", "archived": false, "hash": "561fdd2913a3b83d53695e71a60c30d3a3be5778", "submitter": { "id": 17, "url": "https://patchwork.libcamera.org/api/1.1/people/17/?format=api", "name": "Paul Elder", "email": "paul.elder@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22817/mbox/", "series": [ { "id": 5009, "url": "https://patchwork.libcamera.org/api/1.1/series/5009/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5009", "date": "2025-02-21T09:20:42", "name": "ipa: rkisp1: Honor FrameDurationLimits", "version": 2, "mbox": "https://patchwork.libcamera.org/series/5009/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22817/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22817/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 78F54BDCC1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 21 Feb 2025 09:21:16 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBFC8686B0;\n\tFri, 21 Feb 2025 10:21:15 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A0D0686AB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Feb 2025 10:21:13 +0100 (CET)", "from neptunite.hamster-moth.ts.net (unknown\n\t[IPv6:2404:7a81:160:2100:d97a:61eb:567:25d8])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 37C7511D5;\n\tFri, 21 Feb 2025 10:19:46 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ks89GYBV\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740129589;\n\tbh=5qVkU2LimHGUnPvzfXHXcxk5NVZmJEyfK4WH0uLIJHc=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=Ks89GYBVwcQVOlwE3fzcJ/DJHHwnDxM7xXpf1mfiWBnOqEFAHM5F+ylloIe56G5qm\n\t3kfkwgJpjlj8iu0rY66eCAt3XNX0n3uQJM0q1E/R3Wax1DWc32LYF8YvmC6C4vPSAn\n\ts7GS9JoKdG7wG5bs59soocYIxVErb1NCNeqsj8Is=", "From": "Paul Elder <paul.elder@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Paul Elder <paul.elder@ideasonboard.com>, isaac.scott@ideasonboard.com, \n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tUmang Jain <umang.jain@ideasonboard.com>", "Subject": "[PATCH v2 3/3] rkisp1: Honor the FrameDurationLimits control", "Date": "Fri, 21 Feb 2025 18:20:45 +0900", "Message-Id": "<20250221092045.3896021-4-paul.elder@ideasonboard.com>", "X-Mailer": "git-send-email 2.39.2", "In-Reply-To": "<20250221092045.3896021-1-paul.elder@ideasonboard.com>", "References": "<20250221092045.3896021-1-paul.elder@ideasonboard.com>", "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": "Add support to rkisp1 for controlling the framerate via the\nFrameDurationLimits control.\n\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\n---\nChanges in v2:\n- recover from bitrot\n- fix setting frame duration limits in raw mode\n---\n src/ipa/rkisp1/algorithms/agc.cpp | 60 +++++++++++++++++++-----\n src/ipa/rkisp1/algorithms/agc.h | 3 ++\n src/ipa/rkisp1/ipa_context.cpp | 16 ++++++-\n src/ipa/rkisp1/ipa_context.h | 4 ++\n src/ipa/rkisp1/rkisp1.cpp | 2 +\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +\n 6 files changed, 74 insertions(+), 12 deletions(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex 5fece545677e..4e0e3734117e 100644\n--- a/src/ipa/rkisp1/algorithms/agc.cpp\n+++ b/src/ipa/rkisp1/algorithms/agc.cpp\n@@ -190,6 +190,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n \n \t/* Limit the frame duration to match current initialisation */\n \tControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];\n+\tcontext.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get<int64_t>());\n \tcontext.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());\n \n \t/*\n@@ -307,10 +308,21 @@ void Agc::queueRequest(IPAContext &context,\n \n \tconst auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);\n \tif (frameDurationLimits) {\n-\t\tutils::Duration maxFrameDuration =\n-\t\t\tstd::chrono::milliseconds((*frameDurationLimits).back());\n-\t\tagc.maxFrameDuration = maxFrameDuration;\n+\t\t/* Limit the control value to the limits in ControlInfo */\n+\t\tControlInfo &limits = context.ctrlMap[&controls::FrameDurationLimits];\n+\t\tint64_t minFrameDuration =\n+\t\t\tstd::clamp((*frameDurationLimits).front(),\n+\t\t\t\t limits.min().get<int64_t>(),\n+\t\t\t\t limits.max().get<int64_t>());\n+\t\tint64_t maxFrameDuration =\n+\t\t\tstd::clamp((*frameDurationLimits).back(),\n+\t\t\t\t limits.min().get<int64_t>(),\n+\t\t\t\t limits.max().get<int64_t>());\n+\n+\t\tagc.minFrameDuration = std::chrono::microseconds(minFrameDuration);\n+\t\tagc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);\n \t}\n+\tframeContext.agc.minFrameDuration = agc.minFrameDuration;\n \tframeContext.agc.maxFrameDuration = agc.maxFrameDuration;\n }\n \n@@ -387,6 +399,7 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n \t\t\t\t * frameContext.sensor.exposure;\n \tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n \tmetadata.set(controls::ExposureTime, exposureTime.get<std::micro>());\n+\tmetadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get<std::micro>());\n \tmetadata.set(controls::ExposureTimeMode,\n \t\t frameContext.agc.autoExposureEnabled\n \t\t ? controls::ExposureTimeModeAuto\n@@ -396,13 +409,6 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n \t\t ? controls::AnalogueGainModeAuto\n \t\t : controls::AnalogueGainModeManual);\n \n-\t/* \\todo Use VBlank value calculated from each frame exposure. */\n-\tuint32_t vTotal = context.configuration.sensor.size.height\n-\t\t\t+ context.configuration.sensor.defVBlank;\n-\tutils::Duration frameDuration = context.configuration.sensor.lineDuration\n-\t\t\t\t * vTotal;\n-\tmetadata.set(controls::FrameDuration, frameDuration.get<std::micro>());\n-\n \tmetadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);\n \tmetadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);\n \tmetadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);\n@@ -444,6 +450,27 @@ double Agc::estimateLuminance(double gain) const\n \treturn ySum / expMeans_.size() / 255;\n }\n \n+/**\n+ * \\brief Process frame duration and compute vblank\n+ * \\param[in] context The shared IPA context\n+ * \\param[in] frameContext The current frame context\n+ * \\param[in] frameDuration The target frame duration\n+ *\n+ * Compute and populate vblank from the target frame duration.\n+ */\n+void Agc::processFrameDuration(IPAContext &context,\n+\t\t\t IPAFrameContext &frameContext,\n+\t\t\t utils::Duration frameDuration)\n+{\n+\tIPACameraSensorInfo &sensorInfo = context.sensorInfo;\n+\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n+\n+\tframeContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;\n+\n+\t/* Update frame duration accounting for line length quantization. */\n+\tframeContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;\n+}\n+\n /**\n * \\brief Process RkISP1 statistics, and run AGC operations\n * \\param[in] context The shared IPA context\n@@ -460,6 +487,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \t\t ControlList &metadata)\n {\n \tif (!stats) {\n+\t\tprocessFrameDuration(context, frameContext,\n+\t\t\t\t frameContext.agc.minFrameDuration);\n \t\tfillMetadata(context, frameContext, metadata);\n \t\treturn;\n \t}\n@@ -497,7 +526,9 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \tdouble maxAnalogueGain;\n \n \tif (frameContext.agc.autoExposureEnabled) {\n-\t\tminExposureTime = context.configuration.sensor.minExposureTime;\n+\t\tminExposureTime = std::clamp(frameContext.agc.minFrameDuration,\n+\t\t\t\t\t context.configuration.sensor.minExposureTime,\n+\t\t\t\t\t context.configuration.sensor.maxExposureTime);\n \t\tmaxExposureTime = std::clamp(frameContext.agc.maxFrameDuration,\n \t\t\t\t\t context.configuration.sensor.minExposureTime,\n \t\t\t\t\t context.configuration.sensor.maxExposureTime);\n@@ -541,6 +572,13 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \tactiveState.agc.automatic.exposure = newExposureTime / lineDuration;\n \tactiveState.agc.automatic.gain = aGain;\n \n+\t/*\n+\t * Expand the target frame duration so that we do not run faster than\n+\t * the minimum frame duration when we have short exposures.\n+\t */\n+\tprocessFrameDuration(context, frameContext,\n+\t\t\t std::max(frameContext.agc.minFrameDuration, newExposureTime));\n+\n \tfillMetadata(context, frameContext, metadata);\n \texpMeans_ = {};\n }\ndiff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\nindex aa86f2c5bc21..62bcde999fe3 100644\n--- a/src/ipa/rkisp1/algorithms/agc.h\n+++ b/src/ipa/rkisp1/algorithms/agc.h\n@@ -50,6 +50,9 @@ private:\n \tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n \t\t\t ControlList &metadata);\n \tdouble estimateLuminance(double gain) const override;\n+\tvoid processFrameDuration(IPAContext &context,\n+\t\t\t\t IPAFrameContext &frameContext,\n+\t\t\t\t utils::Duration frameDuration);\n \n \tSpan<const uint8_t> expMeans_;\n \ndiff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\nindex 261c0472a4fc..99611bd5b390 100644\n--- a/src/ipa/rkisp1/ipa_context.cpp\n+++ b/src/ipa/rkisp1/ipa_context.cpp\n@@ -180,6 +180,9 @@ namespace libcamera::ipa::rkisp1 {\n * \\var IPAActiveState::agc.meteringMode\n * \\brief Metering mode as set by the AeMeteringMode control\n *\n+ * \\var IPAActiveState::agc.minFrameDuration\n+ * \\brief Minimum frame duration as set by the FrameDurationLimits control\n+ *\n * \\var IPAActiveState::agc.maxFrameDuration\n * \\brief Maximum frame duration as set by the FrameDurationLimits control\n */\n@@ -282,7 +285,9 @@ namespace libcamera::ipa::rkisp1 {\n * \\brief Automatic Gain Control parameters for this frame\n *\n * The exposure and gain are provided by the AGC algorithm, and are to be\n- * applied to the sensor in order to take effect for this frame.\n+ * applied to the sensor in order to take effect for this frame. Additionally\n+ * the vertical blanking period is determined to maintain a consistent frame\n+ * rate matched to the FrameDurationLimits as set by the user.\n *\n * \\var IPAFrameContext::agc.exposure\n * \\brief Exposure time expressed as a number of lines computed by the algorithm\n@@ -292,6 +297,9 @@ namespace libcamera::ipa::rkisp1 {\n *\n * The gain should be adapted to the sensor specific gain code before applying.\n *\n+ * \\var IPAFrameContext::agc.vblank\n+ * \\brief Vertical blanking parameter computed by the algorithm\n+ *\n * \\var IPAFrameContext::agc.autoExposureEnabled\n * \\brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control\n *\n@@ -307,9 +315,15 @@ namespace libcamera::ipa::rkisp1 {\n * \\var IPAFrameContext::agc.meteringMode\n * \\brief Metering mode as set by the AeMeteringMode control\n *\n+ * \\var IPAFrameContext::agc.minFrameDuration\n+ * \\brief Minimum frame duration as set by the FrameDurationLimits control\n+ *\n * \\var IPAFrameContext::agc.maxFrameDuration\n * \\brief Maximum frame duration as set by the FrameDurationLimits control\n *\n+ * \\var IPAFrameContext::agc.frameDuration\n+ * \\brief The actual FrameDuration used by the algorithm for the frame\n+ *\n * \\var IPAFrameContext::agc.updateMetering\n * \\brief Indicate if new ISP AGC metering parameters need to be applied\n *\ndiff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\nindex c765b928a55f..474f7036f003 100644\n--- a/src/ipa/rkisp1/ipa_context.h\n+++ b/src/ipa/rkisp1/ipa_context.h\n@@ -84,6 +84,7 @@ struct IPAActiveState {\n \t\tcontrols::AeConstraintModeEnum constraintMode;\n \t\tcontrols::AeExposureModeEnum exposureMode;\n \t\tcontrols::AeMeteringModeEnum meteringMode;\n+\t\tutils::Duration minFrameDuration;\n \t\tutils::Duration maxFrameDuration;\n \t} agc;\n \n@@ -125,12 +126,15 @@ struct IPAFrameContext : public FrameContext {\n \tstruct {\n \t\tuint32_t exposure;\n \t\tdouble gain;\n+\t\tuint32_t vblank;\n \t\tbool autoExposureEnabled;\n \t\tbool autoGainEnabled;\n \t\tcontrols::AeConstraintModeEnum constraintMode;\n \t\tcontrols::AeExposureModeEnum exposureMode;\n \t\tcontrols::AeMeteringModeEnum meteringMode;\n+\t\tutils::Duration minFrameDuration;\n \t\tutils::Duration maxFrameDuration;\n+\t\tutils::Duration frameDuration;\n \t\tbool updateMetering;\n \t\tbool autoExposureModeChange;\n \t\tbool autoGainModeChange;\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 0e761249d27c..7547d2f274f4 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -453,10 +453,12 @@ void IPARkISP1::setControls(unsigned int frame)\n \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n \tuint32_t exposure = frameContext.agc.exposure;\n \tuint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);\n+\tuint32_t vblank = frameContext.agc.vblank;\n \n \tControlList ctrls(sensorControls_);\n \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));\n \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));\n+\tctrls.set(V4L2_CID_VBLANK, static_cast<int32_t>(vblank));\n \n \tsetSensorControls.emit(frame, ctrls);\n }\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 1ac8d8ae7ed9..52633fe3cb85 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -1325,6 +1325,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n \tstd::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\n \t\t{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },\n \t\t{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },\n+\t\t{ V4L2_CID_VBLANK, { 1, false } },\n \t};\n \n \tdata->delayedCtrls_ =\n", "prefixes": [ "v2", "3/3" ] }