[v3,12/16] ipa: rkisp1: Implement manual ColourCorrectionMatrix control
diff mbox series

Message ID 20250403154925.382973-13-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • Some rkisp1 awb improvements
Related show

Commit Message

Stefan Klug April 3, 2025, 3:49 p.m. UTC 
Add a manual ColourCorrectionMatrix control. This was already discussed
while implementing manual colour temperature but was never implemented.
The control allows to manually specify the CCM when AwbEnable is false.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---

Changes in v2:
- None

Changes in v3:
- Removed unnecessary log statement
- Improved debug logging for manual ccm
- Added documentation for IPAActiveState::ccm
- Added documentation for IPAFrameContext::ccm
---
 src/ipa/rkisp1/algorithms/ccm.cpp | 62 ++++++++++++++++++++++++++++---
 src/ipa/rkisp1/algorithms/ccm.h   |  6 +++
 src/ipa/rkisp1/ipa_context.cpp    | 19 ++++++++++
 src/ipa/rkisp1/ipa_context.h      |  3 +-
 4 files changed, 84 insertions(+), 6 deletions(-)

Comments

Paul Elder May 7, 2025, 5:20 p.m. UTC | #1 
On Thu, Apr 03, 2025 at 05:49:17PM +0200, Stefan Klug wrote:
> Add a manual ColourCorrectionMatrix control. This was already discussed
> while implementing manual colour temperature but was never implemented.
> The control allows to manually specify the CCM when AwbEnable is false.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> 
> ---
> 
> Changes in v2:
> - None
> 
> Changes in v3:
> - Removed unnecessary log statement
> - Improved debug logging for manual ccm
> - Added documentation for IPAActiveState::ccm
> - Added documentation for IPAFrameContext::ccm
> ---
>  src/ipa/rkisp1/algorithms/ccm.cpp | 62 ++++++++++++++++++++++++++++---
>  src/ipa/rkisp1/algorithms/ccm.h   |  6 +++
>  src/ipa/rkisp1/ipa_context.cpp    | 19 ++++++++++
>  src/ipa/rkisp1/ipa_context.h      |  3 +-
>  4 files changed, 84 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
> index 2e5e91006b55..3a96a5427bc6 100644
> --- a/src/ipa/rkisp1/algorithms/ccm.cpp
> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp
> @@ -36,17 +36,25 @@ namespace ipa::rkisp1::algorithms {
>  
>  LOG_DEFINE_CATEGORY(RkISP1Ccm)
>  
> +constexpr Matrix<float, 3, 3> kIdentity3x3 = Matrix<float, 3, 3>::identity();
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::init
>   */
>  int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
>  {
> +	auto &cmap = context.ctrlMap;
> +	cmap[&controls::ColourCorrectionMatrix] = ControlInfo(
> +		ControlValue(-8.0f),
> +		ControlValue(7.993f),
> +		ControlValue(kIdentity3x3.data()));
> +
>  	int ret = ccm_.readYaml(tuningData["ccms"], "ct", "ccm");
>  	if (ret < 0) {
>  		LOG(RkISP1Ccm, Warning)
>  			<< "Failed to parse 'ccm' "
>  			<< "parameter from tuning file; falling back to unit matrix";
> -		ccm_.setData({ { 0, Matrix<float, 3, 3>::identity() } });
> +		ccm_.setData({ { 0, kIdentity3x3 } });
>  	}
>  
>  	ret = offsets_.readYaml(tuningData["ccms"], "ct", "offsets");
> @@ -61,13 +69,51 @@ int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData
>  	return 0;
>  }
>  
> +/**
> + * \copydoc libcamera::ipa::Algorithm::configure
> + */
> +int Ccm::configure(IPAContext &context,
> +		   [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> +{
> +	auto &as = context.activeState;
> +	as.ccm.manual = kIdentity3x3;
> +	as.ccm.automatic = ccm_.getInterpolated(as.awb.automatic.temperatureK);
> +	return 0;
> +}
> +
> +void Ccm::queueRequest(IPAContext &context,
> +		       [[maybe_unused]] const uint32_t frame,
> +		       IPAFrameContext &frameContext,
> +		       const ControlList &controls)
> +{
> +	/* Nothing to do here, the ccm will be calculated in prepare() */
> +	if (frameContext.awb.autoEnabled)
> +		return;
> +
> +	auto &ccm = context.activeState.ccm;
> +
> +	const auto &colourTemperature = controls.get(controls::ColourTemperature);
> +	const auto &ccmMatrix = controls.get(controls::ColourCorrectionMatrix);
> +	if (ccmMatrix) {
> +		ccm.manual = Matrix<float, 3, 3>(*ccmMatrix);
> +		LOG(RkISP1Ccm, Debug)
> +			<< "Setting manual CCM from CCM control to " << ccm.manual;
> +	} else if (colourTemperature) {
> +		ccm.manual = ccm_.getInterpolated(*colourTemperature);
> +		LOG(RkISP1Ccm, Debug)
> +			<< "Setting manual CCM from CT control to " << ccm.manual;
> +	}
> +
> +	frameContext.ccm.ccm = ccm.manual;
> +}
> +
>  void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
>  			const Matrix<float, 3, 3> &matrix,
>  			const Matrix<int16_t, 3, 1> &offsets)
>  {
>  	/*
>  	 * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to
> -	 * +7.992 (0x3ff)
> +	 * +7.9921875 (0x3ff)
>  	 */
>  	for (unsigned int i = 0; i < 3; i++) {
>  		for (unsigned int j = 0; j < 3; j++)
> @@ -88,14 +134,20 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
>  void Ccm::prepare(IPAContext &context, const uint32_t frame,
>  		  IPAFrameContext &frameContext, RkISP1Params *params)
>  {
> -	uint32_t ct = frameContext.awb.temperatureK;
> +	if (!frameContext.awb.autoEnabled) {
> +		auto config = params->block<BlockType::Ctk>();
> +		config.setEnabled(true);
> +		setParameters(*config, frameContext.ccm.ccm, Matrix<int16_t, 3, 1>());
> +		return;
> +	}
>  
> +	uint32_t ct = frameContext.awb.temperatureK;
>  	/*
>  	 * \todo The colour temperature will likely be noisy, add filtering to
>  	 * avoid updating the CCM matrix all the time.
>  	 */
>  	if (frame > 0 && ct == ct_) {
> -		frameContext.ccm.ccm = context.activeState.ccm.ccm;
> +		frameContext.ccm.ccm = context.activeState.ccm.automatic;
>  		return;
>  	}
>  
> @@ -103,7 +155,7 @@ void Ccm::prepare(IPAContext &context, const uint32_t frame,
>  	Matrix<float, 3, 3> ccm = ccm_.getInterpolated(ct);
>  	Matrix<int16_t, 3, 1> offsets = offsets_.getInterpolated(ct);
>  
> -	context.activeState.ccm.ccm = ccm;
> +	context.activeState.ccm.automatic = ccm;
>  	frameContext.ccm.ccm = ccm;
>  
>  	auto config = params->block<BlockType::Ctk>();
> diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h
> index a5d9a9a45e5d..c301e6e531c8 100644
> --- a/src/ipa/rkisp1/algorithms/ccm.h
> +++ b/src/ipa/rkisp1/algorithms/ccm.h
> @@ -26,6 +26,12 @@ public:
>  	~Ccm() = default;
>  
>  	int init(IPAContext &context, const YamlObject &tuningData) override;
> +	int configure(IPAContext &context,
> +		      const IPACameraSensorInfo &configInfo) override;
> +	void queueRequest(IPAContext &context,
> +			  const uint32_t frame,
> +			  IPAFrameContext &frameContext,
> +			  const ControlList &controls) override;
>  	void prepare(IPAContext &context, const uint32_t frame,
>  		     IPAFrameContext &frameContext,
>  		     RkISP1Params *params) override;
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 7bc42e6de415..cec73c9610da 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -210,6 +210,17 @@ namespace libcamera::ipa::rkisp1 {
>   * \brief Whether the Auto White Balance algorithm is enabled
>   */
>  
> +/**
> + * \var IPAActiveState::ccm
> + * \brief State for the Colour Correction Matrix algorithm
> + *
> + * \var IPAActiveState::ccm.manual
> + * \brief Manual CCM (set through requests)
> + *
> + * \var IPAActiveState::awb.automatic
> + * \brief Automatic CCM (computed by the algorithm)
> + */
> +
>  /**
>   * \var IPAActiveState::cproc
>   * \brief State for the Color Processing algorithm
> @@ -355,6 +366,14 @@ namespace libcamera::ipa::rkisp1 {
>   * \brief Whether the Auto White Balance algorithm is enabled
>   */
>  
> +/**
> + * \var IPAFrameContext::ccm
> + * \brief Colour Correction Matrix parameters for this frame
> + *
> + * \struct IPAFrameContext::ccm.ccm
> + * \brief Colour Correction Matrix
> + */
> +
>  /**
>   * \var IPAFrameContext::cproc
>   * \brief Color Processing parameters for this frame
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 769e9f114e23..f0d504215d34 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -101,7 +101,8 @@ struct IPAActiveState {
>  	} awb;
>  
>  	struct {
> -		Matrix<float, 3, 3> ccm;
> +		Matrix<float, 3, 3> manual;
> +		Matrix<float, 3, 3> automatic;
>  	} ccm;
>  
>  	struct {
> -- 
> 2.43.0
>

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
index 2e5e91006b55..3a96a5427bc6 100644
--- a/src/ipa/rkisp1/algorithms/ccm.cpp
+++ b/src/ipa/rkisp1/algorithms/ccm.cpp
@@ -36,17 +36,25 @@  namespace ipa::rkisp1::algorithms {
 
 LOG_DEFINE_CATEGORY(RkISP1Ccm)
 
+constexpr Matrix<float, 3, 3> kIdentity3x3 = Matrix<float, 3, 3>::identity();
+
 /**
  * \copydoc libcamera::ipa::Algorithm::init
  */
 int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
 {
+	auto &cmap = context.ctrlMap;
+	cmap[&controls::ColourCorrectionMatrix] = ControlInfo(
+		ControlValue(-8.0f),
+		ControlValue(7.993f),
+		ControlValue(kIdentity3x3.data()));
+
 	int ret = ccm_.readYaml(tuningData["ccms"], "ct", "ccm");
 	if (ret < 0) {
 		LOG(RkISP1Ccm, Warning)
 			<< "Failed to parse 'ccm' "
 			<< "parameter from tuning file; falling back to unit matrix";
-		ccm_.setData({ { 0, Matrix<float, 3, 3>::identity() } });
+		ccm_.setData({ { 0, kIdentity3x3 } });
 	}
 
 	ret = offsets_.readYaml(tuningData["ccms"], "ct", "offsets");
@@ -61,13 +69,51 @@  int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData
 	return 0;
 }
 
+/**
+ * \copydoc libcamera::ipa::Algorithm::configure
+ */
+int Ccm::configure(IPAContext &context,
+		   [[maybe_unused]] const IPACameraSensorInfo &configInfo)
+{
+	auto &as = context.activeState;
+	as.ccm.manual = kIdentity3x3;
+	as.ccm.automatic = ccm_.getInterpolated(as.awb.automatic.temperatureK);
+	return 0;
+}
+
+void Ccm::queueRequest(IPAContext &context,
+		       [[maybe_unused]] const uint32_t frame,
+		       IPAFrameContext &frameContext,
+		       const ControlList &controls)
+{
+	/* Nothing to do here, the ccm will be calculated in prepare() */
+	if (frameContext.awb.autoEnabled)
+		return;
+
+	auto &ccm = context.activeState.ccm;
+
+	const auto &colourTemperature = controls.get(controls::ColourTemperature);
+	const auto &ccmMatrix = controls.get(controls::ColourCorrectionMatrix);
+	if (ccmMatrix) {
+		ccm.manual = Matrix<float, 3, 3>(*ccmMatrix);
+		LOG(RkISP1Ccm, Debug)
+			<< "Setting manual CCM from CCM control to " << ccm.manual;
+	} else if (colourTemperature) {
+		ccm.manual = ccm_.getInterpolated(*colourTemperature);
+		LOG(RkISP1Ccm, Debug)
+			<< "Setting manual CCM from CT control to " << ccm.manual;
+	}
+
+	frameContext.ccm.ccm = ccm.manual;
+}
+
 void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
 			const Matrix<float, 3, 3> &matrix,
 			const Matrix<int16_t, 3, 1> &offsets)
 {
 	/*
 	 * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to
-	 * +7.992 (0x3ff)
+	 * +7.9921875 (0x3ff)
 	 */
 	for (unsigned int i = 0; i < 3; i++) {
 		for (unsigned int j = 0; j < 3; j++)
@@ -88,14 +134,20 @@  void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
 void Ccm::prepare(IPAContext &context, const uint32_t frame,
 		  IPAFrameContext &frameContext, RkISP1Params *params)
 {
-	uint32_t ct = frameContext.awb.temperatureK;
+	if (!frameContext.awb.autoEnabled) {
+		auto config = params->block<BlockType::Ctk>();
+		config.setEnabled(true);
+		setParameters(*config, frameContext.ccm.ccm, Matrix<int16_t, 3, 1>());
+		return;
+	}
 
+	uint32_t ct = frameContext.awb.temperatureK;
 	/*
 	 * \todo The colour temperature will likely be noisy, add filtering to
 	 * avoid updating the CCM matrix all the time.
 	 */
 	if (frame > 0 && ct == ct_) {
-		frameContext.ccm.ccm = context.activeState.ccm.ccm;
+		frameContext.ccm.ccm = context.activeState.ccm.automatic;
 		return;
 	}
 
@@ -103,7 +155,7 @@  void Ccm::prepare(IPAContext &context, const uint32_t frame,
 	Matrix<float, 3, 3> ccm = ccm_.getInterpolated(ct);
 	Matrix<int16_t, 3, 1> offsets = offsets_.getInterpolated(ct);
 
-	context.activeState.ccm.ccm = ccm;
+	context.activeState.ccm.automatic = ccm;
 	frameContext.ccm.ccm = ccm;
 
 	auto config = params->block<BlockType::Ctk>();
diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h
index a5d9a9a45e5d..c301e6e531c8 100644
--- a/src/ipa/rkisp1/algorithms/ccm.h
+++ b/src/ipa/rkisp1/algorithms/ccm.h
@@ -26,6 +26,12 @@  public:
 	~Ccm() = default;
 
 	int init(IPAContext &context, const YamlObject &tuningData) override;
+	int configure(IPAContext &context,
+		      const IPACameraSensorInfo &configInfo) override;
+	void queueRequest(IPAContext &context,
+			  const uint32_t frame,
+			  IPAFrameContext &frameContext,
+			  const ControlList &controls) override;
 	void prepare(IPAContext &context, const uint32_t frame,
 		     IPAFrameContext &frameContext,
 		     RkISP1Params *params) override;
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index 7bc42e6de415..cec73c9610da 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -210,6 +210,17 @@  namespace libcamera::ipa::rkisp1 {
  * \brief Whether the Auto White Balance algorithm is enabled
  */
 
+/**
+ * \var IPAActiveState::ccm
+ * \brief State for the Colour Correction Matrix algorithm
+ *
+ * \var IPAActiveState::ccm.manual
+ * \brief Manual CCM (set through requests)
+ *
+ * \var IPAActiveState::awb.automatic
+ * \brief Automatic CCM (computed by the algorithm)
+ */
+
 /**
  * \var IPAActiveState::cproc
  * \brief State for the Color Processing algorithm
@@ -355,6 +366,14 @@  namespace libcamera::ipa::rkisp1 {
  * \brief Whether the Auto White Balance algorithm is enabled
  */
 
+/**
+ * \var IPAFrameContext::ccm
+ * \brief Colour Correction Matrix parameters for this frame
+ *
+ * \struct IPAFrameContext::ccm.ccm
+ * \brief Colour Correction Matrix
+ */
+
 /**
  * \var IPAFrameContext::cproc
  * \brief Color Processing parameters for this frame
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 769e9f114e23..f0d504215d34 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -101,7 +101,8 @@  struct IPAActiveState {
 	} awb;
 
 	struct {
-		Matrix<float, 3, 3> ccm;
+		Matrix<float, 3, 3> manual;
+		Matrix<float, 3, 3> automatic;
 	} ccm;
 
 	struct {