{"id":21406,"url":"https://patchwork.libcamera.org/api/1.1/patches/21406/?format=json","web_url":"https://patchwork.libcamera.org/patch/21406/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240927134624.801004-14-mzamazal@redhat.com>","date":"2024-09-27T13:46:19","name":"[v8,13/18] libcamera: software_isp: Move black level to an algorithm module","commit_ref":"079d5f30ef324952c4c32876576cfbdcec0843b9","pull_url":null,"state":"accepted","archived":false,"hash":"b1b5dfa2c7ac39faa64ef7d7ba47720e6ddffbc3","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/1.1/people/177/?format=json","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/21406/mbox/","series":[{"id":4634,"url":"https://patchwork.libcamera.org/api/1.1/series/4634/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4634","date":"2024-09-27T13:46:06","name":"Software ISP refactoring","version":8,"mbox":"https://patchwork.libcamera.org/series/4634/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/21406/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/21406/checks/","tags":{},"headers":{"Return-Path":"","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 178EFC3257\n\tfor ;\n\tFri, 27 Sep 2024 13:47:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 50ED86352F;\n\tFri, 27 Sep 2024 15:47:15 +0200 (CEST)","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 1B52D63529\n\tfor ;\n\tFri, 27 Sep 2024 15:47:09 +0200 (CEST)","from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com\n\t(ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-694-dl-xkfQOMOae31EL7PfxCw-1;\n\tFri, 27 Sep 2024 09:47:04 -0400","from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (unknown\n\t[10.30.177.40])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id B80D9190346A; Fri, 27 Sep 2024 13:47:03 +0000 (UTC)","from nuthatch.redhat.com (unknown [10.45.224.123])\n\tby mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id 8FF621944E45; Fri, 27 Sep 2024 13:47:01 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"IdHPdNxB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1727444828;\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\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=1PX+KFQ/rp854fhE3GHw7+nIPJOOw7OaSUGf0PaRY+A=;\n\tb=IdHPdNxBFgwk+3DFDtzKcSHSaDfLpqNRKKwYTqsggh1kzXCplVJZe0Al2vieQiRu3EYcKB\n\t/Z0Tk0xqL1KydL2D1EFZUvAT8l3pPTuQ/KskeGjXVudquf2NFfoBkf3A6hbr+0svvHc8tE\n\tELVg0U7SJ7YGbfwwMlj4ozzOmifp/B0=","X-MC-Unique":"dl-xkfQOMOae31EL7PfxCw-1","From":"Milan Zamazal ","To":"libcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ,\n\tUmang Jain ,\n\tLaurent Pinchart ,\n\tDaniel Scally ,\n\tKieran Bingham ","Subject":"[PATCH v8 13/18] libcamera: software_isp: Move black level to an\n\talgorithm module","Date":"Fri, 27 Sep 2024 15:46:19 +0200","Message-ID":"<20240927134624.801004-14-mzamazal@redhat.com>","In-Reply-To":"<20240927134624.801004-1-mzamazal@redhat.com>","References":"<20240927134624.801004-1-mzamazal@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.0 on 10.30.177.40","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Transfer-Encoding":"8bit","Content-Type":"text/plain; charset=\"US-ASCII\"; x-default=true","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The black level determination, already present as a separate class, can\nbe moved to the prepared Algorithm processing structure. It is the\nfirst of the current software ISP algorithms that is called in the stats\nprocessing sequence, which means it is also the first one that should be\nmoved to the new structure. Stats processing starts with calling\nAlgorithm-based processing so the call order of the algorithms is\nretained.\n\nMovement of this algorithm is relatively straightforward because all it\ndoes is processing stats.\n\nThe comment about getting black level from the tuning files is dropped.\nThe black level will be taken from CameraSensorHelper if available and\nthat will be implemented in one of the followup patches.\n\nBlack level is now recomputed on each stats processing. In a future\npatch, after DelayedControls are used, this will be changed to recompute\nthe black level only after exposure/gain changes.\n\nThe black level depends on the sensor used, the computed value can be\nreused in a followup capture sessions with the same sensor. Thus it is\nsufficient to (re)set the initial value in BlackLevel::init.\n\nSigned-off-by: Milan Zamazal \nReviewed-by: Umang Jain \nReviewed-by: Laurent Pinchart \nReviewed-by: Daniel Scally \n---\n src/ipa/simple/algorithms/blc.cpp | 71 ++++++++++++++++++++\n src/ipa/simple/algorithms/blc.h | 31 +++++++++\n src/ipa/simple/algorithms/meson.build | 1 +\n src/ipa/simple/black_level.cpp | 93 ---------------------------\n src/ipa/simple/black_level.h | 33 ----------\n src/ipa/simple/data/uncalibrated.yaml | 1 +\n src/ipa/simple/ipa_context.cpp | 8 +++\n src/ipa/simple/ipa_context.h | 5 ++\n src/ipa/simple/meson.build | 1 -\n src/ipa/simple/soft_simple.cpp | 8 +--\n 10 files changed, 119 insertions(+), 133 deletions(-)\n create mode 100644 src/ipa/simple/algorithms/blc.cpp\n create mode 100644 src/ipa/simple/algorithms/blc.h\n delete mode 100644 src/ipa/simple/black_level.cpp\n delete mode 100644 src/ipa/simple/black_level.h","diff":"diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\nnew file mode 100644\nindex 000000000..755108b05\n--- /dev/null\n+++ b/src/ipa/simple/algorithms/blc.cpp\n@@ -0,0 +1,71 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2024, Red Hat Inc.\n+ *\n+ * Black level handling\n+ */\n+\n+#include \"blc.h\"\n+\n+#include \n+\n+#include \n+\n+namespace libcamera {\n+\n+namespace ipa::soft::algorithms {\n+\n+LOG_DEFINE_CATEGORY(IPASoftBL)\n+\n+BlackLevel::BlackLevel()\n+{\n+}\n+\n+int BlackLevel::configure(IPAContext &context,\n+\t\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n+{\n+\tcontext.activeState.blc.level = 255;\n+\treturn 0;\n+}\n+\n+void BlackLevel::process(IPAContext &context,\n+\t\t\t [[maybe_unused]] const uint32_t frame,\n+\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n+\t\t\t const SwIspStats *stats,\n+\t\t\t [[maybe_unused]] ControlList &metadata)\n+{\n+\tconst SwIspStats::Histogram &histogram = stats->yHistogram;\n+\n+\t/*\n+\t * The constant is selected to be \"good enough\", not overly\n+\t * conservative or aggressive. There is no magic about the given value.\n+\t */\n+\tconstexpr float ignoredPercentage = 0.02;\n+\tconst unsigned int total =\n+\t\tstd::accumulate(begin(histogram), end(histogram), 0);\n+\tconst unsigned int pixelThreshold = ignoredPercentage * total;\n+\tconst unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize;\n+\tconst unsigned int currentBlackIdx =\n+\t\tcontext.activeState.blc.level / histogramRatio;\n+\n+\tfor (unsigned int i = 0, seen = 0;\n+\t i < currentBlackIdx && i < SwIspStats::kYHistogramSize;\n+\t i++) {\n+\t\tseen += histogram[i];\n+\t\tif (seen >= pixelThreshold) {\n+\t\t\tcontext.activeState.blc.level = i * histogramRatio;\n+\t\t\tLOG(IPASoftBL, Debug)\n+\t\t\t\t<< \"Auto-set black level: \"\n+\t\t\t\t<< i << \"/\" << SwIspStats::kYHistogramSize\n+\t\t\t\t<< \" (\" << 100 * (seen - histogram[i]) / total << \"% below, \"\n+\t\t\t\t<< 100 * seen / total << \"% at or below)\";\n+\t\t\tbreak;\n+\t\t}\n+\t};\n+}\n+\n+REGISTER_IPA_ALGORITHM(BlackLevel, \"BlackLevel\")\n+\n+} /* namespace ipa::soft::algorithms */\n+\n+} /* namespace libcamera */\ndiff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\nnew file mode 100644\nindex 000000000..49bdcc610\n--- /dev/null\n+++ b/src/ipa/simple/algorithms/blc.h\n@@ -0,0 +1,31 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2024, Red Hat Inc.\n+ *\n+ * Black level handling\n+ */\n+\n+#pragma once\n+\n+#include \"algorithm.h\"\n+\n+namespace libcamera {\n+\n+namespace ipa::soft::algorithms {\n+\n+class BlackLevel : public Algorithm\n+{\n+public:\n+\tBlackLevel();\n+\t~BlackLevel() = default;\n+\n+\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n+\tvoid process(IPAContext &context, const uint32_t frame,\n+\t\t IPAFrameContext &frameContext,\n+\t\t const SwIspStats *stats,\n+\t\t ControlList &metadata) override;\n+};\n+\n+} /* namespace ipa::soft::algorithms */\n+\n+} /* namespace libcamera */\ndiff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\nindex 31d26e434..1f63c2204 100644\n--- a/src/ipa/simple/algorithms/meson.build\n+++ b/src/ipa/simple/algorithms/meson.build\n@@ -1,4 +1,5 @@\n # SPDX-License-Identifier: CC0-1.0\n \n soft_simple_ipa_algorithms = files([\n+ 'blc.cpp',\n ])\ndiff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp\ndeleted file mode 100644\nindex 37e0109ce..000000000\n--- a/src/ipa/simple/black_level.cpp\n+++ /dev/null\n@@ -1,93 +0,0 @@\n-/* SPDX-License-Identifier: LGPL-2.1-or-later */\n-/*\n- * Copyright (C) 2024, Red Hat Inc.\n- *\n- * black level handling\n- */\n-\n-#include \"black_level.h\"\n-\n-#include \n-\n-#include \n-\n-namespace libcamera {\n-\n-LOG_DEFINE_CATEGORY(IPASoftBL)\n-\n-namespace ipa::soft {\n-\n-/**\n- * \\class BlackLevel\n- * \\brief Object providing black point level for software ISP\n- *\n- * Black level can be provided in hardware tuning files or, if no tuning file is\n- * available for the given hardware, guessed automatically, with less accuracy.\n- * As tuning files are not yet implemented for software ISP, BlackLevel\n- * currently provides only guessed black levels.\n- *\n- * This class serves for tracking black level as a property of the underlying\n- * hardware, not as means of enhancing a particular scene or image.\n- *\n- * The class is supposed to be instantiated for the given camera stream.\n- * The black level can be retrieved using BlackLevel::get() method. It is\n- * initially 0 and may change when updated using BlackLevel::update() method.\n- */\n-\n-BlackLevel::BlackLevel()\n-\t: blackLevel_(255), blackLevelSet_(false)\n-{\n-}\n-\n-/**\n- * \\brief Return the current black level\n- *\n- * \\return The black level, in the range from 0 (minimum) to 255 (maximum).\n- * If the black level couldn't be determined yet, return 0.\n- */\n-uint8_t BlackLevel::get() const\n-{\n-\treturn blackLevelSet_ ? blackLevel_ : 0;\n-}\n-\n-/**\n- * \\brief Update black level from the provided histogram\n- * \\param[in] yHistogram The histogram to be used for updating black level\n- *\n- * The black level is property of the given hardware, not image. It is updated\n- * only if it has not been yet set or if it is lower than the lowest value seen\n- * so far.\n- */\n-void BlackLevel::update(SwIspStats::Histogram &yHistogram)\n-{\n-\t/*\n-\t * The constant is selected to be \"good enough\", not overly conservative or\n-\t * aggressive. There is no magic about the given value.\n-\t */\n-\tconstexpr float ignoredPercentage_ = 0.02;\n-\tconst unsigned int total =\n-\t\tstd::accumulate(begin(yHistogram), end(yHistogram), 0);\n-\tconst unsigned int pixelThreshold = ignoredPercentage_ * total;\n-\tconst unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize;\n-\tconst unsigned int currentBlackIdx = blackLevel_ / histogramRatio;\n-\n-\tfor (unsigned int i = 0, seen = 0;\n-\t i < currentBlackIdx && i < SwIspStats::kYHistogramSize;\n-\t i++) {\n-\t\tseen += yHistogram[i];\n-\t\tif (seen >= pixelThreshold) {\n-\t\t\tblackLevel_ = i * histogramRatio;\n-\t\t\tblackLevelSet_ = true;\n-\t\t\tLOG(IPASoftBL, Debug)\n-\t\t\t\t<< \"Auto-set black level: \"\n-\t\t\t\t<< i << \"/\" << SwIspStats::kYHistogramSize\n-\t\t\t\t<< \" (\" << 100 * (seen - yHistogram[i]) / total << \"% below, \"\n-\t\t\t\t<< 100 * seen / total << \"% at or below)\";\n-\t\t\tbreak;\n-\t\t}\n-\t};\n-}\n-\n-} /* namespace ipa::soft */\n-\n-} /* namespace libcamera */\ndiff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h\ndeleted file mode 100644\nindex a04230c9c..000000000\n--- a/src/ipa/simple/black_level.h\n+++ /dev/null\n@@ -1,33 +0,0 @@\n-/* SPDX-License-Identifier: LGPL-2.1-or-later */\n-/*\n- * Copyright (C) 2024, Red Hat Inc.\n- *\n- * black level handling\n- */\n-\n-#pragma once\n-\n-#include \n-#include \n-\n-#include \"libcamera/internal/software_isp/swisp_stats.h\"\n-\n-namespace libcamera {\n-\n-namespace ipa::soft {\n-\n-class BlackLevel\n-{\n-public:\n-\tBlackLevel();\n-\tuint8_t get() const;\n-\tvoid update(SwIspStats::Histogram &yHistogram);\n-\n-private:\n-\tuint8_t blackLevel_;\n-\tbool blackLevelSet_;\n-};\n-\n-} /* namespace ipa::soft */\n-\n-} /* namespace libcamera */\ndiff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\nindex 2cdc39a84..e0d03d962 100644\n--- a/src/ipa/simple/data/uncalibrated.yaml\n+++ b/src/ipa/simple/data/uncalibrated.yaml\n@@ -3,4 +3,5 @@\n ---\n version: 1\n algorithms:\n+ - BlackLevel:\n ...\ndiff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp\nindex 3c1c72624..268cff329 100644\n--- a/src/ipa/simple/ipa_context.cpp\n+++ b/src/ipa/simple/ipa_context.cpp\n@@ -50,4 +50,12 @@ namespace libcamera::ipa::soft {\n * \\brief The current state of IPA algorithms\n */\n \n+/**\n+ * \\var IPAActiveState::black\n+ * \\brief Context for the Black Level algorithm\n+ *\n+ * \\var IPAActiveState::black.level\n+ * \\brief Current determined black level\n+ */\n+\n } /* namespace libcamera::ipa::soft */\ndiff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\nindex e7d8b8dfd..ac2a59d7c 100644\n--- a/src/ipa/simple/ipa_context.h\n+++ b/src/ipa/simple/ipa_context.h\n@@ -7,6 +7,8 @@\n \n #pragma once\n \n+#include \n+\n #include \n \n namespace libcamera {\n@@ -17,6 +19,9 @@ struct IPASessionConfiguration {\n };\n \n struct IPAActiveState {\n+\tstruct {\n+\t\tuint8_t level;\n+\t} blc;\n };\n \n struct IPAFrameContext : public FrameContext {\ndiff --git a/src/ipa/simple/meson.build b/src/ipa/simple/meson.build\nindex dcd7c70a8..2f9f15f4a 100644\n--- a/src/ipa/simple/meson.build\n+++ b/src/ipa/simple/meson.build\n@@ -8,7 +8,6 @@ ipa_name = 'ipa_soft_simple'\n soft_simple_sources = files([\n 'ipa_context.cpp',\n 'soft_simple.cpp',\n- 'black_level.cpp',\n ])\n \n soft_simple_sources += soft_simple_ipa_algorithms\ndiff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex 5beec45a1..3332e2b10 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -29,7 +29,6 @@\n \n #include \"libipa/camera_sensor_helper.h\"\n \n-#include \"black_level.h\"\n #include \"module.h\"\n \n namespace libcamera {\n@@ -61,7 +60,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module\n {\n public:\n \tIPASoftSimple()\n-\t\t: params_(nullptr), stats_(nullptr), blackLevel_(BlackLevel()),\n+\t\t: params_(nullptr), stats_(nullptr),\n \t\t context_({ {}, {}, { kMaxFrameContexts } }),\n \t\t ignoreUpdates_(0)\n \t{\n@@ -93,7 +92,6 @@ private:\n \tSwIspStats *stats_;\n \tstd::unique_ptr camHelper_;\n \tControlInfoMap sensorInfoMap_;\n-\tBlackLevel blackLevel_;\n \n \tstatic constexpr unsigned int kGammaLookupSize = 1024;\n \tstd::array gammaTable_;\n@@ -303,9 +301,7 @@ void IPASoftSimple::processStats(const uint32_t frame,\n \t\talgo->process(context_, frame, frameContext, stats_, metadata);\n \n \tSwIspStats::Histogram histogram = stats_->yHistogram;\n-\tif (ignoreUpdates_ > 0)\n-\t\tblackLevel_.update(histogram);\n-\tconst uint8_t blackLevel = blackLevel_.get();\n+\tconst uint8_t blackLevel = context_.activeState.blc.level;\n \n \t/*\n \t * Black level must be subtracted to get the correct AWB ratios, they\n","prefixes":["v8","13/18"]}