[v4] ipa: softisp: adjust: Read default contrast from tuning
diff mbox series

Message ID 20261003211131.2820654-1-opensource@inspiredexperts.com
State New
Headers show
Series
  • [v4] ipa: softisp: adjust: Read default contrast from tuning
Related show

Commit Message

James Alexander Oct. 3, 2026, 9:11 p.m. UTC
SoftISP tuning files can enable the Adjust algorithm, but cannot select
the default Contrast control value. Applications must therefore set the
control themselves when a sensor needs a non-neutral value. Once set, the
value persists across requests until changed.

Read an optional contrast value from the Adjust tuning section, validate
it against the advertised control range, and use it as both the control
default and initial state. Tuning files that omit the value retain the
existing neutral default, and application controls can override it.

Since contrast now always has an initial value, make its active and
per-frame state non-optional and remove the fallback paths.

Signed-off-by: James Alexander <opensource@inspiredexperts.com>
---
Built with the simple pipeline and softisp IPA on x86_64 against
c0049ea0605c1492c99b8f82bb04661a04cf1bf1. All seven existing IPA tests
passed. This revision has not been retested on the camera hardware.

Changes in v4:
- Correct the description: an application only needs to set contrast once.
- Use contrast as the tuning key, consistent with libipa's gamma key.
- Set contrast metadata directly from the frame context.
- Reject non-finite contrast values as well as out-of-range values.
- Regenerate the patch without the line wrapping in the v3 email.
- Rebase onto upstream c0049ea0605c1492c99b8f82bb04661a04cf1bf1.

Changes in v3:
- Rename the tuning key to defaultContrast.
- Make the active and per-frame contrast state non-optional.
- Drop the standalone Adjust test pending a common IPA algorithm test
  structure.

Changes in v2:
- Rebase from the simple IPA onto the softisp Adjust algorithm.
- Limit the tuning option to contrast.
- Reject malformed and out-of-range values instead of clamping them.
- Add focused tests for fallback, tuning, request override, and validation.
---
 src/ipa/softisp/algorithms/adjust.cpp | 34 ++++++++++++++++++++++++++--------
 src/ipa/softisp/algorithms/adjust.h   |  3 +++
 src/ipa/softisp/ipa_context.h         |  4 ++--
 3 files changed, 31 insertions(+), 10 deletions(-)

Patch
diff mbox series

diff --git a/src/ipa/softisp/algorithms/adjust.cpp b/src/ipa/softisp/algorithms/adjust.cpp
index 56e2cf0e8..0cba321f1 100644
--- a/src/ipa/softisp/algorithms/adjust.cpp
+++ b/src/ipa/softisp/algorithms/adjust.cpp
@@ -8,6 +8,8 @@ 
 
 #include "adjust.h"
 
+#include <errno.h>
+
 #include <libcamera/base/log.h>
 #include <libcamera/base/utils.h>
 
@@ -19,17 +21,34 @@  namespace libcamera {
 
 namespace ipa::softisp::algorithms {
 
-constexpr float kDefaultContrast = 1.0f;
 constexpr float kDefaultSaturation = 1.0f;
 
+constexpr float kMinContrast = 0.0f;
+constexpr float kMaxContrast = 2.0f;
+
 LOG_DEFINE_CATEGORY(IPASoftIspAdjust)
 
-int Adjust::init(IPAContext &context, [[maybe_unused]] const ValueNode &tuningData)
+int Adjust::init(IPAContext &context, const ValueNode &tuningData)
 {
+	const ValueNode &contrastNode = tuningData["contrast"];
+	const std::optional<float> contrast = contrastNode.get<float>();
+	if (contrastNode && !contrast) {
+		LOG(IPASoftIspAdjust, Error) << "Failed to parse contrast";
+		return -EINVAL;
+	}
+
+	defaultContrast_ = contrast.value_or(kDefaultContrast);
+	if (!(defaultContrast_ >= kMinContrast && defaultContrast_ <= kMaxContrast)) {
+		LOG(IPASoftIspAdjust, Error)
+			<< "contrast must be in the range [" << kMinContrast
+			<< ", " << kMaxContrast << "]";
+		return -EINVAL;
+	}
+
 	context.ctrlMap[&controls::Gamma] =
 		ControlInfo(0.1f, 10.0f, kDefaultGamma);
 	context.ctrlMap[&controls::Contrast] =
-		ControlInfo(0.0f, 2.0f, kDefaultContrast);
+		ControlInfo(kMinContrast, kMaxContrast, defaultContrast_);
 	if (context.ccmEnabled)
 		context.ctrlMap[&controls::Saturation] =
 			ControlInfo(0.0f, 2.0f, kDefaultSaturation);
@@ -40,7 +59,7 @@  int Adjust::configure(IPAContext &context,
 		      [[maybe_unused]] const IPAConfigInfo &configInfo)
 {
 	context.activeState.knobs.gamma = kDefaultGamma;
-	context.activeState.knobs.contrast = std::optional<float>();
+	context.activeState.knobs.contrast = defaultContrast_;
 	context.activeState.knobs.saturation = std::optional<float>();
 
 	return 0;
@@ -59,7 +78,7 @@  void Adjust::queueRequest(typename Module::Context &context,
 
 	const auto &contrast = controls.get(controls::Contrast);
 	if (contrast.has_value()) {
-		context.activeState.knobs.contrast = contrast;
+		context.activeState.knobs.contrast = contrast.value();
 		LOG(IPASoftIspAdjust, Debug) << "Setting contrast to " << contrast.value();
 	}
 
@@ -107,7 +126,7 @@  void Adjust::prepare(IPAContext &context,
 	}
 
 	params->gamma = 1.0 / context.activeState.knobs.gamma;
-	const float contrast = context.activeState.knobs.contrast.value_or(kDefaultContrast);
+	const float contrast = context.activeState.knobs.contrast;
 	params->contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));
 }
 
@@ -120,8 +139,7 @@  void Adjust::process([[maybe_unused]] IPAContext &context,
 	const auto &gamma = frameContext.gamma;
 	metadata.set(controls::Gamma, gamma);
 
-	const auto &contrast = frameContext.contrast;
-	metadata.set(controls::Contrast, contrast.value_or(kDefaultContrast));
+	metadata.set(controls::Contrast, frameContext.contrast);
 
 	const auto &saturation = frameContext.saturation;
 	metadata.set(controls::Saturation, saturation.value_or(kDefaultSaturation));
diff --git a/src/ipa/softisp/algorithms/adjust.h b/src/ipa/softisp/algorithms/adjust.h
index 1acf7cdf1..726e24cfc 100644
--- a/src/ipa/softisp/algorithms/adjust.h
+++ b/src/ipa/softisp/algorithms/adjust.h
@@ -18,6 +18,7 @@  namespace libcamera {
 namespace ipa::softisp::algorithms {
 
 constexpr float kDefaultGamma = 2.2f;
+constexpr float kDefaultContrast = 1.0f;
 
 class Adjust : public Algorithm
 {
@@ -43,6 +44,8 @@  public:
 
 private:
 	void applySaturation(Matrix<float, 3, 3> &ccm, float saturation);
+
+	float defaultContrast_ = kDefaultContrast;
 };
 
 } /* namespace ipa::softisp::algorithms */
diff --git a/src/ipa/softisp/ipa_context.h b/src/ipa/softisp/ipa_context.h
index 397e6c99b..6c1a517d1 100644
--- a/src/ipa/softisp/ipa_context.h
+++ b/src/ipa/softisp/ipa_context.h
@@ -52,7 +52,7 @@  struct IPAActiveState {
 	struct {
 		float gamma;
 		/* 0..2 range, 1.0 = normal */
-		std::optional<float> contrast;
+		float contrast;
 		std::optional<float> saturation;
 	} knobs;
 };
@@ -68,7 +68,7 @@  struct IPAFrameContext : public FrameContext {
 	} sensor;
 
 	float gamma;
-	std::optional<float> contrast;
+	float contrast;
 	std::optional<float> saturation;
 };