ipa: rpi: denoise: Support different denoise configurations

Some use cases may require stronger, or different, denosie settings to
others. For example, the way frames are accumulated during single
exposure HDR means that we may want stronger denoise.

This commit adds such support for different configurations that can be
defined in the tuning file.

Older tuning files, or files where there is only a single
configuration, load only the "normal" denoise configuration.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

src/ipa/rpi/controller/denoise_algorithm.h

@@ -6,6 +6,8 @@
  */
 #pragma once
 
+#include <string>
+
 #include "algorithm.h"
 
 namespace RPiController {
@@ -18,6 +20,8 @@ public:
 	DenoiseAlgorithm(Controller *controller) : Algorithm(controller) {}
 	/* A Denoise algorithm must provide the following: */
 	virtual void setMode(DenoiseMode mode) = 0;
+	/* Some platforms may not be able to define this, so supply a default. */
+	virtual void setConfig([[maybe_unused]] std::string const &name) {}
 };
 
 } /* namespace RPiController */

src/ipa/rpi/controller/rpi/denoise.cpp

@@ -21,6 +21,45 @@ LOG_DEFINE_CATEGORY(RPiDenoise)
 
 #define NAME "rpi.denoise"
 
+int DenoiseConfig::read(const libcamera::YamlObject &params)
+{
+	sdnEnable = params.contains("sdn");
+	if (sdnEnable) {
+		auto &sdnParams = params["sdn"];
+		sdnDeviation = sdnParams["deviation"].get<double>(3.2);
+		sdnStrength = sdnParams["strength"].get<double>(0.25);
+		sdnDeviation2 = sdnParams["deviation2"].get<double>(sdnDeviation);
+		sdnDeviationNoTdn = sdnParams["deviation_no_tdn"].get<double>(sdnDeviation);
+		sdnStrengthNoTdn = sdnParams["strength_no_tdn"].get<double>(sdnStrength);
+		sdnTdnBackoff = sdnParams["backoff"].get<double>(0.75);
+	}
+
+	cdnEnable = params.contains("cdn");
+	if (cdnEnable) {
+		auto &cdnParams = params["cdn"];
+		cdnDeviation = cdnParams["deviation"].get<double>(120);
+		cdnStrength = cdnParams["strength"].get<double>(0.2);
+	}
+
+	tdnEnable = params.contains("tdn");
+	if (tdnEnable) {
+		auto &tdnParams = params["tdn"];
+		tdnDeviation = tdnParams["deviation"].get<double>(0.5);
+		tdnThreshold = tdnParams["threshold"].get<double>(0.75);
+	} else if (sdnEnable) {
+		/*
+		 * If SDN is enabled but TDN isn't, overwrite all the SDN settings
+		 * with the "no TDN" versions. This makes it easier to enable or
+		 * disable TDN in the tuning file without editing all the other
+		 * parameters.
+		 */
+		sdnDeviation = sdnDeviation2 = sdnDeviationNoTdn;
+		sdnStrength = sdnStrengthNoTdn;
+	}
+
+	return 0;
+}
+
 Denoise::Denoise(Controller *controller)
 	: DenoiseAlgorithm(controller), mode_(DenoiseMode::ColourHighQuality)
 {
@@ -33,39 +72,26 @@ char const *Denoise::name() const
 
 int Denoise::read(const libcamera::YamlObject &params)
 {
-	sdnEnable_ = params.contains("sdn");
+	if (!params.contains("normal")) {
-	if (sdnEnable_) {
+		configs_["normal"].read(params);
-		auto &sdnParams = params["sdn"];
+		currentConfig_ = &configs_["normal"];
-		sdnDeviation_ = sdnParams["deviation"].get<double>(3.2);
+
-		sdnStrength_ = sdnParams["strength"].get<double>(0.25);
+		return 0;
-		sdnDeviation2_ = sdnParams["deviation2"].get<double>(sdnDeviation_);
-		sdnDeviationNoTdn_ = sdnParams["deviation_no_tdn"].get<double>(sdnDeviation_);
-		sdnStrengthNoTdn_ = sdnParams["strength_no_tdn"].get<double>(sdnStrength_);
-		sdnTdnBackoff_ = sdnParams["backoff"].get<double>(0.75);
 	}
 
-	cdnEnable_ = params.contains("cdn");
+	for (const auto &[key, value] : params.asDict()) {
-	if (cdnEnable_) {
+		if (configs_[key].read(value)) {
-		auto &cdnParams = params["cdn"];
+			LOG(RPiDenoise, Error) << "Failed to read denoise config " << key;
-		cdnDeviation_ = cdnParams["deviation"].get<double>(120);
+			return -EINVAL;
-		cdnStrength_ = cdnParams["strength"].get<double>(0.2);
+		}
 	}
 
-	tdnEnable_ = params.contains("tdn");
+	auto it = configs_.find("normal");
-	if (tdnEnable_) {
+	if (it == configs_.end()) {
-		auto &tdnParams = params["tdn"];
+		LOG(RPiDenoise, Error) << "No normal denoise settings found";
-		tdnDeviation_ = tdnParams["deviation"].get<double>(0.5);
+		return -EINVAL;
-		tdnThreshold_ = tdnParams["threshold"].get<double>(0.75);
-	} else if (sdnEnable_) {
-		/*
-		 * If SDN is enabled but TDN isn't, overwrite all the SDN settings
-		 * with the "no TDN" versions. This makes it easier to enable or
-		 * disable TDN in the tuning file without editing all the other
-		 * parameters.
-		 */
-		sdnDeviation_ = sdnDeviation2_ = sdnDeviationNoTdn_;
-		sdnStrength_ = sdnStrengthNoTdn_;
 	}
+	currentConfig_ = &it->second;
 
 	return 0;
 }
@@ -78,9 +104,9 @@ void Denoise::switchMode([[maybe_unused]] CameraMode const &cameraMode,
 			 [[maybe_unused]] Metadata *metadata)
 {
 	/* A mode switch effectively resets temporal denoise and it has to start over. */
-	currentSdnDeviation_ = sdnDeviationNoTdn_;
+	currentSdnDeviation_ = currentConfig_->sdnDeviationNoTdn;
-	currentSdnStrength_ = sdnStrengthNoTdn_;
+	currentSdnStrength_ = currentConfig_->sdnStrengthNoTdn;
-	currentSdnDeviation2_ = sdnDeviationNoTdn_;
+	currentSdnDeviation2_ = currentConfig_->sdnDeviationNoTdn;
 }
 
 void Denoise::prepare(Metadata *imageMetadata)
@@ -97,11 +123,11 @@ void Denoise::prepare(Metadata *imageMetadata)
 	if (mode_ == DenoiseMode::Off)
 		return;
 
-	if (sdnEnable_) {
+	if (currentConfig_->sdnEnable) {
 		struct SdnStatus sdn;
 		sdn.noiseConstant = noiseStatus.noiseConstant * currentSdnDeviation_;
 		sdn.noiseSlope = noiseStatus.noiseSlope * currentSdnDeviation_;
-		sdn.noiseConstant2 = noiseStatus.noiseConstant * sdnDeviation2_;
+		sdn.noiseConstant2 = noiseStatus.noiseConstant * currentConfig_->sdnDeviation2;
 		sdn.noiseSlope2 = noiseStatus.noiseSlope * currentSdnDeviation2_;
 		sdn.strength = currentSdnStrength_;
 		imageMetadata->set("sdn.status", sdn);
@@ -113,17 +139,17 @@ void Denoise::prepare(Metadata *imageMetadata)
 			<< " slope2 " << sdn.noiseSlope2;
 
 		/* For the next frame, we back off the SDN parameters as TDN ramps up. */
-		double f = sdnTdnBackoff_;
+		double f = currentConfig_->sdnTdnBackoff;
-		currentSdnDeviation_ = f * currentSdnDeviation_ + (1 - f) * sdnDeviation_;
+		currentSdnDeviation_ = f * currentSdnDeviation_ + (1 - f) * currentConfig_->sdnDeviation;
-		currentSdnStrength_ = f * currentSdnStrength_ + (1 - f) * sdnStrength_;
+		currentSdnStrength_ = f * currentSdnStrength_ + (1 - f) * currentConfig_->sdnStrength;
-		currentSdnDeviation2_ = f * currentSdnDeviation2_ + (1 - f) * sdnDeviation2_;
+		currentSdnDeviation2_ = f * currentSdnDeviation2_ + (1 - f) * currentConfig_->sdnDeviation2;
 	}
 
-	if (tdnEnable_) {
+	if (currentConfig_->tdnEnable) {
 		struct TdnStatus tdn;
-		tdn.noiseConstant = noiseStatus.noiseConstant * tdnDeviation_;
+		tdn.noiseConstant = noiseStatus.noiseConstant * currentConfig_->tdnDeviation;
-		tdn.noiseSlope = noiseStatus.noiseSlope * tdnDeviation_;
+		tdn.noiseSlope = noiseStatus.noiseSlope * currentConfig_->tdnDeviation;
-		tdn.threshold = tdnThreshold_;
+		tdn.threshold = currentConfig_->tdnThreshold;
 		imageMetadata->set("tdn.status", tdn);
 		LOG(RPiDenoise, Debug)
 			<< "programmed tdn threshold " << tdn.threshold
@@ -131,10 +157,10 @@ void Denoise::prepare(Metadata *imageMetadata)
 			<< " slope " << tdn.noiseSlope;
 	}
 
-	if (cdnEnable_ && mode_ != DenoiseMode::ColourOff) {
+	if (currentConfig_->cdnEnable && mode_ != DenoiseMode::ColourOff) {
 		struct CdnStatus cdn;
-		cdn.threshold = cdnDeviation_ * noiseStatus.noiseSlope + noiseStatus.noiseConstant;
+		cdn.threshold = currentConfig_->cdnDeviation * noiseStatus.noiseSlope + noiseStatus.noiseConstant;
-		cdn.strength = cdnStrength_;
+		cdn.strength = currentConfig_->cdnStrength;
 		imageMetadata->set("cdn.status", cdn);
 		LOG(RPiDenoise, Debug)
 			<< "programmed cdn threshold " << cdn.threshold
@@ -148,6 +174,22 @@ void Denoise::setMode(DenoiseMode mode)
 	mode_ = mode;
 }
 
+void Denoise::setConfig(std::string const &name)
+{
+	auto it = configs_.find(name);
+	if (it == configs_.end()) {
+		/*
+		 * Some platforms may have no need for different denoise settings, so we only issue
+		 * a warning if there clearly are several configurations.
+		 */
+		if (configs_.size() > 1)
+			LOG(RPiDenoise, Warning) << "No denoise config found for " << name;
+		else
+			LOG(RPiDenoise, Debug) << "No denoise config found for " << name;
+	} else
+		currentConfig_ = &it->second;
+}
+
 // Register algorithm with the system.
 static Algorithm *Create(Controller *controller)
 {

src/ipa/rpi/controller/rpi/denoise.h

@@ -6,6 +6,9 @@
  */
 #pragma once
 
+#include <map>
+#include <string>
+
 #include "algorithm.h"
 #include "denoise_algorithm.h"
 
@@ -13,6 +16,23 @@ namespace RPiController {
 
 // Algorithm to calculate correct denoise settings.
 
+struct DenoiseConfig {
+	double sdnDeviation;
+	double sdnStrength;
+	double sdnDeviation2;
+	double sdnDeviationNoTdn;
+	double sdnStrengthNoTdn;
+	double sdnTdnBackoff;
+	double cdnDeviation;
+	double cdnStrength;
+	double tdnDeviation;
+	double tdnThreshold;
+	bool tdnEnable;
+	bool sdnEnable;
+	bool cdnEnable;
+	int read(const libcamera::YamlObject &params);
+};
+
 class Denoise : public DenoiseAlgorithm
 {
 public:
@@ -23,22 +43,12 @@ public:
 	void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
 	void prepare(Metadata *imageMetadata) override;
 	void setMode(DenoiseMode mode) override;
+	void setConfig(std::string const &name) override;
 
 private:
-	double sdnDeviation_;
+	std::map<std::string, DenoiseConfig> configs_;
-	double sdnStrength_;
+	DenoiseConfig *currentConfig_;
-	double sdnDeviation2_;
-	double sdnDeviationNoTdn_;
-	double sdnStrengthNoTdn_;
-	double sdnTdnBackoff_;
-	double cdnDeviation_;
-	double cdnStrength_;
-	double tdnDeviation_;
-	double tdnThreshold_;
 	DenoiseMode mode_;
-	bool tdnEnable_;
-	bool sdnEnable_;
-	bool cdnEnable_;
 
 	/* SDN parameters attenuate over time if TDN is running. */
 	double currentSdnDeviation_;