[v2] libcamera: {orientation, transform}FromRotation(): Return `std::optional`
diff mbox series

Message ID 20261019181030.233774-1-barnabas.pocze@ideasonboard.com
State Accepted
Headers show
Series
  • [v2] libcamera: {orientation, transform}FromRotation(): Return `std::optional`
Related show

Commit Message

Barnabás Pőcze Oct. 19, 2026, 6:10 p.m. UTC
Return an empty `std::optional` on failure instead of
using a separate out parameter for signalling success.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
changes in v2:
  * adjust doc comments as requested

v1: https://patchwork.libcamera.org/patch/22654/
---
 include/libcamera/orientation.h               |  3 ++-
 include/libcamera/transform.h                 |  4 +++-
 src/libcamera/orientation.cpp                 | 24 ++++++++-----------
 src/libcamera/sensor/camera_sensor_legacy.cpp |  9 ++++---
 src/libcamera/sensor/camera_sensor_raw.cpp    |  9 ++++---
 src/libcamera/transform.cpp                   | 24 ++++++++-----------
 src/py/libcamera/py_transform.cpp             | 15 ++++++------
 7 files changed, 40 insertions(+), 48 deletions(-)

--
2.56.0

Comments

Laurent Pinchart Oct. 19, 2026, 6:16 p.m. UTC | #1
On Mon, Oct 19, 2026 at 08:10:30PM +0200, Barnabás Pőcze wrote:
> Return an empty `std::optional` on failure instead of
> using a separate out parameter for signalling success.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> changes in v2:
>   * adjust doc comments as requested
> 
> v1: https://patchwork.libcamera.org/patch/22654/
> ---
>  include/libcamera/orientation.h               |  3 ++-
>  include/libcamera/transform.h                 |  4 +++-
>  src/libcamera/orientation.cpp                 | 24 ++++++++-----------
>  src/libcamera/sensor/camera_sensor_legacy.cpp |  9 ++++---
>  src/libcamera/sensor/camera_sensor_raw.cpp    |  9 ++++---
>  src/libcamera/transform.cpp                   | 24 ++++++++-----------
>  src/py/libcamera/py_transform.cpp             | 15 ++++++------
>  7 files changed, 40 insertions(+), 48 deletions(-)
> 
> diff --git a/include/libcamera/orientation.h b/include/libcamera/orientation.h
> index a3b40e6366..4e5b0f5702 100644
> --- a/include/libcamera/orientation.h
> +++ b/include/libcamera/orientation.h
> @@ -8,6 +8,7 @@
>  #pragma once
> 
>  #include <iostream>
> +#include <optional>
> 
>  namespace libcamera {
> 
> @@ -23,7 +24,7 @@ enum class Orientation {
>  	Rotate90,
>  };
> 
> -Orientation orientationFromRotation(int angle, bool *success = nullptr);
> +std::optional<Orientation> orientationFromRotation(int angle);
> 
>  std::ostream &operator<<(std::ostream &out, const Orientation &orientation);
> 
> diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h
> index 4517412a82..4769dc2e92 100644
> --- a/include/libcamera/transform.h
> +++ b/include/libcamera/transform.h
> @@ -7,6 +7,8 @@
> 
>  #pragma once
> 
> +#include <optional>
> +
>  namespace libcamera {
> 
>  enum class Orientation;
> @@ -68,7 +70,7 @@ constexpr Transform operator~(Transform t)
>  	return static_cast<Transform>(~static_cast<int>(t) & 7);
>  }
> 
> -Transform transformFromRotation(int angle, bool *success = nullptr);
> +std::optional<Transform> transformFromRotation(int angle);
> 
>  Transform operator/(const Orientation &o1, const Orientation &o2);
>  Orientation operator*(const Orientation &o, const Transform &t);
> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp
> index 7d7d21ae81..076305f2c1 100644
> --- a/src/libcamera/orientation.cpp
> +++ b/src/libcamera/orientation.cpp
> @@ -58,22 +58,21 @@ namespace libcamera {
>  /**
>   * \brief Return the orientation representing a rotation of the given angle
>   * clockwise
> - * \param[in] angle The angle of rotation in a clockwise sense. Negative values
> - * can be used to represent anticlockwise rotations
> - * \param[out] success Set to `true` if the angle is a multiple of 90 degrees,
> - * otherwise `false`
> - * \return The orientation corresponding to the rotation if \a success was set
> - * to `true`, otherwise the `Rotate0` orientation
> + * \param[in] angle The angle of rotation in a clockwise sense
> + *
> + * The \a angle must be a multiple of 90 degrees. Negative values represent
> + * anticlockwise rotations. If the angle is invalid, the function returns
> + * std::nullopt.
> + *
> + * \return The orientation corresponding to the rotation, or nullopt if the
> + * angle is invalid
>   */
> -Orientation orientationFromRotation(int angle, bool *success)
> +std::optional<Orientation> orientationFromRotation(int angle)
>  {
>  	angle = angle % 360;
>  	if (angle < 0)
>  		angle += 360;
> 
> -	if (success != nullptr)
> -		*success = true;
> -
>  	switch (angle) {
>  	case 0:
>  		return Orientation::Rotate0;
> @@ -85,10 +84,7 @@ Orientation orientationFromRotation(int angle, bool *success)
>  		return Orientation::Rotate270;
>  	}
> 
> -	if (success != nullptr)
> -		*success = false;
> -
> -	return Orientation::Rotate0;
> +	return {};
>  }
> 
>  /**
> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
> index bbe3f2e8fc..cf7410bcee 100644
> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
> @@ -619,14 +619,13 @@ int CameraSensorLegacy::initProperties()
>  		 * Cache the Transform associated with the camera mounting
>  		 * rotation for later use in computeTransform().
>  		 */
> -		bool success;
> -		mountingOrientation_ = orientationFromRotation(propertyValue, &success);
> -		if (!success) {
> +		auto mountingOrientation = orientationFromRotation(propertyValue);
> +		if (!mountingOrientation)
>  			LOG(CameraSensor, Warning)
>  				<< "Invalid rotation of " << propertyValue
>  				<< " degrees - ignoring";
> -			mountingOrientation_ = Orientation::Rotate0;
> -		}
> +
> +		mountingOrientation_ = mountingOrientation.value_or(Orientation::Rotate0);
> 
>  		properties_.set(properties::Rotation, propertyValue);
>  	} else {
> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
> index df7bcf4cbf..75479813c5 100644
> --- a/src/libcamera/sensor/camera_sensor_raw.cpp
> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp
> @@ -628,14 +628,13 @@ int CameraSensorRaw::initProperties()
>  		 * Cache the Transform associated with the camera mounting
>  		 * rotation for later use in computeTransform().
>  		 */
> -		bool success;
> -		mountingOrientation_ = orientationFromRotation(propertyValue, &success);
> -		if (!success) {
> +		auto mountingOrientation = orientationFromRotation(propertyValue);
> +		if (!mountingOrientation)
>  			LOG(CameraSensor, Warning)
>  				<< "Invalid rotation of " << propertyValue
>  				<< " degrees - ignoring";
> -			mountingOrientation_ = Orientation::Rotate0;
> -		}
> +
> +		mountingOrientation_ = mountingOrientation.value_or(Orientation::Rotate0);
> 
>  		properties_.set(properties::Rotation, propertyValue);
>  	} else {
> diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp
> index 9fe8b5620a..126b344cd6 100644
> --- a/src/libcamera/transform.cpp
> +++ b/src/libcamera/transform.cpp
> @@ -268,22 +268,21 @@ Transform operator-(Transform t)
>  /**
>   * \brief Return the transform representing a rotation of the given angle
>   * clockwise
> - * \param[in] angle The angle of rotation in a clockwise sense. Negative values
> - * can be used to represent anticlockwise rotations
> - * \param[out] success Set to `true` if the angle is a multiple of 90 degrees,
> - * otherwise `false`
> - * \return The transform corresponding to the rotation if \a success was set to
> - * `true`, otherwise the `Identity` transform
> + * \param[in] angle The angle of rotation in a clockwise sense
> + *
> + * The \a angle must be a multiple of 90 degrees. Negative values represent
> + * anticlockwise rotations. If the angle is invalid, the function returns
> + * std::nullopt.
> + *
> + * \return The transform corresponding to the rotation, or nullopt if the
> + * angle is invalid
>   */
> -Transform transformFromRotation(int angle, bool *success)
> +std::optional<Transform> transformFromRotation(int angle)
>  {
>  	angle = angle % 360;
>  	if (angle < 0)
>  		angle += 360;
> 
> -	if (success != nullptr)
> -		*success = true;
> -
>  	switch (angle) {
>  	case 0:
>  		return Transform::Identity;
> @@ -295,10 +294,7 @@ Transform transformFromRotation(int angle, bool *success)
>  		return Transform::Rot270;
>  	}
> 
> -	if (success != nullptr)
> -		*success = false;
> -
> -	return Transform::Identity;
> +	return {};
>  }
> 
>  namespace {
> diff --git a/src/py/libcamera/py_transform.cpp b/src/py/libcamera/py_transform.cpp
> index f6b5998394..94a0305d0b 100644
> --- a/src/py/libcamera/py_transform.cpp
> +++ b/src/py/libcamera/py_transform.cpp
> @@ -23,19 +23,18 @@ void init_py_transform(py::module &m)
> 
>  	pyTransform
>  		.def(py::init([](int rotation, bool hflip, bool vflip, bool transpose) {
> -			bool ok;
> -
> -			Transform t = transformFromRotation(rotation, &ok);
> -			if (!ok)
> +			std::optional<Transform> t = transformFromRotation(rotation);
> +			if (!t)
>  				throw std::invalid_argument("Invalid rotation");
> 
>  			if (hflip)
> -				t ^= Transform::HFlip;
> +				*t ^= Transform::HFlip;
>  			if (vflip)
> -				t ^= Transform::VFlip;
> +				*t ^= Transform::VFlip;
>  			if (transpose)
> -				t ^= Transform::Transpose;
> -			return t;
> +				*t ^= Transform::Transpose;
> +
> +			return *t;
>  		}), py::arg("rotation") = 0, py::arg("hflip") = false,
>  		    py::arg("vflip") = false, py::arg("transpose") = false)
>  		.def(py::init([](Transform &other) { return other; }))

Patch
diff mbox series

diff --git a/include/libcamera/orientation.h b/include/libcamera/orientation.h
index a3b40e6366..4e5b0f5702 100644
--- a/include/libcamera/orientation.h
+++ b/include/libcamera/orientation.h
@@ -8,6 +8,7 @@ 
 #pragma once

 #include <iostream>
+#include <optional>

 namespace libcamera {

@@ -23,7 +24,7 @@  enum class Orientation {
 	Rotate90,
 };

-Orientation orientationFromRotation(int angle, bool *success = nullptr);
+std::optional<Orientation> orientationFromRotation(int angle);

 std::ostream &operator<<(std::ostream &out, const Orientation &orientation);

diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h
index 4517412a82..4769dc2e92 100644
--- a/include/libcamera/transform.h
+++ b/include/libcamera/transform.h
@@ -7,6 +7,8 @@ 

 #pragma once

+#include <optional>
+
 namespace libcamera {

 enum class Orientation;
@@ -68,7 +70,7 @@  constexpr Transform operator~(Transform t)
 	return static_cast<Transform>(~static_cast<int>(t) & 7);
 }

-Transform transformFromRotation(int angle, bool *success = nullptr);
+std::optional<Transform> transformFromRotation(int angle);

 Transform operator/(const Orientation &o1, const Orientation &o2);
 Orientation operator*(const Orientation &o, const Transform &t);
diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp
index 7d7d21ae81..076305f2c1 100644
--- a/src/libcamera/orientation.cpp
+++ b/src/libcamera/orientation.cpp
@@ -58,22 +58,21 @@  namespace libcamera {
 /**
  * \brief Return the orientation representing a rotation of the given angle
  * clockwise
- * \param[in] angle The angle of rotation in a clockwise sense. Negative values
- * can be used to represent anticlockwise rotations
- * \param[out] success Set to `true` if the angle is a multiple of 90 degrees,
- * otherwise `false`
- * \return The orientation corresponding to the rotation if \a success was set
- * to `true`, otherwise the `Rotate0` orientation
+ * \param[in] angle The angle of rotation in a clockwise sense
+ *
+ * The \a angle must be a multiple of 90 degrees. Negative values represent
+ * anticlockwise rotations. If the angle is invalid, the function returns
+ * std::nullopt.
+ *
+ * \return The orientation corresponding to the rotation, or nullopt if the
+ * angle is invalid
  */
-Orientation orientationFromRotation(int angle, bool *success)
+std::optional<Orientation> orientationFromRotation(int angle)
 {
 	angle = angle % 360;
 	if (angle < 0)
 		angle += 360;

-	if (success != nullptr)
-		*success = true;
-
 	switch (angle) {
 	case 0:
 		return Orientation::Rotate0;
@@ -85,10 +84,7 @@  Orientation orientationFromRotation(int angle, bool *success)
 		return Orientation::Rotate270;
 	}

-	if (success != nullptr)
-		*success = false;
-
-	return Orientation::Rotate0;
+	return {};
 }

 /**
diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
index bbe3f2e8fc..cf7410bcee 100644
--- a/src/libcamera/sensor/camera_sensor_legacy.cpp
+++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
@@ -619,14 +619,13 @@  int CameraSensorLegacy::initProperties()
 		 * Cache the Transform associated with the camera mounting
 		 * rotation for later use in computeTransform().
 		 */
-		bool success;
-		mountingOrientation_ = orientationFromRotation(propertyValue, &success);
-		if (!success) {
+		auto mountingOrientation = orientationFromRotation(propertyValue);
+		if (!mountingOrientation)
 			LOG(CameraSensor, Warning)
 				<< "Invalid rotation of " << propertyValue
 				<< " degrees - ignoring";
-			mountingOrientation_ = Orientation::Rotate0;
-		}
+
+		mountingOrientation_ = mountingOrientation.value_or(Orientation::Rotate0);

 		properties_.set(properties::Rotation, propertyValue);
 	} else {
diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
index df7bcf4cbf..75479813c5 100644
--- a/src/libcamera/sensor/camera_sensor_raw.cpp
+++ b/src/libcamera/sensor/camera_sensor_raw.cpp
@@ -628,14 +628,13 @@  int CameraSensorRaw::initProperties()
 		 * Cache the Transform associated with the camera mounting
 		 * rotation for later use in computeTransform().
 		 */
-		bool success;
-		mountingOrientation_ = orientationFromRotation(propertyValue, &success);
-		if (!success) {
+		auto mountingOrientation = orientationFromRotation(propertyValue);
+		if (!mountingOrientation)
 			LOG(CameraSensor, Warning)
 				<< "Invalid rotation of " << propertyValue
 				<< " degrees - ignoring";
-			mountingOrientation_ = Orientation::Rotate0;
-		}
+
+		mountingOrientation_ = mountingOrientation.value_or(Orientation::Rotate0);

 		properties_.set(properties::Rotation, propertyValue);
 	} else {
diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp
index 9fe8b5620a..126b344cd6 100644
--- a/src/libcamera/transform.cpp
+++ b/src/libcamera/transform.cpp
@@ -268,22 +268,21 @@  Transform operator-(Transform t)
 /**
  * \brief Return the transform representing a rotation of the given angle
  * clockwise
- * \param[in] angle The angle of rotation in a clockwise sense. Negative values
- * can be used to represent anticlockwise rotations
- * \param[out] success Set to `true` if the angle is a multiple of 90 degrees,
- * otherwise `false`
- * \return The transform corresponding to the rotation if \a success was set to
- * `true`, otherwise the `Identity` transform
+ * \param[in] angle The angle of rotation in a clockwise sense
+ *
+ * The \a angle must be a multiple of 90 degrees. Negative values represent
+ * anticlockwise rotations. If the angle is invalid, the function returns
+ * std::nullopt.
+ *
+ * \return The transform corresponding to the rotation, or nullopt if the
+ * angle is invalid
  */
-Transform transformFromRotation(int angle, bool *success)
+std::optional<Transform> transformFromRotation(int angle)
 {
 	angle = angle % 360;
 	if (angle < 0)
 		angle += 360;

-	if (success != nullptr)
-		*success = true;
-
 	switch (angle) {
 	case 0:
 		return Transform::Identity;
@@ -295,10 +294,7 @@  Transform transformFromRotation(int angle, bool *success)
 		return Transform::Rot270;
 	}

-	if (success != nullptr)
-		*success = false;
-
-	return Transform::Identity;
+	return {};
 }

 namespace {
diff --git a/src/py/libcamera/py_transform.cpp b/src/py/libcamera/py_transform.cpp
index f6b5998394..94a0305d0b 100644
--- a/src/py/libcamera/py_transform.cpp
+++ b/src/py/libcamera/py_transform.cpp
@@ -23,19 +23,18 @@  void init_py_transform(py::module &m)

 	pyTransform
 		.def(py::init([](int rotation, bool hflip, bool vflip, bool transpose) {
-			bool ok;
-
-			Transform t = transformFromRotation(rotation, &ok);
-			if (!ok)
+			std::optional<Transform> t = transformFromRotation(rotation);
+			if (!t)
 				throw std::invalid_argument("Invalid rotation");

 			if (hflip)
-				t ^= Transform::HFlip;
+				*t ^= Transform::HFlip;
 			if (vflip)
-				t ^= Transform::VFlip;
+				*t ^= Transform::VFlip;
 			if (transpose)
-				t ^= Transform::Transpose;
-			return t;
+				*t ^= Transform::Transpose;
+
+			return *t;
 		}), py::arg("rotation") = 0, py::arg("hflip") = false,
 		    py::arg("vflip") = false, py::arg("transpose") = false)
 		.def(py::init([](Transform &other) { return other; }))