[{"id":38552,"web_url":"https://patchwork.libcamera.org/comment/38552/","msgid":"<20260408210813.GG1965119@killaraus.ideasonboard.com>","date":"2026-04-08T21:08:13","subject":"Re: [PATCH 04/13] libcamera: software_isp: Apply gains in CPU ISP","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 07, 2026 at 11:01:07PM +0100, Kieran Bingham wrote:\n> From: Milan Zamazal <mzamazal@redhat.com>\n> \n> One of the preceding patches removed white balance gains from the\n> combined matrix.  Which is correct but the gains are not applied now in\n> CPU ISP when a CCM is used.\n> \n> This patch applies the gains in the CCM table.  It also clamps the pixel\n> values if they are out of range after applying the gains.\n> \n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/software_isp/debayer_cpu.cpp | 45 +++++++++++++++---------------\n>  1 file changed, 23 insertions(+), 22 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index 5356d6bbec11c30fa0b05659d44a91e69e2b79d0..2dc0df5597938cc19317187f7b1ed8a10b1cc111 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -911,45 +911,46 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\n>  \t};\n>  \tconst unsigned int gammaTableSize = gammaTable_.size();\n>  \tRGB<float> blackIndex = params.blackLevel * kRGBLookupSize;\n> +\tRGB<float> gains = params.gains;\n\nThis is never modified,\n\n\tconst RGB<float> &gains = params.gains;\n\n> +\tconst RGB<float> div = (RGB<float>(kRGBLookupSize) - blackIndex).max(1.0);\n>  \n>  \tif (ccmEnabled_) {\n>  \t\tif (gammaUpdateNeeded ||\n> -\t\t    matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {\n> +\t\t    matrixChanged(params.combinedMatrix, params_.combinedMatrix) ||\n> +\t\t    params.gains != params_.gains) {\n>  \t\t\tauto &red = swapRedBlueGains_ ? blueCcm_ : redCcm_;\n>  \t\t\tauto &green = greenCcm_;\n>  \t\t\tauto &blue = swapRedBlueGains_ ? redCcm_ : blueCcm_;\n> -\t\t\tconst unsigned int redIndex = swapRedBlueGains_ ? 2 : 0;\n> -\t\t\tconst unsigned int greenIndex = 1;\n> -\t\t\tconst unsigned int blueIndex = swapRedBlueGains_ ? 0 : 2;\n> -\t\t\tconst RGB<float> div =\n> -\t\t\t\t(RGB<float>(kRGBLookupSize) - blackIndex).max(1.0) /\n> -\t\t\t\tkRGBLookupSize;\n>  \t\t\tfor (unsigned int i = 0; i < kRGBLookupSize; i++) {\n> -\t\t\t\tconst RGB<float> rgb = ((RGB<float>(i) - blackIndex) / div).max(0.0);\n> -\t\t\t\tred[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][0]);\n> -\t\t\t\tred[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][0]);\n> -\t\t\t\tred[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][0]);\n> -\t\t\t\tgreen[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][1]);\n> -\t\t\t\tgreen[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][1]);\n> -\t\t\t\tgreen[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][1]);\n> -\t\t\t\tblue[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][2]);\n> -\t\t\t\tblue[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][2]);\n> -\t\t\t\tblue[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][2]);\n> +\t\t\t\tconst RGB<float> rgb =\n> +\t\t\t\t\t(gains * (RGB<float>(i) - blackIndex) * kRGBLookupSize / div)\n> +\t\t\t\t\t\t.min(kRGBLookupSize - 1)\n> +\t\t\t\t\t\t.max(0.0);\n\nMaybe this would be a bit more readable ?\n\n\t\t\t\tRGB<float> rgb = gains * (RGB<float>(i) - blackIndex)\n\t\t\t\t\t       * kRGBLookupSize / div;\n\t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n\n(maybe we should add a clamp() member function to Vector)\n\nYou could also add a variable before the loop:\n\n\t\t\tconst float scale = kRGBLookupSize / div;\n\nand write here\n\n\t\t\t\tRGB<float> rgb = gains * (RGB<float>(i) - blackIndex) * scale;\n\t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n\nOr even scale the gains before the loop\n\n\t\t\tconst RGB<float> gains = params.gain * kRGBLookupSize / div;\n\n\t\t\t...\n\n\t\t\t\tRGB<float> rgb = (RGB<float>(i) - blackIndex) * gains;\n\t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n\nI'm starting to think the could would actually become more readable if\nwe dropped the div variable.\n\n\t\t\t/*\n\t\t\t * Combine the colour gains with the black level\n\t\t\t * subtraction scaling factor. Clamp the black level to\n\t\t\t * an arbitrary limit of 0.5 to avoid divisions by 0.\n\t\t\t */\n\t\t\tconst RGB<float> blackLevel = params.blackLevel.min(0.5);\n\t\t\tconst RGB<float> gains = params.gain / (RGB<float>(1.0) - blackLevel);\n\n\t\t\t...\n\n\t\t\t\tRGB<float> rgb = (RGB<float>(i) - blackIndex) * gains;\n\t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n\n(possibly combining the two lines in one if you want to make the\nvariable const)\n\nThe gain calculation now uses variables in the [0.0, 1.0] range only\ninstead of mixing them with [0, kRGBLookupSize-1], combined with the\ncomment I think this is as readable as it can get.\n\nThe second branch of the if below should be adapted similarly.\n\nI'm pretty sure this invalidates some of my comments in 03/13, sorry\nabout that.\n\n> +\t\t\t\tred[i].r = std::round(rgb.r() * params.combinedMatrix[0][0]);\n> +\t\t\t\tred[i].g = std::round(rgb.g() * params.combinedMatrix[1][0]);\n> +\t\t\t\tred[i].b = std::round(rgb.b() * params.combinedMatrix[2][0]);\n> +\t\t\t\tgreen[i].r = std::round(rgb.r() * params.combinedMatrix[0][1]);\n> +\t\t\t\tgreen[i].g = std::round(rgb.g() * params.combinedMatrix[1][1]);\n> +\t\t\t\tgreen[i].b = std::round(rgb.b() * params.combinedMatrix[2][1]);\n> +\t\t\t\tblue[i].r = std::round(rgb.r() * params.combinedMatrix[0][2]);\n> +\t\t\t\tblue[i].g = std::round(rgb.g() * params.combinedMatrix[1][2]);\n> +\t\t\t\tblue[i].b = std::round(rgb.b() * params.combinedMatrix[2][2]);\n> +\t\t\t\tif (swapRedBlueGains_) {\n> +\t\t\t\t\tstd::swap(red[i].r, red[i].b);\n> +\t\t\t\t\tstd::swap(green[i].r, green[i].b);\n> +\t\t\t\t\tstd::swap(blue[i].r, blue[i].b);\n> +\t\t\t\t}\n\nI think there's a change in behaviour here. Before this patch, when\nswapping red and blue gains, we had\n\n\tred.r = rgb.r * params.combinedMatrix[2][0]\n\tred.b = rgb.b * params.combinedMatrix[0][0]\n\nWith this patch, we first compute\n\n\tred.r = rgb.r * params.combinedMatrix[0][0]\n\tred.b = rgb.b * params.combinedMatrix[2][0]\n\nand then swap red.r and red.b, so\n\n\tred.r = rgb.b * params.combinedMatrix[2][0]\n\tred.b = rgb.r * params.combinedMatrix[0][0]\n\nNote how rgb.r and rgb.b have been swapped.\n\n>  \t\t\t\tgammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n>  \t\t\t}\n>  \t\t}\n>  \t} else {\n>  \t\tif (gammaUpdateNeeded || params.gains != params_.gains) {\n> -\t\t\tauto &gains = params.gains;\n>  \t\t\tauto &red = swapRedBlueGains_ ? blue_ : red_;\n>  \t\t\tauto &green = green_;\n>  \t\t\tauto &blue = swapRedBlueGains_ ? red_ : blue_;\n> -\t\t\tconst RGB<float> div =\n> -\t\t\t\t(RGB<float>(kRGBLookupSize) - blackIndex).max(1.0) /\n> -\t\t\t\tgammaTableSize;\n>  \t\t\tfor (unsigned int i = 0; i < kRGBLookupSize; i++) {\n>  \t\t\t\tconst RGB<float> lutGains =\n> -\t\t\t\t\t(gains * (RGB<float>(i) - blackIndex) / div)\n> +\t\t\t\t\t(gains * (RGB<float>(i) - blackIndex) * gammaTableSize / div)\n>  \t\t\t\t\t\t.min(gammaTableSize - 1)\n>  \t\t\t\t\t\t.max(0.0);\n\nSo here I think it would become\n\n\t\t\t/*\n\t\t\t * Combine the colour gains with the black level\n\t\t\t * subtraction scaling factor. Clamp the black level to\n\t\t\t * an arbitrary limit of 0.5 to avoid divisions by 0.\n\t\t\t */\n\t\t\tconst RGB<float> blackLevel = params.blackLevel.min(0.5);\n\t\t\tconst RGB<float> gains = params.gain / (RGB<float>(1.0) - blackLevel);\n\n\t\t\t...\n\n\t\t\t\tRGB<float> rgb = (RGB<float>(i) - blackIndex) * gains\n\t\t\t\t\t       * gammaTableSize / kRGBLookupSize;\n\t\t\t\trgb = rgb.max(0.0).min(gammaTableSize - 1);\n\nThe blackLevel and gains variables being identical in the two branches,\nthey can be moved to the top of the function.\n\n>  \t\t\t\tred[i] = gammaTable_[lutGains.r()];\n>","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 6D662BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  8 Apr 2026 21:08:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9106C62E31;\n\tWed,  8 Apr 2026 23:08:15 +0200 (CEST)","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 AEF3562E08\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  8 Apr 2026 23:08:14 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-703d-e500--2a1.rev.dnainternet.fi\n\t[IPv6:2001:14ba:703d:e500::2a1])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1DB901BA;\n\tWed,  8 Apr 2026 23:06:46 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MIBT/uyj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1775682406;\n\tbh=MYtLQpXPq9ZErz+tKV0hup1tifM680Xz4d/LmvF2los=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MIBT/uyjbmeu5T3/qLDsGR8e8H0VfUFq/F97ch35jbe+GyGkKx4SFhUfD7nZWDg5n\n\tHtrY0nbiV8kjZ7+nQqxYRkvCk9Bi0bP4qbwrjs+eJDlDsxaoqM6gyXy1d6Lx7sOSOy\n\tbzpo8lUjypoiu+tI1fOW5OP7Nx3FbMH4AEpTEmvs=","Date":"Thu, 9 Apr 2026 00:08:13 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal <mzamazal@redhat.com>","Subject":"Re: [PATCH 04/13] libcamera: software_isp: Apply gains in CPU ISP","Message-ID":"<20260408210813.GG1965119@killaraus.ideasonboard.com>","References":"<20260407-kbingham-awb-split-v1-0-a39af3f4dc20@ideasonboard.com>\n\t<20260407-kbingham-awb-split-v1-4-a39af3f4dc20@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260407-kbingham-awb-split-v1-4-a39af3f4dc20@ideasonboard.com>","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>"}},{"id":38564,"web_url":"https://patchwork.libcamera.org/comment/38564/","msgid":"<85zf3c9vkf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-04-09T12:17:52","subject":"Re: [PATCH 04/13] libcamera: software_isp: Apply gains in CPU ISP","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for review.\n\nLaurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n\n> On Tue, Apr 07, 2026 at 11:01:07PM +0100, Kieran Bingham wrote:\n>> From: Milan Zamazal <mzamazal@redhat.com>\n>> \n>\n>> One of the preceding patches removed white balance gains from the\n>> combined matrix.  Which is correct but the gains are not applied now in\n>> CPU ISP when a CCM is used.\n>> \n>> This patch applies the gains in the CCM table.  It also clamps the pixel\n>> values if they are out of range after applying the gains.\n>> \n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>  src/libcamera/software_isp/debayer_cpu.cpp | 45 +++++++++++++++---------------\n>>  1 file changed, 23 insertions(+), 22 deletions(-)\n>> \n>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>> index 5356d6bbec11c30fa0b05659d44a91e69e2b79d0..2dc0df5597938cc19317187f7b1ed8a10b1cc111 100644\n>> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>> @@ -911,45 +911,46 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\n>>  \t};\n>>  \tconst unsigned int gammaTableSize = gammaTable_.size();\n>>  \tRGB<float> blackIndex = params.blackLevel * kRGBLookupSize;\n>> +\tRGB<float> gains = params.gains;\n>\n> This is never modified,\n>\n> \tconst RGB<float> &gains = params.gains;\n>\n>> +\tconst RGB<float> div = (RGB<float>(kRGBLookupSize) - blackIndex).max(1.0);\n>>  \n>>  \tif (ccmEnabled_) {\n>>  \t\tif (gammaUpdateNeeded ||\n>> -\t\t    matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {\n>> +\t\t    matrixChanged(params.combinedMatrix, params_.combinedMatrix) ||\n>> +\t\t    params.gains != params_.gains) {\n>>  \t\t\tauto &red = swapRedBlueGains_ ? blueCcm_ : redCcm_;\n>>  \t\t\tauto &green = greenCcm_;\n>>  \t\t\tauto &blue = swapRedBlueGains_ ? redCcm_ : blueCcm_;\n>> -\t\t\tconst unsigned int redIndex = swapRedBlueGains_ ? 2 : 0;\n>> -\t\t\tconst unsigned int greenIndex = 1;\n>> -\t\t\tconst unsigned int blueIndex = swapRedBlueGains_ ? 0 : 2;\n>> -\t\t\tconst RGB<float> div =\n>> -\t\t\t\t(RGB<float>(kRGBLookupSize) - blackIndex).max(1.0) /\n>> -\t\t\t\tkRGBLookupSize;\n>>  \t\t\tfor (unsigned int i = 0; i < kRGBLookupSize; i++) {\n>> -\t\t\t\tconst RGB<float> rgb = ((RGB<float>(i) - blackIndex) / div).max(0.0);\n>> -\t\t\t\tred[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][0]);\n>> -\t\t\t\tred[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][0]);\n>> -\t\t\t\tred[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][0]);\n>> -\t\t\t\tgreen[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][1]);\n>> -\t\t\t\tgreen[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][1]);\n>> -\t\t\t\tgreen[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][1]);\n>> -\t\t\t\tblue[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][2]);\n>> -\t\t\t\tblue[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][2]);\n>> -\t\t\t\tblue[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][2]);\n>> +\t\t\t\tconst RGB<float> rgb =\n>> +\t\t\t\t\t(gains * (RGB<float>(i) - blackIndex) * kRGBLookupSize / div)\n>> +\t\t\t\t\t\t.min(kRGBLookupSize - 1)\n>> +\t\t\t\t\t\t.max(0.0);\n>\n> Maybe this would be a bit more readable ?\n>\n> \t\t\t\tRGB<float> rgb = gains * (RGB<float>(i) - blackIndex)\n> \t\t\t\t\t       * kRGBLookupSize / div;\n> \t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n>\n> (maybe we should add a clamp() member function to Vector)\n>\n> You could also add a variable before the loop:\n>\n> \t\t\tconst float scale = kRGBLookupSize / div;\n>\n> and write here\n>\n> \t\t\t\tRGB<float> rgb = gains * (RGB<float>(i) - blackIndex) * scale;\n> \t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n>\n> Or even scale the gains before the loop\n>\n> \t\t\tconst RGB<float> gains = params.gain * kRGBLookupSize / div;\n>\n> \t\t\t...\n>\n> \t\t\t\tRGB<float> rgb = (RGB<float>(i) - blackIndex) * gains;\n> \t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n>\n> I'm starting to think the could would actually become more readable if\n> we dropped the div variable.\n>\n> \t\t\t/*\n> \t\t\t * Combine the colour gains with the black level\n> \t\t\t * subtraction scaling factor. Clamp the black level to\n> \t\t\t * an arbitrary limit of 0.5 to avoid divisions by 0.\n> \t\t\t */\n> \t\t\tconst RGB<float> blackLevel = params.blackLevel.min(0.5);\n> \t\t\tconst RGB<float> gains = params.gain / (RGB<float>(1.0) - blackLevel);\n>\n> \t\t\t...\n>\n> \t\t\t\tRGB<float> rgb = (RGB<float>(i) - blackIndex) * gains;\n> \t\t\t\trgb = rgb.max(0.0).min(kRGBLookupSize - 1);\n>\n> (possibly combining the two lines in one if you want to make the\n> variable const)\n>\n> The gain calculation now uses variables in the [0.0, 1.0] range only\n> instead of mixing them with [0, kRGBLookupSize-1], combined with the\n> comment I think this is as readable as it can get.\n>\n> The second branch of the if below should be adapted similarly.\n>\n> I'm pretty sure this invalidates some of my comments in 03/13, sorry\n> about that.\n>\n>> +\t\t\t\tred[i].r = std::round(rgb.r() * params.combinedMatrix[0][0]);\n>> +\t\t\t\tred[i].g = std::round(rgb.g() * params.combinedMatrix[1][0]);\n>> +\t\t\t\tred[i].b = std::round(rgb.b() * params.combinedMatrix[2][0]);\n>> +\t\t\t\tgreen[i].r = std::round(rgb.r() * params.combinedMatrix[0][1]);\n>> +\t\t\t\tgreen[i].g = std::round(rgb.g() * params.combinedMatrix[1][1]);\n>> +\t\t\t\tgreen[i].b = std::round(rgb.b() * params.combinedMatrix[2][1]);\n>> +\t\t\t\tblue[i].r = std::round(rgb.r() * params.combinedMatrix[0][2]);\n>> +\t\t\t\tblue[i].g = std::round(rgb.g() * params.combinedMatrix[1][2]);\n>> +\t\t\t\tblue[i].b = std::round(rgb.b() * params.combinedMatrix[2][2]);\n>> +\t\t\t\tif (swapRedBlueGains_) {\n>> +\t\t\t\t\tstd::swap(red[i].r, red[i].b);\n>> +\t\t\t\t\tstd::swap(green[i].r, green[i].b);\n>> +\t\t\t\t\tstd::swap(blue[i].r, blue[i].b);\n>> +\t\t\t\t}\n>\n> I think there's a change in behaviour here. \n>\n> Before this patch, when swapping red and blue gains, we had\n>\n> \tred.r = rgb.r * params.combinedMatrix[2][0]\n> \tred.b = rgb.b * params.combinedMatrix[0][0]\n>\n> With this patch, we first compute\n>\n> \tred.r = rgb.r * params.combinedMatrix[0][0]\n> \tred.b = rgb.b * params.combinedMatrix[2][0]\n>\n> and then swap red.r and red.b, so\n>\n> \tred.r = rgb.b * params.combinedMatrix[2][0]\n> \tred.b = rgb.r * params.combinedMatrix[0][0]\n>\n> Note how rgb.r and rgb.b have been swapped.\n\nIndeed, good catch.  The bug is in the preceding black level patch,\n`red' table items should be all multiplied by rgb.r(), similarly for the\nother colours.\n\n>>  \t\t\t\tgammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n>>  \t\t\t}\n>>  \t\t}\n>>  \t} else {\n>>  \t\tif (gammaUpdateNeeded || params.gains != params_.gains) {\n>> -\t\t\tauto &gains = params.gains;\n>>  \t\t\tauto &red = swapRedBlueGains_ ? blue_ : red_;\n>>  \t\t\tauto &green = green_;\n>>  \t\t\tauto &blue = swapRedBlueGains_ ? red_ : blue_;\n>> -\t\t\tconst RGB<float> div =\n>> -\t\t\t\t(RGB<float>(kRGBLookupSize) - blackIndex).max(1.0) /\n>> -\t\t\t\tgammaTableSize;\n>>  \t\t\tfor (unsigned int i = 0; i < kRGBLookupSize; i++) {\n>>  \t\t\t\tconst RGB<float> lutGains =\n>> -\t\t\t\t\t(gains * (RGB<float>(i) - blackIndex) / div)\n>> +\t\t\t\t\t(gains * (RGB<float>(i) - blackIndex) * gammaTableSize / div)\n>>  \t\t\t\t\t\t.min(gammaTableSize - 1)\n>>  \t\t\t\t\t\t.max(0.0);\n>\n> So here I think it would become\n>\n> \t\t\t/*\n> \t\t\t * Combine the colour gains with the black level\n> \t\t\t * subtraction scaling factor. Clamp the black level to\n> \t\t\t * an arbitrary limit of 0.5 to avoid divisions by 0.\n> \t\t\t */\n> \t\t\tconst RGB<float> blackLevel = params.blackLevel.min(0.5);\n> \t\t\tconst RGB<float> gains = params.gain / (RGB<float>(1.0) - blackLevel);\n>\n> \t\t\t...\n>\n> \t\t\t\tRGB<float> rgb = (RGB<float>(i) - blackIndex) * gains\n> \t\t\t\t\t       * gammaTableSize / kRGBLookupSize;\n> \t\t\t\trgb = rgb.max(0.0).min(gammaTableSize - 1);\n>\n> The blackLevel and gains variables being identical in the two branches,\n> they can be moved to the top of the function.\n>\n>>  \t\t\t\tred[i] = gammaTable_[lutGains.r()];\n>>","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 5BE37BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  9 Apr 2026 12:18:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74C6262E48;\n\tThu,  9 Apr 2026 14:18:02 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E5F0662010\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  9 Apr 2026 14:18:00 +0200 (CEST)","from mail-wr1-f72.google.com (mail-wr1-f72.google.com\n\t[209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-529-9gsj-j-XOAuZDsR0OZFm4Q-1; Thu, 09 Apr 2026 08:17:58 -0400","by mail-wr1-f72.google.com with SMTP id\n\tffacd0b85a97d-43cf5b4dac8so859537f8f.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 09 Apr 2026 05:17:58 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-43d5e969bbfsm4162732f8f.1.2026.04.09.05.17.53\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Apr 2026 05:17:53 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"YjT6zKYq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1775737079;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=/2wGHHVDi8KREYZ9zjiMGvtqXr5Kt+zMF1oRTCsH+ik=;\n\tb=YjT6zKYqs32jW8IlUNy+6moKWH8eBAQpTHtKvvAUmfj9y8fnExuSPvqe88ms4diqE+6Iv5\n\taQjY+WLoPP+uVW5XjkK/7HBmJUEgVHowmJuy0RNeqRoMjrsrQaUY3dLBKS1RvyB5a06ZT6\n\tqLoklTgcsfn7Wz85175sDzNllVjo2Qc=","X-MC-Unique":"9gsj-j-XOAuZDsR0OZFm4Q-1","X-Mimecast-MFC-AGG-ID":"9gsj-j-XOAuZDsR0OZFm4Q_1775737077","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1775737077; x=1776341877;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=/2wGHHVDi8KREYZ9zjiMGvtqXr5Kt+zMF1oRTCsH+ik=;\n\tb=jbN/dN5/8oaq1orl6onyNRyv3Q0kJNJ3XmWXj+ba/0DUex2bOhwWvZs/pbKHo20EFH\n\twirFdv7D3jzfEU1oC3XqxYYiTlF1WLR6130sK5gA9NnCz7H8/83JD8ZlRgmI+vdt3Up/\n\toHlvYGOdyV6g8XKs+HwwK5ZEzUH/DZt5M6nEgww1vAnQGIYoxg+ilbzMSj9SLwnOxMsu\n\t5UC+Vsh7Z0Q4wOjkj2VGjghNq5zoz6O/HUD0SNK+Lhkb2T1eAIPnEKCJrs51qLNu3nAR\n\tM/WM65oCK7Z6eqsK6fmsx1WRkNQ6BJv56c4RmcouyC8CGdGArFESEL/co+aTEXw80mVj\n\tHxzw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXLBkMRWNDDFimNa7nL3anW3VaDCNV0EmnCV/fZO8UviY9ehwapvM97kWoybxEYLQuxsqvvf+x2zwP+k8UYCKM=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxG+ZqhJsyZdSmSyTp1qEl2b0qfndqeyxK2/Cwygl8uGZDOxi4E\n\tpO5Mgm2pZwAa7A5PxUcnBnunojGYgmhpBqyGqRUkSIfUUFSL92QioanI3xGsjQCwjGUUpYyRd0M\n\tAMvLU7G53NMBEtSphSC3S1/8svmvL7BehU9vJTTb23MViEm1LXa/nEBLDy+NaCMQhqU/0wrVbPE\n\tNMAA7Y68qDTQO+jKB2S1QPriGxqNeqjWyw+0XZkAfZj6mpfZE3/6yk3c/lS+M=","X-Gm-Gg":"AeBDietfSFlbcqkB5fKY05PDkUS/BmMJSBiOyLwcwmMcmDM1GK6KnDO6ac7nJoZx4Ny\n\tPbTXlz/xBWS+jWeIYDf7gbkcLSeQGsnFAZiP5FcijDNSBKYDv022v5zPhh+TYWiZbKcA9zgH392\n\t+zU3v5Dk2F3pUwqOsk4XOt6z48ysCRt1Y8qcRZAJl/OJrGQ6RAvt/XjBQi+n2o44cLj6NNJP0Dy\n\t8mxsrbbfc+fkCm7V64vNkBLUA2L+/21TPEFpzCISdXCpP3AuEdv3XYmUarVBlbMSrzaEfPI+094\n\tNu9WHwdXC7QEe6BJRf0cIwoVQZYVZv8lIlo+UO+iL6RBNMJLRwDemsvs8Q4e/alwqGtE00dI28A\n\tT/F8nv/+sJn1mvJncJTT68IokuEO0mYbTZReZZigMbnQkjePiBDrdJNaz2hIK7PtaE8liK65jtO\n\t0=","X-Received":["by 2002:a05:6000:1842:b0:43c:fe59:6696 with SMTP id\n\tffacd0b85a97d-43d2930043cmr36404230f8f.46.1775737076757; \n\tThu, 09 Apr 2026 05:17:56 -0700 (PDT)","by 2002:a05:6000:1842:b0:43c:fe59:6696 with SMTP id\n\tffacd0b85a97d-43d2930043cmr36404063f8f.46.1775737074871; \n\tThu, 09 Apr 2026 05:17:54 -0700 (PDT)"],"From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 04/13] libcamera: software_isp: Apply gains in CPU ISP","In-Reply-To":"<20260408210813.GG1965119@killaraus.ideasonboard.com> (Laurent\n\tPinchart's message of \"Thu, 9 Apr 2026 00:08:13 +0300\")","References":"<20260407-kbingham-awb-split-v1-0-a39af3f4dc20@ideasonboard.com>\n\t<20260407-kbingham-awb-split-v1-4-a39af3f4dc20@ideasonboard.com>\n\t<20260408210813.GG1965119@killaraus.ideasonboard.com>","Date":"Thu, 09 Apr 2026 14:17:52 +0200","Message-ID":"<85zf3c9vkf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"QWdwY7Xd12sXoBw6qzGGqf9buSvV1MJtr-B3jaHvzHw_1775737077","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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>"}}]