[v2] gstreamer: Adding static rotation property to GstLibcameraDevice and mutable orientation property to GstLibcameraSrc

23666 diff mbox series

- rotation: is a static camera property reported
  through Camera::properties().
  This is exposed in GstLibcameraDevice as an extra property.
  You can read it with gst-device-monitor-1.0
  or GstDevice::gst_device_get_properties.

- orientation: is a configurable parameter of the camera that only
  concerns with the actual images rotation.
  This is exposed as a GST_PARAM_MUTABLE_READY parameter of
  GstLibcameraSrc.
  It gets validated via CameraConfiguation::validate().
  If the camera doesn't support it and CameraConfiguration::Adjusted
  is returned the adjusted configuration will be
  logged as a warning.
---
 src/gstreamer/gstlibcameraprovider.cpp |  5 ++
 src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-
 2 files changed, 102 insertions(+), 1 deletion(-)

Hi Giacomo,

Thank you for the patch.

On 6/26/25 3:38 PM, Giacomo Cappellini wrote:
> - rotation: is a static camera property reported
>    through Camera::properties().
>    This is exposed in GstLibcameraDevice as an extra property.
>    You can read it with gst-device-monitor-1.0
>    or GstDevice::gst_device_get_properties.
>
> - orientation: is a configurable parameter of the camera that only
>    concerns with the actual images rotation.
>    This is exposed as a GST_PARAM_MUTABLE_READY parameter of
>    GstLibcameraSrc.
>    It gets validated via CameraConfiguation::validate().
>    If the camera doesn't support it and CameraConfiguration::Adjusted
>    is returned the adjusted configuration will be
>    logged as a warning.


Both of these are separate things, so separate the patches please for 
easier review.

> ---
>   src/gstreamer/gstlibcameraprovider.cpp |  5 ++
>   src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-
>   2 files changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index 5da96ea3..d3384c57 100644
> --- a/src/gstreamer/gstlibcameraprovider.cpp
> +++ b/src/gstreamer/gstlibcameraprovider.cpp
> @@ -10,6 +10,7 @@
>   
>   #include "gstlibcameraprovider.h"
>   
> +#include <libcamera/libcamera.h>
>   #include <libcamera/camera.h>
>   #include <libcamera/camera_manager.h>
>   
> @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>   	static const std::array roles{ StreamRole::VideoRecording };
>   	g_autoptr(GstCaps) caps = gst_caps_new_empty();
>   	const gchar *name = camera->id().c_str();
> +	const int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);


The .value_or(0) was troubling me a bit, in case we have missing 
rotation property set, but looking at initProperties() in CameraSensor 
classes, it seems libcamera also sets it to '0' in case the rotation of 
the sensor cannot be queried.

So .value_or(0) is really optional here, but doesn't hurt to have it.

>   
>   	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
>   	if (!config || config->size() != roles.size()) {
> @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>   				       "display-name", name,
>   				       "caps", caps,
>   				       "device-class", "Source/Video",
> +					   "properties", gst_structure_new("device-properties",
> +							"rotation", G_TYPE_INT, rotation,
> +							NULL),


Do not create the structure here as this has 2 problems:

- Firstly, when more camera properties are added to this GstStructure, 
it will be a bit cumbersome to read the code

- Secondly, you are having a memory leak here.

So create the properties GstStructure above gst_libcamera_device_new() as

         g_autoptr(GstStructure) props = gst_structure_new(...);

g_autoptr() will auto free `props` when it goes out of scope.

>   				       nullptr));
>   }
>   
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 3aca4eed..fe3249ff 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -32,6 +32,7 @@
>   #include <tuple>
>   #include <utility>
>   #include <vector>
> +#include <sstream>
>   
>   #include <libcamera/camera.h>
>   #include <libcamera/camera_manager.h>
> @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {
>   	GstTask *task;
>   
>   	gchar *camera_name;
> +	Orientation orientation;


You should save here GstVideoOrientationMethod

>   
>   	std::atomic<GstEvent *> pending_eos;
>   
> @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {
>   enum {
>   	PROP_0,
>   	PROP_CAMERA_NAME,
> +	PROP_ORIENTATION,
>   	PROP_LAST
>   };
>   
> @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>   		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>   	}
>   
> +	/* Set orientation control. */
> +	state->config_->orientation = self->orientation;


Corresponding mapped value of libcamera::orientation from 
GstVideoOrientationMethod self->orientation needs to be passed here.


> +
>   	/* Validate the configuration. */
> -	if (state->config_->validate() == CameraConfiguration::Invalid)
> +	switch(state->config_->validate()) {
> +	case CameraConfiguration::Valid:
> +		GST_DEBUG_OBJECT(self, "Camera configuration is valid");
> +		break;
> +	case CameraConfiguration::Adjusted:
> +	{
> +		/*
> +		 * The configuration has been adjusted and is now valid.
> +		 * Parameters may have changed for any stream, and stream configurations may have been removed.
> +		 * Log the adjusted configuration.
> +		 */
> +		for (gsize i = 0; i < state->config_->size(); i++) {
> +			const StreamConfiguration &cfg = state->config_->at(i);
> +			GST_WARNING_OBJECT(self, "Camera configuration adjusted for stream %zu: %s", i, cfg.toString().c_str());
> +		}


This is not correct, CameraConfiguration::Adjusted can be returned if 
one (or more) parts of CameraConfiguration is adjusted. So it may happen 
that stream configurations are unchanged but something else is adjusted 
- but still you would be printing the GST_WARNINGs here.

> +		std::ostringstream oss;
> +		oss << state->config_->orientation;
> +		GST_WARNING_OBJECT(self, "Camera configuration adjusted orientation: %s", oss.str().c_str());


Similarly, not true again, orientation might have not been adjusted but 
one of other parts of camera configuration. This would misled.

> +		self->orientation = state->config_->orientation;
> +		break;
> +	}

I think reading the state->config_->orientation (libcamera::orientation) 
and updating the applied / validated orientation self->orientation is a 
good idea, but you should drop all the switch-case handling above.

If you really want to print a debug/warning that orientation is adjusted 
- you need to compare the orientation before and after validate() call.


> +	case CameraConfiguration::Invalid:
> +		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> +				  ("Camera configuration is not supported"),
> +				  ("CameraConfiguration::validate() returned Invalid"));
>   		return false;
> +	}
>   
>   	int ret = state->cam_->configure(state->config_.get());
>   	if (ret) {
> @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
>   		g_free(self->camera_name);
>   		self->camera_name = g_value_dup_string(value);
>   		break;
> +	case PROP_ORIENTATION:
> +		self->orientation = (libcamera::Orientation)g_value_get_enum(value);


Application are expected to use GstVideoOrientationMethod values

> +		break;
>   	default:
>   		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
>   			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
>   	case PROP_CAMERA_NAME:
>   		g_value_set_string(value, self->camera_name);
>   		break;
> +	case PROP_ORIENTATION:
> +		g_value_set_enum(value, (gint)self->orientation);
> +		break;
>   	default:
>   		if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
>   			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
>   	gst_element_remove_pad(element, pad);
>   }
>   
> +static GType
> +gst_libcamera_orientation_get_type()


All this needs to be handled by GST_VIDEO_ORIENTATION_*  enum which is 
global to gstreamer and application are expected to work with those. 
Please refer to Nicolas' comments  from v1 review.

> +{
> +	static GType type = 0;
> +	static const GEnumValue values[] = {
> +		{
> +			static_cast<gint>(libcamera::Orientation::Rotate0),
> +			"libcamera::Orientation::Rotate0",
> +			"rotate-0",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate0Mirror),
> +			"libcamera::Orientation::Rotate0Mirror",
> +			"rotate-0-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate180),
> +			"libcamera::Orientation::Rotate180",
> +			"rotate-180",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate180Mirror),
> +			"libcamera::Orientation::Rotate180Mirror",
> +			"rotate-180-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate90Mirror),
> +			"libcamera::Orientation::Rotate90Mirror",
> +			"rotate-90-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate270),
> +			"libcamera::Orientation::Rotate270",
> +			"rotate-270",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate270Mirror),
> +			"libcamera::Orientation::Rotate270Mirror",
> +			"rotate-270-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate90),
> +			"libcamera::Orientation::Rotate90",
> +			"rotate-90",
> +		},
> +		{ 0, nullptr, nullptr }
> +	};
> +
> +	if (!type)
> +		type = g_enum_register_static("GstLibcameraOrientation", values);
> +
> +	return type;
> +}
> +
>   static void
>   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>   {
> @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>   							     | G_PARAM_STATIC_STRINGS));
>   	g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
>   
> +	/* Register the orientation enum type. */
> +	GType orientation_type = gst_libcamera_orientation_get_type();
> +	spec = g_param_spec_enum("orientation", "Orientation",
> +					       "Select the orientation of the camera.",
> +					       orientation_type,
> +					       static_cast<gint>(libcamera::Orientation::Rotate0),
> +					       (GParamFlags)(GST_PARAM_MUTABLE_READY
> +							     | G_PARAM_CONSTRUCT
> +							     | G_PARAM_READWRITE
> +							     | G_PARAM_STATIC_STRINGS));
> +	g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
> +
>   	GstCameraControls::installProperties(object_class, PROP_LAST);
>   }
>

Hi,

Le jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :
> - rotation: is a static camera property reported
>   through Camera::properties().
>   This is exposed in GstLibcameraDevice as an extra property.
>   You can read it with gst-device-monitor-1.0
>   or GstDevice::gst_device_get_properties.
> 
> - orientation: is a configurable parameter of the camera that only
>   concerns with the actual images rotation.
>   This is exposed as a GST_PARAM_MUTABLE_READY parameter of
>   GstLibcameraSrc.
>   It gets validated via CameraConfiguation::validate().
>   If the camera doesn't support it and CameraConfiguration::Adjusted
>   is returned the adjusted configuration will be
>   logged as a warning.
> ---
>  src/gstreamer/gstlibcameraprovider.cpp |  5 ++
>  src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-
>  2 files changed, 102 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index 5da96ea3..d3384c57 100644
> --- a/src/gstreamer/gstlibcameraprovider.cpp
> +++ b/src/gstreamer/gstlibcameraprovider.cpp
> @@ -10,6 +10,7 @@
>  
>  #include "gstlibcameraprovider.h"
>  
> +#include <libcamera/libcamera.h>
>  #include <libcamera/camera.h>
>  #include <libcamera/camera_manager.h>
>  
> @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>  	static const std::array roles{ StreamRole::VideoRecording };
>  	g_autoptr(GstCaps) caps = gst_caps_new_empty();
>  	const gchar *name = camera->id().c_str();
> +	const int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);
>  
>  	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
>  	if (!config || config->size() != roles.size()) {
> @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>  				       "display-name", name,
>  				       "caps", caps,
>  				       "device-class", "Source/Video",
> +					   "properties", gst_structure_new("device-properties",
> +							"rotation", G_TYPE_INT, rotation,

What do you think of calling this "mounting-rotation" or "mounting-orientation"
? I'm also wondering if we really want to expose libcamera internal enum values
here. Speed is not a problem, I'd use static strings.

> +							NULL),
>  				       nullptr));
>  }
>  
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 3aca4eed..fe3249ff 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -32,6 +32,7 @@
>  #include <tuple>
>  #include <utility>
>  #include <vector>
> +#include <sstream>
>  
>  #include <libcamera/camera.h>
>  #include <libcamera/camera_manager.h>
> @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {
>  	GstTask *task;
>  
>  	gchar *camera_name;
> +	Orientation orientation;
>  
>  	std::atomic<GstEvent *> pending_eos;
>  
> @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {
>  enum {
>  	PROP_0,
>  	PROP_CAMERA_NAME,
> +	PROP_ORIENTATION,
>  	PROP_LAST
>  };
>  
> @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>  		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>  	}
>  
> +	/* Set orientation control. */
> +	state->config_->orientation = self->orientation;
> +
>  	/* Validate the configuration. */
> -	if (state->config_->validate() == CameraConfiguration::Invalid)
> +	switch(state->config_->validate()) {
> +	case CameraConfiguration::Valid:
> +		GST_DEBUG_OBJECT(self, "Camera configuration is valid");
> +		break;
> +	case CameraConfiguration::Adjusted:
> +	{
> +		/*
> +		 * The configuration has been adjusted and is now valid.
> +		 * Parameters may have changed for any stream, and stream configurations may have been removed.
> +		 * Log the adjusted configuration.
> +		 */
> +		for (gsize i = 0; i < state->config_->size(); i++) {
> +			const StreamConfiguration &cfg = state->config_->at(i);
> +			GST_WARNING_OBJECT(self, "Camera configuration adjusted for stream %zu: %s", i, cfg.toString().c_str());
> +		}
> +		std::ostringstream oss;
> +		oss << state->config_->orientation;
> +		GST_WARNING_OBJECT(self, "Camera configuration adjusted orientation: %s", oss.str().c_str());
> +		self->orientation = state->config_->orientation;
> +		break;
> +	}
> +	case CameraConfiguration::Invalid:
> +		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> +				  ("Camera configuration is not supported"),
> +				  ("CameraConfiguration::validate() returned Invalid"));
>  		return false;
> +	}
>  
>  	int ret = state->cam_->configure(state->config_.get());
>  	if (ret) {
> @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
>  		g_free(self->camera_name);
>  		self->camera_name = g_value_dup_string(value);
>  		break;
> +	case PROP_ORIENTATION:
> +		self->orientation = (libcamera::Orientation)g_value_get_enum(value);
> +		break;
>  	default:
>  		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
>  			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
>  	case PROP_CAMERA_NAME:
>  		g_value_set_string(value, self->camera_name);
>  		break;
> +	case PROP_ORIENTATION:
> +		g_value_set_enum(value, (gint)self->orientation);
> +		break;
>  	default:
>  		if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
>  			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
>  	gst_element_remove_pad(element, pad);
>  }
>  
> +static GType
> +gst_libcamera_orientation_get_type()
> +{
> +	static GType type = 0;
> +	static const GEnumValue values[] = {
> +		{
> +			static_cast<gint>(libcamera::Orientation::Rotate0),
> +			"libcamera::Orientation::Rotate0",
> +			"rotate-0",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate0Mirror),
> +			"libcamera::Orientation::Rotate0Mirror",
> +			"rotate-0-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate180),
> +			"libcamera::Orientation::Rotate180",
> +			"rotate-180",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate180Mirror),
> +			"libcamera::Orientation::Rotate180Mirror",
> +			"rotate-180-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate90Mirror),
> +			"libcamera::Orientation::Rotate90Mirror",
> +			"rotate-90-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate270),
> +			"libcamera::Orientation::Rotate270",
> +			"rotate-270",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate270Mirror),
> +			"libcamera::Orientation::Rotate270Mirror",
> +			"rotate-270-mirror",
> +		}, {
> +			static_cast<gint>(libcamera::Orientation::Rotate90),
> +			"libcamera::Orientation::Rotate90",
> +			"rotate-90",
> +		},
> +		{ 0, nullptr, nullptr }
> +	};
> +
> +	if (!type)
> +		type = g_enum_register_static("GstLibcameraOrientation", values);
> +
> +	return type;
> +}

So remain to debate is this enum. We have to decide around leaking libcamera API
into GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation
enum and names. Its libcamera project choice, be aware that if we move it back
into GStreamer in the future, this will change, since we try really hard to stop
leaking project specific enum values.

Nicolas

> +
>  static void
>  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>  {
> @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>  							     | G_PARAM_STATIC_STRINGS));
>  	g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
>  
> +	/* Register the orientation enum type. */
> +	GType orientation_type = gst_libcamera_orientation_get_type();
> +	spec = g_param_spec_enum("orientation", "Orientation",
> +					       "Select the orientation of the camera.",
> +					       orientation_type,
> +					       static_cast<gint>(libcamera::Orientation::Rotate0),
> +					       (GParamFlags)(GST_PARAM_MUTABLE_READY
> +							     | G_PARAM_CONSTRUCT
> +							     | G_PARAM_READWRITE
> +							     | G_PARAM_STATIC_STRINGS));
> +	g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
> +
>  	GstCameraControls::installProperties(object_class, PROP_LAST);
>  }
>

Quoting Nicolas Dufresne (2025-06-26 14:55:15)
> Hi,
> 
> Le jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :
> > - rotation: is a static camera property reported
> >   through Camera::properties().
> >   This is exposed in GstLibcameraDevice as an extra property.
> >   You can read it with gst-device-monitor-1.0
> >   or GstDevice::gst_device_get_properties.
> > 
> > - orientation: is a configurable parameter of the camera that only
> >   concerns with the actual images rotation.
> >   This is exposed as a GST_PARAM_MUTABLE_READY parameter of
> >   GstLibcameraSrc.
> >   It gets validated via CameraConfiguation::validate().
> >   If the camera doesn't support it and CameraConfiguration::Adjusted
> >   is returned the adjusted configuration will be
> >   logged as a warning.
> > ---
> >  src/gstreamer/gstlibcameraprovider.cpp |  5 ++
> >  src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-
> >  2 files changed, 102 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> > index 5da96ea3..d3384c57 100644
> > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > @@ -10,6 +10,7 @@
> >  
> >  #include "gstlibcameraprovider.h"
> >  
> > +#include <libcamera/libcamera.h>
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> >  
> > @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
> >       static const std::array roles{ StreamRole::VideoRecording };
> >       g_autoptr(GstCaps) caps = gst_caps_new_empty();
> >       const gchar *name = camera->id().c_str();
> > +     const int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);
> >  
> >       std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
> >       if (!config || config->size() != roles.size()) {
> > @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
> >                                      "display-name", name,
> >                                      "caps", caps,
> >                                      "device-class", "Source/Video",
> > +                                        "properties", gst_structure_new("device-properties",
> > +                                                     "rotation", G_TYPE_INT, rotation,
> 
> What do you think of calling this "mounting-rotation" or "mounting-orientation"
> ? I'm also wondering if we really want to expose libcamera internal enum values
> here. Speed is not a problem, I'd use static strings.
> 
> > +                                                     NULL),
> >                                      nullptr));
> >  }
> >  
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 3aca4eed..fe3249ff 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -32,6 +32,7 @@
> >  #include <tuple>
> >  #include <utility>
> >  #include <vector>
> > +#include <sstream>
> >  
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> > @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {
> >       GstTask *task;
> >  
> >       gchar *camera_name;
> > +     Orientation orientation;
> >  
> >       std::atomic<GstEvent *> pending_eos;
> >  
> > @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {
> >  enum {
> >       PROP_0,
> >       PROP_CAMERA_NAME,
> > +     PROP_ORIENTATION,
> >       PROP_LAST
> >  };
> >  
> > @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> >               gst_libcamera_get_framerate_from_caps(caps, element_caps);
> >       }
> >  
> > +     /* Set orientation control. */
> > +     state->config_->orientation = self->orientation;
> > +
> >       /* Validate the configuration. */
> > -     if (state->config_->validate() == CameraConfiguration::Invalid)
> > +     switch(state->config_->validate()) {
> > +     case CameraConfiguration::Valid:
> > +             GST_DEBUG_OBJECT(self, "Camera configuration is valid");
> > +             break;
> > +     case CameraConfiguration::Adjusted:
> > +     {
> > +             /*
> > +              * The configuration has been adjusted and is now valid.
> > +              * Parameters may have changed for any stream, and stream configurations may have been removed.
> > +              * Log the adjusted configuration.
> > +              */
> > +             for (gsize i = 0; i < state->config_->size(); i++) {
> > +                     const StreamConfiguration &cfg = state->config_->at(i);
> > +                     GST_WARNING_OBJECT(self, "Camera configuration adjusted for stream %zu: %s", i, cfg.toString().c_str());
> > +             }
> > +             std::ostringstream oss;
> > +             oss << state->config_->orientation;
> > +             GST_WARNING_OBJECT(self, "Camera configuration adjusted orientation: %s", oss.str().c_str());
> > +             self->orientation = state->config_->orientation;
> > +             break;
> > +     }
> > +     case CameraConfiguration::Invalid:
> > +             GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> > +                               ("Camera configuration is not supported"),
> > +                               ("CameraConfiguration::validate() returned Invalid"));
> >               return false;
> > +     }
> >  
> >       int ret = state->cam_->configure(state->config_.get());
> >       if (ret) {
> > @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
> >               g_free(self->camera_name);
> >               self->camera_name = g_value_dup_string(value);
> >               break;
> > +     case PROP_ORIENTATION:
> > +             self->orientation = (libcamera::Orientation)g_value_get_enum(value);
> > +             break;
> >       default:
> >               if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
> >                       G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
> >       case PROP_CAMERA_NAME:
> >               g_value_set_string(value, self->camera_name);
> >               break;
> > +     case PROP_ORIENTATION:
> > +             g_value_set_enum(value, (gint)self->orientation);
> > +             break;
> >       default:
> >               if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
> >                       G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
> >       gst_element_remove_pad(element, pad);
> >  }
> >  
> > +static GType
> > +gst_libcamera_orientation_get_type()
> > +{
> > +     static GType type = 0;
> > +     static const GEnumValue values[] = {
> > +             {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate0),
> > +                     "libcamera::Orientation::Rotate0",
> > +                     "rotate-0",
> > +             }, {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate0Mirror),
> > +                     "libcamera::Orientation::Rotate0Mirror",
> > +                     "rotate-0-mirror",
> > +             }, {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate180),
> > +                     "libcamera::Orientation::Rotate180",
> > +                     "rotate-180",
> > +             }, {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate180Mirror),
> > +                     "libcamera::Orientation::Rotate180Mirror",
> > +                     "rotate-180-mirror",
> > +             }, {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate90Mirror),
> > +                     "libcamera::Orientation::Rotate90Mirror",
> > +                     "rotate-90-mirror",
> > +             }, {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate270),
> > +                     "libcamera::Orientation::Rotate270",
> > +                     "rotate-270",
> > +             }, {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate270Mirror),
> > +                     "libcamera::Orientation::Rotate270Mirror",
> > +                     "rotate-270-mirror",
> > +             }, {
> > +                     static_cast<gint>(libcamera::Orientation::Rotate90),
> > +                     "libcamera::Orientation::Rotate90",
> > +                     "rotate-90",
> > +             },
> > +             { 0, nullptr, nullptr }
> > +     };
> > +
> > +     if (!type)
> > +             type = g_enum_register_static("GstLibcameraOrientation", values);
> > +
> > +     return type;
> > +}
> 
> So remain to debate is this enum. We have to decide around leaking libcamera API
> into GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation
> enum and names. Its libcamera project choice, be aware that if we move it back
> into GStreamer in the future, this will change, since we try really hard to stop
> leaking project specific enum values.

The whole point of the gstlibcamerasrc is to map gstreamer <-> libcamera
- so if there's an existing means for gstreamer to define this with all
other elements - this code should be mapping those (GstVideoOrientation?) to
the libcamera equivalent in my view.

--
Kieran


> 
> Nicolas
> 
> > +
> >  static void
> >  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >  {
> > @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >                                                            | G_PARAM_STATIC_STRINGS));
> >       g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
> >  
> > +     /* Register the orientation enum type. */
> > +     GType orientation_type = gst_libcamera_orientation_get_type();
> > +     spec = g_param_spec_enum("orientation", "Orientation",
> > +                                            "Select the orientation of the camera.",
> > +                                            orientation_type,
> > +                                            static_cast<gint>(libcamera::Orientation::Rotate0),
> > +                                            (GParamFlags)(GST_PARAM_MUTABLE_READY
> > +                                                          | G_PARAM_CONSTRUCT
> > +                                                          | G_PARAM_READWRITE
> > +                                                          | G_PARAM_STATIC_STRINGS));
> > +     g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
> > +
> >       GstCameraControls::installProperties(object_class, PROP_LAST);
> >  }
> >

Now that people which knows about gstreamer made the real comments,
let me do the nitpicking.

On Thu, Jun 26, 2025 at 09:55:15AM -0400, Nicolas Dufresne wrote:
> Hi,
>
> Le jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :
> > - rotation: is a static camera property reported
> >   through Camera::properties().
> >   This is exposed in GstLibcameraDevice as an extra property.
> >   You can read it with gst-device-monitor-1.0
> >   or GstDevice::gst_device_get_properties.
> >
> > - orientation: is a configurable parameter of the camera that only
> >   concerns with the actual images rotation.
> >   This is exposed as a GST_PARAM_MUTABLE_READY parameter of
> >   GstLibcameraSrc.
> >   It gets validated via CameraConfiguation::validate().
> >   If the camera doesn't support it and CameraConfiguration::Adjusted
> >   is returned the adjusted configuration will be
> >   logged as a warning.

Please take into account the comments you have received on v1 and try
to comply with the requirements here specified
https://cbea.ms/git-commit/

1) Reduce commit title length (the suggested 50 cols limits is very
short, try to stay in 80 at least)
2) Use imperative style as suggested in v1
3) Try to use a commit message style that matches the existing
   history
4) Missing Signed-off-by: as commented on v1

This patch should be broken in 2 separate patches, as Umang suggested

- gstreamer: Report the camera mounting rotation
- gstreamer: Add support for Orientation

Each commit should go around the lines of:

"libcamera reports through the Rotation property the camera physical
mounting rotation. Add an extra property to GstLibcameraDevice
to report it."

"libcamera allows to control the images orientation through the
CameraConfiguration::orientation field. Expose a GST_PARAM_MUTABLE_READY
parameter in GstLibcameraSrc to control it.

As the requested orientation might be adjusted by
CameraConfiguation::validate(), inspect the actually image orientation
after validate() and expose it."

(I don't think you should warn, unless the same warning happens for
other paramters that could be possibile adjusted)

> > ---
> >  src/gstreamer/gstlibcameraprovider.cpp |  5 ++
> >  src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-
> >  2 files changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> > index 5da96ea3..d3384c57 100644
> > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > @@ -10,6 +10,7 @@
> >  
> >  #include "gstlibcameraprovider.h"
> >  
> > +#include <libcamera/libcamera.h>
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> >  
> > @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
> >  	static const std::array roles{ StreamRole::VideoRecording };
> >  	g_autoptr(GstCaps) caps = gst_caps_new_empty();
> >  	const gchar *name = camera->id().c_str();
> > +	const int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);
> >  
> >  	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
> >  	if (!config || config->size() != roles.size()) {
> > @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
> >  				       "display-name", name,
> >  				       "caps", caps,
> >  				       "device-class", "Source/Video",
> > +					   "properties", gst_structure_new("device-properties",
> > +							"rotation", G_TYPE_INT, rotation,
>
> What do you think of calling this "mounting-rotation" or "mounting-orientation"
> ? I'm also wondering if we really want to expose libcamera internal enum values
> here. Speed is not a problem, I'd use static strings.
>
> > +							NULL),
> >  				       nullptr));
> >  }
> >  
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 3aca4eed..fe3249ff 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -32,6 +32,7 @@
> >  #include <tuple>
> >  #include <utility>
> >  #include <vector>
> > +#include <sstream>
> >  
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> > @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {
> >  	GstTask *task;
> >  
> >  	gchar *camera_name;
> > +	Orientation orientation;
> >  
> >  	std::atomic<GstEvent *> pending_eos;
> >  
> > @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {
> >  enum {
> >  	PROP_0,
> >  	PROP_CAMERA_NAME,
> > +	PROP_ORIENTATION,
> >  	PROP_LAST
> >  };
> >  
> > @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> >  		gst_libcamera_get_framerate_from_caps(caps, element_caps);
> >  	}
> >  
> > +	/* Set orientation control. */
> > +	state->config_->orientation = self->orientation;
> > +
> >  	/* Validate the configuration. */
> > -	if (state->config_->validate() == CameraConfiguration::Invalid)
> > +	switch(state->config_->validate()) {
> > +	case CameraConfiguration::Valid:
> > +		GST_DEBUG_OBJECT(self, "Camera configuration is valid");
> > +		break;
> > +	case CameraConfiguration::Adjusted:
> > +	{
> > +		/*
> > +		 * The configuration has been adjusted and is now valid.
> > +		 * Parameters may have changed for any stream, and stream configurations may have been removed.
> > +		 * Log the adjusted configuration.
> > +		 */
> > +		for (gsize i = 0; i < state->config_->size(); i++) {
> > +			const StreamConfiguration &cfg = state->config_->at(i);
> > +			GST_WARNING_OBJECT(self, "Camera configuration adjusted for stream %zu: %s", i, cfg.toString().c_str());
> > +		}
> > +		std::ostringstream oss;
> > +		oss << state->config_->orientation;
> > +		GST_WARNING_OBJECT(self, "Camera configuration adjusted orientation: %s", oss.str().c_str());
> > +		self->orientation = state->config_->orientation;
> > +		break;
> > +	}
> > +	case CameraConfiguration::Invalid:
> > +		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> > +				  ("Camera configuration is not supported"),
> > +				  ("CameraConfiguration::validate() returned Invalid"));
> >  		return false;
> > +	}
> >  
> >  	int ret = state->cam_->configure(state->config_.get());
> >  	if (ret) {
> > @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
> >  		g_free(self->camera_name);
> >  		self->camera_name = g_value_dup_string(value);
> >  		break;
> > +	case PROP_ORIENTATION:
> > +		self->orientation = (libcamera::Orientation)g_value_get_enum(value);
> > +		break;
> >  	default:
> >  		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
> >  			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
> >  	case PROP_CAMERA_NAME:
> >  		g_value_set_string(value, self->camera_name);
> >  		break;
> > +	case PROP_ORIENTATION:
> > +		g_value_set_enum(value, (gint)self->orientation);
> > +		break;
> >  	default:
> >  		if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
> >  			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
> >  	gst_element_remove_pad(element, pad);
> >  }
> >  
> > +static GType
> > +gst_libcamera_orientation_get_type()
> > +{
> > +	static GType type = 0;
> > +	static const GEnumValue values[] = {
> > +		{
> > +			static_cast<gint>(libcamera::Orientation::Rotate0),
> > +			"libcamera::Orientation::Rotate0",
> > +			"rotate-0",
> > +		}, {
> > +			static_cast<gint>(libcamera::Orientation::Rotate0Mirror),
> > +			"libcamera::Orientation::Rotate0Mirror",
> > +			"rotate-0-mirror",
> > +		}, {
> > +			static_cast<gint>(libcamera::Orientation::Rotate180),
> > +			"libcamera::Orientation::Rotate180",
> > +			"rotate-180",
> > +		}, {
> > +			static_cast<gint>(libcamera::Orientation::Rotate180Mirror),
> > +			"libcamera::Orientation::Rotate180Mirror",
> > +			"rotate-180-mirror",
> > +		}, {
> > +			static_cast<gint>(libcamera::Orientation::Rotate90Mirror),
> > +			"libcamera::Orientation::Rotate90Mirror",
> > +			"rotate-90-mirror",
> > +		}, {
> > +			static_cast<gint>(libcamera::Orientation::Rotate270),
> > +			"libcamera::Orientation::Rotate270",
> > +			"rotate-270",
> > +		}, {
> > +			static_cast<gint>(libcamera::Orientation::Rotate270Mirror),
> > +			"libcamera::Orientation::Rotate270Mirror",
> > +			"rotate-270-mirror",
> > +		}, {
> > +			static_cast<gint>(libcamera::Orientation::Rotate90),
> > +			"libcamera::Orientation::Rotate90",
> > +			"rotate-90",
> > +		},
> > +		{ 0, nullptr, nullptr }
> > +	};
> > +
> > +	if (!type)
> > +		type = g_enum_register_static("GstLibcameraOrientation", values);
> > +
> > +	return type;
> > +}
>
> So remain to debate is this enum. We have to decide around leaking libcamera API
> into GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation
> enum and names. Its libcamera project choice, be aware that if we move it back
> into GStreamer in the future, this will change, since we try really hard to stop
> leaking project specific enum values.
>

If this is exposed through a gstreamer element, it should use
gstreamer types, doesn't it ? I see no value in leaking libcamera
names into gstreamer, but again, not a gstreamer expert :)

> Nicolas
>
> > +
> >  static void
> >  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >  {
> > @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >  							     | G_PARAM_STATIC_STRINGS));
> >  	g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
> >  
> > +	/* Register the orientation enum type. */
> > +	GType orientation_type = gst_libcamera_orientation_get_type();
> > +	spec = g_param_spec_enum("orientation", "Orientation",
> > +					       "Select the orientation of the camera.",
> > +					       orientation_type,
> > +					       static_cast<gint>(libcamera::Orientation::Rotate0),
> > +					       (GParamFlags)(GST_PARAM_MUTABLE_READY
> > +							     | G_PARAM_CONSTRUCT
> > +							     | G_PARAM_READWRITE
> > +							     | G_PARAM_STATIC_STRINGS));
> > +	g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
> > +
> >  	GstCameraControls::installProperties(object_class, PROP_LAST);
> >  }
> >

Hi Nicolas

On 6/26/25 7:25 PM, Nicolas Dufresne wrote:
> Hi,
>
> Le jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :
>> - rotation: is a static camera property reported
>>    through Camera::properties().
>>    This is exposed in GstLibcameraDevice as an extra property.
>>    You can read it with gst-device-monitor-1.0
>>    or GstDevice::gst_device_get_properties.
>>
>> - orientation: is a configurable parameter of the camera that only
>>    concerns with the actual images rotation.
>>    This is exposed as a GST_PARAM_MUTABLE_READY parameter of
>>    GstLibcameraSrc.
>>    It gets validated via CameraConfiguation::validate().
>>    If the camera doesn't support it and CameraConfiguration::Adjusted
>>    is returned the adjusted configuration will be
>>    logged as a warning.
>> ---
>>   src/gstreamer/gstlibcameraprovider.cpp |  5 ++
>>   src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-
>>   2 files changed, 102 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
>> index 5da96ea3..d3384c57 100644
>> --- a/src/gstreamer/gstlibcameraprovider.cpp
>> +++ b/src/gstreamer/gstlibcameraprovider.cpp
>> @@ -10,6 +10,7 @@
>>   
>>   #include "gstlibcameraprovider.h"
>>   
>> +#include <libcamera/libcamera.h>
>>   #include <libcamera/camera.h>
>>   #include <libcamera/camera_manager.h>
>>   
>> @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>>   	static const std::array roles{ StreamRole::VideoRecording };
>>   	g_autoptr(GstCaps) caps = gst_caps_new_empty();
>>   	const gchar *name = camera->id().c_str();
>> +	const int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);
>>   
>>   	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
>>   	if (!config || config->size() != roles.size()) {
>> @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>>   				       "display-name", name,
>>   				       "caps", caps,
>>   				       "device-class", "Source/Video",
>> +					   "properties", gst_structure_new("device-properties",
>> +							"rotation", G_TYPE_INT, rotation,
> What do you think of calling this "mounting-rotation" or "mounting-orientation"
> ? I'm also wondering if we really want to expose libcamera internal enum values
> here. Speed is not a problem, I'd use static strings.


Mind that properties::rotation is not a enum Orientation. It is a int32_t.

Only way to get libcamera::Orientation enum from 'Rotation' is when you 
make explicit call
to orientationFromRotation(), which has not been done here.

I agree with usage of static human-friendly strings to report the 
mounting-rotation.

>
>> +							NULL),
>>   				       nullptr));
>>   }
>>   
>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
>> index 3aca4eed..fe3249ff 100644
>> --- a/src/gstreamer/gstlibcamerasrc.cpp
>> +++ b/src/gstreamer/gstlibcamerasrc.cpp
>> @@ -32,6 +32,7 @@
>>   #include <tuple>
>>   #include <utility>
>>   #include <vector>
>> +#include <sstream>
>>   
>>   #include <libcamera/camera.h>
>>   #include <libcamera/camera_manager.h>
>> @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {
>>   	GstTask *task;
>>   
>>   	gchar *camera_name;
>> +	Orientation orientation;
>>   
>>   	std::atomic<GstEvent *> pending_eos;
>>   
>> @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {
>>   enum {
>>   	PROP_0,
>>   	PROP_CAMERA_NAME,
>> +	PROP_ORIENTATION,
>>   	PROP_LAST
>>   };
>>   
>> @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>>   		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>>   	}
>>   
>> +	/* Set orientation control. */
>> +	state->config_->orientation = self->orientation;
>> +
>>   	/* Validate the configuration. */
>> -	if (state->config_->validate() == CameraConfiguration::Invalid)
>> +	switch(state->config_->validate()) {
>> +	case CameraConfiguration::Valid:
>> +		GST_DEBUG_OBJECT(self, "Camera configuration is valid");
>> +		break;
>> +	case CameraConfiguration::Adjusted:
>> +	{
>> +		/*
>> +		 * The configuration has been adjusted and is now valid.
>> +		 * Parameters may have changed for any stream, and stream configurations may have been removed.
>> +		 * Log the adjusted configuration.
>> +		 */
>> +		for (gsize i = 0; i < state->config_->size(); i++) {
>> +			const StreamConfiguration &cfg = state->config_->at(i);
>> +			GST_WARNING_OBJECT(self, "Camera configuration adjusted for stream %zu: %s", i, cfg.toString().c_str());
>> +		}
>> +		std::ostringstream oss;
>> +		oss << state->config_->orientation;
>> +		GST_WARNING_OBJECT(self, "Camera configuration adjusted orientation: %s", oss.str().c_str());
>> +		self->orientation = state->config_->orientation;
>> +		break;
>> +	}
>> +	case CameraConfiguration::Invalid:
>> +		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
>> +				  ("Camera configuration is not supported"),
>> +				  ("CameraConfiguration::validate() returned Invalid"));
>>   		return false;
>> +	}
>>   
>>   	int ret = state->cam_->configure(state->config_.get());
>>   	if (ret) {
>> @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
>>   		g_free(self->camera_name);
>>   		self->camera_name = g_value_dup_string(value);
>>   		break;
>> +	case PROP_ORIENTATION:
>> +		self->orientation = (libcamera::Orientation)g_value_get_enum(value);
>> +		break;
>>   	default:
>>   		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
>>   			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
>> @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
>>   	case PROP_CAMERA_NAME:
>>   		g_value_set_string(value, self->camera_name);
>>   		break;
>> +	case PROP_ORIENTATION:
>> +		g_value_set_enum(value, (gint)self->orientation);
>> +		break;
>>   	default:
>>   		if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
>>   			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
>> @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
>>   	gst_element_remove_pad(element, pad);
>>   }
>>   
>> +static GType
>> +gst_libcamera_orientation_get_type()
>> +{
>> +	static GType type = 0;
>> +	static const GEnumValue values[] = {
>> +		{
>> +			static_cast<gint>(libcamera::Orientation::Rotate0),
>> +			"libcamera::Orientation::Rotate0",
>> +			"rotate-0",
>> +		}, {
>> +			static_cast<gint>(libcamera::Orientation::Rotate0Mirror),
>> +			"libcamera::Orientation::Rotate0Mirror",
>> +			"rotate-0-mirror",
>> +		}, {
>> +			static_cast<gint>(libcamera::Orientation::Rotate180),
>> +			"libcamera::Orientation::Rotate180",
>> +			"rotate-180",
>> +		}, {
>> +			static_cast<gint>(libcamera::Orientation::Rotate180Mirror),
>> +			"libcamera::Orientation::Rotate180Mirror",
>> +			"rotate-180-mirror",
>> +		}, {
>> +			static_cast<gint>(libcamera::Orientation::Rotate90Mirror),
>> +			"libcamera::Orientation::Rotate90Mirror",
>> +			"rotate-90-mirror",
>> +		}, {
>> +			static_cast<gint>(libcamera::Orientation::Rotate270),
>> +			"libcamera::Orientation::Rotate270",
>> +			"rotate-270",
>> +		}, {
>> +			static_cast<gint>(libcamera::Orientation::Rotate270Mirror),
>> +			"libcamera::Orientation::Rotate270Mirror",
>> +			"rotate-270-mirror",
>> +		}, {
>> +			static_cast<gint>(libcamera::Orientation::Rotate90),
>> +			"libcamera::Orientation::Rotate90",
>> +			"rotate-90",
>> +		},
>> +		{ 0, nullptr, nullptr }
>> +	};
>> +
>> +	if (!type)
>> +		type = g_enum_register_static("GstLibcameraOrientation", values);
>> +
>> +	return type;
>> +}
> So remain to debate is this enum. We have to decide around leaking libcamera API
> into GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation
> enum and names. Its libcamera project choice, be aware that if we move it back
> into GStreamer in the future, this will change, since we try really hard to stop
> leaking project specific enum values.
>
> Nicolas
>
>> +
>>   static void
>>   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>>   {
>> @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>>   							     | G_PARAM_STATIC_STRINGS));
>>   	g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
>>   
>> +	/* Register the orientation enum type. */
>> +	GType orientation_type = gst_libcamera_orientation_get_type();
>> +	spec = g_param_spec_enum("orientation", "Orientation",
>> +					       "Select the orientation of the camera.",
>> +					       orientation_type,
>> +					       static_cast<gint>(libcamera::Orientation::Rotate0),
>> +					       (GParamFlags)(GST_PARAM_MUTABLE_READY
>> +							     | G_PARAM_CONSTRUCT
>> +							     | G_PARAM_READWRITE
>> +							     | G_PARAM_STATIC_STRINGS));
>> +	g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
>> +
>>   	GstCameraControls::installProperties(object_class, PROP_LAST);
>>   }
>>