[v2,5/5] libcamera: software_isp: Pass color lookup tables as floats

19973 diff mbox series

diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h
index 6aaa7d4c..a29eb37a 100644
--- a/include/libcamera/internal/software_isp/debayer_params.h
+++ b/include/libcamera/internal/software_isp/debayer_params.h
@@ -18,7 +18,7 @@  namespace libcamera {
 struct DebayerParams {
 	static constexpr unsigned int kRGBLookupSize = 256;
 
-	using ColorLookupTable = std::array<uint8_t, kRGBLookupSize>;
+	using ColorLookupTable = std::array<float, kRGBLookupSize>;
 
 	ColorLookupTable red;
 	ColorLookupTable green;
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index 0af23ee7..017fd15a 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -86,7 +86,7 @@  private:
 	BlackLevel blackLevel_;
 
 	static constexpr unsigned int kGammaLookupSize = 1024;
-	std::array<uint8_t, kGammaLookupSize> gammaTable_;
+	std::array<float, kGammaLookupSize> gammaTable_;
 	int lastBlackLevel_ = -1;
 
 	int32_t exposureMin_, exposureMax_;
@@ -283,7 +283,7 @@  void IPASoftSimple::processStats(const ControlList &sensorControls)
 		std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);
 		const float divisor = kGammaLookupSize - blackIndex - 1.0;
 		for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)
-			gammaTable_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, gamma);
+			gammaTable_[i] = powf((i - blackIndex) / divisor, gamma);
 
 		lastBlackLevel_ = blackLevel;
 	}
diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO
index 4fcee39b..6bdc5905 100644
--- a/src/libcamera/software_isp/TODO
+++ b/src/libcamera/software_isp/TODO
@@ -72,19 +72,6 @@  stats in hardware, such as the i.MX7), but please keep it on your radar.
 
 ---
 
-4. Hide internal representation of gains from callers
-
-> struct DebayerParams {
-> 	static constexpr unsigned int kGain10 = 256;
-
-Forcing the caller to deal with the internal representation of gains
-isn't nice, especially given that it precludes implementing gains of
-different precisions in different backend. Wouldn't it be better to pass
-the values as floating point numbers, and convert them to the internal
-representation in the implementation of process() before using them ?
-
----
-
 5. Store ISP parameters in per-frame buffers
 
 > /**
diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index 548c2ccd..acc9cd21 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -30,17 +30,17 @@  namespace libcamera {
 
 /**
  * \var DebayerParams::red
- * \brief Lookup table for red color, mapping input values to output values
+ * \brief Lookup table for red color, mapping input values to 0.0..1.0
  */
 
 /**
  * \var DebayerParams::green
- * \brief Lookup table for green color, mapping input values to output values
+ * \brief Lookup table for green color, mapping input values to 0.0..1.0
  */
 
 /**
  * \var DebayerParams::blue
- * \brief Lookup table for blue color, mapping input values to output values
+ * \brief Lookup table for blue color, mapping input values to 0.0..1.0
  */
 
 /**
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index 8b2b2f40..df4c6e88 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -11,6 +11,8 @@ 
 
 #include "debayer_cpu.h"
 
+#include <stddef.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <time.h>
 
@@ -688,6 +690,14 @@  static inline int64_t timeDiff(timespec &after, timespec &before)
 	       (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;
 }
 
+void DebayerCpu::updateColorLookupTable(
+	const DebayerParams::ColorLookupTable &src,
+	ColorLookupTable &dst)
+{
+	for (std::size_t i = 0; i < src.size(); i++)
+		dst[i] = src[i] * UINT8_MAX;
+}
+
 void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)
 {
 	timespec frameStartTime;
@@ -697,9 +707,11 @@  void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
 		clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);
 	}
 
-	green_ = params.green;
-	red_ = swapRedBlueGains_ ? params.blue : params.red;
-	blue_ = swapRedBlueGains_ ? params.red : params.blue;
+	updateColorLookupTable(params.green, green_);
+	updateColorLookupTable(swapRedBlueGains_ ? params.blue : params.red,
+			       red_);
+	updateColorLookupTable(swapRedBlueGains_ ? params.red : params.blue,
+			       blue_);
 
 	/* Copy metadata from the input buffer */
 	FrameMetadata &metadata = output->_d()->metadata();
diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
index 47373426..18808076 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -18,6 +18,7 @@ 
 #include <libcamera/base/object.h>
 
 #include "libcamera/internal/bayer_format.h"
+#include "libcamera/internal/software_isp/debayer_params.h"
 
 #include "debayer.h"
 #include "swstats_cpu.h"
@@ -125,9 +126,12 @@  private:
 	/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */
 	static constexpr unsigned int kMaxLineBuffers = 5;
 
-	DebayerParams::ColorLookupTable red_;
-	DebayerParams::ColorLookupTable green_;
-	DebayerParams::ColorLookupTable blue_;
+	using ColorLookupTable = std::array<uint8_t, DebayerParams::kRGBLookupSize>;
+	void updateColorLookupTable(const DebayerParams::ColorLookupTable &src,
+				    ColorLookupTable &dst);
+	ColorLookupTable red_;
+	ColorLookupTable green_;
+	ColorLookupTable blue_;
 	debayerFn debayer0_;
 	debayerFn debayer1_;
 	debayerFn debayer2_;