[RFC,2/2] libcamera: bitdepth: Adapt camera_sensor_helper to use BitDepth
diff mbox series

Message ID 20241127144655.1074720-3-isaac.scott@ideasonboard.com
State New
Headers show
Series
  • Add BitDepthValue for simplified bit depth conversion
Related show

Commit Message

Isaac Scott Nov. 27, 2024, 2:46 p.m. UTC
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 <isaac.scott@ideasonboard.com>
---
 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(-)

Patch
diff mbox series

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 <vector>
 
 #include <libcamera/base/class.h>
+#include "bitdepth.h"
 
 namespace libcamera {
 
@@ -25,7 +26,7 @@  public:
 	CameraSensorHelper() = default;
 	virtual ~CameraSensorHelper() = default;
 
-	std::optional<int16_t> blackLevel() const { return blackLevel_; }
+	std::optional<BitDepthValue<16>> 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<int16_t> blackLevel_;
+	std::optional<BitDepthValue<16>> 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 <libcamera/base/log.h>
 
+#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 <numeric>
 
 #include <libcamera/base/log.h>
+#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 <stdint.h>
 
 #include <libipa/fc_queue.h>
+#include "libipa/bitdepth.h"
 
 namespace libcamera {
 
@@ -24,13 +25,13 @@  struct IPASessionConfiguration {
 		double againMin, againMax, againMinStep;
 	} agc;
 	struct {
-		std::optional<uint8_t> level;
+		std::optional<BitDepthValue<8>> 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 {
 		/*