[{"id":32213,"web_url":"https://patchwork.libcamera.org/comment/32213/","msgid":"<173192890554.576258.409863343293498291@ping.linuxembedded.co.uk>","date":"2024-11-18T11:21:45","subject":"Re: [RFC PATCH 11/11] ipa: rkisp1: awb: Replace manual calculations\n\twith Vector and Matrix","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-11-17 22:17:12)\n> Processing of the statistics and estimation of the colour temperature\n> involve linear algebra. Replace the manual calculations with usage of\n> the Vector and Matrix classes.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 44 +++++++++++++++++++------------\n> 1 file changed, 27 insertions(+), 17 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 1c572055acdd..c089523c8bde 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -169,17 +169,21 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> \n> uint32_t Awb::estimateCCT(const RGB &rgb)\n> {\n> - /* Convert the RGB values to CIE tristimulus values (XYZ) */\n> - double X = -0.14282 * rgb.r() + 1.54924 * rgb.g() - 0.95641 * rgb.b();\n> - double Y = -0.32466 * rgb.r() + 1.57837 * rgb.g() - 0.73191 * rgb.b();\n> - double Z = -0.68202 * rgb.r() + 0.77073 * rgb.g() + 0.56332 * rgb.b();\n> + /*\n> + * Convert the RGB values to CIE tristimulus values (XYZ) and normalize\n> + * it (xyz).\n> + */\n> + static const Matrix rgb2xyz({\n> + -0.14282, 1.54924, -0.95641,\n> + -0.32466, 1.57837, -0.73191,\n> + -0.68202, 0.77073, 0.56332\n> + });\n> \n> - /* Calculate the normalized chromaticity values */\n> - double x = X / (X + Y + Z);\n> - double y = Y / (X + Y + Z);\n> + Vector xyz = rgb2xyz * rgb;\n> + xyz.normalize();\n> \n> /* Calculate CCT */\n> - double n = (x - 0.3320) / (0.1858 - y);\n> + double n = (xyz.x() - 0.3320) / (0.1858 - xyz.y());\n> return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> }\n> \n> @@ -215,9 +219,11 @@ void Awb::process(IPAContext &context,\n> rgbMeans.b() = awb->awb_mean[0].mean_cb_or_b;\n> } else {\n> /* Get the YCbCr mean values */\n> - double yMean = awb->awb_mean[0].mean_y_or_g;\n> - double cbMean = awb->awb_mean[0].mean_cb_or_b;\n> - double crMean = awb->awb_mean[0].mean_cr_or_r;\n> + Vector yuvMeans({\n> + static_cast(awb->awb_mean[0].mean_y_or_g),\n> + static_cast(awb->awb_mean[0].mean_cb_or_b),\n> + static_cast(awb->awb_mean[0].mean_cr_or_r)\n> + });\n> \n> /*\n> * Convert from YCbCr to RGB.\n> @@ -231,12 +237,16 @@ void Awb::process(IPAContext &context,\n> * [1,1636, -0,4045, -0,7949]\n> * [1,1636, 1,9912, -0,0250]]\n\nThis table in the comment might now be redundant or duplicated given the\nnew style below.\n\nBut I think that's exactly the purpose of the series and a good thing\n\n> */\n> - yMean -= 16;\n> - cbMean -= 128;\n> - crMean -= 128;\n> - rgbMeans.r() = 1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean;\n> - rgbMeans.g() = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean;\n> - rgbMeans.b() = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean;\n> + static const Matrix yuv2rgbMatrix({\n> + 1.1636, -0.0623, 1.6008,\n> + 1.1636, -0.4045, -0.7949,\n> + 1.1636, 1.9912, -0.0250\n> + });\n\nThat's far more recognisable as a 3x3 matrix transform parameter I think\n\nSo I think this series is a useful development (I'm sure/hope I said\nthat last time I saw something very similar too, but it seems it never\ngot in ?)\n\nReviewed-by: Kieran Bingham \n\n\nI'm also sure we've got patches that move these functions so you might\nneed to check that to see who can solve the race.\n\n--\nKieran\n\n\n> + static const Vector yuv2rgbOffset({\n> + 16, 128, 128\n> + });\n> +\n> + rgbMeans = yuv2rgbMatrix * (yuvMeans - yuv2rgbOffset);\n> \n> /*\n> * Due to hardware rounding errors in the YCbCr means, the\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 AA2D1C32DD\n\tfor ;\n\tMon, 18 Nov 2024 11:21:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AAB92658C1;\n\tMon, 18 Nov 2024 12:21:50 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6001F60532\n\tfor ;\n\tMon, 18 Nov 2024 12:21:48 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 83E765B3;\n\tMon, 18 Nov 2024 12:21:31 +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=\"OvnqRV2K\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731928891;\n\tbh=Y/rKApGj8Vpjf+F8aCs9ZI63pEpQ9BY2bKfYK1gLkD0=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=OvnqRV2Kn0Pvwa4X72uSItHbgjY6f7bkVtoMPrd3Ltt4QosvcbLsMYYG3adMnd21a\n\tocVVtwlOuXMJQbHiBgjy5s4H0GQX3yCqaZqEAgSeqvynaO5Ckgcp80k9pRQqScwUte\n\tkksXgXKSLeOCuqnGl8ScN/dEKQbJ6ZA/H6rQStLc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241117221712.29616-12-laurent.pinchart@ideasonboard.com>","References":"<20241117221712.29616-1-laurent.pinchart@ideasonboard.com>\n\t<20241117221712.29616-12-laurent.pinchart@ideasonboard.com>","Subject":"Re: [RFC PATCH 11/11] ipa: rkisp1: awb: Replace manual calculations\n\twith Vector and Matrix","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 18 Nov 2024 11:21:45 +0000","Message-ID":"<173192890554.576258.409863343293498291@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}},{"id":32236,"web_url":"https://patchwork.libcamera.org/comment/32236/","msgid":"<20241118151133.GR31681@pendragon.ideasonboard.com>","date":"2024-11-18T15:11:33","subject":"Re: [RFC PATCH 11/11] ipa: rkisp1: awb: Replace manual calculations\n\twith Vector and Matrix","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Nov 18, 2024 at 11:21:45AM +0000, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2024-11-17 22:17:12)\n> > Processing of the statistics and estimation of the colour temperature\n> > involve linear algebra. Replace the manual calculations with usage of\n> > the Vector and Matrix classes.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/rkisp1/algorithms/awb.cpp | 44 +++++++++++++++++++------------\n> > 1 file changed, 27 insertions(+), 17 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index 1c572055acdd..c089523c8bde 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -169,17 +169,21 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> > \n> > uint32_t Awb::estimateCCT(const RGB &rgb)\n> > {\n> > - /* Convert the RGB values to CIE tristimulus values (XYZ) */\n> > - double X = -0.14282 * rgb.r() + 1.54924 * rgb.g() - 0.95641 * rgb.b();\n> > - double Y = -0.32466 * rgb.r() + 1.57837 * rgb.g() - 0.73191 * rgb.b();\n> > - double Z = -0.68202 * rgb.r() + 0.77073 * rgb.g() + 0.56332 * rgb.b();\n> > + /*\n> > + * Convert the RGB values to CIE tristimulus values (XYZ) and normalize\n> > + * it (xyz).\n> > + */\n> > + static const Matrix rgb2xyz({\n> > + -0.14282, 1.54924, -0.95641,\n> > + -0.32466, 1.57837, -0.73191,\n> > + -0.68202, 0.77073, 0.56332\n> > + });\n> > \n> > - /* Calculate the normalized chromaticity values */\n> > - double x = X / (X + Y + Z);\n> > - double y = Y / (X + Y + Z);\n> > + Vector xyz = rgb2xyz * rgb;\n> > + xyz.normalize();\n> > \n> > /* Calculate CCT */\n> > - double n = (x - 0.3320) / (0.1858 - y);\n> > + double n = (xyz.x() - 0.3320) / (0.1858 - xyz.y());\n> > return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> > }\n> > \n> > @@ -215,9 +219,11 @@ void Awb::process(IPAContext &context,\n> > rgbMeans.b() = awb->awb_mean[0].mean_cb_or_b;\n> > } else {\n> > /* Get the YCbCr mean values */\n> > - double yMean = awb->awb_mean[0].mean_y_or_g;\n> > - double cbMean = awb->awb_mean[0].mean_cb_or_b;\n> > - double crMean = awb->awb_mean[0].mean_cr_or_r;\n> > + Vector yuvMeans({\n> > + static_cast(awb->awb_mean[0].mean_y_or_g),\n> > + static_cast(awb->awb_mean[0].mean_cb_or_b),\n> > + static_cast(awb->awb_mean[0].mean_cr_or_r)\n> > + });\n> > \n> > /*\n> > * Convert from YCbCr to RGB.\n> > @@ -231,12 +237,16 @@ void Awb::process(IPAContext &context,\n> > * [1,1636, -0,4045, -0,7949]\n> > * [1,1636, 1,9912, -0,0250]]\n> \n> This table in the comment might now be redundant or duplicated given the\n> new style below.\n\nI think the matrix should be moved to the newly colour.h that Dan is\nintroducing, I expect the comment to be removed then.\n\n> But I think that's exactly the purpose of the series and a good thing\n> \n> > */\n> > - yMean -= 16;\n> > - cbMean -= 128;\n> > - crMean -= 128;\n> > - rgbMeans.r() = 1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean;\n> > - rgbMeans.g() = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean;\n> > - rgbMeans.b() = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean;\n> > + static const Matrix yuv2rgbMatrix({\n> > + 1.1636, -0.0623, 1.6008,\n> > + 1.1636, -0.4045, -0.7949,\n> > + 1.1636, 1.9912, -0.0250\n> > + });\n> \n> That's far more recognisable as a 3x3 matrix transform parameter I think\n> \n> So I think this series is a useful development (I'm sure/hope I said\n> that last time I saw something very similar too, but it seems it never\n> got in ?)\n> \n> Reviewed-by: Kieran Bingham \n> \n> \n> I'm also sure we've got patches that move these functions so you might\n> need to check that to see who can solve the race.\n\nYes, I'm aware of that. If Dan pushes his series first I'll rebase this,\nno problem.\n\n> > + static const Vector yuv2rgbOffset({\n> > + 16, 128, 128\n> > + });\n> > +\n> > + rgbMeans = yuv2rgbMatrix * (yuvMeans - yuv2rgbOffset);\n> > \n> > /*\n> > * Due to hardware rounding errors in the YCbCr means, the","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 817B7C32EA\n\tfor ;\n\tMon, 18 Nov 2024 15:11:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7F3F9658E4;\n\tMon, 18 Nov 2024 16:11:44 +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 C8BB5658DC\n\tfor ;\n\tMon, 18 Nov 2024 16:11:42 +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 C5BF4316;\n\tMon, 18 Nov 2024 16:11:25 +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=\"aigjn/hN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731942686;\n\tbh=XgyF1nTJkpJxmpJRLNcTPvVUBsHCez3h0xKY5JSk9TY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aigjn/hNcS/YD7ZkTfzxo0nMjHdz2jlZjd6WRuRuEzuToF2OvePFAmsALRZQxXK4F\n\tZecuKdGnsmuKsjBlwVZi+0v6/srNIVqbwmkKbVVfGtBByWrPxu4IhJlK7QZnJp+1/q\n\tDkiRbVU4LuwFIDHN1K0V4uN5CxQ4oXnmTwks0WkY=","Date":"Mon, 18 Nov 2024 17:11:33 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH 11/11] ipa: rkisp1: awb: Replace manual calculations\n\twith Vector and Matrix","Message-ID":"<20241118151133.GR31681@pendragon.ideasonboard.com>","References":"<20241117221712.29616-1-laurent.pinchart@ideasonboard.com>\n\t<20241117221712.29616-12-laurent.pinchart@ideasonboard.com>\n\t<173192890554.576258.409863343293498291@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<173192890554.576258.409863343293498291@ping.linuxembedded.co.uk>","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\" "}}]