[{"id":32312,"web_url":"https://patchwork.libcamera.org/comment/32312/","msgid":"","date":"2024-11-20T15:15:42","subject":"Re: [PATCH v3 6/9] ipa: rpi: ccm: Replace local matrix\n\timplementation with the one from libcamera","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Stefan,\n\n\n\nOn Wed, 20 Nov 2024 at 14:28, Stefan Klug \nwrote:\n\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> Reviewed-by: Laurent Pinchart \n>\n\nAcked-by: Naushir Patuck \n\n\n>\n> ---\n> Changes in v3:\n> - Collected tags\n> - Reformatted matrices\n> - Tested on actual device\n> ---\n> src/ipa/rpi/controller/rpi/ccm.cpp | 59 ++++++++++--------------------\n> src/ipa/rpi/controller/rpi/ccm.h | 35 +-----------------\n> 2 files changed, 22 insertions(+), 72 deletions(-)\n>\n> diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp\n> b/src/ipa/rpi/controller/rpi/ccm.cpp\n> index 7f63f3fdb702..8607f1521b5a 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> - memset(m, 0, sizeof(m));\n> -}\n> -Matrix3x3::Matrix3x3(double m0, double m1, double m2, double m3, double\n> m4, double m5,\n> - double m6, double m7, double m8)\n> -{\n> - m[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] =\n> m4,\n> - m[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> - double *ptr = (double *)m;\n> -\n> - if (params.size() != 9) {\n> - LOG(RPiCcm, Error) << \"Wrong number of values in CCM\";\n> - return -EINVAL;\n> - }\n> -\n> - for (const auto ¶m : params.asList()) {\n> - auto value = param.get();\n> - if (!value)\n> - return -EINVAL;\n> - *ptr++ = *value;\n> - }\n> -\n> - return 0;\n> -}\n> +using Matrix3x3 = Matrix;\n>\n> Ccm::Ccm(Controller *controller)\n> : 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> - int ret;\n> -\n> if (params.contains(\"saturation\")) {\n> config_.saturation =\n> params[\"saturation\"].get(ipa::Pwl{});\n> if (config_.saturation.empty())\n> @@ -83,9 +54,12 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n>\n> CtCcm ctCcm;\n> ctCcm.ct = *value;\n> - ret = ctCcm.ccm.read(p[\"ccm\"]);\n> - if (ret)\n> - return ret;\n> +\n> + auto ccm = p[\"ccm\"].get();\n> + if (!ccm)\n> + return -EINVAL;\n> +\n> + ctCcm.ccm = *ccm;\n>\n> if (!config_.ccms.empty() && ctCcm.ct <=\n> config_.ccms.back().ct) {\n> LOG(RPiCcm, Error)\n> @@ -143,11 +117,18 @@ Matrix3x3 calculateCcm(std::vector const\n> &ccms, double ct)\n>\n> Matrix3x3 applySaturation(Matrix3x3 const &ccm, double saturation)\n> {\n> - Matrix3x3 RGB2Y(0.299, 0.587, 0.114, -0.169, -0.331, 0.500, 0.500,\n> -0.419,\n> - -0.081);\n> - Matrix3x3 Y2RGB(1.000, 0.000, 1.402, 1.000, -0.345, -0.714, 1.000,\n> 1.771,\n> - 0.000);\n> - Matrix3x3 S(1, 0, 0, 0, saturation, 0, 0, 0, saturation);\n> + static const Matrix3x3 RGB2Y({ 0.299, 0.587, 0.114,\n> + -0.169, -0.331, 0.500,\n> + 0.500, -0.419, -0.081 });\n> +\n> + static const Matrix3x3 Y2RGB({ 1.000, 0.000, 1.402,\n> + 1.000, -0.345, -0.714,\n> + 1.000, 1.771, 0.000 });\n> +\n> + Matrix3x3 S({ 1, 0, 0,\n> + 0, saturation, 0,\n> + 0, 0, saturation });\n> +\n> return Y2RGB * S * RGB2Y * ccm;\n> }\n>\n> @@ -181,7 +162,7 @@ void Ccm::prepare(Metadata *imageMetadata)\n> for (int j = 0; j < 3; j++)\n> for (int i = 0; i < 3; i++)\n> ccmStatus.matrix[j * 3 + i] =\n> - std::max(-8.0, std::min(7.9999,\n> ccm.m[j][i]));\n> + std::max(-8.0, std::min(7.9999,\n> ccm[j][i]));\n> LOG(RPiCcm, Debug)\n> << \"colour temperature \" << awb.temperatureK << \"K\";\n> LOG(RPiCcm, Debug)\n> diff --git a/src/ipa/rpi/controller/rpi/ccm.h\n> 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> - Matrix3x3(double m0, double m1, double m2, double m3, double m4,\n> double m5,\n> - double m6, double m7, double m8);\n> - Matrix3x3();\n> - double m[3][3];\n> - int read(const libcamera::YamlObject ¶ms);\n> -};\n> -static inline Matrix3x3 operator*(double d, Matrix3x3 const &m)\n> -{\n> - return Matrix3x3(m.m[0][0] * d, m.m[0][1] * d, m.m[0][2] * d,\n> - m.m[1][0] * d, m.m[1][1] * d, m.m[1][2] * d,\n> - 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\n> &m2)\n> -{\n> - Matrix3x3 m;\n> - for (int i = 0; i < 3; i++)\n> - for (int j = 0; j < 3; j++)\n> - m.m[i][j] = m1.m[i][0] * m2.m[0][j] +\n> - m1.m[i][1] * m2.m[1][j] +\n> - m1.m[i][2] * m2.m[2][j];\n> - return m;\n> -}\n> -static inline Matrix3x3 operator+(Matrix3x3 const &m1, Matrix3x3 const\n> &m2)\n> -{\n> - Matrix3x3 m;\n> - for (int i = 0; i < 3; i++)\n> - for (int j = 0; j < 3; j++)\n> - m.m[i][j] = m1.m[i][j] + m2.m[i][j];\n> - return m;\n> -}\n> -\n> struct CtCcm {\n> double ct;\n> - Matrix3x3 ccm;\n> + libcamera::Matrix ccm;\n> };\n>\n> struct CcmConfig {\n> --\n> 2.43.0\n>\n>","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 A0532C0F2A\n\tfor ;\n\tWed, 20 Nov 2024 15:16:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8F91C65F72;\n\tWed, 20 Nov 2024 16:16:22 +0100 (CET)","from mail-yw1-x1129.google.com (mail-yw1-x1129.google.com\n\t[IPv6:2607:f8b0:4864:20::1129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8769F65F5E\n\tfor ;\n\tWed, 20 Nov 2024 16:16:19 +0100 (CET)","by mail-yw1-x1129.google.com with SMTP id\n\t00721157ae682-6ee91fa7f10so2491187b3.0\n\tfor ;\n\tWed, 20 Nov 2024 07:16:19 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Qf2X3HZV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1732115778; x=1732720578;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=gjeOIB1NTkiL5sboQIGw35ayM0cTlC3TqFoY98wavCY=;\n\tb=Qf2X3HZVlyW1ymarfTWLjWCL4/wLzZDz7SywBEIvz4v7Wn3rvIizYbovz6s6arcpe/\n\t47bPIhi5FWbLJR7LB0eKvzOQqfTolYDE4RDlscs+oFWvH3F2uhMZd1tsdUM6xGh8uf/g\n\tHDVy+GYPAp8BS5Z34GKlOS6GSrUsLQhXSW15c8EhggVx8DIDN1Fsdj/to4+YiJhbqbvi\n\tM6CSRVg215kwTW1qSCzQsfCmOjz8j/j/dU9BxIrLI7fOKyzyK+13EA44Xn3qvtuALuCC\n\tz1vwlKQAU96B1y3/3tHJgc9zCetnBZWc39RPtB8IiluYkK0E6Ii7iACAC2wL45LBHxCk\n\ttC4w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1732115778; x=1732720578;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=gjeOIB1NTkiL5sboQIGw35ayM0cTlC3TqFoY98wavCY=;\n\tb=DGVIDoth9usMgNZgCj7+UlnZ1V7JXvinCZDTGxs0oPCNpeplH2AaRoLEXyWOqFgPVC\n\tUg/6ckzHjLMQHErFz6/+Za1dHUYpEG3g+PH7I9AzC9ahv32g09kOzJd5oiCPYiGknwx2\n\tHX1hkndGy/oCIL8nnQw5ZibGIrQxk3se4FwsdMhhXGEMO1Pcp70nxUMn9J8vS4qsddc9\n\tFN4zXvsN4DTNWz//z37wqQPIlSrAjzMWKOJbGyqUVDn4R8Z7glNnT0lMBnC4alT/jqvb\n\t3GaVxvHj0lVJaeaopHKB7QZ/ukDq0zxEg+NG8qbcWAoH1+oYVTbN3cX7IFndcpWZY72x\n\tYY6g==","X-Gm-Message-State":"AOJu0YyPac9fNwL5YsxsSepo/nO4kxrD2pUAFBP5Jm4iRtO1g/TZGOKt\n\tVEEAwmdiRg8zrm2l5C0rSNM2WEVcFMcLFt8/J8Q7IP61zw0ko0WX5UhHwcA+HMm7hmoMAtDtPmR\n\tfrxVSkkJ4kdjOef0jV0o8T/Dr/C1UKNBe1IaZaWJyQDYus+HT","X-Gm-Gg":"ASbGncsw5QOZfVRziZIZNBHxU8ERWApORrI2G0g7hgvu7ymOawrA60cpgEUXMT8+K3A\n\tucsjRv5jxppDyI7ncXhq+2HnWxwRtp8igK9ksJxg7wJpwGtZutXYloMV2VmrlMe0=","X-Google-Smtp-Source":"AGHT+IF3bahpKv0MksxADKwmB9zyXWTfS+wv9YjnNlIYMuGqPRNEp5yjR0oAs/8RLO7+A2VXPDVmJ9N9dfX1XU64HEE=","X-Received":"by 2002:a05:690c:63c8:b0:6ea:881b:b534 with SMTP id\n\t00721157ae682-6eebd0f1f81mr12683677b3.1.1732115778371;\n\tWed, 20 Nov 2024 07:16:18 -0800 (PST)","MIME-Version":"1.0","References":"<20241120142807.2093301-1-stefan.klug@ideasonboard.com>\n\t<20241120142807.2093301-7-stefan.klug@ideasonboard.com>","In-Reply-To":"<20241120142807.2093301-7-stefan.klug@ideasonboard.com>","From":"Naushir Patuck ","Date":"Wed, 20 Nov 2024 15:15:42 +0000","Message-ID":"","Subject":"Re: [PATCH v3 6/9] ipa: rpi: ccm: Replace local matrix\n\timplementation with the one from libcamera","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000ae1f26062759a184\"","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\" "}}]