[{"id":32277,"web_url":"https://patchwork.libcamera.org/comment/32277/","msgid":"<20241119114119.GM31681@pendragon.ideasonboard.com>","date":"2024-11-19T11:41:19","subject":"Re: [PATCH v2 6/9] ipa: rpi: ccm: Replace local matrix\n\timplementation with the one from libcamera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nThank you for the patch.\n\nOn Tue, Nov 19, 2024 at 11:37:33AM +0100, Stefan Klug wrote:\n> The RaspberryPi IPA contains a private Matrix3x3 class inside the ccm\n> algorithm. Replace it with the Matrix class available in\n> libcamera/internal.\n> \n> While at it, mark the matrices RGB2Y and Y2RGB as static const.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rpi/controller/rpi/ccm.cpp | 52 ++++++++----------------------\n> src/ipa/rpi/controller/rpi/ccm.h | 35 ++------------------\n> 2 files changed, 15 insertions(+), 72 deletions(-)\n> \n> diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp\n> index 7f63f3fdb702..2f40cb4e09ee 100644\n> --- a/src/ipa/rpi/controller/rpi/ccm.cpp\n> +++ b/src/ipa/rpi/controller/rpi/ccm.cpp\n> @@ -29,34 +29,7 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> \n> #define NAME \"rpi.ccm\"\n> \n> -Matrix3x3::Matrix3x3()\n> -{\n> -\tmemset(m, 0, sizeof(m));\n> -}\n> -Matrix3x3::Matrix3x3(double m0, double m1, double m2, double m3, double m4, double m5,\n> -\t double m6, double m7, double m8)\n> -{\n> -\tm[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] = m4,\n> -\tm[1][2] = m5, m[2][0] = m6, m[2][1] = m7, m[2][2] = m8;\n> -}\n> -int Matrix3x3::read(const libcamera::YamlObject ¶ms)\n> -{\n> -\tdouble *ptr = (double *)m;\n> -\n> -\tif (params.size() != 9) {\n> -\t\tLOG(RPiCcm, Error) << \"Wrong number of values in CCM\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> -\tfor (const auto ¶m : params.asList()) {\n> -\t\tauto value = param.get();\n> -\t\tif (!value)\n> -\t\t\treturn -EINVAL;\n> -\t\t*ptr++ = *value;\n> -\t}\n> -\n> -\treturn 0;\n> -}\n> +using Matrix3x3 = Matrix;\n> \n> Ccm::Ccm(Controller *controller)\n> \t: CcmAlgorithm(controller), saturation_(1.0) {}\n> @@ -68,8 +41,6 @@ char const *Ccm::name() const\n> \n> int Ccm::read(const libcamera::YamlObject ¶ms)\n> {\n> -\tint ret;\n> -\n> \tif (params.contains(\"saturation\")) {\n> \t\tconfig_.saturation = params[\"saturation\"].get(ipa::Pwl{});\n> \t\tif (config_.saturation.empty())\n> @@ -83,9 +54,12 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n> \n> \t\tCtCcm ctCcm;\n> \t\tctCcm.ct = *value;\n> -\t\tret = ctCcm.ccm.read(p[\"ccm\"]);\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> +\n> +\t\tauto ccm = p[\"ccm\"].get();\n> +\t\tif (!ccm)\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tctCcm.ccm = *ccm;\n> \n> \t\tif (!config_.ccms.empty() && ctCcm.ct <= config_.ccms.back().ct) {\n> \t\t\tLOG(RPiCcm, Error)\n> @@ -143,11 +117,11 @@ Matrix3x3 calculateCcm(std::vector const &ccms, double ct)\n> \n> Matrix3x3 applySaturation(Matrix3x3 const &ccm, double saturation)\n> {\n> -\tMatrix3x3 RGB2Y(0.299, 0.587, 0.114, -0.169, -0.331, 0.500, 0.500, -0.419,\n> -\t\t -0.081);\n> -\tMatrix3x3 Y2RGB(1.000, 0.000, 1.402, 1.000, -0.345, -0.714, 1.000, 1.771,\n> -\t\t 0.000);\n> -\tMatrix3x3 S(1, 0, 0, 0, saturation, 0, 0, 0, saturation);\n> +\tstatic const Matrix3x3 RGB2Y({ 0.299, 0.587, 0.114, -0.169, -0.331,\n> +\t\t\t\t 0.500, 0.500, -0.419, -0.081 });\n> +\tstatic const Matrix3x3 Y2RGB({ 1.000, 0.000, 1.402, 1.000, -0.345,\n> +\t\t\t\t -0.714, 1.000, 1.771, 0.000 });\n> +\tMatrix3x3 S({ 1, 0, 0, 0, saturation, 0, 0, 0, saturation });\n\nLet's format it a bit nicer:\n\n\tstatic const Matrix3x3 RGB2Y({\n\t\t 0.299, 0.587, 0.114,\n\t\t-0.169, -0.331, 0.500,\n\t\t 0.500, -0.419, -0.081\n\t});\n\n\tstatic const Matrix3x3 Y2RGB({\n\t\t1.000, 0.000, 1.402,\n\t\t1.000, -0.345, -0.714,\n\t\t1.000, 1.771, 0.000\n\t});\n\n\tMatrix3x3 S({\n\t\t1, 0, 0,\n\t\t0, saturation, 0,\n\t\t0, 0, saturation\n\t});\n\nReviewed-by: Laurent Pinchart \n\n> \treturn Y2RGB * S * RGB2Y * ccm;\n> }\n> \n> @@ -181,7 +155,7 @@ void Ccm::prepare(Metadata *imageMetadata)\n> \tfor (int j = 0; j < 3; j++)\n> \t\tfor (int i = 0; i < 3; i++)\n> \t\t\tccmStatus.matrix[j * 3 + i] =\n> -\t\t\t\tstd::max(-8.0, std::min(7.9999, ccm.m[j][i]));\n> +\t\t\t\tstd::max(-8.0, std::min(7.9999, ccm[j][i]));\n> \tLOG(RPiCcm, Debug)\n> \t\t<< \"colour temperature \" << awb.temperatureK << \"K\";\n> \tLOG(RPiCcm, Debug)\n> diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h\n> index 8e7f9616c2ab..c05dbb17a264 100644\n> --- a/src/ipa/rpi/controller/rpi/ccm.h\n> +++ b/src/ipa/rpi/controller/rpi/ccm.h\n> @@ -8,6 +8,7 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/matrix.h\"\n> #include \n> \n> #include \"../ccm_algorithm.h\"\n> @@ -16,41 +17,9 @@ namespace RPiController {\n> \n> /* Algorithm to calculate colour matrix. Should be placed after AWB. */\n> \n> -struct Matrix3x3 {\n> -\tMatrix3x3(double m0, double m1, double m2, double m3, double m4, double m5,\n> -\t double m6, double m7, double m8);\n> -\tMatrix3x3();\n> -\tdouble m[3][3];\n> -\tint read(const libcamera::YamlObject ¶ms);\n> -};\n> -static inline Matrix3x3 operator*(double d, Matrix3x3 const &m)\n> -{\n> -\treturn Matrix3x3(m.m[0][0] * d, m.m[0][1] * d, m.m[0][2] * d,\n> -\t\t m.m[1][0] * d, m.m[1][1] * d, m.m[1][2] * d,\n> -\t\t m.m[2][0] * d, m.m[2][1] * d, m.m[2][2] * d);\n> -}\n> -static inline Matrix3x3 operator*(Matrix3x3 const &m1, Matrix3x3 const &m2)\n> -{\n> -\tMatrix3x3 m;\n> -\tfor (int i = 0; i < 3; i++)\n> -\t\tfor (int j = 0; j < 3; j++)\n> -\t\t\tm.m[i][j] = m1.m[i][0] * m2.m[0][j] +\n> -\t\t\t\t m1.m[i][1] * m2.m[1][j] +\n> -\t\t\t\t m1.m[i][2] * m2.m[2][j];\n> -\treturn m;\n> -}\n> -static inline Matrix3x3 operator+(Matrix3x3 const &m1, Matrix3x3 const &m2)\n> -{\n> -\tMatrix3x3 m;\n> -\tfor (int i = 0; i < 3; i++)\n> -\t\tfor (int j = 0; j < 3; j++)\n> -\t\t\tm.m[i][j] = m1.m[i][j] + m2.m[i][j];\n> -\treturn m;\n> -}\n> -\n> struct CtCcm {\n> \tdouble ct;\n> -\tMatrix3x3 ccm;\n> +\tlibcamera::Matrix ccm;\n> };\n> \n> struct CcmConfig {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6ABECC326B\n\tfor ;\n\tTue, 19 Nov 2024 11:41:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 55EB565F09;\n\tTue, 19 Nov 2024 12:41:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E7CD65F03\n\tfor ;\n\tTue, 19 Nov 2024 12:41:34 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCFC875A;\n\tTue, 19 Nov 2024 12:41:12 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"o7gyFqED\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732016474;\n\tbh=8ywnRhDW7nT/p1TvlLKCrXoIk+xjyd10RyClJzL3Tiw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=o7gyFqED75biGsF4i7CWWxfjVjiVDQjLfhsafXVOdRgXf9uYnhitkWS6EL+DSMtoF\n\t6Rb0GTeEMpkbb9u0VBaeP5VfCKqC983bFkVtAd7ixi5YToJ+xjP9mlqM8JDic5KIW7\n\tCQYTqSlMkj0QCDWIVFa2gJ2hhbsgxtC3IH15i6h4=","Date":"Tue, 19 Nov 2024 13:41:19 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 6/9] ipa: rpi: ccm: Replace local matrix\n\timplementation with the one from libcamera","Message-ID":"<20241119114119.GM31681@pendragon.ideasonboard.com>","References":"<20241119103740.1919807-1-stefan.klug@ideasonboard.com>\n\t<20241119103740.1919807-7-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241119103740.1919807-7-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]