[{"id":37898,"web_url":"https://patchwork.libcamera.org/comment/37898/","msgid":"","date":"2026-01-23T08:09:15","subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitching","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Rui\n\nOn Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:\n> Implement support for switching between different noise reduction modes.\n> This allows the DPF algorithm to be configured with different parameters\n> based on the requested noise reduction level (e.g., minimal, fast, high\n> quality).\n>\n> Mode configurations are stored in the tuning data as a list of modes,\n> with each mode specifying its 'type' and corresponding DPF parameters.\n> An optional 'ActiveMode' setting allows defining the default mode at\n> startup, defaulting to \"ReductionOff\" if not specified.\n>\n> The Dpf class is refactored to store configurations in a vector and\n> track the current mode using an iterator, which avoids data copying\n> during runtime.\n>\n> Signed-off-by: Rui Wang \n>\n> ---\n> changelog since v5:\n> - Update log verbos from Info to Debug in loadReductionConfig\n> - Update log mode value to string to in loadReductionConfig\n> - improving the return value changed like :\n> if (ret != 0) -> if (ret)\n>\n> Reviewed-by tags from v5 are carried over (no function changes).\n> changelog since v6:\n> - add { controls::draft::NoiseReductionModeOff, \"off\" }, to fix\n> out_of_range issue\n>\n> changelog since v7:\n> - Delete base config parse from parseConfig\n>\n> changelog since v8:\n> - remove config_ strengthConfig_ by replacing activeMode_ iterator\n> to avoiding data copy during config loading\n> - Update kModesMap from std::map\n> std::map for quick search improvement\n> - add ActiveMode as Stefan and Jacopo's review comments\n> - update type : auto -> int for ret value\n> - name change loadRecuctionConfig -> loadConfig\n> - delete parseMode\n>\n> changelog since v9: As Stefan's suggestion\n> - Update dpf reduction mode config structure format\n> from list to dictionary :\n> NoiseReductionMode:\n> NoiseReductionMinimal:\n> ***\n> NoiseReductionZSL:\n> ***\n>\n> changelog since v10:\n> - replace kModeMap with NoiseReductionModeNameValueMap and other\n> accordingly\n> ---\n> src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----\n> src/ipa/rkisp1/algorithms/dpf.h | 12 +++-\n> 2 files changed, 92 insertions(+), 13 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index dd3effa1..78a79fa5 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {\n> LOG_DEFINE_CATEGORY(RkISP1Dpf)\n>\n> Dpf::Dpf()\n> -\t: config_({}), strengthConfig_({})\n> +\t: noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())\n> {\n> }\n>\n> @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n>\n> int Dpf::parseConfig(const YamlObject &tuningData)\n> {\n> -\t/* Parse base config. */\n> -\tint ret = parseSingleConfig(tuningData, config_, strengthConfig_);\n> -\tif (ret)\n> -\t\treturn ret;\n> +\t/* Parse noise reduction modes. */\n> +\tif (!tuningData.contains(\"NoiseReductionModes\")) {\n> +\t\tLOG(RkISP1Dpf, Error) << \"Missing modes in DPF tuning data\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tconst YamlObject &modesObject = tuningData[\"NoiseReductionModes\"];\n> +\tif (!modesObject.isDictionary()) {\n> +\t\tLOG(RkISP1Dpf, Error) << \"NoiseReductionModes must be a dictionary\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tnoiseReductionModes_.clear();\n> +\tfor (const auto &[modeName, modeData] : modesObject.asDict()) {\n> +\t\tauto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);\n> +\t\tif (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> +\t\t\tLOG(RkISP1Dpf, Error) << \"Unknown mode type: \" << modeName;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tModeConfig mode;\n> +\t\tmode.modeValue = it->second;\n> +\t\tint ret = parseSingleConfig(modeData, mode.dpf, mode.strength);\n> +\t\tif (ret) {\n> +\t\t\tLOG(RkISP1Dpf, Error) << \"Failed to parse mode: \" << modeName;\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tnoiseReductionModes_.push_back(mode);\n> +\t}\n> +\n> +\t/*\n> +\t * Parse the optional ActiveMode.\n> +\t * If not present, default to \"NoiseReductionModeOff\".\n> +\t */\n> +\tstd::string activeMode =\n> +\t\ttuningData[\"ActiveMode\"].get().value_or(\"NoiseReductionModeOff\");\n\nLooking at other algorithms this seems to be more appropriate as\n\"activeMode\" ?\n\n> +\tauto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);\n> +\tif (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> +\t\tLOG(RkISP1Dpf, Warning) << \"Invalid ActiveMode: \" << activeMode;\n> +\t\tactiveMode_ = noiseReductionModes_.end();\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tif (!loadConfig(it->second)) {\n> +\t\t/* If the default \"NoiseReductionModeOff\" mode is requested but not configured, disable DPF. */\n\nThe comment fits on 2 lines\n\nI'm not sure I get what you're protecting agains here but I might not\nbe completely awake. Is this beacause NoiseReductionModeOff is not in\nthe noiseReductionModes_ list ?\n\nCan't you add it by default as we don't expect users to configure it\nfrom tuning file ?\n\n\n> +\t\tif (it->second == controls::draft::NoiseReductionModeOff)\n> +\t\t\tactiveMode_ = noiseReductionModes_.end();\n> +\t\telse\n> +\t\t\treturn -EINVAL;\n> +\t}\n>\n> \treturn 0;\n> }\n> @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,\n> \treturn 0;\n> }\n>\n> +bool Dpf::loadConfig(int32_t mode)\n> +{\n> +\tauto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),\n> +\t\t\t [mode](const ModeConfig &m) {\n> +\t\t\t\t return m.modeValue == mode;\n> +\t\t\t });\n> +\tif (it == noiseReductionModes_.end()) {\n> +\t\tLOG(RkISP1Dpf, Warning)\n> +\t\t\t<< \"No DPF config for reduction mode: \" << mode;\n> +\t\treturn false;\n> +\t}\n> +\n> +\tactiveMode_ = it;\n> +\n> +\tLOG(RkISP1Dpf, Debug)\n> +\t\t<< \"DPF mode=Reduction (config loaded)\"\n> +\t\t<< \" mode= \" << mode;\n\nSimply\n\n\tLOG(RkISP1Dpf, Debug)\n\t\t<< \"Use DPF mode \" << mode;\n\nOr something similar ?\n\n> +\n> +\treturn true;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> */\n> @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,\n>\n> \tconst auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> \tif (denoise) {\n> -\t\tLOG(RkISP1Dpf, Debug) << \"Set denoise to \" << *denoise;\n> -\n> \t\tswitch (*denoise) {\n> \t\tcase controls::draft::NoiseReductionModeOff:\n> \t\t\tif (dpf.denoise) {\n> @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,\n> \t\tcase controls::draft::NoiseReductionModeMinimal:\n> \t\tcase controls::draft::NoiseReductionModeHighQuality:\n> \t\tcase controls::draft::NoiseReductionModeFast:\n> -\t\t\tif (!dpf.denoise) {\n> -\t\t\t\tdpf.denoise = true;\n> +\t\tcase controls::draft::NoiseReductionModeZSL:\n> +\t\t\tif (loadConfig(*denoise)) {\n> \t\t\t\tupdate = true;\n> +\t\t\t\tdpf.denoise = true;\n> \t\t\t}\n> \t\t\tbreak;\n> \t\tdefault:\n> @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,\n> \t\t\t\t<< *denoise;\n> \t\t\tbreak;\n> \t\t}\n> +\t\tif (update)\n> +\t\t\tLOG(RkISP1Dpf, Debug) << \"Set denoise to \" << modeName(*denoise);\n> \t}\n>\n> \tframeContext.dpf.denoise = dpf.denoise;\n> @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> \tstrengthConfig.setEnabled(frameContext.dpf.denoise);\n>\n> \tif (frameContext.dpf.denoise) {\n> -\t\t*config = config_;\n> -\t\t*strengthConfig = strengthConfig_;\n> +\t\tconst ModeConfig &modeConfig = *activeMode_;\n> +\n> +\t\t*config = modeConfig.dpf;\n> +\t\t*strengthConfig = modeConfig.strength;\n>\n> \t\tconst auto &awb = context.configuration.awb;\n> \t\tconst auto &lsc = context.configuration.lsc;\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> index 39186c55..11fc88e4 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.h\n> +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> @@ -30,13 +30,21 @@ public:\n> \t\t RkISP1Params *params) override;\n>\n> private:\n> +\tstruct ModeConfig {\n> +\t\tint32_t modeValue;\n> +\t\trkisp1_cif_isp_dpf_config dpf;\n> +\t\trkisp1_cif_isp_dpf_strength_config strength;\n> +\t};\n> +\n> \tint parseConfig(const YamlObject &tuningData);\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>\n> -\tstruct rkisp1_cif_isp_dpf_config config_;\n> -\tstruct rkisp1_cif_isp_dpf_strength_config strengthConfig_;\n> +\tbool loadConfig(int32_t mode);\n> +\n> +\tstd::vector noiseReductionModes_;\n> +\tstd::vector::const_iterator activeMode_;\n> };\n>\n> } /* namespace ipa::rkisp1::algorithms */\n> --\n> 2.43.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 22202C3220\n\tfor ;\n\tFri, 23 Jan 2026 08:09:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F1F961FC4;\n\tFri, 23 Jan 2026 09:09:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0E7A61FBB\n\tfor ;\n\tFri, 23 Jan 2026 09:09:18 +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 6032D8FA;\n\tFri, 23 Jan 2026 09:08:45 +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=\"bYXjZfkb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769155725;\n\tbh=2KuYt4uxGFFwrZ9iknvccVCm7quFwg/uYcoA38Ay5U4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bYXjZfkbtXewEnPQkhdsPhNC975OEX6yQnLFV7qvEhkOKcH6WO0ONTgpdmHogcnCA\n\ty6XGRuaiZcj1BkWDOUbI8T6VtEIhPqUkl3X3V3+WXWpWqRlvIbIdHuAqbhFN1/+Kem\n\tvVaHC01Avb+CWyFKlQOnRmVFu9vihpNqBUPV7UhA=","Date":"Fri, 23 Jan 2026 09:09:15 +0100","From":"Jacopo Mondi ","To":"Rui Wang ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitching","Message-ID":"","References":"<20260122234709.2061370-1-rui.wang@ideasonboard.com>\n\t<20260122234709.2061370-3-rui.wang@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260122234709.2061370-3-rui.wang@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37931,"web_url":"https://patchwork.libcamera.org/comment/37931/","msgid":"<176928787561.2934053.7215668025551189062@rui-Precision-7560.local>","date":"2026-01-24T20:51:15","subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitchings","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2026-01-23 03:09:15)\n> Hi Rui\n> \n> On Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:\n> > Implement support for switching between different noise reduction modes.\n> > This allows the DPF algorithm to be configured with different parameters\n> > based on the requested noise reduction level (e.g., minimal, fast, high\n> > quality).\n> >\n> > Mode configurations are stored in the tuning data as a list of modes,\n> > with each mode specifying its 'type' and corresponding DPF parameters.\n> > An optional 'ActiveMode' setting allows defining the default mode at\n> > startup, defaulting to \"ReductionOff\" if not specified.\n> >\n> > The Dpf class is refactored to store configurations in a vector and\n> > track the current mode using an iterator, which avoids data copying\n> > during runtime.\n> >\n> > Signed-off-by: Rui Wang \n> >\n> > ---\n> > changelog since v5:\n> > - Update log verbos from Info to Debug in loadReductionConfig\n> > - Update log mode value to string to in loadReductionConfig\n> > - improving the return value changed like :\n> > if (ret != 0) -> if (ret)\n> >\n> > Reviewed-by tags from v5 are carried over (no function changes).\n> > changelog since v6:\n> > - add { controls::draft::NoiseReductionModeOff, \"off\" }, to fix\n> > out_of_range issue\n> >\n> > changelog since v7:\n> > - Delete base config parse from parseConfig\n> >\n> > changelog since v8:\n> > - remove config_ strengthConfig_ by replacing activeMode_ iterator\n> > to avoiding data copy during config loading\n> > - Update kModesMap from std::map\n> > std::map for quick search improvement\n> > - add ActiveMode as Stefan and Jacopo's review comments\n> > - update type : auto -> int for ret value\n> > - name change loadRecuctionConfig -> loadConfig\n> > - delete parseMode\n> >\n> > changelog since v9: As Stefan's suggestion\n> > - Update dpf reduction mode config structure format\n> > from list to dictionary :\n> > NoiseReductionMode:\n> > NoiseReductionMinimal:\n> > ***\n> > NoiseReductionZSL:\n> > ***\n> >\n> > changelog since v10:\n> > - replace kModeMap with NoiseReductionModeNameValueMap and other\n> > accordingly\n> > ---\n> > src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----\n> > src/ipa/rkisp1/algorithms/dpf.h | 12 +++-\n> > 2 files changed, 92 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > index dd3effa1..78a79fa5 100644\n> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {\n> > LOG_DEFINE_CATEGORY(RkISP1Dpf)\n> >\n> > Dpf::Dpf()\n> > - : config_({}), strengthConfig_({})\n> > + : noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())\n> > {\n> > }\n> >\n> > @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> >\n> > int Dpf::parseConfig(const YamlObject &tuningData)\n> > {\n> > - /* Parse base config. */\n> > - int ret = parseSingleConfig(tuningData, config_, strengthConfig_);\n> > - if (ret)\n> > - return ret;\n> > + /* Parse noise reduction modes. */\n> > + if (!tuningData.contains(\"NoiseReductionModes\")) {\n> > + LOG(RkISP1Dpf, Error) << \"Missing modes in DPF tuning data\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + const YamlObject &modesObject = tuningData[\"NoiseReductionModes\"];\n> > + if (!modesObject.isDictionary()) {\n> > + LOG(RkISP1Dpf, Error) << \"NoiseReductionModes must be a dictionary\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + noiseReductionModes_.clear();\n> > + for (const auto &[modeName, modeData] : modesObject.asDict()) {\n> > + auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);\n> > + if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> > + LOG(RkISP1Dpf, Error) << \"Unknown mode type: \" << modeName;\n> > + return -EINVAL;\n> > + }\n> > +\n> > + ModeConfig mode;\n> > + mode.modeValue = it->second;\n> > + int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);\n> > + if (ret) {\n> > + LOG(RkISP1Dpf, Error) << \"Failed to parse mode: \" << modeName;\n> > + return ret;\n> > + }\n> > +\n> > + noiseReductionModes_.push_back(mode);\n> > + }\n> > +\n> > + /*\n> > + * Parse the optional ActiveMode.\n> > + * If not present, default to \"NoiseReductionModeOff\".\n> > + */\n> > + std::string activeMode =\n> > + tuningData[\"ActiveMode\"].get().value_or(\"NoiseReductionModeOff\");\n> \n> Looking at other algorithms this seems to be more appropriate as\n> \"activeMode\" ?\n\n I will check other module to check if upper \"A\" in tuning config.\n Then update in v2\n> \n> > + auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);\n> > + if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> > + LOG(RkISP1Dpf, Warning) << \"Invalid ActiveMode: \" << activeMode;\n> > + activeMode_ = noiseReductionModes_.end();\n> > + return 0;\n> > + }\n> > +\n> > + if (!loadConfig(it->second)) {\n> > + /* If the default \"NoiseReductionModeOff\" mode is requested but not configured, disable DPF. */\n> \n> The comment fits on 2 lines\n> \n> I'm not sure I get what you're protecting agains here but I might not\n> be completely awake. Is this beacause NoiseReductionModeOff is not in\n> the noiseReductionModes_ list ?\n> \n yes, NoiseRectionValueMap contains \"OFF\" , and noiseRectionModes_ has only\n valid config for each mode.\n I can improve the logic ：\n if(\"OFF\") {\n\t activeMode_ = end\n\t retun 0;\n }\n\n> Can't you add it by default as we don't expect users to configure it\n> from tuning file ?\n> \n Hello Jacopo,\n As both DPF and Filter denoise share same control NoiseReductionMode\n I am afraid the tuning config for them has some mode mismatch.\n and currently I prefer \"OFF\" as default without any configuration.\n from code implementation it is much straitforward, and once \"AUTO\"\n denoise mode implemented, I will swith to AUTO as default without any \n declaration in tuning file.\n Do you think this strategy acceptable ?\n\n\n \n\n> \n> > + if (it->second == controls::draft::NoiseReductionModeOff)\n> > + activeMode_ = noiseReductionModes_.end();\n> > + else\n> > + return -EINVAL;\n> > + }\n> >\n> > return 0;\n> > }\n> > @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,\n> > return 0;\n> > }\n> >\n> > +bool Dpf::loadConfig(int32_t mode)\n> > +{\n> > + auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),\n> > + [mode](const ModeConfig &m) {\n> > + return m.modeValue == mode;\n> > + });\n> > + if (it == noiseReductionModes_.end()) {\n> > + LOG(RkISP1Dpf, Warning)\n> > + << \"No DPF config for reduction mode: \" << mode;\n> > + return false;\n> > + }\n> > +\n> > + activeMode_ = it;\n> > +\n> > + LOG(RkISP1Dpf, Debug)\n> > + << \"DPF mode=Reduction (config loaded)\"\n> > + << \" mode= \" << mode;\n> \n> Simply\n> \n> LOG(RkISP1Dpf, Debug)\n> << \"Use DPF mode \" << mode;\n> \n> Or something similar ?\n> \n> > +\n> > + return true;\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > */\n> > @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,\n> >\n> > const auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> > if (denoise) {\n> > - LOG(RkISP1Dpf, Debug) << \"Set denoise to \" << *denoise;\n> > -\n> > switch (*denoise) {\n> > case controls::draft::NoiseReductionModeOff:\n> > if (dpf.denoise) {\n> > @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,\n> > case controls::draft::NoiseReductionModeMinimal:\n> > case controls::draft::NoiseReductionModeHighQuality:\n> > case controls::draft::NoiseReductionModeFast:\n> > - if (!dpf.denoise) {\n> > - dpf.denoise = true;\n> > + case controls::draft::NoiseReductionModeZSL:\n> > + if (loadConfig(*denoise)) {\n> > update = true;\n> > + dpf.denoise = true;\n> > }\n> > break;\n> > default:\n> > @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,\n> > << *denoise;\n> > break;\n> > }\n> > + if (update)\n> > + LOG(RkISP1Dpf, Debug) << \"Set denoise to \" << modeName(*denoise);\n> > }\n> >\n> > frameContext.dpf.denoise = dpf.denoise;\n> > @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> > strengthConfig.setEnabled(frameContext.dpf.denoise);\n> >\n> > if (frameContext.dpf.denoise) {\n> > - *config = config_;\n> > - *strengthConfig = strengthConfig_;\n> > + const ModeConfig &modeConfig = *activeMode_;\n> > +\n> > + *config = modeConfig.dpf;\n> > + *strengthConfig = modeConfig.strength;\n> >\n> > const auto &awb = context.configuration.awb;\n> > const auto &lsc = context.configuration.lsc;\n> > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> > index 39186c55..11fc88e4 100644\n> > --- a/src/ipa/rkisp1/algorithms/dpf.h\n> > +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> > @@ -30,13 +30,21 @@ public:\n> > RkISP1Params *params) override;\n> >\n> > private:\n> > + struct ModeConfig {\n> > + int32_t modeValue;\n> > + rkisp1_cif_isp_dpf_config dpf;\n> > + rkisp1_cif_isp_dpf_strength_config strength;\n> > + };\n> > +\n> > int parseConfig(const YamlObject &tuningData);\n> > int parseSingleConfig(const YamlObject &tuningData,\n> > rkisp1_cif_isp_dpf_config &config,\n> > rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n> >\n> > - struct rkisp1_cif_isp_dpf_config config_;\n> > - struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;\n> > + bool loadConfig(int32_t mode);\n> > +\n> > + std::vector noiseReductionModes_;\n> > + std::vector::const_iterator activeMode_;\n> > };\n> >\n> > } /* namespace ipa::rkisp1::algorithms */\n> > --\n> > 2.43.0\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1EB53C3220\n\tfor ;\n\tSat, 24 Jan 2026 20:51:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2518361FC4;\n\tSat, 24 Jan 2026 21:51:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C71AD61FA0\n\tfor ;\n\tSat, 24 Jan 2026 21:51:28 +0100 (CET)","from pyrite.rasen.tech (unknown [209.216.103.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3AA6CA98;\n\tSat, 24 Jan 2026 21:50:54 +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=\"tooVxpYM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769287854;\n\tbh=0TzIjD6iOmzulzALUWxsNAdLXwmWDEu3FsABkN+/ixA=;\n\th=In-Reply-To:References:From:Cc:To:Subject:Date:From;\n\tb=tooVxpYMTrhi4AhgIg/JizyClkSyzLcchCEBNEl5oaYEAVtOvmNPZmrEFw24BKWWR\n\tTPjrs6bS71GYVPT0qAyUXBpG80jBpslpHQicFkfbs/LtFXhrt7w7BSNNY2bQjChtkU\n\tcklsOXlEu0Cjr1RiU+CNsAOR+c+cPfo9GSKqCM/s=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20260122234709.2061370-1-rui.wang@ideasonboard.com>\n\t<20260122234709.2061370-3-rui.wang@ideasonboard.com>\n\t","From":"Rui Wang ","Cc":"libcamera-devel@lists.libcamera.org","To":"Jacopo Mondi ","Subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitchings","Date":"Sat, 24 Jan 2026 15:51:15 -0500","Message-ID":"<176928787561.2934053.7215668025551189062@rui-Precision-7560.local>","User-Agent":"alot/0.12","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37932,"web_url":"https://patchwork.libcamera.org/comment/37932/","msgid":"","date":"2026-01-26T08:04:14","subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitchings","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Rui\n\nOn Sat, Jan 24, 2026 at 03:51:15PM -0500, Rui Wang wrote:\n> Quoting Jacopo Mondi (2026-01-23 03:09:15)\n> > Hi Rui\n> >\n> > On Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:\n> > > Implement support for switching between different noise reduction modes.\n> > > This allows the DPF algorithm to be configured with different parameters\n> > > based on the requested noise reduction level (e.g., minimal, fast, high\n> > > quality).\n> > >\n> > > Mode configurations are stored in the tuning data as a list of modes,\n> > > with each mode specifying its 'type' and corresponding DPF parameters.\n> > > An optional 'ActiveMode' setting allows defining the default mode at\n> > > startup, defaulting to \"ReductionOff\" if not specified.\n> > >\n> > > The Dpf class is refactored to store configurations in a vector and\n> > > track the current mode using an iterator, which avoids data copying\n> > > during runtime.\n> > >\n> > > Signed-off-by: Rui Wang \n> > >\n> > > ---\n> > > changelog since v5:\n> > > - Update log verbos from Info to Debug in loadReductionConfig\n> > > - Update log mode value to string to in loadReductionConfig\n> > > - improving the return value changed like :\n> > > if (ret != 0) -> if (ret)\n> > >\n> > > Reviewed-by tags from v5 are carried over (no function changes).\n> > > changelog since v6:\n> > > - add { controls::draft::NoiseReductionModeOff, \"off\" }, to fix\n> > > out_of_range issue\n> > >\n> > > changelog since v7:\n> > > - Delete base config parse from parseConfig\n> > >\n> > > changelog since v8:\n> > > - remove config_ strengthConfig_ by replacing activeMode_ iterator\n> > > to avoiding data copy during config loading\n> > > - Update kModesMap from std::map\n> > > std::map for quick search improvement\n> > > - add ActiveMode as Stefan and Jacopo's review comments\n> > > - update type : auto -> int for ret value\n> > > - name change loadRecuctionConfig -> loadConfig\n> > > - delete parseMode\n> > >\n> > > changelog since v9: As Stefan's suggestion\n> > > - Update dpf reduction mode config structure format\n> > > from list to dictionary :\n> > > NoiseReductionMode:\n> > > NoiseReductionMinimal:\n> > > ***\n> > > NoiseReductionZSL:\n> > > ***\n> > >\n> > > changelog since v10:\n> > > - replace kModeMap with NoiseReductionModeNameValueMap and other\n> > > accordingly\n> > > ---\n> > > src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----\n> > > src/ipa/rkisp1/algorithms/dpf.h | 12 +++-\n> > > 2 files changed, 92 insertions(+), 13 deletions(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > index dd3effa1..78a79fa5 100644\n> > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {\n> > > LOG_DEFINE_CATEGORY(RkISP1Dpf)\n> > >\n> > > Dpf::Dpf()\n> > > - : config_({}), strengthConfig_({})\n> > > + : noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())\n> > > {\n> > > }\n> > >\n> > > @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> > >\n> > > int Dpf::parseConfig(const YamlObject &tuningData)\n> > > {\n> > > - /* Parse base config. */\n> > > - int ret = parseSingleConfig(tuningData, config_, strengthConfig_);\n> > > - if (ret)\n> > > - return ret;\n> > > + /* Parse noise reduction modes. */\n> > > + if (!tuningData.contains(\"NoiseReductionModes\")) {\n> > > + LOG(RkISP1Dpf, Error) << \"Missing modes in DPF tuning data\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + const YamlObject &modesObject = tuningData[\"NoiseReductionModes\"];\n> > > + if (!modesObject.isDictionary()) {\n> > > + LOG(RkISP1Dpf, Error) << \"NoiseReductionModes must be a dictionary\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + noiseReductionModes_.clear();\n> > > + for (const auto &[modeName, modeData] : modesObject.asDict()) {\n> > > + auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);\n> > > + if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> > > + LOG(RkISP1Dpf, Error) << \"Unknown mode type: \" << modeName;\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + ModeConfig mode;\n> > > + mode.modeValue = it->second;\n> > > + int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);\n> > > + if (ret) {\n> > > + LOG(RkISP1Dpf, Error) << \"Failed to parse mode: \" << modeName;\n> > > + return ret;\n> > > + }\n> > > +\n> > > + noiseReductionModes_.push_back(mode);\n> > > + }\n> > > +\n> > > + /*\n> > > + * Parse the optional ActiveMode.\n> > > + * If not present, default to \"NoiseReductionModeOff\".\n> > > + */\n> > > + std::string activeMode =\n> > > + tuningData[\"ActiveMode\"].get().value_or(\"NoiseReductionModeOff\");\n> >\n> > Looking at other algorithms this seems to be more appropriate as\n> > \"activeMode\" ?\n>\n> I will check other module to check if upper \"A\" in tuning config.\n> Then update in v2\n> >\n> > > + auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);\n> > > + if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> > > + LOG(RkISP1Dpf, Warning) << \"Invalid ActiveMode: \" << activeMode;\n> > > + activeMode_ = noiseReductionModes_.end();\n> > > + return 0;\n> > > + }\n> > > +\n> > > + if (!loadConfig(it->second)) {\n> > > + /* If the default \"NoiseReductionModeOff\" mode is requested but not configured, disable DPF. */\n> >\n> > The comment fits on 2 lines\n> >\n> > I'm not sure I get what you're protecting agains here but I might not\n> > be completely awake. Is this beacause NoiseReductionModeOff is not in\n> > the noiseReductionModes_ list ?\n> >\n> yes, NoiseRectionValueMap contains \"OFF\" , and noiseRectionModes_ has only\n> valid config for each mode.\n> I can improve the logic ：\n> if(\"OFF\") {\n> \t activeMode_ = end\n> \t retun 0;\n> }\n>\n> > Can't you add it by default as we don't expect users to configure it\n> > from tuning file ?\n> >\n> Hello Jacopo,\n> As both DPF and Filter denoise share same control NoiseReductionMode\n> I am afraid the tuning config for them has some mode mismatch.\n> and currently I prefer \"OFF\" as default without any configuration.\n> from code implementation it is much straitforward, and once \"AUTO\"\n> denoise mode implemented, I will swith to AUTO as default without any\n> declaration in tuning file.\n> Do you think this strategy acceptable ?\n\nI'm sorry if I can't my messages get through. I'll work on my English,\nbut I meant to suggest to add\n\n noiseReductionModes_.push_back(NoiseReductionModeOff);\n\nbefore the for() loop\n\n\n>\n>\n>\n>\n> >\n> > > + if (it->second == controls::draft::NoiseReductionModeOff)\n> > > + activeMode_ = noiseReductionModes_.end();\n> > > + else\n> > > + return -EINVAL;\n> > > + }\n> > >\n> > > return 0;\n> > > }\n> > > @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,\n> > > return 0;\n> > > }\n> > >\n> > > +bool Dpf::loadConfig(int32_t mode)\n> > > +{\n> > > + auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),\n> > > + [mode](const ModeConfig &m) {\n> > > + return m.modeValue == mode;\n> > > + });\n> > > + if (it == noiseReductionModes_.end()) {\n> > > + LOG(RkISP1Dpf, Warning)\n> > > + << \"No DPF config for reduction mode: \" << mode;\n> > > + return false;\n> > > + }\n> > > +\n> > > + activeMode_ = it;\n> > > +\n> > > + LOG(RkISP1Dpf, Debug)\n> > > + << \"DPF mode=Reduction (config loaded)\"\n> > > + << \" mode= \" << mode;\n> >\n> > Simply\n> >\n> > LOG(RkISP1Dpf, Debug)\n> > << \"Use DPF mode \" << mode;\n> >\n> > Or something similar ?\n> >\n> > > +\n> > > + return true;\n> > > +}\n> > > +\n> > > /**\n> > > * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > > */\n> > > @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,\n> > >\n> > > const auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> > > if (denoise) {\n> > > - LOG(RkISP1Dpf, Debug) << \"Set denoise to \" << *denoise;\n> > > -\n> > > switch (*denoise) {\n> > > case controls::draft::NoiseReductionModeOff:\n> > > if (dpf.denoise) {\n> > > @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,\n> > > case controls::draft::NoiseReductionModeMinimal:\n> > > case controls::draft::NoiseReductionModeHighQuality:\n> > > case controls::draft::NoiseReductionModeFast:\n> > > - if (!dpf.denoise) {\n> > > - dpf.denoise = true;\n> > > + case controls::draft::NoiseReductionModeZSL:\n> > > + if (loadConfig(*denoise)) {\n> > > update = true;\n> > > + dpf.denoise = true;\n> > > }\n> > > break;\n> > > default:\n> > > @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,\n> > > << *denoise;\n> > > break;\n> > > }\n> > > + if (update)\n> > > + LOG(RkISP1Dpf, Debug) << \"Set denoise to \" << modeName(*denoise);\n> > > }\n> > >\n> > > frameContext.dpf.denoise = dpf.denoise;\n> > > @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> > > strengthConfig.setEnabled(frameContext.dpf.denoise);\n> > >\n> > > if (frameContext.dpf.denoise) {\n> > > - *config = config_;\n> > > - *strengthConfig = strengthConfig_;\n> > > + const ModeConfig &modeConfig = *activeMode_;\n> > > +\n> > > + *config = modeConfig.dpf;\n> > > + *strengthConfig = modeConfig.strength;\n> > >\n> > > const auto &awb = context.configuration.awb;\n> > > const auto &lsc = context.configuration.lsc;\n> > > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> > > index 39186c55..11fc88e4 100644\n> > > --- a/src/ipa/rkisp1/algorithms/dpf.h\n> > > +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> > > @@ -30,13 +30,21 @@ public:\n> > > RkISP1Params *params) override;\n> > >\n> > > private:\n> > > + struct ModeConfig {\n> > > + int32_t modeValue;\n> > > + rkisp1_cif_isp_dpf_config dpf;\n> > > + rkisp1_cif_isp_dpf_strength_config strength;\n> > > + };\n> > > +\n> > > int parseConfig(const YamlObject &tuningData);\n> > > int parseSingleConfig(const YamlObject &tuningData,\n> > > rkisp1_cif_isp_dpf_config &config,\n> > > rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n> > >\n> > > - struct rkisp1_cif_isp_dpf_config config_;\n> > > - struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;\n> > > + bool loadConfig(int32_t mode);\n> > > +\n> > > + std::vector noiseReductionModes_;\n> > > + std::vector::const_iterator activeMode_;\n> > > };\n> > >\n> > > } /* namespace ipa::rkisp1::algorithms */\n> > > --\n> > > 2.43.0\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BE6C9C3220\n\tfor ;\n\tMon, 26 Jan 2026 08:04:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BEE4E61FC9;\n\tMon, 26 Jan 2026 09:04:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E866261A35\n\tfor ;\n\tMon, 26 Jan 2026 09:04:17 +0100 (CET)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 61856557;\n\tMon, 26 Jan 2026 09:03: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=\"u6wug6/E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769414622;\n\tbh=yZfVX9txZsDy6+u8Xz03R0eoa83rV+cyzC0uuCO26p0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u6wug6/E0dLOq72KlJ1LhFPo/hmDWWlP/8jhjrR/+6Nd3pDQL1CTAYyCE8vuwcQFb\n\tY/V+IiTPuixrjBDbpaepTvlIC3meGFZqIPJZW8XrrfKf0WXHePMXt76CYjxCt61N3J\n\tGDREwQPYtcr/Yj5xdR2y6+DrwrG86QaEjxhCA/14=","Date":"Mon, 26 Jan 2026 09:04:14 +0100","From":"Jacopo Mondi ","To":"Rui Wang ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitchings","Message-ID":"","References":"<20260122234709.2061370-1-rui.wang@ideasonboard.com>\n\t<20260122234709.2061370-3-rui.wang@ideasonboard.com>\n\t\n\t<176928787561.2934053.7215668025551189062@rui-Precision-7560.local>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<176928787561.2934053.7215668025551189062@rui-Precision-7560.local>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":38041,"web_url":"https://patchwork.libcamera.org/comment/38041/","msgid":"<176979504177.1710132.11122717513746867379@rui-Precision-7560.local>","date":"2026-01-30T17:44:01","subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitchings","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2026-01-26 03:04:14)\n> Hi Rui\n> \n> On Sat, Jan 24, 2026 at 03:51:15PM -0500, Rui Wang wrote:\n> > Quoting Jacopo Mondi (2026-01-23 03:09:15)\n> > > Hi Rui\n> > >\n> > > On Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:\n> > > > Implement support for switching between different noise reduction modes.\n> > > > This allows the DPF algorithm to be configured with different parameters\n> > > > based on the requested noise reduction level (e.g., minimal, fast, high\n> > > > quality).\n> > > >\n> > > > Mode configurations are stored in the tuning data as a list of modes,\n> > > > with each mode specifying its 'type' and corresponding DPF parameters.\n> > > > An optional 'ActiveMode' setting allows defining the default mode at\n> > > > startup, defaulting to \"ReductionOff\" if not specified.\n> > > >\n> > > > The Dpf class is refactored to store configurations in a vector and\n> > > > track the current mode using an iterator, which avoids data copying\n> > > > during runtime.\n> > > >\n> > > > Signed-off-by: Rui Wang \n> > > >\n> > > > ---\n> > > > changelog since v5:\n> > > > - Update log verbos from Info to Debug in loadReductionConfig\n> > > > - Update log mode value to string to in loadReductionConfig\n> > > > - improving the return value changed like :\n> > > > if (ret != 0) -> if (ret)\n> > > >\n> > > > Reviewed-by tags from v5 are carried over (no function changes).\n> > > > changelog since v6:\n> > > > - add { controls::draft::NoiseReductionModeOff, \"off\" }, to fix\n> > > > out_of_range issue\n> > > >\n> > > > changelog since v7:\n> > > > - Delete base config parse from parseConfig\n> > > >\n> > > > changelog since v8:\n> > > > - remove config_ strengthConfig_ by replacing activeMode_ iterator\n> > > > to avoiding data copy during config loading\n> > > > - Update kModesMap from std::map\n> > > > std::map for quick search improvement\n> > > > - add ActiveMode as Stefan and Jacopo's review comments\n> > > > - update type : auto -> int for ret value\n> > > > - name change loadRecuctionConfig -> loadConfig\n> > > > - delete parseMode\n> > > >\n> > > > changelog since v9: As Stefan's suggestion\n> > > > - Update dpf reduction mode config structure format\n> > > > from list to dictionary :\n> > > > NoiseReductionMode:\n> > > > NoiseReductionMinimal:\n> > > > ***\n> > > > NoiseReductionZSL:\n> > > > ***\n> > > >\n> > > > changelog since v10:\n> > > > - replace kModeMap with NoiseReductionModeNameValueMap and other\n> > > > accordingly\n> > > > ---\n> > > > src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----\n> > > > src/ipa/rkisp1/algorithms/dpf.h | 12 +++-\n> > > > 2 files changed, 92 insertions(+), 13 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > > index dd3effa1..78a79fa5 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> > > > @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {\n> > > > LOG_DEFINE_CATEGORY(RkISP1Dpf)\n> > > >\n> > > > Dpf::Dpf()\n> > > > - : config_({}), strengthConfig_({})\n> > > > + : noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())\n> > > > {\n> > > > }\n> > > >\n> > > > @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> > > >\n> > > > int Dpf::parseConfig(const YamlObject &tuningData)\n> > > > {\n> > > > - /* Parse base config. */\n> > > > - int ret = parseSingleConfig(tuningData, config_, strengthConfig_);\n> > > > - if (ret)\n> > > > - return ret;\n> > > > + /* Parse noise reduction modes. */\n> > > > + if (!tuningData.contains(\"NoiseReductionModes\")) {\n> > > > + LOG(RkISP1Dpf, Error) << \"Missing modes in DPF tuning data\";\n> > > > + return -EINVAL;\n> > > > + }\n> > > > +\n> > > > + const YamlObject &modesObject = tuningData[\"NoiseReductionModes\"];\n> > > > + if (!modesObject.isDictionary()) {\n> > > > + LOG(RkISP1Dpf, Error) << \"NoiseReductionModes must be a dictionary\";\n> > > > + return -EINVAL;\n> > > > + }\n> > > > +\n> > > > + noiseReductionModes_.clear();\n> > > > + for (const auto &[modeName, modeData] : modesObject.asDict()) {\n> > > > + auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);\n> > > > + if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> > > > + LOG(RkISP1Dpf, Error) << \"Unknown mode type: \" << modeName;\n> > > > + return -EINVAL;\n> > > > + }\n> > > > +\n> > > > + ModeConfig mode;\n> > > > + mode.modeValue = it->second;\n> > > > + int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);\n> > > > + if (ret) {\n> > > > + LOG(RkISP1Dpf, Error) << \"Failed to parse mode: \" << modeName;\n> > > > + return ret;\n> > > > + }\n> > > > +\n> > > > + noiseReductionModes_.push_back(mode);\n> > > > + }\n> > > > +\n> > > > + /*\n> > > > + * Parse the optional ActiveMode.\n> > > > + * If not present, default to \"NoiseReductionModeOff\".\n> > > > + */\n> > > > + std::string activeMode =\n> > > > + tuningData[\"ActiveMode\"].get().value_or(\"NoiseReductionModeOff\");\n> > >\n> > > Looking at other algorithms this seems to be more appropriate as\n> > > \"activeMode\" ?\n> >\n> > I will check other module to check if upper \"A\" in tuning config.\n> > Then update in v2\n> > >\n> > > > + auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);\n> > > > + if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {\n> > > > + LOG(RkISP1Dpf, Warning) << \"Invalid ActiveMode: \" << activeMode;\n> > > > + activeMode_ = noiseReductionModes_.end();\n> > > > + return 0;\n> > > > + }\n> > > > +\n> > > > + if (!loadConfig(it->second)) {\n> > > > + /* If the default \"NoiseReductionModeOff\" mode is requested but not configured, disable DPF. */\n> > >\n> > > The comment fits on 2 lines\n> > >\n> > > I'm not sure I get what you're protecting agains here but I might not\n> > > be completely awake. Is this beacause NoiseReductionModeOff is not in\n> > > the noiseReductionModes_ list ?\n> > >\n> > yes, NoiseRectionValueMap contains \"OFF\" , and noiseRectionModes_ has only\n> > valid config for each mode.\n> > I can improve the logic ：\n> > if(\"OFF\") {\n> > activeMode_ = end\n> > retun 0;\n> > }\n> >\n> > > Can't you add it by default as we don't expect users to configure it\n> > > from tuning file ?\n> > >\n> > Hello Jacopo,\n> > As both DPF and Filter denoise share same control NoiseReductionMode\n> > I am afraid the tuning config for them has some mode mismatch.\n> > and currently I prefer \"OFF\" as default without any configuration.\n> > from code implementation it is much straitforward, and once \"AUTO\"\n> > denoise mode implemented, I will swith to AUTO as default without any\n> > declaration in tuning file.\n> > Do you think this strategy acceptable ?\n> \n> I'm sorry if I can't my messages get through. I'll work on my English,\n> but I meant to suggest to add\n> \n> noiseReductionModes_.push_back(NoiseReductionModeOff);\n> \n> before the for() loop\n>\nyes , that's suggestion to add \"Off\" into the map , and it will reduce some exception check when traversing elements\n> \n> >\n> >\n> >\n> >\n> > >\n> > > > + if (it->second == controls::draft::NoiseReductionModeOff)\n> > > > + activeMode_ = noiseReductionModes_.end();\n> > > > + else\n> > > > + return -EINVAL;\n> > > > + }\n> > > >\n> > > > return 0;\n> > > > }\n> > > > @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,\n> > > > return 0;\n> > > > }\n> > > >\n> > > > +bool Dpf::loadConfig(int32_t mode)\n> > > > +{\n> > > > + auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),\n> > > > + [mode](const ModeConfig &m) {\n> > > > + return m.modeValue == mode;\n> > > > + });\n> > > > + if (it == noiseReductionModes_.end()) {\n> > > > + LOG(RkISP1Dpf, Warning)\n> > > > + << \"No DPF config for reduction mode: \" << mode;\n> > > > + return false;\n> > > > + }\n> > > > +\n> > > > + activeMode_ = it;\n> > > > +\n> > > > + LOG(RkISP1Dpf, Debug)\n> > > > + << \"DPF mode=Reduction (config loaded)\"\n> > > > + << \" mode= \" << mode;\n> > >\n> > > Simply\n> > >\n> > > LOG(RkISP1Dpf, Debug)\n> > > << \"Use DPF mode \" << mode;\n> > >\n> > > Or something similar ?\n> > >\n> > > > +\n> > > > + return true;\n> > > > +}\n> > > > +\n> > > > /**\n> > > > * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > > > */\n> > > > @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,\n> > > >\n> > > > const auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> > > > if (denoise) {\n> > > > - LOG(RkISP1Dpf, Debug) << \"Set denoise to \" << *denoise;\n> > > > -\n> > > > switch (*denoise) {\n> > > > case controls::draft::NoiseReductionModeOff:\n> > > > if (dpf.denoise) {\n> > > > @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,\n> > > > case controls::draft::NoiseReductionModeMinimal:\n> > > > case controls::draft::NoiseReductionModeHighQuality:\n> > > > case controls::draft::NoiseReductionModeFast:\n> > > > - if (!dpf.denoise) {\n> > > > - dpf.denoise = true;\n> > > > + case controls::draft::NoiseReductionModeZSL:\n> > > > + if (loadConfig(*denoise)) {\n> > > > update = true;\n> > > > + dpf.denoise = true;\n> > > > }\n> > > > break;\n> > > > default:\n> > > > @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,\n> > > > << *denoise;\n> > > > break;\n> > > > }\n> > > > + if (update)\n> > > > + LOG(RkISP1Dpf, Debug) << \"Set denoise to \" << modeName(*denoise);\n> > > > }\n> > > >\n> > > > frameContext.dpf.denoise = dpf.denoise;\n> > > > @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> > > > strengthConfig.setEnabled(frameContext.dpf.denoise);\n> > > >\n> > > > if (frameContext.dpf.denoise) {\n> > > > - *config = config_;\n> > > > - *strengthConfig = strengthConfig_;\n> > > > + const ModeConfig &modeConfig = *activeMode_;\n> > > > +\n> > > > + *config = modeConfig.dpf;\n> > > > + *strengthConfig = modeConfig.strength;\n> > > >\n> > > > const auto &awb = context.configuration.awb;\n> > > > const auto &lsc = context.configuration.lsc;\n> > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> > > > index 39186c55..11fc88e4 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/dpf.h\n> > > > +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> > > > @@ -30,13 +30,21 @@ public:\n> > > > RkISP1Params *params) override;\n> > > >\n> > > > private:\n> > > > + struct ModeConfig {\n> > > > + int32_t modeValue;\n> > > > + rkisp1_cif_isp_dpf_config dpf;\n> > > > + rkisp1_cif_isp_dpf_strength_config strength;\n> > > > + };\n> > > > +\n> > > > int parseConfig(const YamlObject &tuningData);\n> > > > int parseSingleConfig(const YamlObject &tuningData,\n> > > > rkisp1_cif_isp_dpf_config &config,\n> > > > rkisp1_cif_isp_dpf_strength_config &strengthConfig);\n> > > >\n> > > > - struct rkisp1_cif_isp_dpf_config config_;\n> > > > - struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;\n> > > > + bool loadConfig(int32_t mode);\n> > > > +\n> > > > + std::vector noiseReductionModes_;\n> > > > + std::vector::const_iterator activeMode_;\n> > > > };\n> > > >\n> > > > } /* namespace ipa::rkisp1::algorithms */\n> > > > --\n> > > > 2.43.0\n> > > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B952FBD78E\n\tfor ;\n\tFri, 30 Jan 2026 17:44:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF26A61FD2;\n\tFri, 30 Jan 2026 18:44:17 +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 BD1A361F84\n\tfor ;\n\tFri, 30 Jan 2026 18:44:15 +0100 (CET)","from pyrite.rasen.tech (unknown [209.216.103.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2413B22F;\n\tFri, 30 Jan 2026 18:43:36 +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=\"bF0ECEbZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769795017;\n\tbh=0/LjZDKwZOokjjVJwkZR/YcokW6YGFpYiQ0zvk7mtw8=;\n\th=In-Reply-To:References:From:Cc:To:Subject:Date:From;\n\tb=bF0ECEbZU1SDlS4KZTOewBRbJEI8uVAjVobcuEz5A3w2v/BPOND7HTkKiiHGVT5L/\n\tGfqDOYjVQcVxziD5ObF0UUb6/BSZKwr2ekG/ziqdY42bB763nxzf6oUPvgPjmCO6QA\n\tjHNSYrEpsulb0XBC8HePT1tCqRLGuJxf5AQMIlI0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20260122234709.2061370-1-rui.wang@ideasonboard.com>\n\t<20260122234709.2061370-3-rui.wang@ideasonboard.com>\n\t\n\t<176928787561.2934053.7215668025551189062@rui-Precision-7560.local>\n\t","From":"Rui Wang ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","To":"Jacopo Mondi ","Subject":"Re: [PATCH v11 2/7] ipa: rkisp1: algorithms: dpf: Implement mode\n\tswitchings","Date":"Fri, 30 Jan 2026 12:44:01 -0500","Message-ID":"<176979504177.1710132.11122717513746867379@rui-Precision-7560.local>","User-Agent":"alot/0.12","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]