[v3,11/14] libcamera: ipa: simple: Use float type for adjustment controls
diff mbox series

Message ID 20260114113016.25162-12-mzamazal@redhat.com
State Superseded
Headers show
Series
  • Simple pipeline IPA cleanup
Related show

Commit Message

Milan Zamazal Jan. 14, 2026, 11:30 a.m. UTC
control_ids.h defines the contrast type as float, let's use the same in
simple IPA, instead of double.  Saturation and gamma already use float,
except for the knobs, let's use float for the knobs too.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 include/libcamera/internal/software_isp/debayer_params.h | 2 +-
 src/ipa/simple/algorithms/adjust.cpp                     | 6 +++---
 src/ipa/simple/algorithms/lut.cpp                        | 2 +-
 src/ipa/simple/ipa_context.h                             | 8 ++++----
 4 files changed, 9 insertions(+), 9 deletions(-)

Comments

Barnabás Pőcze Jan. 19, 2026, 10:39 a.m. UTC | #1
2026. 01. 14. 12:30 keltezéssel, Milan Zamazal írta:
> control_ids.h defines the contrast type as float, let's use the same in
> simple IPA, instead of double.  Saturation and gamma already use float,
> except for the knobs, let's use float for the knobs too.
                       ^
                  initializers

I would add that because looking at the code gamma and saturation seem to
be using `float` already, except that those parts are initialized with
`std::optional<double>()`, which is implicitly converted. I usually
prefer using `.reset()` (or maybe ` = {}`) to avoid such behaviour.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>   include/libcamera/internal/software_isp/debayer_params.h | 2 +-
>   src/ipa/simple/algorithms/adjust.cpp                     | 6 +++---
>   src/ipa/simple/algorithms/lut.cpp                        | 2 +-
>   src/ipa/simple/ipa_context.h                             | 8 ++++----
>   4 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h
> index 256c7d43d..2d69bd295 100644
> --- a/include/libcamera/internal/software_isp/debayer_params.h
> +++ b/include/libcamera/internal/software_isp/debayer_params.h
> @@ -59,7 +59,7 @@ struct DebayerParams {
>   	Matrix<float, 3, 3> ccm;
>   	RGB<float> blackLevel;
>   	float gamma;
> -	double contrastExp;
> +	float contrastExp;
>   };
>   
>   } /* namespace libcamera */
> diff --git a/src/ipa/simple/algorithms/adjust.cpp b/src/ipa/simple/algorithms/adjust.cpp
> index cfbc39610..95032799f 100644
> --- a/src/ipa/simple/algorithms/adjust.cpp
> +++ b/src/ipa/simple/algorithms/adjust.cpp
> @@ -33,9 +33,9 @@ int Adjust::init(IPAContext &context, [[maybe_unused]] const YamlObject &tuningD
>   int Adjust::configure(IPAContext &context,
>   		      [[maybe_unused]] const IPAConfigInfo &configInfo)
>   {
> -	context.activeState.knobs.gamma = std::optional<double>();
> -	context.activeState.knobs.contrast = std::optional<double>();
> -	context.activeState.knobs.saturation = std::optional<double>();
> +	context.activeState.knobs.gamma = std::optional<float>();
> +	context.activeState.knobs.contrast = std::optional<float>();
> +	context.activeState.knobs.saturation = std::optional<float>();
>   
>   	return 0;
>   }
> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp
> index a6dbd7b1d..f31a7e631 100644
> --- a/src/ipa/simple/algorithms/lut.cpp
> +++ b/src/ipa/simple/algorithms/lut.cpp
> @@ -41,7 +41,7 @@ void Lut::updateGammaTable(IPAContext &context)
>   		1.0 / context.activeState.knobs.gamma.value_or(kDefaultGamma);
>   	const auto contrast = context.activeState.knobs.contrast.value_or(1.0);
>   	/* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */
> -	double contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));
> +	float contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));
>   
>   	if (!context.gpuIspEnabled) {
>   		auto &gammaTable = context.activeState.gamma.gammaTable;
> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> index 680b72eb9..85455bbfa 100644
> --- a/src/ipa/simple/ipa_context.h
> +++ b/src/ipa/simple/ipa_context.h
> @@ -58,8 +58,8 @@ struct IPAActiveState {
>   		std::array<double, kGammaLookupSize> gammaTable;
>   		uint8_t blackLevel;
>   		float gamma;
> -		double contrast;
> -		double contrastExp;
> +		float contrast;
> +		float contrastExp;
>   	} gamma;
>   
>   	Matrix<float, 3, 3> ccm;
> @@ -69,7 +69,7 @@ struct IPAActiveState {
>   	struct {
>   		std::optional<float> gamma;
>   		/* 0..2 range, 1.0 = normal */
> -		std::optional<double> contrast;
> +		std::optional<float> contrast;
>   		std::optional<float> saturation;
>   	} knobs;
>   };
> @@ -88,7 +88,7 @@ struct IPAFrameContext : public FrameContext {
>   	} gains;
>   
>   	std::optional<float> gamma;
> -	std::optional<double> contrast;
> +	std::optional<float> contrast;
>   	std::optional<float> saturation;
>   };
>

Patch
diff mbox series

diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h
index 256c7d43d..2d69bd295 100644
--- a/include/libcamera/internal/software_isp/debayer_params.h
+++ b/include/libcamera/internal/software_isp/debayer_params.h
@@ -59,7 +59,7 @@  struct DebayerParams {
 	Matrix<float, 3, 3> ccm;
 	RGB<float> blackLevel;
 	float gamma;
-	double contrastExp;
+	float contrastExp;
 };
 
 } /* namespace libcamera */
diff --git a/src/ipa/simple/algorithms/adjust.cpp b/src/ipa/simple/algorithms/adjust.cpp
index cfbc39610..95032799f 100644
--- a/src/ipa/simple/algorithms/adjust.cpp
+++ b/src/ipa/simple/algorithms/adjust.cpp
@@ -33,9 +33,9 @@  int Adjust::init(IPAContext &context, [[maybe_unused]] const YamlObject &tuningD
 int Adjust::configure(IPAContext &context,
 		      [[maybe_unused]] const IPAConfigInfo &configInfo)
 {
-	context.activeState.knobs.gamma = std::optional<double>();
-	context.activeState.knobs.contrast = std::optional<double>();
-	context.activeState.knobs.saturation = std::optional<double>();
+	context.activeState.knobs.gamma = std::optional<float>();
+	context.activeState.knobs.contrast = std::optional<float>();
+	context.activeState.knobs.saturation = std::optional<float>();
 
 	return 0;
 }
diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp
index a6dbd7b1d..f31a7e631 100644
--- a/src/ipa/simple/algorithms/lut.cpp
+++ b/src/ipa/simple/algorithms/lut.cpp
@@ -41,7 +41,7 @@  void Lut::updateGammaTable(IPAContext &context)
 		1.0 / context.activeState.knobs.gamma.value_or(kDefaultGamma);
 	const auto contrast = context.activeState.knobs.contrast.value_or(1.0);
 	/* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */
-	double contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));
+	float contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));
 
 	if (!context.gpuIspEnabled) {
 		auto &gammaTable = context.activeState.gamma.gammaTable;
diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
index 680b72eb9..85455bbfa 100644
--- a/src/ipa/simple/ipa_context.h
+++ b/src/ipa/simple/ipa_context.h
@@ -58,8 +58,8 @@  struct IPAActiveState {
 		std::array<double, kGammaLookupSize> gammaTable;
 		uint8_t blackLevel;
 		float gamma;
-		double contrast;
-		double contrastExp;
+		float contrast;
+		float contrastExp;
 	} gamma;
 
 	Matrix<float, 3, 3> ccm;
@@ -69,7 +69,7 @@  struct IPAActiveState {
 	struct {
 		std::optional<float> gamma;
 		/* 0..2 range, 1.0 = normal */
-		std::optional<double> contrast;
+		std::optional<float> contrast;
 		std::optional<float> saturation;
 	} knobs;
 };
@@ -88,7 +88,7 @@  struct IPAFrameContext : public FrameContext {
 	} gains;
 
 	std::optional<float> gamma;
-	std::optional<double> contrast;
+	std::optional<float> contrast;
 	std::optional<float> saturation;
 };