[1/1] libcamera: software_isp: Actually apply black level from tuning data
diff mbox series

Message ID 20241108094333.332643-2-mzamazal@redhat.com
State New
Headers show
Series
  • libcamera: software_isp: Actually apply black level from tuning data
Related show

Commit Message

Milan Zamazal Nov. 8, 2024, 9:43 a.m. UTC 
The black level obtained from the tuning file in software ISP is
retrieved in init (because this is the standard algorithm method with
access to tuning data) and stored into context.  But the context gets
reset in configure and the black level is lost and never applied.

Let's store the black level from the tuning file into an algorithm
instance variable and put it into the context only later in configure.
This is similar to what rkisp1 IPA does with the values obtained from
the tuning file.

Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 ("libcamera: software_isp: Clear IPA context on configure and stop")
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/ipa/simple/algorithms/blc.cpp | 7 +++++--
 src/ipa/simple/algorithms/blc.h   | 4 ++++
 src/ipa/simple/soft_simple.cpp    | 3 +--
 3 files changed, 10 insertions(+), 4 deletions(-)

Patch
diff mbox series 

diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp
index b4e32fe1c..1d7d370b5 100644
--- a/src/ipa/simple/algorithms/blc.cpp
+++ b/src/ipa/simple/algorithms/blc.cpp
@@ -21,7 +21,8 @@  BlackLevel::BlackLevel()
 {
 }
 
-int BlackLevel::init(IPAContext &context, const YamlObject &tuningData)
+int BlackLevel::init([[maybe_unused]] IPAContext &context,
+		     const YamlObject &tuningData)
 {
 	auto blackLevel = tuningData["blackLevel"].get<int16_t>();
 	if (blackLevel.has_value()) {
@@ -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;
+		definedLevel_ = blackLevel.value() >> 8;
 	}
 	return 0;
 }
@@ -37,6 +38,8 @@  int BlackLevel::init(IPAContext &context, const YamlObject &tuningData)
 int BlackLevel::configure(IPAContext &context,
 			  [[maybe_unused]] const IPAConfigInfo &configInfo)
 {
+	if (definedLevel_.has_value())
+		context.configuration.black.level = definedLevel_;
 	context.activeState.blc.level =
 		context.configuration.black.level.value_or(255);
 	return 0;
diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h
index 2cf2a8774..453123d27 100644
--- a/src/ipa/simple/algorithms/blc.h
+++ b/src/ipa/simple/algorithms/blc.h
@@ -7,6 +7,9 @@ 
 
 #pragma once
 
+#include <optional>
+#include <stdint.h>
+
 #include "algorithm.h"
 
 namespace libcamera {
@@ -29,6 +32,7 @@  public:
 private:
 	uint32_t exposure_;
 	double gain_;
+	std::optional<uint8_t> definedLevel_;
 };
 
 } /* namespace ipa::soft::algorithms */
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index c8ad55a21..825c06757 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -206,8 +206,7 @@  int IPASoftSimple::configure(const IPAConfigInfo &configInfo)
 			(context_.configuration.agc.againMax -
 			 context_.configuration.agc.againMin) /
 			100.0;
-		if (!context_.configuration.black.level.has_value() &&
-		    camHelper_->blackLevel().has_value()) {
+		if (camHelper_->blackLevel().has_value()) {
 			/*
 			 * The black level from camHelper_ is a 16 bit value, software ISP
 			 * works with 8 bit pixel values, both regardless of the actual