gstreamer: Adding property to control orientation

23628 diff mbox series

"orientation" parameter added to the libcamerasrc
element to control the orientation of the camera feed.

GST_PARAM_MUTABLE_READY allows the parameter to be changed
only when pipeline is in the READY state

The parameter is fed into the existing
CameraConfiguration orientation property

This allows users to specify the rotation of the video stream,
enhancing flexibility in camera applications.
---
 src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Hi Giacomo - thanks for the patch. The functionality looks good to me, but I have a couple of 
code-style nitpicks:

On 23/06/2025 17:35, Giacomo Cappellini wrote:
> "orientation" parameter added to the libcamerasrc
> element to control the orientation of the camera feed.
>
> GST_PARAM_MUTABLE_READY allows the parameter to be changed
> only when pipeline is in the READY state
I would drop this sentence from the commit message and add it as a comment above the line of code.
>
> The parameter is fed into the existing
> CameraConfiguration orientation property
>
> This allows users to specify the rotation of the video stream,
> enhancing flexibility in camera applications.
> ---


The commit message also needs a "Signed-off-by" tag; see the linux kernel docs [1] for an 
explanation of the meaning. You can add the tag automatically to a commit message using "git commit -s"


[1] 
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

>   src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
>   1 file changed, 72 insertions(+)
>
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 51ba8b67..b07156af 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
>   	GstTask *task;
>   
>   	gchar *camera_name;
> +	Orientation orientation;
If you're using a type from a header, ideally include it explicitly (so include 
#<libcamera/orientation.h> in this case).
>   
>   	std::atomic<GstEvent *> pending_eos;
>   
> @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
>   enum {
>   	PROP_0,
>   	PROP_CAMERA_NAME,
> +	PROP_ORIENTATION,
>   	PROP_LAST
>   };
>   
> @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>   		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>   	}
>   
> +	// print state->orientation for debugging purposes
You can drop this comment - the line below is clear enough that it's unnecessary.
> +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
Empty line here please.
> +	/* Set the orientation control. */
> +	state->config_->orientation = self->orientation;
> +
>   	/* Validate the configuration. */
>   	if (state->config_->validate() == CameraConfiguration::Invalid)
>   		return false;
> @@ -926,6 +933,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 +955,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 +1133,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;
> +}
I think I prefer a global array of GEnumValues personally, but I'll let others weigh in on that one.
> +
>   static void
>   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>   {
> @@ -1154,6 +1214,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));
Could you align the parameters to the opening parenthesis please?
> +	g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
> +
>   	GstCameraControls::installProperties(object_class, PROP_LAST);
>   }
>

CC'ing Nicolas.

On Tue, Jun 24, 2025 at 10:28:45PM +0100, Daniel Scally wrote:
> Hi Giacomo - thanks for the patch. The functionality looks good to me,
> but I have a couple of code-style nitpicks:

The commit message subject and body is typically written in the
imperative mood, so the subject would be

gstreamer: Add property to control orientation

> On 23/06/2025 17:35, Giacomo Cappellini wrote:
> > "orientation" parameter added to the libcamerasrc
> > element to control the orientation of the camera feed.

And this would be

Add a "orientation" parameter to the libcamerasrc element to control the
orientation of the camera feed.

> > GST_PARAM_MUTABLE_READY allows the parameter to be changed
> > only when pipeline is in the READY state
>
> I would drop this sentence from the commit message and add it as a
> comment above the line of code.
>
> > The parameter is fed into the existing
> > CameraConfiguration orientation property

Missing period at the end of the sentence.

> >
> > This allows users to specify the rotation of the video stream,
> > enhancing flexibility in camera applications.
> > ---
> 
> The commit message also needs a "Signed-off-by" tag; see the linux kernel docs [1] for an 

Or the libcamera doc :-)

https://libcamera.org/contributing.html#submitting-patches

> explanation of the meaning. You can add the tag automatically to a commit message using "git commit -s"
> 
> [1] https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
> 
> >   src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
> >   1 file changed, 72 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 51ba8b67..b07156af 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
> >   	GstTask *task;
> >   
> >   	gchar *camera_name;
> > +	Orientation orientation;
>
> If you're using a type from a header, ideally include it explicitly (so include 
> #<libcamera/orientation.h> in this case).
>
> >   
> >   	std::atomic<GstEvent *> pending_eos;
> >   
> > @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
> >   enum {
> >   	PROP_0,
> >   	PROP_CAMERA_NAME,
> > +	PROP_ORIENTATION,
> >   	PROP_LAST
> >   };
> >   
> > @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> >   		gst_libcamera_get_framerate_from_caps(caps, element_caps);
> >   	}
> >   
> > +	// print state->orientation for debugging purposes
>
> You can drop this comment - the line below is clear enough that it's unnecessary.

And is the debug print useful, or was it only used during development ?

> > +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
>
> Empty line here please.
>
> > +	/* Set the orientation control. */

s/ control// as it's not a control.

> > +	state->config_->orientation = self->orientation;
> > +
> >   	/* Validate the configuration. */
> >   	if (state->config_->validate() == CameraConfiguration::Invalid)
> >   		return false;
> > @@ -926,6 +933,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);

		self->orientation = static_cast<libcamera::Orientation>(g_value_get_enum(value));

as it's C++.

> > +		break;
> >   	default:
> >   		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
> >   			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -945,6 +955,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 +1133,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;
> > +}
>
> I think I prefer a global array of GEnumValues personally, but I'll
> let others weigh in on that one.

Do you mean outside of the function ? As it's tightly related to the
registration of the enum, I think I'd keep it here.

> > +
> >   static void
> >   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >   {
> > @@ -1154,6 +1214,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));
>
> Could you align the parameters to the opening parenthesis please?

That would be

	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 Giacomo,

Thank you for the patch.

On 6/23/25 10:05 PM, Giacomo Cappellini wrote:
> "orientation" parameter added to the libcamerasrc
> element to control the orientation of the camera feed.
>
> GST_PARAM_MUTABLE_READY allows the parameter to be changed
> only when pipeline is in the READY state
>
> The parameter is fed into the existing
> CameraConfiguration orientation property
>
> This allows users to specify the rotation of the video stream,
> enhancing flexibility in camera applications.
> ---
>   src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
>   1 file changed, 72 insertions(+)


I believe we need to implement the GstVideoDirection interface[1] for 
libcamerasrc, and map GstVideoOrientationMethod [2] to 
libcamera::Orientation values and pass it to the 
CameraConfiguration::orientation.

Here is a quick snippet to implement the interface:


diff --git a/src/gstreamer/gstlibcamerasrc.cpp 
b/src/gstreamer/gstlibcamerasrc.cpp
index 51ba8b67..2577fb5f 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -163,9 +163,15 @@ enum {
  static void gst_libcamera_src_child_proxy_init(gpointer g_iface,
                                                gpointer iface_data);

+
+static void
+gst_libcamera_src_video_direction_init(GstVideoDirectionInterface *iface);
+
  G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, 
GST_TYPE_ELEMENT,
G_IMPLEMENT_INTERFACE(GST_TYPE_CHILD_PROXY,
- gst_libcamera_src_child_proxy_init)
+ gst_libcamera_src_child_proxy_init);
+ G_IMPLEMENT_INTERFACE(GST_TYPE_VIDEO_DIRECTION,
+ gst_libcamera_src_video_direction_init);
                         GST_DEBUG_CATEGORY_INIT(source_debug, 
"libcamerasrc", 0,
                                                 "libcamera Source"))

@@ -1186,3 +1192,10 @@ gst_libcamera_src_child_proxy_init(gpointer 
g_iface, [[maybe_unused]] gpointer i
         iface->get_child_by_index = 
gst_libcamera_src_child_proxy_get_child_by_index;
         iface->get_children_count = 
gst_libcamera_src_child_proxy_get_children_count;
  }
+
+static void
+gst_libcamera_src_video_direction_init([[maybe_unused]] 
GstVideoDirectionInterface *iface)
+{
+       /* We implement the video-direction property */
+
+}

[1] : 
https://gstreamer.freedesktop.org/documentation/video/gstvideodirection.html?gi-language=c

[2]: 
https://gstreamer.freedesktop.org/documentation/video/gstvideo.html?gi-language=c#GstVideoOrientationMethod

> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 51ba8b67..b07156af 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
>   	GstTask *task;
>   
>   	gchar *camera_name;
> +	Orientation orientation;


This should be GstVideoOrientationMethod

>   
>   	std::atomic<GstEvent *> pending_eos;
>   
> @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
>   enum {
>   	PROP_0,
>   	PROP_CAMERA_NAME,
> +	PROP_ORIENTATION,
>   	PROP_LAST
>   };
>   
> @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>   		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>   	}
>   
> +	// print state->orientation for debugging purposes
> +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
> +	/* Set the orientation control. */
> +	state->config_->orientation = self->orientation;
> +


This can be

     state->config_->orientation = 
gst_libcamera_src_gst_orientation_method_to_orientation(self->orientation);


gst_libcamera_src_gst_orientation_method_to_orientation() returns the 
corresponding libcamera::Orientation from GstOrientationMethod value.  I 
hope you get the idea.


>   	/* Validate the configuration. */
>   	if (state->config_->validate() == CameraConfiguration::Invalid)
>   		return false;
> @@ -926,6 +933,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 +955,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 +1133,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;
> +}
> +
>   static void
>   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>   {
> @@ -1154,6 +1214,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),
And use GST_VIDEO_ORIENTATION_IDENTITY here.
> +					       (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,

adding my own review on top of other's.

Le lundi 23 juin 2025 à 18:35 +0200, Giacomo Cappellini a écrit :
> "orientation" parameter added to the libcamerasrc
> element to control the orientation of the camera feed.
> 
> GST_PARAM_MUTABLE_READY allows the parameter to be changed
> only when pipeline is in the READY state
> 
> The parameter is fed into the existing
> CameraConfiguration orientation property
> 
> This allows users to specify the rotation of the video stream,
> enhancing flexibility in camera applications.

Since this is 90 degrees and flips only, it should be clarified, I saw
some comment about free form rotation already.

Some clarification need in regard to this vs camera physical orientation,
e.g. if a sensor has been mounted with such a 90 degree rotation from
its reference for a specific device.

> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 51ba8b67..b07156af 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
>  	GstTask *task;
>  
>  	gchar *camera_name;
> +	Orientation orientation;
>  
>  	std::atomic<GstEvent *> pending_eos;
>  
> @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
>  enum {
>  	PROP_0,
>  	PROP_CAMERA_NAME,
> +	PROP_ORIENTATION,
>  	PROP_LAST
>  };
>  
> @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>  		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>  	}
>  
> +	// print state->orientation for debugging purposes
> +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
> +	/* Set the orientation control. */
> +	state->config_->orientation = self->orientation;
> +
>  	/* Validate the configuration. */
>  	if (state->config_->validate() == CameraConfiguration::Invalid)
>  		return false;
> @@ -926,6 +933,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 +955,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 +1133,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),

When you write a GStreamer application, you usually import and link GStreamer libraries,
but don't link or import libcamera. For this reason, it would be nicer to use the values
from GST_VIDEO_ORIENTATION_*. This enum has been made global in GStreamer for that
purpose, but it does not mean you have to support all of it, or forced to use its naming.

Though, not using the name naming as videoflip, glvideoflip, glimagesink etc. it very
confusing. Here's my suggestion, change this to (C++iffy this):

static const GEnumValue rotate_methods[] = {
  {GST_VIDEO_ORIENTATION_IDENTITY, "Identity (no rotation)", "none"},
  {GST_VIDEO_ORIENTATION_90R, "Rotate clockwise 90 degrees", "clockwise"},
  {GST_VIDEO_ORIENTATION_180, "Rotate 180 degrees", "rotate-180"},
  {GST_VIDEO_ORIENTATION_90L, "Rotate counter-clockwise 90 degrees",
      "counterclockwise"},
  {GST_VIDEO_ORIENTATION_HORIZ, "Flip horizontally", "horizontal-flip"},
  {GST_VIDEO_ORIENTATION_VERT, "Flip vertically", "vertical-flip"},
  {GST_VIDEO_ORIENTATION_UL_LR,
      "Flip across upper left/lower right diagonal", "upper-left-diagonal"},
  {GST_VIDEO_ORIENTATION_UR_LL,
      "Flip across upper right/lower left diagonal", "upper-right-diagonal"},
// This can be added later if we think its useful/usable even though usually only
// flips are supported
//  {GST_VIDEO_ORIENTATION_AUTO,
//      "Select rotate method based on image-orientation tag", "automatic"},
  {0, NULL, NULL},
};

struct rotate_item {
	GstVideoOrientation gst;
	libcamera::Orientation lc;
};

static const rotate_enum_map[] = {
	{ GST_VIDEO_ORIENTATION_IDENTITY, libcamera::Orientation::Rotate0},
	...
};

The orientation will be bound checked already by the GOBject property machinery.


> +			"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 +1214,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.",

This needs a better description, and a better name. For me, the orientation of a camera is
basically the fixed (physical) orientation against a device specific reference point, not
omething you can select. Even PTZ cameras don't have that kind of control physically.

While dedicated elements such as videoflip or glvideoflip just call this method, a good 
name to follow could be glimagesink which calls this rotate-methode.

Nicolas

> +					       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 Umang,

Le mercredi 25 juin 2025 à 14:14 +0530, Umang Jain a écrit :
> Hi Giacomo,
> 
> Thank you for the patch.
> 
> On 6/23/25 10:05 PM, Giacomo Cappellini wrote:
> > "orientation" parameter added to the libcamerasrc
> > element to control the orientation of the camera feed.
> > 
> > GST_PARAM_MUTABLE_READY allows the parameter to be changed
> > only when pipeline is in the READY state
> > 
> > The parameter is fed into the existing
> > CameraConfiguration orientation property
> > 
> > This allows users to specify the rotation of the video stream,
> > enhancing flexibility in camera applications.
> > ---
> >   src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
> >   1 file changed, 72 insertions(+)
> 
> 
> I believe we need to implement the GstVideoDirection interface[1] for 
> libcamerasrc, and map GstVideoOrientationMethod [2] to 
> libcamera::Orientation values and pass it to the 
> CameraConfiguration::orientation.

I'm not entirely certain that implementing the interface is a good idea. We
don't have any software fallback, so we can't guaranty the effect. I don't
know a lot of users for this interface, and may have a hard time verifying
the existing application needs.

> 
> Here is a quick snippet to implement the interface:
> 
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp 
> b/src/gstreamer/gstlibcamerasrc.cpp
> index 51ba8b67..2577fb5f 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -163,9 +163,15 @@ enum {
>   static void gst_libcamera_src_child_proxy_init(gpointer g_iface,
>                                                 gpointer iface_data);
> 
> +
> +static void
> +gst_libcamera_src_video_direction_init(GstVideoDirectionInterface *iface);
> +
>   G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, 
> GST_TYPE_ELEMENT,
> G_IMPLEMENT_INTERFACE(GST_TYPE_CHILD_PROXY,
> - gst_libcamera_src_child_proxy_init)
> + gst_libcamera_src_child_proxy_init);
> + G_IMPLEMENT_INTERFACE(GST_TYPE_VIDEO_DIRECTION,
> + gst_libcamera_src_video_direction_init);
>                          GST_DEBUG_CATEGORY_INIT(source_debug, 
> "libcamerasrc", 0,
>                                                  "libcamera Source"))
> 
> @@ -1186,3 +1192,10 @@ gst_libcamera_src_child_proxy_init(gpointer 
> g_iface, [[maybe_unused]] gpointer i
>          iface->get_child_by_index = 
> gst_libcamera_src_child_proxy_get_child_by_index;
>          iface->get_children_count = 
> gst_libcamera_src_child_proxy_get_children_count;
>   }
> +
> +static void
> +gst_libcamera_src_video_direction_init([[maybe_unused]] 
> GstVideoDirectionInterface *iface)
> +{
> +       /* We implement the video-direction property */
> +
> +}
> 
> [1] : 
> https://gstreamer.freedesktop.org/documentation/video/gstvideodirection.html?gi-language=c
> 
> [2]: 
> https://gstreamer.freedesktop.org/documentation/video/gstvideo.html?gi-language=c#GstVideoOrientationMethod
> 
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 51ba8b67..b07156af 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
> >   	GstTask *task;
> >   
> >   	gchar *camera_name;
> > +	Orientation orientation;
> 
> 
> This should be GstVideoOrientationMethod
> 
> >   
> >   	std::atomic<GstEvent *> pending_eos;
> >   
> > @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
> >   enum {
> >   	PROP_0,
> >   	PROP_CAMERA_NAME,
> > +	PROP_ORIENTATION,
> >   	PROP_LAST
> >   };
> >   
> > @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> >   		gst_libcamera_get_framerate_from_caps(caps, element_caps);
> >   	}
> >   
> > +	// print state->orientation for debugging purposes
> > +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
> > +	/* Set the orientation control. */
> > +	state->config_->orientation = self->orientation;
> > +
> 
> 
> This can be
> 
>      state->config_->orientation = 
> gst_libcamera_src_gst_orientation_method_to_orientation(self->orientation);
> 
> 
> gst_libcamera_src_gst_orientation_method_to_orientation() returns the 
> corresponding libcamera::Orientation from GstOrientationMethod value.  I 
> hope you get the idea.
> 
> 
> >   	/* Validate the configuration. */
> >   	if (state->config_->validate() == CameraConfiguration::Invalid)
> >   		return false;
> > @@ -926,6 +933,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 +955,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 +1133,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;
> > +}
> > +
> >   static void
> >   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >   {
> > @@ -1154,6 +1214,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),
> And use GST_VIDEO_ORIENTATION_IDENTITY here.
> > +					       (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 Wed, Jun 25, 2025 at 09:02:24AM -0400, Nicolas Dufresne wrote:
> Hi,
>
> adding my own review on top of other's.
>
> Le lundi 23 juin 2025 à 18:35 +0200, Giacomo Cappellini a écrit :
> > "orientation" parameter added to the libcamerasrc
> > element to control the orientation of the camera feed.
> >
> > GST_PARAM_MUTABLE_READY allows the parameter to be changed
> > only when pipeline is in the READY state
> >
> > The parameter is fed into the existing
> > CameraConfiguration orientation property
> >
> > This allows users to specify the rotation of the video stream,
> > enhancing flexibility in camera applications.
>
> Since this is 90 degrees and flips only, it should be clarified, I saw
> some comment about free form rotation already.
>
> Some clarification need in regard to this vs camera physical orientation,
> e.g. if a sensor has been mounted with such a 90 degree rotation from
> its reference for a specific device.

Just to clarify, from a libcamera point of view, the mounting rotation
is a physical property of the camera reported through
properties::Rotation, while CameraConfiguration::orientation instead
is only about the orientation of the images you get. If you ask for a
0deg (or 180 for that matter) rotation you'll get non-rotated images
(or 180 degrees rotated images), regardless of the mounting
orientation as libcamera corrects that for you (provided you've asked
a rotation that can be achieved on the system, usually only 0/180
through sensor flips).
https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/sensor/camera_sensor_legacy.cpp#n932

To summarize:

- mounting rotation is a static camera properties reported through
  Camera::properties(). I'm not sure how you want/should expose this
  thtough the gstreamer API.

- orientation: is a configurable parameters of the camera that only
  concerns with the actual images rotation. If you ask X you'll get X,
  provided the camera pipeline can do that.

Hope it clarifies things in this regard.

Thanks
  j

>
> > ---
> >  src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 51ba8b67..b07156af 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
> >  	GstTask *task;
> >  
> >  	gchar *camera_name;
> > +	Orientation orientation;
> >  
> >  	std::atomic<GstEvent *> pending_eos;
> >  
> > @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
> >  enum {
> >  	PROP_0,
> >  	PROP_CAMERA_NAME,
> > +	PROP_ORIENTATION,
> >  	PROP_LAST
> >  };
> >  
> > @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> >  		gst_libcamera_get_framerate_from_caps(caps, element_caps);
> >  	}
> >  
> > +	// print state->orientation for debugging purposes
> > +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
> > +	/* Set the orientation control. */
> > +	state->config_->orientation = self->orientation;
> > +
> >  	/* Validate the configuration. */
> >  	if (state->config_->validate() == CameraConfiguration::Invalid)
> >  		return false;
> > @@ -926,6 +933,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 +955,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 +1133,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),
>
> When you write a GStreamer application, you usually import and link GStreamer libraries,
> but don't link or import libcamera. For this reason, it would be nicer to use the values
> from GST_VIDEO_ORIENTATION_*. This enum has been made global in GStreamer for that
> purpose, but it does not mean you have to support all of it, or forced to use its naming.
>
> Though, not using the name naming as videoflip, glvideoflip, glimagesink etc. it very
> confusing. Here's my suggestion, change this to (C++iffy this):
>
> static const GEnumValue rotate_methods[] = {
>   {GST_VIDEO_ORIENTATION_IDENTITY, "Identity (no rotation)", "none"},
>   {GST_VIDEO_ORIENTATION_90R, "Rotate clockwise 90 degrees", "clockwise"},
>   {GST_VIDEO_ORIENTATION_180, "Rotate 180 degrees", "rotate-180"},
>   {GST_VIDEO_ORIENTATION_90L, "Rotate counter-clockwise 90 degrees",
>       "counterclockwise"},
>   {GST_VIDEO_ORIENTATION_HORIZ, "Flip horizontally", "horizontal-flip"},
>   {GST_VIDEO_ORIENTATION_VERT, "Flip vertically", "vertical-flip"},
>   {GST_VIDEO_ORIENTATION_UL_LR,
>       "Flip across upper left/lower right diagonal", "upper-left-diagonal"},
>   {GST_VIDEO_ORIENTATION_UR_LL,
>       "Flip across upper right/lower left diagonal", "upper-right-diagonal"},
> // This can be added later if we think its useful/usable even though usually only
> // flips are supported
> //  {GST_VIDEO_ORIENTATION_AUTO,
> //      "Select rotate method based on image-orientation tag", "automatic"},
>   {0, NULL, NULL},
> };
>
> struct rotate_item {
> 	GstVideoOrientation gst;
> 	libcamera::Orientation lc;
> };
>
> static const rotate_enum_map[] = {
> 	{ GST_VIDEO_ORIENTATION_IDENTITY, libcamera::Orientation::Rotate0},
> 	...
> };
>
> The orientation will be bound checked already by the GOBject property machinery.
>
>
> > +			"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 +1214,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.",
>
> This needs a better description, and a better name. For me, the orientation of a camera is
> basically the fixed (physical) orientation against a device specific reference point, not
> omething you can select. Even PTZ cameras don't have that kind of control physically.
>
> While dedicated elements such as videoflip or glvideoflip just call this method, a good
> name to follow could be glimagesink which calls this rotate-methode.
>
> Nicolas
>
> > +					       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);
> >  }
> >

On Wed, Jun 25, 2025 at 09:02:24AM -0400, Nicolas Dufresne wrote:
> Hi,
>
> adding my own review on top of other's.
>
> Le lundi 23 juin 2025 à 18:35 +0200, Giacomo Cappellini a écrit :
> > "orientation" parameter added to the libcamerasrc
> > element to control the orientation of the camera feed.
> >
> > GST_PARAM_MUTABLE_READY allows the parameter to be changed
> > only when pipeline is in the READY state
> >
> > The parameter is fed into the existing
> > CameraConfiguration orientation property
> >
> > This allows users to specify the rotation of the video stream,
> > enhancing flexibility in camera applications.
>
> Since this is 90 degrees and flips only, it should be clarified, I saw
> some comment about free form rotation already.
>
> Some clarification need in regard to this vs camera physical orientation,
> e.g. if a sensor has been mounted with such a 90 degree rotation from
> its reference for a specific device.
>
> > ---
> >  src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 51ba8b67..b07156af 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
> >  	GstTask *task;
> >  
> >  	gchar *camera_name;
> > +	Orientation orientation;
> >  
> >  	std::atomic<GstEvent *> pending_eos;
> >  
> > @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
> >  enum {
> >  	PROP_0,
> >  	PROP_CAMERA_NAME,
> > +	PROP_ORIENTATION,
> >  	PROP_LAST
> >  };
> >  
> > @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> >  		gst_libcamera_get_framerate_from_caps(caps, element_caps);
> >  	}
> >  
> > +	// print state->orientation for debugging purposes
> > +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
> > +	/* Set the orientation control. */
> > +	state->config_->orientation = self->orientation;
> > +
> >  	/* Validate the configuration. */
> >  	if (state->config_->validate() == CameraConfiguration::Invalid)
> >  		return false;
> > @@ -926,6 +933,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 +955,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 +1133,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),
>
> When you write a GStreamer application, you usually import and link GStreamer libraries,
> but don't link or import libcamera. For this reason, it would be nicer to use the values
> from GST_VIDEO_ORIENTATION_*. This enum has been made global in GStreamer for that
> purpose, but it does not mean you have to support all of it, or forced to use its naming.
>
> Though, not using the name naming as videoflip, glvideoflip, glimagesink etc. it very
> confusing. Here's my suggestion, change this to (C++iffy this):
>
> static const GEnumValue rotate_methods[] = {
>   {GST_VIDEO_ORIENTATION_IDENTITY, "Identity (no rotation)", "none"},
>   {GST_VIDEO_ORIENTATION_90R, "Rotate clockwise 90 degrees", "clockwise"},
>   {GST_VIDEO_ORIENTATION_180, "Rotate 180 degrees", "rotate-180"},
>   {GST_VIDEO_ORIENTATION_90L, "Rotate counter-clockwise 90 degrees",
>       "counterclockwise"},
>   {GST_VIDEO_ORIENTATION_HORIZ, "Flip horizontally", "horizontal-flip"},
>   {GST_VIDEO_ORIENTATION_VERT, "Flip vertically", "vertical-flip"},
>   {GST_VIDEO_ORIENTATION_UL_LR,
>       "Flip across upper left/lower right diagonal", "upper-left-diagonal"},
>   {GST_VIDEO_ORIENTATION_UR_LL,
>       "Flip across upper right/lower left diagonal", "upper-right-diagonal"},
> // This can be added later if we think its useful/usable even though usually only
> // flips are supported
> //  {GST_VIDEO_ORIENTATION_AUTO,
> //      "Select rotate method based on image-orientation tag", "automatic"},
>   {0, NULL, NULL},
> };
>
> struct rotate_item {
> 	GstVideoOrientation gst;
> 	libcamera::Orientation lc;
> };
>
> static const rotate_enum_map[] = {
> 	{ GST_VIDEO_ORIENTATION_IDENTITY, libcamera::Orientation::Rotate0},
> 	...
> };
>
> The orientation will be bound checked already by the GOBject property machinery.
>
>
> > +			"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 +1214,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.",
>
> This needs a better description, and a better name. For me, the orientation of a camera is
> basically the fixed (physical) orientation against a device specific reference point, not
> omething you can select. Even PTZ cameras don't have that kind of control physically.

In facts, this is more precisely described as "orientation of the
images" for the reasons explained in my just-sent previous reply.

>
> While dedicated elements such as videoflip or glvideoflip just call this method, a good
> name to follow could be glimagesink which calls this rotate-methode.
>

Although, if you have something more gstreamer-specific to use, that's
fine. My main point here is that CameraConfiguration::orientation
controls the orientation of the images you receive, libcamera takes
into account the sensor rotation for you when computing the transform
to apply to get images oriented as you have asked.

As said, in the case the pipeline doesn't support the orientation
you've asked for, it will fall back to no-corrections (you'll get
images back that match the camera mounting rotation). You should
probably catch this validating that CameraConfiguration::orientation
has not been Adjusted by CameraConfiguration::validate() and act
accordingly to inform other elements further down the pipeline, but
that's very gstreamer specific so I don't know if this is required or
not.


> Nicolas
>
> > +					       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/25/25 6:35 PM, Nicolas Dufresne wrote:
> Hi Umang,
>
> Le mercredi 25 juin 2025 à 14:14 +0530, Umang Jain a écrit :
>> Hi Giacomo,
>>
>> Thank you for the patch.
>>
>> On 6/23/25 10:05 PM, Giacomo Cappellini wrote:
>>> "orientation" parameter added to the libcamerasrc
>>> element to control the orientation of the camera feed.
>>>
>>> GST_PARAM_MUTABLE_READY allows the parameter to be changed
>>> only when pipeline is in the READY state
>>>
>>> The parameter is fed into the existing
>>> CameraConfiguration orientation property
>>>
>>> This allows users to specify the rotation of the video stream,
>>> enhancing flexibility in camera applications.
>>> ---
>>>    src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++++++++++
>>>    1 file changed, 72 insertions(+)
>>
>> I believe we need to implement the GstVideoDirection interface[1] for
>> libcamerasrc, and map GstVideoOrientationMethod [2] to
>> libcamera::Orientation values and pass it to the
>> CameraConfiguration::orientation.
> I'm not entirely certain that implementing the interface is a good idea. We
> don't have any software fallback, so we can't guaranty the effect. I don't
> know a lot of users for this interface, and may have a hard time verifying
> the existing application needs.


My observations are same as ours when it comes to usage of the 
GstVideoDirection interface in other elements. Maybe that's the reason 
that the interface is not very well used, is because elements probably 
don't implement all the enum values or have a fallback.

So, indeed we can drop the interface part.


>
>> Here is a quick snippet to implement the interface:
>>
>>
>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp
>> b/src/gstreamer/gstlibcamerasrc.cpp
>> index 51ba8b67..2577fb5f 100644
>> --- a/src/gstreamer/gstlibcamerasrc.cpp
>> +++ b/src/gstreamer/gstlibcamerasrc.cpp
>> @@ -163,9 +163,15 @@ enum {
>>    static void gst_libcamera_src_child_proxy_init(gpointer g_iface,
>>                                                  gpointer iface_data);
>>
>> +
>> +static void
>> +gst_libcamera_src_video_direction_init(GstVideoDirectionInterface *iface);
>> +
>>    G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src,
>> GST_TYPE_ELEMENT,
>> G_IMPLEMENT_INTERFACE(GST_TYPE_CHILD_PROXY,
>> - gst_libcamera_src_child_proxy_init)
>> + gst_libcamera_src_child_proxy_init);
>> + G_IMPLEMENT_INTERFACE(GST_TYPE_VIDEO_DIRECTION,
>> + gst_libcamera_src_video_direction_init);
>>                           GST_DEBUG_CATEGORY_INIT(source_debug,
>> "libcamerasrc", 0,
>>                                                   "libcamera Source"))
>>
>> @@ -1186,3 +1192,10 @@ gst_libcamera_src_child_proxy_init(gpointer
>> g_iface, [[maybe_unused]] gpointer i
>>           iface->get_child_by_index =
>> gst_libcamera_src_child_proxy_get_child_by_index;
>>           iface->get_children_count =
>> gst_libcamera_src_child_proxy_get_children_count;
>>    }
>> +
>> +static void
>> +gst_libcamera_src_video_direction_init([[maybe_unused]]
>> GstVideoDirectionInterface *iface)
>> +{
>> +       /* We implement the video-direction property */
>> +
>> +}
>>
>> [1] :
>> https://gstreamer.freedesktop.org/documentation/video/gstvideodirection.html?gi-language=c
>>
>> [2]:
>> https://gstreamer.freedesktop.org/documentation/video/gstvideo.html?gi-language=c#GstVideoOrientationMethod
>>
>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
>>> index 51ba8b67..b07156af 100644
>>> --- a/src/gstreamer/gstlibcamerasrc.cpp
>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp
>>> @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
>>>    	GstTask *task;
>>>    
>>>    	gchar *camera_name;
>>> +	Orientation orientation;
>>
>> This should be GstVideoOrientationMethod
>>
>>>    
>>>    	std::atomic<GstEvent *> pending_eos;
>>>    
>>> @@ -157,6 +158,7 @@ struct _GstLibcameraSrc {
>>>    enum {
>>>    	PROP_0,
>>>    	PROP_CAMERA_NAME,
>>> +	PROP_ORIENTATION,
>>>    	PROP_LAST
>>>    };
>>>    
>>> @@ -616,6 +618,11 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>>>    		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>>>    	}
>>>    
>>> +	// print state->orientation for debugging purposes
>>> +	GST_DEBUG_OBJECT(self, "Orientation: %d", (int)self->orientation);
>>> +	/* Set the orientation control. */
>>> +	state->config_->orientation = self->orientation;
>>> +
>>
>> This can be
>>
>>       state->config_->orientation =
>> gst_libcamera_src_gst_orientation_method_to_orientation(self->orientation);
>>
>>
>> gst_libcamera_src_gst_orientation_method_to_orientation() returns the
>> corresponding libcamera::Orientation from GstOrientationMethod value.  I
>> hope you get the idea.
>>
>>
>>>    	/* Validate the configuration. */
>>>    	if (state->config_->validate() == CameraConfiguration::Invalid)
>>>    		return false;
>>> @@ -926,6 +933,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 +955,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 +1133,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;
>>> +}
>>> +
>>>    static void
>>>    gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>>>    {
>>> @@ -1154,6 +1214,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),
>> And use GST_VIDEO_ORIENTATION_IDENTITY here.
>>> +					       (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 mercredi 25 juin 2025 à 15:17 +0200, Jacopo Mondi a écrit :
> Hi Nicolas

[...]

> To summarize:
> 
> - mounting rotation is a static camera properties reported through
>   Camera::properties(). I'm not sure how you want/should expose this
>   thtough the gstreamer API.

*If* you want to expose it, I would place that in the GstDevice in the
device properties. Application will be able to get this through
gst_device_get_properties() once the camera have beeen enumerated.

Nicolas

On Wed, Jun 25, 2025 at 01:37:56PM -0400, Nicolas Dufresne wrote:
> Le mercredi 25 juin 2025 à 15:17 +0200, Jacopo Mondi a écrit :
> > Hi Nicolas
> 
> [...]
> 
> > To summarize:

Thanks Jacopo for that clear and accurate summary.

> > - mounting rotation is a static camera properties reported through
> >   Camera::properties(). I'm not sure how you want/should expose this
> >   thtough the gstreamer API.
> 
> *If* you want to expose it, I would place that in the GstDevice in the
> device properties. Application will be able to get this through
> gst_device_get_properties() once the camera have beeen enumerated.

Ack. I think it's useful to expose it, and making it a GstDevice
property seems a good solution. It's a separate issue from controlling
the image orientation though, so it can be addresses separately.

Hi,

I just sent [PATCH v2] gstreamer: Adding static rotation property to
GstLibcameraDevice and mutable orientation property to GstLibcameraSrc

As discussed here,, I put rotation in GstDevice and orientation in
GstLibcameraSrc. I make no use of GstVideoDirection.

Regards,
G.C.

Il giorno gio 26 giu 2025 alle ore 12:25 Laurent Pinchart
<laurent.pinchart@ideasonboard.com> ha scritto:
>
> On Wed, Jun 25, 2025 at 01:37:56PM -0400, Nicolas Dufresne wrote:
> > Le mercredi 25 juin 2025 à 15:17 +0200, Jacopo Mondi a écrit :
> > > Hi Nicolas
> >
> > [...]
> >
> > > To summarize:
>
> Thanks Jacopo for that clear and accurate summary.
>
> > > - mounting rotation is a static camera properties reported through
> > >   Camera::properties(). I'm not sure how you want/should expose this
> > >   thtough the gstreamer API.
> >
> > *If* you want to expose it, I would place that in the GstDevice in the
> > device properties. Application will be able to get this through
> > gst_device_get_properties() once the camera have beeen enumerated.
>
> Ack. I think it's useful to expose it, and making it a GstDevice
> property seems a good solution. It's a separate issue from controlling
> the image orientation though, so it can be addresses separately.
>
> --
> Regards,
>
> Laurent Pinchart