From patchwork Wed Oct 28 23:30:36 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 28661 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 65117C324C for ; Wed, 28 Oct 2026 23:30:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0A3A26908C; Thu, 29 Oct 2026 00:30:40 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="lHwEkkNZ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BE8C769056 for ; Thu, 29 Oct 2026 00:30:37 +0100 (CET) Received: from killaraus.ideasonboard.com (2001-14ba-70f3-e800--a06.rev.dnainternet.fi [IPv6:2001:14ba:70f3:e800::a06]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7C2576DC; Thu, 29 Oct 2026 00:28:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1793230104; bh=RNEUNLiy8bKcVfZJnHaWZtfW+AM7LIwEba5NKRB1TJo=; h=From:To:Cc:Subject:Date:From; b=lHwEkkNZWxYWwSn7FP+ucksT4NcbMl2UaqoFOa7qulBNl53skxVKbycfqbvnfJoQv ApofMz8dWznOl+FOlc0EVZsWDfMVYvBNuL1v8zGdMkytg50lPPkFotMrYGdgrRu0aj L0byBmXH+2BSttQbtoU4YNE330PTTmb/0CZQ7iJM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: Geir Isene , Hans de Goede , Martin Neiva de Carvalho , Milan Zamazal , Robert Mader , Tobias Predel Subject: [PATCH v3] libcamera: software_isp: Exclude saturated pixels from AWB statistics Date: Thu, 29 Oct 2026 01:30:36 +0200 Message-ID: <20261028233036.2747196-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.55.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" AWB statistics in the software ISP accumulate the value of all pixels to calculate per-channel means. Saturated pixels skew those means, as clipping gives them a different colour (unless they have a neutral colour value and are fully saturated on all colour channels). A large number of saturated pixels can have a significant impact on the AWB operation and incorrectly shift the hue of the image. Fix this by accumulating unsaturated pixels only. The saturation level is hardcoded to 95% of the maximum value to leave some margin for noise and avoid the close-to-saturation non-linearity that can also skew the calculated means. It may be possible to improve the implementation by calculating the saturation threshold dynamically based on sensor characterization and noise level. Reported-by: Geir Isene Reported-by: Martin Neiva de Carvalho Reported-by: Tobias Predel Reviewed-by: Hans de Goede Reviewed-by: Milan Zamazal Tested-by: Robert Mader Signed-off-by: Laurent Pinchart --- Changes since v2: - Rewrite commit message and comments - Set saturation threshold to 95% - Rename sumSamples_ to numPixels_ - Strip authorship from LLM-generated patch --- .../internal/software_isp/swisp_stats.h | 23 ++++++++--- src/ipa/softisp/algorithms/awb.cpp | 14 +++++-- src/libcamera/software_isp/swstats_cpu.cpp | 38 ++++++++++++++----- 3 files changed, 56 insertions(+), 19 deletions(-) base-commit: e117198c015057761bb0bbaa85f963c41ce412d7 diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h index d9d0d9be84a6..65d826f6cff3 100644 --- a/include/libcamera/internal/software_isp/swisp_stats.h +++ b/include/libcamera/internal/software_isp/swisp_stats.h @@ -15,11 +15,20 @@ namespace libcamera { /** - * \brief Struct that holds the statistics for the Software ISP + * \brief Statistics for the Software ISP * - * The struct value types are large enough to not overflow. - * Should they still overflow for some reason, no check is performed and they - * wrap around. + * The software ISP computes two types of statistics: + * + * - The accumulator counts the number of non-saturated pixels and sums their + * R, G and B channels individually. Pixels that have any saturated channel + * are skipped as they don't provide accurate colour information due to + * clipping. + * + * - The histogram groups pixels in bins based on their luminance and counts + * the number of pixels in each bin. Saturated pixels are included. + * + * The value types are large enough to not overflow. Should they still overflow + * for some reason, no check is performed and they wrap around. */ struct SwIspStats { /** @@ -28,9 +37,13 @@ struct SwIspStats { */ bool valid; /** - * \brief Sums of colour channels of all the sampled pixels + * \brief Sums of colour channels of all the unsaturated pixels */ RGB sum_; + /** + * \brief Number of unsaturated pixels accumulated in \a sum_ + */ + uint64_t numPixels_; /** * \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..fb9bcb06a75e 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,23 @@ SoftIspAwbStats Awb::calculateRgbMeans(IPAContext &context, if (!stats->valid) return {}; - const SwIspStats::Histogram &histogram = stats->yHistogram; const uint8_t blackLevel = context.activeState.blc.level; + const uint64_t nPixels = stats->numPixels_; + + /* + * In the unlikely case where all pixels are saturated, return invalid + * statistics. This shouldn't happen in normal conditions but can be + * triggered by severe over-exposure. + */ + 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..74633815ed0a 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"; } +/* + * Hardcode the AWB stats saturation threshold to 95% of the maximum value. + * + * \todo Investigate if this should be made dynamic, based on sensor + * (non-)linearity and noise level. + */ +static constexpr unsigned int kSaturationThreshold = 0.95 * 256; + 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; + uint64_t sumB = 0; \ + uint64_t nSamples = 0; -#define SWSTATS_ACCUMULATE_LINE_STATS(div) \ - sumR += r; \ - sumG += g; \ - sumB += b; \ - \ - yVal = r * kRedYMul; \ - yVal += g * kGreenYMul; \ - yVal += b * kBlueYMul; \ +#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.numPixels_ += 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.numPixels_ = 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_->numPixels_ = 0; sharedStats_->yHistogram.fill(0); for (const auto &s : stats_) { sharedStats_->sum_ += s.sum_; + sharedStats_->numPixels_ += s.numPixels_; for (unsigned int j = 0; j < SwIspStats::kYHistogramSize; j++) sharedStats_->yHistogram[j] += s.yHistogram[j]; }