[v1] ipa: rkisp1: cproc: Fix brightness and contrast value handling
diff mbox series

Message ID 20260116121658.215712-1-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • [v1] ipa: rkisp1: cproc: Fix brightness and contrast value handling
Related show

Commit Message

Stefan Klug Jan. 16, 2026, 12:16 p.m. UTC
By reverse engineering it was discovered that the transfer curve applied
by the CPROC module is

Yout = Yin * contrast + brightness/2.

However the most common expectation on contrast is to not change the
middle gray of an image. So the expected curve is:

Yout = (Yin - 0.5) * contrast + 0.5 + brightness

Map the user expectation to the hardware by changing the brightness
value written to the hardware. This also includes multiplying the
requested brightness value by two.

Note: The limits of the brightness control are left as is even though
they can not be reached without changing the contrast. But limiting them
to [0.5, 0.5] would cut off parts of the combined control space of
contrast + brightness.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

This patch needs to be applied on top of the quantization series:
https://patchwork.libcamera.org/project/libcamera/list/?series=5708
---
 src/ipa/rkisp1/algorithms/cproc.cpp | 38 +++++++++++++++++++++++------
 src/ipa/rkisp1/ipa_context.h        |  1 +
 2 files changed, 31 insertions(+), 8 deletions(-)

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp
index 7484e4780094..ec2b264a79b9 100644
--- a/src/ipa/rkisp1/algorithms/cproc.cpp
+++ b/src/ipa/rkisp1/algorithms/cproc.cpp
@@ -100,13 +100,9 @@  void ColorProcessing::queueRequest(IPAContext &context,
 
 	const auto &brightness = controls.get(controls::Brightness);
 	if (brightness) {
-		BrightnessQ value = *brightness;
-		if (cproc.brightness != value) {
-			cproc.brightness = value;
-			update = true;
-		}
-
-		LOG(RkISP1CProc, Debug) << "Set brightness to " << value;
+		cproc.requestedBrightness = *brightness;
+		LOG(RkISP1CProc, Debug) << "Set brightness to " << *brightness;
+		update = true;
 	}
 
 	const auto &contrast = controls.get(controls::Contrast);
@@ -120,6 +116,24 @@  void ColorProcessing::queueRequest(IPAContext &context,
 		LOG(RkISP1CProc, Debug) << "Set contrast to " << value;
 	}
 
+	/*
+	 * The CPROC module applies the transfer function
+	 *
+	 * Yout = Yin * contrast + brightness/2
+	 *
+	 * Most users expect that changing contrast doesn't change the middle
+	 * gray and that the brightness value is normalized to one. So they
+	 * expect a transfer function of
+	 *
+	 * Yout = (Yin - 0.5) * contrast + 0.5 + brightness
+	 *
+	 * To implement that with the given hardware we need to correct the
+	 * brightness value.
+	 */
+	if (update) {
+		cproc.brightness = (1 - cproc.contrast.value()) + 2 * cproc.requestedBrightness;
+	}
+
 	const auto &hue = controls.get(controls::Hue);
 	if (hue) {
 		/* Scale the Hue from ]-90, +90] */
@@ -179,7 +193,15 @@  void ColorProcessing::process([[maybe_unused]] IPAContext &context,
 			      [[maybe_unused]] const rkisp1_stat_buffer *stats,
 			      ControlList &metadata)
 {
-	metadata.set(controls::Brightness, frameContext.cproc.brightness.value());
+	/*
+	 * Report the actually applied brightness value. See queueRequest() for
+	 * more details.
+	 */
+	float actualBrightness = (frameContext.cproc.brightness.value() -
+				  (1 - frameContext.cproc.contrast.value())) /
+				 2;
+
+	metadata.set(controls::Brightness, actualBrightness);
 	metadata.set(controls::Contrast, frameContext.cproc.contrast.value());
 	metadata.set(controls::Hue, frameContext.cproc.hue.value() * kHueScale);
 	metadata.set(controls::Saturation, frameContext.cproc.saturation.value());
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 80b035044cda..309567858b64 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -122,6 +122,7 @@  struct IPAActiveState {
 	} ccm;
 
 	struct {
+		float requestedBrightness;
 		BrightnessQ brightness;
 		ContrastQ contrast;
 		HueQ hue;