Patch Detail
Show a patch.
GET /api/1.1/patches/21973/?format=api
{ "id": 21973, "url": "https://patchwork.libcamera.org/api/1.1/patches/21973/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21973/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20241118221618.13953-14-laurent.pinchart@ideasonboard.com>", "date": "2024-11-18T22:16:14", "name": "[v3,13/17] ipa: libipa: colour: Use the RGB class to model RGB values", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "ca645f8a46fbfe10b71e0920f95fb04e5f3cc306", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21973/mbox/", "series": [ { "id": 4798, "url": "https://patchwork.libcamera.org/api/1.1/series/4798/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4798", "date": "2024-11-18T22:16:01", "name": "Improve linear algebra helpers in libipa", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4798/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21973/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21973/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 755A5C32F0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 18 Nov 2024 22:17:00 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A2CD765EE9;\n\tMon, 18 Nov 2024 23:16:59 +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 0425F658FC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 18 Nov 2024 23:16:46 +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 EE8418DB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 18 Nov 2024 23:16:28 +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=\"uMaf+x5S\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731968189;\n\tbh=Ipm5bKH+UlIsSXDdQf8fVMRQNwTu3V2nmuWjTyaaFl8=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=uMaf+x5SoFzskogiJdyXu4+8ugIZzN+aFDtYoQCKUoMF+hKZKHLfhHRmmAcJnZUkj\n\tD4R6crf9A/CmqV4LRlzn0OCCWUL4SwsH6WGeuT1KtNBlzHn8wd3amQotCc6orKqOgO\n\trAFeT7mWOzWvUy+kbJyFP11wMp1sRZBVlLpHdBro=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[PATCH v3 13/17] ipa: libipa: colour: Use the RGB class to model RGB\n\tvalues", "Date": "Tue, 19 Nov 2024 00:16:14 +0200", "Message-ID": "<20241118221618.13953-14-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.45.2", "In-Reply-To": "<20241118221618.13953-1-laurent.pinchart@ideasonboard.com>", "References": "<20241118221618.13953-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The rec601LuminanceFromRGB() and estimateCCT() functions take RGB\ntriplets as three variables. Replace them with instances of the RGB\nclass and adapt the users accordingly. Only variables passed directly to\nthese functions are converted to RGB instances, further conversion of\nIPA moduels to the RGB class will be performed separately.\n\nWhile at it, fix a typo in the documentation of the estimateCCT()\nfunction.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/ipu3/algorithms/agc.cpp | 14 +++----\n src/ipa/ipu3/algorithms/awb.cpp | 2 +-\n src/ipa/libipa/colours.cpp | 22 +++++-----\n src/ipa/libipa/colours.h | 6 ++-\n src/ipa/rkisp1/algorithms/awb.cpp | 47 ++++++++++++----------\n src/ipa/rpi/controller/rpi/agc_channel.cpp | 21 +++++-----\n 6 files changed, 55 insertions(+), 57 deletions(-)", "diff": "diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\nindex 466b3fb31a6c..fda4daa6306c 100644\n--- a/src/ipa/ipu3/algorithms/agc.cpp\n+++ b/src/ipa/ipu3/algorithms/agc.cpp\n@@ -178,18 +178,16 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n */\n double Agc::estimateLuminance(double gain) const\n {\n-\tdouble redSum = 0, greenSum = 0, blueSum = 0;\n+\tRGB<double> sum{ 0.0 };\n \n \tfor (unsigned int i = 0; i < rgbTriples_.size(); i++) {\n-\t\tredSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);\n-\t\tgreenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);\n-\t\tblueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);\n+\t\tsum.r() += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);\n+\t\tsum.g() += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);\n+\t\tsum.b() += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);\n \t}\n \n-\tdouble ySum = rec601LuminanceFromRGB(redSum * rGain_,\n-\t\t\t\t\t greenSum * gGain_,\n-\t\t\t\t\t blueSum * bGain_);\n-\n+\tRGB<double> gains{{ rGain_, gGain_, bGain_ }};\n+\tdouble ySum = rec601LuminanceFromRGB(sum * gains);\n \treturn ySum / (bdsGrid_.height * bdsGrid_.width) / 255;\n }\n \ndiff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\nindex 7c6bff09147c..55de05d9e39f 100644\n--- a/src/ipa/ipu3/algorithms/awb.cpp\n+++ b/src/ipa/ipu3/algorithms/awb.cpp\n@@ -412,7 +412,7 @@ void Awb::awbGreyWorld()\n \t blueGain = sumBlue.g() / (sumBlue.b() + 1);\n \n \t/* Color temperature is not relevant in Grey world but still useful to estimate it :-) */\n-\tasyncResults_.temperatureK = estimateCCT(sumRed.r(), sumRed.g(), sumBlue.b());\n+\tasyncResults_.temperatureK = estimateCCT({{ sumRed.r(), sumRed.g(), sumBlue.b() }});\n \n \t/*\n \t * Gain values are unsigned integer value ranging [0, 8) with 13 bit\ndiff --git a/src/ipa/libipa/colours.cpp b/src/ipa/libipa/colours.cpp\nindex 9fcb53b08ffb..6c597093ddf8 100644\n--- a/src/ipa/libipa/colours.cpp\n+++ b/src/ipa/libipa/colours.cpp\n@@ -21,9 +21,7 @@ namespace ipa {\n \n /**\n * \\brief Estimate luminance from RGB values following ITU-R BT.601\n- * \\param[in] r The red value\n- * \\param[in] g The green value\n- * \\param[in] b The blue value\n+ * \\param[in] rgb The RGB value\n *\n * This function estimates a luminance value from a triplet of Red, Green and\n * Blue values, following the formula defined by ITU-R Recommendation BT.601-7\n@@ -31,21 +29,19 @@ namespace ipa {\n *\n * \\return The estimated luminance value\n */\n-double rec601LuminanceFromRGB(double r, double g, double b)\n+double rec601LuminanceFromRGB(const RGB<double> &rgb)\n {\n-\treturn (r * .299) + (g * .587) + (b * .114);\n+\treturn (rgb.r() * .299) + (rgb.g() * .587) + (rgb.b() * .114);\n }\n \n /**\n * \\brief Estimate correlated colour temperature from RGB color space input\n- * \\param[in] red The input red value\n- * \\param[in] green The input green value\n- * \\param[in] blue The input blue value\n+ * \\param[in] rgb The RGB value\n *\n * This function estimates the correlated color temperature RGB color space\n * input. In physics and color science, the Planckian locus or black body locus\n * is the path or locus that the color of an incandescent black body would take\n- * in a particular chromaticity space as the blackbody temperature changes.\n+ * in a particular chromaticity space as the black body temperature changes.\n *\n * If a narrow range of color temperatures is considered (those encapsulating\n * daylight being the most practical case) one can approximate the Planckian\n@@ -56,12 +52,12 @@ double rec601LuminanceFromRGB(double r, double g, double b)\n *\n * \\return The estimated color temperature\n */\n-uint32_t estimateCCT(double red, double green, double blue)\n+uint32_t estimateCCT(const RGB<double> &rgb)\n {\n \t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n-\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n-\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n-\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n+\tdouble X = (-0.14282) * rgb.r() + (1.54924) * rgb.g() + (-0.95641) * rgb.b();\n+\tdouble Y = (-0.32466) * rgb.r() + (1.57837) * rgb.g() + (-0.73191) * rgb.b();\n+\tdouble Z = (-0.68202) * rgb.r() + (0.77073) * rgb.g() + (0.56332) * rgb.b();\n \n \t/* Calculate the normalized chromaticity values */\n \tdouble x = X / (X + Y + Z);\ndiff --git a/src/ipa/libipa/colours.h b/src/ipa/libipa/colours.h\nindex b42ed0ac1612..fa6a8b575cc7 100644\n--- a/src/ipa/libipa/colours.h\n+++ b/src/ipa/libipa/colours.h\n@@ -9,12 +9,14 @@\n \n #include <stdint.h>\n \n+#include \"vector.h\"\n+\n namespace libcamera {\n \n namespace ipa {\n \n-double rec601LuminanceFromRGB(double r, double g, double b);\n-uint32_t estimateCCT(double red, double green, double blue);\n+double rec601LuminanceFromRGB(const RGB<double> &rgb);\n+uint32_t estimateCCT(const RGB<double> &rgb);\n \n } /* namespace ipa */\n \ndiff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\nindex 5c1d9511ce8b..dbeaf81565ff 100644\n--- a/src/ipa/rkisp1/algorithms/awb.cpp\n+++ b/src/ipa/rkisp1/algorithms/awb.cpp\n@@ -192,9 +192,7 @@ void Awb::process(IPAContext &context,\n \tconst rkisp1_cif_isp_stat *params = &stats->params;\n \tconst rkisp1_cif_isp_awb_stat *awb = ¶ms->awb;\n \tIPAActiveState &activeState = context.activeState;\n-\tdouble greenMean;\n-\tdouble redMean;\n-\tdouble blueMean;\n+\tRGB<double> rgbMeans;\n \n \tmetadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);\n \tmetadata.set(controls::ColourGains, {\n@@ -209,9 +207,11 @@ void Awb::process(IPAContext &context,\n \t}\n \n \tif (rgbMode_) {\n-\t\tgreenMean = awb->awb_mean[0].mean_y_or_g;\n-\t\tredMean = awb->awb_mean[0].mean_cr_or_r;\n-\t\tblueMean = awb->awb_mean[0].mean_cb_or_b;\n+\t\trgbMeans = {{\n+\t\t\tstatic_cast<double>(awb->awb_mean[0].mean_y_or_g),\n+\t\t\tstatic_cast<double>(awb->awb_mean[0].mean_cr_or_r),\n+\t\t\tstatic_cast<double>(awb->awb_mean[0].mean_cb_or_b)\n+\t\t}};\n \t} else {\n \t\t/* Get the YCbCr mean values */\n \t\tdouble yMean = awb->awb_mean[0].mean_y_or_g;\n@@ -233,9 +233,11 @@ void Awb::process(IPAContext &context,\n \t\tyMean -= 16;\n \t\tcbMean -= 128;\n \t\tcrMean -= 128;\n-\t\tredMean = 1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean;\n-\t\tgreenMean = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean;\n-\t\tblueMean = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean;\n+\t\trgbMeans = {{\n+\t\t\t1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean,\n+\t\t\t1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean,\n+\t\t\t1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean\n+\t\t}};\n \n \t\t/*\n \t\t * Due to hardware rounding errors in the YCbCr means, the\n@@ -243,9 +245,7 @@ void Awb::process(IPAContext &context,\n \t\t * negative gains, messing up calculation. Prevent this by\n \t\t * clamping the means to positive values.\n \t\t */\n-\t\tredMean = std::max(redMean, 0.0);\n-\t\tgreenMean = std::max(greenMean, 0.0);\n-\t\tblueMean = std::max(blueMean, 0.0);\n+\t\trgbMeans = rgbMeans.max(0.0);\n \t}\n \n \t/*\n@@ -253,19 +253,22 @@ void Awb::process(IPAContext &context,\n \t * divide by the gains that were used to get the raw means from the\n \t * sensor.\n \t */\n-\tredMean /= frameContext.awb.gains.red;\n-\tgreenMean /= frameContext.awb.gains.green;\n-\tblueMean /= frameContext.awb.gains.blue;\n+\tRGB<double> gains{{\n+\t\tframeContext.awb.gains.red,\n+\t\tframeContext.awb.gains.green,\n+\t\tframeContext.awb.gains.blue\n+\t}};\n+\trgbMeans /= gains;\n \n \t/*\n \t * If the means are too small we don't have enough information to\n \t * meaningfully calculate gains. Freeze the algorithm in that case.\n \t */\n-\tif (redMean < kMeanMinThreshold && greenMean < kMeanMinThreshold &&\n-\t blueMean < kMeanMinThreshold)\n+\tif (rgbMeans.r() < kMeanMinThreshold && rgbMeans.g() < kMeanMinThreshold &&\n+\t rgbMeans.b() < kMeanMinThreshold)\n \t\treturn;\n \n-\tactiveState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n+\tactiveState.awb.temperatureK = estimateCCT(rgbMeans);\n \n \t/* Metadata shall contain the up to date measurement */\n \tmetadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n@@ -275,8 +278,8 @@ void Awb::process(IPAContext &context,\n \t * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the\n \t * divisor to a minimum value of 1.0.\n \t */\n-\tdouble redGain = greenMean / std::max(redMean, 1.0);\n-\tdouble blueGain = greenMean / std::max(blueMean, 1.0);\n+\tdouble redGain = rgbMeans.g() / std::max(rgbMeans.r(), 1.0);\n+\tdouble blueGain = rgbMeans.g() / std::max(rgbMeans.b(), 1.0);\n \n \t/*\n \t * Clamp the gain values to the hardware, which expresses gains as Q2.8\n@@ -298,8 +301,8 @@ void Awb::process(IPAContext &context,\n \n \tLOG(RkISP1Awb, Debug)\n \t\t<< std::showpoint\n-\t\t<< \"Means [\" << redMean << \", \" << greenMean << \", \" << blueMean\n-\t\t<< \"], gains [\" << activeState.awb.gains.automatic.red << \", \"\n+\t\t<< \"Means \" << rgbMeans\n+\t\t<< \", gains [\" << activeState.awb.gains.automatic.red << \", \"\n \t\t<< activeState.awb.gains.automatic.green << \", \"\n \t\t<< activeState.awb.gains.automatic.blue << \"], temp \"\n \t\t<< activeState.awb.temperatureK << \"K\";\ndiff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp\nindex 8583f4f31907..a99beb70b89a 100644\n--- a/src/ipa/rpi/controller/rpi/agc_channel.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp\n@@ -13,6 +13,7 @@\n #include <libcamera/base/log.h>\n \n #include \"libipa/colours.h\"\n+#include \"libipa/vector.h\"\n \n #include \"../awb_status.h\"\n #include \"../device_status.h\"\n@@ -680,12 +681,13 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n \t * Note that the weights are applied by the IPA to the statistics directly,\n \t * before they are given to us here.\n \t */\n-\tdouble rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n+\tipa::RGB<double> sum{ 0.0 };\n+\tdouble pixelSum = 0;\n \tfor (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n \t\tauto ®ion = stats->agcRegions.get(i);\n-\t\trSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n-\t\tgSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n-\t\tbSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n+\t\tsum.r() += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n+\t\tsum.g() += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n+\t\tsum.b() += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n \t\tpixelSum += region.counted;\n \t}\n \tif (pixelSum == 0.0) {\n@@ -693,14 +695,11 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n \t\treturn 0;\n \t}\n \n-\tdouble ySum;\n \t/* Factor in the AWB correction if needed. */\n-\tif (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {\n-\t\tySum = ipa::rec601LuminanceFromRGB(rSum * awb.gainR,\n-\t\t\t\t\t\t gSum * awb.gainG,\n-\t\t\t\t\t\t bSum * awb.gainB);\n-\t} else\n-\t\tySum = ipa::rec601LuminanceFromRGB(rSum, gSum, bSum);\n+\tif (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb)\n+\t\tsum *= ipa::RGB<double>{{ awb.gainR, awb.gainR, awb.gainB }};\n+\n+\tdouble ySum = ipa::rec601LuminanceFromRGB(sum);\n \n \treturn ySum / pixelSum / (1 << 16);\n }\n", "prefixes": [ "v3", "13/17" ] }