[v3,09/21] ipa: libipa: gamma: Use `std::optional` for segment lengths
diff mbox series

Message ID 20260918120949.191668-10-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: rcar-gen4 + rpp-x1
Related show

Commit Message

Barnabás Pőcze Sept. 18, 2026, 12:09 p.m. UTC 
Pass the segment sizes in an optional `std::span` to `init()`. The advatange
of this over an empty span is that a new `NLutNodes`-dependent `init()` can
be added to `GammaAlgorithm` to enforce the size during compilation.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/libipa/gamma.cpp | 95 ++++++++++++++++++++++------------------
 src/ipa/libipa/gamma.h   | 13 ++++--
 2 files changed, 63 insertions(+), 45 deletions(-)

Comments

Jacopo Mondi Sept. 25, 2026, 7:27 a.m. UTC | #1 
Hi Barnabás

On Fri, Sep 18, 2026 at 02:09:37PM +0200, Barnabás Pőcze wrote:
> Pass the segment sizes in an optional `std::span` to `init()`. The advatange
> of this over an empty span is that a new `NLutNodes`-dependent `init()` can
> be added to `GammaAlgorithm` to enforce the size during compilation.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/ipa/libipa/gamma.cpp | 95 ++++++++++++++++++++++------------------
>  src/ipa/libipa/gamma.h   | 13 ++++--
>  2 files changed, 63 insertions(+), 45 deletions(-)
>
> diff --git a/src/ipa/libipa/gamma.cpp b/src/ipa/libipa/gamma.cpp
> index 20807ac601..fa850aa281 100644
> --- a/src/ipa/libipa/gamma.cpp
> +++ b/src/ipa/libipa/gamma.cpp
> @@ -69,53 +69,22 @@ const float kDefaultGamma = 2.2f;
>   */
>
>  /**
> - * \brief Initialise the algorithm with the given tuning data
> - * \param[out] controls The ControlList into which this algorithm's supported
> - * controls will be emplaced.
> - * \param[in] tuningData The tuning data to use with the algorithm
> - * \param[in] segments A vector of segment spacings to define a custom
> - * X coordinate system for the curve
> + * \copybrief GammaAlgorithm::init()
>   *
> - * Parse \a tuningData and \a segments to initialize the gamma correction curve.
> - * The tuning data may contain a default gamma value to use; otherwise the value
> - * of \a kDefaultGamma will be taken as the default. The piecewise linear
> - * function will be applied on a number of knots whose position is described by
> - * the optional \a segments argument, which describes each segment's relative
> - * length.
> - *
> - * For example, if the gamma correction has to be applied on 16 equally spaced
> - * sampling points, a \a segments array like:
> - *
> - * [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
> - *
> - * would result in evenly spaced knee-points along the X-axis.
> + * This function provides the underlying non-templated implementation for
> + * GammaAlgorithm::init(). Refer to the documentation of that function
> + * for the details.
>   *
> - * Hardware may expect the knee-points to be spaced more densely towards the
> - * start of the curve and more sparsely towards the end, in which case an
> - * alternative array might be:
> - *
> - * [1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 8, 8, 8, 8, 8]
> - *
> - * As the values in \a segments represent the distance between two knee-points
> - * relative to the total distance between the first and last point, the length
> - * of \a segments should be equal to the number of knee-points minus one.
> - *;
> - * If an IPA implementation doesnt't provide \a segments, the GammaAlgorithm
> - * class consturcts an evenly-spaced default.
> - *
> - * IPA modules are expected to call this function as part of their
> - * implementation of Algorithm::init()
> - *
> - * @return 0 on success, a negative error code otherwise
> + * \sa GammaAlgorithm::init()
>   */
>  int GammaAlgorithmBase::init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
> -			     std::span<const unsigned int> segments)
> +			     std::optional<std::span<const unsigned int>> segments)
>  {
>  	/*
>  	 * If the caller doesn't pass in a segment list we simply construct the
>  	 * position of the knee-points assuming equally spaced segments.
>  	 */
> -	if (segments.empty()) {
> +	if (!segments) {
>  		for (unsigned int i = 0; i < nLutNodes_; i++)
>  			kneePoints_[i] = static_cast<float>(i) / (nLutNodes_ - 1);
>  	} else {
> @@ -123,17 +92,17 @@ int GammaAlgorithmBase::init(ControlInfoMap::Map &controls, const ValueNode &tun
>  		 * As segments holds the distance between the knee-points, we
>  		 * expect one fewer segment entries than we have LUT nodes.
>  		 */
> -		if (segments.size() != nLutNodes_ - 1)
> +		if (segments->size() != nLutNodes_ - 1)
>  			return -EINVAL;

Isn't this now enforced at compile time ?

>
> -		float total = std::accumulate(segments.begin(), segments.end(), 0.0f);
> +		float total = std::accumulate(segments->begin(), segments->end(), 0.0f);
>  		float x = 0.0f;
>
>  		for (unsigned int i = 0; i < nLutNodes_; i++) {
>  			kneePoints_[i] = x / total;
>
> -			if (i < segments.size())
> -				x += segments[i];
> +			if (i < segments->size())
> +				x += (*segments)[i];
>  		}
>  	}
>
> @@ -247,6 +216,48 @@ void GammaAlgorithmBase::process(gamma::FrameContext &context, ControlList &meta
>   * - https://en.wikipedia.org/wiki/SRGB
>   */
>
> +/**
> + * \fn GammaAlgorithm::init()
> + * \brief Initialise the algorithm with the given tuning data
> + * \param[out] controls The ControlList into which this algorithm's supported
> + * controls will be emplaced.
> + * \param[in] tuningData The tuning data to use with the algorithm
> + * \param[in] segments A vector of segment spacings to define a custom
> + * X coordinate system for the curve
> + *
> + * Parse \a tuningData and \a segments to initialize the gamma correction curve.
> + * The tuning data may contain a default gamma value to use; otherwise the value
> + * of \a kDefaultGamma will be taken as the default. The piecewise linear
> + * function will be applied on a number of knots whose position is described by
> + * the optional \a segments argument, which describes each segment's relative
> + * length.
> + *
> + * For example, if the gamma correction has to be applied on 16 equally spaced
> + * sampling points, a \a segments array like:
> + *
> + * [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
> + *
> + * would result in evenly spaced knee-points along the X-axis.
> + *
> + * Hardware may expect the knee-points to be spaced more densely towards the
> + * start of the curve and more sparsely towards the end, in which case an
> + * alternative array might be:
> + *
> + * [1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 8, 8, 8, 8, 8]
> + *
> + * As the values in \a segments represent the distance between two knee-points
> + * relative to the total distance between the first and last point, the length
> + * of \a segments should be equal to the number of knee-points minus one.
> + *;
> + * If an IPA implementation doesn't provide \a segments, the GammaAlgorithm
> + * class constructs an evenly-spaced default.
> + *
> + * IPA modules are expected to call this function as part of their
> + * implementation of Algorithm::init()
> + *
> + * @return 0 on success, a negative error code otherwise
> + */
> +
>  /**
>   * \fn GammaAlgorithm::prepare()
>   * \tparam T The type of data expected by the hardware's look-up table
> diff --git a/src/ipa/libipa/gamma.h b/src/ipa/libipa/gamma.h
> index a8872a6c75..946df13cc4 100644
> --- a/src/ipa/libipa/gamma.h
> +++ b/src/ipa/libipa/gamma.h
> @@ -8,6 +8,7 @@
>  #pragma once
>
>  #include <cmath>
> +#include <optional>
>  #include <span>
>  #include <vector>
>
> @@ -46,15 +47,15 @@ public:
>  	{
>  	}
>
> -	int init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
> -		 std::span<const unsigned int> segments = {});
> -
>  	void configure(gamma::ActiveState &state);
>  	void queueRequest(gamma::ActiveState &state, const uint32_t frame,
>  			  gamma::FrameContext &context, const ControlList &controls);
>  	void process(gamma::FrameContext &context, ControlList &metadata);
>
>  protected:
> +	int init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
> +		 std::optional<std::span<const unsigned int>> segmentLengths);
> +
>  	unsigned int nLutNodes_;
>  	float defaultGamma_;
>  	std::vector<float> kneePoints_;
> @@ -71,6 +72,12 @@ public:
>  	{
>  	}
>
> +	int init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
> +		 std::optional<std::span<const unsigned int, NLutNodes - 1>> segments = {})
> +	{
> +		return GammaAlgorithmBase::init(controls, tuningData, segments);
> +	}
> +

Nice!

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>  	template<typename T>
>  	void prepare(gamma::FrameContext &context, std::span<T, NLutNodes> lut)
>  	{
> --
> 2.55.0
>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/gamma.cpp b/src/ipa/libipa/gamma.cpp
index 20807ac601..fa850aa281 100644
--- a/src/ipa/libipa/gamma.cpp
+++ b/src/ipa/libipa/gamma.cpp
@@ -69,53 +69,22 @@  const float kDefaultGamma = 2.2f;
  */
 
 /**
- * \brief Initialise the algorithm with the given tuning data
- * \param[out] controls The ControlList into which this algorithm's supported
- * controls will be emplaced.
- * \param[in] tuningData The tuning data to use with the algorithm
- * \param[in] segments A vector of segment spacings to define a custom
- * X coordinate system for the curve
+ * \copybrief GammaAlgorithm::init()
  *
- * Parse \a tuningData and \a segments to initialize the gamma correction curve.
- * The tuning data may contain a default gamma value to use; otherwise the value
- * of \a kDefaultGamma will be taken as the default. The piecewise linear
- * function will be applied on a number of knots whose position is described by
- * the optional \a segments argument, which describes each segment's relative
- * length.
- *
- * For example, if the gamma correction has to be applied on 16 equally spaced
- * sampling points, a \a segments array like:
- *
- * [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
- *
- * would result in evenly spaced knee-points along the X-axis.
+ * This function provides the underlying non-templated implementation for
+ * GammaAlgorithm::init(). Refer to the documentation of that function
+ * for the details.
  *
- * Hardware may expect the knee-points to be spaced more densely towards the
- * start of the curve and more sparsely towards the end, in which case an
- * alternative array might be:
- *
- * [1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 8, 8, 8, 8, 8]
- *
- * As the values in \a segments represent the distance between two knee-points
- * relative to the total distance between the first and last point, the length
- * of \a segments should be equal to the number of knee-points minus one.
- *;
- * If an IPA implementation doesnt't provide \a segments, the GammaAlgorithm
- * class consturcts an evenly-spaced default.
- *
- * IPA modules are expected to call this function as part of their
- * implementation of Algorithm::init()
- *
- * @return 0 on success, a negative error code otherwise
+ * \sa GammaAlgorithm::init()
  */
 int GammaAlgorithmBase::init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
-			     std::span<const unsigned int> segments)
+			     std::optional<std::span<const unsigned int>> segments)
 {
 	/*
 	 * If the caller doesn't pass in a segment list we simply construct the
 	 * position of the knee-points assuming equally spaced segments.
 	 */
-	if (segments.empty()) {
+	if (!segments) {
 		for (unsigned int i = 0; i < nLutNodes_; i++)
 			kneePoints_[i] = static_cast<float>(i) / (nLutNodes_ - 1);
 	} else {
@@ -123,17 +92,17 @@  int GammaAlgorithmBase::init(ControlInfoMap::Map &controls, const ValueNode &tun
 		 * As segments holds the distance between the knee-points, we
 		 * expect one fewer segment entries than we have LUT nodes.
 		 */
-		if (segments.size() != nLutNodes_ - 1)
+		if (segments->size() != nLutNodes_ - 1)
 			return -EINVAL;
 
-		float total = std::accumulate(segments.begin(), segments.end(), 0.0f);
+		float total = std::accumulate(segments->begin(), segments->end(), 0.0f);
 		float x = 0.0f;
 
 		for (unsigned int i = 0; i < nLutNodes_; i++) {
 			kneePoints_[i] = x / total;
 
-			if (i < segments.size())
-				x += segments[i];
+			if (i < segments->size())
+				x += (*segments)[i];
 		}
 	}
 
@@ -247,6 +216,48 @@  void GammaAlgorithmBase::process(gamma::FrameContext &context, ControlList &meta
  * - https://en.wikipedia.org/wiki/SRGB
  */
 
+/**
+ * \fn GammaAlgorithm::init()
+ * \brief Initialise the algorithm with the given tuning data
+ * \param[out] controls The ControlList into which this algorithm's supported
+ * controls will be emplaced.
+ * \param[in] tuningData The tuning data to use with the algorithm
+ * \param[in] segments A vector of segment spacings to define a custom
+ * X coordinate system for the curve
+ *
+ * Parse \a tuningData and \a segments to initialize the gamma correction curve.
+ * The tuning data may contain a default gamma value to use; otherwise the value
+ * of \a kDefaultGamma will be taken as the default. The piecewise linear
+ * function will be applied on a number of knots whose position is described by
+ * the optional \a segments argument, which describes each segment's relative
+ * length.
+ *
+ * For example, if the gamma correction has to be applied on 16 equally spaced
+ * sampling points, a \a segments array like:
+ *
+ * [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
+ *
+ * would result in evenly spaced knee-points along the X-axis.
+ *
+ * Hardware may expect the knee-points to be spaced more densely towards the
+ * start of the curve and more sparsely towards the end, in which case an
+ * alternative array might be:
+ *
+ * [1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 8, 8, 8, 8, 8]
+ *
+ * As the values in \a segments represent the distance between two knee-points
+ * relative to the total distance between the first and last point, the length
+ * of \a segments should be equal to the number of knee-points minus one.
+ *;
+ * If an IPA implementation doesn't provide \a segments, the GammaAlgorithm
+ * class constructs an evenly-spaced default.
+ *
+ * IPA modules are expected to call this function as part of their
+ * implementation of Algorithm::init()
+ *
+ * @return 0 on success, a negative error code otherwise
+ */
+
 /**
  * \fn GammaAlgorithm::prepare()
  * \tparam T The type of data expected by the hardware's look-up table
diff --git a/src/ipa/libipa/gamma.h b/src/ipa/libipa/gamma.h
index a8872a6c75..946df13cc4 100644
--- a/src/ipa/libipa/gamma.h
+++ b/src/ipa/libipa/gamma.h
@@ -8,6 +8,7 @@ 
 #pragma once
 
 #include <cmath>
+#include <optional>
 #include <span>
 #include <vector>
 
@@ -46,15 +47,15 @@  public:
 	{
 	}
 
-	int init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
-		 std::span<const unsigned int> segments = {});
-
 	void configure(gamma::ActiveState &state);
 	void queueRequest(gamma::ActiveState &state, const uint32_t frame,
 			  gamma::FrameContext &context, const ControlList &controls);
 	void process(gamma::FrameContext &context, ControlList &metadata);
 
 protected:
+	int init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
+		 std::optional<std::span<const unsigned int>> segmentLengths);
+
 	unsigned int nLutNodes_;
 	float defaultGamma_;
 	std::vector<float> kneePoints_;
@@ -71,6 +72,12 @@  public:
 	{
 	}
 
+	int init(ControlInfoMap::Map &controls, const ValueNode &tuningData,
+		 std::optional<std::span<const unsigned int, NLutNodes - 1>> segments = {})
+	{
+		return GammaAlgorithmBase::init(controls, tuningData, segments);
+	}
+
 	template<typename T>
 	void prepare(gamma::FrameContext &context, std::span<T, NLutNodes> lut)
 	{