Patch Detail
Show a patch.
GET /api/1.1/patches/22178/?format=api
{ "id": 22178, "url": "https://patchwork.libcamera.org/api/1.1/patches/22178/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22178/", "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": "<20241205112241.641964-7-paul.elder@ideasonboard.com>", "date": "2024-12-05T11:22:39", "name": "[v4,6/8] ipa: rkisp1: Port to the new AEGC controls", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "4e478f92cd2a101a1f9448a791ac1eb4a9ff6f22", "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/22178/mbox/", "series": [ { "id": 4849, "url": "https://patchwork.libcamera.org/api/1.1/series/4849/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4849", "date": "2024-12-05T11:22:33", "name": "AEGC controls", "version": 4, "mbox": "https://patchwork.libcamera.org/series/4849/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22178/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22178/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 2E07AC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 5 Dec 2024 11:23:08 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 33FC86610A;\n\tThu, 5 Dec 2024 12:23:07 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D04A660EA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 5 Dec 2024 12:23:02 +0100 (CET)", "from neptunite.flets-east.jp (unknown\n\t[IPv6:2404:7a81:160:2100:9fc8:318e:a239:9653])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4C8C2B3;\n\tThu, 5 Dec 2024 12:22:32 +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=\"Agg0hHXQ\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733397753;\n\tbh=ODi/BNLmwisT51iCxJ0yY1PpHh8b/5KzpxA/bT72PrY=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=Agg0hHXQNPW2ZoFPXpY+8zsyQqqyiJDkpjpuy6GQ3R3ZySHjcvQRaGbIGp94wDL3n\n\tZxRuF7qPmGJbDULp/y2UBvdkDBHesiVfmcA/jcg3GvD1K1Qp2GK/NscTfDOg5JnNvk\n\tt97nzzpwcR+jY4Fq5pdL1sbptLpynJVFvbA9wteo=", "From": "Paul Elder <paul.elder@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Paul Elder <paul.elder@ideasonboard.com>", "Subject": "[PATCH v4 6/8] ipa: rkisp1: Port to the new AEGC controls", "Date": "Thu, 5 Dec 2024 20:22:39 +0900", "Message-Id": "<20241205112241.641964-7-paul.elder@ideasonboard.com>", "X-Mailer": "git-send-email 2.39.2", "In-Reply-To": "<20241205112241.641964-1-paul.elder@ideasonboard.com>", "References": "<20241205112241.641964-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": "The 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>\n\n---\nChanges in v4:\n- make auto the default value in the control infos\n- implement the expected behavior when transitioning between manual and\n auto modes\n\nNew in v3\n\nBack 2 years ago in v2 RkISP1 didn't yet support AeEnable properly yet\nso the AeEnable control was simply removed.\n---\n src/ipa/rkisp1/algorithms/agc.cpp | 102 +++++++++++++++++++++++++-----\n src/ipa/rkisp1/ipa_context.cpp | 22 +++++--\n src/ipa/rkisp1/ipa_context.h | 8 ++-\n 3 files changed, 110 insertions(+), 22 deletions(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex 40e5a8f481b2..c92115e8cc0d 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,44 @@ 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\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 +269,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 +318,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 +385,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@@ -459,6 +518,17 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \t\t\t\t\t / context.configuration.sensor.lineDuration;\n \tactiveState.agc.automatic.gain = aGain;\n \n+\t/*\n+\t * Populate auto exposure and gain from the active manual values when\n+\t * transitioning from manual to auto\n+\t *\n+\t * The frame context doesn't need to be populated here as it will be populated in prepare()\n+\t */\n+\tif (frameContext.agc.autoGainEnabled && frameContext.agc.autoGainModeChange)\n+\t\tcontext.activeState.agc.automatic.exposure = context.activeState.agc.manual.exposure;\n+\tif (frameContext.agc.autoGainEnabled && frameContext.agc.autoGainModeChange)\n+\t\tcontext.activeState.agc.automatic.gain = context.activeState.agc.manual.gain;\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 80b99df8eaf8..c7ccc3093078 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,14 @@ 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 between the previous and\n+ * current frame\n+ *\n+ * \\var IPAFrameContext::agc.autoGainModeChange\n+ * \\brief Indicate if autoGainEnabled has changed between the previous and\n+ * current frame\n */\n \n /**\ndiff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\nindex deb8c196f1b8..bc3d5a24be3a 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": [ "v4", "6/8" ] }