[v6,9/9] ipa: rpi: awb: Make it possible to set the colour temperature directly
diff mbox series

Message ID 20241219175729.293782-10-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • rkisp1: Add manual colour temperature control
Related show

Commit Message

Stefan Klug Dec. 19, 2024, 5:57 p.m. UTC 
From: David Plowman <david.plowman@raspberrypi.com>

ColourTemperature is now exported as a writable control so that
applications can set it directly. The AWB algorithm class now requires
a method to be provided to perform this operation. The method should
clamp the passed value to the calibrated range known to the algorithm.

The default range is set very wide to cover all conceivable future AWB
calibrations. It will always be clamped before use.

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

--- Changes in v6:
- Added this commit from https://patchwork.libcamera.org/patch/19232/
---
 src/ipa/rpi/common/ipa_base.cpp        | 20 ++++++++++++++++++++
 src/ipa/rpi/controller/awb_algorithm.h |  1 +
 src/ipa/rpi/controller/rpi/awb.cpp     | 18 ++++++++++++++++++
 src/ipa/rpi/controller/rpi/awb.h       |  1 +
 4 files changed, 40 insertions(+)

Comments

Stefan Klug Dec. 20, 2024, 10:03 a.m. UTC | #1 
Hi David,

That patch is mostly unmodified. The only fix I had to apply was the
change from config_.ctR.domain().clip() to config_.ctR.domain().clamp().

So I guess you are fine with merging it?

Best regards,
Stefan

On Thu, Dec 19, 2024 at 06:57:26PM +0100, Stefan Klug wrote:
> From: David Plowman <david.plowman@raspberrypi.com>
> 
> ColourTemperature is now exported as a writable control so that
> applications can set it directly. The AWB algorithm class now requires
> a method to be provided to perform this operation. The method should
> clamp the passed value to the calibrated range known to the algorithm.
> 
> The default range is set very wide to cover all conceivable future AWB
> calibrations. It will always be clamped before use.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Tested-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> --- Changes in v6:
> - Added this commit from https://patchwork.libcamera.org/patch/19232/
> ---
>  src/ipa/rpi/common/ipa_base.cpp        | 20 ++++++++++++++++++++
>  src/ipa/rpi/controller/awb_algorithm.h |  1 +
>  src/ipa/rpi/controller/rpi/awb.cpp     | 18 ++++++++++++++++++
>  src/ipa/rpi/controller/rpi/awb.h       |  1 +
>  4 files changed, 40 insertions(+)
> 
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 45e2a1d7a6eb..0c8aee699155 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -81,6 +81,7 @@ const ControlInfoMap::Map ipaColourControls{
>  	{ &controls::AwbEnable, ControlInfo(false, true) },
>  	{ &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
>  	{ &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> +	{ &controls::ColourTemperature, ControlInfo(100, 100000) },
>  	{ &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
>  };
>  
> @@ -1011,6 +1012,25 @@ void IpaBase::applyControls(const ControlList &controls)
>  			break;
>  		}
>  
> +		case controls::COLOUR_TEMPERATURE: {
> +			/* Silently ignore this control for a mono sensor. */
> +			if (monoSensor_)
> +				break;
> +
> +			auto temperatureK = ctrl.second.get<int32_t>();
> +			RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
> +				controller_.getAlgorithm("awb"));
> +			if (!awb) {
> +				LOG(IPARPI, Warning)
> +					<< "Could not set COLOUR_TEMPERATURE - no AWB algorithm";
> +				break;
> +			}
> +
> +			awb->setColourTemperature(temperatureK);
> +			/* This metadata will get reported back automatically. */
> +			break;
> +		}
> +
>  		case controls::BRIGHTNESS: {
>  			RPiController::ContrastAlgorithm *contrast = dynamic_cast<RPiController::ContrastAlgorithm *>(
>  				controller_.getAlgorithm("contrast"));
> diff --git a/src/ipa/rpi/controller/awb_algorithm.h b/src/ipa/rpi/controller/awb_algorithm.h
> index 1779b0500a04..d941ed4e3476 100644
> --- a/src/ipa/rpi/controller/awb_algorithm.h
> +++ b/src/ipa/rpi/controller/awb_algorithm.h
> @@ -19,6 +19,7 @@ public:
>  	virtual void initialValues(double &gainR, double &gainB) = 0;
>  	virtual void setMode(std::string const &modeName) = 0;
>  	virtual void setManualGains(double manualR, double manualB) = 0;
> +	virtual void setColourTemperature(double temperatureK) = 0;
>  	virtual void enableAuto() = 0;
>  	virtual void disableAuto() = 0;
>  };
> diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
> index c277a1764112..8479ae40951d 100644
> --- a/src/ipa/rpi/controller/rpi/awb.cpp
> +++ b/src/ipa/rpi/controller/rpi/awb.cpp
> @@ -293,6 +293,24 @@ void Awb::setManualGains(double manualR, double manualB)
>  	}
>  }
>  
> +void Awb::setColourTemperature(double temperatureK)
> +{
> +	if (!config_.bayes) {
> +		LOG(RPiAwb, Warning) << "AWB uncalibrated - cannot set colour temperature";
> +		return;
> +	}
> +
> +	temperatureK = config_.ctR.domain().clamp(temperatureK);
> +	manualR_ = 1 / config_.ctR.eval(temperatureK);
> +	manualB_ = 1 / config_.ctB.eval(temperatureK);
> +
> +	syncResults_.temperatureK = temperatureK;
> +	syncResults_.gainR = manualR_;
> +	syncResults_.gainG = 1.0;
> +	syncResults_.gainB = manualB_;
> +	prevSyncResults_ = syncResults_;
> +}
> +
>  void Awb::switchMode([[maybe_unused]] CameraMode const &cameraMode,
>  		     Metadata *metadata)
>  {
> diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
> index 5d628b47c8a6..86640f8f8e21 100644
> --- a/src/ipa/rpi/controller/rpi/awb.h
> +++ b/src/ipa/rpi/controller/rpi/awb.h
> @@ -105,6 +105,7 @@ public:
>  	void initialValues(double &gainR, double &gainB) override;
>  	void setMode(std::string const &name) override;
>  	void setManualGains(double manualR, double manualB) override;
> +	void setColourTemperature(double temperatureK) override;
>  	void enableAuto() override;
>  	void disableAuto() override;
>  	void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> -- 
> 2.43.0
>

Patch
diff mbox series 

diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index 45e2a1d7a6eb..0c8aee699155 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -81,6 +81,7 @@  const ControlInfoMap::Map ipaColourControls{
 	{ &controls::AwbEnable, ControlInfo(false, true) },
 	{ &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
 	{ &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
+	{ &controls::ColourTemperature, ControlInfo(100, 100000) },
 	{ &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
 };
 
@@ -1011,6 +1012,25 @@  void IpaBase::applyControls(const ControlList &controls)
 			break;
 		}
 
+		case controls::COLOUR_TEMPERATURE: {
+			/* Silently ignore this control for a mono sensor. */
+			if (monoSensor_)
+				break;
+
+			auto temperatureK = ctrl.second.get<int32_t>();
+			RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
+				controller_.getAlgorithm("awb"));
+			if (!awb) {
+				LOG(IPARPI, Warning)
+					<< "Could not set COLOUR_TEMPERATURE - no AWB algorithm";
+				break;
+			}
+
+			awb->setColourTemperature(temperatureK);
+			/* This metadata will get reported back automatically. */
+			break;
+		}
+
 		case controls::BRIGHTNESS: {
 			RPiController::ContrastAlgorithm *contrast = dynamic_cast<RPiController::ContrastAlgorithm *>(
 				controller_.getAlgorithm("contrast"));
diff --git a/src/ipa/rpi/controller/awb_algorithm.h b/src/ipa/rpi/controller/awb_algorithm.h
index 1779b0500a04..d941ed4e3476 100644
--- a/src/ipa/rpi/controller/awb_algorithm.h
+++ b/src/ipa/rpi/controller/awb_algorithm.h
@@ -19,6 +19,7 @@  public:
 	virtual void initialValues(double &gainR, double &gainB) = 0;
 	virtual void setMode(std::string const &modeName) = 0;
 	virtual void setManualGains(double manualR, double manualB) = 0;
+	virtual void setColourTemperature(double temperatureK) = 0;
 	virtual void enableAuto() = 0;
 	virtual void disableAuto() = 0;
 };
diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
index c277a1764112..8479ae40951d 100644
--- a/src/ipa/rpi/controller/rpi/awb.cpp
+++ b/src/ipa/rpi/controller/rpi/awb.cpp
@@ -293,6 +293,24 @@  void Awb::setManualGains(double manualR, double manualB)
 	}
 }
 
+void Awb::setColourTemperature(double temperatureK)
+{
+	if (!config_.bayes) {
+		LOG(RPiAwb, Warning) << "AWB uncalibrated - cannot set colour temperature";
+		return;
+	}
+
+	temperatureK = config_.ctR.domain().clamp(temperatureK);
+	manualR_ = 1 / config_.ctR.eval(temperatureK);
+	manualB_ = 1 / config_.ctB.eval(temperatureK);
+
+	syncResults_.temperatureK = temperatureK;
+	syncResults_.gainR = manualR_;
+	syncResults_.gainG = 1.0;
+	syncResults_.gainB = manualB_;
+	prevSyncResults_ = syncResults_;
+}
+
 void Awb::switchMode([[maybe_unused]] CameraMode const &cameraMode,
 		     Metadata *metadata)
 {
diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
index 5d628b47c8a6..86640f8f8e21 100644
--- a/src/ipa/rpi/controller/rpi/awb.h
+++ b/src/ipa/rpi/controller/rpi/awb.h
@@ -105,6 +105,7 @@  public:
 	void initialValues(double &gainR, double &gainB) override;
 	void setMode(std::string const &name) override;
 	void setManualGains(double manualR, double manualB) override;
+	void setColourTemperature(double temperatureK) override;
 	void enableAuto() override;
 	void disableAuto() override;
 	void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;