[{"id":39201,"web_url":"https://patchwork.libcamera.org/comment/39201/","msgid":"<178199683048.2732614.17411488518816313683@ping.linuxembedded.co.uk>","date":"2026-06-20T23:07:10","subject":"Re: [PATCH 5/7] libcamera: software_isp: Fix black level handling in\n\tCPU ISP","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2026-06-21 00:00:32)\n> From: Milan Zamazal <mzamazal@redhat.com>\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> [Kieran: Fix indexing from Milans' suggestion, use new clamp]\n> [Kieran: Use enumerate, and div updates from Laurent's suggestion]\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n\nAnd this of course\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>","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 3950BC3303\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 20 Jun 2026 23:07:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 21D33656E5;\n\tSun, 21 Jun 2026 01:07: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 AAF78656DE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 21 Jun 2026 01:07:13 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CC7C1B8B;\n\tSun, 21 Jun 2026 01:06:36 +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=\"ahByVllw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1781996796;\n\tbh=mx6JpuZFimW9UJmeWedPFmpOQixEWMwHKajudmQ8Wb8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ahByVllwVfY9ekUBpVn9Dv9s4f0+G+DTaIkEqHXKRbRnB0uEzueU+S5n/abwKv4zF\n\t4IMSUYdmPfk3yODqfLKhIV+iWwRksYUD3d0J/X3B1Id2Y1cu4kL6mC4CzXuZ4rABl7\n\tpQ7iYbTxdKD9+oYQ6jH4UvWkrnahQyzFfr6DXvMQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260621-kbingham-awb-saturation-v1-5-b91ea59c6cfb@ideasonboard.com>","References":"<20260621-kbingham-awb-saturation-v1-0-b91ea59c6cfb@ideasonboard.com>\n\t<20260621-kbingham-awb-saturation-v1-5-b91ea59c6cfb@ideasonboard.com>","Subject":"Re: [PATCH 5/7] libcamera: software_isp: Fix black level handling in\n\tCPU ISP","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Milan Zamazal <mzamazal@redhat.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Sun, 21 Jun 2026 00:07:10 +0100","Message-ID":"<178199683048.2732614.17411488518816313683@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":39285,"web_url":"https://patchwork.libcamera.org/comment/39285/","msgid":"<85qzly8hxf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-06-22T19:59:56","subject":"Re: [PATCH 5/7] 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":"Hi Kieran,\n\nthank you for the update.\n\nKieran Bingham <kieran.bingham@ideasonboard.com> writes:\n\n> From: Milan Zamazal <mzamazal@redhat.com>\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> [Kieran: Fix indexing from Milans' suggestion, use new clamp]\n> [Kieran: Use enumerate, and div updates from Laurent's suggestion]\n\nAs for Kieran's changes:\n\nReviewed-by: Milan Zamazal <mzamazal@redhat.com>\n\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/software_isp/debayer_cpu.cpp | 60 ++++++++++++++----------------\n>  1 file changed, 28 insertions(+), 32 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index d2596d32bbcdeaaab2e2c287e3f01ae22c442884..9d6a08b3333f988570cb5d3f7cea4c117f7c2530 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -979,32 +979,20 @@ void DebayerCpuThread::process4(uint32_t frame, const uint8_t *src, uint8_t *dst\n>  \n>  void DebayerCpu::updateGammaTable(const DebayerParams &params)\n>  {\n> -\tconst RGB<float> 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 (auto [i, value] : utils::enumerate(gammaTable_)) {\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>  \t\t\tnormalized = 0.5 * std::pow(normalized / 0.5, contrastExp);\n>  \t\telse\n>  \t\t\tnormalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrastExp);\n> -\t\tgammaTable_[i] = UINT8_MAX *\n> -\t\t\t\t std::pow(normalized, gamma);\n> +\t\tvalue = UINT8_MAX * 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 &params)\n> @@ -1016,11 +1004,15 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\n>  \tif (gammaUpdateNeeded)\n>  \t\tupdateGammaTable(params);\n>  \n> +\t/* Processing order: black level -> gains -> gamma */\n>  \tauto matrixChanged = [](const Matrix<float, 3, 3> &m1, const Matrix<float, 3, 3> &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<double>(kRGBLookupSize) / gammaTableSize;\n> +\n> +\tconst RGB<float> blackIndex = params.blackLevel * kRGBLookupSize;\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> @@ -1030,17 +1022,19 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\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> +\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<float> rgb = ((RGB<float>(i) - blackIndex) * kRGBLookupSize / 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.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> +\t\t\t\tgammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n>  \t\t\t}\n>  \t\t}\n>  \t} else {\n> @@ -1049,12 +1043,14 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\n>  \t\t\tauto &red = swapRedBlueGains_ ? blue_ : red_;\n>  \t\t\tauto &green = green_;\n>  \t\t\tauto &blue = swapRedBlueGains_ ? red_ : blue_;\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<float> lutGains = (gains * i / div).min(gammaTableSize - 1);\n> -\t\t\t\tred[i] = gammaTable_[static_cast<unsigned int>(lutGains.r())];\n> -\t\t\t\tgreen[i] = gammaTable_[static_cast<unsigned int>(lutGains.g())];\n> -\t\t\t\tblue[i] = gammaTable_[static_cast<unsigned int>(lutGains.b())];\n> +\t\t\t\tconst RGB<float> lutGains =\n> +\t\t\t\t\t(gains * (RGB<float>(i) - blackIndex) * gammaTableSize / div)\n> +\t\t\t\t\t\t.clamp(0.0, gammaTableSize - 1);\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":"<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 61F21C3301\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 22 Jun 2026 20:00:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C52C6575C;\n\tMon, 22 Jun 2026 22:00:05 +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 8774D656D3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 22 Jun 2026 22:00:03 +0200 (CEST)","from mail-wr1-f70.google.com (mail-wr1-f70.google.com\n\t[209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-207-Fjw5vBCBNveqjznjZjU80Q-1; Mon, 22 Jun 2026 15:59:59 -0400","by mail-wr1-f70.google.com with SMTP id\n\tffacd0b85a97d-4629f312a67so4224681f8f.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 22 Jun 2026 12:59:59 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-4.net.vodafone.cz. [77.48.47.4])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-46666c57afasm29746739f8f.29.2026.06.22.12.59.57\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 22 Jun 2026 12:59: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=\"R7xjAetQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1782158402;\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=quz5gOZI7+rAKjOfGGiQcQuwPjtCEeJEVDlNWysNJTo=;\n\tb=R7xjAetQEXuywMLxeGmzJQWwfJK7GZIm8zNxpajVCA3UzDn8Jum5mjkPJ43otlu7n/eUSL\n\tEYf/yb55wByyoafY438TKY0u+H5wakVmn+OvB6V/kcWgfvZLTiJgY7UiRibsxHJiqLWlZe\n\tz8kRVoEyLNpz/DTacThjOURwQvS43Gk=","X-MC-Unique":"Fjw5vBCBNveqjznjZjU80Q-1","X-Mimecast-MFC-AGG-ID":"Fjw5vBCBNveqjznjZjU80Q_1782158399","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1782158398; x=1782763198;\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=quz5gOZI7+rAKjOfGGiQcQuwPjtCEeJEVDlNWysNJTo=;\n\tb=P5GrO7xGmPewjUCXL/noFkIMMprEDbt9VHCl5BblvUF3Vw4mYlhOF2PfZ57BPRCHvy\n\tJiYPgm8ixyQGBi/CARR89bnFr+JeZq9DggwmYKdzGgumepVN8yWduLnGo3ITQhmSgWKy\n\twYBCYQKIxodYi8FqUCRRJhDvPV/jfn5nVSJdblxhihL38KpIQiKm1WmNSPbOvmJE1oBV\n\t8VT4rtQD4NCdlQrhaYOwN1EWOoMtZOAKwgHDF5jEIXfe3zA+R7rJbfWAZjLBBYT01IwE\n\tTlZvaFqROjuIhbayi0XofzKVh9R24VJOpgJYROgJ45Vbk0BLRVxiz4sTrTHw96X21LOt\n\tc22A==","X-Gm-Message-State":"AOJu0YxUaB7s6XPJgH7ZUhqZhdYLW4UP0cuKRe5Fxt+kxITPS9C7cEh9\n\tazkf7pE3VaJckUW/YQBgcgno/RYxR5k0iyEECcuEQYPthEekmQ04FN1oLgHNzrSF5UlOUOPN6Qm\n\t115ni175I3xjYygLpRKvWlqcZxALBT+Gl0Gzl9OA7wSG/jNm1TibbISt4XWE8jS1sfTqtALofKS\n\ttpIJp25M5yyw3AxKK6+mAkGaASS9+0njnQxXDwojcwPFd5t3eyQcXAn9myZBA=","X-Gm-Gg":"AfdE7cklvPj/RJB2FoOVZ/rRI5h3JzRUEy/QrL8g0y6DHzCZ7adnQVscn06iadu0kxU\n\toIXXZY3mcWfvOTH/l1hULkjp6H0ZJnObcBFkqkN9J/jg1DaRJ/52q+FNWebqJCTnWHv5Vb/AsMp\n\tw5bzisB5SDS5VI9GB9AbMhQmsZjTDFkxLuyaMYgAQGl+Qep15EYot6A46PRsz/dGKdOq3D5blzx\n\tkitF1tZiNbmZeVXeX9GazL65gEBEwD/UMHC2o61bM5KLOU1rlG7NIU9QM/r6A5iaLDBt60GXGTF\n\t961x2jnWOS7BI11ISWTOHcgFw29tkiDcJZgX4NoVs9YVHKJIkDv0uJ9sAvG5/e8xNkcDtHZ+fv9\n\t3x6uShFO80XbeqrhYWcQb/OtjNvgy6mhaSG2n8pNjHqzK2+KbOs7x/hAio7p/7W32","X-Received":["by 2002:a05:6000:240f:b0:469:763f:941c with SMTP id\n\tffacd0b85a97d-469763f965dmr6299240f8f.23.1782158398587; \n\tMon, 22 Jun 2026 12:59:58 -0700 (PDT)","by 2002:a05:6000:240f:b0:469:763f:941c with SMTP id\n\tffacd0b85a97d-469763f965dmr6299199f8f.23.1782158398006; \n\tMon, 22 Jun 2026 12:59:58 -0700 (PDT)"],"From":"Milan Zamazal <mzamazal@redhat.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 5/7] libcamera: software_isp: Fix black level handling\n\tin CPU ISP","In-Reply-To":"<20260621-kbingham-awb-saturation-v1-5-b91ea59c6cfb@ideasonboard.com>\n\t(Kieran Bingham's message of \"Sun, 21 Jun 2026 00:00:32 +0100\")","References":"<20260621-kbingham-awb-saturation-v1-0-b91ea59c6cfb@ideasonboard.com>\n\t<20260621-kbingham-awb-saturation-v1-5-b91ea59c6cfb@ideasonboard.com>","Date":"Mon, 22 Jun 2026 21:59:56 +0200","Message-ID":"<85qzly8hxf.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":"jGcyR97qXzGHxfU26iB_hfBfFv26Cm9RczI-YN2UN70_1782158399","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>"}},{"id":39302,"web_url":"https://patchwork.libcamera.org/comment/39302/","msgid":"<178219069788.522140.1837739343760332726@ping.linuxembedded.co.uk>","date":"2026-06-23T04:58:17","subject":"Re: [PATCH 5/7] libcamera: software_isp: Fix black level handling in\n\tCPU ISP","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2026-06-22 20:59:56)\n> Hi Kieran,\n> \n> thank you for the update.\n> \n> Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n> \n> > From: Milan Zamazal <mzamazal@redhat.com>\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> > [Kieran: Fix indexing from Milans' suggestion, use new clamp]\n> > [Kieran: Use enumerate, and div updates from Laurent's suggestion]\n> \n> As for Kieran's changes:\n> \n> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>\n\nAnd for Milan's original code in the first place:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> > ---\n> >  src/libcamera/software_isp/debayer_cpu.cpp | 60 ++++++++++++++----------------\n> >  1 file changed, 28 insertions(+), 32 deletions(-)\n> >\n> > diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> > index d2596d32bbcdeaaab2e2c287e3f01ae22c442884..9d6a08b3333f988570cb5d3f7cea4c117f7c2530 100644\n> > --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> > +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> > @@ -979,32 +979,20 @@ void DebayerCpuThread::process4(uint32_t frame, const uint8_t *src, uint8_t *dst\n> >  \n> >  void DebayerCpu::updateGammaTable(const DebayerParams &params)\n> >  {\n> > -     const RGB<float> blackLevel = params.blackLevel;\n> > -     /* Take let's say the green channel black level */\n> > -     const unsigned int blackIndex = blackLevel[1] * gammaTable_.size();\n> >       const float gamma = params.gamma;\n> >       const float contrastExp = params.contrastExp;\n> >  \n> > -     const float divisor = gammaTable_.size() - blackIndex - 1.0;\n> > -     for (unsigned int i = blackIndex; i < gammaTable_.size(); i++) {\n> > -             float normalized = (i - blackIndex) / divisor;\n> > +     const float divisor = gammaTable_.size() - 1.0;\n> > +     for (auto [i, value] : utils::enumerate(gammaTable_)) {\n> > +             float normalized = i / divisor;\n> >               /* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> >               /* Apply simple S-curve */\n> >               if (normalized < 0.5)\n> >                       normalized = 0.5 * std::pow(normalized / 0.5, contrastExp);\n> >               else\n> >                       normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrastExp);\n> > -             gammaTable_[i] = UINT8_MAX *\n> > -                              std::pow(normalized, gamma);\n> > +             value = UINT8_MAX * std::pow(normalized, gamma);\n> >       }\n> > -     /*\n> > -      * Due to CCM operations, the table lookup may reach indices below the black\n> > -      * level. Let's set the table values below black level to the minimum\n> > -      * non-black value to prevent problems when the minimum value is\n> > -      * significantly non-zero (for example, when the image should be all grey).\n> > -      */\n> > -     std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex,\n> > -               gammaTable_[blackIndex]);\n> >  }\n> >  \n> >  void DebayerCpu::updateLookupTables(const DebayerParams &params)\n> > @@ -1016,11 +1004,15 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\n> >       if (gammaUpdateNeeded)\n> >               updateGammaTable(params);\n> >  \n> > +     /* Processing order: black level -> gains -> gamma */\n> >       auto matrixChanged = [](const Matrix<float, 3, 3> &m1, const Matrix<float, 3, 3> &m2) -> bool {\n> >               return !std::equal(m1.data().begin(), m1.data().end(), m2.data().begin());\n> >       };\n> >       const unsigned int gammaTableSize = gammaTable_.size();\n> > -     const double div = static_cast<double>(kRGBLookupSize) / gammaTableSize;\n> > +\n> > +     const RGB<float> blackIndex = params.blackLevel * kRGBLookupSize;\n> > +     const RGB<float> div = (RGB<float>(kRGBLookupSize) - blackIndex).max(1.0);\n> > +\n> >       if (ccmEnabled_) {\n> >               if (gammaUpdateNeeded ||\n> >                   matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {\n> > @@ -1030,17 +1022,19 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\n> >                       const unsigned int redIndex = swapRedBlueGains_ ? 2 : 0;\n> >                       const unsigned int greenIndex = 1;\n> >                       const unsigned int blueIndex = swapRedBlueGains_ ? 0 : 2;\n> > +\n> >                       for (unsigned int i = 0; i < kRGBLookupSize; i++) {\n> > -                             red[i].r = std::round(i * params.combinedMatrix[redIndex][0]);\n> > -                             red[i].g = std::round(i * params.combinedMatrix[greenIndex][0]);\n> > -                             red[i].b = std::round(i * params.combinedMatrix[blueIndex][0]);\n> > -                             green[i].r = std::round(i * params.combinedMatrix[redIndex][1]);\n> > -                             green[i].g = std::round(i * params.combinedMatrix[greenIndex][1]);\n> > -                             green[i].b = std::round(i * params.combinedMatrix[blueIndex][1]);\n> > -                             blue[i].r = std::round(i * params.combinedMatrix[redIndex][2]);\n> > -                             blue[i].g = std::round(i * params.combinedMatrix[greenIndex][2]);\n> > -                             blue[i].b = std::round(i * params.combinedMatrix[blueIndex][2]);\n> > -                             gammaLut_[i] = gammaTable_[i / div];\n> > +                             const RGB<float> rgb = ((RGB<float>(i) - blackIndex) * kRGBLookupSize / div).max(0.0);\n> > +                             red[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][0]);\n> > +                             red[i].g = std::round(rgb.r() * params.combinedMatrix[greenIndex][0]);\n> > +                             red[i].b = std::round(rgb.r() * params.combinedMatrix[blueIndex][0]);\n> > +                             green[i].r = std::round(rgb.g() * params.combinedMatrix[redIndex][1]);\n> > +                             green[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][1]);\n> > +                             green[i].b = std::round(rgb.g() * params.combinedMatrix[blueIndex][1]);\n> > +                             blue[i].r = std::round(rgb.b() * params.combinedMatrix[redIndex][2]);\n> > +                             blue[i].g = std::round(rgb.b() * params.combinedMatrix[greenIndex][2]);\n> > +                             blue[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][2]);\n> > +                             gammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];\n> >                       }\n> >               }\n> >       } else {\n> > @@ -1049,12 +1043,14 @@ void DebayerCpu::updateLookupTables(const DebayerParams &params)\n> >                       auto &red = swapRedBlueGains_ ? blue_ : red_;\n> >                       auto &green = green_;\n> >                       auto &blue = swapRedBlueGains_ ? red_ : blue_;\n> > +\n> >                       for (unsigned int i = 0; i < kRGBLookupSize; i++) {\n> > -                             /* Apply gamma after gain! */\n> > -                             const RGB<float> lutGains = (gains * i / div).min(gammaTableSize - 1);\n> > -                             red[i] = gammaTable_[static_cast<unsigned int>(lutGains.r())];\n> > -                             green[i] = gammaTable_[static_cast<unsigned int>(lutGains.g())];\n> > -                             blue[i] = gammaTable_[static_cast<unsigned int>(lutGains.b())];\n> > +                             const RGB<float> lutGains =\n> > +                                     (gains * (RGB<float>(i) - blackIndex) * gammaTableSize / div)\n> > +                                             .clamp(0.0, gammaTableSize - 1);\n> > +                             red[i] = gammaTable_[lutGains.r()];\n> > +                             green[i] = gammaTable_[lutGains.g()];\n> > +                             blue[i] = gammaTable_[lutGains.b()];\n> >                       }\n> >               }\n> >       }\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 5AA43C3261\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Jun 2026 04:58:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F9BB6576C;\n\tTue, 23 Jun 2026 06:58:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8A72D61F3F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jun 2026 06:58:20 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0025A2D7;\n\tTue, 23 Jun 2026 06:57: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=\"u+9ot7Q/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782190662;\n\tbh=lO53SZf9Mn2ocUTs68rAMnO5+O2iHtTK8ImC+ixZzTw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=u+9ot7Q/MagoTqehlTSAzTwz/DXxqYdt4uTF/F0XuFQ4nd2n8IVfd7FcBoJut4MhV\n\tTCgHFnYlj14IBT60MArvyyQmylC/UTbyhVqtNWjMwX/N5yzON6jtUUCgoinDkhu+Yw\n\tmYYezKFHq0YJHjRzW+Wzoj5TU86CJJFF19vKU5bQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85qzly8hxf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20260621-kbingham-awb-saturation-v1-0-b91ea59c6cfb@ideasonboard.com>\n\t<20260621-kbingham-awb-saturation-v1-5-b91ea59c6cfb@ideasonboard.com>\n\t<85qzly8hxf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH 5/7] libcamera: software_isp: Fix black level handling in\n\tCPU ISP","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","To":"Milan Zamazal <mzamazal@redhat.com>","Date":"Tue, 23 Jun 2026 05:58:17 +0100","Message-ID":"<178219069788.522140.1837739343760332726@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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>"}}]