From patchwork Mon Oct 26 17:34:33 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robert Mader X-Patchwork-Id: 28642 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 452F7C3261 for ; Mon, 26 Oct 2026 17:35:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C525069013; Mon, 26 Oct 2026 18:35:05 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=collabora.com header.i=robert.mader@collabora.com header.b="cYEFy87C"; dkim-atps=neutral Received: from sender5-op-o11.zoho.com (sender5-op-o11.zoho.com [165.173.182.11]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2330268FD7 for ; Mon, 26 Oct 2026 18:35:03 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; t=1793036100; cv=none; d=zohomail.com; s=zohoarc; b=h6lFG5ih1VpMg9iGqtGvAr1mhDOnBCS24Wj1OT6Mhv7fSvK9qj3VtTHDdGvZI5bqNbp1/cF2Rbx21sJCFv4UF1pZx2Z+BoKAUYDdi3oLDZJ70XcIROnjM5oCdOZdVGcRue01RZ1zMTYGzbRQcjMeachLjoSG/8WzWyblpMHo4Yc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1793036100; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:MIME-Version:Message-ID:Subject:Subject:To:To:Message-Id:Reply-To; bh=f3OMqMM4XPxePcaiJdrQSJZjfUnQL5w1ZYwxaXToyJ8=; b=Q3Y+o8Pt2zKzDksh82aK2gT92303QGk4D/Q87O2P3R5i/n4o0pdMMVMJCY2S/CikIQOYUZ4WBaX9Po09N1sNOqLPpAWb3roVW5GsWM2DpU0x6piue44aI8067P6auc1FdEaIf7SMHvRVGFhow1Uko6iMiICgIJNgFFYcD0QibsI= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=collabora.com; spf=pass smtp.mailfrom=robert.mader@collabora.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1793036100; s=zohomail; d=collabora.com; i=robert.mader@collabora.com; h=From:From:To:To:Cc:Cc:Subject:Subject:Date:Date:Message-ID:MIME-Version:Content-Transfer-Encoding:Message-Id:Reply-To; bh=f3OMqMM4XPxePcaiJdrQSJZjfUnQL5w1ZYwxaXToyJ8=; b=cYEFy87CF/SBe9B9dCsMFeZJ8kUliEMorqcC/uf5N0xKIP88kbeb7lHu/YPq1thG 7PfqUMqtVkmL/zLCkpLng/mTeXFvJCdsgF8WzaNxixxdY2aEUicCNovoxQo/tvUgzVM zq6L78wtcrDobcmmASimWOEEQPSasYbDgXHYdVt8= Received: by smtp.zohomail.com with SMTPS id 1793036098713783.400901682976; Mon, 26 Oct 2026 10:34:58 -0700 (PDT) From: Robert Mader To: libcamera-devel@lists.libcamera.org Cc: Martin Neiva de Carvalho , Tobias Predel , Geir Isene , Hans de Goede , Milan Zamazal , Robert Mader Subject: [PATCH v2] libcamera: software_isp: Exclude saturated pixels from colour sums Date: Mon, 26 Oct 2026 18:34:33 +0100 Message-ID: <20261026173433.69339-1-robert.mader@collabora.com> X-Mailer: git-send-email 2.56.0 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Martin Neiva de Carvalho The grey world estimate takes the sums of the colour channels over the frame and assumes the scene averages to grey. Saturated pixels - both fully and partially - break that: whatever colour a pixel was, once one or more channels clip it carries wrong information about the illuminant. It is however the brightest kind of pixel there is, and the sums are weighted by brightness, so it carries a large share of the vote. Measured on a scene with a sunlit white wall filling part of the frame, 9% of the photosites were clipped and, being at full scale, accounted for roughly a third of the total sums. The estimate was dragged towards neutral and left a visible cyan cast: surfaces that should have been neutral came out with a linear R/G of 0.855 and B/G of 0.942. Leaving the saturated pixels out brings those to 0.944 and 0.986. Keep a separate count of the pixels that did contribute, because the black level the IPA subtracts from the sums is a per-pixel offset and would otherwise be scaled by the wrong number. Signed-off-by: Martin Neiva de Carvalho Tested-by: Tobias Predel Tested-by: Geir Isene Reviewed-by: Hans de Goede Reviewed-by: Milan Zamazal Tested-by: Robert Mader --- Changes in V2: - Split out from the series "softisp: Five fixes found on a camera with no hardware ISP" (https://patchwork.libcamera.org/cover/28173/) - Apply suggestions by Hans and Milan to comments and the commit message - no changes to the code - Collect tags --- .../internal/software_isp/swisp_stats.h | 8 ++++ src/ipa/softisp/algorithms/awb.cpp | 15 +++++-- src/libcamera/software_isp/swstats_cpu.cpp | 40 ++++++++++++++----- 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h index d9d0d9be84a6..aa315f6daecd 100644 --- a/include/libcamera/internal/software_isp/swisp_stats.h +++ b/include/libcamera/internal/software_isp/swisp_stats.h @@ -31,6 +31,14 @@ struct SwIspStats { * \brief Sums of colour channels of all the sampled pixels */ RGB sum_; + /** + * \brief Number of sampled pixels that contributed to sum_ + * + * Saturated pixels are left out of the sums, so this can be smaller + * than the number of pixels sampled. It is needed to subtract the black + * level from the sums, which is a per-pixel offset. + */ + uint64_t sumSamples_; /** * \brief Number of bins in the yHistogram */ diff --git a/src/ipa/softisp/algorithms/awb.cpp b/src/ipa/softisp/algorithms/awb.cpp index 55fd326fc182..b48441987a02 100644 --- a/src/ipa/softisp/algorithms/awb.cpp +++ b/src/ipa/softisp/algorithms/awb.cpp @@ -7,7 +7,6 @@ #include "awb.h" -#include #include #include @@ -118,16 +117,24 @@ SoftIspAwbStats Awb::calculateRgbMeans(IPAContext &context, if (!stats->valid) return {}; - const SwIspStats::Histogram &histogram = stats->yHistogram; const uint8_t blackLevel = context.activeState.blc.level; + /* + * Saturated pixels are not counted in the sums, so the number of + * contributing pixels comes from the statistics rather than from the + * histogram, which counts every pixel sampled. + */ + const uint64_t nPixels = stats->sumSamples_; + + /* Nothing but saturation was sampled, so there is nothing to say. */ + if (!nPixels) + return {}; + /* * Black level must be subtracted to get the correct AWB ratios, they * would be off if they were computed from the whole brightness range * rather than from the sensor range. */ - const uint64_t nPixels = std::accumulate( - histogram.begin(), histogram.end(), uint64_t(0)); const uint64_t offset = blackLevel * nPixels; const uint64_t minValid = 1; diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp index 7fb77ce7ddd6..7ae6ce4a5e93 100644 --- a/src/libcamera/software_isp/swstats_cpu.cpp +++ b/src/libcamera/software_isp/swstats_cpu.cpp @@ -167,6 +167,14 @@ SwStatsCpu::SwStatsCpu(const CameraManager &cm) << "Failed to create shared memory for statistics"; } +/* + * Pixels with any colour channel at or above this value are treated as + * saturated. Fully saturated pixels don't represent the values for white + * balance computation correctly; nearly saturated pixels may provide inaccurate + * values from hardware. + */ +static constexpr unsigned int kSaturationThreshold = 248; + static constexpr unsigned int kRedYMul = 77; /* 0.299 * 256 */ static constexpr unsigned int kGreenYMul = 150; /* 0.587 * 256 */ static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */ @@ -177,22 +185,29 @@ static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */ \ uint64_t sumR = 0; \ uint64_t sumG = 0; \ - uint64_t sumB = 0; - -#define SWSTATS_ACCUMULATE_LINE_STATS(div) \ - sumR += r; \ - sumG += g; \ - sumB += b; \ - \ - yVal = r * kRedYMul; \ - yVal += g * kGreenYMul; \ - yVal += b * kBlueYMul; \ + uint64_t sumB = 0; \ + uint64_t nSamples = 0; + +#define SWSTATS_ACCUMULATE_LINE_STATS(div) \ + if (r < kSaturationThreshold * (div) && \ + g < kSaturationThreshold * (div) && \ + b < kSaturationThreshold * (div)) { \ + sumR += r; \ + sumG += g; \ + sumB += b; \ + nSamples++; \ + } \ + \ + yVal = r * kRedYMul; \ + yVal += g * kGreenYMul; \ + yVal += b * kBlueYMul; \ stats.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++; #define SWSTATS_FINISH_LINE_STATS() \ stats.sum_.r() += sumR; \ stats.sum_.g() += sumG; \ - stats.sum_.b() += sumB; + stats.sum_.b() += sumB; \ + stats.sumSamples_ += nSamples; void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[], SwIspStats &stats) { @@ -391,6 +406,7 @@ void SwStatsCpu::startFrame(uint32_t frame) for (auto &s : stats_) { s.sum_ = RGB({ 0, 0, 0 }); + s.sumSamples_ = 0; s.yHistogram.fill(0); } } @@ -408,9 +424,11 @@ void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId) if (valid) { sharedStats_->sum_ = RGB({ 0, 0, 0 }); + sharedStats_->sumSamples_ = 0; sharedStats_->yHistogram.fill(0); for (const auto &s : stats_) { sharedStats_->sum_ += s.sum_; + sharedStats_->sumSamples_ += s.sumSamples_; for (unsigned int j = 0; j < SwIspStats::kYHistogramSize; j++) sharedStats_->yHistogram[j] += s.yHistogram[j]; }