[{"id":38524,"web_url":"https://patchwork.libcamera.org/comment/38524/","msgid":"<20260408003407.GO1268443@killaraus.ideasonboard.com>","date":"2026-04-08T00:34:07","subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin 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:06PM +0100, Kieran Bingham wrote:\n> From: Milan Zamazal \n> \n> The black level handling in CPU ISP has two flaws:\n> \n> - The black level is applied after white balance rather than before.\n> \n> - It doesn't handle black levels with different values for individual\n> colour channels.\n> \n> The flaws are in both CCM and non-CCM cases. The wrong black level and\n> white balance application order is well visible when the white balance\n> gains are significantly different from 1.0. Then the output differs\n> significantly from GPU ISP output, which uses the correct order.\n> \n> This patch changes the computations of the lookup tables in a way that\n> fixes both the problems.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 60 +++++++++++++++---------------\n> 1 file changed, 30 insertions(+), 30 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index dd0fff8714144417467bac16a1055d1185782d14..5356d6bbec11c30fa0b05659d44a91e69e2b79d0 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -879,15 +879,12 @@ void DebayerCpuThread::process4(uint32_t frame, const uint8_t *src, uint8_t *dst\n> \n> void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> {\n> -\tconst RGB blackLevel = params.blackLevel;\n> -\t/* Take let's say the green channel black level */\n> -\tconst unsigned int blackIndex = blackLevel[1] * gammaTable_.size();\n> \tconst float gamma = params.gamma;\n> \tconst float contrastExp = params.contrastExp;\n> \n> -\tconst float divisor = gammaTable_.size() - blackIndex - 1.0;\n> -\tfor (unsigned int i = blackIndex; i < gammaTable_.size(); i++) {\n> -\t\tfloat normalized = (i - blackIndex) / divisor;\n> +\tconst float divisor = gammaTable_.size() - 1.0;\n> +\tfor (unsigned int i = 0; i < gammaTable_.size(); i++) {\n\n\nI think this could now be written as\n\n\tfor (auto [i, value] : utils::enumerate(gammaTable_)) {\n\n> +\t\tfloat normalized = i / divisor;\n> \t\t/* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> \t\t/* Apply simple S-curve */\n> \t\tif (normalized < 0.5)\n> @@ -897,14 +894,6 @@ void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> \t\tgammaTable_[i] = UINT8_MAX *\n> \t\t\t\t std::pow(normalized, gamma);\n\nand here,\n\n\t\tvalue = UINT8_MAX * std::pow(normalized, gamma);\n\nif you think that's clearer.\n\n> \t}\n> -\t/*\n> -\t * Due to CCM operations, the table lookup may reach indices below the black\n> -\t * level. Let's set the table values below black level to the minimum\n> -\t * non-black value to prevent problems when the minimum value is\n> -\t * significantly non-zero (for example, when the image should be all grey).\n> -\t */\n> -\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex,\n> -\t\t gammaTable_[blackIndex]);\n> }\n> \n> void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> @@ -916,11 +905,13 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> \tif (gammaUpdateNeeded)\n> \t\tupdateGammaTable(params);\n> \n> +\t/* Processing order: black level -> gains -> gamma */\n> \tauto matrixChanged = [](const Matrix &m1, const Matrix &m2) -> bool {\n> \t\treturn !std::equal(m1.data().begin(), m1.data().end(), m2.data().begin());\n> \t};\n> \tconst unsigned int gammaTableSize = gammaTable_.size();\n> -\tconst double div = static_cast(kRGBLookupSize) / gammaTableSize;\n> +\tRGB blackIndex = params.blackLevel * kRGBLookupSize;\n> +\n> \tif (ccmEnabled_) {\n> \t\tif (gammaUpdateNeeded ||\n> \t\t matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {\n> @@ -930,17 +921,21 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(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\tred[i].r = std::round(i * params.combinedMatrix[redIndex][0]);\n> -\t\t\t\tred[i].g = std::round(i * params.combinedMatrix[greenIndex][0]);\n> -\t\t\t\tred[i].b = std::round(i * params.combinedMatrix[blueIndex][0]);\n> -\t\t\t\tgreen[i].r = std::round(i * params.combinedMatrix[redIndex][1]);\n> -\t\t\t\tgreen[i].g = std::round(i * params.combinedMatrix[greenIndex][1]);\n> -\t\t\t\tgreen[i].b = std::round(i * params.combinedMatrix[blueIndex][1]);\n> -\t\t\t\tblue[i].r = std::round(i * params.combinedMatrix[redIndex][2]);\n> -\t\t\t\tblue[i].g = std::round(i * params.combinedMatrix[greenIndex][2]);\n> -\t\t\t\tblue[i].b = std::round(i * params.combinedMatrix[blueIndex][2]);\n> -\t\t\t\tgammaLut_[i] = gammaTable_[i / div];\n> +\t\t\t\tconst RGB rgb = ((RGB(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\tgammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n> \t\t\t}\n> \t\t}\n> \t} else {\n> @@ -949,12 +944,17 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(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\t/* Apply gamma after gain! */\n> -\t\t\t\tconst RGB lutGains = (gains * i / div).min(gammaTableSize - 1);\n> -\t\t\t\tred[i] = gammaTable_[static_cast(lutGains.r())];\n> -\t\t\t\tgreen[i] = gammaTable_[static_cast(lutGains.g())];\n> -\t\t\t\tblue[i] = gammaTable_[static_cast(lutGains.b())];\n> +\t\t\t\tconst RGB lutGains =\n> +\t\t\t\t\t(gains * (RGB(i) - blackIndex) / div)\n> +\t\t\t\t\t\t.min(gammaTableSize - 1)\n> +\t\t\t\t\t\t.max(0.0);\n> +\t\t\t\tred[i] = gammaTable_[lutGains.r()];\n> +\t\t\t\tgreen[i] = gammaTable_[lutGains.g()];\n> +\t\t\t\tblue[i] = gammaTable_[lutGains.b()];\n> \t\t\t}\n> \t\t}\n> \t}\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 7F8BBBDCBD\n\tfor ;\n\tWed, 8 Apr 2026 00:34:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AEFAA62DA6;\n\tWed, 8 Apr 2026 02:34:10 +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 49C9E62CEB\n\tfor ;\n\tWed, 8 Apr 2026 02:34:09 +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 6285F1121; \n\tWed, 8 Apr 2026 02:32:41 +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=\"uLf1Lpeh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1775608361;\n\tbh=wMuaJlelEx/9trtaVc9udD/TNvSRK78lnQCBXC2g5EE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uLf1LpehSJ3oYfVQTHOicYIInukEG2zeGJxrOQ0j98eOyTWnaLEoHPpfMCpySppAd\n\tEeYkeBgaScN7Ew9Z/HCLafvWygcVf+/e2+tj9MC/IASPL4kYU89m8uKuOPVnOrR1mF\n\tfeOE7WmJLwdqAILmk7E4YSnahAYqiyIsMqNXhiOM=","Date":"Wed, 8 Apr 2026 03:34:07 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal ","Subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin CPU ISP","Message-ID":"<20260408003407.GO1268443@killaraus.ideasonboard.com>","References":"<20260407-kbingham-awb-split-v1-0-a39af3f4dc20@ideasonboard.com>\n\t<20260407-kbingham-awb-split-v1-3-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-3-a39af3f4dc20@ideasonboard.com>","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":38551,"web_url":"https://patchwork.libcamera.org/comment/38551/","msgid":"<20260408194547.GF1965119@killaraus.ideasonboard.com>","date":"2026-04-08T19:45:47","subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin 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:06PM +0100, Kieran Bingham wrote:\n> From: Milan Zamazal \n> \n> The black level handling in CPU ISP has two flaws:\n> \n> - The black level is applied after white balance rather than before.\n> \n> - It doesn't handle black levels with different values for individual\n> colour channels.\n> \n> The flaws are in both CCM and non-CCM cases. The wrong black level and\n> white balance application order is well visible when the white balance\n> gains are significantly different from 1.0. Then the output differs\n> significantly from GPU ISP output, which uses the correct order.\n> \n> This patch changes the computations of the lookup tables in a way that\n> fixes both the problems.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 60 +++++++++++++++---------------\n> 1 file changed, 30 insertions(+), 30 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index dd0fff8714144417467bac16a1055d1185782d14..5356d6bbec11c30fa0b05659d44a91e69e2b79d0 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -879,15 +879,12 @@ void DebayerCpuThread::process4(uint32_t frame, const uint8_t *src, uint8_t *dst\n> \n> void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> {\n> -\tconst RGB blackLevel = params.blackLevel;\n> -\t/* Take let's say the green channel black level */\n> -\tconst unsigned int blackIndex = blackLevel[1] * gammaTable_.size();\n> \tconst float gamma = params.gamma;\n> \tconst float contrastExp = params.contrastExp;\n> \n> -\tconst float divisor = gammaTable_.size() - blackIndex - 1.0;\n> -\tfor (unsigned int i = blackIndex; i < gammaTable_.size(); i++) {\n> -\t\tfloat normalized = (i - blackIndex) / divisor;\n> +\tconst float divisor = gammaTable_.size() - 1.0;\n> +\tfor (unsigned int i = 0; i < gammaTable_.size(); i++) {\n> +\t\tfloat normalized = i / divisor;\n> \t\t/* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> \t\t/* Apply simple S-curve */\n> \t\tif (normalized < 0.5)\n> @@ -897,14 +894,6 @@ void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> \t\tgammaTable_[i] = UINT8_MAX *\n> \t\t\t\t std::pow(normalized, gamma);\n> \t}\n> -\t/*\n> -\t * Due to CCM operations, the table lookup may reach indices below the black\n> -\t * level. Let's set the table values below black level to the minimum\n> -\t * non-black value to prevent problems when the minimum value is\n> -\t * significantly non-zero (for example, when the image should be all grey).\n> -\t */\n> -\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex,\n> -\t\t gammaTable_[blackIndex]);\n> }\n> \n> void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> @@ -916,11 +905,13 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> \tif (gammaUpdateNeeded)\n> \t\tupdateGammaTable(params);\n> \n> +\t/* Processing order: black level -> gains -> gamma */\n> \tauto matrixChanged = [](const Matrix &m1, const Matrix &m2) -> bool {\n> \t\treturn !std::equal(m1.data().begin(), m1.data().end(), m2.data().begin());\n> \t};\n> \tconst unsigned int gammaTableSize = gammaTable_.size();\n> -\tconst double div = static_cast(kRGBLookupSize) / gammaTableSize;\n> +\tRGB blackIndex = params.blackLevel * kRGBLookupSize;\n\nThe div variable is moved back here in the next patch. We can avoid\ngoing back and forth:\n\n\tconst RGB blackIndex = params.blackLevel * kRGBLookupSize;\n\tconst RGB div = (RGB(kRGBLookupSize) - blackIndex).max(1.0);\n\nand...\n\n> +\n> \tif (ccmEnabled_) {\n> \t\tif (gammaUpdateNeeded ||\n> \t\t matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {\n> @@ -930,17 +921,21 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(kRGBLookupSize) - blackIndex).max(1.0) /\n> +\t\t\t\tkRGBLookupSize;\n\n... drop this...\n\n> \t\t\tfor (unsigned int i = 0; i < kRGBLookupSize; i++) {\n> -\t\t\t\tred[i].r = std::round(i * params.combinedMatrix[redIndex][0]);\n> -\t\t\t\tred[i].g = std::round(i * params.combinedMatrix[greenIndex][0]);\n> -\t\t\t\tred[i].b = std::round(i * params.combinedMatrix[blueIndex][0]);\n> -\t\t\t\tgreen[i].r = std::round(i * params.combinedMatrix[redIndex][1]);\n> -\t\t\t\tgreen[i].g = std::round(i * params.combinedMatrix[greenIndex][1]);\n> -\t\t\t\tgreen[i].b = std::round(i * params.combinedMatrix[blueIndex][1]);\n> -\t\t\t\tblue[i].r = std::round(i * params.combinedMatrix[redIndex][2]);\n> -\t\t\t\tblue[i].g = std::round(i * params.combinedMatrix[greenIndex][2]);\n> -\t\t\t\tblue[i].b = std::round(i * params.combinedMatrix[blueIndex][2]);\n> -\t\t\t\tgammaLut_[i] = gammaTable_[i / div];\n> +\t\t\t\tconst RGB rgb = ((RGB(i) - blackIndex) / div).max(0.0);\n\n\t\t\t\tconst RGB rgb = ((RGB(i) - blackIndex) * kRGBLookupSize / div).max(0.0);\n\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\tgammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n> \t\t\t}\n> \t\t}\n> \t} else {\n> @@ -949,12 +944,17 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(kRGBLookupSize) - blackIndex).max(1.0) /\n> +\t\t\t\tgammaTableSize;\n\n... drop too ...\n\n> \t\t\tfor (unsigned int i = 0; i < kRGBLookupSize; i++) {\n> -\t\t\t\t/* Apply gamma after gain! */\n> -\t\t\t\tconst RGB lutGains = (gains * i / div).min(gammaTableSize - 1);\n> -\t\t\t\tred[i] = gammaTable_[static_cast(lutGains.r())];\n> -\t\t\t\tgreen[i] = gammaTable_[static_cast(lutGains.g())];\n> -\t\t\t\tblue[i] = gammaTable_[static_cast(lutGains.b())];\n> +\t\t\t\tconst RGB lutGains =\n> +\t\t\t\t\t(gains * (RGB(i) - blackIndex) / div)\n\n\t\t\t\t\t(gains * (RGB(i) - blackIndex) * gammaTableSize / div)\n\nThis will reduce the churn in patch 04/13.\n\n> +\t\t\t\t\t\t.min(gammaTableSize - 1)\n> +\t\t\t\t\t\t.max(0.0);\n> +\t\t\t\tred[i] = gammaTable_[lutGains.r()];\n> +\t\t\t\tgreen[i] = gammaTable_[lutGains.g()];\n> +\t\t\t\tblue[i] = gammaTable_[lutGains.b()];\n> \t\t\t}\n> \t\t}\n> \t}\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 886A7BDCBD\n\tfor ;\n\tWed, 8 Apr 2026 19:45:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3E9D62E2C;\n\tWed, 8 Apr 2026 21:45:49 +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 10BCA62CE6\n\tfor ;\n\tWed, 8 Apr 2026 21:45:49 +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 744D5241;\n\tWed, 8 Apr 2026 21:44:20 +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=\"PORlIIsO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1775677460;\n\tbh=IHCWJ18tq+IAm9fO0s700kEYGntrMi8uj5ayEyxQxms=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PORlIIsOxSIPben/r0+8EIF/ihRL2YtGS4tqyM3hOygiQGisivylgAPZqG1/yTH/4\n\tlBW/D6PekSVuIc4THfGptXtVxgjE+IXAwfGsvy0242Hb3JDexwBupj3qi0X1AwEJ3u\n\toR1WhWy9RV3oPHju6JH00lYTVWKrlYFi7mJq3KdY=","Date":"Wed, 8 Apr 2026 22:45:47 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal ","Subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin CPU ISP","Message-ID":"<20260408194547.GF1965119@killaraus.ideasonboard.com>","References":"<20260407-kbingham-awb-split-v1-0-a39af3f4dc20@ideasonboard.com>\n\t<20260407-kbingham-awb-split-v1-3-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-3-a39af3f4dc20@ideasonboard.com>","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":38565,"web_url":"https://patchwork.libcamera.org/comment/38565/","msgid":"<85mrzc9ujn.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-04-09T12:39:56","subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin CPU ISP","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Kieran Bingham writes:\n\n> From: Milan Zamazal \n>\n> The black level handling in CPU ISP has two flaws:\n>\n> - The black level is applied after white balance rather than before.\n>\n> - It doesn't handle black levels with different values for individual\n> colour channels.\n>\n> The flaws are in both CCM and non-CCM cases. The wrong black level and\n> white balance application order is well visible when the white balance\n> gains are significantly different from 1.0. Then the output differs\n> significantly from GPU ISP output, which uses the correct order.\n>\n> This patch changes the computations of the lookup tables in a way that\n> fixes both the problems.\n>\n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 60 +++++++++++++++---------------\n> 1 file changed, 30 insertions(+), 30 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index dd0fff8714144417467bac16a1055d1185782d14..5356d6bbec11c30fa0b05659d44a91e69e2b79d0 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -879,15 +879,12 @@ void DebayerCpuThread::process4(uint32_t frame, const uint8_t *src, uint8_t *dst\n> \n> void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> {\n> -\tconst RGB blackLevel = params.blackLevel;\n> -\t/* Take let's say the green channel black level */\n> -\tconst unsigned int blackIndex = blackLevel[1] * gammaTable_.size();\n> \tconst float gamma = params.gamma;\n> \tconst float contrastExp = params.contrastExp;\n> \n> -\tconst float divisor = gammaTable_.size() - blackIndex - 1.0;\n> -\tfor (unsigned int i = blackIndex; i < gammaTable_.size(); i++) {\n> -\t\tfloat normalized = (i - blackIndex) / divisor;\n> +\tconst float divisor = gammaTable_.size() - 1.0;\n> +\tfor (unsigned int i = 0; i < gammaTable_.size(); i++) {\n> +\t\tfloat normalized = i / divisor;\n> \t\t/* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> \t\t/* Apply simple S-curve */\n> \t\tif (normalized < 0.5)\n> @@ -897,14 +894,6 @@ void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> \t\tgammaTable_[i] = UINT8_MAX *\n> \t\t\t\t std::pow(normalized, gamma);\n> \t}\n> -\t/*\n> -\t * Due to CCM operations, the table lookup may reach indices below the black\n> -\t * level. Let's set the table values below black level to the minimum\n> -\t * non-black value to prevent problems when the minimum value is\n> -\t * significantly non-zero (for example, when the image should be all grey).\n> -\t */\n> -\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex,\n> -\t\t gammaTable_[blackIndex]);\n> }\n> \n> void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> @@ -916,11 +905,13 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> \tif (gammaUpdateNeeded)\n> \t\tupdateGammaTable(params);\n> \n> +\t/* Processing order: black level -> gains -> gamma */\n> \tauto matrixChanged = [](const Matrix &m1, const Matrix &m2) -> bool {\n> \t\treturn !std::equal(m1.data().begin(), m1.data().end(), m2.data().begin());\n> \t};\n> \tconst unsigned int gammaTableSize = gammaTable_.size();\n> -\tconst double div = static_cast(kRGBLookupSize) / gammaTableSize;\n> +\tRGB blackIndex = params.blackLevel * kRGBLookupSize;\n> +\n> \tif (ccmEnabled_) {\n> \t\tif (gammaUpdateNeeded ||\n> \t\t matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {\n> @@ -930,17 +921,21 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(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\tred[i].r = std::round(i * params.combinedMatrix[redIndex][0]);\n> -\t\t\t\tred[i].g = std::round(i * params.combinedMatrix[greenIndex][0]);\n> -\t\t\t\tred[i].b = std::round(i * params.combinedMatrix[blueIndex][0]);\n> -\t\t\t\tgreen[i].r = std::round(i * params.combinedMatrix[redIndex][1]);\n> -\t\t\t\tgreen[i].g = std::round(i * params.combinedMatrix[greenIndex][1]);\n> -\t\t\t\tgreen[i].b = std::round(i * params.combinedMatrix[blueIndex][1]);\n> -\t\t\t\tblue[i].r = std::round(i * params.combinedMatrix[redIndex][2]);\n> -\t\t\t\tblue[i].g = std::round(i * params.combinedMatrix[greenIndex][2]);\n> -\t\t\t\tblue[i].b = std::round(i * params.combinedMatrix[blueIndex][2]);\n> -\t\t\t\tgammaLut_[i] = gammaTable_[i / div];\n> +\t\t\t\tconst RGB rgb = ((RGB(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\nThe replacements of `i' in the multiplications are wrong. Each of the\n`red', `green', `blue' tables is applied on a raw pixel of the\ncorresponding colour. This means e.g. all the rgb components of\n`red[i]' should use the red black level, i.e. rgb.r(); similarly for the\nother colours.\n\nThis is the correct version:\n\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.r() * params.combinedMatrix[greenIndex][0]);\n\t\t\t\tred[i].b = std::round(rgb.r() * params.combinedMatrix[blueIndex][0]);\n\t\t\t\tgreen[i].r = std::round(rgb.g() * 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.g() * params.combinedMatrix[blueIndex][1]);\n\t\t\t\tblue[i].r = std::round(rgb.b() * params.combinedMatrix[redIndex][2]);\n\t\t\t\tblue[i].g = std::round(rgb.b() * params.combinedMatrix[greenIndex][2]);\n\t\t\t\tblue[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][2]);\n\nThis bug is usually not observed here as the black levels are often all\nthe same for the three colours. But it gets more pronounced in the next\npatch, where incorrect AWB gains are applied to non-diagonal elements of\nthe matrix. It may still not be clearly visible unless the AWB gains\nare very different for each of the colours and the matrix elements are\nalso sufficiently different.\n\n> +\t\t\t\tgammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n> \t\t\t}\n> \t\t}\n> \t} else {\n> @@ -949,12 +944,17 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(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\t/* Apply gamma after gain! */\n> -\t\t\t\tconst RGB lutGains = (gains * i / div).min(gammaTableSize - 1);\n> -\t\t\t\tred[i] = gammaTable_[static_cast(lutGains.r())];\n> -\t\t\t\tgreen[i] = gammaTable_[static_cast(lutGains.g())];\n> -\t\t\t\tblue[i] = gammaTable_[static_cast(lutGains.b())];\n> +\t\t\t\tconst RGB lutGains =\n> +\t\t\t\t\t(gains * (RGB(i) - blackIndex) / div)\n> +\t\t\t\t\t\t.min(gammaTableSize - 1)\n> +\t\t\t\t\t\t.max(0.0);\n> +\t\t\t\tred[i] = gammaTable_[lutGains.r()];\n> +\t\t\t\tgreen[i] = gammaTable_[lutGains.g()];\n> +\t\t\t\tblue[i] = gammaTable_[lutGains.b()];\n> \t\t\t}\n> \t\t}\n> \t}","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 B19DABDCBD\n\tfor ;\n\tThu, 9 Apr 2026 12:40:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92F8A62E4A;\n\tThu, 9 Apr 2026 14:40:04 +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 797FF62010\n\tfor ;\n\tThu, 9 Apr 2026 14:40:03 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-121-twjwXbVaOEy3BV2G6ICgfQ-1; Thu, 09 Apr 2026 08:40:00 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-4837b6f6b93so7074025e9.3\n\tfor ;\n\tThu, 09 Apr 2026 05:40:00 -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\t5b1f17b1804b1-488cd1a83d4sm71928625e9.4.2026.04.09.05.39.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Apr 2026 05:39:57 -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=\"a7u9xOk5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1775738402;\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=IeEgs4M0HSWZXeH4IcADJjQFzpNzBH/FiGVEEcZ5qMc=;\n\tb=a7u9xOk5o+Nlm2hnAO3NC4yx7XftZsWscau8yeLfZjuXj+qoT91fpPPIw51BJz4zFDi1b0\n\tCL/WmOeUPUWY6sefl/o1uS90qD/r323Zzzl9EcfsFnzV0o1Q0Ii7vHVBNwibCPYg7kK00d\n\tF/kJAImAC36hkwwTl8A9tqTzhr6Jdrc=","X-MC-Unique":"twjwXbVaOEy3BV2G6ICgfQ-1","X-Mimecast-MFC-AGG-ID":"twjwXbVaOEy3BV2G6ICgfQ_1775738400","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1775738399; x=1776343199;\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=IeEgs4M0HSWZXeH4IcADJjQFzpNzBH/FiGVEEcZ5qMc=;\n\tb=KdhzER37+BVCKVhXP/YMa+4InTtOJev1vW4JVrtFAaSgk5C88aR2k6I/A1KhrUJ8xH\n\t51iSEqEYqxHtAQ+ZFtP3D3ahgFP8MLn4xhOTo3U1S3zF2kEmf5GM2AKVJ0Ng9PC/ZzXD\n\tCGxqce2Nk9mfXr9jC98Y+MVSCe+6gc9ThRAzpz2lCUvOZ/Di+qh3BC4LPJgOCNY9YYw1\n\tYXYd2PcZSTpCt3jB13NtQXuXQ9evePrHEdV6b9tqww8k3ey4jmnjamAWd/VR6Gx582TW\n\tST1WDr0DIGzEBjtqP6HNfVuwYq/irSsjmZt7xE83cng+c3GJsF5i4ZssC4t1PbiwJbI+\n\tveUg==","X-Gm-Message-State":"AOJu0Yx2zpu9IUVNaoHkUjEgICVwgS8zTaRRjzdsjRihmGgvrGfwOlo+\n\tRT5SEUH/hoVXu0p8qkP4Zi1S9kaP4AfUNmAL+bF0qVFzxU5pBD9kYTP8K4JeW/x2iiT8aDHNhQI\n\t6aoVipNlf+A90pqaw5nGtmuJk7ZvYoamsxZJtc149pPM+kC/JUe/d6RW8c0PnUQggb+aVvqX3Wi\n\t21LDFfzoTg6OFBJKLsLGzqIG1cgXvwABr1RGhhGpQqGytvhju1EpgLPRd/rqo=","X-Gm-Gg":"AeBDies6xstyYxZK5a8HioOGhqcA1C8+YkO1n9U3rsH8/scan/8aGtQw+HSXnPP2qyw\n\tFXYnVTLQYn3TLiGt4NLTBeHQOt7pPUZ3Uw2UEFO7hPD+qYdZ9/ilGkAa2V+tuXz06L7vPZqWCWW\n\tgKW6q6ejUpNwDDJy10CWXdY7r5D8Zptlv0ByzbHXY/KzJ6Pb1a/pnTC3JrClRbfb2ZUfZSbJxnX\n\tGQrC52Xy9LXq3+6mkA6waU6tTV5wyKwGd+uK2ygWsY61Sc9GgFihrPxk4bX4jGjkm6hzj0WPFyc\n\ty34Pb9nmPXDcLPqKKqlQfOLnufqt47NZ8QCLHgbFTOjwhE2rOO7LTIDmadq+okeZpY4NEtUBVck\n\t0NJo1uIEBWte6I4MqmSnjzQLuwSqK/r8nNOIhxu3F6+jEfaAjjQqm4Upm5JfCok0jmopwZBOhIl\n\t8=","X-Received":["by 2002:a05:600c:a110:b0:488:a824:fe04 with SMTP id\n\t5b1f17b1804b1-488cd012d80mr37902885e9.26.1775738399093; \n\tThu, 09 Apr 2026 05:39:59 -0700 (PDT)","by 2002:a05:600c:a110:b0:488:a824:fe04 with SMTP id\n\t5b1f17b1804b1-488cd012d80mr37902495e9.26.1775738398504; \n\tThu, 09 Apr 2026 05:39:58 -0700 (PDT)"],"From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin CPU ISP","In-Reply-To":"<20260407-kbingham-awb-split-v1-3-a39af3f4dc20@ideasonboard.com>\n\t(Kieran Bingham's message of \"Tue, 07 Apr 2026 23:01:06 +0100\")","References":"<20260407-kbingham-awb-split-v1-0-a39af3f4dc20@ideasonboard.com>\n\t<20260407-kbingham-awb-split-v1-3-a39af3f4dc20@ideasonboard.com>","Date":"Thu, 09 Apr 2026 14:39:56 +0200","Message-ID":"<85mrzc9ujn.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":"dmYEcko2hD0ou2VPxW2jJCjYX7zumhd-A3nPdPq-9Ok_1775738400","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":"","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":38772,"web_url":"https://patchwork.libcamera.org/comment/38772/","msgid":"","date":"2026-05-07T12:50:42","subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin CPU ISP","submitter":{"id":242,"url":"https://patchwork.libcamera.org/api/people/242/","name":"Hans de Goede","email":"johannes.goede@oss.qualcomm.com"},"content":"Hi,\n\nOn 8-Apr-26 00:01, Kieran Bingham wrote:\n> From: Milan Zamazal \n> \n> The black level handling in CPU ISP has two flaws:\n> \n> - The black level is applied after white balance rather than before.\n> \n> - It doesn't handle black levels with different values for individual\n> colour channels.\n> \n> The flaws are in both CCM and non-CCM cases. The wrong black level and\n> white balance application order is well visible when the white balance\n> gains are significantly different from 1.0. Then the output differs\n> significantly from GPU ISP output, which uses the correct order.\n> \n> This patch changes the computations of the lookup tables in a way that\n> fixes both the problems.\n> \n> Signed-off-by: Milan Zamazal \n\nThis is missing Kieran's S-o-b here (assuming kernel rules of passing\nthrough your hands means adding S-o-b).\n\nThe patch looks good to me:\n\nReviewed-by: Hans de Goede \n\nThis was send indepently before and I've had this on my review\nlist for forever. AFAICT this is independent of the rest of this\nseries and as this is a bugfix IMHO it would be good to get this\none merged right away.\n\nWhile at it maybe also merge 1/13 right away to make the series\na bit smaller, since 1/13 is just whitespace changes?\n\nRegards,\n\nHans\n\n\n\n\n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 60 +++++++++++++++---------------\n> 1 file changed, 30 insertions(+), 30 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index dd0fff8714144417467bac16a1055d1185782d14..5356d6bbec11c30fa0b05659d44a91e69e2b79d0 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -879,15 +879,12 @@ void DebayerCpuThread::process4(uint32_t frame, const uint8_t *src, uint8_t *dst\n> \n> void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> {\n> -\tconst RGB blackLevel = params.blackLevel;\n> -\t/* Take let's say the green channel black level */\n> -\tconst unsigned int blackIndex = blackLevel[1] * gammaTable_.size();\n> \tconst float gamma = params.gamma;\n> \tconst float contrastExp = params.contrastExp;\n> \n> -\tconst float divisor = gammaTable_.size() - blackIndex - 1.0;\n> -\tfor (unsigned int i = blackIndex; i < gammaTable_.size(); i++) {\n> -\t\tfloat normalized = (i - blackIndex) / divisor;\n> +\tconst float divisor = gammaTable_.size() - 1.0;\n> +\tfor (unsigned int i = 0; i < gammaTable_.size(); i++) {\n> +\t\tfloat normalized = i / divisor;\n> \t\t/* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> \t\t/* Apply simple S-curve */\n> \t\tif (normalized < 0.5)\n> @@ -897,14 +894,6 @@ void DebayerCpu::updateGammaTable(const DebayerParams ¶ms)\n> \t\tgammaTable_[i] = UINT8_MAX *\n> \t\t\t\t std::pow(normalized, gamma);\n> \t}\n> -\t/*\n> -\t * Due to CCM operations, the table lookup may reach indices below the black\n> -\t * level. Let's set the table values below black level to the minimum\n> -\t * non-black value to prevent problems when the minimum value is\n> -\t * significantly non-zero (for example, when the image should be all grey).\n> -\t */\n> -\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex,\n> -\t\t gammaTable_[blackIndex]);\n> }\n> \n> void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> @@ -916,11 +905,13 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\n> \tif (gammaUpdateNeeded)\n> \t\tupdateGammaTable(params);\n> \n> +\t/* Processing order: black level -> gains -> gamma */\n> \tauto matrixChanged = [](const Matrix &m1, const Matrix &m2) -> bool {\n> \t\treturn !std::equal(m1.data().begin(), m1.data().end(), m2.data().begin());\n> \t};\n> \tconst unsigned int gammaTableSize = gammaTable_.size();\n> -\tconst double div = static_cast(kRGBLookupSize) / gammaTableSize;\n> +\tRGB blackIndex = params.blackLevel * kRGBLookupSize;\n> +\n> \tif (ccmEnabled_) {\n> \t\tif (gammaUpdateNeeded ||\n> \t\t matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {\n> @@ -930,17 +921,21 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(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\tred[i].r = std::round(i * params.combinedMatrix[redIndex][0]);\n> -\t\t\t\tred[i].g = std::round(i * params.combinedMatrix[greenIndex][0]);\n> -\t\t\t\tred[i].b = std::round(i * params.combinedMatrix[blueIndex][0]);\n> -\t\t\t\tgreen[i].r = std::round(i * params.combinedMatrix[redIndex][1]);\n> -\t\t\t\tgreen[i].g = std::round(i * params.combinedMatrix[greenIndex][1]);\n> -\t\t\t\tgreen[i].b = std::round(i * params.combinedMatrix[blueIndex][1]);\n> -\t\t\t\tblue[i].r = std::round(i * params.combinedMatrix[redIndex][2]);\n> -\t\t\t\tblue[i].g = std::round(i * params.combinedMatrix[greenIndex][2]);\n> -\t\t\t\tblue[i].b = std::round(i * params.combinedMatrix[blueIndex][2]);\n> -\t\t\t\tgammaLut_[i] = gammaTable_[i / div];\n> +\t\t\t\tconst RGB rgb = ((RGB(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\tgammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n> \t\t\t}\n> \t\t}\n> \t} else {\n> @@ -949,12 +944,17 @@ void DebayerCpu::updateLookupTables(const DebayerParams ¶ms)\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 div =\n> +\t\t\t\t(RGB(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\t/* Apply gamma after gain! */\n> -\t\t\t\tconst RGB lutGains = (gains * i / div).min(gammaTableSize - 1);\n> -\t\t\t\tred[i] = gammaTable_[static_cast(lutGains.r())];\n> -\t\t\t\tgreen[i] = gammaTable_[static_cast(lutGains.g())];\n> -\t\t\t\tblue[i] = gammaTable_[static_cast(lutGains.b())];\n> +\t\t\t\tconst RGB lutGains =\n> +\t\t\t\t\t(gains * (RGB(i) - blackIndex) / div)\n> +\t\t\t\t\t\t.min(gammaTableSize - 1)\n> +\t\t\t\t\t\t.max(0.0);\n> +\t\t\t\tred[i] = gammaTable_[lutGains.r()];\n> +\t\t\t\tgreen[i] = gammaTable_[lutGains.g()];\n> +\t\t\t\tblue[i] = gammaTable_[lutGains.b()];\n> \t\t\t}\n> \t\t}\n> \t}\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 63834BDCB5\n\tfor ;\n\tThu, 7 May 2026 12:50:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C3FA6301E;\n\tThu, 7 May 2026 14:50:50 +0200 (CEST)","from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com\n\t[205.220.180.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C67162FEC\n\tfor ;\n\tThu, 7 May 2026 14:50:47 +0200 (CEST)","from pps.filterd (m0279872.ppops.net [127.0.0.1])\n\tby mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id\n\t647AnJ9X3924344 for ;\n\tThu, 7 May 2026 12:50:46 GMT","from mail-vs1-f71.google.com (mail-vs1-f71.google.com\n\t[209.85.217.71])\n\tby mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4e0c88b3wv-1\n\t(version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT)\n\tfor ;\n\tThu, 07 May 2026 12:50:46 +0000 (GMT)","by mail-vs1-f71.google.com with SMTP id\n\tada2fe7eead31-60fc71622f7so323529137.3\n\tfor ;\n\tThu, 07 May 2026 05:50:46 -0700 (PDT)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-bc81cd34cd2sm80934766b.6.2026.05.07.05.50.43\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 07 May 2026 05:50:43 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=qualcomm.com header.i=@qualcomm.com\n\theader.b=\"WRsmmGm1\"; dkim=pass (2048-bit key;\n\tunprotected) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com\n\theader.b=\"LSKOhQG3\"; dkim-atps=neutral","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h=\n\tcc:content-transfer-encoding:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to; s=qcppdkim1; bh=\n\tkbZsVoDZieCXBxHnP34/k4RNdQwDbZGHzg5c1HF+0y4=; b=WRsmmGm1oTlD1o7s\n\tXRGL5mmOFNPlut+hMYlNakHS1jHxE4DJn5JYQ00BEQaUpZIkQwwUIlj9I9X+IU1O\n\tc9gd/ph9URaD6nKVCTWKf727SLuzp6T2JNHIcSXyXVExQERDKg6WI7m6FDdnsx3X\n\tAKZfUiE51hK16JBQi1C48W4IuQ6ci8AHWdgKlzqKcKH9fhPeJ3fM5vVJEJwE3TIR\n\tm/IUrLJF3lGj7od79MHf9LOlT9w539Xzkg/tmt/OkCSXnsJNrfrGrOyuPjY8MuAh\n\t36aIkcWjke2cFKI7BRuX7ysjpPjA0dtc6+2aDRnEjUtl10zlpd//qQsWykJz0339\n\tAiQekw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=oss.qualcomm.com; s=google; t=1778158245; x=1778763045;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:from:to\n\t:cc:subject:date:message-id:reply-to;\n\tbh=kbZsVoDZieCXBxHnP34/k4RNdQwDbZGHzg5c1HF+0y4=;\n\tb=LSKOhQG37BJJSO/rOay77f7q5CWbZmeV0JdF7eisWmDQBT3BOGb5cQI/p/6USd99Pe\n\tu1HcxgtKvV7kmiZX+Dgje4CMIANEIXxXQ2ekBC5EcsbV8bSOT/2nHcJyYKKgzDWK2mQf\n\tDUzKWEIqX1TbgyveKXfXaM4W2Zu4TXVs23LSOjPOplELKH04eMC6j5xWA8HWhJnSrX39\n\t2fn9m1DCRJD44UINppRxJhIV1UNrMpzhSVRstjzj49bjQZUJK8XuPbFR9vHiatTkY7Kk\n\tuLPli2hbYAG/XVVr45H9y8LpnOcYhzog8onz459UwwOWNP3BYh1EoKtxPJX7eIBiqD2V\n\txIhQ=="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1778158245; x=1778763045;\n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=kbZsVoDZieCXBxHnP34/k4RNdQwDbZGHzg5c1HF+0y4=;\n\tb=oezRTJXmTQqT8pH89bWssId7eIz1AdGPEbGqRIowfWvZxaEA9+GDlHehnWNOlEzblv\n\t+pbSUDpMhUsbgU038ekOMRLszcJ4HLb8+eiXCKDOyI520Xs/4zzZ1AqG1IEQXQWaCN/k\n\tm0rps30FEj/kHFqOI6om3ZcFqf8NA44PnkKPpT3NHfpJnfwa3QNuC5vbfw0R0cv5LQXY\n\tOp77nIs9+ZhmYAhS/WXF9Bq9vIwB7pfcumAEm4j8hCsLaTQxWBeWQatopasefnzdS1no\n\t5cqUn/zuzHnzZDt+d4g7Ke/Eo8sbhj1a/Nk0tO1o8Bd5kkYI0M3sXAjW5rxjg/tYVk45\n\tYF9g==","X-Forwarded-Encrypted":"i=1;\n\tAFNElJ8pI25Mqnbb3YA93Aauuuc4QOuNu2YoCXwXwII2bWsVrdssQyWqoBjbKrrxF/lHufRQjybj8js2DE3/wMkSzgg=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yxqp3RnT9uxGUg07QIZy73rz0kaM6OEVXc/D+dJx/vGLZxtevBE\n\tCipRdEfU7F+5EVuYIoX1Jcow0ymHPjUiPON6UL/rWdEzZoO567QGegOiWTVDH3AhiRfIumg5+LR\n\tgzY64h2x66M8f2rrgnuJC7DroeKEeX+3Vv3o5iNHcGKbtwqVOH49ZDId6znDByix/M2zuQxqemk\n\tEb75V9jgeYGnc=","X-Gm-Gg":"AeBDievSA76YY/Q7cn2u2xn0UP7sIZWoBpRQFY053OJbbHmohlJl1CfjQzdHOly0HCB\n\tyGOPXLP72VJbQaR6aTIlYkha5XToKdFI6IB80O45qHiqCfulKbxdP1pjVt949IIUcMRyRotwrMd\n\tosYxWNVZWTLzj88Mh5q6ThNHmeljnhW/AV8SqEwJn4nnEBtqNMnRV9W1weAyptxeZz8aivKJAHP\n\tCjdfCndF8REr7c6Y6ElZJpdegGDljXtRW1Y2433SXR3ay0402bwKjtifqgBakabgeqLDDqsmHJC\n\tfxvcTEZUS3XPKlPrlpSJ+6rUalYnENg1fwc+BuzvelhCwdQ8XY62JdDgfeR68geD6/h3TCsBAqu\n\tYSjJtzpkY5RVNv4cv6jV/Z1HOjKeTGNR4SuXqaOqcOTbrBx9pM3FlFp/WZORmWNqAlrG/54PpVO\n\t/NbQFJcwUJgz1JTAYZBpvude0c6geo2Pt7D3AcK1izY/aSUzj7XvJMmc83//KdHs6eMdBNLYt7F\n\toiCYJQyrt9tonEQru67BdRf+2g=","X-Received":["by 2002:a05:6102:304e:b0:62e:79ee:4cda with SMTP id\n\tada2fe7eead31-630f8e6ad78mr3739977137.3.1778158245225; \n\tThu, 07 May 2026 05:50:45 -0700 (PDT)","by 2002:a05:6102:304e:b0:62e:79ee:4cda with SMTP id\n\tada2fe7eead31-630f8e6ad78mr3739969137.3.1778158244682; \n\tThu, 07 May 2026 05:50:44 -0700 (PDT)"],"Message-ID":"","Date":"Thu, 7 May 2026 14:50:42 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","From":"johannes.goede@oss.qualcomm.com","Subject":"Re: [PATCH 03/13] libcamera: software_isp: Fix black level handling\n\tin CPU ISP","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ","References":"<20260407-kbingham-awb-split-v1-0-a39af3f4dc20@ideasonboard.com>\n\t<20260407-kbingham-awb-split-v1-3-a39af3f4dc20@ideasonboard.com>","Content-Language":"en-US, nl","In-Reply-To":"<20260407-kbingham-awb-split-v1-3-a39af3f4dc20@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Proofpoint-ORIG-GUID":"PWCnqRq1HW2J5KmyQH6tR459j28yxsYB","X-Proofpoint-Spam-Details-Enc":"AW1haW4tMjYwNTA3MDEyNyBTYWx0ZWRfX456Okx6/OhFK\n\tctUR94j5whR36NjMcEfpSkpZ86VHLN2VMvFr+VTiYBmYYOcCh5b7ij9E9LOMCbSmdkfo6ZQqnlW\n\tjE+pDdwb30++5cC78p9Iy4EVK92wNVJ+Keb5RIjcVwOOv3M8U+5r6aUYdTa2oBDFDQPg8/GXqeU\n\tKYUFwERR1nV4QMYylcAYIb3wPA+QSRx334S2KudzA7xDIYi7aTKmC9Y3Jfxf9AaWJwgkPm0Ww17\n\t43cybK+rEdgrpYDRSCJIzwP4Ni2r1ghjAlPZadrNuxbvdbkcdWIhUgbnHsSrdiGWozx8SBPrVex\n\tj1EP+7xcvyF4ZKZctSGscfm2hq9gg+tlfbo3//hid1whNUTN5/FQh/ZmnBPvK5Xo92R9b4fGFUG\n\tJBMzVgNDqJx/1eTyZuP5JoJhkbSKUdNMdnQTM+aJl8f5tqyf/+FDeJS5GLfQrkO8/epe/POdASn\n\tw1GyIxTLmedoCatsLeA==","X-Proofpoint-GUID":"PWCnqRq1HW2J5KmyQH6tR459j28yxsYB","X-Authority-Analysis":"v=2.4 cv=X8Zi7mTe c=1 sm=1 tr=0 ts=69fc8aa6 cx=c_pps\n\ta=P2rfLEam3zuxRRdjJWA2cw==:117 a=xqWC_Br6kY4A:10 a=IkcTkHD0fZMA:10\n\ta=NGcC8JguVDcA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22\n\ta=u7WPNUs3qKkmUXheDGA7:22 a=yx91gb_oNiZeI1HMLzn7:22 a=20KFwNOVAAAA:8\n\ta=EUspDBNiAAAA:8 a=mcL0tKSX4Ea4cpkElgYA:9 a=QEXdDO2ut3YA:10\n\ta=ODZdjJIeia2B_SHc_B0f:22","X-Proofpoint-Virus-Version":"vendor=baseguard\n\tengine=ICAP:2.0.293, Aquarius:18.0.1143, Hydra:6.1.51,\n\tFMLib:17.12.100.49\n\tdefinitions=2026-05-07_01,2026-05-06_01,2025-10-01_01","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\timpostorscore=0 suspectscore=0 spamscore=0 adultscore=0\n\tmalwarescore=0\n\tlowpriorityscore=0 clxscore=1015 phishscore=0 priorityscore=1501\n\tbulkscore=0\n\tclassifier=typeunknown authscore=0 authtc= authcc= route=outbound\n\tadjust=0\n\treason=mlx scancount=1 engine=8.22.0-2604200000\n\tdefinitions=main-2605070127","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\" "}}]