{"id":22817,"url":"https://patchwork.libcamera.org/api/patches/22817/?format=json","web_url":"https://patchwork.libcamera.org/patch/22817/","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":"<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/people/17/?format=json","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/series/5009/?format=json","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":"","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 ;\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 ;\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 ","To":"libcamera-devel@lists.libcamera.org","Cc":"Paul Elder , isaac.scott@ideasonboard.com, \n\tKieran Bingham ,\n\tUmang Jain ","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":"","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":"Add support to rkisp1 for controlling the framerate via the\nFrameDurationLimits control.\n\nSigned-off-by: Paul Elder \nSigned-off-by: Kieran Bingham \nReviewed-by: Umang Jain \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());\n \tcontext.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get());\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(),\n+\t\t\t\t limits.max().get());\n+\t\tint64_t maxFrameDuration =\n+\t\t\tstd::clamp((*frameDurationLimits).back(),\n+\t\t\t\t limits.min().get(),\n+\t\t\t\t limits.max().get());\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());\n+\tmetadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get());\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());\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 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(exposure));\n \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain));\n+\tctrls.set(V4L2_CID_VBLANK, static_cast(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 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"]}