[v11,2/7] ipa: rkisp1: algorithms: dpf: Implement mode switching
diff mbox series

Message ID 20260122234709.2061370-3-rui.wang@ideasonboard.com
State Superseded
Headers show
Series
  • refactor DPF parsing and initialization
Related show

Commit Message

Rui Wang Jan. 22, 2026, 11:47 p.m. UTC
Implement support for switching between different noise reduction modes.
This allows the DPF algorithm to be configured with different parameters
based on the requested noise reduction level (e.g., minimal, fast, high
quality).

Mode configurations are stored in the tuning data as a list of modes,
with each mode specifying its 'type' and corresponding DPF parameters.
An optional 'ActiveMode' setting allows defining the default mode at
startup, defaulting to "ReductionOff" if not specified.

The Dpf class is refactored to store configurations in a vector and
track the current mode using an iterator, which avoids data copying
during runtime.

Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>

---
changelog since v5:
 - Update log verbos from Info to Debug in loadReductionConfig
 - Update log mode value to string to in loadReductionConfig
 - improving the return value changed like :
       if (ret != 0)  -> if (ret)

 Reviewed-by tags from v5 are carried over (no function changes).
changelog since v6:
 - add { controls::draft::NoiseReductionModeOff, "off" }, to fix
   out_of_range issue

changelog since v7:
 - Delete base config parse from parseConfig

changelog since v8:
 - remove config_ strengthConfig_ by replacing activeMode_ iterator
   to avoiding data copy during config loading
 - Update kModesMap from std::map<int32_t, std:string>
    std::map<std::string, int32_t> for quick search improvement
 - add ActiveMode as Stefan and Jacopo's review comments
 - update type : auto -> int for ret value
 - name change loadRecuctionConfig -> loadConfig
 - delete parseMode

changelog since v9: As Stefan's suggestion
  - Update dpf reduction mode config structure format
    from  list to dictionary :
     NoiseReductionMode:
       NoiseReductionMinimal:
        ***
       NoiseReductionZSL:
        ***

changelog since v10:
  - replace kModeMap with NoiseReductionModeNameValueMap and other
    accordingly
---
 src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----
 src/ipa/rkisp1/algorithms/dpf.h   | 12 +++-
 2 files changed, 92 insertions(+), 13 deletions(-)

Comments

Jacopo Mondi Jan. 23, 2026, 8:09 a.m. UTC | #1
Hi Rui

On Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:
> Implement support for switching between different noise reduction modes.
> This allows the DPF algorithm to be configured with different parameters
> based on the requested noise reduction level (e.g., minimal, fast, high
> quality).
>
> Mode configurations are stored in the tuning data as a list of modes,
> with each mode specifying its 'type' and corresponding DPF parameters.
> An optional 'ActiveMode' setting allows defining the default mode at
> startup, defaulting to "ReductionOff" if not specified.
>
> The Dpf class is refactored to store configurations in a vector and
> track the current mode using an iterator, which avoids data copying
> during runtime.
>
> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
>
> ---
> changelog since v5:
>  - Update log verbos from Info to Debug in loadReductionConfig
>  - Update log mode value to string to in loadReductionConfig
>  - improving the return value changed like :
>        if (ret != 0)  -> if (ret)
>
>  Reviewed-by tags from v5 are carried over (no function changes).
> changelog since v6:
>  - add { controls::draft::NoiseReductionModeOff, "off" }, to fix
>    out_of_range issue
>
> changelog since v7:
>  - Delete base config parse from parseConfig
>
> changelog since v8:
>  - remove config_ strengthConfig_ by replacing activeMode_ iterator
>    to avoiding data copy during config loading
>  - Update kModesMap from std::map<int32_t, std:string>
>     std::map<std::string, int32_t> for quick search improvement
>  - add ActiveMode as Stefan and Jacopo's review comments
>  - update type : auto -> int for ret value
>  - name change loadRecuctionConfig -> loadConfig
>  - delete parseMode
>
> changelog since v9: As Stefan's suggestion
>   - Update dpf reduction mode config structure format
>     from  list to dictionary :
>      NoiseReductionMode:
>        NoiseReductionMinimal:
>         ***
>        NoiseReductionZSL:
>         ***
>
> changelog since v10:
>   - replace kModeMap with NoiseReductionModeNameValueMap and other
>     accordingly
> ---
>  src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----
>  src/ipa/rkisp1/algorithms/dpf.h   | 12 +++-
>  2 files changed, 92 insertions(+), 13 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index dd3effa1..78a79fa5 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {
>  LOG_DEFINE_CATEGORY(RkISP1Dpf)
>
>  Dpf::Dpf()
> -	: config_({}), strengthConfig_({})
> +	: noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())
>  {
>  }
>
> @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
>
>  int Dpf::parseConfig(const YamlObject &tuningData)
>  {
> -	/* Parse base config. */
> -	int ret = parseSingleConfig(tuningData, config_, strengthConfig_);
> -	if (ret)
> -		return ret;
> +	/* Parse noise reduction modes. */
> +	if (!tuningData.contains("NoiseReductionModes")) {
> +		LOG(RkISP1Dpf, Error) << "Missing modes in DPF tuning data";
> +		return -EINVAL;
> +	}
> +
> +	const YamlObject &modesObject = tuningData["NoiseReductionModes"];
> +	if (!modesObject.isDictionary()) {
> +		LOG(RkISP1Dpf, Error) << "NoiseReductionModes must be a dictionary";
> +		return -EINVAL;
> +	}
> +
> +	noiseReductionModes_.clear();
> +	for (const auto &[modeName, modeData] : modesObject.asDict()) {
> +		auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);
> +		if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> +			LOG(RkISP1Dpf, Error) << "Unknown mode type: " << modeName;
> +			return -EINVAL;
> +		}
> +
> +		ModeConfig mode;
> +		mode.modeValue = it->second;
> +		int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);
> +		if (ret) {
> +			LOG(RkISP1Dpf, Error) << "Failed to parse mode: " << modeName;
> +			return ret;
> +		}
> +
> +		noiseReductionModes_.push_back(mode);
> +	}
> +
> +	/*
> +	 * Parse the optional ActiveMode.
> +	 * If not present, default to "NoiseReductionModeOff".
> +	 */
> +	std::string activeMode =
> +		tuningData["ActiveMode"].get<std::string>().value_or("NoiseReductionModeOff");

Looking at other algorithms this seems to be more appropriate as
"activeMode" ?

> +	auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);
> +	if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> +		LOG(RkISP1Dpf, Warning) << "Invalid ActiveMode: " << activeMode;
> +		activeMode_ = noiseReductionModes_.end();
> +		return 0;
> +	}
> +
> +	if (!loadConfig(it->second)) {
> +		/* If the default "NoiseReductionModeOff" mode is requested but not configured, disable DPF. */

The comment fits on 2 lines

I'm not sure I get what you're protecting agains here but I might not
be completely awake. Is this beacause NoiseReductionModeOff is not in
the noiseReductionModes_ list ?

Can't you add it by default as we don't expect users to configure it
from tuning file ?


> +		if (it->second == controls::draft::NoiseReductionModeOff)
> +			activeMode_ = noiseReductionModes_.end();
> +		else
> +			return -EINVAL;
> +	}
>
>  	return 0;
>  }
> @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,
>  	return 0;
>  }
>
> +bool Dpf::loadConfig(int32_t mode)
> +{
> +	auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),
> +			       [mode](const ModeConfig &m) {
> +				       return m.modeValue == mode;
> +			       });
> +	if (it == noiseReductionModes_.end()) {
> +		LOG(RkISP1Dpf, Warning)
> +			<< "No DPF config for reduction mode: " << mode;
> +		return false;
> +	}
> +
> +	activeMode_ = it;
> +
> +	LOG(RkISP1Dpf, Debug)
> +		<< "DPF mode=Reduction (config loaded)"
> +		<< " mode= " << mode;

Simply

	LOG(RkISP1Dpf, Debug)
		<< "Use DPF mode " << mode;

Or something similar ?

> +
> +	return true;
> +}
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::queueRequest
>   */
> @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,
>
>  	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
>  	if (denoise) {
> -		LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
> -
>  		switch (*denoise) {
>  		case controls::draft::NoiseReductionModeOff:
>  			if (dpf.denoise) {
> @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,
>  		case controls::draft::NoiseReductionModeMinimal:
>  		case controls::draft::NoiseReductionModeHighQuality:
>  		case controls::draft::NoiseReductionModeFast:
> -			if (!dpf.denoise) {
> -				dpf.denoise = true;
> +		case controls::draft::NoiseReductionModeZSL:
> +			if (loadConfig(*denoise)) {
>  				update = true;
> +				dpf.denoise = true;
>  			}
>  			break;
>  		default:
> @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,
>  				<< *denoise;
>  			break;
>  		}
> +		if (update)
> +			LOG(RkISP1Dpf, Debug) << "Set denoise to " << modeName(*denoise);
>  	}
>
>  	frameContext.dpf.denoise = dpf.denoise;
> @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>  	strengthConfig.setEnabled(frameContext.dpf.denoise);
>
>  	if (frameContext.dpf.denoise) {
> -		*config = config_;
> -		*strengthConfig = strengthConfig_;
> +		const ModeConfig &modeConfig = *activeMode_;
> +
> +		*config = modeConfig.dpf;
> +		*strengthConfig = modeConfig.strength;
>
>  		const auto &awb = context.configuration.awb;
>  		const auto &lsc = context.configuration.lsc;
> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> index 39186c55..11fc88e4 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.h
> +++ b/src/ipa/rkisp1/algorithms/dpf.h
> @@ -30,13 +30,21 @@ public:
>  		     RkISP1Params *params) override;
>
>  private:
> +	struct ModeConfig {
> +		int32_t modeValue;
> +		rkisp1_cif_isp_dpf_config dpf;
> +		rkisp1_cif_isp_dpf_strength_config strength;
> +	};
> +
>  	int parseConfig(const YamlObject &tuningData);
>  	int parseSingleConfig(const YamlObject &tuningData,
>  			      rkisp1_cif_isp_dpf_config &config,
>  			      rkisp1_cif_isp_dpf_strength_config &strengthConfig);
>
> -	struct rkisp1_cif_isp_dpf_config config_;
> -	struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
> +	bool loadConfig(int32_t mode);
> +
> +	std::vector<ModeConfig> noiseReductionModes_;
> +	std::vector<ModeConfig>::const_iterator activeMode_;
>  };
>
>  } /* namespace ipa::rkisp1::algorithms */
> --
> 2.43.0
>
Rui Wang Jan. 24, 2026, 8:51 p.m. UTC | #2
Quoting Jacopo Mondi (2026-01-23 03:09:15)
> Hi Rui
> 
> On Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:
> > Implement support for switching between different noise reduction modes.
> > This allows the DPF algorithm to be configured with different parameters
> > based on the requested noise reduction level (e.g., minimal, fast, high
> > quality).
> >
> > Mode configurations are stored in the tuning data as a list of modes,
> > with each mode specifying its 'type' and corresponding DPF parameters.
> > An optional 'ActiveMode' setting allows defining the default mode at
> > startup, defaulting to "ReductionOff" if not specified.
> >
> > The Dpf class is refactored to store configurations in a vector and
> > track the current mode using an iterator, which avoids data copying
> > during runtime.
> >
> > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> >
> > ---
> > changelog since v5:
> >  - Update log verbos from Info to Debug in loadReductionConfig
> >  - Update log mode value to string to in loadReductionConfig
> >  - improving the return value changed like :
> >        if (ret != 0)  -> if (ret)
> >
> >  Reviewed-by tags from v5 are carried over (no function changes).
> > changelog since v6:
> >  - add { controls::draft::NoiseReductionModeOff, "off" }, to fix
> >    out_of_range issue
> >
> > changelog since v7:
> >  - Delete base config parse from parseConfig
> >
> > changelog since v8:
> >  - remove config_ strengthConfig_ by replacing activeMode_ iterator
> >    to avoiding data copy during config loading
> >  - Update kModesMap from std::map<int32_t, std:string>
> >     std::map<std::string, int32_t> for quick search improvement
> >  - add ActiveMode as Stefan and Jacopo's review comments
> >  - update type : auto -> int for ret value
> >  - name change loadRecuctionConfig -> loadConfig
> >  - delete parseMode
> >
> > changelog since v9: As Stefan's suggestion
> >   - Update dpf reduction mode config structure format
> >     from  list to dictionary :
> >      NoiseReductionMode:
> >        NoiseReductionMinimal:
> >         ***
> >        NoiseReductionZSL:
> >         ***
> >
> > changelog since v10:
> >   - replace kModeMap with NoiseReductionModeNameValueMap and other
> >     accordingly
> > ---
> >  src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----
> >  src/ipa/rkisp1/algorithms/dpf.h   | 12 +++-
> >  2 files changed, 92 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > index dd3effa1..78a79fa5 100644
> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {
> >  LOG_DEFINE_CATEGORY(RkISP1Dpf)
> >
> >  Dpf::Dpf()
> > -     : config_({}), strengthConfig_({})
> > +     : noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())
> >  {
> >  }
> >
> > @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
> >
> >  int Dpf::parseConfig(const YamlObject &tuningData)
> >  {
> > -     /* Parse base config. */
> > -     int ret = parseSingleConfig(tuningData, config_, strengthConfig_);
> > -     if (ret)
> > -             return ret;
> > +     /* Parse noise reduction modes. */
> > +     if (!tuningData.contains("NoiseReductionModes")) {
> > +             LOG(RkISP1Dpf, Error) << "Missing modes in DPF tuning data";
> > +             return -EINVAL;
> > +     }
> > +
> > +     const YamlObject &modesObject = tuningData["NoiseReductionModes"];
> > +     if (!modesObject.isDictionary()) {
> > +             LOG(RkISP1Dpf, Error) << "NoiseReductionModes must be a dictionary";
> > +             return -EINVAL;
> > +     }
> > +
> > +     noiseReductionModes_.clear();
> > +     for (const auto &[modeName, modeData] : modesObject.asDict()) {
> > +             auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);
> > +             if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> > +                     LOG(RkISP1Dpf, Error) << "Unknown mode type: " << modeName;
> > +                     return -EINVAL;
> > +             }
> > +
> > +             ModeConfig mode;
> > +             mode.modeValue = it->second;
> > +             int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);
> > +             if (ret) {
> > +                     LOG(RkISP1Dpf, Error) << "Failed to parse mode: " << modeName;
> > +                     return ret;
> > +             }
> > +
> > +             noiseReductionModes_.push_back(mode);
> > +     }
> > +
> > +     /*
> > +      * Parse the optional ActiveMode.
> > +      * If not present, default to "NoiseReductionModeOff".
> > +      */
> > +     std::string activeMode =
> > +             tuningData["ActiveMode"].get<std::string>().value_or("NoiseReductionModeOff");
> 
> Looking at other algorithms this seems to be more appropriate as
> "activeMode" ?

  I will check other module to check if upper "A" in tuning config.
  Then update in v2
> 
> > +     auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);
> > +     if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> > +             LOG(RkISP1Dpf, Warning) << "Invalid ActiveMode: " << activeMode;
> > +             activeMode_ = noiseReductionModes_.end();
> > +             return 0;
> > +     }
> > +
> > +     if (!loadConfig(it->second)) {
> > +             /* If the default "NoiseReductionModeOff" mode is requested but not configured, disable DPF. */
> 
> The comment fits on 2 lines
> 
> I'm not sure I get what you're protecting agains here but I might not
> be completely awake. Is this beacause NoiseReductionModeOff is not in
> the noiseReductionModes_ list ?
> 
  yes, NoiseRectionValueMap contains "OFF" , and noiseRectionModes_ has only
  valid config for each mode.
  I can improve the logic :
   if("OFF") {
	   activeMode_ = end
	   retun 0;
   }

> Can't you add it by default as we don't expect users to configure it
> from tuning file ?
> 
  Hello Jacopo,
  As both DPF and Filter denoise share same control NoiseReductionMode
  I am afraid the tuning config for them has some mode mismatch.
  and currently I prefer "OFF" as default without any configuration.
  from code implementation it is much straitforward, and once "AUTO"
  denoise mode implemented, I will swith to AUTO as default without any 
  declaration in tuning file.
  Do you think this strategy acceptable ?


  

> 
> > +             if (it->second == controls::draft::NoiseReductionModeOff)
> > +                     activeMode_ = noiseReductionModes_.end();
> > +             else
> > +                     return -EINVAL;
> > +     }
> >
> >       return 0;
> >  }
> > @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,
> >       return 0;
> >  }
> >
> > +bool Dpf::loadConfig(int32_t mode)
> > +{
> > +     auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),
> > +                            [mode](const ModeConfig &m) {
> > +                                    return m.modeValue == mode;
> > +                            });
> > +     if (it == noiseReductionModes_.end()) {
> > +             LOG(RkISP1Dpf, Warning)
> > +                     << "No DPF config for reduction mode: " << mode;
> > +             return false;
> > +     }
> > +
> > +     activeMode_ = it;
> > +
> > +     LOG(RkISP1Dpf, Debug)
> > +             << "DPF mode=Reduction (config loaded)"
> > +             << " mode= " << mode;
> 
> Simply
> 
>         LOG(RkISP1Dpf, Debug)
>                 << "Use DPF mode " << mode;
> 
> Or something similar ?
> 
> > +
> > +     return true;
> > +}
> > +
> >  /**
> >   * \copydoc libcamera::ipa::Algorithm::queueRequest
> >   */
> > @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,
> >
> >       const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> >       if (denoise) {
> > -             LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
> > -
> >               switch (*denoise) {
> >               case controls::draft::NoiseReductionModeOff:
> >                       if (dpf.denoise) {
> > @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,
> >               case controls::draft::NoiseReductionModeMinimal:
> >               case controls::draft::NoiseReductionModeHighQuality:
> >               case controls::draft::NoiseReductionModeFast:
> > -                     if (!dpf.denoise) {
> > -                             dpf.denoise = true;
> > +             case controls::draft::NoiseReductionModeZSL:
> > +                     if (loadConfig(*denoise)) {
> >                               update = true;
> > +                             dpf.denoise = true;
> >                       }
> >                       break;
> >               default:
> > @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,
> >                               << *denoise;
> >                       break;
> >               }
> > +             if (update)
> > +                     LOG(RkISP1Dpf, Debug) << "Set denoise to " << modeName(*denoise);
> >       }
> >
> >       frameContext.dpf.denoise = dpf.denoise;
> > @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
> >       strengthConfig.setEnabled(frameContext.dpf.denoise);
> >
> >       if (frameContext.dpf.denoise) {
> > -             *config = config_;
> > -             *strengthConfig = strengthConfig_;
> > +             const ModeConfig &modeConfig = *activeMode_;
> > +
> > +             *config = modeConfig.dpf;
> > +             *strengthConfig = modeConfig.strength;
> >
> >               const auto &awb = context.configuration.awb;
> >               const auto &lsc = context.configuration.lsc;
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> > index 39186c55..11fc88e4 100644
> > --- a/src/ipa/rkisp1/algorithms/dpf.h
> > +++ b/src/ipa/rkisp1/algorithms/dpf.h
> > @@ -30,13 +30,21 @@ public:
> >                    RkISP1Params *params) override;
> >
> >  private:
> > +     struct ModeConfig {
> > +             int32_t modeValue;
> > +             rkisp1_cif_isp_dpf_config dpf;
> > +             rkisp1_cif_isp_dpf_strength_config strength;
> > +     };
> > +
> >       int parseConfig(const YamlObject &tuningData);
> >       int parseSingleConfig(const YamlObject &tuningData,
> >                             rkisp1_cif_isp_dpf_config &config,
> >                             rkisp1_cif_isp_dpf_strength_config &strengthConfig);
> >
> > -     struct rkisp1_cif_isp_dpf_config config_;
> > -     struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
> > +     bool loadConfig(int32_t mode);
> > +
> > +     std::vector<ModeConfig> noiseReductionModes_;
> > +     std::vector<ModeConfig>::const_iterator activeMode_;
> >  };
> >
> >  } /* namespace ipa::rkisp1::algorithms */
> > --
> > 2.43.0
> >
Jacopo Mondi Jan. 26, 2026, 8:04 a.m. UTC | #3
Hi Rui

On Sat, Jan 24, 2026 at 03:51:15PM -0500, Rui Wang wrote:
> Quoting Jacopo Mondi (2026-01-23 03:09:15)
> > Hi Rui
> >
> > On Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:
> > > Implement support for switching between different noise reduction modes.
> > > This allows the DPF algorithm to be configured with different parameters
> > > based on the requested noise reduction level (e.g., minimal, fast, high
> > > quality).
> > >
> > > Mode configurations are stored in the tuning data as a list of modes,
> > > with each mode specifying its 'type' and corresponding DPF parameters.
> > > An optional 'ActiveMode' setting allows defining the default mode at
> > > startup, defaulting to "ReductionOff" if not specified.
> > >
> > > The Dpf class is refactored to store configurations in a vector and
> > > track the current mode using an iterator, which avoids data copying
> > > during runtime.
> > >
> > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> > >
> > > ---
> > > changelog since v5:
> > >  - Update log verbos from Info to Debug in loadReductionConfig
> > >  - Update log mode value to string to in loadReductionConfig
> > >  - improving the return value changed like :
> > >        if (ret != 0)  -> if (ret)
> > >
> > >  Reviewed-by tags from v5 are carried over (no function changes).
> > > changelog since v6:
> > >  - add { controls::draft::NoiseReductionModeOff, "off" }, to fix
> > >    out_of_range issue
> > >
> > > changelog since v7:
> > >  - Delete base config parse from parseConfig
> > >
> > > changelog since v8:
> > >  - remove config_ strengthConfig_ by replacing activeMode_ iterator
> > >    to avoiding data copy during config loading
> > >  - Update kModesMap from std::map<int32_t, std:string>
> > >     std::map<std::string, int32_t> for quick search improvement
> > >  - add ActiveMode as Stefan and Jacopo's review comments
> > >  - update type : auto -> int for ret value
> > >  - name change loadRecuctionConfig -> loadConfig
> > >  - delete parseMode
> > >
> > > changelog since v9: As Stefan's suggestion
> > >   - Update dpf reduction mode config structure format
> > >     from  list to dictionary :
> > >      NoiseReductionMode:
> > >        NoiseReductionMinimal:
> > >         ***
> > >        NoiseReductionZSL:
> > >         ***
> > >
> > > changelog since v10:
> > >   - replace kModeMap with NoiseReductionModeNameValueMap and other
> > >     accordingly
> > > ---
> > >  src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----
> > >  src/ipa/rkisp1/algorithms/dpf.h   | 12 +++-
> > >  2 files changed, 92 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > > index dd3effa1..78a79fa5 100644
> > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > > @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {
> > >  LOG_DEFINE_CATEGORY(RkISP1Dpf)
> > >
> > >  Dpf::Dpf()
> > > -     : config_({}), strengthConfig_({})
> > > +     : noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())
> > >  {
> > >  }
> > >
> > > @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
> > >
> > >  int Dpf::parseConfig(const YamlObject &tuningData)
> > >  {
> > > -     /* Parse base config. */
> > > -     int ret = parseSingleConfig(tuningData, config_, strengthConfig_);
> > > -     if (ret)
> > > -             return ret;
> > > +     /* Parse noise reduction modes. */
> > > +     if (!tuningData.contains("NoiseReductionModes")) {
> > > +             LOG(RkISP1Dpf, Error) << "Missing modes in DPF tuning data";
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     const YamlObject &modesObject = tuningData["NoiseReductionModes"];
> > > +     if (!modesObject.isDictionary()) {
> > > +             LOG(RkISP1Dpf, Error) << "NoiseReductionModes must be a dictionary";
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     noiseReductionModes_.clear();
> > > +     for (const auto &[modeName, modeData] : modesObject.asDict()) {
> > > +             auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);
> > > +             if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> > > +                     LOG(RkISP1Dpf, Error) << "Unknown mode type: " << modeName;
> > > +                     return -EINVAL;
> > > +             }
> > > +
> > > +             ModeConfig mode;
> > > +             mode.modeValue = it->second;
> > > +             int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);
> > > +             if (ret) {
> > > +                     LOG(RkISP1Dpf, Error) << "Failed to parse mode: " << modeName;
> > > +                     return ret;
> > > +             }
> > > +
> > > +             noiseReductionModes_.push_back(mode);
> > > +     }
> > > +
> > > +     /*
> > > +      * Parse the optional ActiveMode.
> > > +      * If not present, default to "NoiseReductionModeOff".
> > > +      */
> > > +     std::string activeMode =
> > > +             tuningData["ActiveMode"].get<std::string>().value_or("NoiseReductionModeOff");
> >
> > Looking at other algorithms this seems to be more appropriate as
> > "activeMode" ?
>
>   I will check other module to check if upper "A" in tuning config.
>   Then update in v2
> >
> > > +     auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);
> > > +     if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> > > +             LOG(RkISP1Dpf, Warning) << "Invalid ActiveMode: " << activeMode;
> > > +             activeMode_ = noiseReductionModes_.end();
> > > +             return 0;
> > > +     }
> > > +
> > > +     if (!loadConfig(it->second)) {
> > > +             /* If the default "NoiseReductionModeOff" mode is requested but not configured, disable DPF. */
> >
> > The comment fits on 2 lines
> >
> > I'm not sure I get what you're protecting agains here but I might not
> > be completely awake. Is this beacause NoiseReductionModeOff is not in
> > the noiseReductionModes_ list ?
> >
>   yes, NoiseRectionValueMap contains "OFF" , and noiseRectionModes_ has only
>   valid config for each mode.
>   I can improve the logic :
>    if("OFF") {
> 	   activeMode_ = end
> 	   retun 0;
>    }
>
> > Can't you add it by default as we don't expect users to configure it
> > from tuning file ?
> >
>   Hello Jacopo,
>   As both DPF and Filter denoise share same control NoiseReductionMode
>   I am afraid the tuning config for them has some mode mismatch.
>   and currently I prefer "OFF" as default without any configuration.
>   from code implementation it is much straitforward, and once "AUTO"
>   denoise mode implemented, I will swith to AUTO as default without any
>   declaration in tuning file.
>   Do you think this strategy acceptable ?

I'm sorry if I can't my messages get through. I'll work on my English,
but I meant to suggest to add

        noiseReductionModes_.push_back(NoiseReductionModeOff);

before the for() loop


>
>
>
>
> >
> > > +             if (it->second == controls::draft::NoiseReductionModeOff)
> > > +                     activeMode_ = noiseReductionModes_.end();
> > > +             else
> > > +                     return -EINVAL;
> > > +     }
> > >
> > >       return 0;
> > >  }
> > > @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,
> > >       return 0;
> > >  }
> > >
> > > +bool Dpf::loadConfig(int32_t mode)
> > > +{
> > > +     auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),
> > > +                            [mode](const ModeConfig &m) {
> > > +                                    return m.modeValue == mode;
> > > +                            });
> > > +     if (it == noiseReductionModes_.end()) {
> > > +             LOG(RkISP1Dpf, Warning)
> > > +                     << "No DPF config for reduction mode: " << mode;
> > > +             return false;
> > > +     }
> > > +
> > > +     activeMode_ = it;
> > > +
> > > +     LOG(RkISP1Dpf, Debug)
> > > +             << "DPF mode=Reduction (config loaded)"
> > > +             << " mode= " << mode;
> >
> > Simply
> >
> >         LOG(RkISP1Dpf, Debug)
> >                 << "Use DPF mode " << mode;
> >
> > Or something similar ?
> >
> > > +
> > > +     return true;
> > > +}
> > > +
> > >  /**
> > >   * \copydoc libcamera::ipa::Algorithm::queueRequest
> > >   */
> > > @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,
> > >
> > >       const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> > >       if (denoise) {
> > > -             LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
> > > -
> > >               switch (*denoise) {
> > >               case controls::draft::NoiseReductionModeOff:
> > >                       if (dpf.denoise) {
> > > @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,
> > >               case controls::draft::NoiseReductionModeMinimal:
> > >               case controls::draft::NoiseReductionModeHighQuality:
> > >               case controls::draft::NoiseReductionModeFast:
> > > -                     if (!dpf.denoise) {
> > > -                             dpf.denoise = true;
> > > +             case controls::draft::NoiseReductionModeZSL:
> > > +                     if (loadConfig(*denoise)) {
> > >                               update = true;
> > > +                             dpf.denoise = true;
> > >                       }
> > >                       break;
> > >               default:
> > > @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,
> > >                               << *denoise;
> > >                       break;
> > >               }
> > > +             if (update)
> > > +                     LOG(RkISP1Dpf, Debug) << "Set denoise to " << modeName(*denoise);
> > >       }
> > >
> > >       frameContext.dpf.denoise = dpf.denoise;
> > > @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
> > >       strengthConfig.setEnabled(frameContext.dpf.denoise);
> > >
> > >       if (frameContext.dpf.denoise) {
> > > -             *config = config_;
> > > -             *strengthConfig = strengthConfig_;
> > > +             const ModeConfig &modeConfig = *activeMode_;
> > > +
> > > +             *config = modeConfig.dpf;
> > > +             *strengthConfig = modeConfig.strength;
> > >
> > >               const auto &awb = context.configuration.awb;
> > >               const auto &lsc = context.configuration.lsc;
> > > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> > > index 39186c55..11fc88e4 100644
> > > --- a/src/ipa/rkisp1/algorithms/dpf.h
> > > +++ b/src/ipa/rkisp1/algorithms/dpf.h
> > > @@ -30,13 +30,21 @@ public:
> > >                    RkISP1Params *params) override;
> > >
> > >  private:
> > > +     struct ModeConfig {
> > > +             int32_t modeValue;
> > > +             rkisp1_cif_isp_dpf_config dpf;
> > > +             rkisp1_cif_isp_dpf_strength_config strength;
> > > +     };
> > > +
> > >       int parseConfig(const YamlObject &tuningData);
> > >       int parseSingleConfig(const YamlObject &tuningData,
> > >                             rkisp1_cif_isp_dpf_config &config,
> > >                             rkisp1_cif_isp_dpf_strength_config &strengthConfig);
> > >
> > > -     struct rkisp1_cif_isp_dpf_config config_;
> > > -     struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
> > > +     bool loadConfig(int32_t mode);
> > > +
> > > +     std::vector<ModeConfig> noiseReductionModes_;
> > > +     std::vector<ModeConfig>::const_iterator activeMode_;
> > >  };
> > >
> > >  } /* namespace ipa::rkisp1::algorithms */
> > > --
> > > 2.43.0
> > >
Rui Wang Jan. 30, 2026, 5:44 p.m. UTC | #4
Quoting Jacopo Mondi (2026-01-26 03:04:14)
> Hi Rui
> 
> On Sat, Jan 24, 2026 at 03:51:15PM -0500, Rui Wang wrote:
> > Quoting Jacopo Mondi (2026-01-23 03:09:15)
> > > Hi Rui
> > >
> > > On Thu, Jan 22, 2026 at 06:47:04PM -0500, Rui Wang wrote:
> > > > Implement support for switching between different noise reduction modes.
> > > > This allows the DPF algorithm to be configured with different parameters
> > > > based on the requested noise reduction level (e.g., minimal, fast, high
> > > > quality).
> > > >
> > > > Mode configurations are stored in the tuning data as a list of modes,
> > > > with each mode specifying its 'type' and corresponding DPF parameters.
> > > > An optional 'ActiveMode' setting allows defining the default mode at
> > > > startup, defaulting to "ReductionOff" if not specified.
> > > >
> > > > The Dpf class is refactored to store configurations in a vector and
> > > > track the current mode using an iterator, which avoids data copying
> > > > during runtime.
> > > >
> > > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> > > >
> > > > ---
> > > > changelog since v5:
> > > >  - Update log verbos from Info to Debug in loadReductionConfig
> > > >  - Update log mode value to string to in loadReductionConfig
> > > >  - improving the return value changed like :
> > > >        if (ret != 0)  -> if (ret)
> > > >
> > > >  Reviewed-by tags from v5 are carried over (no function changes).
> > > > changelog since v6:
> > > >  - add { controls::draft::NoiseReductionModeOff, "off" }, to fix
> > > >    out_of_range issue
> > > >
> > > > changelog since v7:
> > > >  - Delete base config parse from parseConfig
> > > >
> > > > changelog since v8:
> > > >  - remove config_ strengthConfig_ by replacing activeMode_ iterator
> > > >    to avoiding data copy during config loading
> > > >  - Update kModesMap from std::map<int32_t, std:string>
> > > >     std::map<std::string, int32_t> for quick search improvement
> > > >  - add ActiveMode as Stefan and Jacopo's review comments
> > > >  - update type : auto -> int for ret value
> > > >  - name change loadRecuctionConfig -> loadConfig
> > > >  - delete parseMode
> > > >
> > > > changelog since v9: As Stefan's suggestion
> > > >   - Update dpf reduction mode config structure format
> > > >     from  list to dictionary :
> > > >      NoiseReductionMode:
> > > >        NoiseReductionMinimal:
> > > >         ***
> > > >        NoiseReductionZSL:
> > > >         ***
> > > >
> > > > changelog since v10:
> > > >   - replace kModeMap with NoiseReductionModeNameValueMap and other
> > > >     accordingly
> > > > ---
> > > >  src/ipa/rkisp1/algorithms/dpf.cpp | 93 +++++++++++++++++++++++++++----
> > > >  src/ipa/rkisp1/algorithms/dpf.h   | 12 +++-
> > > >  2 files changed, 92 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > > > index dd3effa1..78a79fa5 100644
> > > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> > > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > > > @@ -37,7 +37,7 @@ namespace ipa::rkisp1::algorithms {
> > > >  LOG_DEFINE_CATEGORY(RkISP1Dpf)
> > > >
> > > >  Dpf::Dpf()
> > > > -     : config_({}), strengthConfig_({})
> > > > +     : noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())
> > > >  {
> > > >  }
> > > >
> > > > @@ -57,10 +57,57 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
> > > >
> > > >  int Dpf::parseConfig(const YamlObject &tuningData)
> > > >  {
> > > > -     /* Parse base config. */
> > > > -     int ret = parseSingleConfig(tuningData, config_, strengthConfig_);
> > > > -     if (ret)
> > > > -             return ret;
> > > > +     /* Parse noise reduction modes. */
> > > > +     if (!tuningData.contains("NoiseReductionModes")) {
> > > > +             LOG(RkISP1Dpf, Error) << "Missing modes in DPF tuning data";
> > > > +             return -EINVAL;
> > > > +     }
> > > > +
> > > > +     const YamlObject &modesObject = tuningData["NoiseReductionModes"];
> > > > +     if (!modesObject.isDictionary()) {
> > > > +             LOG(RkISP1Dpf, Error) << "NoiseReductionModes must be a dictionary";
> > > > +             return -EINVAL;
> > > > +     }
> > > > +
> > > > +     noiseReductionModes_.clear();
> > > > +     for (const auto &[modeName, modeData] : modesObject.asDict()) {
> > > > +             auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);
> > > > +             if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> > > > +                     LOG(RkISP1Dpf, Error) << "Unknown mode type: " << modeName;
> > > > +                     return -EINVAL;
> > > > +             }
> > > > +
> > > > +             ModeConfig mode;
> > > > +             mode.modeValue = it->second;
> > > > +             int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);
> > > > +             if (ret) {
> > > > +                     LOG(RkISP1Dpf, Error) << "Failed to parse mode: " << modeName;
> > > > +                     return ret;
> > > > +             }
> > > > +
> > > > +             noiseReductionModes_.push_back(mode);
> > > > +     }
> > > > +
> > > > +     /*
> > > > +      * Parse the optional ActiveMode.
> > > > +      * If not present, default to "NoiseReductionModeOff".
> > > > +      */
> > > > +     std::string activeMode =
> > > > +             tuningData["ActiveMode"].get<std::string>().value_or("NoiseReductionModeOff");
> > >
> > > Looking at other algorithms this seems to be more appropriate as
> > > "activeMode" ?
> >
> >   I will check other module to check if upper "A" in tuning config.
> >   Then update in v2
> > >
> > > > +     auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);
> > > > +     if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
> > > > +             LOG(RkISP1Dpf, Warning) << "Invalid ActiveMode: " << activeMode;
> > > > +             activeMode_ = noiseReductionModes_.end();
> > > > +             return 0;
> > > > +     }
> > > > +
> > > > +     if (!loadConfig(it->second)) {
> > > > +             /* If the default "NoiseReductionModeOff" mode is requested but not configured, disable DPF. */
> > >
> > > The comment fits on 2 lines
> > >
> > > I'm not sure I get what you're protecting agains here but I might not
> > > be completely awake. Is this beacause NoiseReductionModeOff is not in
> > > the noiseReductionModes_ list ?
> > >
> >   yes, NoiseRectionValueMap contains "OFF" , and noiseRectionModes_ has only
> >   valid config for each mode.
> >   I can improve the logic :
> >    if("OFF") {
> >          activeMode_ = end
> >          retun 0;
> >    }
> >
> > > Can't you add it by default as we don't expect users to configure it
> > > from tuning file ?
> > >
> >   Hello Jacopo,
> >   As both DPF and Filter denoise share same control NoiseReductionMode
> >   I am afraid the tuning config for them has some mode mismatch.
> >   and currently I prefer "OFF" as default without any configuration.
> >   from code implementation it is much straitforward, and once "AUTO"
> >   denoise mode implemented, I will swith to AUTO as default without any
> >   declaration in tuning file.
> >   Do you think this strategy acceptable ?
> 
> I'm sorry if I can't my messages get through. I'll work on my English,
> but I meant to suggest to add
> 
>         noiseReductionModes_.push_back(NoiseReductionModeOff);
> 
> before the for() loop
>
yes , that's suggestion to add "Off" into the map , and it will reduce some exception check when traversing elements
> 
> >
> >
> >
> >
> > >
> > > > +             if (it->second == controls::draft::NoiseReductionModeOff)
> > > > +                     activeMode_ = noiseReductionModes_.end();
> > > > +             else
> > > > +                     return -EINVAL;
> > > > +     }
> > > >
> > > >       return 0;
> > > >  }
> > > > @@ -193,6 +240,27 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,
> > > >       return 0;
> > > >  }
> > > >
> > > > +bool Dpf::loadConfig(int32_t mode)
> > > > +{
> > > > +     auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),
> > > > +                            [mode](const ModeConfig &m) {
> > > > +                                    return m.modeValue == mode;
> > > > +                            });
> > > > +     if (it == noiseReductionModes_.end()) {
> > > > +             LOG(RkISP1Dpf, Warning)
> > > > +                     << "No DPF config for reduction mode: " << mode;
> > > > +             return false;
> > > > +     }
> > > > +
> > > > +     activeMode_ = it;
> > > > +
> > > > +     LOG(RkISP1Dpf, Debug)
> > > > +             << "DPF mode=Reduction (config loaded)"
> > > > +             << " mode= " << mode;
> > >
> > > Simply
> > >
> > >         LOG(RkISP1Dpf, Debug)
> > >                 << "Use DPF mode " << mode;
> > >
> > > Or something similar ?
> > >
> > > > +
> > > > +     return true;
> > > > +}
> > > > +
> > > >  /**
> > > >   * \copydoc libcamera::ipa::Algorithm::queueRequest
> > > >   */
> > > > @@ -206,8 +274,6 @@ void Dpf::queueRequest(IPAContext &context,
> > > >
> > > >       const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> > > >       if (denoise) {
> > > > -             LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
> > > > -
> > > >               switch (*denoise) {
> > > >               case controls::draft::NoiseReductionModeOff:
> > > >                       if (dpf.denoise) {
> > > > @@ -218,9 +284,10 @@ void Dpf::queueRequest(IPAContext &context,
> > > >               case controls::draft::NoiseReductionModeMinimal:
> > > >               case controls::draft::NoiseReductionModeHighQuality:
> > > >               case controls::draft::NoiseReductionModeFast:
> > > > -                     if (!dpf.denoise) {
> > > > -                             dpf.denoise = true;
> > > > +             case controls::draft::NoiseReductionModeZSL:
> > > > +                     if (loadConfig(*denoise)) {
> > > >                               update = true;
> > > > +                             dpf.denoise = true;
> > > >                       }
> > > >                       break;
> > > >               default:
> > > > @@ -229,6 +296,8 @@ void Dpf::queueRequest(IPAContext &context,
> > > >                               << *denoise;
> > > >                       break;
> > > >               }
> > > > +             if (update)
> > > > +                     LOG(RkISP1Dpf, Debug) << "Set denoise to " << modeName(*denoise);
> > > >       }
> > > >
> > > >       frameContext.dpf.denoise = dpf.denoise;
> > > > @@ -251,8 +320,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
> > > >       strengthConfig.setEnabled(frameContext.dpf.denoise);
> > > >
> > > >       if (frameContext.dpf.denoise) {
> > > > -             *config = config_;
> > > > -             *strengthConfig = strengthConfig_;
> > > > +             const ModeConfig &modeConfig = *activeMode_;
> > > > +
> > > > +             *config = modeConfig.dpf;
> > > > +             *strengthConfig = modeConfig.strength;
> > > >
> > > >               const auto &awb = context.configuration.awb;
> > > >               const auto &lsc = context.configuration.lsc;
> > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> > > > index 39186c55..11fc88e4 100644
> > > > --- a/src/ipa/rkisp1/algorithms/dpf.h
> > > > +++ b/src/ipa/rkisp1/algorithms/dpf.h
> > > > @@ -30,13 +30,21 @@ public:
> > > >                    RkISP1Params *params) override;
> > > >
> > > >  private:
> > > > +     struct ModeConfig {
> > > > +             int32_t modeValue;
> > > > +             rkisp1_cif_isp_dpf_config dpf;
> > > > +             rkisp1_cif_isp_dpf_strength_config strength;
> > > > +     };
> > > > +
> > > >       int parseConfig(const YamlObject &tuningData);
> > > >       int parseSingleConfig(const YamlObject &tuningData,
> > > >                             rkisp1_cif_isp_dpf_config &config,
> > > >                             rkisp1_cif_isp_dpf_strength_config &strengthConfig);
> > > >
> > > > -     struct rkisp1_cif_isp_dpf_config config_;
> > > > -     struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
> > > > +     bool loadConfig(int32_t mode);
> > > > +
> > > > +     std::vector<ModeConfig> noiseReductionModes_;
> > > > +     std::vector<ModeConfig>::const_iterator activeMode_;
> > > >  };
> > > >
> > > >  } /* namespace ipa::rkisp1::algorithms */
> > > > --
> > > > 2.43.0
> > > >

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
index dd3effa1..78a79fa5 100644
--- a/src/ipa/rkisp1/algorithms/dpf.cpp
+++ b/src/ipa/rkisp1/algorithms/dpf.cpp
@@ -37,7 +37,7 @@  namespace ipa::rkisp1::algorithms {
 LOG_DEFINE_CATEGORY(RkISP1Dpf)
 
 Dpf::Dpf()
-	: config_({}), strengthConfig_({})
+	: noiseReductionModes_({}), activeMode_(noiseReductionModes_.end())
 {
 }
 
@@ -57,10 +57,57 @@  int Dpf::init([[maybe_unused]] IPAContext &context,
 
 int Dpf::parseConfig(const YamlObject &tuningData)
 {
-	/* Parse base config. */
-	int ret = parseSingleConfig(tuningData, config_, strengthConfig_);
-	if (ret)
-		return ret;
+	/* Parse noise reduction modes. */
+	if (!tuningData.contains("NoiseReductionModes")) {
+		LOG(RkISP1Dpf, Error) << "Missing modes in DPF tuning data";
+		return -EINVAL;
+	}
+
+	const YamlObject &modesObject = tuningData["NoiseReductionModes"];
+	if (!modesObject.isDictionary()) {
+		LOG(RkISP1Dpf, Error) << "NoiseReductionModes must be a dictionary";
+		return -EINVAL;
+	}
+
+	noiseReductionModes_.clear();
+	for (const auto &[modeName, modeData] : modesObject.asDict()) {
+		auto it = controls::draft::NoiseReductionModeNameValueMap.find(modeName);
+		if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
+			LOG(RkISP1Dpf, Error) << "Unknown mode type: " << modeName;
+			return -EINVAL;
+		}
+
+		ModeConfig mode;
+		mode.modeValue = it->second;
+		int ret = parseSingleConfig(modeData, mode.dpf, mode.strength);
+		if (ret) {
+			LOG(RkISP1Dpf, Error) << "Failed to parse mode: " << modeName;
+			return ret;
+		}
+
+		noiseReductionModes_.push_back(mode);
+	}
+
+	/*
+	 * Parse the optional ActiveMode.
+	 * If not present, default to "NoiseReductionModeOff".
+	 */
+	std::string activeMode =
+		tuningData["ActiveMode"].get<std::string>().value_or("NoiseReductionModeOff");
+	auto it = controls::draft::NoiseReductionModeNameValueMap.find(activeMode);
+	if (it == controls::draft::NoiseReductionModeNameValueMap.end()) {
+		LOG(RkISP1Dpf, Warning) << "Invalid ActiveMode: " << activeMode;
+		activeMode_ = noiseReductionModes_.end();
+		return 0;
+	}
+
+	if (!loadConfig(it->second)) {
+		/* If the default "NoiseReductionModeOff" mode is requested but not configured, disable DPF. */
+		if (it->second == controls::draft::NoiseReductionModeOff)
+			activeMode_ = noiseReductionModes_.end();
+		else
+			return -EINVAL;
+	}
 
 	return 0;
 }
@@ -193,6 +240,27 @@  int Dpf::parseSingleConfig(const YamlObject &tuningData,
 	return 0;
 }
 
+bool Dpf::loadConfig(int32_t mode)
+{
+	auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),
+			       [mode](const ModeConfig &m) {
+				       return m.modeValue == mode;
+			       });
+	if (it == noiseReductionModes_.end()) {
+		LOG(RkISP1Dpf, Warning)
+			<< "No DPF config for reduction mode: " << mode;
+		return false;
+	}
+
+	activeMode_ = it;
+
+	LOG(RkISP1Dpf, Debug)
+		<< "DPF mode=Reduction (config loaded)"
+		<< " mode= " << mode;
+
+	return true;
+}
+
 /**
  * \copydoc libcamera::ipa::Algorithm::queueRequest
  */
@@ -206,8 +274,6 @@  void Dpf::queueRequest(IPAContext &context,
 
 	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
 	if (denoise) {
-		LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
-
 		switch (*denoise) {
 		case controls::draft::NoiseReductionModeOff:
 			if (dpf.denoise) {
@@ -218,9 +284,10 @@  void Dpf::queueRequest(IPAContext &context,
 		case controls::draft::NoiseReductionModeMinimal:
 		case controls::draft::NoiseReductionModeHighQuality:
 		case controls::draft::NoiseReductionModeFast:
-			if (!dpf.denoise) {
-				dpf.denoise = true;
+		case controls::draft::NoiseReductionModeZSL:
+			if (loadConfig(*denoise)) {
 				update = true;
+				dpf.denoise = true;
 			}
 			break;
 		default:
@@ -229,6 +296,8 @@  void Dpf::queueRequest(IPAContext &context,
 				<< *denoise;
 			break;
 		}
+		if (update)
+			LOG(RkISP1Dpf, Debug) << "Set denoise to " << modeName(*denoise);
 	}
 
 	frameContext.dpf.denoise = dpf.denoise;
@@ -251,8 +320,10 @@  void Dpf::prepare(IPAContext &context, const uint32_t frame,
 	strengthConfig.setEnabled(frameContext.dpf.denoise);
 
 	if (frameContext.dpf.denoise) {
-		*config = config_;
-		*strengthConfig = strengthConfig_;
+		const ModeConfig &modeConfig = *activeMode_;
+
+		*config = modeConfig.dpf;
+		*strengthConfig = modeConfig.strength;
 
 		const auto &awb = context.configuration.awb;
 		const auto &lsc = context.configuration.lsc;
diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
index 39186c55..11fc88e4 100644
--- a/src/ipa/rkisp1/algorithms/dpf.h
+++ b/src/ipa/rkisp1/algorithms/dpf.h
@@ -30,13 +30,21 @@  public:
 		     RkISP1Params *params) override;
 
 private:
+	struct ModeConfig {
+		int32_t modeValue;
+		rkisp1_cif_isp_dpf_config dpf;
+		rkisp1_cif_isp_dpf_strength_config strength;
+	};
+
 	int parseConfig(const YamlObject &tuningData);
 	int parseSingleConfig(const YamlObject &tuningData,
 			      rkisp1_cif_isp_dpf_config &config,
 			      rkisp1_cif_isp_dpf_strength_config &strengthConfig);
 
-	struct rkisp1_cif_isp_dpf_config config_;
-	struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
+	bool loadConfig(int32_t mode);
+
+	std::vector<ModeConfig> noiseReductionModes_;
+	std::vector<ModeConfig>::const_iterator activeMode_;
 };
 
 } /* namespace ipa::rkisp1::algorithms */