Patch Detail
Show a patch.
GET /api/1.1/patches/22593/?format=api
{ "id": 22593, "url": "https://patchwork.libcamera.org/api/1.1/patches/22593/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22593/", "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": "<20250120204515.24096-7-laurent.pinchart@ideasonboard.com>", "date": "2025-01-20T20:44:57", "name": "[v9,06/12] ipa: rkisp1: Port to the new AEGC controls", "commit_ref": "3d23f325fd8d6dd0961b8ead04551f478585c526", "pull_url": null, "state": "accepted", "archived": false, "hash": "95796112a55e8969416b28294e68909cac7669a3", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22593/mbox/", "series": [ { "id": 4958, "url": "https://patchwork.libcamera.org/api/1.1/series/4958/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4958", "date": "2025-01-20T20:44:51", "name": "AEGC controls", "version": 9, "mbox": "https://patchwork.libcamera.org/series/4958/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22593/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22593/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 53177C3311\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jan 2025 20:45:41 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE6BD68561;\n\tMon, 20 Jan 2025 21:45:40 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C85EE6854E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jan 2025 21:45:30 +0100 (CET)", "from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 313C3CDB;\n\tMon, 20 Jan 2025 21:44:29 +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=\"IXw/qMsu\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737405869;\n\tbh=Afjm/PtZYlgNjY2jtQ7thibtC6zfVQsNwkZyYzYkJug=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=IXw/qMsun7sMvnpphnxtJSrN3RKPKoKEv+TH9cbYgLgwG123RuId9C0oxeMo6qgju\n\t1Fjj8RLxYIhIb8Cl0EOjmYN/jmQ09oeyultg9ked2LGZxLsS+I4tSsfdbHKcO3a1is\n\tg18B+t/zRrg7l5WM94XRe8mlA5tKvA6Za209ybaY=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Naushir Patuck <naush@raspberrypi.com>,\n\tDavid Plowman <david.plowman@raspberrypi.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>", "Subject": "[PATCH v9 06/12] ipa: rkisp1: Port to the new AEGC controls", "Date": "Mon, 20 Jan 2025 22:44:57 +0200", "Message-ID": "<20250120204515.24096-7-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.45.2", "In-Reply-To": "<20250120204515.24096-1-laurent.pinchart@ideasonboard.com>", "References": "<20250120204515.24096-1-laurent.pinchart@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\nThe newly introduced controls to drive the AEGC algorithm allow\ncontrolling the computation of the exposure time and analogue gain\nseparately.\n\nAugument the RkISP1 AEGC implementation to handle the exposure and gain\ncontrols separately using the new controls.\n\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/rkisp1/algorithms/agc.cpp | 131 ++++++++++++++++++++++++------\n src/ipa/rkisp1/ipa_context.cpp | 24 +++++-\n src/ipa/rkisp1/ipa_context.h | 8 +-\n 3 files changed, 133 insertions(+), 30 deletions(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex 40e5a8f481b2..78122a1f05f8 100644\n--- a/src/ipa/rkisp1/algorithms/agc.cpp\n+++ b/src/ipa/rkisp1/algorithms/agc.cpp\n@@ -148,7 +148,14 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)\n \tif (ret)\n \t\treturn ret;\n \n-\tcontext.ctrlMap[&controls::AeEnable] = ControlInfo(false, true);\n+\tcontext.ctrlMap[&controls::ExposureTimeMode] =\n+\t\tControlInfo(static_cast<int32_t>(controls::ExposureTimeModeAuto),\n+\t\t\t static_cast<int32_t>(controls::ExposureTimeModeManual),\n+\t\t\t static_cast<int32_t>(controls::ExposureTimeModeAuto));\n+\tcontext.ctrlMap[&controls::AnalogueGainMode] =\n+\t\tControlInfo(static_cast<int32_t>(controls::AnalogueGainModeAuto),\n+\t\t\t static_cast<int32_t>(controls::AnalogueGainModeManual),\n+\t\t\t static_cast<int32_t>(controls::AnalogueGainModeAuto));\n \tcontext.ctrlMap.merge(controls());\n \n \treturn 0;\n@@ -169,7 +176,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n \t\t10ms / context.configuration.sensor.lineDuration;\n \tcontext.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;\n \tcontext.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n-\tcontext.activeState.agc.autoEnabled = !context.configuration.raw;\n+\tcontext.activeState.agc.autoExposureEnabled = !context.configuration.raw;\n+\tcontext.activeState.agc.autoGainEnabled = !context.configuration.raw;\n \n \tcontext.activeState.agc.constraintMode =\n \t\tstatic_cast<controls::AeConstraintModeEnum>(constraintModes().begin()->first);\n@@ -215,18 +223,47 @@ void Agc::queueRequest(IPAContext &context,\n \tauto &agc = context.activeState.agc;\n \n \tif (!context.configuration.raw) {\n-\t\tconst auto &agcEnable = controls.get(controls::AeEnable);\n-\t\tif (agcEnable && *agcEnable != agc.autoEnabled) {\n-\t\t\tagc.autoEnabled = *agcEnable;\n+\t\tconst auto &aeEnable = controls.get(controls::ExposureTimeMode);\n+\t\tif (aeEnable &&\n+\t\t (*aeEnable == controls::ExposureTimeModeAuto) != agc.autoExposureEnabled) {\n+\t\t\tagc.autoExposureEnabled = (*aeEnable == controls::ExposureTimeModeAuto);\n \n \t\t\tLOG(RkISP1Agc, Debug)\n-\t\t\t\t<< (agc.autoEnabled ? \"Enabling\" : \"Disabling\")\n-\t\t\t\t<< \" AGC\";\n+\t\t\t\t<< (agc.autoExposureEnabled ? \"Enabling\" : \"Disabling\")\n+\t\t\t\t<< \" AGC (exposure)\";\n+\n+\t\t\t/*\n+\t\t\t * If we go from auto -> manual with no manual control\n+\t\t\t * set, use the last computed value, which we don't\n+\t\t\t * know until prepare() so save this information.\n+\t\t\t *\n+\t\t\t * \\todo Check the previous frame at prepare() time\n+\t\t\t * instead of saving a flag here\n+\t\t\t */\n+\t\t\tif (!agc.autoExposureEnabled && !controls.get(controls::ExposureTime))\n+\t\t\t\tframeContext.agc.autoExposureModeChange = true;\n+\t\t}\n+\n+\t\tconst auto &agEnable = controls.get(controls::AnalogueGainMode);\n+\t\tif (agEnable &&\n+\t\t (*agEnable == controls::AnalogueGainModeAuto) != agc.autoGainEnabled) {\n+\t\t\tagc.autoGainEnabled = (*agEnable == controls::AnalogueGainModeAuto);\n+\n+\t\t\tLOG(RkISP1Agc, Debug)\n+\t\t\t\t<< (agc.autoGainEnabled ? \"Enabling\" : \"Disabling\")\n+\t\t\t\t<< \" AGC (gain)\";\n+\t\t\t/*\n+\t\t\t * If we go from auto -> manual with no manual control\n+\t\t\t * set, use the last computed value, which we don't\n+\t\t\t * know until prepare() so save this information.\n+\t\t\t */\n+\t\t\tif (!agc.autoGainEnabled && !controls.get(controls::AnalogueGain))\n+\t\t\t\tframeContext.agc.autoGainModeChange = true;\n \t\t}\n \t}\n \n \tconst auto &exposure = controls.get(controls::ExposureTime);\n-\tif (exposure && !agc.autoEnabled) {\n+\tif (exposure && !agc.autoExposureEnabled) {\n \t\tagc.manual.exposure = *exposure * 1.0us\n \t\t\t\t / context.configuration.sensor.lineDuration;\n \n@@ -235,18 +272,19 @@ void Agc::queueRequest(IPAContext &context,\n \t}\n \n \tconst auto &gain = controls.get(controls::AnalogueGain);\n-\tif (gain && !agc.autoEnabled) {\n+\tif (gain && !agc.autoGainEnabled) {\n \t\tagc.manual.gain = *gain;\n \n \t\tLOG(RkISP1Agc, Debug) << \"Set gain to \" << agc.manual.gain;\n \t}\n \n-\tframeContext.agc.autoEnabled = agc.autoEnabled;\n+\tframeContext.agc.autoExposureEnabled = agc.autoExposureEnabled;\n+\tframeContext.agc.autoGainEnabled = agc.autoGainEnabled;\n \n-\tif (!frameContext.agc.autoEnabled) {\n+\tif (!frameContext.agc.autoExposureEnabled)\n \t\tframeContext.agc.exposure = agc.manual.exposure;\n+\tif (!frameContext.agc.autoGainEnabled)\n \t\tframeContext.agc.gain = agc.manual.gain;\n-\t}\n \n \tconst auto &meteringMode = controls.get(controls::AeMeteringMode);\n \tif (meteringMode) {\n@@ -283,9 +321,26 @@ void Agc::queueRequest(IPAContext &context,\n void Agc::prepare(IPAContext &context, const uint32_t frame,\n \t\t IPAFrameContext &frameContext, RkISP1Params *params)\n {\n-\tif (frameContext.agc.autoEnabled) {\n-\t\tframeContext.agc.exposure = context.activeState.agc.automatic.exposure;\n-\t\tframeContext.agc.gain = context.activeState.agc.automatic.gain;\n+\tuint32_t activeAutoExposure = context.activeState.agc.automatic.exposure;\n+\tdouble activeAutoGain = context.activeState.agc.automatic.gain;\n+\n+\t/* Populate exposure and gain in auto mode */\n+\tif (frameContext.agc.autoExposureEnabled)\n+\t\tframeContext.agc.exposure = activeAutoExposure;\n+\tif (frameContext.agc.autoGainEnabled)\n+\t\tframeContext.agc.gain = activeAutoGain;\n+\n+\t/*\n+\t * Populate manual exposure and gain from the active auto values when\n+\t * transitioning from auto to manual\n+\t */\n+\tif (!frameContext.agc.autoExposureEnabled && frameContext.agc.autoExposureModeChange) {\n+\t\tcontext.activeState.agc.manual.exposure = activeAutoExposure;\n+\t\tframeContext.agc.exposure = activeAutoExposure;\n+\t}\n+\tif (!frameContext.agc.autoGainEnabled && frameContext.agc.autoGainModeChange) {\n+\t\tcontext.activeState.agc.manual.gain = activeAutoGain;\n+\t\tframeContext.agc.gain = activeAutoGain;\n \t}\n \n \tif (frame > 0 && !frameContext.agc.updateMetering)\n@@ -333,7 +388,14 @@ 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::AeEnable, frameContext.agc.autoEnabled);\n+\tmetadata.set(controls::ExposureTimeMode,\n+\t\t frameContext.agc.autoExposureEnabled\n+\t\t ? controls::ExposureTimeModeAuto\n+\t\t : controls::ExposureTimeModeManual);\n+\tmetadata.set(controls::AnalogueGainMode,\n+\t\t frameContext.agc.autoGainEnabled\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@@ -424,14 +486,35 @@ 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-\tutils::Duration maxExposureTime =\n-\t\tstd::clamp(frameContext.agc.maxFrameDuration,\n-\t\t\t context.configuration.sensor.minExposureTime,\n-\t\t\t context.configuration.sensor.maxExposureTime);\n-\tsetLimits(context.configuration.sensor.minExposureTime,\n-\t\t maxExposureTime,\n-\t\t context.configuration.sensor.minAnalogueGain,\n-\t\t context.configuration.sensor.maxAnalogueGain);\n+\t/*\n+\t * Set the AGC limits using the fixed exposure time and/or gain in\n+\t * manual mode, or the sensor limits in auto mode.\n+\t */\n+\tutils::Duration minExposureTime;\n+\tutils::Duration maxExposureTime;\n+\tdouble minAnalogueGain;\n+\tdouble maxAnalogueGain;\n+\n+\tif (frameContext.agc.autoExposureEnabled) {\n+\t\tminExposureTime = context.configuration.sensor.minExposureTime;\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+\t} else {\n+\t\tminExposureTime = context.configuration.sensor.lineDuration\n+\t\t\t\t* frameContext.agc.exposure;\n+\t\tmaxExposureTime = minExposureTime;\n+\t}\n+\n+\tif (frameContext.agc.autoGainEnabled) {\n+\t\tminAnalogueGain = context.configuration.sensor.minAnalogueGain;\n+\t\tmaxAnalogueGain = context.configuration.sensor.maxAnalogueGain;\n+\t} else {\n+\t\tminAnalogueGain = frameContext.agc.gain;\n+\t\tmaxAnalogueGain = frameContext.agc.gain;\n+\t}\n+\n+\tsetLimits(minExposureTime, maxExposureTime, minAnalogueGain, maxAnalogueGain);\n \n \t/*\n \t * The Agc algorithm needs to know the effective exposure value that was\ndiff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\nindex 80b99df8eaf8..261c0472a4fc 100644\n--- a/src/ipa/rkisp1/ipa_context.cpp\n+++ b/src/ipa/rkisp1/ipa_context.cpp\n@@ -165,8 +165,11 @@ namespace libcamera::ipa::rkisp1 {\n * \\var IPAActiveState::agc.automatic.gain\n * \\brief Automatic analogue gain multiplier\n *\n- * \\var IPAActiveState::agc.autoEnabled\n- * \\brief Manual/automatic AGC state as set by the AeEnable control\n+ * \\var IPAActiveState::agc.autoExposureEnabled\n+ * \\brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control\n+ *\n+ * \\var IPAActiveState::agc.autoGainEnabled\n+ * \\brief Manual/automatic AGC state (gain) as set by the AnalogueGainMode control\n *\n * \\var IPAActiveState::agc.constraintMode\n * \\brief Constraint mode as set by the AeConstraintMode control\n@@ -289,8 +292,11 @@ namespace libcamera::ipa::rkisp1 {\n *\n * The gain should be adapted to the sensor specific gain code before applying.\n *\n- * \\var IPAFrameContext::agc.autoEnabled\n- * \\brief Manual/automatic AGC state as set by the AeEnable control\n+ * \\var IPAFrameContext::agc.autoExposureEnabled\n+ * \\brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control\n+ *\n+ * \\var IPAFrameContext::agc.autoGainEnabled\n+ * \\brief Manual/automatic AGC state (gain) as set by the AnalogueGainMode control\n *\n * \\var IPAFrameContext::agc.constraintMode\n * \\brief Constraint mode as set by the AeConstraintMode control\n@@ -306,6 +312,16 @@ namespace libcamera::ipa::rkisp1 {\n *\n * \\var IPAFrameContext::agc.updateMetering\n * \\brief Indicate if new ISP AGC metering parameters need to be applied\n+ *\n+ * \\var IPAFrameContext::agc.autoExposureModeChange\n+ * \\brief Indicate if autoExposureEnabled has changed from true in the previous\n+ * frame to false in the current frame, and no manual exposure value has been\n+ * supplied in the current frame.\n+ *\n+ * \\var IPAFrameContext::agc.autoGainModeChange\n+ * \\brief Indicate if autoGainEnabled has changed from true in the previous\n+ * frame to false in the current frame, and no manual gain value has been\n+ * supplied in the current frame.\n */\n \n /**\ndiff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\nindex b83c182296c5..5d5b79fa615e 100644\n--- a/src/ipa/rkisp1/ipa_context.h\n+++ b/src/ipa/rkisp1/ipa_context.h\n@@ -79,7 +79,8 @@ struct IPAActiveState {\n \t\t\tdouble gain;\n \t\t} automatic;\n \n-\t\tbool autoEnabled;\n+\t\tbool autoExposureEnabled;\n+\t\tbool autoGainEnabled;\n \t\tcontrols::AeConstraintModeEnum constraintMode;\n \t\tcontrols::AeExposureModeEnum exposureMode;\n \t\tcontrols::AeMeteringModeEnum meteringMode;\n@@ -124,12 +125,15 @@ struct IPAFrameContext : public FrameContext {\n \tstruct {\n \t\tuint32_t exposure;\n \t\tdouble gain;\n-\t\tbool autoEnabled;\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 maxFrameDuration;\n \t\tbool updateMetering;\n+\t\tbool autoExposureModeChange;\n+\t\tbool autoGainModeChange;\n \t} agc;\n \n \tstruct {\n", "prefixes": [ "v9", "06/12" ] }