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

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

Commit Message

James Alexander Sept. 16, 2026, 7:27 p.m. UTC
SoftISP tuning files can enable the Adjust algorithm, but they cannot
select the default Contrast control value. This forces applications to
supply the control on every request when a sensor needs a non-neutral
default.

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 per-request controls remain authoritative.

Add focused tests for the neutral fallback, a tuned default, a request
override, and invalid tuning.

Signed-off-by: James Alexander <opensource@inspiredexperts.com>
---
Changes in v2:
- Rebase the change from the simple IPA onto the current softisp Adjust
  algorithm.
- Limit the tuning option to contrast. Gamma is being moved to the common
  libipa GammaAlgorithm, and saturation remains tied to CCM behavior.
- 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 |  27 +++++-
 src/ipa/softisp/algorithms/adjust.h   |   3 +
 test/ipa/meson.build                  |   1 +
 test/ipa/softisp/adjust.cpp           | 125 ++++++++++++++++++++++++++
 test/ipa/softisp/meson.build          |  17 ++++
 5 files changed, 169 insertions(+), 4 deletions(-)
 create mode 100644 test/ipa/softisp/adjust.cpp
 create mode 100644 test/ipa/softisp/meson.build

Patch
diff mbox series

diff --git a/src/ipa/softisp/algorithms/adjust.cpp b/src/ipa/softisp/algorithms/adjust.cpp
index 56e2cf0..8672033 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;
diff --git a/src/ipa/softisp/algorithms/adjust.h b/src/ipa/softisp/algorithms/adjust.h
index 1acf7cd..726e24c 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/test/ipa/meson.build b/test/ipa/meson.build
index ceed15b..f249091 100644
--- a/test/ipa/meson.build
+++ b/test/ipa/meson.build
@@ -1,6 +1,7 @@ 
 # SPDX-License-Identifier: CC0-1.0
 
 subdir('libipa')
+subdir('softisp')
 
 ipa_test = [
     {'name': 'ipa_module_test', 'sources': ['ipa_module_test.cpp']},
diff --git a/test/ipa/softisp/adjust.cpp b/test/ipa/softisp/adjust.cpp
new file mode 100644
index 0000000..24e862b
--- /dev/null
+++ b/test/ipa/softisp/adjust.cpp
@@ -0,0 +1,125 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2026, James Alexander
+ *
+ * Soft ISP image adjustment algorithm tests
+ */
+
+#include <cmath>
+#include <errno.h>
+#include <iostream>
+#include <memory>
+
+#include <libcamera/control_ids.h>
+
+#include "libcamera/internal/value_node.h"
+
+#include "algorithms/adjust.h"
+
+#include "test.h"
+
+using namespace libcamera;
+using namespace libcamera::ipa::softisp;
+using namespace libcamera::ipa::softisp::algorithms;
+
+namespace {
+
+constexpr float kEpsilon = 0.0001f;
+
+bool closeEnough(float lhs, float rhs)
+{
+	return std::abs(lhs - rhs) < kEpsilon;
+}
+
+} /* namespace */
+
+class AdjustTest : public Test
+{
+protected:
+	int testDefaults()
+	{
+		IPAContext context(1);
+		ValueNode tuning;
+		Adjust adjust;
+
+		if (adjust.init(context, tuning))
+			return TestFail;
+
+		IPAConfigInfo configInfo{};
+		if (adjust.configure(context, configInfo))
+			return TestFail;
+
+		const float contrast = context.ctrlMap.at(&controls::Contrast).def().get<float>();
+		if (!closeEnough(contrast, 1.0f) ||
+		    !context.activeState.knobs.contrast ||
+		    !closeEnough(*context.activeState.knobs.contrast, 1.0f)) {
+			std::cerr << "Default contrast was not preserved" << std::endl;
+			return TestFail;
+		}
+
+		return TestPass;
+	}
+
+	int testTunedDefaultAndControl()
+	{
+		IPAContext context(1);
+		ValueNode tuning;
+		tuning.add("contrast", std::make_unique<ValueNode>(1.15f));
+		Adjust adjust;
+
+		if (adjust.init(context, tuning))
+			return TestFail;
+
+		IPAConfigInfo configInfo{};
+		if (adjust.configure(context, configInfo))
+			return TestFail;
+
+		const float contrast = context.ctrlMap.at(&controls::Contrast).def().get<float>();
+		if (!closeEnough(contrast, 1.15f) ||
+		    !context.activeState.knobs.contrast ||
+		    !closeEnough(*context.activeState.knobs.contrast, 1.15f)) {
+			std::cerr << "Tuned contrast default was not applied" << std::endl;
+			return TestFail;
+		}
+
+		ControlList controlsList(controls::controls);
+		controlsList.set(controls::Contrast, 0.9f);
+		IPAFrameContext frameContext{};
+		adjust.queueRequest(context, 0, frameContext, controlsList);
+
+		if (!context.activeState.knobs.contrast ||
+		    !closeEnough(*context.activeState.knobs.contrast, 0.9f)) {
+			std::cerr << "Request control did not override tuned contrast" << std::endl;
+			return TestFail;
+		}
+
+		return TestPass;
+	}
+
+	int testInvalidTuning()
+	{
+		IPAContext context(1);
+		ValueNode tuning;
+		tuning.add("contrast", std::make_unique<ValueNode>(3.0f));
+		Adjust adjust;
+
+		if (adjust.init(context, tuning) != -EINVAL) {
+			std::cerr << "Out-of-range contrast tuning was accepted" << std::endl;
+			return TestFail;
+		}
+
+		return TestPass;
+	}
+
+	int run() override
+	{
+		if (testDefaults() != TestPass ||
+		    testTunedDefaultAndControl() != TestPass ||
+		    testInvalidTuning() != TestPass)
+			return TestFail;
+
+		return TestPass;
+	}
+};
+
+TEST_REGISTER(AdjustTest)
diff --git a/test/ipa/softisp/meson.build b/test/ipa/softisp/meson.build
new file mode 100644
index 0000000..78ad612
--- /dev/null
+++ b/test/ipa/softisp/meson.build
@@ -0,0 +1,17 @@ 
+# SPDX-License-Identifier: CC0-1.0
+
+softisp_adjust_test = executable(
+    'softisp_adjust',
+    [
+        'adjust.cpp',
+        '../../../src/ipa/softisp/algorithms/adjust.cpp',
+    ],
+    dependencies : [libcamera_private, libipa_dep],
+    link_with : [test_libraries],
+    include_directories : [
+        test_includes_internal,
+        include_directories('../../../src/ipa/softisp'),
+    ],
+)
+
+test('softisp_adjust', softisp_adjust_test, suite : 'ipa')