ipa: rpi: ccm: Implement "manual" CCM mode
diff mbox series

Message ID 20250804111118.10190-1-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • ipa: rpi: ccm: Implement "manual" CCM mode
Related show

Commit Message

David Plowman Aug. 4, 2025, 11:11 a.m. UTC 
The CCM algorithm will now let an explicit colour matrix be set when
AWB is in manual mode.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/rpi/common/ipa_base.cpp        | 67 +++++++++++++++++++++++---
 src/ipa/rpi/common/ipa_base.h          |  2 +
 src/ipa/rpi/controller/ccm_algorithm.h |  6 +++
 src/ipa/rpi/controller/rpi/ccm.cpp     | 21 +++++++-
 src/ipa/rpi/controller/rpi/ccm.h       |  5 +-
 5 files changed, 92 insertions(+), 9 deletions(-)

Comments

David Plowman Aug. 4, 2025, 12:52 p.m. UTC | #1 
There's some stuff wrong here. Because our CCM algorithm is
independent of the AWB, the coupling between the two (AWB must be off
for a manual CCM) doesn't really work. It's not really what I would
have wanted, either (why can't you have a fixed CCM just because AWB
is running?), but I suppose I can implement what is documented.

I'm going to have to post a v2.

David

On Mon, 4 Aug 2025 at 12:11, David Plowman
<david.plowman@raspberrypi.com> wrote:
>
> The CCM algorithm will now let an explicit colour matrix be set when
> AWB is in manual mode.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/rpi/common/ipa_base.cpp        | 67 +++++++++++++++++++++++---
>  src/ipa/rpi/common/ipa_base.h          |  2 +
>  src/ipa/rpi/controller/ccm_algorithm.h |  6 +++
>  src/ipa/rpi/controller/rpi/ccm.cpp     | 21 +++++++-
>  src/ipa/rpi/controller/rpi/ccm.h       |  5 +-
>  5 files changed, 92 insertions(+), 9 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index ce2343e9..3153bee6 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{
>         { &controls::AwbEnable, ControlInfo(false, true) },
>         { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
>         { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> +       { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },
>         { &controls::ColourTemperature, ControlInfo(100, 100000) },
>         { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
>  };
> @@ -126,7 +127,7 @@ namespace ipa::RPi {
>  IpaBase::IpaBase()
>         : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),
>           stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),
> -         firstStart_(true), flickerState_({ 0, 0s })
> +         firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)
>  {
>  }
>
> @@ -1049,13 +1050,21 @@ void IpaBase::applyControls(const ControlList &controls)
>                                 break;
>                         }
>
> -                       if (ctrl.second.get<bool>() == false)
> +                       awbEnabled_ = ctrl.second.get<bool>();
> +
> +                       if (!awbEnabled_)
>                                 awb->disableAuto();
> -                       else
> +                       else {
>                                 awb->enableAuto();
>
> -                       libcameraMetadata_.set(controls::AwbEnable,
> -                                              ctrl.second.get<bool>());
> +                               /* If AWB is running, the CCM must go back to auto as well. */
> +                               RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(
> +                                       controller_.getAlgorithm("ccm"));
> +                               if (ccm)
> +                                       ccm->enableAuto();
> +                       }
> +
> +                       libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);
>                         break;
>                 }
>
> @@ -1098,10 +1107,20 @@ void IpaBase::applyControls(const ControlList &controls)
>                         }
>
>                         awb->setManualGains(gains[0], gains[1]);
> -                       if (gains[0] != 0.0f && gains[1] != 0.0f)
> +                       if (gains[0] != 0.0f && gains[1] != 0.0f) {
>                                 /* A gain of 0.0f will switch back to auto mode. */
>                                 libcameraMetadata_.set(controls::ColourGains,
>                                                        { gains[0], gains[1] });
> +                               awbEnabled_ = false; /* puts AWB into manual mode */
> +                       } else {
> +                               awbEnabled_ = true; /* puts AWB into auto mode */
> +
> +                               /* If AWB is running, the CCM must go back to auto as well. */
> +                               RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(
> +                                       controller_.getAlgorithm("ccm"));
> +                               if (ccm)
> +                                       ccm->enableAuto();
> +                       }
>                         break;
>                 }
>
> @@ -1120,10 +1139,46 @@ void IpaBase::applyControls(const ControlList &controls)
>                         }
>
>                         awb->setColourTemperature(temperatureK);
> +                       awbEnabled_ = false; /* this puts AWB into manual mode */
>                         /* This metadata will get reported back automatically. */
>                         break;
>                 }
>
> +               case controls::COLOUR_CORRECTION_MATRIX: {
> +                       if (monoSensor_)
> +                               break;
> +
> +                       auto floats = ctrl.second.get<Span<const float>>();
> +                       RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(
> +                               controller_.getAlgorithm("ccm"));
> +                       if (!ccm) {
> +                               LOG(IPARPI, Warning)
> +                                       << "Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm";
> +                               break;
> +                       }
> +
> +                       /* We are guaranteed this control contains 9 values. Nevertheless: */
> +                       assert(floats.size() == 9);
> +
> +                       Matrix<double, 3, 3> matrix;
> +                       for (std::size_t i = 0; i < 3; ++i)
> +                               for (std::size_t j = 0; j < 3; ++j)
> +                                       matrix[i][j] = static_cast<double>(floats[i * 3 + j]);
> +
> +                       ccm->setCcm(matrix);
> +
> +                       /*
> +                        * But if AWB is running, go back to auto mode. The CCM gets remembered,
> +                        * which avoids the race between setting the CCM and disabling AWB in
> +                        * the same set of controls.
> +                        */
> +                       if (awbEnabled_)
> +                               ccm->enableAuto();
> +
> +                       /* This metadata will be reported back automatically. */
> +                       break;
> +               }
> +
>                 case controls::BRIGHTNESS: {
>                         RPiController::ContrastAlgorithm *contrast = dynamic_cast<RPiController::ContrastAlgorithm *>(
>                                 controller_.getAlgorithm("contrast"));
> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
> index e2f6e330..5348f2ea 100644
> --- a/src/ipa/rpi/common/ipa_base.h
> +++ b/src/ipa/rpi/common/ipa_base.h
> @@ -140,6 +140,8 @@ private:
>                 int32_t mode;
>                 utils::Duration manualPeriod;
>         } flickerState_;
> +
> +       bool awbEnabled_;
>  };
>
>  } /* namespace ipa::RPi */
> diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h
> index 6678ba75..785c408a 100644
> --- a/src/ipa/rpi/controller/ccm_algorithm.h
> +++ b/src/ipa/rpi/controller/ccm_algorithm.h
> @@ -6,16 +6,22 @@
>   */
>  #pragma once
>
> +#include "libcamera/internal/matrix.h"
> +
>  #include "algorithm.h"
>
>  namespace RPiController {
>
> +using Matrix3x3 = libcamera::Matrix<double, 3, 3>;
> +
>  class CcmAlgorithm : public Algorithm
>  {
>  public:
>         CcmAlgorithm(Controller *controller) : Algorithm(controller) {}
>         /* A CCM algorithm must provide the following: */
> +       virtual void enableAuto() = 0;
>         virtual void setSaturation(double saturation) = 0;
> +       virtual void setCcm(Matrix3x3 const &matrix) = 0;
>  };
>
>  } /* namespace RPiController */
> diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp
> index 8607f152..25bcea2d 100644
> --- a/src/ipa/rpi/controller/rpi/ccm.cpp
> +++ b/src/ipa/rpi/controller/rpi/ccm.cpp
> @@ -32,7 +32,7 @@ LOG_DEFINE_CATEGORY(RPiCcm)
>  using Matrix3x3 = Matrix<double, 3, 3>;
>
>  Ccm::Ccm(Controller *controller)
> -       : CcmAlgorithm(controller), saturation_(1.0) {}
> +       : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0) {}
>
>  char const *Ccm::name() const
>  {
> @@ -78,11 +78,22 @@ int Ccm::read(const libcamera::YamlObject &params)
>         return 0;
>  }
>
> +void Ccm::enableAuto()
> +{
> +       enableAuto_ = true;
> +}
> +
>  void Ccm::setSaturation(double saturation)
>  {
>         saturation_ = saturation;
>  }
>
> +void Ccm::setCcm(Matrix3x3 const &matrix)
> +{
> +       enableAuto_ = false;
> +       manualCcm_ = matrix;
> +}
> +
>  void Ccm::initialise()
>  {
>  }
> @@ -151,7 +162,13 @@ void Ccm::prepare(Metadata *imageMetadata)
>                 LOG(RPiCcm, Warning) << "no colour temperature found";
>         if (!luxOk)
>                 LOG(RPiCcm, Warning) << "no lux value found";
> -       Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);
> +
> +       Matrix3x3 ccm;
> +       if (enableAuto_)
> +               ccm = calculateCcm(config_.ccms, awb.temperatureK);
> +       else
> +               ccm = manualCcm_;
> +
>         double saturation = saturation_;
>         struct CcmStatus ccmStatus;
>         ccmStatus.saturation = saturation;
> diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h
> index c05dbb17..70f28ed3 100644
> --- a/src/ipa/rpi/controller/rpi/ccm.h
> +++ b/src/ipa/rpi/controller/rpi/ccm.h
> @@ -8,7 +8,6 @@
>
>  #include <vector>
>
> -#include "libcamera/internal/matrix.h"
>  #include <libipa/pwl.h>
>
>  #include "../ccm_algorithm.h"
> @@ -33,13 +32,17 @@ public:
>         Ccm(Controller *controller = NULL);
>         char const *name() const override;
>         int read(const libcamera::YamlObject &params) override;
> +       void enableAuto() override;
>         void setSaturation(double saturation) override;
> +       void setCcm(Matrix3x3 const &matrix) override;
>         void initialise() override;
>         void prepare(Metadata *imageMetadata) override;
>
>  private:
>         CcmConfig config_;
> +       bool enableAuto_;
>         double saturation_;
> +       Matrix3x3 manualCcm_;
>  };
>
>  } /* namespace RPiController */
> --
> 2.39.5
>

Patch
diff mbox series 

diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index ce2343e9..3153bee6 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -94,6 +94,7 @@  const ControlInfoMap::Map ipaColourControls{
 	{ &controls::AwbEnable, ControlInfo(false, true) },
 	{ &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
 	{ &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
+	{ &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },
 	{ &controls::ColourTemperature, ControlInfo(100, 100000) },
 	{ &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
 };
@@ -126,7 +127,7 @@  namespace ipa::RPi {
 IpaBase::IpaBase()
 	: controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),
 	  stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),
-	  firstStart_(true), flickerState_({ 0, 0s })
+	  firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)
 {
 }
 
@@ -1049,13 +1050,21 @@  void IpaBase::applyControls(const ControlList &controls)
 				break;
 			}
 
-			if (ctrl.second.get<bool>() == false)
+			awbEnabled_ = ctrl.second.get<bool>();
+
+			if (!awbEnabled_)
 				awb->disableAuto();
-			else
+			else {
 				awb->enableAuto();
 
-			libcameraMetadata_.set(controls::AwbEnable,
-					       ctrl.second.get<bool>());
+				/* If AWB is running, the CCM must go back to auto as well. */
+				RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(
+					controller_.getAlgorithm("ccm"));
+				if (ccm)
+					ccm->enableAuto();
+			}
+
+			libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);
 			break;
 		}
 
@@ -1098,10 +1107,20 @@  void IpaBase::applyControls(const ControlList &controls)
 			}
 
 			awb->setManualGains(gains[0], gains[1]);
-			if (gains[0] != 0.0f && gains[1] != 0.0f)
+			if (gains[0] != 0.0f && gains[1] != 0.0f) {
 				/* A gain of 0.0f will switch back to auto mode. */
 				libcameraMetadata_.set(controls::ColourGains,
 						       { gains[0], gains[1] });
+				awbEnabled_ = false; /* puts AWB into manual mode */
+			} else {
+				awbEnabled_ = true; /* puts AWB into auto mode */
+
+				/* If AWB is running, the CCM must go back to auto as well. */
+				RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(
+					controller_.getAlgorithm("ccm"));
+				if (ccm)
+					ccm->enableAuto();
+			}
 			break;
 		}
 
@@ -1120,10 +1139,46 @@  void IpaBase::applyControls(const ControlList &controls)
 			}
 
 			awb->setColourTemperature(temperatureK);
+			awbEnabled_ = false; /* this puts AWB into manual mode */
 			/* This metadata will get reported back automatically. */
 			break;
 		}
 
+		case controls::COLOUR_CORRECTION_MATRIX: {
+			if (monoSensor_)
+				break;
+
+			auto floats = ctrl.second.get<Span<const float>>();
+			RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(
+				controller_.getAlgorithm("ccm"));
+			if (!ccm) {
+				LOG(IPARPI, Warning)
+					<< "Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm";
+				break;
+			}
+
+			/* We are guaranteed this control contains 9 values. Nevertheless: */
+			assert(floats.size() == 9);
+
+			Matrix<double, 3, 3> matrix;
+			for (std::size_t i = 0; i < 3; ++i)
+				for (std::size_t j = 0; j < 3; ++j)
+					matrix[i][j] = static_cast<double>(floats[i * 3 + j]);
+
+			ccm->setCcm(matrix);
+
+			/*
+			 * But if AWB is running, go back to auto mode. The CCM gets remembered,
+			 * which avoids the race between setting the CCM and disabling AWB in
+			 * the same set of controls.
+			 */
+			if (awbEnabled_)
+				ccm->enableAuto();
+
+			/* This metadata will be reported back automatically. */
+			break;
+		}
+
 		case controls::BRIGHTNESS: {
 			RPiController::ContrastAlgorithm *contrast = dynamic_cast<RPiController::ContrastAlgorithm *>(
 				controller_.getAlgorithm("contrast"));
diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
index e2f6e330..5348f2ea 100644
--- a/src/ipa/rpi/common/ipa_base.h
+++ b/src/ipa/rpi/common/ipa_base.h
@@ -140,6 +140,8 @@  private:
 		int32_t mode;
 		utils::Duration manualPeriod;
 	} flickerState_;
+
+	bool awbEnabled_;
 };
 
 } /* namespace ipa::RPi */
diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h
index 6678ba75..785c408a 100644
--- a/src/ipa/rpi/controller/ccm_algorithm.h
+++ b/src/ipa/rpi/controller/ccm_algorithm.h
@@ -6,16 +6,22 @@ 
  */
 #pragma once
 
+#include "libcamera/internal/matrix.h"
+
 #include "algorithm.h"
 
 namespace RPiController {
 
+using Matrix3x3 = libcamera::Matrix<double, 3, 3>;
+
 class CcmAlgorithm : public Algorithm
 {
 public:
 	CcmAlgorithm(Controller *controller) : Algorithm(controller) {}
 	/* A CCM algorithm must provide the following: */
+	virtual void enableAuto() = 0;
 	virtual void setSaturation(double saturation) = 0;
+	virtual void setCcm(Matrix3x3 const &matrix) = 0;
 };
 
 } /* namespace RPiController */
diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp
index 8607f152..25bcea2d 100644
--- a/src/ipa/rpi/controller/rpi/ccm.cpp
+++ b/src/ipa/rpi/controller/rpi/ccm.cpp
@@ -32,7 +32,7 @@  LOG_DEFINE_CATEGORY(RPiCcm)
 using Matrix3x3 = Matrix<double, 3, 3>;
 
 Ccm::Ccm(Controller *controller)
-	: CcmAlgorithm(controller), saturation_(1.0) {}
+	: CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0) {}
 
 char const *Ccm::name() const
 {
@@ -78,11 +78,22 @@  int Ccm::read(const libcamera::YamlObject &params)
 	return 0;
 }
 
+void Ccm::enableAuto()
+{
+	enableAuto_ = true;
+}
+
 void Ccm::setSaturation(double saturation)
 {
 	saturation_ = saturation;
 }
 
+void Ccm::setCcm(Matrix3x3 const &matrix)
+{
+	enableAuto_ = false;
+	manualCcm_ = matrix;
+}
+
 void Ccm::initialise()
 {
 }
@@ -151,7 +162,13 @@  void Ccm::prepare(Metadata *imageMetadata)
 		LOG(RPiCcm, Warning) << "no colour temperature found";
 	if (!luxOk)
 		LOG(RPiCcm, Warning) << "no lux value found";
-	Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);
+
+	Matrix3x3 ccm;
+	if (enableAuto_)
+		ccm = calculateCcm(config_.ccms, awb.temperatureK);
+	else
+		ccm = manualCcm_;
+
 	double saturation = saturation_;
 	struct CcmStatus ccmStatus;
 	ccmStatus.saturation = saturation;
diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h
index c05dbb17..70f28ed3 100644
--- a/src/ipa/rpi/controller/rpi/ccm.h
+++ b/src/ipa/rpi/controller/rpi/ccm.h
@@ -8,7 +8,6 @@ 
 
 #include <vector>
 
-#include "libcamera/internal/matrix.h"
 #include <libipa/pwl.h>
 
 #include "../ccm_algorithm.h"
@@ -33,13 +32,17 @@  public:
 	Ccm(Controller *controller = NULL);
 	char const *name() const override;
 	int read(const libcamera::YamlObject &params) override;
+	void enableAuto() override;
 	void setSaturation(double saturation) override;
+	void setCcm(Matrix3x3 const &matrix) override;
 	void initialise() override;
 	void prepare(Metadata *imageMetadata) override;
 
 private:
 	CcmConfig config_;
+	bool enableAuto_;
 	double saturation_;
+	Matrix3x3 manualCcm_;
 };
 
 } /* namespace RPiController */