[{"id":37969,"web_url":"https://patchwork.libcamera.org/comment/37969/","msgid":"<20260127144731.GA2502390@killaraus>","date":"2026-01-27T14:47:31","subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 27, 2026 at 01:37:28PM +0100, Milan Zamazal wrote:\n> The black level offset subtracted in AWB is wrong.  It assumes that the\n> stats contain sums of the individual colour pixels.  But they actually\n> contain sums of the colour channels of larger \"superpixels\" consisting\n> of the individual colour pixels.  Each of the RGB colour values and the\n> computed luminosity (a histogram entry) are added once to the stats per\n> such a superpixel.  This means the offset computed from the black level\n> and the number of pixels should be used as it is, not divided.\n> \n> The patch fixes the subtracted offset.  Since it is the same for all the\n> three colours, a SwIspStats helper method returning an RGB instance is\n> introduced.  The individual colour variables are still retained in\n> SwIspStats for maximum efficiency when gathering the stats.  SwIspStats\n> docstrings are changed to avoid the original confusion.\n> \n> Fixes: 4e13c6f55bd667 (\"Honor black level in AWB\")\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  .../internal/software_isp/swisp_stats.h         | 17 +++++++++++++----\n>  src/ipa/simple/algorithms/awb.cpp               | 10 ++++------\n>  2 files changed, 17 insertions(+), 10 deletions(-)\n> \n> diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h\n> index 3c9860185..f8816209f 100644\n> --- a/include/libcamera/internal/software_isp/swisp_stats.h\n> +++ b/include/libcamera/internal/software_isp/swisp_stats.h\n> @@ -10,6 +10,8 @@\n>  #include <array>\n>  #include <stdint.h>\n>  \n> +#include \"libcamera/internal/vector.h\"\n> +\n>  namespace libcamera {\n>  \n>  /**\n> @@ -26,17 +28,24 @@ struct SwIspStats {\n>  \t */\n>  \tbool valid;\n>  \t/**\n> -\t * \\brief Holds the sum of all sampled red pixels\n> +\t * \\brief Holds the sum of red channels of all the sampled pixels\n>  \t */\n>  \tuint64_t sumR_;\n>  \t/**\n> -\t * \\brief Holds the sum of all sampled green pixels\n> +\t * \\brief Holds the sum of green channels of all the sampled pixels\n>  \t */\n>  \tuint64_t sumG_;\n>  \t/**\n> -\t * \\brief Holds the sum of all sampled blue pixels\n> +\t * \\brief Holds the sum of blue channels of all the sampled pixels\n>  \t */\n>  \tuint64_t sumB_;\n> +\t/**\n> +\t * \\brief Return the sums of colour channels of all the sampled pixels\n> +\t */\n> +\tRGB<uint64_t> rgbSum() const\n> +\t{\n> +\t\treturn RGB<uint64_t>({ sumR_, sumG_, sumB_ });\n> +\t}\n\nI would have replaced all this with\n\n\tRGB<uint64_t> sum_;\n\n(with an appropriate documentation block)\n\n>  \t/**\n>  \t * \\brief Number of bins in the yHistogram\n>  \t */\n> @@ -46,7 +55,7 @@ struct SwIspStats {\n>  \t */\n>  \tusing Histogram = std::array<uint32_t, kYHistogramSize>;\n>  \t/**\n> -\t * \\brief A histogram of luminance values\n> +\t * \\brief A histogram of luminance values of all the sampled pixels\n>  \t */\n>  \tHistogram yHistogram;\n>  };\n> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n> index 0080865aa..4c8bfd2de 100644\n> --- a/src/ipa/simple/algorithms/awb.cpp\n> +++ b/src/ipa/simple/algorithms/awb.cpp\n> @@ -1,6 +1,6 @@\n>  /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>  /*\n> - * Copyright (C) 2024, Red Hat Inc.\n> + * Copyright (C) 2024-2026 Red Hat Inc.\n>   *\n>   * Auto white balance\n>   */\n> @@ -73,9 +73,7 @@ void Awb::process(IPAContext &context,\n>  \t\thistogram.begin(), histogram.end(), uint64_t(0));\n>  \tconst uint64_t offset = blackLevel * nPixels;\n>  \tconst uint64_t minValid = 1;\n> -\tconst uint64_t sumR = stats->sumR_ > offset / 4 ? stats->sumR_ - offset / 4 : minValid;\n> -\tconst uint64_t sumG = stats->sumG_ > offset / 2 ? stats->sumG_ - offset / 2 : minValid;\n> -\tconst uint64_t sumB = stats->sumB_ > offset / 4 ? stats->sumB_ - offset / 4 : minValid;\n> +\tconst RGB<uint64_t> sum = (stats->rgbSum() - offset).max(minValid);\n>  \n>  \t/*\n>  \t * Calculate red and blue gains for AWB.\n> @@ -83,9 +81,9 @@ void Awb::process(IPAContext &context,\n>  \t */\n>  \tauto &gains = context.activeState.awb.gains;\n>  \tgains = { {\n> -\t\tsumR <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumR,\n> +\t\tsum.r() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.r(),\n>  \t\t1.0,\n> -\t\tsumB <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumB,\n> +\t\tsum.b() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.b(),\n>  \t} };\n>  \n>  \tRGB<double> rgbGains{ { 1 / gains.r(), 1 / gains.g(), 1 / gains.b() } };","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 2A45DC3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Jan 2026 14:47:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CCFB61FCE;\n\tTue, 27 Jan 2026 15:47:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9475961FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jan 2026 15:47:33 +0100 (CET)","from pendragon.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 0F4C1177A; \n\tTue, 27 Jan 2026 15:46:57 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DqLH/Cjh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769525217;\n\tbh=FGbpTriy+U2oyO9GyDQDMginuRmpaUFs1FMixQq6LzQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DqLH/CjhWJHSo/2Hivl/hUQfkywgH1TuHfw+BkfUNMOTvo2qezvtBzYFHEXZiTeop\n\tholtVG/UeeD5dzJdEN71KxTkWy82stFxDyU1ThWiDYJ3ep+UPQjWwcW/z8iuSy7jIy\n\tT59IvGChcNevFgsi2KFVe86ZXcITr5wi3gwjiqfU=","Date":"Tue, 27 Jan 2026 16:47:31 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","Message-ID":"<20260127144731.GA2502390@killaraus>","References":"<20260127123728.70513-1-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260127123728.70513-1-mzamazal@redhat.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":37970,"web_url":"https://patchwork.libcamera.org/comment/37970/","msgid":"<69e38997-03de-4517-8855-f6286bfd612b@collabora.com>","date":"2026-01-27T14:56:36","subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Nice, this seems to help quite a bit with reducing green tint on some \nqcom devices (with black levels set in the tuning files).\n\nOn 27.01.26 13:37, Milan Zamazal wrote:\n> The black level offset subtracted in AWB is wrong.  It assumes that the\n> stats contain sums of the individual colour pixels.  But they actually\n> contain sums of the colour channels of larger \"superpixels\" consisting\n> of the individual colour pixels.  Each of the RGB colour values and the\n> computed luminosity (a histogram entry) are added once to the stats per\n> such a superpixel.  This means the offset computed from the black level\n> and the number of pixels should be used as it is, not divided.\n>\n> The patch fixes the subtracted offset.  Since it is the same for all the\n> three colours, a SwIspStats helper method returning an RGB instance is\n> introduced.  The individual colour variables are still retained in\n> SwIspStats for maximum efficiency when gathering the stats.  SwIspStats\n> docstrings are changed to avoid the original confusion.\n>\n> Fixes: 4e13c6f55bd667 (\"Honor black level in AWB\")\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>   .../internal/software_isp/swisp_stats.h         | 17 +++++++++++++----\n>   src/ipa/simple/algorithms/awb.cpp               | 10 ++++------\n>   2 files changed, 17 insertions(+), 10 deletions(-)\n>\n> diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h\n> index 3c9860185..f8816209f 100644\n> --- a/include/libcamera/internal/software_isp/swisp_stats.h\n> +++ b/include/libcamera/internal/software_isp/swisp_stats.h\n> @@ -10,6 +10,8 @@\n>   #include <array>\n>   #include <stdint.h>\n>   \n> +#include \"libcamera/internal/vector.h\"\n> +\n>   namespace libcamera {\n>   \n>   /**\n> @@ -26,17 +28,24 @@ struct SwIspStats {\n>   \t */\n>   \tbool valid;\n>   \t/**\n> -\t * \\brief Holds the sum of all sampled red pixels\n> +\t * \\brief Holds the sum of red channels of all the sampled pixels\n>   \t */\n>   \tuint64_t sumR_;\n>   \t/**\n> -\t * \\brief Holds the sum of all sampled green pixels\n> +\t * \\brief Holds the sum of green channels of all the sampled pixels\n>   \t */\n>   \tuint64_t sumG_;\n>   \t/**\n> -\t * \\brief Holds the sum of all sampled blue pixels\n> +\t * \\brief Holds the sum of blue channels of all the sampled pixels\n>   \t */\n>   \tuint64_t sumB_;\n> +\t/**\n> +\t * \\brief Return the sums of colour channels of all the sampled pixels\n> +\t */\n> +\tRGB<uint64_t> rgbSum() const\n> +\t{\n> +\t\treturn RGB<uint64_t>({ sumR_, sumG_, sumB_ });\n> +\t}\n>   \t/**\n>   \t * \\brief Number of bins in the yHistogram\n>   \t */\n> @@ -46,7 +55,7 @@ struct SwIspStats {\n>   \t */\n>   \tusing Histogram = std::array<uint32_t, kYHistogramSize>;\n>   \t/**\n> -\t * \\brief A histogram of luminance values\n> +\t * \\brief A histogram of luminance values of all the sampled pixels\n>   \t */\n>   \tHistogram yHistogram;\n>   };\n> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n> index 0080865aa..4c8bfd2de 100644\n> --- a/src/ipa/simple/algorithms/awb.cpp\n> +++ b/src/ipa/simple/algorithms/awb.cpp\n> @@ -1,6 +1,6 @@\n>   /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>   /*\n> - * Copyright (C) 2024, Red Hat Inc.\n> + * Copyright (C) 2024-2026 Red Hat Inc.\n>    *\n>    * Auto white balance\n>    */\n> @@ -73,9 +73,7 @@ void Awb::process(IPAContext &context,\n>   \t\thistogram.begin(), histogram.end(), uint64_t(0));\n>   \tconst uint64_t offset = blackLevel * nPixels;\n>   \tconst uint64_t minValid = 1;\n> -\tconst uint64_t sumR = stats->sumR_ > offset / 4 ? stats->sumR_ - offset / 4 : minValid;\n> -\tconst uint64_t sumG = stats->sumG_ > offset / 2 ? stats->sumG_ - offset / 2 : minValid;\n> -\tconst uint64_t sumB = stats->sumB_ > offset / 4 ? stats->sumB_ - offset / 4 : minValid;\n> +\tconst RGB<uint64_t> sum = (stats->rgbSum() - offset).max(minValid);\n>   \n>   \t/*\n>   \t * Calculate red and blue gains for AWB.\n> @@ -83,9 +81,9 @@ void Awb::process(IPAContext &context,\n>   \t */\n>   \tauto &gains = context.activeState.awb.gains;\n>   \tgains = { {\n> -\t\tsumR <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumR,\n> +\t\tsum.r() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.r(),\n>   \t\t1.0,\n> -\t\tsumB <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumB,\n> +\t\tsum.b() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.b(),\n>   \t} };\n>   \n>   \tRGB<double> rgbGains{ { 1 / gains.r(), 1 / gains.g(), 1 / gains.b() } };","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 1DEFBC3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Jan 2026 14:56:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68FB361FC8;\n\tTue, 27 Jan 2026 15:56:46 +0100 (CET)","from sender4-op-o12.zoho.com (sender4-op-o12.zoho.com\n\t[136.143.188.12])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7100761FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jan 2026 15:56:44 +0100 (CET)","by mx.zohomail.com with SMTPS id 1769525799556157.35793610915243; \n\tTue, 27 Jan 2026 06:56:39 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"Em7FVfC7\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1769525801; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=lA1OvaF9upwui1RIJt14fLY52WUSAiWp4GgQZYxpzJxmO63Yq1NFjg0u8H05GW6xmp7X1ZBcjddE7la/Im6NMat66/7YVuXuUuLSIbariuU1WJq0Acb93RDOyoAZNFJ/0tpbLgRA0j7LyVBJQYUCzhHF3dBl2Cg9oRdh0y0Q27U=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1769525801;\n\th=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To:Cc;\n\tbh=gXguXX3fVgGu7+IIoxQ63E0as/ULFCLB3T5lELD1yX0=; \n\tb=MM4RTlWAoTfk14NL1ZySTU7ll3Oue+GOBTZDzqQ7YSj38okmjWkqLQK4LY1KZCELZ6C0SGvIgFTxk/8VNOvzseZKXBmaGzFkrf8GGzQ1TQTPorGET0p69CNKdqS5cXM1mjSilqVMGexeAGxDrUsoOjKs4+xkBUkzqWOSY36yf0Y=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass  header.i=collabora.com;\n\tspf=pass  smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=<robert.mader@collabora.com>","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1769525801;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To:Cc;\n\tbh=gXguXX3fVgGu7+IIoxQ63E0as/ULFCLB3T5lELD1yX0=;\n\tb=Em7FVfC7LFqYOfh227kdyfP01c2pqOki/h1d9zP/GPMSCgunybTqZZp04AOYnwWj\n\tixNWQhVdDCwQr9mREXGpuqqFfRCfzRpc+PFQxWHsP8MbSQ5URIikV/uL7XtEuB6+riX\n\tAo1nF0P0pNl2bLiSuQrFo4lv3gi/KNBgpfTeUzT4=","Message-ID":"<69e38997-03de-4517-8855-f6286bfd612b@collabora.com>","Date":"Tue, 27 Jan 2026 15:56:36 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","To":"libcamera-devel@lists.libcamera.org","References":"<20260127123728.70513-1-mzamazal@redhat.com>","Content-Language":"en-US, de-DE","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<20260127123728.70513-1-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":37972,"web_url":"https://patchwork.libcamera.org/comment/37972/","msgid":"<85sebrt3tm.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-01-27T16:31:33","subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n\n> On Tue, Jan 27, 2026 at 01:37:28PM +0100, Milan Zamazal wrote:\n>> The black level offset subtracted in AWB is wrong.  It assumes that the\n>> stats contain sums of the individual colour pixels.  But they actually\n>\n>> contain sums of the colour channels of larger \"superpixels\" consisting\n>> of the individual colour pixels.  Each of the RGB colour values and the\n>> computed luminosity (a histogram entry) are added once to the stats per\n>> such a superpixel.  This means the offset computed from the black level\n>> and the number of pixels should be used as it is, not divided.\n>> \n>> The patch fixes the subtracted offset.  Since it is the same for all the\n>> three colours, a SwIspStats helper method returning an RGB instance is\n>> introduced.  The individual colour variables are still retained in\n>> SwIspStats for maximum efficiency when gathering the stats.  SwIspStats\n>> docstrings are changed to avoid the original confusion.\n>> \n>> Fixes: 4e13c6f55bd667 (\"Honor black level in AWB\")\n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>  .../internal/software_isp/swisp_stats.h         | 17 +++++++++++++----\n>>  src/ipa/simple/algorithms/awb.cpp               | 10 ++++------\n>>  2 files changed, 17 insertions(+), 10 deletions(-)\n>> \n>> diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h\n>> index 3c9860185..f8816209f 100644\n>> --- a/include/libcamera/internal/software_isp/swisp_stats.h\n>> +++ b/include/libcamera/internal/software_isp/swisp_stats.h\n>> @@ -10,6 +10,8 @@\n>>  #include <array>\n>>  #include <stdint.h>\n>>  \n>> +#include \"libcamera/internal/vector.h\"\n>> +\n>>  namespace libcamera {\n>>  \n>>  /**\n>> @@ -26,17 +28,24 @@ struct SwIspStats {\n>>  \t */\n>>  \tbool valid;\n>>  \t/**\n>> -\t * \\brief Holds the sum of all sampled red pixels\n>> +\t * \\brief Holds the sum of red channels of all the sampled pixels\n>>  \t */\n>>  \tuint64_t sumR_;\n>>  \t/**\n>> -\t * \\brief Holds the sum of all sampled green pixels\n>> +\t * \\brief Holds the sum of green channels of all the sampled pixels\n>>  \t */\n>>  \tuint64_t sumG_;\n>>  \t/**\n>> -\t * \\brief Holds the sum of all sampled blue pixels\n>> +\t * \\brief Holds the sum of blue channels of all the sampled pixels\n>>  \t */\n>>  \tuint64_t sumB_;\n>> +\t/**\n>> +\t * \\brief Return the sums of colour channels of all the sampled pixels\n>> +\t */\n>> +\tRGB<uint64_t> rgbSum() const\n>> +\t{\n>> +\t\treturn RGB<uint64_t>({ sumR_, sumG_, sumB_ });\n>> +\t}\n>\n> I would have replaced all this with\n>\n> \tRGB<uint64_t> sum_;\n\nOK, I measured the performance impact and there is none in my\nenvironment.  Done in v2.\n\n> (with an appropriate documentation block)\n\nI don't feel particularly creative here for an internal structure.  Do\nwe need to mention anything special here?\n\n>>  \t/**\n>>  \t * \\brief Number of bins in the yHistogram\n>>  \t */\n>> @@ -46,7 +55,7 @@ struct SwIspStats {\n>>  \t */\n>>  \tusing Histogram = std::array<uint32_t, kYHistogramSize>;\n>>  \t/**\n>> -\t * \\brief A histogram of luminance values\n>> +\t * \\brief A histogram of luminance values of all the sampled pixels\n>>  \t */\n>>  \tHistogram yHistogram;\n>>  };\n>> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n>> index 0080865aa..4c8bfd2de 100644\n>> --- a/src/ipa/simple/algorithms/awb.cpp\n>> +++ b/src/ipa/simple/algorithms/awb.cpp\n>> @@ -1,6 +1,6 @@\n>>  /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>  /*\n>> - * Copyright (C) 2024, Red Hat Inc.\n>> + * Copyright (C) 2024-2026 Red Hat Inc.\n>>   *\n>>   * Auto white balance\n>>   */\n>> @@ -73,9 +73,7 @@ void Awb::process(IPAContext &context,\n>>  \t\thistogram.begin(), histogram.end(), uint64_t(0));\n>>  \tconst uint64_t offset = blackLevel * nPixels;\n>>  \tconst uint64_t minValid = 1;\n>> -\tconst uint64_t sumR = stats->sumR_ > offset / 4 ? stats->sumR_ - offset / 4 : minValid;\n>> -\tconst uint64_t sumG = stats->sumG_ > offset / 2 ? stats->sumG_ - offset / 2 : minValid;\n>> -\tconst uint64_t sumB = stats->sumB_ > offset / 4 ? stats->sumB_ - offset / 4 : minValid;\n>> +\tconst RGB<uint64_t> sum = (stats->rgbSum() - offset).max(minValid);\n>>  \n>>  \t/*\n>>  \t * Calculate red and blue gains for AWB.\n>> @@ -83,9 +81,9 @@ void Awb::process(IPAContext &context,\n>>  \t */\n>>  \tauto &gains = context.activeState.awb.gains;\n>>  \tgains = { {\n>> -\t\tsumR <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumR,\n>> +\t\tsum.r() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.r(),\n>>  \t\t1.0,\n>> -\t\tsumB <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumB,\n>> +\t\tsum.b() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.b(),\n>>  \t} };\n>>  \n>>  \tRGB<double> rgbGains{ { 1 / gains.r(), 1 / gains.g(), 1 / gains.b() } };","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 6874BC3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Jan 2026 16:31:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 600A261FC8;\n\tTue, 27 Jan 2026 17:31:41 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BF3B961FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jan 2026 17:31:39 +0100 (CET)","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-170-1jGRSF-PMN-KyIQntmTpcg-1; Tue, 27 Jan 2026 11:31:37 -0500","by mail-wr1-f70.google.com with SMTP id\n\tffacd0b85a97d-430fcfe4494so5426642f8f.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jan 2026 08:31:37 -0800 (PST)","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-435b1e7156dsm39645134f8f.20.2026.01.27.08.31.33\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 27 Jan 2026 08:31:34 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Pb/1W/do\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1769531498;\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=dWs/hewLvml1Te+QypIQj3hOaHW5xitl86ouDuLdzWs=;\n\tb=Pb/1W/do+92BVyLCqFOnQ2CNV3NJzAp0CaORrZ+wxunorTEDG2fxyIudKI34lmvVfQOi0U\n\tD2DDPkaqioVtifjBMJc5uw7l+mJu2a+/lcW3cQMAcoXv4Iha2nf+l8JB9Ejs7IIF2XTH3I\n\tiKJr5GWHuYmpvR7zk9TBZX3ZfD3/zSQ=","X-MC-Unique":"1jGRSF-PMN-KyIQntmTpcg-1","X-Mimecast-MFC-AGG-ID":"1jGRSF-PMN-KyIQntmTpcg_1769531496","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1769531495; x=1770136295;\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=dWs/hewLvml1Te+QypIQj3hOaHW5xitl86ouDuLdzWs=;\n\tb=wEz5GoAMVcMdlytMIHxkCHJy3iRuGpN7gt6KwL++cgKN7numUAZ69+iMjbRCWEwdee\n\tvuv2HuCDVd4rIV2wrz3pfGV7PeELfWltOXhw+CKOWZ5ZqHE4O/8Z8TXIPL6kzfh/s7dt\n\txXLvlDCmT4VyMWIWrCKDXaFmB5keTYHUFaqcMTrsGARsnCw+QW/zg09geamFOi8FGo1S\n\tlAvFr4pviW8R8SkyFE3IlMS2VNgL5W8NR8T6TfwgzTLOi2k7B5Gz7v9VaDzJZJGH5wMN\n\tsVWa5iQrwXPjYBy4tW/m7rAbztOvxLRehXr9Ej1ZW8RVl/yF8spxUJy3nIHH8HWCMsCy\n\thi0w==","X-Gm-Message-State":"AOJu0YxFfn+7lHHP3xK3PMMx/8/hmNvZq0KnMdPpTxIijed6eHm3P0dM\n\tw6/6vyl6sDtVtiGPPmeE/IxpaHVSWKUbRVnJxjdVM3ppJ8hHhIGo1Bwwf8p9+XGnzkzMOmAlnhZ\n\tCCH8AZQci2K6i4J7xeL7zhBCe/K1fTQBWpJOmUPIXN6Y+wd+XX/RVupdLeoelOQv6Mw6trqoubW\n\tE9Nz8zJ1H5HtW4IN95LKu6WsUkXkIyyi8T+G/88wmdpekY01ayhqxTFnj7iy0=","X-Gm-Gg":"AZuq6aK4GUJ8nmyhE13KIuaKEe53DKDBdTtb5rbzQX4zY2YOVppNRdDNYDF+fmTl16i\n\t0CTxy4niqFV41OuToswbSS6JNccg7hYfsdX1BOfXqFxzDHMc5/wChsDMw+QUMkohrjA96d0Jp6D\n\tv/wpHe3fYe3loxbYmX2tay/yt/lvKDxhspyRyi07QwYtYwu4icJWAB4q8Dh9IHjE10Ns+TriHGW\n\tFjcNHYprwhieKZDZqPa3AQypLxPPjORqBocUy8UPbJh2JhcmUFztbuAM/l5hRwcFKCRwl9dkFOD\n\tPKeKjIXCZOtiZvkvtVzHIlqyoGnNOX6aTu+TbNBsXQckTGGA0NeYmdC3M6uEC/+cIVseTktdA87\n\t0mQB7SQkpvTeOhuoT+y1eoil3LAvblnIaLwMKRx63njPO6m1wZobjJzDbzJpNLi8=","X-Received":["by 2002:a05:6000:2508:b0:430:8583:d19b with SMTP id\n\tffacd0b85a97d-435dd0b125fmr3310004f8f.33.1769531495487; \n\tTue, 27 Jan 2026 08:31:35 -0800 (PST)","by 2002:a05:6000:2508:b0:430:8583:d19b with SMTP id\n\tffacd0b85a97d-435dd0b125fmr3309953f8f.33.1769531494935; \n\tTue, 27 Jan 2026 08:31:34 -0800 (PST)"],"From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","In-Reply-To":"<20260127144731.GA2502390@killaraus> (Laurent Pinchart's message\n\tof \"Tue, 27 Jan 2026 16:47:31 +0200\")","References":"<20260127123728.70513-1-mzamazal@redhat.com>\n\t<20260127144731.GA2502390@killaraus>","Date":"Tue, 27 Jan 2026 17:31:33 +0100","Message-ID":"<85sebrt3tm.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":"fl_dfMI4MT6SLJLQsM4QVF0tYmQkhfMMJSmxjCcU-5E_1769531496","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":37973,"web_url":"https://patchwork.libcamera.org/comment/37973/","msgid":"<85o6mft3k4.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-01-27T16:37:15","subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Robert Mader <robert.mader@collabora.com> writes:\n\n> Nice, this seems to help quite a bit with reducing green tint on some qcom devices (with black levels set in the tuning files).\n\nThank you for testing and confirmation it works as expected in properly\nset up environments.\n\n> On 27.01.26 13:37, Milan Zamazal wrote:\n>> The black level offset subtracted in AWB is wrong.  It assumes that the\n>> stats contain sums of the individual colour pixels.  But they actually\n>> contain sums of the colour channels of larger \"superpixels\" consisting\n>> of the individual colour pixels.  Each of the RGB colour values and the\n>> computed luminosity (a histogram entry) are added once to the stats per\n>> such a superpixel.  This means the offset computed from the black level\n>> and the number of pixels should be used as it is, not divided.\n>>\n>> The patch fixes the subtracted offset.  Since it is the same for all the\n>> three colours, a SwIspStats helper method returning an RGB instance is\n>> introduced.  The individual colour variables are still retained in\n>> SwIspStats for maximum efficiency when gathering the stats.  SwIspStats\n>> docstrings are changed to avoid the original confusion.\n>>\n>> Fixes: 4e13c6f55bd667 (\"Honor black level in AWB\")\n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>   .../internal/software_isp/swisp_stats.h         | 17 +++++++++++++----\n>>   src/ipa/simple/algorithms/awb.cpp               | 10 ++++------\n>>   2 files changed, 17 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h\n>> index 3c9860185..f8816209f 100644\n>> --- a/include/libcamera/internal/software_isp/swisp_stats.h\n>> +++ b/include/libcamera/internal/software_isp/swisp_stats.h\n>> @@ -10,6 +10,8 @@\n>>   #include <array>\n>>   #include <stdint.h>\n>>   +#include \"libcamera/internal/vector.h\"\n>> +\n>>   namespace libcamera {\n>>     /**\n>> @@ -26,17 +28,24 @@ struct SwIspStats {\n>>   \t */\n>>   \tbool valid;\n>>   \t/**\n>> -\t * \\brief Holds the sum of all sampled red pixels\n>> +\t * \\brief Holds the sum of red channels of all the sampled pixels\n>>   \t */\n>>   \tuint64_t sumR_;\n>>   \t/**\n>> -\t * \\brief Holds the sum of all sampled green pixels\n>> +\t * \\brief Holds the sum of green channels of all the sampled pixels\n>>   \t */\n>>   \tuint64_t sumG_;\n>>   \t/**\n>> -\t * \\brief Holds the sum of all sampled blue pixels\n>> +\t * \\brief Holds the sum of blue channels of all the sampled pixels\n>>   \t */\n>>   \tuint64_t sumB_;\n>> +\t/**\n>> +\t * \\brief Return the sums of colour channels of all the sampled pixels\n>> +\t */\n>> +\tRGB<uint64_t> rgbSum() const\n>> +\t{\n>> +\t\treturn RGB<uint64_t>({ sumR_, sumG_, sumB_ });\n>> +\t}\n>>   \t/**\n>>   \t * \\brief Number of bins in the yHistogram\n>>   \t */\n>> @@ -46,7 +55,7 @@ struct SwIspStats {\n>>   \t */\n>>   \tusing Histogram = std::array<uint32_t, kYHistogramSize>;\n>>   \t/**\n>> -\t * \\brief A histogram of luminance values\n>> +\t * \\brief A histogram of luminance values of all the sampled pixels\n>>   \t */\n>>   \tHistogram yHistogram;\n>>   };\n>> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n>> index 0080865aa..4c8bfd2de 100644\n>> --- a/src/ipa/simple/algorithms/awb.cpp\n>> +++ b/src/ipa/simple/algorithms/awb.cpp\n>> @@ -1,6 +1,6 @@\n>>   /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>   /*\n>> - * Copyright (C) 2024, Red Hat Inc.\n>> + * Copyright (C) 2024-2026 Red Hat Inc.\n>>    *\n>>    * Auto white balance\n>>    */\n>> @@ -73,9 +73,7 @@ void Awb::process(IPAContext &context,\n>>   \t\thistogram.begin(), histogram.end(), uint64_t(0));\n>>   \tconst uint64_t offset = blackLevel * nPixels;\n>>   \tconst uint64_t minValid = 1;\n>> -\tconst uint64_t sumR = stats->sumR_ > offset / 4 ? stats->sumR_ - offset / 4 : minValid;\n>> -\tconst uint64_t sumG = stats->sumG_ > offset / 2 ? stats->sumG_ - offset / 2 : minValid;\n>> -\tconst uint64_t sumB = stats->sumB_ > offset / 4 ? stats->sumB_ - offset / 4 : minValid;\n>> +\tconst RGB<uint64_t> sum = (stats->rgbSum() - offset).max(minValid);\n>>     \t/*\n>>   \t * Calculate red and blue gains for AWB.\n>> @@ -83,9 +81,9 @@ void Awb::process(IPAContext &context,\n>>   \t */\n>>   \tauto &gains = context.activeState.awb.gains;\n>>   \tgains = { {\n>> -\t\tsumR <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumR,\n>> +\t\tsum.r() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.r(),\n>>   \t\t1.0,\n>> -\t\tsumB <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumB,\n>> +\t\tsum.b() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.b(),\n>>   \t} };\n>>     \tRGB<double> rgbGains{ { 1 / gains.r(), 1 / gains.g(), 1 / gains.b() } };","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 2D0BCC3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Jan 2026 16:37:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5142D61FC8;\n\tTue, 27 Jan 2026 17:37:23 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D1DC261FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jan 2026 17:37:21 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-396-Jeavdoh7NNKtAEfEpL4rpw-1; Tue, 27 Jan 2026 11:37:18 -0500","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-47ee1fe7b24so50726655e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jan 2026 08:37:18 -0800 (PST)","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-48066c3732dsm71625345e9.11.2026.01.27.08.37.15\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 27 Jan 2026 08:37:16 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"RPgVou/U\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1769531840;\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=vonU7q+u80Pcgck6RDqpxkHiVjE3fEMpQMGVY/EOgz8=;\n\tb=RPgVou/UW9u45PRYZsl8gixHTcRS1BC65AsFPKObsBEl8Wnk+gXGRjfFL1YrlMkjzoqdXu\n\tUmeYo5dNuAayYg9h62upiI7oySBxRDtwD9QFjrGB97hY0Q5YO3g7p4FeYs5SHdNMKftk6P\n\toYwJdxHLvLsyFgSFwGugOZVAgQ5Ol7A=","X-MC-Unique":"Jeavdoh7NNKtAEfEpL4rpw-1","X-Mimecast-MFC-AGG-ID":"Jeavdoh7NNKtAEfEpL4rpw_1769531838","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1769531837; x=1770136637;\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=vonU7q+u80Pcgck6RDqpxkHiVjE3fEMpQMGVY/EOgz8=;\n\tb=fc/OVJTyHDoWT3v3IYg/cmDlxP+nKxGBpw7fLI+9qKepKFsnWcyp48hfhaAOcUyj4B\n\t/NhUY4TKhQuO/sowd4A+kzTTA7ze47f2K4uBikYO7SBRkTh4BFhugxxLN12JeS14Sq3v\n\tvYipIO8eIYfEJ0ZhZdVQkl9ppyrUhnQSLfbUTrxVjeeDcFcHQtoF4rM8g3f+rIG+Ayse\n\tnCrxYpz8k45Be7tb7Zll2DIYz1fdq8GGeoAo4K50HMuJXMSsH+4t/t1MGLsNAdF8jzdB\n\tAFcfaM49/6Q6NQLHOgNQeNLFhJJc1Fs1ofWsPNlN4HdhIjjaF9+bT5VYNnTMQmS0GRxc\n\tqxrQ==","X-Gm-Message-State":"AOJu0YxWpGp9uH1fMtXzecP81nCFa+R3HR3P07StBFa3Zm1nuM6YzJg8\n\t7eVaJbtseR8rwD9SLn5oYu8ev7HOAcR2q296COzrNy9HB+gJd9Ye43u3XfDOtoZOyFTCr1Hlj1B\n\tak7hpbVC68EHA37hyKNKNMmdKk+W04rPg6k99sFxLqeODObwTQI5s21d/+zlmoFpv7UlMPQDKaG\n\tqW3J8ofUjxAxQgmhm0wVXxfNCbSLVITUOt6J1lXfng+8fZDPKPFnojrDySgdg=","X-Gm-Gg":"AZuq6aKcUtIPHrb3WhOW+N32ugmICrtzwGXIcAS0aclpqgvbHePEDHJVKsGsSeHySNe\n\tz7bz0qj6K4yF9rs9eUywFo2FY9bzGe4NLjtjKuEG808/dVRFyhJLoVyk9QavKdXQj+Xk7yDHqxQ\n\tD6gGSR0vMWECdjpEFV2gre8gOKd1B4EN1G2c1o63VUnhIvZ/aA1Vct3yYal6lUAegn4YVdeEulc\n\tXnY5goxa5jMHcemxpI1GY5ubKdBZ0yxo93+WTyrVA2UvZVpTQzYjfYpuSXiLGViIaQdF4M4u6sR\n\thpzURRPNGA12q0NFvQa+KAKerNJ3GEh1CoEV9yxsIpi9mNCz6QrlPFz4uFDn1CCMP1Prz6YQ/pG\n\tAWqlsnukiu1zzhw9iaPP0Iwm6qqNaxnoxgGw349/A8y7Tin+vHPJXGLMR339vk/8=","X-Received":["by 2002:a05:600c:8010:b0:471:1717:411 with SMTP id\n\t5b1f17b1804b1-48069c5569bmr31226965e9.24.1769531837200; \n\tTue, 27 Jan 2026 08:37:17 -0800 (PST)","by 2002:a05:600c:8010:b0:471:1717:411 with SMTP id\n\t5b1f17b1804b1-48069c5569bmr31226555e9.24.1769531836626; \n\tTue, 27 Jan 2026 08:37:16 -0800 (PST)"],"From":"Milan Zamazal <mzamazal@redhat.com>","To":"Robert Mader <robert.mader@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","In-Reply-To":"<69e38997-03de-4517-8855-f6286bfd612b@collabora.com> (Robert\n\tMader's message of \"Tue, 27 Jan 2026 15:56:36 +0100\")","References":"<20260127123728.70513-1-mzamazal@redhat.com>\n\t<69e38997-03de-4517-8855-f6286bfd612b@collabora.com>","Date":"Tue, 27 Jan 2026 17:37:15 +0100","Message-ID":"<85o6mft3k4.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":"pxQ5R5JVkuHmLgI6Xrvf1GzwL6mXEG9LQ6nB85p8t5U_1769531838","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":37974,"web_url":"https://patchwork.libcamera.org/comment/37974/","msgid":"<20260127164015.GA2512313@killaraus>","date":"2026-01-27T16:40:15","subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 27, 2026 at 05:31:33PM +0100, Milan Zamazal wrote:\n> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n> \n> > On Tue, Jan 27, 2026 at 01:37:28PM +0100, Milan Zamazal wrote:\n> >> The black level offset subtracted in AWB is wrong.  It assumes that the\n> >> stats contain sums of the individual colour pixels.  But they actually\n> >\n> >> contain sums of the colour channels of larger \"superpixels\" consisting\n> >> of the individual colour pixels.  Each of the RGB colour values and the\n> >> computed luminosity (a histogram entry) are added once to the stats per\n> >> such a superpixel.  This means the offset computed from the black level\n> >> and the number of pixels should be used as it is, not divided.\n> >> \n> >> The patch fixes the subtracted offset.  Since it is the same for all the\n> >> three colours, a SwIspStats helper method returning an RGB instance is\n> >> introduced.  The individual colour variables are still retained in\n> >> SwIspStats for maximum efficiency when gathering the stats.  SwIspStats\n> >> docstrings are changed to avoid the original confusion.\n> >> \n> >> Fixes: 4e13c6f55bd667 (\"Honor black level in AWB\")\n> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> >> ---\n> >>  .../internal/software_isp/swisp_stats.h         | 17 +++++++++++++----\n> >>  src/ipa/simple/algorithms/awb.cpp               | 10 ++++------\n> >>  2 files changed, 17 insertions(+), 10 deletions(-)\n> >> \n> >> diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h\n> >> index 3c9860185..f8816209f 100644\n> >> --- a/include/libcamera/internal/software_isp/swisp_stats.h\n> >> +++ b/include/libcamera/internal/software_isp/swisp_stats.h\n> >> @@ -10,6 +10,8 @@\n> >>  #include <array>\n> >>  #include <stdint.h>\n> >>  \n> >> +#include \"libcamera/internal/vector.h\"\n> >> +\n> >>  namespace libcamera {\n> >>  \n> >>  /**\n> >> @@ -26,17 +28,24 @@ struct SwIspStats {\n> >>  \t */\n> >>  \tbool valid;\n> >>  \t/**\n> >> -\t * \\brief Holds the sum of all sampled red pixels\n> >> +\t * \\brief Holds the sum of red channels of all the sampled pixels\n> >>  \t */\n> >>  \tuint64_t sumR_;\n> >>  \t/**\n> >> -\t * \\brief Holds the sum of all sampled green pixels\n> >> +\t * \\brief Holds the sum of green channels of all the sampled pixels\n> >>  \t */\n> >>  \tuint64_t sumG_;\n> >>  \t/**\n> >> -\t * \\brief Holds the sum of all sampled blue pixels\n> >> +\t * \\brief Holds the sum of blue channels of all the sampled pixels\n> >>  \t */\n> >>  \tuint64_t sumB_;\n> >> +\t/**\n> >> +\t * \\brief Return the sums of colour channels of all the sampled pixels\n> >> +\t */\n> >> +\tRGB<uint64_t> rgbSum() const\n> >> +\t{\n> >> +\t\treturn RGB<uint64_t>({ sumR_, sumG_, sumB_ });\n> >> +\t}\n> >\n> > I would have replaced all this with\n> >\n> > \tRGB<uint64_t> sum_;\n> \n> OK, I measured the performance impact and there is none in my\n> environment.  Done in v2.\n> \n> > (with an appropriate documentation block)\n> \n> I don't feel particularly creative here for an internal structure.  Do\n> we need to mention anything special here?\n\nNo, I just mentioned it to make it clear I wasn't suggesting dropping\nthe documentation. Something like\n\n\t/**\n\t * \\brief The sum of all sampled pixels, separately for R, G and B.\n\t */\n\nwould do.\n\n> >>  \t/**\n> >>  \t * \\brief Number of bins in the yHistogram\n> >>  \t */\n> >> @@ -46,7 +55,7 @@ struct SwIspStats {\n> >>  \t */\n> >>  \tusing Histogram = std::array<uint32_t, kYHistogramSize>;\n> >>  \t/**\n> >> -\t * \\brief A histogram of luminance values\n> >> +\t * \\brief A histogram of luminance values of all the sampled pixels\n> >>  \t */\n> >>  \tHistogram yHistogram;\n> >>  };\n> >> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n> >> index 0080865aa..4c8bfd2de 100644\n> >> --- a/src/ipa/simple/algorithms/awb.cpp\n> >> +++ b/src/ipa/simple/algorithms/awb.cpp\n> >> @@ -1,6 +1,6 @@\n> >>  /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>  /*\n> >> - * Copyright (C) 2024, Red Hat Inc.\n> >> + * Copyright (C) 2024-2026 Red Hat Inc.\n> >>   *\n> >>   * Auto white balance\n> >>   */\n> >> @@ -73,9 +73,7 @@ void Awb::process(IPAContext &context,\n> >>  \t\thistogram.begin(), histogram.end(), uint64_t(0));\n> >>  \tconst uint64_t offset = blackLevel * nPixels;\n> >>  \tconst uint64_t minValid = 1;\n> >> -\tconst uint64_t sumR = stats->sumR_ > offset / 4 ? stats->sumR_ - offset / 4 : minValid;\n> >> -\tconst uint64_t sumG = stats->sumG_ > offset / 2 ? stats->sumG_ - offset / 2 : minValid;\n> >> -\tconst uint64_t sumB = stats->sumB_ > offset / 4 ? stats->sumB_ - offset / 4 : minValid;\n> >> +\tconst RGB<uint64_t> sum = (stats->rgbSum() - offset).max(minValid);\n> >>  \n> >>  \t/*\n> >>  \t * Calculate red and blue gains for AWB.\n> >> @@ -83,9 +81,9 @@ void Awb::process(IPAContext &context,\n> >>  \t */\n> >>  \tauto &gains = context.activeState.awb.gains;\n> >>  \tgains = { {\n> >> -\t\tsumR <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumR,\n> >> +\t\tsum.r() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.r(),\n> >>  \t\t1.0,\n> >> -\t\tsumB <= sumG / 4 ? 4.0f : static_cast<float>(sumG) / sumB,\n> >> +\t\tsum.b() <= sum.g() / 4 ? 4.0f : static_cast<float>(sum.g()) / sum.b(),\n> >>  \t} };\n> >>  \n> >>  \tRGB<double> rgbGains{ { 1 / gains.r(), 1 / gains.g(), 1 / gains.b() } };","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 A6462C3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Jan 2026 16:40:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC81D61FC8;\n\tTue, 27 Jan 2026 17:40:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2443C61FBF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jan 2026 17:40:17 +0100 (CET)","from pendragon.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 A4976465;\n\tTue, 27 Jan 2026 17:39:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OS4wcC86\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769531980;\n\tbh=oeOxa47fJw6q451nFV5jHbmyx8zi8nEErP4bKx1Dhxg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OS4wcC86BvzH+pQtn2mcjBsvvrLqwXyE8i92/JYbCZ6+pA0RCRry4hIx4YztcOkf5\n\tQD9PU/t863EjRF6PFuEd6op9MX8FUU+fA3ul6OPI6Y+de4YmlusjmF9xamYKOmWCy/\n\tQ+/Hez1L53D16BZu83h3nK1WIl9BkCk2lgyje1zQ=","Date":"Tue, 27 Jan 2026 18:40:15 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: simple: Fix black level offsets in AWB","Message-ID":"<20260127164015.GA2512313@killaraus>","References":"<20260127123728.70513-1-mzamazal@redhat.com>\n\t<20260127144731.GA2502390@killaraus>\n\t<85sebrt3tm.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<85sebrt3tm.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","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>"}}]