From patchwork Fri Sep 20 18:31:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 21318 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 3A9D5C32D8 for ; Fri, 20 Sep 2024 18:32:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A897363508; Fri, 20 Sep 2024 20:32:44 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="ANcQ8XwL"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 99B2563508 for ; Fri, 20 Sep 2024 20:32:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1726857159; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JxLo7gAi0YmWlFDKVXDpWbDVFdzVlZ/bQfjl65ioC+U=; b=ANcQ8XwLRQowC7T4uODmsROm58fOm8haozrDXAYLRqi8k08lVnHwU6FMfHsQKkvskjAa6l pXDqfG6HirmOl4t9OXzoHyTd2tmhyvW8XVoWe95MZ3Z7t70f/zEm+mnnoikzrWL/hzs8WD 0Grzh6ixGN7cVexQHoP1LAIfegdgnzo= Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-518-6KBS8OyzMHif1C4dKpXZeQ-1; Fri, 20 Sep 2024 14:32:36 -0400 X-MC-Unique: 6KBS8OyzMHif1C4dKpXZeQ-1 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (unknown [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 5C0F019792E3; Fri, 20 Sep 2024 18:32:35 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.224.12]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 694381955DC6; Fri, 20 Sep 2024 18:32:33 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , Umang Jain , Laurent Pinchart , Daniel Scally , Kieran Bingham Subject: [PATCH v7 13/18] libcamera: software_isp: Move black level to an algorithm module Date: Fri, 20 Sep 2024 20:31:38 +0200 Message-ID: <20240920183143.1674117-14-mzamazal@redhat.com> In-Reply-To: <20240920183143.1674117-1-mzamazal@redhat.com> References: <20240920183143.1674117-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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" The black level determination, already present as a separate class, can be moved to the prepared Algorithm processing structure. It is the first of the current software ISP algorithms that is called in the stats processing sequence, which means it is also the first one that should be moved to the new structure. Stats processing starts with calling Algorithm-based processing so the call order of the algorithms is retained. Movement of this algorithm is relatively straightforward because all it does is processing stats. The comment about getting black level from the tuning files is dropped. The black level will be taken from CameraSensorHelper if available and that will be implemented in one of the followup patches. Black level is now recomputed on each stats processing. In a future patch, after DelayedControls are used, this will be changed to recompute the black level only after exposure/gain changes. The black level depends on the sensor used, the computed value can be reused in a followup capture sessions with the same sensor. Thus it is sufficient to (re)set the initial value in BlackLevel::init. Signed-off-by: Milan Zamazal Reviewed-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Daniel Scally --- src/ipa/simple/algorithms/blc.cpp | 71 ++++++++++++++++++++ src/ipa/simple/algorithms/blc.h | 31 +++++++++ src/ipa/simple/algorithms/meson.build | 1 + src/ipa/simple/black_level.cpp | 93 --------------------------- src/ipa/simple/black_level.h | 33 ---------- src/ipa/simple/data/uncalibrated.yaml | 1 + src/ipa/simple/ipa_context.cpp | 8 +++ src/ipa/simple/ipa_context.h | 5 ++ src/ipa/simple/meson.build | 1 - src/ipa/simple/soft_simple.cpp | 8 +-- 10 files changed, 119 insertions(+), 133 deletions(-) create mode 100644 src/ipa/simple/algorithms/blc.cpp create mode 100644 src/ipa/simple/algorithms/blc.h delete mode 100644 src/ipa/simple/black_level.cpp delete mode 100644 src/ipa/simple/black_level.h diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp new file mode 100644 index 00000000..755108b0 --- /dev/null +++ b/src/ipa/simple/algorithms/blc.cpp @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Red Hat Inc. + * + * Black level handling + */ + +#include "blc.h" + +#include + +#include + +namespace libcamera { + +namespace ipa::soft::algorithms { + +LOG_DEFINE_CATEGORY(IPASoftBL) + +BlackLevel::BlackLevel() +{ +} + +int BlackLevel::configure(IPAContext &context, + [[maybe_unused]] const IPAConfigInfo &configInfo) +{ + context.activeState.blc.level = 255; + return 0; +} + +void BlackLevel::process(IPAContext &context, + [[maybe_unused]] const uint32_t frame, + [[maybe_unused]] IPAFrameContext &frameContext, + const SwIspStats *stats, + [[maybe_unused]] ControlList &metadata) +{ + const SwIspStats::Histogram &histogram = stats->yHistogram; + + /* + * The constant is selected to be "good enough", not overly + * conservative or aggressive. There is no magic about the given value. + */ + constexpr float ignoredPercentage = 0.02; + const unsigned int total = + std::accumulate(begin(histogram), end(histogram), 0); + const unsigned int pixelThreshold = ignoredPercentage * total; + const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; + const unsigned int currentBlackIdx = + context.activeState.blc.level / histogramRatio; + + for (unsigned int i = 0, seen = 0; + i < currentBlackIdx && i < SwIspStats::kYHistogramSize; + i++) { + seen += histogram[i]; + if (seen >= pixelThreshold) { + context.activeState.blc.level = i * histogramRatio; + LOG(IPASoftBL, Debug) + << "Auto-set black level: " + << i << "/" << SwIspStats::kYHistogramSize + << " (" << 100 * (seen - histogram[i]) / total << "% below, " + << 100 * seen / total << "% at or below)"; + break; + } + }; +} + +REGISTER_IPA_ALGORITHM(BlackLevel, "BlackLevel") + +} /* namespace ipa::soft::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h new file mode 100644 index 00000000..49bdcc61 --- /dev/null +++ b/src/ipa/simple/algorithms/blc.h @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Red Hat Inc. + * + * Black level handling + */ + +#pragma once + +#include "algorithm.h" + +namespace libcamera { + +namespace ipa::soft::algorithms { + +class BlackLevel : public Algorithm +{ +public: + BlackLevel(); + ~BlackLevel() = default; + + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; + void process(IPAContext &context, const uint32_t frame, + IPAFrameContext &frameContext, + const SwIspStats *stats, + ControlList &metadata) override; +}; + +} /* namespace ipa::soft::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build index 31d26e43..1f63c220 100644 --- a/src/ipa/simple/algorithms/meson.build +++ b/src/ipa/simple/algorithms/meson.build @@ -1,4 +1,5 @@ # SPDX-License-Identifier: CC0-1.0 soft_simple_ipa_algorithms = files([ + 'blc.cpp', ]) diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp deleted file mode 100644 index 37e0109c..00000000 --- a/src/ipa/simple/black_level.cpp +++ /dev/null @@ -1,93 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2024, Red Hat Inc. - * - * black level handling - */ - -#include "black_level.h" - -#include - -#include - -namespace libcamera { - -LOG_DEFINE_CATEGORY(IPASoftBL) - -namespace ipa::soft { - -/** - * \class BlackLevel - * \brief Object providing black point level for software ISP - * - * Black level can be provided in hardware tuning files or, if no tuning file is - * available for the given hardware, guessed automatically, with less accuracy. - * As tuning files are not yet implemented for software ISP, BlackLevel - * currently provides only guessed black levels. - * - * This class serves for tracking black level as a property of the underlying - * hardware, not as means of enhancing a particular scene or image. - * - * The class is supposed to be instantiated for the given camera stream. - * The black level can be retrieved using BlackLevel::get() method. It is - * initially 0 and may change when updated using BlackLevel::update() method. - */ - -BlackLevel::BlackLevel() - : blackLevel_(255), blackLevelSet_(false) -{ -} - -/** - * \brief Return the current black level - * - * \return The black level, in the range from 0 (minimum) to 255 (maximum). - * If the black level couldn't be determined yet, return 0. - */ -uint8_t BlackLevel::get() const -{ - return blackLevelSet_ ? blackLevel_ : 0; -} - -/** - * \brief Update black level from the provided histogram - * \param[in] yHistogram The histogram to be used for updating black level - * - * The black level is property of the given hardware, not image. It is updated - * only if it has not been yet set or if it is lower than the lowest value seen - * so far. - */ -void BlackLevel::update(SwIspStats::Histogram &yHistogram) -{ - /* - * The constant is selected to be "good enough", not overly conservative or - * aggressive. There is no magic about the given value. - */ - constexpr float ignoredPercentage_ = 0.02; - const unsigned int total = - std::accumulate(begin(yHistogram), end(yHistogram), 0); - const unsigned int pixelThreshold = ignoredPercentage_ * total; - const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; - const unsigned int currentBlackIdx = blackLevel_ / histogramRatio; - - for (unsigned int i = 0, seen = 0; - i < currentBlackIdx && i < SwIspStats::kYHistogramSize; - i++) { - seen += yHistogram[i]; - if (seen >= pixelThreshold) { - blackLevel_ = i * histogramRatio; - blackLevelSet_ = true; - LOG(IPASoftBL, Debug) - << "Auto-set black level: " - << i << "/" << SwIspStats::kYHistogramSize - << " (" << 100 * (seen - yHistogram[i]) / total << "% below, " - << 100 * seen / total << "% at or below)"; - break; - } - }; -} - -} /* namespace ipa::soft */ - -} /* namespace libcamera */ diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h deleted file mode 100644 index a04230c9..00000000 --- a/src/ipa/simple/black_level.h +++ /dev/null @@ -1,33 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2024, Red Hat Inc. - * - * black level handling - */ - -#pragma once - -#include -#include - -#include "libcamera/internal/software_isp/swisp_stats.h" - -namespace libcamera { - -namespace ipa::soft { - -class BlackLevel -{ -public: - BlackLevel(); - uint8_t get() const; - void update(SwIspStats::Histogram &yHistogram); - -private: - uint8_t blackLevel_; - bool blackLevelSet_; -}; - -} /* namespace ipa::soft */ - -} /* namespace libcamera */ diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml index 2cdc39a8..e0d03d96 100644 --- a/src/ipa/simple/data/uncalibrated.yaml +++ b/src/ipa/simple/data/uncalibrated.yaml @@ -3,4 +3,5 @@ --- version: 1 algorithms: + - BlackLevel: ... diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp index 3c1c7262..268cff32 100644 --- a/src/ipa/simple/ipa_context.cpp +++ b/src/ipa/simple/ipa_context.cpp @@ -50,4 +50,12 @@ namespace libcamera::ipa::soft { * \brief The current state of IPA algorithms */ +/** + * \var IPAActiveState::black + * \brief Context for the Black Level algorithm + * + * \var IPAActiveState::black.level + * \brief Current determined black level + */ + } /* namespace libcamera::ipa::soft */ diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index e7d8b8df..ac2a59d7 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -7,6 +7,8 @@ #pragma once +#include + #include namespace libcamera { @@ -17,6 +19,9 @@ struct IPASessionConfiguration { }; struct IPAActiveState { + struct { + uint8_t level; + } blc; }; struct IPAFrameContext : public FrameContext { diff --git a/src/ipa/simple/meson.build b/src/ipa/simple/meson.build index dcd7c70a..2f9f15f4 100644 --- a/src/ipa/simple/meson.build +++ b/src/ipa/simple/meson.build @@ -8,7 +8,6 @@ ipa_name = 'ipa_soft_simple' soft_simple_sources = files([ 'ipa_context.cpp', 'soft_simple.cpp', - 'black_level.cpp', ]) soft_simple_sources += soft_simple_ipa_algorithms diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index 5beec45a..3332e2b1 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -29,7 +29,6 @@ #include "libipa/camera_sensor_helper.h" -#include "black_level.h" #include "module.h" namespace libcamera { @@ -61,7 +60,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module { public: IPASoftSimple() - : params_(nullptr), stats_(nullptr), blackLevel_(BlackLevel()), + : params_(nullptr), stats_(nullptr), context_({ {}, {}, { kMaxFrameContexts } }), ignoreUpdates_(0) { @@ -93,7 +92,6 @@ private: SwIspStats *stats_; std::unique_ptr camHelper_; ControlInfoMap sensorInfoMap_; - BlackLevel blackLevel_; static constexpr unsigned int kGammaLookupSize = 1024; std::array gammaTable_; @@ -303,9 +301,7 @@ void IPASoftSimple::processStats(const uint32_t frame, algo->process(context_, frame, frameContext, stats_, metadata); SwIspStats::Histogram histogram = stats_->yHistogram; - if (ignoreUpdates_ > 0) - blackLevel_.update(histogram); - const uint8_t blackLevel = blackLevel_.get(); + const uint8_t blackLevel = context_.activeState.blc.level; /* * Black level must be subtracted to get the correct AWB ratios, they