libcamera: software_isp: Fix black level handling in CPU ISP
diff mbox series

Message ID 20260210163744.79510-1-mzamazal@redhat.com
State New
Headers show
Series
  • libcamera: software_isp: Fix black level handling in CPU ISP
Related show

Commit Message

Milan Zamazal Feb. 10, 2026, 4:37 p.m. UTC
The black level handling in CPU ISP has two flaws:

- The black level is applied after white balance rather than before.

- It doesn't handle black levels with different values for individual
  colour channels.

The flaws are in both CCM and non-CCM cases.  The wrong black level and
white balance application order is well visible when the white balance
gains are significantly different from 1.0.  Then the output differs
significantly from GPU ISP output, which uses the correct order.

This patch changes the computations of the lookup tables in a way that
fixes both the problems.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/software_isp/debayer_cpu.cpp | 63 +++++++++++-----------
 1 file changed, 33 insertions(+), 30 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index af7af0a8d..569567a49 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -752,15 +752,12 @@  void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)
 
 void DebayerCpu::updateGammaTable(DebayerParams &params)
 {
-	const RGB<float> blackLevel = params.blackLevel;
-	/* Take let's say the green channel black level */
-	const unsigned int blackIndex = blackLevel[1] * gammaTable_.size();
 	const float gamma = params.gamma;
 	const float contrastExp = params.contrastExp;
 
-	const float divisor = gammaTable_.size() - blackIndex - 1.0;
-	for (unsigned int i = blackIndex; i < gammaTable_.size(); i++) {
-		float normalized = (i - blackIndex) / divisor;
+	const float divisor = gammaTable_.size() - 1.0;
+	for (unsigned int i = 0; i < gammaTable_.size(); i++) {
+		float normalized = i / divisor;
 		/* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */
 		/* Apply simple S-curve */
 		if (normalized < 0.5)
@@ -770,14 +767,6 @@  void DebayerCpu::updateGammaTable(DebayerParams &params)
 		gammaTable_[i] = UINT8_MAX *
 				 std::pow(normalized, gamma);
 	}
-	/*
-	 * Due to CCM operations, the table lookup may reach indices below the black
-	 * level. Let's set the table values below black level to the minimum
-	 * non-black value to prevent problems when the minimum value is
-	 * significantly non-zero (for example, when the image should be all grey).
-	 */
-	std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex,
-		  gammaTable_[blackIndex]);
 }
 
 void DebayerCpu::updateLookupTables(DebayerParams &params)
@@ -789,11 +778,15 @@  void DebayerCpu::updateLookupTables(DebayerParams &params)
 	if (gammaUpdateNeeded)
 		updateGammaTable(params);
 
+	/* Processing order: black level -> gains -> gamma */
 	auto matrixChanged = [](const Matrix<float, 3, 3> &m1, const Matrix<float, 3, 3> &m2) -> bool {
 		return !std::equal(m1.data().begin(), m1.data().end(), m2.data().begin());
 	};
 	const unsigned int gammaTableSize = gammaTable_.size();
-	const double div = static_cast<double>(kRGBLookupSize) / gammaTableSize;
+	RGB<float> blackIndex = params.blackLevel * kRGBLookupSize;
+	if (swapRedBlueGains_)
+		blackIndex = RGB<float>({ blackIndex.b(), blackIndex.g(), blackIndex.r() });
+
 	if (ccmEnabled_) {
 		if (gammaUpdateNeeded ||
 		    matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {
@@ -803,17 +796,22 @@  void DebayerCpu::updateLookupTables(DebayerParams &params)
 			const unsigned int redIndex = swapRedBlueGains_ ? 2 : 0;
 			const unsigned int greenIndex = 1;
 			const unsigned int blueIndex = swapRedBlueGains_ ? 0 : 2;
+			const RGB<float> div =
+				(RGB<float>(kRGBLookupSize) - blackIndex).max(1.0) /
+				kRGBLookupSize;
 			for (unsigned int i = 0; i < kRGBLookupSize; i++) {
-				red[i].r = std::round(i * params.combinedMatrix[redIndex][0]);
-				red[i].g = std::round(i * params.combinedMatrix[greenIndex][0]);
-				red[i].b = std::round(i * params.combinedMatrix[blueIndex][0]);
-				green[i].r = std::round(i * params.combinedMatrix[redIndex][1]);
-				green[i].g = std::round(i * params.combinedMatrix[greenIndex][1]);
-				green[i].b = std::round(i * params.combinedMatrix[blueIndex][1]);
-				blue[i].r = std::round(i * params.combinedMatrix[redIndex][2]);
-				blue[i].g = std::round(i * params.combinedMatrix[greenIndex][2]);
-				blue[i].b = std::round(i * params.combinedMatrix[blueIndex][2]);
-				gammaLut_[i] = gammaTable_[i / div];
+				const RGB<float> rgb = ((RGB<float>(i) - blackIndex) / div).max(0.0);
+				red[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][0]);
+				red[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][0]);
+				red[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][0]);
+				green[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][1]);
+				green[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][1]);
+				green[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][1]);
+				blue[i].r = std::round(rgb.r() * params.combinedMatrix[redIndex][2]);
+				blue[i].g = std::round(rgb.g() * params.combinedMatrix[greenIndex][2]);
+				blue[i].b = std::round(rgb.b() * params.combinedMatrix[blueIndex][2]);
+				;
+				gammaLut_[i] = gammaTable_[i * gammaTableSize / kRGBLookupSize];
 			}
 		}
 	} else {
@@ -822,12 +820,17 @@  void DebayerCpu::updateLookupTables(DebayerParams &params)
 			auto &red = swapRedBlueGains_ ? blue_ : red_;
 			auto &green = green_;
 			auto &blue = swapRedBlueGains_ ? red_ : blue_;
+			const RGB<float> div =
+				(RGB<float>(kRGBLookupSize) - blackIndex).max(1.0) /
+				gammaTableSize;
 			for (unsigned int i = 0; i < kRGBLookupSize; i++) {
-				/* Apply gamma after gain! */
-				const RGB<float> lutGains = (gains * i / div).min(gammaTableSize - 1);
-				red[i] = gammaTable_[static_cast<unsigned int>(lutGains.r())];
-				green[i] = gammaTable_[static_cast<unsigned int>(lutGains.g())];
-				blue[i] = gammaTable_[static_cast<unsigned int>(lutGains.b())];
+				const RGB<float> lutGains =
+					(gains * (RGB<float>(i) - blackIndex) / div)
+						.min(gammaTableSize - 1)
+						.max(0.0);
+				red[i] = gammaTable_[lutGains.r()];
+				green[i] = gammaTable_[lutGains.g()];
+				blue[i] = gammaTable_[lutGains.b()];
 			}
 		}
 	}