Patch Detail
Show a patch.
GET /api/patches/21953/?format=api
{ "id": 21953, "url": "https://patchwork.libcamera.org/api/patches/21953/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21953/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20241118000738.18977-12-laurent.pinchart@ideasonboard.com>", "date": "2024-11-18T00:07:37", "name": "[RFC,v2,11/12] ipa: rkisp1: awb: Use RGB class to store colour gains", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "467a79762e57e282550d7072875d7cca3b19a0fe", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21953/mbox/", "series": [ { "id": 4796, "url": "https://patchwork.libcamera.org/api/series/4796/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4796", "date": "2024-11-18T00:07:26", "name": "Improve linear algebra helpers in libipa", "version": 2, "mbox": "https://patchwork.libcamera.org/series/4796/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21953/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21953/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 45679C32E0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 18 Nov 2024 00:08:14 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D346658D6;\n\tMon, 18 Nov 2024 01:08:13 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 966DE658B9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 18 Nov 2024 01:08:02 +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 49ACC75A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 18 Nov 2024 01:07:46 +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=\"u1NQO/oH\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731888466;\n\tbh=PKv/OBTdXtT40Qv9Q5UJ+fbeiVATtaeki5ypXOwj+VQ=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=u1NQO/oHLWUf3CUVK0UrsrZAhl9MGldXum6MDDLi/LA0ReXYr3z/842EbLIb7du6c\n\tb/xOXfZ8OsuzSofu5n2HsKAAAbgHVeWQAcRo7yV6LAvVyD7X3fS/k3rGJf43/9OA+I\n\tTngRodjJzk6QSkPuz/ZYsY9TtXgCJe7T9e6Nv5Oo=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[RFC PATCH v2 11/12] ipa: rkisp1: awb: Use RGB class to store colour\n\tgains", "Date": "Mon, 18 Nov 2024 02:07:37 +0200", "Message-ID": "<20241118000738.18977-12-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.45.2", "In-Reply-To": "<20241118000738.18977-1-laurent.pinchart@ideasonboard.com>", "References": "<20241118000738.18977-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": "Replace the individual colour gains with instances of the RGB<double>\nclass. This simplifies the code that performs calculations on the gains.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/rkisp1/algorithms/awb.cpp | 102 ++++++++++++------------------\n src/ipa/rkisp1/algorithms/awb.h | 2 +-\n src/ipa/rkisp1/ipa_context.cpp | 31 +--------\n src/ipa/rkisp1/ipa_context.h | 20 ++----\n 4 files changed, 48 insertions(+), 107 deletions(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\nindex b3c00bef9b7e..1c572055acdd 100644\n--- a/src/ipa/rkisp1/algorithms/awb.cpp\n+++ b/src/ipa/rkisp1/algorithms/awb.cpp\n@@ -45,12 +45,8 @@ Awb::Awb()\n int Awb::configure(IPAContext &context,\n \t\t const IPACameraSensorInfo &configInfo)\n {\n-\tcontext.activeState.awb.gains.manual.red = 1.0;\n-\tcontext.activeState.awb.gains.manual.blue = 1.0;\n-\tcontext.activeState.awb.gains.manual.green = 1.0;\n-\tcontext.activeState.awb.gains.automatic.red = 1.0;\n-\tcontext.activeState.awb.gains.automatic.blue = 1.0;\n-\tcontext.activeState.awb.gains.automatic.green = 1.0;\n+\tcontext.activeState.awb.gains.manual = RGB<double>({ 1.0, 1.0, 1.0 });\n+\tcontext.activeState.awb.gains.automatic = RGB<double>({ 1.0, 1.0, 1.0 });\n \tcontext.activeState.awb.autoEnabled = true;\n \n \t/*\n@@ -87,21 +83,17 @@ void Awb::queueRequest(IPAContext &context,\n \n \tconst auto &colourGains = controls.get(controls::ColourGains);\n \tif (colourGains && !awb.autoEnabled) {\n-\t\tawb.gains.manual.red = (*colourGains)[0];\n-\t\tawb.gains.manual.blue = (*colourGains)[1];\n+\t\tawb.gains.manual.r() = (*colourGains)[0];\n+\t\tawb.gains.manual.b() = (*colourGains)[1];\n \n \t\tLOG(RkISP1Awb, Debug)\n-\t\t\t<< \"Set colour gains to red: \" << awb.gains.manual.red\n-\t\t\t<< \", blue: \" << awb.gains.manual.blue;\n+\t\t\t<< \"Set colour gains to \" << awb.gains.manual;\n \t}\n \n \tframeContext.awb.autoEnabled = awb.autoEnabled;\n \n-\tif (!awb.autoEnabled) {\n-\t\tframeContext.awb.gains.red = awb.gains.manual.red;\n-\t\tframeContext.awb.gains.green = 1.0;\n-\t\tframeContext.awb.gains.blue = awb.gains.manual.blue;\n-\t}\n+\tif (!awb.autoEnabled)\n+\t\tframeContext.awb.gains = awb.gains.manual;\n }\n \n /**\n@@ -114,19 +106,16 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n \t * This is the latest time we can read the active state. This is the\n \t * most up-to-date automatic values we can read.\n \t */\n-\tif (frameContext.awb.autoEnabled) {\n-\t\tframeContext.awb.gains.red = context.activeState.awb.gains.automatic.red;\n-\t\tframeContext.awb.gains.green = context.activeState.awb.gains.automatic.green;\n-\t\tframeContext.awb.gains.blue = context.activeState.awb.gains.automatic.blue;\n-\t}\n+\tif (frameContext.awb.autoEnabled)\n+\t\tframeContext.awb.gains = context.activeState.awb.gains.automatic;\n \n \tauto gainConfig = params->block<BlockType::AwbGain>();\n \tgainConfig.setEnabled(true);\n \n-\tgainConfig->gain_green_b = std::clamp<int>(256 * frameContext.awb.gains.green, 0, 0x3ff);\n-\tgainConfig->gain_blue = std::clamp<int>(256 * frameContext.awb.gains.blue, 0, 0x3ff);\n-\tgainConfig->gain_red = std::clamp<int>(256 * frameContext.awb.gains.red, 0, 0x3ff);\n-\tgainConfig->gain_green_r = std::clamp<int>(256 * frameContext.awb.gains.green, 0, 0x3ff);\n+\tgainConfig->gain_green_b = std::clamp<int>(256 * frameContext.awb.gains.g(), 0, 0x3ff);\n+\tgainConfig->gain_blue = std::clamp<int>(256 * frameContext.awb.gains.b(), 0, 0x3ff);\n+\tgainConfig->gain_red = std::clamp<int>(256 * frameContext.awb.gains.r(), 0, 0x3ff);\n+\tgainConfig->gain_green_r = std::clamp<int>(256 * frameContext.awb.gains.g(), 0, 0x3ff);\n \n \t/* If we have already set the AWB measurement parameters, return. */\n \tif (frame > 0)\n@@ -178,12 +167,12 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n \t}\n }\n \n-uint32_t Awb::estimateCCT(double red, double green, double blue)\n+uint32_t Awb::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);\n@@ -206,14 +195,12 @@ 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-\t\t\tstatic_cast<float>(frameContext.awb.gains.red),\n-\t\t\tstatic_cast<float>(frameContext.awb.gains.blue)\n+\t\t\tstatic_cast<float>(frameContext.awb.gains.r()),\n+\t\t\tstatic_cast<float>(frameContext.awb.gains.b())\n \t\t});\n \tmetadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n \n@@ -223,9 +210,9 @@ 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.r() = awb->awb_mean[0].mean_cr_or_r;\n+\t\trgbMeans.g() = awb->awb_mean[0].mean_y_or_g;\n+\t\trgbMeans.b() = awb->awb_mean[0].mean_cb_or_b;\n \t} else {\n \t\t/* Get the YCbCr mean values */\n \t\tdouble yMean = awb->awb_mean[0].mean_y_or_g;\n@@ -247,9 +234,9 @@ 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.r() = 1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean;\n+\t\trgbMeans.g() = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean;\n+\t\trgbMeans.b() = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean;\n \n \t\t/*\n \t\t * Due to hardware rounding errors in the YCbCr means, the\n@@ -257,9 +244,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@@ -267,19 +252,17 @@ 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+\trgbMeans /= frameContext.awb.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@@ -289,8 +272,11 @@ 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+\tRGB<double> gains({\n+\t\trgbMeans.g() / std::max(rgbMeans.r(), 1.0),\n+\t\t1.0,\n+\t\trgbMeans.g() / std::max(rgbMeans.b(), 1.0)\n+\t});\n \n \t/*\n \t * Clamp the gain values to the hardware, which expresses gains as Q2.8\n@@ -298,24 +284,18 @@ void Awb::process(IPAContext &context,\n \t * divisions by zero when computing the raw means in subsequent\n \t * iterations.\n \t */\n-\tredGain = std::clamp(redGain, 1.0 / 256, 1023.0 / 256);\n-\tblueGain = std::clamp(blueGain, 1.0 / 256, 1023.0 / 256);\n+\tgains = gains.max(1.0 / 256).min(1023.0 / 256);\n \n \t/* Filter the values to avoid oscillations. */\n \tdouble speed = 0.2;\n-\tredGain = speed * redGain + (1 - speed) * activeState.awb.gains.automatic.red;\n-\tblueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.automatic.blue;\n+\tgains = gains * speed + activeState.awb.gains.automatic * (1 - speed);\n \n-\tactiveState.awb.gains.automatic.red = redGain;\n-\tactiveState.awb.gains.automatic.blue = blueGain;\n-\tactiveState.awb.gains.automatic.green = 1.0;\n+\tactiveState.awb.gains.automatic = gains;\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<< activeState.awb.gains.automatic.green << \", \"\n-\t\t<< activeState.awb.gains.automatic.blue << \"], temp \"\n+\t\t<< \"Means \" << rgbMeans << \", gains \"\n+\t\t<< activeState.awb.gains.automatic << \", temp \"\n \t\t<< activeState.awb.temperatureK << \"K\";\n }\n \ndiff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\nindex b3b2c0bbb9ae..058c0fc53490 100644\n--- a/src/ipa/rkisp1/algorithms/awb.h\n+++ b/src/ipa/rkisp1/algorithms/awb.h\n@@ -32,7 +32,7 @@ public:\n \t\t ControlList &metadata) override;\n \n private:\n-\tuint32_t estimateCCT(double red, double green, double blue);\n+\tuint32_t estimateCCT(const RGB<double> &rgb);\n \n \tbool rgbMode_;\n };\ndiff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\nindex 14d0c02a2b32..8f545cd76d52 100644\n--- a/src/ipa/rkisp1/ipa_context.cpp\n+++ b/src/ipa/rkisp1/ipa_context.cpp\n@@ -188,30 +188,12 @@ namespace libcamera::ipa::rkisp1 {\n * \\struct IPAActiveState::awb.gains\n * \\brief White balance gains\n *\n- * \\struct IPAActiveState::awb.gains.manual\n+ * \\var IPAActiveState::awb.gains.manual\n * \\brief Manual white balance gains (set through requests)\n *\n- * \\var IPAActiveState::awb.gains.manual.red\n- * \\brief Manual white balance gain for R channel\n- *\n- * \\var IPAActiveState::awb.gains.manual.green\n- * \\brief Manual white balance gain for G channel\n- *\n- * \\var IPAActiveState::awb.gains.manual.blue\n- * \\brief Manual white balance gain for B channel\n- *\n- * \\struct IPAActiveState::awb.gains.automatic\n+ * \\var IPAActiveState::awb.gains.automatic\n * \\brief Automatic white balance gains (computed by the algorithm)\n *\n- * \\var IPAActiveState::awb.gains.automatic.red\n- * \\brief Automatic white balance gain for R channel\n- *\n- * \\var IPAActiveState::awb.gains.automatic.green\n- * \\brief Automatic white balance gain for G channel\n- *\n- * \\var IPAActiveState::awb.gains.automatic.blue\n- * \\brief Automatic white balance gain for B channel\n- *\n * \\var IPAActiveState::awb.temperatureK\n * \\brief Estimated color temperature\n *\n@@ -333,15 +315,6 @@ namespace libcamera::ipa::rkisp1 {\n * \\struct IPAFrameContext::awb.gains\n * \\brief White balance gains\n *\n- * \\var IPAFrameContext::awb.gains.red\n- * \\brief White balance gain for R channel\n- *\n- * \\var IPAFrameContext::awb.gains.green\n- * \\brief White balance gain for G channel\n- *\n- * \\var IPAFrameContext::awb.gains.blue\n- * \\brief White balance gain for B channel\n- *\n * \\var IPAFrameContext::awb.temperatureK\n * \\brief Estimated color temperature\n *\ndiff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\nindex 7b93a9e9461d..b4dec0c3288d 100644\n--- a/src/ipa/rkisp1/ipa_context.h\n+++ b/src/ipa/rkisp1/ipa_context.h\n@@ -25,6 +25,7 @@\n #include <libipa/camera_sensor_helper.h>\n #include <libipa/fc_queue.h>\n #include <libipa/matrix.h>\n+#include <libipa/vector.h>\n \n namespace libcamera {\n \n@@ -87,16 +88,8 @@ struct IPAActiveState {\n \n \tstruct {\n \t\tstruct {\n-\t\t\tstruct {\n-\t\t\t\tdouble red;\n-\t\t\t\tdouble green;\n-\t\t\t\tdouble blue;\n-\t\t\t} manual;\n-\t\t\tstruct {\n-\t\t\t\tdouble red;\n-\t\t\t\tdouble green;\n-\t\t\t\tdouble blue;\n-\t\t\t} automatic;\n+\t\t\tRGB<double> manual;\n+\t\t\tRGB<double> automatic;\n \t\t} gains;\n \n \t\tunsigned int temperatureK;\n@@ -140,12 +133,7 @@ struct IPAFrameContext : public FrameContext {\n \t} agc;\n \n \tstruct {\n-\t\tstruct {\n-\t\t\tdouble red;\n-\t\t\tdouble green;\n-\t\t\tdouble blue;\n-\t\t} gains;\n-\n+\t\tRGB<double> gains;\n \t\tbool autoEnabled;\n \t} awb;\n \n", "prefixes": [ "RFC", "v2", "11/12" ] }