From patchwork Wed Nov 27 14:46:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Isaac Scott X-Patchwork-Id: 22125 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 12759C3200 for ; Wed, 27 Nov 2024 14:47:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 909F9660E5; Wed, 27 Nov 2024 15:47:08 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="EnYZwTcX"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 71AAB660C6 for ; Wed, 27 Nov 2024 15:47:04 +0100 (CET) Received: from isaac-ThinkPad-T16-Gen-2.lan (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 47AF81230; Wed, 27 Nov 2024 15:46:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1732718801; bh=qmLKoFdXCOTxCkS4DjH2PXsgg53UpRfNC7pxpZIgm4g=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EnYZwTcX6W0jHpRTm2vpr3wCm9eP35Y975n6y1srVeAt77VEobFGNqbEdIU+zZnTU Ngzv7/llyEB3+3FU1PIZM53HOZfPXRQ4kLZyw+ZDA+tsqeFE4P6EN4++2dkBQcf98+ uGU+xZPxle2ifTioyuMq4mGoevztLLaHBeew96lk= From: Isaac Scott To: libcamera devel Cc: Isaac Scott Subject: [RFC PATCH 1/2] libcamera: bitdepth: Add BitDepth implementation Date: Wed, 27 Nov 2024 14:46:54 +0000 Message-ID: <20241127144655.1074720-2-isaac.scott@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20241127144655.1074720-1-isaac.scott@ideasonboard.com> References: <20241127144655.1074720-1-isaac.scott@ideasonboard.com> 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" Add a class template that simplifies bit depth conversion. This functionality allows users to avoid having to perform inline bitshifting to convert values of one bit depth to another. For example, this makes it easier to input values from datasheets, where a value may be expressed in a certain bit depth. It also removes the potential for human errors when making these conversions manually, and in a lot of cases makes bit depth conversions more readable. Signed-off-by: Isaac Scott --- src/ipa/libipa/bitdepth.h | 86 ++++++++++++++++++++++++++++ test/ipa/libipa/bitdepth.cpp | 107 +++++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 src/ipa/libipa/bitdepth.h create mode 100644 test/ipa/libipa/bitdepth.cpp diff --git a/src/ipa/libipa/bitdepth.h b/src/ipa/libipa/bitdepth.h new file mode 100644 index 00000000..145ee093 --- /dev/null +++ b/src/ipa/libipa/bitdepth.h @@ -0,0 +1,86 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024 Ideas On Board Oy. + * + * BitDepth class to abstract bit shift operations + */ + +#pragma once + +template +class BitDepthValue +{ +public: + static_assert(BitDepth > 0, "Bit depth must be positive"); + + BitDepthValue() + : value_(0) {} + + BitDepthValue(int value) + : value_(value) {} + + BitDepthValue(unsigned int value) + : value_(value) {} + + template + BitDepthValue convert() const + { + static_assert(TargetBitDepth > 0, "Bit depth must be positive"); + + unsigned int shift; + + if constexpr (BitDepth > TargetBitDepth) { + shift = BitDepth - TargetBitDepth; + return BitDepthValue(value_ >> shift); + } else if constexpr (BitDepth < TargetBitDepth) { + shift = TargetBitDepth - BitDepth; + return BitDepthValue(value_ << shift); + } else { + return BitDepthValue(value_); + } + } + + unsigned int value() const + { + return value_; + } + + template + operator BitDepthValue() const + { + return convert(); + } + + operator unsigned int() const + { + return value_; + } + +private: + unsigned int value_; +}; + +inline BitDepthValue<8> operator"" _8bit(unsigned long long value) +{ + return BitDepthValue<8>(static_cast(value)); +} + +inline BitDepthValue<10> operator"" _10bit(unsigned long long value) +{ + return BitDepthValue<10>(static_cast(value)); +} + +inline BitDepthValue<12> operator"" _12bit(unsigned long long value) +{ + return BitDepthValue<12>(static_cast(value)); +} + +inline BitDepthValue<14> operator"" _14bit(unsigned long long value) +{ + return BitDepthValue<14>(static_cast(value)); +} + +inline BitDepthValue<16> operator"" _16bit(unsigned long long value) +{ + return BitDepthValue<16>(static_cast(value)); +} diff --git a/test/ipa/libipa/bitdepth.cpp b/test/ipa/libipa/bitdepth.cpp new file mode 100644 index 00000000..d854637d --- /dev/null +++ b/test/ipa/libipa/bitdepth.cpp @@ -0,0 +1,107 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024 Ideas On Board Oy. + * + * BitDepth converter tests + */ + +#include "../../../src/ipa/libipa/bitdepth.h" + +#include +#include +#include +#include +#include +#include + +#include + +#include "../../libtest/test.h" +using namespace std; + +#define ASSERT_EQ(a, b) \ + if ((a) != (b)) { \ + printf(#a " != " #b "\n"); \ + return TestFail; \ + } + +class BitDepthConverterTest : public Test +{ +protected: + int run() + { + /* 10-bit to 16-bit conversion */ + BitDepthValue<10> value10bit = 64_10bit; + BitDepthValue<16> value16bit = value10bit.convert<16>(); + ASSERT_EQ(value16bit.value(), 4096); + + /* Convert implicitly from another BitDepthValue */ + value10bit = BitDepthValue<8>(16); + ASSERT_EQ(value10bit.value(), 64); + value10bit = 1_8bit; + ASSERT_EQ(value10bit.value(), 4); + + /* Read value explicity and implicitly */ + value10bit = BitDepthValue<10>(64); + ASSERT_EQ(value10bit.value(), 64); + ASSERT_EQ(value10bit, 64); + + /* 12-bit to 8-bit conversion */ + BitDepthValue<12> value12bit = 4096_12bit; + BitDepthValue<8> value8bit = value12bit.convert<8>(); + ASSERT_EQ(value8bit.value(), 256); + + /* Explicit bit depth assignment and conversion */ + value16bit = 32768_16bit; + value12bit = value16bit.convert<12>(); + ASSERT_EQ(value12bit.value(), 2048); + + /* Test hex conversion */ + value8bit = 0xFF_8bit; + ASSERT_EQ(value8bit, 255); + + /* Test conversion to same bit depth makes no difference */ + value16bit = 255_16bit; + value16bit = value16bit.convert<16>(); + ASSERT_EQ(value16bit, 255); + + /* Implicit bit depth assignment */ + value12bit = 10; + ASSERT_EQ(value12bit.value(), 10); + + /* 8-bit to 12-bit conversion */ + value8bit = 128_8bit; + value12bit = value8bit.convert<12>(); + ASSERT_EQ(value12bit.value(), 2048); + + /* 16-bit to 8-bit conversion */ + value16bit = 65535_16bit; + value8bit = value16bit.convert<8>(); + ASSERT_EQ(value8bit.value(), 255); + + /* Implicit assignment with int */ + value8bit = 200; + ASSERT_EQ(value8bit.value(), 200); + + /* 8-bit to 16-bit and back again */ + value8bit = 150_8bit; + value16bit = value8bit.convert<16>(); + value8bit = value16bit.convert<8>(); + ASSERT_EQ(value8bit.value(), 150); + + /* 12-bit to 16-bit and back again */ + value12bit = 3000_12bit; + value16bit = value12bit.convert<16>(); + ASSERT_EQ(value12bit, 3000); + ASSERT_EQ(value16bit, 48000) + value12bit = value16bit.convert<12>(); + ASSERT_EQ(value12bit.value(), 3000); + + /* Test negatives fail */ + //value12bit = BitDepthValue<-1>; + + return TestPass; + } +}; + +TEST_REGISTER(BitDepthConverterTest) From patchwork Wed Nov 27 14:46:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Isaac Scott X-Patchwork-Id: 22126 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 4C394C3200 for ; Wed, 27 Nov 2024 14:47:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0A9C3660E2; Wed, 27 Nov 2024 15:47:10 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="pnELYpN4"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B08E660CC for ; Wed, 27 Nov 2024 15:47:04 +0100 (CET) Received: from isaac-ThinkPad-T16-Gen-2.lan (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DD49132B; Wed, 27 Nov 2024 15:46:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1732718801; bh=4X8G55S0q85DqIdRvLB6LYno0H0+XBO/XrvQQYQ6zSc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=pnELYpN4h5Nqfm8y9jBPFLja3yhBI4OXQw+y/0y69wfKRA10DW81oy++i9lHGTLPv RiDwz+PUc0flWqAB9ANzx8GU/+/YiiUhKskGZVDf0i5Iuj0DSuff4eRCNcaOrxc7SO B7WPXxUbGLVtJao5q41iCMZ/FwTzts9e29cOwihM= From: Isaac Scott To: libcamera devel Cc: Isaac Scott Subject: [RFC PATCH 2/2] libcamera: bitdepth: Adapt camera_sensor_helper to use BitDepth Date: Wed, 27 Nov 2024 14:46:55 +0000 Message-ID: <20241127144655.1074720-3-isaac.scott@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20241127144655.1074720-1-isaac.scott@ideasonboard.com> References: <20241127144655.1074720-1-isaac.scott@ideasonboard.com> 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" Adapt the camera_sensor_helper class to use BitDepth instead of bit shifting for black levels as an example of how the BitDepth implementation can be used. Signed-off-by: Isaac Scott --- src/ipa/libipa/camera_sensor_helper.cpp | 18 +++++++++--------- src/ipa/libipa/camera_sensor_helper.h | 5 +++-- src/ipa/simple/algorithms/awb.cpp | 3 ++- src/ipa/simple/algorithms/blc.cpp | 10 ++++++---- src/ipa/simple/ipa_context.h | 5 +++-- src/ipa/simple/soft_simple.cpp | 5 ++--- 6 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp index c6169bdc..1031dbd1 100644 --- a/src/ipa/libipa/camera_sensor_helper.cpp +++ b/src/ipa/libipa/camera_sensor_helper.cpp @@ -407,7 +407,7 @@ public: CameraSensorHelperAr0144() { /* Power-on default value: 168 at 12bits. */ - blackLevel_ = 2688; + blackLevel_ = 168_12bit; } uint32_t gainCode(double gain) const override @@ -525,7 +525,7 @@ public: CameraSensorHelperImx214() { /* From datasheet: 64 at 10bits. */ - blackLevel_ = 4096; + blackLevel_ = 64_10bit; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 0, 512, -1, 512 }; } @@ -538,7 +538,7 @@ public: CameraSensorHelperImx219() { /* From datasheet: 64 at 10bits. */ - blackLevel_ = 4096; + blackLevel_ = 64_10bit; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 0, 256, -1, 256 }; } @@ -551,7 +551,7 @@ public: CameraSensorHelperImx258() { /* From datasheet: 0x40 at 10bits. */ - blackLevel_ = 4096; + blackLevel_ = 0x40_10bit; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 0, 512, -1, 512 }; } @@ -564,7 +564,7 @@ public: CameraSensorHelperImx283() { /* From datasheet: 0x32 at 10bits. */ - blackLevel_ = 3200; + blackLevel_ = 0x32_10bit; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 0, 2048, -1, 2048 }; } @@ -604,7 +604,7 @@ public: CameraSensorHelperImx335() { /* From datasheet: 0x32 at 10bits. */ - blackLevel_ = 3200; + blackLevel_ = 0x32_10bit; gainType_ = AnalogueGainExponential; gainConstants_.exp = { 1.0, expGainDb(0.3) }; } @@ -665,7 +665,7 @@ public: CameraSensorHelperOv4689() { /* From datasheet: 0x40 at 12bits. */ - blackLevel_ = 1024; + blackLevel_ = 0x40_12bit; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 1, 0, 0, 128 }; } @@ -678,7 +678,7 @@ public: CameraSensorHelperOv5640() { /* From datasheet: 0x10 at 10bits. */ - blackLevel_ = 1024; + blackLevel_ = 0x10_10bit; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 1, 0, 0, 16 }; } @@ -713,7 +713,7 @@ public: CameraSensorHelperOv5675() { /* From Linux kernel driver: 0x40 at 10bits. */ - blackLevel_ = 4096; + blackLevel_ = 0x40_10bit; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 1, 0, 0, 128 }; } diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h index 75868205..b72606bf 100644 --- a/src/ipa/libipa/camera_sensor_helper.h +++ b/src/ipa/libipa/camera_sensor_helper.h @@ -14,6 +14,7 @@ #include #include +#include "bitdepth.h" namespace libcamera { @@ -25,7 +26,7 @@ public: CameraSensorHelper() = default; virtual ~CameraSensorHelper() = default; - std::optional blackLevel() const { return blackLevel_; } + std::optional> blackLevel() const { return blackLevel_; } virtual uint32_t gainCode(double gain) const; virtual double gain(uint32_t gainCode) const; @@ -52,7 +53,7 @@ protected: AnalogueGainExpConstants exp; }; - std::optional blackLevel_; + std::optional> blackLevel_; AnalogueGainType gainType_; AnalogueGainConstants gainConstants_; diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp index 195de41d..cfce5869 100644 --- a/src/ipa/simple/algorithms/awb.cpp +++ b/src/ipa/simple/algorithms/awb.cpp @@ -12,6 +12,7 @@ #include +#include "libipa/bitdepth.h" #include "simple/ipa_context.h" namespace libcamera { @@ -36,7 +37,7 @@ void Awb::process(IPAContext &context, [[maybe_unused]] ControlList &metadata) { const SwIspStats::Histogram &histogram = stats->yHistogram; - const uint8_t blackLevel = context.activeState.blc.level; + const BitDepthValue<8> blackLevel = context.activeState.blc.level; /* * Black level must be subtracted to get the correct AWB ratios, they diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp index b4e32fe1..7c5d3f6d 100644 --- a/src/ipa/simple/algorithms/blc.cpp +++ b/src/ipa/simple/algorithms/blc.cpp @@ -10,6 +10,7 @@ #include #include +#include "libipa/bitdepth.h" namespace libcamera { @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const YamlObject &tuningData) * Convert 16 bit values from the tuning file to 8 bit black * level for the SoftISP. */ - context.configuration.black.level = blackLevel.value() >> 8; + context.configuration.black.level->convert<8>(); } return 0; } @@ -38,7 +39,7 @@ int BlackLevel::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo) { context.activeState.blc.level = - context.configuration.black.level.value_or(255); + context.configuration.black.level.value_or(255_8bit); return 0; } @@ -68,14 +69,15 @@ void BlackLevel::process(IPAContext &context, const unsigned int pixelThreshold = ignoredPercentage * total; const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; const unsigned int currentBlackIdx = - context.activeState.blc.level / histogramRatio; + context.activeState.blc.level.value() / 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; + context.activeState.blc.level = + BitDepthValue<8>(i * histogramRatio); exposure_ = frameContext.sensor.exposure; gain_ = frameContext.sensor.gain; LOG(IPASoftBL, Debug) diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index fd121eeb..be3b967a 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -12,6 +12,7 @@ #include #include +#include "libipa/bitdepth.h" namespace libcamera { @@ -24,13 +25,13 @@ struct IPASessionConfiguration { double againMin, againMax, againMinStep; } agc; struct { - std::optional level; + std::optional> level; } black; }; struct IPAActiveState { struct { - uint8_t level; + BitDepthValue<8> level; } blc; struct { diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index ac2a9421..292fa81f 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -211,11 +211,10 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) /* * The black level from camHelper_ is a 16 bit value, software ISP * works with 8 bit pixel values, both regardless of the actual - * sensor pixel width. Hence we obtain the pixel-based black value - * by dividing the value from the helper by 256. + * sensor pixel width. */ context_.configuration.black.level = - camHelper_->blackLevel().value() / 256; + camHelper_->blackLevel(); } } else { /*