[v2] libcamera: software_isp: Reset stored exposure in black level
diff mbox series

Message ID 20250319095533.24550-1-mzamazal@redhat.com
State New
Headers show
Series
  • [v2] libcamera: software_isp: Reset stored exposure in black level
Related show

Commit Message

Milan Zamazal March 19, 2025, 9:55 a.m. UTC 
Automatic black level setting in software ISP updates the determined
black level value when exposure or gain change.  It stores the last
exposure and gain values to detect the change.

BlackLevel::configure() resets the stored black level value but not the
exposure and gain values.  This can prevent updating the black value and
cause bad image output, e.g. after suspending and resuming a camera, if
exposure and gain remain unchanged.

Let's store exposure and gain in IPAActiveState.  Although the values
are not supposed to be used outside BlackLevel class, storing them in
the context has the advantage of their automatic reset together with the
other context contents and having them in `blc' struct indicates their
relationship to the black value computation.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=259
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/ipa/simple/algorithms/blc.cpp | 10 +++++-----
 src/ipa/simple/algorithms/blc.h   |  2 --
 src/ipa/simple/ipa_context.h      |  2 ++
 3 files changed, 7 insertions(+), 7 deletions(-)

Patch
diff mbox series 

diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp
index 1d7d370b..089e492a 100644
--- a/src/ipa/simple/algorithms/blc.cpp
+++ b/src/ipa/simple/algorithms/blc.cpp
@@ -1,6 +1,6 @@ 
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 /*
- * Copyright (C) 2024, Red Hat Inc.
+ * Copyright (C) 2024-2025, Red Hat Inc.
  *
  * Black level handling
  */
@@ -54,8 +54,8 @@  void BlackLevel::process(IPAContext &context,
 	if (context.configuration.black.level.has_value())
 		return;
 
-	if (frameContext.sensor.exposure == exposure_ &&
-	    frameContext.sensor.gain == gain_) {
+	if (frameContext.sensor.exposure == context.activeState.blc.lastExposure &&
+	    frameContext.sensor.gain == context.activeState.blc.lastGain) {
 		return;
 	}
 
@@ -79,8 +79,8 @@  void BlackLevel::process(IPAContext &context,
 		seen += histogram[i];
 		if (seen >= pixelThreshold) {
 			context.activeState.blc.level = i * histogramRatio;
-			exposure_ = frameContext.sensor.exposure;
-			gain_ = frameContext.sensor.gain;
+			context.activeState.blc.lastExposure = frameContext.sensor.exposure;
+			context.activeState.blc.lastGain = frameContext.sensor.gain;
 			LOG(IPASoftBL, Debug)
 				<< "Auto-set black level: "
 				<< i << "/" << SwIspStats::kYHistogramSize
diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h
index 52d59cab..db9e6d63 100644
--- a/src/ipa/simple/algorithms/blc.h
+++ b/src/ipa/simple/algorithms/blc.h
@@ -30,8 +30,6 @@  public:
 		     ControlList &metadata) override;
 
 private:
-	int32_t exposure_;
-	double gain_;
 	std::optional<uint8_t> definedLevel_;
 };
 
diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
index 4af51306..b1198af5 100644
--- a/src/ipa/simple/ipa_context.h
+++ b/src/ipa/simple/ipa_context.h
@@ -33,6 +33,8 @@  struct IPASessionConfiguration {
 struct IPAActiveState {
 	struct {
 		uint8_t level;
+		int32_t lastExposure;
+		double lastGain;
 	} blc;
 
 	struct {