[{"id":38058,"web_url":"https://patchwork.libcamera.org/comment/38058/","msgid":"<aYH_Lf1ZrZ04hKrp@zed>","date":"2026-02-03T14:08:20","subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Rui,\n\nOn Sun, Feb 01, 2026 at 02:16:03PM -0500, Rui Wang wrote:\n> Register NoiseReductionMode controls based on the tuning data and default\n> to the active DPF mode. Remove the static NoiseReductionMode entry from\n> the IPA control map now that DPF owns its registration.\n>\n> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>\n> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n>\n> ---\n>   changelog :\n>     --No change since v10\n>   changelog since v11:\n>    - Simple Debug log\n> ---\n>  src/ipa/rkisp1/algorithms/dpf.cpp | 21 +++++++++++++++++++++\n>  src/ipa/rkisp1/algorithms/dpf.h   |  1 +\n>  src/ipa/rkisp1/rkisp1.cpp         |  1 -\n>  3 files changed, 22 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index 9d7fcc1c..303d5cab 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -52,6 +52,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n>  \tif (ret)\n>  \t\treturn ret;\n>\n> +\t/* Register available controls. */\n> +\tregisterControls(context);\n> +\n>  \treturn 0;\n>  }\n>\n> @@ -117,6 +120,24 @@ int Dpf::parseConfig(const YamlObject &tuningData)\n>  \treturn 0;\n>  }\n>\n> +void Dpf::registerControls(IPAContext &context)\n> +{\n> +\t/*\n> +\t * Populate the control map with the available noise reduction modes.\n> +\t * This allows applications to query and select from the modes defined\n> +\t * in the tuning data.\n> +\t */\n> +\tstd::vector<ControlValue> modes{};\n> +\tfor (const auto &mode : noiseReductionModes_) {\n> +\t\tmodes.emplace_back(mode.modeValue);\n> +\t}\n\nNo {}\n\n> +\t/*\n> +\t * Set the default mode to the active mode.\n> +\t */\n\nFits in 1 line\n\n> +\tcontext.ctrlMap[&controls::draft::NoiseReductionMode] =\n> +\t\tControlInfo(modes, activeMode_->modeValue);\n\nOn v11 I asked the following question, and I'll paste it here with\nyour reply below, as I haven't replied on v11 (sorry about that).\n\n-------------------------------------------------------------------------------\n> I might have missed why you need a separate function to re-loop over\n> modes instead of doing this when parsing modes.\n>\n> Was the patch I sent not functional ? Do you prefer a different\n> approach ? Can you share your design decisions or simply reply to that\n> email if you didn't agree with the suggestion explaining why ?\n\nHello Jacopo,\n\n\nThe patch you sharing of add ctrlMap is working ,\n\nI am thinking all controls can be declare in registerControls , and in\nfuture manual mode\n\nwould explicit many controls will add here. but actually , it will\nintroduce a looper to push back\n\neach element from noiseReductionModes_ to control map.and it can also\ndecouple the ctrlMap\n\nwith tuning config\n-------------------------------------------------------------------------------\n\nI'm not sure I 100% understand why manual mode affects the available\nmodes map.\n\ncontext.ctrlMap[&controls::draft::NoiseReductionMode] should be\npopulated with the entries from tuning file.\n\nWhat I would expect (copying here from my v8 proposal at\nhttps://patchwork.libcamera.org/patch/25818/#37709)\n\nWith \"minimal\" and \"highquality\" in the tuning\nfile:\n\n# cam -c1 --list-controls\nControl: [inout] draft::NoiseReductionMode:\n  - NoiseReductionModeOff (0) [default]\n  - NoiseReductionModeMinimal (3)\n  - NoiseReductionModeHighQuality (2)\n\nWith only \"minimal\" in the tuning file\n\n# cam -c1 --list-controls\nControl: [inout] draft::NoiseReductionMode:\n  - NoiseReductionModeOff (0) [default]\n  - NoiseReductionModeMinimal (3)\n\netc etc\n\nI expect manual mode to allow users to override the single parameters\nthat define a \"mode\" not to change the enumeration of available modes.\n\nAnyway, since I might be missing the overall plan and you seem\nconvinced this is the way to go let's:\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nand we'll eventually rework on top if needed\n\n\n> +}\n> +\n>  int Dpf::parseSingleConfig(const YamlObject &tuningData,\n>  \t\t\t   rkisp1_cif_isp_dpf_config &config,\n>  \t\t\t   rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> index 11fc88e4..43effcbe 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.h\n> +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> @@ -37,6 +37,7 @@ private:\n>  \t};\n>\n>  \tint parseConfig(const YamlObject &tuningData);\n> +\tvoid registerControls(IPAContext &context);\n>  \tint parseSingleConfig(const YamlObject &tuningData,\n>  \t\t\t      rkisp1_cif_isp_dpf_config &config,\n>  \t\t\t      rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index fbcc3910..402ed62c 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -120,7 +120,6 @@ const IPAHwSettings ipaHwSettingsV12{\n>  /* List of controls handled by the RkISP1 IPA */\n>  const ControlInfoMap::Map rkisp1Controls{\n>  \t{ &controls::DebugMetadataEnable, ControlInfo(false, true, false) },\n> -\t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n>  };\n>\n>  } /* namespace */\n> --\n> 2.43.0\n>","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 24035BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Feb 2026 14:08:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 134DD62015;\n\tTue,  3 Feb 2026 15:08:25 +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 A7A8B61A35\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Feb 2026 15:08:23 +0100 (CET)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 236645B2;\n\tTue,  3 Feb 2026 15:07:42 +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=\"dYTe4ivO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770127662;\n\tbh=M58v3ew2SKOohmEXXbaVycb+fjrMhz5k6BTuzYaL72w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dYTe4ivOugzv9G2hfkTqilpRydoAN4sbyUyEYr63RWJuVuQlz8+/3e4+5X46bnpse\n\txci81HuSG3uM8Fb0wHbe95zz0kjLC4t8kQ9QrePjh3iA2KAn6sPF8AgozYouZjVIE5\n\tNdyHcQm4gpxxVcV7jg1zZKHzcHc1OF+fVkrBmwEs=","Date":"Tue, 3 Feb 2026 15:08:20 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Rui Wang <rui.wang@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, \n\tIsaac Scott <isaac.scott@ideasonboard.com>","Subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","Message-ID":"<aYH_Lf1ZrZ04hKrp@zed>","References":"<20260201191607.2740223-1-rui.wang@ideasonboard.com>\n\t<20260201191607.2740223-4-rui.wang@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260201191607.2740223-4-rui.wang@ideasonboard.com>","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>"}},{"id":38064,"web_url":"https://patchwork.libcamera.org/comment/38064/","msgid":"<9d69f0f2-ad27-4bc5-8d7e-92fd0c57c0d7@ideasonboard.com>","date":"2026-02-03T19:07:35","subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"On 2026-02-03 09:08, Jacopo Mondi wrote:\n> Rui,\n>\n> On Sun, Feb 01, 2026 at 02:16:03PM -0500, Rui Wang wrote:\n>> Register NoiseReductionMode controls based on the tuning data and default\n>> to the active DPF mode. Remove the static NoiseReductionMode entry from\n>> the IPA control map now that DPF owns its registration.\n>>\n>> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>\n>> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n>>\n>> ---\n>>    changelog :\n>>      --No change since v10\n>>    changelog since v11:\n>>     - Simple Debug log\n>> ---\n>>   src/ipa/rkisp1/algorithms/dpf.cpp | 21 +++++++++++++++++++++\n>>   src/ipa/rkisp1/algorithms/dpf.h   |  1 +\n>>   src/ipa/rkisp1/rkisp1.cpp         |  1 -\n>>   3 files changed, 22 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n>> index 9d7fcc1c..303d5cab 100644\n>> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n>> @@ -52,6 +52,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n>>   \tif (ret)\n>>   \t\treturn ret;\n>>\n>> +\t/* Register available controls. */\n>> +\tregisterControls(context);\n>> +\n>>   \treturn 0;\n>>   }\n>>\n>> @@ -117,6 +120,24 @@ int Dpf::parseConfig(const YamlObject &tuningData)\n>>   \treturn 0;\n>>   }\n>>\n>> +void Dpf::registerControls(IPAContext &context)\n>> +{\n>> +\t/*\n>> +\t * Populate the control map with the available noise reduction modes.\n>> +\t * This allows applications to query and select from the modes defined\n>> +\t * in the tuning data.\n>> +\t */\n>> +\tstd::vector<ControlValue> modes{};\n>> +\tfor (const auto &mode : noiseReductionModes_) {\n>> +\t\tmodes.emplace_back(mode.modeValue);\n>> +\t}\n> No {}\n>\n>> +\t/*\n>> +\t * Set the default mode to the active mode.\n>> +\t */\n> Fits in 1 line\n>\n>> +\tcontext.ctrlMap[&controls::draft::NoiseReductionMode] =\n>> +\t\tControlInfo(modes, activeMode_->modeValue);\n> On v11 I asked the following question, and I'll paste it here with\n> your reply below, as I haven't replied on v11 (sorry about that).\n>\n> -------------------------------------------------------------------------------\n>> I might have missed why you need a separate function to re-loop over\n>> modes instead of doing this when parsing modes.\n>>\n>> Was the patch I sent not functional ? Do you prefer a different\n>> approach ? Can you share your design decisions or simply reply to that\n>> email if you didn't agree with the suggestion explaining why ?\n> Hello Jacopo,\n>\n>\n> The patch you sharing of add ctrlMap is working ,\n>\n> I am thinking all controls can be declare in registerControls , and in\n> future manual mode\n>\n> would explicit many controls will add here. but actually , it will\n> introduce a looper to push back\n>\n> each element from noiseReductionModes_ to control map.and it can also\n> decouple the ctrlMap\n>\n> with tuning config\n> -------------------------------------------------------------------------------\n>\n> I'm not sure I 100% understand why manual mode affects the available\n> modes map.\n>\n> context.ctrlMap[&controls::draft::NoiseReductionMode] should be\n> populated with the entries from tuning file.\n\nHello ,\nSorry I did not express clearly in v11 reply. and made you misunderstood.\n\nI am going to exposure all dpf regsiters into controls as manual mode.\n\nlike : DomainFilter kernel value[6] nll [17] , strength [3]\n\nand all those excute into this registerControls\n\n\n  Rui\n\n> What I would expect (copying here from my v8 proposal at\n> https://patchwork.libcamera.org/patch/25818/#37709)\n>\n> With \"minimal\" and \"highquality\" in the tuning\n> file:\n>\n> # cam -c1 --list-controls\n> Control: [inout] draft::NoiseReductionMode:\n>    - NoiseReductionModeOff (0) [default]\n>    - NoiseReductionModeMinimal (3)\n>    - NoiseReductionModeHighQuality (2)\n>\n> With only \"minimal\" in the tuning file\n>\n> # cam -c1 --list-controls\n> Control: [inout] draft::NoiseReductionMode:\n>    - NoiseReductionModeOff (0) [default]\n>    - NoiseReductionModeMinimal (3)\n>\n> etc etc\n>\nI build and run current branch and with imx219.yaml\n\n# cam -c1 --list-controls\nControl: [inout] draft::NoiseReductionMode:\n   - NoiseReductionModeOff (0)\n   - NoiseReductionModeMinimal (3)\n   - NoiseReductionModeHighQuality (2) [default]\n\nis your camera as imx219 ?\n\n\n\n>\n> I expect manual mode to allow users to override the single parameters\n> that define a \"mode\" not to change the enumeration of available modes.\n>\n> Anyway, since I might be missing the overall plan and you seem\n> convinced this is the way to go let's:\n>\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>\n> and we'll eventually rework on top if needed\n>\n>\n>> +}\n>> +\n>>   int Dpf::parseSingleConfig(const YamlObject &tuningData,\n>>   \t\t\t   rkisp1_cif_isp_dpf_config &config,\n>>   \t\t\t   rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n>> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n>> index 11fc88e4..43effcbe 100644\n>> --- a/src/ipa/rkisp1/algorithms/dpf.h\n>> +++ b/src/ipa/rkisp1/algorithms/dpf.h\n>> @@ -37,6 +37,7 @@ private:\n>>   \t};\n>>\n>>   \tint parseConfig(const YamlObject &tuningData);\n>> +\tvoid registerControls(IPAContext &context);\n>>   \tint parseSingleConfig(const YamlObject &tuningData,\n>>   \t\t\t      rkisp1_cif_isp_dpf_config &config,\n>>   \t\t\t      rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index fbcc3910..402ed62c 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -120,7 +120,6 @@ const IPAHwSettings ipaHwSettingsV12{\n>>   /* List of controls handled by the RkISP1 IPA */\n>>   const ControlInfoMap::Map rkisp1Controls{\n>>   \t{ &controls::DebugMetadataEnable, ControlInfo(false, true, false) },\n>> -\t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n>>   };\n>>\n>>   } /* namespace */\n>> --\n>> 2.43.0\n>>","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 B8B38BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Feb 2026 19:07:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC5B66201A;\n\tTue,  3 Feb 2026 20:07:50 +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 7370961A35\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Feb 2026 20:07:49 +0100 (CET)","from [192.168.31.114] (unknown [209.216.103.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 28A77673;\n\tTue,  3 Feb 2026 20:07:06 +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=\"QGi9sUgh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770145627;\n\tbh=Eox8qMF/j09oIL8Gp6xFwT52aYkY3asb7rRssteeq1w=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=QGi9sUghmCK8k7q4z9VsizFFmgY58sHb29N1YKimpNL/bsBNThu4J/SKhiNqHldgL\n\tea9jItPYmz6Lj3WQUp0uzY8PG8gXa0Pc1tO7t9BzX2EV6n/KADcaMSCkB+c6JODbbQ\n\tRX4LxcZzxOX21I8HMw1BrXtGeQ4QOMd9eVVU95rA=","Message-ID":"<9d69f0f2-ad27-4bc5-8d7e-92fd0c57c0d7@ideasonboard.com>","Date":"Tue, 3 Feb 2026 14:07:35 -0500","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tIsaac Scott <isaac.scott@ideasonboard.com>","References":"<20260201191607.2740223-1-rui.wang@ideasonboard.com>\n\t<20260201191607.2740223-4-rui.wang@ideasonboard.com>\n\t<aYH_Lf1ZrZ04hKrp@zed>","Content-Language":"en-US","From":"rui wang <rui.wang@ideasonboard.com>","In-Reply-To":"<aYH_Lf1ZrZ04hKrp@zed>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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>"}},{"id":38071,"web_url":"https://patchwork.libcamera.org/comment/38071/","msgid":"<aYMdr78wTMammI81@zed>","date":"2026-02-04T10:26:35","subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Rui\n\nOn Tue, Feb 03, 2026 at 02:07:35PM -0500, rui wang wrote:\n>\n> On 2026-02-03 09:08, Jacopo Mondi wrote:\n> > Rui,\n> >\n> > On Sun, Feb 01, 2026 at 02:16:03PM -0500, Rui Wang wrote:\n> > > Register NoiseReductionMode controls based on the tuning data and default\n> > > to the active DPF mode. Remove the static NoiseReductionMode entry from\n> > > the IPA control map now that DPF owns its registration.\n> > >\n> > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>\n> > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> > >\n> > > ---\n> > >    changelog :\n> > >      --No change since v10\n> > >    changelog since v11:\n> > >     - Simple Debug log\n> > > ---\n> > >   src/ipa/rkisp1/algorithms/dpf.cpp | 21 +++++++++++++++++++++\n> > >   src/ipa/rkisp1/algorithms/dpf.h   |  1 +\n> > >   src/ipa/rkisp1/rkisp1.cpp         |  1 -\n> > >   3 files changed, 22 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > index 9d7fcc1c..303d5cab 100644\n> > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > @@ -52,6 +52,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> > >   \tif (ret)\n> > >   \t\treturn ret;\n> > >\n> > > +\t/* Register available controls. */\n> > > +\tregisterControls(context);\n> > > +\n> > >   \treturn 0;\n> > >   }\n> > >\n> > > @@ -117,6 +120,24 @@ int Dpf::parseConfig(const YamlObject &tuningData)\n> > >   \treturn 0;\n> > >   }\n> > >\n> > > +void Dpf::registerControls(IPAContext &context)\n> > > +{\n> > > +\t/*\n> > > +\t * Populate the control map with the available noise reduction modes.\n> > > +\t * This allows applications to query and select from the modes defined\n> > > +\t * in the tuning data.\n> > > +\t */\n> > > +\tstd::vector<ControlValue> modes{};\n> > > +\tfor (const auto &mode : noiseReductionModes_) {\n> > > +\t\tmodes.emplace_back(mode.modeValue);\n> > > +\t}\n> > No {}\n> >\n> > > +\t/*\n> > > +\t * Set the default mode to the active mode.\n> > > +\t */\n> > Fits in 1 line\n> >\n> > > +\tcontext.ctrlMap[&controls::draft::NoiseReductionMode] =\n\nI just noticed it is probably worth moving NoiseReductionMode out of\ndraft and make it a core libcamera control\n\n> > > +\t\tControlInfo(modes, activeMode_->modeValue);\n> > On v11 I asked the following question, and I'll paste it here with\n> > your reply below, as I haven't replied on v11 (sorry about that).\n> >\n> > -------------------------------------------------------------------------------\n> > > I might have missed why you need a separate function to re-loop over\n> > > modes instead of doing this when parsing modes.\n> > >\n> > > Was the patch I sent not functional ? Do you prefer a different\n> > > approach ? Can you share your design decisions or simply reply to that\n> > > email if you didn't agree with the suggestion explaining why ?\n> > Hello Jacopo,\n> >\n> >\n> > The patch you sharing of add ctrlMap is working ,\n> >\n> > I am thinking all controls can be declare in registerControls , and in\n> > future manual mode\n> >\n> > would explicit many controls will add here. but actually , it will\n> > introduce a looper to push back\n> >\n> > each element from noiseReductionModes_ to control map.and it can also\n> > decouple the ctrlMap\n> >\n> > with tuning config\n> > -------------------------------------------------------------------------------\n> >\n> > I'm not sure I 100% understand why manual mode affects the available\n> > modes map.\n> >\n> > context.ctrlMap[&controls::draft::NoiseReductionMode] should be\n> > populated with the entries from tuning file.\n>\n> Hello ,\n> Sorry I did not express clearly in v11 reply. and made you misunderstood.\n>\n> I am going to exposure all dpf regsiters into controls as manual mode.\n>\n> like : DomainFilter kernel value[6] nll [17] , strength [3]\n>\n> and all those excute into this registerControls\n\nmmm, ok, however manual controls won't be registered as values for\ncontrols::draft::NoiseReductionMode but will go through other\ncontrols, right ?\n\nAlso, the manual controls are platform specific ones which expose the\nregister interface of the RkISP1. How do we plan to expose them ?\nThrough a dedicated set of controls in a newly introduced\ncontrols::rkisp1:: namespace ?\n\n>\n>\n>  Rui\n>\n> > What I would expect (copying here from my v8 proposal at\n> > https://patchwork.libcamera.org/patch/25818/#37709)\n> >\n> > With \"minimal\" and \"highquality\" in the tuning\n> > file:\n> >\n> > # cam -c1 --list-controls\n> > Control: [inout] draft::NoiseReductionMode:\n> >    - NoiseReductionModeOff (0) [default]\n> >    - NoiseReductionModeMinimal (3)\n> >    - NoiseReductionModeHighQuality (2)\n> >\n> > With only \"minimal\" in the tuning file\n> >\n> > # cam -c1 --list-controls\n> > Control: [inout] draft::NoiseReductionMode:\n> >    - NoiseReductionModeOff (0) [default]\n> >    - NoiseReductionModeMinimal (3)\n> >\n> > etc etc\n> >\n> I build and run current branch and with imx219.yaml\n>\n> # cam -c1 --list-controls\n> Control: [inout] draft::NoiseReductionMode:\n>   - NoiseReductionModeOff (0)\n>   - NoiseReductionModeMinimal (3)\n>   - NoiseReductionModeHighQuality (2) [default]\n>\n> is your camera as imx219 ?\n>\n>\n>\n> >\n> > I expect manual mode to allow users to override the single parameters\n> > that define a \"mode\" not to change the enumeration of available modes.\n> >\n> > Anyway, since I might be missing the overall plan and you seem\n> > convinced this is the way to go let's:\n> >\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> >\n> > and we'll eventually rework on top if needed\n> >\n> >\n> > > +}\n> > > +\n> > >   int Dpf::parseSingleConfig(const YamlObject &tuningData,\n> > >   \t\t\t   rkisp1_cif_isp_dpf_config &config,\n> > >   \t\t\t   rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n> > > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> > > index 11fc88e4..43effcbe 100644\n> > > --- a/src/ipa/rkisp1/algorithms/dpf.h\n> > > +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> > > @@ -37,6 +37,7 @@ private:\n> > >   \t};\n> > >\n> > >   \tint parseConfig(const YamlObject &tuningData);\n> > > +\tvoid registerControls(IPAContext &context);\n> > >   \tint parseSingleConfig(const YamlObject &tuningData,\n> > >   \t\t\t      rkisp1_cif_isp_dpf_config &config,\n> > >   \t\t\t      rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index fbcc3910..402ed62c 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -120,7 +120,6 @@ const IPAHwSettings ipaHwSettingsV12{\n> > >   /* List of controls handled by the RkISP1 IPA */\n> > >   const ControlInfoMap::Map rkisp1Controls{\n> > >   \t{ &controls::DebugMetadataEnable, ControlInfo(false, true, false) },\n> > > -\t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > >   };\n> > >\n> > >   } /* namespace */\n> > > --\n> > > 2.43.0\n> > >","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 10F26C31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Feb 2026 10:26:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 134E762029;\n\tWed,  4 Feb 2026 11:26: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 7BC1F615B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Feb 2026 11:26:38 +0100 (CET)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7C118C79;\n\tWed,  4 Feb 2026 11:25:56 +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=\"XP1FAbEH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770200756;\n\tbh=yWAKmubnDs6xWUtblQ/iSPmoxGAFhKkedw4zv4w1pSc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XP1FAbEH64bRR6NekqKwmQSp6rZW+HLHIH3nvX81rR0irbSEye+jnQ2rbTD3whsHy\n\tU7HuBm8mDlf3M5jG3reAdCh1758Lz7RD7vTrwvfc7nOXBfPexG4nJO6IpTdfVSD4D1\n\tOULYZTy76N4SiUpq7VsECPukrTJczCEVe6y/++fw=","Date":"Wed, 4 Feb 2026 11:26:35 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"rui wang <rui.wang@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org,\n\tIsaac Scott <isaac.scott@ideasonboard.com>","Subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","Message-ID":"<aYMdr78wTMammI81@zed>","References":"<20260201191607.2740223-1-rui.wang@ideasonboard.com>\n\t<20260201191607.2740223-4-rui.wang@ideasonboard.com>\n\t<aYH_Lf1ZrZ04hKrp@zed>\n\t<9d69f0f2-ad27-4bc5-8d7e-92fd0c57c0d7@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<9d69f0f2-ad27-4bc5-8d7e-92fd0c57c0d7@ideasonboard.com>","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>"}},{"id":38072,"web_url":"https://patchwork.libcamera.org/comment/38072/","msgid":"<3b640776-531c-4423-9ab8-81282bcfee98@ideasonboard.com>","date":"2026-02-04T12:29:03","subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"On 2026-02-04 05:26, Jacopo Mondi wrote:\n> Hi Rui\n>\n> On Tue, Feb 03, 2026 at 02:07:35PM -0500, rui wang wrote:\n>> On 2026-02-03 09:08, Jacopo Mondi wrote:\n>>> Rui,\n>>>\n>>> On Sun, Feb 01, 2026 at 02:16:03PM -0500, Rui Wang wrote:\n>>>> Register NoiseReductionMode controls based on the tuning data and default\n>>>> to the active DPF mode. Remove the static NoiseReductionMode entry from\n>>>> the IPA control map now that DPF owns its registration.\n>>>>\n>>>> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>\n>>>> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n>>>>\n>>>> ---\n>>>>     changelog :\n>>>>       --No change since v10\n>>>>     changelog since v11:\n>>>>      - Simple Debug log\n>>>> ---\n>>>>    src/ipa/rkisp1/algorithms/dpf.cpp | 21 +++++++++++++++++++++\n>>>>    src/ipa/rkisp1/algorithms/dpf.h   |  1 +\n>>>>    src/ipa/rkisp1/rkisp1.cpp         |  1 -\n>>>>    3 files changed, 22 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n>>>> index 9d7fcc1c..303d5cab 100644\n>>>> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n>>>> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n>>>> @@ -52,6 +52,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n>>>>    \tif (ret)\n>>>>    \t\treturn ret;\n>>>>\n>>>> +\t/* Register available controls. */\n>>>> +\tregisterControls(context);\n>>>> +\n>>>>    \treturn 0;\n>>>>    }\n>>>>\n>>>> @@ -117,6 +120,24 @@ int Dpf::parseConfig(const YamlObject &tuningData)\n>>>>    \treturn 0;\n>>>>    }\n>>>>\n>>>> +void Dpf::registerControls(IPAContext &context)\n>>>> +{\n>>>> +\t/*\n>>>> +\t * Populate the control map with the available noise reduction modes.\n>>>> +\t * This allows applications to query and select from the modes defined\n>>>> +\t * in the tuning data.\n>>>> +\t */\n>>>> +\tstd::vector<ControlValue> modes{};\n>>>> +\tfor (const auto &mode : noiseReductionModes_) {\n>>>> +\t\tmodes.emplace_back(mode.modeValue);\n>>>> +\t}\n>>> No {}\n>>>\n>>>> +\t/*\n>>>> +\t * Set the default mode to the active mode.\n>>>> +\t */\n>>> Fits in 1 line\n>>>\n>>>> +\tcontext.ctrlMap[&controls::draft::NoiseReductionMode] =\n> I just noticed it is probably worth moving NoiseReductionMode out of\n> draft and make it a core libcamera control\n\n\nYes , I am think moving this draft controls to libcamera controls after \nthose patch merged\n\nit and will submit into seperate PR for this ,because it will change \nRPI's module as well.\n\n\n\n\n>\n>>>> +\t\tControlInfo(modes, activeMode_->modeValue);\n>>> On v11 I asked the following question, and I'll paste it here with\n>>> your reply below, as I haven't replied on v11 (sorry about that).\n>>>\n>>> -------------------------------------------------------------------------------\n>>>> I might have missed why you need a separate function to re-loop over\n>>>> modes instead of doing this when parsing modes.\n>>>>\n>>>> Was the patch I sent not functional ? Do you prefer a different\n>>>> approach ? Can you share your design decisions or simply reply to that\n>>>> email if you didn't agree with the suggestion explaining why ?\n>>> Hello Jacopo,\n>>>\n>>>\n>>> The patch you sharing of add ctrlMap is working ,\n>>>\n>>> I am thinking all controls can be declare in registerControls , and in\n>>> future manual mode\n>>>\n>>> would explicit many controls will add here. but actually , it will\n>>> introduce a looper to push back\n>>>\n>>> each element from noiseReductionModes_ to control map.and it can also\n>>> decouple the ctrlMap\n>>>\n>>> with tuning config\n>>> -------------------------------------------------------------------------------\n>>>\n>>> I'm not sure I 100% understand why manual mode affects the available\n>>> modes map.\n>>>\n>>> context.ctrlMap[&controls::draft::NoiseReductionMode] should be\n>>> populated with the entries from tuning file.\n>> Hello ,\n>> Sorry I did not express clearly in v11 reply. and made you misunderstood.\n>>\n>> I am going to exposure all dpf regsiters into controls as manual mode.\n>>\n>> like : DomainFilter kernel value[6] nll [17] , strength [3]\n>>\n>> and all those excute into this registerControls\n> mmm, ok, however manual controls won't be registered as values for\n> controls::draft::NoiseReductionMode but will go through other\n> controls, right ?\n>\n> Also, the manual controls are platform specific ones which expose the\n> register interface of the RkISP1. How do we plan to expose them ?\n> Through a dedicated set of controls in a newly introduced\n> controls::rkisp1:: namespace ?\n\n\n          As my expectation ,\"Manual control\" will  belong to\n\n\n          controls::draft::NoiseReductionMode ,\n\n           From CamShark or any client once controls::draft::Manual\n\n\n          is selected, the all DPF's register values can be edit and\n          accepted\n\n\n          from controls in manual mode.\n\n\n            and \"manual mode\" would not break other\n\n\n          NoiseReductionMode logic .\n\n\n          all those dpf registers  will be in namespace :\n\n\n          controls::rkisp1\n          I had a debug version :\n          https://git.ideasonboard.com/ruiwang2002/libcamera_rui_1/pulls/2/files#diff-4420c2393ceee9cd5ff9a751d04baf9ee580c739\n\n\n>\n>>\n>>   Rui\n>>\n>>> What I would expect (copying here from my v8 proposal at\n>>> https://patchwork.libcamera.org/patch/25818/#37709)\n>>>\n>>> With \"minimal\" and \"highquality\" in the tuning\n>>> file:\n>>>\n>>> # cam -c1 --list-controls\n>>> Control: [inout] draft::NoiseReductionMode:\n>>>     - NoiseReductionModeOff (0) [default]\n>>>     - NoiseReductionModeMinimal (3)\n>>>     - NoiseReductionModeHighQuality (2)\n>>>\n>>> With only \"minimal\" in the tuning file\n>>>\n>>> # cam -c1 --list-controls\n>>> Control: [inout] draft::NoiseReductionMode:\n>>>     - NoiseReductionModeOff (0) [default]\n>>>     - NoiseReductionModeMinimal (3)\n>>>\n>>> etc etc\n>>>\n>> I build and run current branch and with imx219.yaml\n>>\n>> # cam -c1 --list-controls\n>> Control: [inout] draft::NoiseReductionMode:\n>>    - NoiseReductionModeOff (0)\n>>    - NoiseReductionModeMinimal (3)\n>>    - NoiseReductionModeHighQuality (2) [default]\n>>\n>> is your camera as imx219 ?\n>>\n>>\n>>\n>>> I expect manual mode to allow users to override the single parameters\n>>> that define a \"mode\" not to change the enumeration of available modes.\n>>>\n>>> Anyway, since I might be missing the overall plan and you seem\n>>> convinced this is the way to go let's:\n>>>\n>>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>>>\n>>> and we'll eventually rework on top if needed\n>>>\n>>>\n>>>> +}\n>>>> +\n>>>>    int Dpf::parseSingleConfig(const YamlObject &tuningData,\n>>>>    \t\t\t   rkisp1_cif_isp_dpf_config &config,\n>>>>    \t\t\t   rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n>>>> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n>>>> index 11fc88e4..43effcbe 100644\n>>>> --- a/src/ipa/rkisp1/algorithms/dpf.h\n>>>> +++ b/src/ipa/rkisp1/algorithms/dpf.h\n>>>> @@ -37,6 +37,7 @@ private:\n>>>>    \t};\n>>>>\n>>>>    \tint parseConfig(const YamlObject &tuningData);\n>>>> +\tvoid registerControls(IPAContext &context);\n>>>>    \tint parseSingleConfig(const YamlObject &tuningData,\n>>>>    \t\t\t      rkisp1_cif_isp_dpf_config &config,\n>>>>    \t\t\t      rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n>>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>>> index fbcc3910..402ed62c 100644\n>>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>>> @@ -120,7 +120,6 @@ const IPAHwSettings ipaHwSettingsV12{\n>>>>    /* List of controls handled by the RkISP1 IPA */\n>>>>    const ControlInfoMap::Map rkisp1Controls{\n>>>>    \t{ &controls::DebugMetadataEnable, ControlInfo(false, true, false) },\n>>>> -\t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n>>>>    };\n>>>>\n>>>>    } /* namespace */\n>>>> --\n>>>> 2.43.0\n>>>>","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 9CC2ABD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Feb 2026 12:29:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9FEC62029;\n\tWed,  4 Feb 2026 13:29:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6EB3261FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Feb 2026 13:29:17 +0100 (CET)","from [192.168.31.114] (unknown [209.216.103.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7A77846F;\n\tWed,  4 Feb 2026 13:28:34 +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=\"arldAYyx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770208115;\n\tbh=CGQqSg6oZ/kBCq1vUUeC/FAgMZtf9hpAJgoXDK/cA14=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=arldAYyxUw0DBM/0kCbdrCcyPzT+g5PeCv4dJ75V1uIGeeebTzo3ET/QogZjfQRyJ\n\tCephbXDoP4dnMrSto9z1qRjWnfZ5XuD0zoThrge9aJMiE+ucaaMkbESJ4v8kthC2OS\n\t1gBHWwNpP+fddvvJBxTokAlU/AlPmXxqSmMbQxEk=","Message-ID":"<3b640776-531c-4423-9ab8-81282bcfee98@ideasonboard.com>","Date":"Wed, 4 Feb 2026 07:29:03 -0500","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tIsaac Scott <isaac.scott@ideasonboard.com>","References":"<20260201191607.2740223-1-rui.wang@ideasonboard.com>\n\t<20260201191607.2740223-4-rui.wang@ideasonboard.com>\n\t<aYH_Lf1ZrZ04hKrp@zed>\n\t<9d69f0f2-ad27-4bc5-8d7e-92fd0c57c0d7@ideasonboard.com>\n\t<aYMdr78wTMammI81@zed>","Content-Language":"en-US","From":"rui wang <rui.wang@ideasonboard.com>","In-Reply-To":"<aYMdr78wTMammI81@zed>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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>"}},{"id":38073,"web_url":"https://patchwork.libcamera.org/comment/38073/","msgid":"<aYNBFI58UY3EibxR@zed>","date":"2026-02-04T13:00:52","subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Rui\n\nOn Wed, Feb 04, 2026 at 07:29:03AM -0500, rui wang wrote:\n>\n> On 2026-02-04 05:26, Jacopo Mondi wrote:\n> > Hi Rui\n> >\n> > On Tue, Feb 03, 2026 at 02:07:35PM -0500, rui wang wrote:\n> > > On 2026-02-03 09:08, Jacopo Mondi wrote:\n> > > > Rui,\n> > > >\n> > > > On Sun, Feb 01, 2026 at 02:16:03PM -0500, Rui Wang wrote:\n> > > > > Register NoiseReductionMode controls based on the tuning data and default\n> > > > > to the active DPF mode. Remove the static NoiseReductionMode entry from\n> > > > > the IPA control map now that DPF owns its registration.\n> > > > >\n> > > > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>\n> > > > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> > > > >\n> > > > > ---\n> > > > >     changelog :\n> > > > >       --No change since v10\n> > > > >     changelog since v11:\n> > > > >      - Simple Debug log\n> > > > > ---\n> > > > >    src/ipa/rkisp1/algorithms/dpf.cpp | 21 +++++++++++++++++++++\n> > > > >    src/ipa/rkisp1/algorithms/dpf.h   |  1 +\n> > > > >    src/ipa/rkisp1/rkisp1.cpp         |  1 -\n> > > > >    3 files changed, 22 insertions(+), 1 deletion(-)\n> > > > >\n> > > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > > > index 9d7fcc1c..303d5cab 100644\n> > > > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > > > @@ -52,6 +52,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> > > > >    \tif (ret)\n> > > > >    \t\treturn ret;\n> > > > >\n> > > > > +\t/* Register available controls. */\n> > > > > +\tregisterControls(context);\n> > > > > +\n> > > > >    \treturn 0;\n> > > > >    }\n> > > > >\n> > > > > @@ -117,6 +120,24 @@ int Dpf::parseConfig(const YamlObject &tuningData)\n> > > > >    \treturn 0;\n> > > > >    }\n> > > > >\n> > > > > +void Dpf::registerControls(IPAContext &context)\n> > > > > +{\n> > > > > +\t/*\n> > > > > +\t * Populate the control map with the available noise reduction modes.\n> > > > > +\t * This allows applications to query and select from the modes defined\n> > > > > +\t * in the tuning data.\n> > > > > +\t */\n> > > > > +\tstd::vector<ControlValue> modes{};\n> > > > > +\tfor (const auto &mode : noiseReductionModes_) {\n> > > > > +\t\tmodes.emplace_back(mode.modeValue);\n> > > > > +\t}\n> > > > No {}\n> > > >\n> > > > > +\t/*\n> > > > > +\t * Set the default mode to the active mode.\n> > > > > +\t */\n> > > > Fits in 1 line\n> > > >\n> > > > > +\tcontext.ctrlMap[&controls::draft::NoiseReductionMode] =\n> > I just noticed it is probably worth moving NoiseReductionMode out of\n> > draft and make it a core libcamera control\n>\n>\n> Yes , I am think moving this draft controls to libcamera controls after\n> those patch merged\n\nmm, ok, I would do it before adding more users but I guess that's not\na big deal as long as it happens quickly. Otherwise if we add support\nin rkisp1 for controls::draft::NoiseReductionMode to\nlater change it to controls::NoiseReductionMode in the next release,\nit seems an ABI breakage we could easily avoid.\n\n>\n> it and will submit into seperate PR for this ,because it will change RPI's\n> module as well.\n>\n>\n>\n>\n> >\n> > > > > +\t\tControlInfo(modes, activeMode_->modeValue);\n> > > > On v11 I asked the following question, and I'll paste it here with\n> > > > your reply below, as I haven't replied on v11 (sorry about that).\n> > > >\n> > > > -------------------------------------------------------------------------------\n> > > > > I might have missed why you need a separate function to re-loop over\n> > > > > modes instead of doing this when parsing modes.\n> > > > >\n> > > > > Was the patch I sent not functional ? Do you prefer a different\n> > > > > approach ? Can you share your design decisions or simply reply to that\n> > > > > email if you didn't agree with the suggestion explaining why ?\n> > > > Hello Jacopo,\n> > > >\n> > > >\n> > > > The patch you sharing of add ctrlMap is working ,\n> > > >\n> > > > I am thinking all controls can be declare in registerControls , and in\n> > > > future manual mode\n> > > >\n> > > > would explicit many controls will add here. but actually , it will\n> > > > introduce a looper to push back\n> > > >\n> > > > each element from noiseReductionModes_ to control map.and it can also\n> > > > decouple the ctrlMap\n> > > >\n> > > > with tuning config\n> > > > -------------------------------------------------------------------------------\n> > > >\n> > > > I'm not sure I 100% understand why manual mode affects the available\n> > > > modes map.\n> > > >\n> > > > context.ctrlMap[&controls::draft::NoiseReductionMode] should be\n> > > > populated with the entries from tuning file.\n> > > Hello ,\n> > > Sorry I did not express clearly in v11 reply. and made you misunderstood.\n> > >\n> > > I am going to exposure all dpf regsiters into controls as manual mode.\n> > >\n> > > like : DomainFilter kernel value[6] nll [17] , strength [3]\n> > >\n> > > and all those excute into this registerControls\n> > mmm, ok, however manual controls won't be registered as values for\n> > controls::draft::NoiseReductionMode but will go through other\n> > controls, right ?\n> >\n> > Also, the manual controls are platform specific ones which expose the\n> > register interface of the RkISP1. How do we plan to expose them ?\n> > Through a dedicated set of controls in a newly introduced\n> > controls::rkisp1:: namespace ?\n>\n>\n>          As my expectation ,\"Manual control\" will  belong to\n>\n>\n>          controls::draft::NoiseReductionMode ,\n>\n>           From CamShark or any client once controls::draft::Manual\n>\n>\n>          is selected, the all DPF's register values can be edit and\n>          accepted\n\nDo you mean you plan to add a:\n        controls::draft::NoiseReductionModeManual\n\nvalue in the list of supported modes of\ncontrols::draft::NoiseReductionMode ?\n\n\n>\n>\n>          from controls in manual mode.\n>\n>\n>            and \"manual mode\" would not break other\n>\n>\n>          NoiseReductionMode logic .\n>\n>\n>          all those dpf registers  will be in namespace :\n>\n>\n>          controls::rkisp1\n>          I had a debug version :\n>          https://git.ideasonboard.com/ruiwang2002/libcamera_rui_1/pulls/2/files#diff-4420c2393ceee9cd5ff9a751d04baf9ee580c739\n\nAnd once controls::draft::NoiseReductionModeManual is added the user\ncan control the parameters of the denoise engine throuhg the controls\nyou have added in the above pull request ?\n\nThat seems like a good plan, but to me NoiseReductionModeManual is\nlogically equivalent to NoiseReductionModeOff.\n\nIt doesn't require an entry in the tuning file. If the algorithm\nimplementation supports manual mode, the algorithm will add it to the\nlist of supported modes for controls::draft::NoiseReductionMode.\n\nSo, once manual mode support is added you can add it to the\nnoiseReductionModes_ list in your Dpf::parseConfig() implementation\nbefore parsing the yaml file.\n\nDo we agree or is your interpreatation different ?\n\n>\n>\n> >\n> > >\n> > >   Rui\n> > >\n> > > > What I would expect (copying here from my v8 proposal at\n> > > > https://patchwork.libcamera.org/patch/25818/#37709)\n> > > >\n> > > > With \"minimal\" and \"highquality\" in the tuning\n> > > > file:\n> > > >\n> > > > # cam -c1 --list-controls\n> > > > Control: [inout] draft::NoiseReductionMode:\n> > > >     - NoiseReductionModeOff (0) [default]\n> > > >     - NoiseReductionModeMinimal (3)\n> > > >     - NoiseReductionModeHighQuality (2)\n> > > >\n> > > > With only \"minimal\" in the tuning file\n> > > >\n> > > > # cam -c1 --list-controls\n> > > > Control: [inout] draft::NoiseReductionMode:\n> > > >     - NoiseReductionModeOff (0) [default]\n> > > >     - NoiseReductionModeMinimal (3)\n> > > >\n> > > > etc etc\n> > > >\n> > > I build and run current branch and with imx219.yaml\n> > >\n> > > # cam -c1 --list-controls\n> > > Control: [inout] draft::NoiseReductionMode:\n> > >    - NoiseReductionModeOff (0)\n> > >    - NoiseReductionModeMinimal (3)\n> > >    - NoiseReductionModeHighQuality (2) [default]\n> > >\n> > > is your camera as imx219 ?\n> > >\n> > >\n> > >\n> > > > I expect manual mode to allow users to override the single parameters\n> > > > that define a \"mode\" not to change the enumeration of available modes.\n> > > >\n> > > > Anyway, since I might be missing the overall plan and you seem\n> > > > convinced this is the way to go let's:\n> > > >\n> > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > >\n> > > > and we'll eventually rework on top if needed\n> > > >\n> > > >\n> > > > > +}\n> > > > > +\n> > > > >    int Dpf::parseSingleConfig(const YamlObject &tuningData,\n> > > > >    \t\t\t   rkisp1_cif_isp_dpf_config &config,\n> > > > >    \t\t\t   rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n> > > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> > > > > index 11fc88e4..43effcbe 100644\n> > > > > --- a/src/ipa/rkisp1/algorithms/dpf.h\n> > > > > +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> > > > > @@ -37,6 +37,7 @@ private:\n> > > > >    \t};\n> > > > >\n> > > > >    \tint parseConfig(const YamlObject &tuningData);\n> > > > > +\tvoid registerControls(IPAContext &context);\n> > > > >    \tint parseSingleConfig(const YamlObject &tuningData,\n> > > > >    \t\t\t      rkisp1_cif_isp_dpf_config &config,\n> > > > >    \t\t\t      rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n> > > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > index fbcc3910..402ed62c 100644\n> > > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > @@ -120,7 +120,6 @@ const IPAHwSettings ipaHwSettingsV12{\n> > > > >    /* List of controls handled by the RkISP1 IPA */\n> > > > >    const ControlInfoMap::Map rkisp1Controls{\n> > > > >    \t{ &controls::DebugMetadataEnable, ControlInfo(false, true, false) },\n> > > > > -\t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > >    };\n> > > > >\n> > > > >    } /* namespace */\n> > > > > --\n> > > > > 2.43.0\n> > > > >","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 999AEC31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Feb 2026 13:00:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9BEC56202B;\n\tWed,  4 Feb 2026 14:00:56 +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 75F2261FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Feb 2026 14:00:55 +0100 (CET)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 20BC01023;\n\tWed,  4 Feb 2026 14:00:13 +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=\"N+rQamsk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770210013;\n\tbh=tSmyflHAZQyCcQh52eR7Du/03eg8ZnvQ6Hja9BtjhRA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=N+rQamskRGoCibsFffcDCb9e/QlgoTWDruWRsYc2ivU4N0HTGZTouEzXLzbIESZi8\n\tWEy6kY+YA3H9GhUdsbrasKzOjaYvTWhslHAR7A03CUC1ErYbdNcR69ukxOG5qJCmaW\n\tAtsnHDWRJU7DG8JmTxiJjtmx2gxJYnDNQZKaLPVs=","Date":"Wed, 4 Feb 2026 14:00:52 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"rui wang <rui.wang@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org,\n\tIsaac Scott <isaac.scott@ideasonboard.com>","Subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","Message-ID":"<aYNBFI58UY3EibxR@zed>","References":"<20260201191607.2740223-1-rui.wang@ideasonboard.com>\n\t<20260201191607.2740223-4-rui.wang@ideasonboard.com>\n\t<aYH_Lf1ZrZ04hKrp@zed>\n\t<9d69f0f2-ad27-4bc5-8d7e-92fd0c57c0d7@ideasonboard.com>\n\t<aYMdr78wTMammI81@zed>\n\t<3b640776-531c-4423-9ab8-81282bcfee98@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<3b640776-531c-4423-9ab8-81282bcfee98@ideasonboard.com>","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>"}},{"id":38075,"web_url":"https://patchwork.libcamera.org/comment/38075/","msgid":"<aa35ba34-8ce5-4f71-b11a-10a98c475bf5@ideasonboard.com>","date":"2026-02-04T13:55:44","subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"On 2026-02-04 08:00, Jacopo Mondi wrote:\n> Hi Rui\n>\n> On Wed, Feb 04, 2026 at 07:29:03AM -0500, rui wang wrote:\n>> On 2026-02-04 05:26, Jacopo Mondi wrote:\n>>> Hi Rui\n>>>\n>>> On Tue, Feb 03, 2026 at 02:07:35PM -0500, rui wang wrote:\n>>>> On 2026-02-03 09:08, Jacopo Mondi wrote:\n>>>>> Rui,\n>>>>>\n>>>>> On Sun, Feb 01, 2026 at 02:16:03PM -0500, Rui Wang wrote:\n>>>>>> Register NoiseReductionMode controls based on the tuning data and default\n>>>>>> to the active DPF mode. Remove the static NoiseReductionMode entry from\n>>>>>> the IPA control map now that DPF owns its registration.\n>>>>>>\n>>>>>> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>\n>>>>>> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n>>>>>>\n>>>>>> ---\n>>>>>>      changelog :\n>>>>>>        --No change since v10\n>>>>>>      changelog since v11:\n>>>>>>       - Simple Debug log\n>>>>>> ---\n>>>>>>     src/ipa/rkisp1/algorithms/dpf.cpp | 21 +++++++++++++++++++++\n>>>>>>     src/ipa/rkisp1/algorithms/dpf.h   |  1 +\n>>>>>>     src/ipa/rkisp1/rkisp1.cpp         |  1 -\n>>>>>>     3 files changed, 22 insertions(+), 1 deletion(-)\n>>>>>>\n>>>>>> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n>>>>>> index 9d7fcc1c..303d5cab 100644\n>>>>>> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n>>>>>> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n>>>>>> @@ -52,6 +52,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n>>>>>>     \tif (ret)\n>>>>>>     \t\treturn ret;\n>>>>>>\n>>>>>> +\t/* Register available controls. */\n>>>>>> +\tregisterControls(context);\n>>>>>> +\n>>>>>>     \treturn 0;\n>>>>>>     }\n>>>>>>\n>>>>>> @@ -117,6 +120,24 @@ int Dpf::parseConfig(const YamlObject &tuningData)\n>>>>>>     \treturn 0;\n>>>>>>     }\n>>>>>>\n>>>>>> +void Dpf::registerControls(IPAContext &context)\n>>>>>> +{\n>>>>>> +\t/*\n>>>>>> +\t * Populate the control map with the available noise reduction modes.\n>>>>>> +\t * This allows applications to query and select from the modes defined\n>>>>>> +\t * in the tuning data.\n>>>>>> +\t */\n>>>>>> +\tstd::vector<ControlValue> modes{};\n>>>>>> +\tfor (const auto &mode : noiseReductionModes_) {\n>>>>>> +\t\tmodes.emplace_back(mode.modeValue);\n>>>>>> +\t}\n>>>>> No {}\n>>>>>\n>>>>>> +\t/*\n>>>>>> +\t * Set the default mode to the active mode.\n>>>>>> +\t */\n>>>>> Fits in 1 line\n>>>>>\n>>>>>> +\tcontext.ctrlMap[&controls::draft::NoiseReductionMode] =\n>>> I just noticed it is probably worth moving NoiseReductionMode out of\n>>> draft and make it a core libcamera control\n>>\n>> Yes , I am think moving this draft controls to libcamera controls after\n>> those patch merged\n> mm, ok, I would do it before adding more users but I guess that's not\n> a big deal as long as it happens quickly. Otherwise if we add support\n> in rkisp1 for controls::draft::NoiseReductionMode to\n> later change it to controls::NoiseReductionMode in the next release,\n> it seems an ABI breakage we could easily avoid.\n\nsure , once this patch series merged I will push one PR for \ndraft::control rename\n\nand will check with Kieran regarding to ABI breakage details\n\n>\n>> it and will submit into seperate PR for this ,because it will change RPI's\n>> module as well.\n>>\n>>\n>>\n>>\n>>>>>> +\t\tControlInfo(modes, activeMode_->modeValue);\n>>>>> On v11 I asked the following question, and I'll paste it here with\n>>>>> your reply below, as I haven't replied on v11 (sorry about that).\n>>>>>\n>>>>> -------------------------------------------------------------------------------\n>>>>>> I might have missed why you need a separate function to re-loop over\n>>>>>> modes instead of doing this when parsing modes.\n>>>>>>\n>>>>>> Was the patch I sent not functional ? Do you prefer a different\n>>>>>> approach ? Can you share your design decisions or simply reply to that\n>>>>>> email if you didn't agree with the suggestion explaining why ?\n>>>>> Hello Jacopo,\n>>>>>\n>>>>>\n>>>>> The patch you sharing of add ctrlMap is working ,\n>>>>>\n>>>>> I am thinking all controls can be declare in registerControls , and in\n>>>>> future manual mode\n>>>>>\n>>>>> would explicit many controls will add here. but actually , it will\n>>>>> introduce a looper to push back\n>>>>>\n>>>>> each element from noiseReductionModes_ to control map.and it can also\n>>>>> decouple the ctrlMap\n>>>>>\n>>>>> with tuning config\n>>>>> -------------------------------------------------------------------------------\n>>>>>\n>>>>> I'm not sure I 100% understand why manual mode affects the available\n>>>>> modes map.\n>>>>>\n>>>>> context.ctrlMap[&controls::draft::NoiseReductionMode] should be\n>>>>> populated with the entries from tuning file.\n>>>> Hello ,\n>>>> Sorry I did not express clearly in v11 reply. and made you misunderstood.\n>>>>\n>>>> I am going to exposure all dpf regsiters into controls as manual mode.\n>>>>\n>>>> like : DomainFilter kernel value[6] nll [17] , strength [3]\n>>>>\n>>>> and all those excute into this registerControls\n>>> mmm, ok, however manual controls won't be registered as values for\n>>> controls::draft::NoiseReductionMode but will go through other\n>>> controls, right ?\n>>>\n>>> Also, the manual controls are platform specific ones which expose the\n>>> register interface of the RkISP1. How do we plan to expose them ?\n>>> Through a dedicated set of controls in a newly introduced\n>>> controls::rkisp1:: namespace ?\n>>\n>>           As my expectation ,\"Manual control\" will  belong to\n>>\n>>\n>>           controls::draft::NoiseReductionMode ,\n>>\n>>            From CamShark or any client once controls::draft::Manual\n>>\n>>\n>>           is selected, the all DPF's register values can be edit and\n>>           accepted\n> Do you mean you plan to add a:\n>          controls::draft::NoiseReductionModeManual\n>\n> value in the list of supported modes of\n> controls::draft::NoiseReductionMode ?\n>\n>\n>>\n>>           from controls in manual mode.\n>>\n>>\n>>             and \"manual mode\" would not break other\n>>\n>>\n>>           NoiseReductionMode logic .\n>>\n>>\n>>           all those dpf registers  will be in namespace :\n>>\n>>\n>>           controls::rkisp1\n>>           I had a debug version :\n>>           https://git.ideasonboard.com/ruiwang2002/libcamera_rui_1/pulls/2/files#diff-4420c2393ceee9cd5ff9a751d04baf9ee580c739\n> And once controls::draft::NoiseReductionModeManual is added the user\n> can control the parameters of the denoise engine throuhg the controls\n> you have added in the above pull request ?\n\nYes , all dpf regs value can by write from camshark controls.\n\nit is valuable for manual tuning dpf peformance and also provide some \nspecific\n\ndenoise  profile as user expectation\n\n> That seems like a good plan, but to me NoiseReductionModeManual is\n> logically equivalent to NoiseReductionModeOff.\nyes\n\nNoiseReductionModeManual is equivalent to \"OFF\" \"Mininal\"\n\n>\n> It doesn't require an entry in the tuning file. If the algorithm\n> implementation supports manual mode, the algorithm will add it to the\n> list of supported modes for controls::draft::NoiseReductionMode.\n>\n> So, once manual mode support is added you can add it to the\n> noiseReductionModes_ list in your Dpf::parseConfig() implementation\n> before parsing the yaml file.\n>\n> Do we agree or is your interpreatation different ?\n\nparseConfig would parse maunual init setting from yaml as well like we discussed ever\nnoiseReductionModes_ will be added manual mode's config once parse successfully\n\n\n>\n>>\n>>>>    Rui\n>>>>\n>>>>> What I would expect (copying here from my v8 proposal at\n>>>>> https://patchwork.libcamera.org/patch/25818/#37709)\n>>>>>\n>>>>> With \"minimal\" and \"highquality\" in the tuning\n>>>>> file:\n>>>>>\n>>>>> # cam -c1 --list-controls\n>>>>> Control: [inout] draft::NoiseReductionMode:\n>>>>>      - NoiseReductionModeOff (0) [default]\n>>>>>      - NoiseReductionModeMinimal (3)\n>>>>>      - NoiseReductionModeHighQuality (2)\n>>>>>\n>>>>> With only \"minimal\" in the tuning file\n>>>>>\n>>>>> # cam -c1 --list-controls\n>>>>> Control: [inout] draft::NoiseReductionMode:\n>>>>>      - NoiseReductionModeOff (0) [default]\n>>>>>      - NoiseReductionModeMinimal (3)\n>>>>>\n>>>>> etc etc\n>>>>>\n>>>> I build and run current branch and with imx219.yaml\n>>>>\n>>>> # cam -c1 --list-controls\n>>>> Control: [inout] draft::NoiseReductionMode:\n>>>>     - NoiseReductionModeOff (0)\n>>>>     - NoiseReductionModeMinimal (3)\n>>>>     - NoiseReductionModeHighQuality (2) [default]\n>>>>\n>>>> is your camera as imx219 ?\n>>>>\n>>>>\n>>>>\n>>>>> I expect manual mode to allow users to override the single parameters\n>>>>> that define a \"mode\" not to change the enumeration of available modes.\n>>>>>\n>>>>> Anyway, since I might be missing the overall plan and you seem\n>>>>> convinced this is the way to go let's:\n>>>>>\n>>>>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>>>>>\n>>>>> and we'll eventually rework on top if needed\n>>>>>\n>>>>>\n>>>>>> +}\n>>>>>> +\n>>>>>>     int Dpf::parseSingleConfig(const YamlObject &tuningData,\n>>>>>>     \t\t\t   rkisp1_cif_isp_dpf_config &config,\n>>>>>>     \t\t\t   rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n>>>>>> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n>>>>>> index 11fc88e4..43effcbe 100644\n>>>>>> --- a/src/ipa/rkisp1/algorithms/dpf.h\n>>>>>> +++ b/src/ipa/rkisp1/algorithms/dpf.h\n>>>>>> @@ -37,6 +37,7 @@ private:\n>>>>>>     \t};\n>>>>>>\n>>>>>>     \tint parseConfig(const YamlObject &tuningData);\n>>>>>> +\tvoid registerControls(IPAContext &context);\n>>>>>>     \tint parseSingleConfig(const YamlObject &tuningData,\n>>>>>>     \t\t\t      rkisp1_cif_isp_dpf_config &config,\n>>>>>>     \t\t\t      rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n>>>>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>>>>> index fbcc3910..402ed62c 100644\n>>>>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>>>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>>>>> @@ -120,7 +120,6 @@ const IPAHwSettings ipaHwSettingsV12{\n>>>>>>     /* List of controls handled by the RkISP1 IPA */\n>>>>>>     const ControlInfoMap::Map rkisp1Controls{\n>>>>>>     \t{ &controls::DebugMetadataEnable, ControlInfo(false, true, false) },\n>>>>>> -\t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n>>>>>>     };\n>>>>>>\n>>>>>>     } /* namespace */\n>>>>>> --\n>>>>>> 2.43.0\n>>>>>>","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 DF7F6C31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Feb 2026 13:55:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2A8AE6202E;\n\tWed,  4 Feb 2026 14:55:59 +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 9973961FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Feb 2026 14:55:57 +0100 (CET)","from [192.168.31.114] (unknown [209.216.103.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BB5FD833;\n\tWed,  4 Feb 2026 14:55:14 +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=\"bLIZzLIR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770213315;\n\tbh=3chmc1Jur9UDhDYuT0YXmSNtA/O6l1fbc7qE/YfT2fs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=bLIZzLIREdYS+K2DByUw73Zc8i79O0HGceL6AnZVhnwxmTQyBS12gOJJDbm+K66/R\n\t/nPtqnFdFUep2BLCrwVELju3HmdJz1Rzg+c+yE5QrWotb033FzgQ4nOaeieYtlsWmc\n\ttJ8A92jDBprCASXdc3VImpyqIyc065JNI6Y6UV7E=","Message-ID":"<aa35ba34-8ce5-4f71-b11a-10a98c475bf5@ideasonboard.com>","Date":"Wed, 4 Feb 2026 08:55:44 -0500","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v12 3/7] ipa: rkisp1: algorithms: register noise\n\treduction controls","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tIsaac Scott <isaac.scott@ideasonboard.com>","References":"<20260201191607.2740223-1-rui.wang@ideasonboard.com>\n\t<20260201191607.2740223-4-rui.wang@ideasonboard.com>\n\t<aYH_Lf1ZrZ04hKrp@zed>\n\t<9d69f0f2-ad27-4bc5-8d7e-92fd0c57c0d7@ideasonboard.com>\n\t<aYMdr78wTMammI81@zed>\n\t<3b640776-531c-4423-9ab8-81282bcfee98@ideasonboard.com>\n\t<aYNBFI58UY3EibxR@zed>","Content-Language":"en-US","From":"rui wang <rui.wang@ideasonboard.com>","In-Reply-To":"<aYNBFI58UY3EibxR@zed>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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>"}}]