[v2,6/9] ipa: rpi: ccm: Replace local matrix implementation with the one from libcamera
diff mbox series

Message ID 20241119103740.1919807-7-stefan.klug@ideasonboard.com
State Superseded
Headers show
Series
  • ove Matrix class from libipa to libcamera
Related show

Commit Message

Stefan Klug Nov. 19, 2024, 10:37 a.m. UTC 
The RaspberryPi IPA contains a private Matrix3x3 class inside the ccm
algorithm. Replace it with the Matrix class available in
libcamera/internal.

While at it, mark the matrices RGB2Y and Y2RGB as static const.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/rpi/controller/rpi/ccm.cpp | 52 ++++++++----------------------
 src/ipa/rpi/controller/rpi/ccm.h   | 35 ++------------------
 2 files changed, 15 insertions(+), 72 deletions(-)

Comments

Laurent Pinchart Nov. 19, 2024, 11:41 a.m. UTC | #1 
Hi Stefan,

Thank you for the patch.

On Tue, Nov 19, 2024 at 11:37:33AM +0100, Stefan Klug wrote:
> The RaspberryPi IPA contains a private Matrix3x3 class inside the ccm
> algorithm. Replace it with the Matrix class available in
> libcamera/internal.
> 
> While at it, mark the matrices RGB2Y and Y2RGB as static const.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  src/ipa/rpi/controller/rpi/ccm.cpp | 52 ++++++++----------------------
>  src/ipa/rpi/controller/rpi/ccm.h   | 35 ++------------------
>  2 files changed, 15 insertions(+), 72 deletions(-)
> 
> diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp
> index 7f63f3fdb702..2f40cb4e09ee 100644
> --- a/src/ipa/rpi/controller/rpi/ccm.cpp
> +++ b/src/ipa/rpi/controller/rpi/ccm.cpp
> @@ -29,34 +29,7 @@ LOG_DEFINE_CATEGORY(RPiCcm)
>  
>  #define NAME "rpi.ccm"
>  
> -Matrix3x3::Matrix3x3()
> -{
> -	memset(m, 0, sizeof(m));
> -}
> -Matrix3x3::Matrix3x3(double m0, double m1, double m2, double m3, double m4, double m5,
> -	       double m6, double m7, double m8)
> -{
> -	m[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] = m4,
> -	m[1][2] = m5, m[2][0] = m6, m[2][1] = m7, m[2][2] = m8;
> -}
> -int Matrix3x3::read(const libcamera::YamlObject &params)
> -{
> -	double *ptr = (double *)m;
> -
> -	if (params.size() != 9) {
> -		LOG(RPiCcm, Error) << "Wrong number of values in CCM";
> -		return -EINVAL;
> -	}
> -
> -	for (const auto &param : params.asList()) {
> -		auto value = param.get<double>();
> -		if (!value)
> -			return -EINVAL;
> -		*ptr++ = *value;
> -	}
> -
> -	return 0;
> -}
> +using Matrix3x3 = Matrix<double, 3, 3>;
>  
>  Ccm::Ccm(Controller *controller)
>  	: CcmAlgorithm(controller), saturation_(1.0) {}
> @@ -68,8 +41,6 @@ char const *Ccm::name() const
>  
>  int Ccm::read(const libcamera::YamlObject &params)
>  {
> -	int ret;
> -
>  	if (params.contains("saturation")) {
>  		config_.saturation = params["saturation"].get<ipa::Pwl>(ipa::Pwl{});
>  		if (config_.saturation.empty())
> @@ -83,9 +54,12 @@ int Ccm::read(const libcamera::YamlObject &params)
>  
>  		CtCcm ctCcm;
>  		ctCcm.ct = *value;
> -		ret = ctCcm.ccm.read(p["ccm"]);
> -		if (ret)
> -			return ret;
> +
> +		auto ccm = p["ccm"].get<Matrix3x3>();
> +		if (!ccm)
> +			return -EINVAL;
> +
> +		ctCcm.ccm = *ccm;
>  
>  		if (!config_.ccms.empty() && ctCcm.ct <= config_.ccms.back().ct) {
>  			LOG(RPiCcm, Error)
> @@ -143,11 +117,11 @@ Matrix3x3 calculateCcm(std::vector<CtCcm> const &ccms, double ct)
>  
>  Matrix3x3 applySaturation(Matrix3x3 const &ccm, double saturation)
>  {
> -	Matrix3x3 RGB2Y(0.299, 0.587, 0.114, -0.169, -0.331, 0.500, 0.500, -0.419,
> -		     -0.081);
> -	Matrix3x3 Y2RGB(1.000, 0.000, 1.402, 1.000, -0.345, -0.714, 1.000, 1.771,
> -		     0.000);
> -	Matrix3x3 S(1, 0, 0, 0, saturation, 0, 0, 0, saturation);
> +	static const Matrix3x3 RGB2Y({ 0.299, 0.587, 0.114, -0.169, -0.331,
> +				       0.500, 0.500, -0.419, -0.081 });
> +	static const Matrix3x3 Y2RGB({ 1.000, 0.000, 1.402, 1.000, -0.345,
> +				       -0.714, 1.000, 1.771, 0.000 });
> +	Matrix3x3 S({ 1, 0, 0, 0, saturation, 0, 0, 0, saturation });

Let's format it a bit nicer:

	static const Matrix3x3 RGB2Y({
		 0.299,  0.587,  0.114,
		-0.169, -0.331,  0.500,
		 0.500, -0.419, -0.081
	});

	static const Matrix3x3 Y2RGB({
		1.000,  0.000,  1.402,
		1.000, -0.345, -0.714,
		1.000,  1.771,  0.000
	});

	Matrix3x3 S({
		1, 0, 0,
		0, saturation, 0,
		0, 0, saturation
	});

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	return Y2RGB * S * RGB2Y * ccm;
>  }
>  
> @@ -181,7 +155,7 @@ void Ccm::prepare(Metadata *imageMetadata)
>  	for (int j = 0; j < 3; j++)
>  		for (int i = 0; i < 3; i++)
>  			ccmStatus.matrix[j * 3 + i] =
> -				std::max(-8.0, std::min(7.9999, ccm.m[j][i]));
> +				std::max(-8.0, std::min(7.9999, ccm[j][i]));
>  	LOG(RPiCcm, Debug)
>  		<< "colour temperature " << awb.temperatureK << "K";
>  	LOG(RPiCcm, Debug)
> diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h
> index 8e7f9616c2ab..c05dbb17a264 100644
> --- a/src/ipa/rpi/controller/rpi/ccm.h
> +++ b/src/ipa/rpi/controller/rpi/ccm.h
> @@ -8,6 +8,7 @@
>  
>  #include <vector>
>  
> +#include "libcamera/internal/matrix.h"
>  #include <libipa/pwl.h>
>  
>  #include "../ccm_algorithm.h"
> @@ -16,41 +17,9 @@ namespace RPiController {
>  
>  /* Algorithm to calculate colour matrix. Should be placed after AWB. */
>  
> -struct Matrix3x3 {
> -	Matrix3x3(double m0, double m1, double m2, double m3, double m4, double m5,
> -	       double m6, double m7, double m8);
> -	Matrix3x3();
> -	double m[3][3];
> -	int read(const libcamera::YamlObject &params);
> -};
> -static inline Matrix3x3 operator*(double d, Matrix3x3 const &m)
> -{
> -	return Matrix3x3(m.m[0][0] * d, m.m[0][1] * d, m.m[0][2] * d,
> -		      m.m[1][0] * d, m.m[1][1] * d, m.m[1][2] * d,
> -		      m.m[2][0] * d, m.m[2][1] * d, m.m[2][2] * d);
> -}
> -static inline Matrix3x3 operator*(Matrix3x3 const &m1, Matrix3x3 const &m2)
> -{
> -	Matrix3x3 m;
> -	for (int i = 0; i < 3; i++)
> -		for (int j = 0; j < 3; j++)
> -			m.m[i][j] = m1.m[i][0] * m2.m[0][j] +
> -				    m1.m[i][1] * m2.m[1][j] +
> -				    m1.m[i][2] * m2.m[2][j];
> -	return m;
> -}
> -static inline Matrix3x3 operator+(Matrix3x3 const &m1, Matrix3x3 const &m2)
> -{
> -	Matrix3x3 m;
> -	for (int i = 0; i < 3; i++)
> -		for (int j = 0; j < 3; j++)
> -			m.m[i][j] = m1.m[i][j] + m2.m[i][j];
> -	return m;
> -}
> -
>  struct CtCcm {
>  	double ct;
> -	Matrix3x3 ccm;
> +	libcamera::Matrix<double, 3, 3> ccm;
>  };
>  
>  struct CcmConfig {

Patch
diff mbox series 

diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp
index 7f63f3fdb702..2f40cb4e09ee 100644
--- a/src/ipa/rpi/controller/rpi/ccm.cpp
+++ b/src/ipa/rpi/controller/rpi/ccm.cpp
@@ -29,34 +29,7 @@  LOG_DEFINE_CATEGORY(RPiCcm)
 
 #define NAME "rpi.ccm"
 
-Matrix3x3::Matrix3x3()
-{
-	memset(m, 0, sizeof(m));
-}
-Matrix3x3::Matrix3x3(double m0, double m1, double m2, double m3, double m4, double m5,
-	       double m6, double m7, double m8)
-{
-	m[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] = m4,
-	m[1][2] = m5, m[2][0] = m6, m[2][1] = m7, m[2][2] = m8;
-}
-int Matrix3x3::read(const libcamera::YamlObject &params)
-{
-	double *ptr = (double *)m;
-
-	if (params.size() != 9) {
-		LOG(RPiCcm, Error) << "Wrong number of values in CCM";
-		return -EINVAL;
-	}
-
-	for (const auto &param : params.asList()) {
-		auto value = param.get<double>();
-		if (!value)
-			return -EINVAL;
-		*ptr++ = *value;
-	}
-
-	return 0;
-}
+using Matrix3x3 = Matrix<double, 3, 3>;
 
 Ccm::Ccm(Controller *controller)
 	: CcmAlgorithm(controller), saturation_(1.0) {}
@@ -68,8 +41,6 @@  char const *Ccm::name() const
 
 int Ccm::read(const libcamera::YamlObject &params)
 {
-	int ret;
-
 	if (params.contains("saturation")) {
 		config_.saturation = params["saturation"].get<ipa::Pwl>(ipa::Pwl{});
 		if (config_.saturation.empty())
@@ -83,9 +54,12 @@  int Ccm::read(const libcamera::YamlObject &params)
 
 		CtCcm ctCcm;
 		ctCcm.ct = *value;
-		ret = ctCcm.ccm.read(p["ccm"]);
-		if (ret)
-			return ret;
+
+		auto ccm = p["ccm"].get<Matrix3x3>();
+		if (!ccm)
+			return -EINVAL;
+
+		ctCcm.ccm = *ccm;
 
 		if (!config_.ccms.empty() && ctCcm.ct <= config_.ccms.back().ct) {
 			LOG(RPiCcm, Error)
@@ -143,11 +117,11 @@  Matrix3x3 calculateCcm(std::vector<CtCcm> const &ccms, double ct)
 
 Matrix3x3 applySaturation(Matrix3x3 const &ccm, double saturation)
 {
-	Matrix3x3 RGB2Y(0.299, 0.587, 0.114, -0.169, -0.331, 0.500, 0.500, -0.419,
-		     -0.081);
-	Matrix3x3 Y2RGB(1.000, 0.000, 1.402, 1.000, -0.345, -0.714, 1.000, 1.771,
-		     0.000);
-	Matrix3x3 S(1, 0, 0, 0, saturation, 0, 0, 0, saturation);
+	static const Matrix3x3 RGB2Y({ 0.299, 0.587, 0.114, -0.169, -0.331,
+				       0.500, 0.500, -0.419, -0.081 });
+	static const Matrix3x3 Y2RGB({ 1.000, 0.000, 1.402, 1.000, -0.345,
+				       -0.714, 1.000, 1.771, 0.000 });
+	Matrix3x3 S({ 1, 0, 0, 0, saturation, 0, 0, 0, saturation });
 	return Y2RGB * S * RGB2Y * ccm;
 }
 
@@ -181,7 +155,7 @@  void Ccm::prepare(Metadata *imageMetadata)
 	for (int j = 0; j < 3; j++)
 		for (int i = 0; i < 3; i++)
 			ccmStatus.matrix[j * 3 + i] =
-				std::max(-8.0, std::min(7.9999, ccm.m[j][i]));
+				std::max(-8.0, std::min(7.9999, ccm[j][i]));
 	LOG(RPiCcm, Debug)
 		<< "colour temperature " << awb.temperatureK << "K";
 	LOG(RPiCcm, Debug)
diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h
index 8e7f9616c2ab..c05dbb17a264 100644
--- a/src/ipa/rpi/controller/rpi/ccm.h
+++ b/src/ipa/rpi/controller/rpi/ccm.h
@@ -8,6 +8,7 @@ 
 
 #include <vector>
 
+#include "libcamera/internal/matrix.h"
 #include <libipa/pwl.h>
 
 #include "../ccm_algorithm.h"
@@ -16,41 +17,9 @@  namespace RPiController {
 
 /* Algorithm to calculate colour matrix. Should be placed after AWB. */
 
-struct Matrix3x3 {
-	Matrix3x3(double m0, double m1, double m2, double m3, double m4, double m5,
-	       double m6, double m7, double m8);
-	Matrix3x3();
-	double m[3][3];
-	int read(const libcamera::YamlObject &params);
-};
-static inline Matrix3x3 operator*(double d, Matrix3x3 const &m)
-{
-	return Matrix3x3(m.m[0][0] * d, m.m[0][1] * d, m.m[0][2] * d,
-		      m.m[1][0] * d, m.m[1][1] * d, m.m[1][2] * d,
-		      m.m[2][0] * d, m.m[2][1] * d, m.m[2][2] * d);
-}
-static inline Matrix3x3 operator*(Matrix3x3 const &m1, Matrix3x3 const &m2)
-{
-	Matrix3x3 m;
-	for (int i = 0; i < 3; i++)
-		for (int j = 0; j < 3; j++)
-			m.m[i][j] = m1.m[i][0] * m2.m[0][j] +
-				    m1.m[i][1] * m2.m[1][j] +
-				    m1.m[i][2] * m2.m[2][j];
-	return m;
-}
-static inline Matrix3x3 operator+(Matrix3x3 const &m1, Matrix3x3 const &m2)
-{
-	Matrix3x3 m;
-	for (int i = 0; i < 3; i++)
-		for (int j = 0; j < 3; j++)
-			m.m[i][j] = m1.m[i][j] + m2.m[i][j];
-	return m;
-}
-
 struct CtCcm {
 	double ct;
-	Matrix3x3 ccm;
+	libcamera::Matrix<double, 3, 3> ccm;
 };
 
 struct CcmConfig {