Patch Detail
Show a patch.
GET /api/patches/21623/?format=api
{ "id": 21623, "url": "https://patchwork.libcamera.org/api/patches/21623/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21623/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20241014154747.2295253-4-kieran.bingham@ideasonboard.com>", "date": "2024-10-14T15:47:47", "name": "[3/3] rkisp1: Honor the FrameDurationLimits control", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "a31f4ea0f2dad7bf2b3fb76529c0c7a2d3c5f758", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21623/mbox/", "series": [ { "id": 4697, "url": "https://patchwork.libcamera.org/api/series/4697/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4697", "date": "2024-10-14T15:47:44", "name": "ipa: rkisp1: Honor FrameDurationLimits", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4697/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21623/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21623/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 81FE3C32F4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Oct 2024 15:47:59 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A663D6537C;\n\tMon, 14 Oct 2024 17:47:56 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE4646537E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Oct 2024 17:47:51 +0200 (CEST)", "from Monstersaurus.lgs-net.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EB3FA1BAE;\n\tMon, 14 Oct 2024 17:46:10 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OgGPz9xY\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728920771;\n\tbh=drBSXRzmjMPrQnNbI+yZGFBPw6McSxyxZ7MizNbASiE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=OgGPz9xYbMRDM2Uv71/nnVGgzwbRTN9JexRt26qSJsYgidCRa2R6P2SvZiwWJxH3j\n\t4y/HkNwnnoZ+DLykTosZ/3yq++ZSM/03bVXqFB2O5VOqknUQpW1vWFA61hc9HLCDTM\n\tDcesLL/iUot2FGfYiw06gsq4eVjT/av7Rp+t9vCI=", "From": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Cc": "Paul Elder <paul.elder@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>", "Subject": "[PATCH 3/3] rkisp1: Honor the FrameDurationLimits control", "Date": "Mon, 14 Oct 2024 16:47:47 +0100", "Message-Id": "<20241014154747.2295253-4-kieran.bingham@ideasonboard.com>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20241014154747.2295253-1-kieran.bingham@ideasonboard.com>", "References": "<20241014154747.2295253-1-kieran.bingham@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": "From: Paul Elder <paul.elder@ideasonboard.com>\n\nAdd 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>\n---\n src/ipa/rkisp1/algorithms/agc.cpp | 52 ++++++++++++++++++------\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 | 7 ++++\n 5 files changed, 68 insertions(+), 13 deletions(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex e23ab120b3e2..088ecf42c480 100644\n--- a/src/ipa/rkisp1/algorithms/agc.cpp\n+++ b/src/ipa/rkisp1/algorithms/agc.cpp\n@@ -180,6 +180,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@@ -267,10 +268,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@@ -330,15 +342,8 @@ 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::AeEnable, frameContext.agc.autoEnabled);\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@@ -400,6 +405,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \t\treturn;\n \t}\n \n+\tIPACameraSensorInfo &sensorInfo = context.sensorInfo;\n \tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n \n \t/*\n@@ -418,11 +424,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \t\t [](uint32_t x) { return x >> 4; });\n \texpMeans_ = { params->ae.exp_mean, context.hw->numAeCells };\n \n+\t/*\n+\t * Limit the allowed shutter speeds for the exposure helper based on\n+\t * the frame duration limits.\n+\t */\n+\tutils::Duration minShutterSpeed =\n+\t\tstd::clamp(frameContext.agc.minFrameDuration,\n+\t\t\t context.configuration.sensor.minShutterSpeed,\n+\t\t\t context.configuration.sensor.maxShutterSpeed);\n \tutils::Duration maxShutterSpeed =\n \t\tstd::clamp(frameContext.agc.maxFrameDuration,\n \t\t\t context.configuration.sensor.minShutterSpeed,\n \t\t\t context.configuration.sensor.maxShutterSpeed);\n-\tsetLimits(context.configuration.sensor.minShutterSpeed,\n+\tsetLimits(minShutterSpeed,\n \t\t maxShutterSpeed,\n \t\t context.configuration.sensor.minAnalogueGain,\n \t\t context.configuration.sensor.maxAnalogueGain);\n@@ -451,6 +465,20 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \tactiveState.agc.automatic.exposure = shutterTime / lineDuration;\n \tactiveState.agc.automatic.gain = aGain;\n \n+\t/*\n+\t * Determine what our FrameDuration must be and adapt VBLANK to suit\n+\t * this if we need to expand the shutter time calculated to fill the\n+\t * remaining time so that we do not run faster than the minimum frame\n+\t * duration (maximum requested frame rate) when we have short exposures.\n+\t */\n+\tutils::Duration frameDuration = std::max(frameContext.agc.minFrameDuration,\n+\t\t\t\t\t\t shutterTime);\n+\n+\tframeContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;\n+\n+\t/* Update accounting for line length quantization. */\n+\tframeContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;\n+\n \tfillMetadata(context, frameContext, metadata);\n \texpMeans_ = {};\n }\ndiff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\nindex 14d0c02a2b32..c5eb0d64ec29 100644\n--- a/src/ipa/rkisp1/ipa_context.cpp\n+++ b/src/ipa/rkisp1/ipa_context.cpp\n@@ -177,6 +177,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@@ -297,7 +300,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@@ -307,6 +312,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.autoEnabled\n * \\brief Manual/automatic AGC state as set by the AeEnable control\n *\n@@ -319,9 +327,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 e274d9b01e1c..60c4d647f1ef 100644\n--- a/src/ipa/rkisp1/ipa_context.h\n+++ b/src/ipa/rkisp1/ipa_context.h\n@@ -79,6 +79,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@@ -128,11 +129,14 @@ struct IPAFrameContext : public FrameContext {\n \tstruct {\n \t\tuint32_t exposure;\n \t\tdouble gain;\n+\t\tuint32_t vblank;\n \t\tbool autoEnabled;\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} agc;\n \ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 47777ece783f..17d56fb4e901 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -449,10 +449,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 42961c120036..1ec12185bb78 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -394,6 +394,12 @@ void RkISP1CameraData::paramFilled(unsigned int frame, unsigned int bytesused)\n void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n \t\t\t\t\t const ControlList &sensorControls)\n {\n+\tif (frame == 0) {\n+\t\tControlList controls = sensorControls;\n+\t\tsensor_->setControls(&controls);\n+\t\treturn;\n+\t}\n+\n \tdelayedCtrls_->push(sensorControls);\n }\n \n@@ -1138,6 +1144,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n \tstd::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\n \t\t{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },\n \t\t{ V4L2_CID_EXPOSURE, { 2, false } },\n+\t\t{ V4L2_CID_VBLANK, { 1, false } },\n \t};\n \n \tdata->delayedCtrls_ =\n", "prefixes": [ "3/3" ] }