[{"id":34656,"web_url":"https://patchwork.libcamera.org/comment/34656/","msgid":"<7de4369a-247d-4107-a1bc-66ce975d9917@igalia.com>","date":"2025-06-26T11:52:57","subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Giacomo,\n\nThank you for the patch.\n\nOn 6/26/25 3:38 PM, Giacomo Cappellini wrote:\n> - rotation: is a static camera property reported\n>    through Camera::properties().\n>    This is exposed in GstLibcameraDevice as an extra property.\n>    You can read it with gst-device-monitor-1.0\n>    or GstDevice::gst_device_get_properties.\n>\n> - orientation: is a configurable parameter of the camera that only\n>    concerns with the actual images rotation.\n>    This is exposed as a GST_PARAM_MUTABLE_READY parameter of\n>    GstLibcameraSrc.\n>    It gets validated via CameraConfiguation::validate().\n>    If the camera doesn't support it and CameraConfiguration::Adjusted\n>    is returned the adjusted configuration will be\n>    logged as a warning.\n\n\nBoth of these are separate things, so separate the patches please for \neasier review.\n\n> ---\n>   src/gstreamer/gstlibcameraprovider.cpp |  5 ++\n>   src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-\n>   2 files changed, 102 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> index 5da96ea3..d3384c57 100644\n> --- a/src/gstreamer/gstlibcameraprovider.cpp\n> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> @@ -10,6 +10,7 @@\n>   \n>   #include \"gstlibcameraprovider.h\"\n>   \n> +#include <libcamera/libcamera.h>\n>   #include <libcamera/camera.h>\n>   #include <libcamera/camera_manager.h>\n>   \n> @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n>   \tstatic const std::array roles{ StreamRole::VideoRecording };\n>   \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n>   \tconst gchar *name = camera->id().c_str();\n> +\tconst int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);\n\n\nThe .value_or(0) was troubling me a bit, in case we have missing \nrotation property set, but looking at initProperties() in CameraSensor \nclasses, it seems libcamera also sets it to '0' in case the rotation of \nthe sensor cannot be queried.\n\nSo .value_or(0) is really optional here, but doesn't hurt to have it.\n\n>   \n>   \tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n>   \tif (!config || config->size() != roles.size()) {\n> @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n>   \t\t\t\t       \"display-name\", name,\n>   \t\t\t\t       \"caps\", caps,\n>   \t\t\t\t       \"device-class\", \"Source/Video\",\n> +\t\t\t\t\t   \"properties\", gst_structure_new(\"device-properties\",\n> +\t\t\t\t\t\t\t\"rotation\", G_TYPE_INT, rotation,\n> +\t\t\t\t\t\t\tNULL),\n\n\nDo not create the structure here as this has 2 problems:\n\n- Firstly, when more camera properties are added to this GstStructure, \nit will be a bit cumbersome to read the code\n\n- Secondly, you are having a memory leak here.\n\nSo create the properties GstStructure above gst_libcamera_device_new() as\n\n         g_autoptr(GstStructure) props = gst_structure_new(...);\n\ng_autoptr() will auto free `props` when it goes out of scope.\n\n>   \t\t\t\t       nullptr));\n>   }\n>   \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 3aca4eed..fe3249ff 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -32,6 +32,7 @@\n>   #include <tuple>\n>   #include <utility>\n>   #include <vector>\n> +#include <sstream>\n>   \n>   #include <libcamera/camera.h>\n>   #include <libcamera/camera_manager.h>\n> @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {\n>   \tGstTask *task;\n>   \n>   \tgchar *camera_name;\n> +\tOrientation orientation;\n\n\nYou should save here GstVideoOrientationMethod\n\n>   \n>   \tstd::atomic<GstEvent *> pending_eos;\n>   \n> @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {\n>   enum {\n>   \tPROP_0,\n>   \tPROP_CAMERA_NAME,\n> +\tPROP_ORIENTATION,\n>   \tPROP_LAST\n>   };\n>   \n> @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>   \t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n>   \t}\n>   \n> +\t/* Set orientation control. */\n> +\tstate->config_->orientation = self->orientation;\n\n\nCorresponding mapped value of libcamera::orientation from \nGstVideoOrientationMethod self->orientation needs to be passed here.\n\n\n> +\n>   \t/* Validate the configuration. */\n> -\tif (state->config_->validate() == CameraConfiguration::Invalid)\n> +\tswitch(state->config_->validate()) {\n> +\tcase CameraConfiguration::Valid:\n> +\t\tGST_DEBUG_OBJECT(self, \"Camera configuration is valid\");\n> +\t\tbreak;\n> +\tcase CameraConfiguration::Adjusted:\n> +\t{\n> +\t\t/*\n> +\t\t * The configuration has been adjusted and is now valid.\n> +\t\t * Parameters may have changed for any stream, and stream configurations may have been removed.\n> +\t\t * Log the adjusted configuration.\n> +\t\t */\n> +\t\tfor (gsize i = 0; i < state->config_->size(); i++) {\n> +\t\t\tconst StreamConfiguration &cfg = state->config_->at(i);\n> +\t\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted for stream %zu: %s\", i, cfg.toString().c_str());\n> +\t\t}\n\n\nThis is not correct, CameraConfiguration::Adjusted can be returned if \none (or more) parts of CameraConfiguration is adjusted. So it may happen \nthat stream configurations are unchanged but something else is adjusted \n- but still you would be printing the GST_WARNINGs here.\n\n> +\t\tstd::ostringstream oss;\n> +\t\toss << state->config_->orientation;\n> +\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted orientation: %s\", oss.str().c_str());\n\n\nSimilarly, not true again, orientation might have not been adjusted but \none of other parts of camera configuration. This would misled.\n\n> +\t\tself->orientation = state->config_->orientation;\n> +\t\tbreak;\n> +\t}\n\nI think reading the state->config_->orientation (libcamera::orientation) \nand updating the applied / validated orientation self->orientation is a \ngood idea, but you should drop all the switch-case handling above.\n\nIf you really want to print a debug/warning that orientation is adjusted \n- you need to compare the orientation before and after validate() call.\n\n\n> +\tcase CameraConfiguration::Invalid:\n> +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> +\t\t\t\t  (\"Camera configuration is not supported\"),\n> +\t\t\t\t  (\"CameraConfiguration::validate() returned Invalid\"));\n>   \t\treturn false;\n> +\t}\n>   \n>   \tint ret = state->cam_->configure(state->config_.get());\n>   \tif (ret) {\n> @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n>   \t\tg_free(self->camera_name);\n>   \t\tself->camera_name = g_value_dup_string(value);\n>   \t\tbreak;\n> +\tcase PROP_ORIENTATION:\n> +\t\tself->orientation = (libcamera::Orientation)g_value_get_enum(value);\n\n\nApplication are expected to use GstVideoOrientationMethod values\n\n> +\t\tbreak;\n>   \tdefault:\n>   \t\tif (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))\n>   \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,\n>   \tcase PROP_CAMERA_NAME:\n>   \t\tg_value_set_string(value, self->camera_name);\n>   \t\tbreak;\n> +\tcase PROP_ORIENTATION:\n> +\t\tg_value_set_enum(value, (gint)self->orientation);\n> +\t\tbreak;\n>   \tdefault:\n>   \t\tif (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))\n>   \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>   \tgst_element_remove_pad(element, pad);\n>   }\n>   \n> +static GType\n> +gst_libcamera_orientation_get_type()\n\n\nAll this needs to be handled by GST_VIDEO_ORIENTATION_*  enum which is \nglobal to gstreamer and application are expected to work with those. \nPlease refer to Nicolas' comments  from v1 review.\n\n> +{\n> +\tstatic GType type = 0;\n> +\tstatic const GEnumValue values[] = {\n> +\t\t{\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0),\n> +\t\t\t\"libcamera::Orientation::Rotate0\",\n> +\t\t\t\"rotate-0\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate0Mirror\",\n> +\t\t\t\"rotate-0-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180),\n> +\t\t\t\"libcamera::Orientation::Rotate180\",\n> +\t\t\t\"rotate-180\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate180Mirror\",\n> +\t\t\t\"rotate-180-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate90Mirror\",\n> +\t\t\t\"rotate-90-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270),\n> +\t\t\t\"libcamera::Orientation::Rotate270\",\n> +\t\t\t\"rotate-270\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate270Mirror\",\n> +\t\t\t\"rotate-270-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90),\n> +\t\t\t\"libcamera::Orientation::Rotate90\",\n> +\t\t\t\"rotate-90\",\n> +\t\t},\n> +\t\t{ 0, nullptr, nullptr }\n> +\t};\n> +\n> +\tif (!type)\n> +\t\ttype = g_enum_register_static(\"GstLibcameraOrientation\", values);\n> +\n> +\treturn type;\n> +}\n> +\n>   static void\n>   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>   {\n> @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>   \t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n>   \tg_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);\n>   \n> +\t/* Register the orientation enum type. */\n> +\tGType orientation_type = gst_libcamera_orientation_get_type();\n> +\tspec = g_param_spec_enum(\"orientation\", \"Orientation\",\n> +\t\t\t\t\t       \"Select the orientation of the camera.\",\n> +\t\t\t\t\t       orientation_type,\n> +\t\t\t\t\t       static_cast<gint>(libcamera::Orientation::Rotate0),\n> +\t\t\t\t\t       (GParamFlags)(GST_PARAM_MUTABLE_READY\n> +\t\t\t\t\t\t\t     | G_PARAM_CONSTRUCT\n> +\t\t\t\t\t\t\t     | G_PARAM_READWRITE\n> +\t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n> +\tg_object_class_install_property(object_class, PROP_ORIENTATION, spec);\n> +\n>   \tGstCameraControls::installProperties(object_class, PROP_LAST);\n>   }\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D84B9C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 26 Jun 2025 11:52:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 762CD68DF6;\n\tThu, 26 Jun 2025 13:52:58 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CFE768DE8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Jun 2025 13:52:56 +0200 (CEST)","from [49.36.69.141] (helo=[192.168.29.2])\n\tby fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128)\n\t(Exim) id 1uUl9z-008xzI-FJ\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Jun 2025 13:52:55 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"BBipLVYk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:In-Reply-To:From:\n\tReferences:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To:Cc:\n\tContent-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:\n\tResent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:\n\tList-Subscribe:List-Post:List-Owner:List-Archive;\n\tbh=X9Kcfc5lAFUcYd90qFZ4p/Xray6kvU8eRm9Av1iLjX8=;\n\tb=BBipLVYkLwJ3x0c5+BeFVLemT1\n\t4tf/TWQ8Qht8zDtYXZ1C/ASCKUy0WESBgqmKC1FrvmsTwZDZ62MZM6D2uA671TvceBK5cZg+9O+eU\n\tLbu9I4aFqIVFF1Udq8zX8hniwB7K6AFHRlSW4gmmU3vzqe2zP9XsqWDl+xZyOhCcq2bnGCbJhBaAB\n\tsagkVHYFpDEo0j4XmjwFZ6EAB4ZmmmfIXrt5qniQwJSP2Dm+SlLXIH93Gk5ASrbLdBplSnfwpWYRM\n\t6nT0vqOhoKm60Ayhjh04LTbFGPNTvKO1xEJxeQ1JHzcrtF2OUNsauYW5lzpDiAPjkmnP4XINsapQI\n\tt/iBK8Sg==;","Message-ID":"<7de4369a-247d-4107-a1bc-66ce975d9917@igalia.com>","Date":"Thu, 26 Jun 2025 17:22:57 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","To":"libcamera-devel@lists.libcamera.org","References":"<20250626101017.3358487-1-giacomo.cappellini.87@gmail.com>","Content-Language":"en-US","From":"Umang Jain <uajain@igalia.com>","In-Reply-To":"<20250626101017.3358487-1-giacomo.cappellini.87@gmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34668,"web_url":"https://patchwork.libcamera.org/comment/34668/","msgid":"<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","date":"2025-06-26T13:55:15","subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hi,\n\nLe jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :\n> - rotation: is a static camera property reported\n>   through Camera::properties().\n>   This is exposed in GstLibcameraDevice as an extra property.\n>   You can read it with gst-device-monitor-1.0\n>   or GstDevice::gst_device_get_properties.\n> \n> - orientation: is a configurable parameter of the camera that only\n>   concerns with the actual images rotation.\n>   This is exposed as a GST_PARAM_MUTABLE_READY parameter of\n>   GstLibcameraSrc.\n>   It gets validated via CameraConfiguation::validate().\n>   If the camera doesn't support it and CameraConfiguration::Adjusted\n>   is returned the adjusted configuration will be\n>   logged as a warning.\n> ---\n>  src/gstreamer/gstlibcameraprovider.cpp |  5 ++\n>  src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-\n>  2 files changed, 102 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> index 5da96ea3..d3384c57 100644\n> --- a/src/gstreamer/gstlibcameraprovider.cpp\n> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> @@ -10,6 +10,7 @@\n>  \n>  #include \"gstlibcameraprovider.h\"\n>  \n> +#include <libcamera/libcamera.h>\n>  #include <libcamera/camera.h>\n>  #include <libcamera/camera_manager.h>\n>  \n> @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n>  \tstatic const std::array roles{ StreamRole::VideoRecording };\n>  \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n>  \tconst gchar *name = camera->id().c_str();\n> +\tconst int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);\n>  \n>  \tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n>  \tif (!config || config->size() != roles.size()) {\n> @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n>  \t\t\t\t       \"display-name\", name,\n>  \t\t\t\t       \"caps\", caps,\n>  \t\t\t\t       \"device-class\", \"Source/Video\",\n> +\t\t\t\t\t   \"properties\", gst_structure_new(\"device-properties\",\n> +\t\t\t\t\t\t\t\"rotation\", G_TYPE_INT, rotation,\n\nWhat do you think of calling this \"mounting-rotation\" or \"mounting-orientation\"\n? I'm also wondering if we really want to expose libcamera internal enum values\nhere. Speed is not a problem, I'd use static strings.\n\n> +\t\t\t\t\t\t\tNULL),\n>  \t\t\t\t       nullptr));\n>  }\n>  \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 3aca4eed..fe3249ff 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -32,6 +32,7 @@\n>  #include <tuple>\n>  #include <utility>\n>  #include <vector>\n> +#include <sstream>\n>  \n>  #include <libcamera/camera.h>\n>  #include <libcamera/camera_manager.h>\n> @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {\n>  \tGstTask *task;\n>  \n>  \tgchar *camera_name;\n> +\tOrientation orientation;\n>  \n>  \tstd::atomic<GstEvent *> pending_eos;\n>  \n> @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {\n>  enum {\n>  \tPROP_0,\n>  \tPROP_CAMERA_NAME,\n> +\tPROP_ORIENTATION,\n>  \tPROP_LAST\n>  };\n>  \n> @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>  \t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n>  \t}\n>  \n> +\t/* Set orientation control. */\n> +\tstate->config_->orientation = self->orientation;\n> +\n>  \t/* Validate the configuration. */\n> -\tif (state->config_->validate() == CameraConfiguration::Invalid)\n> +\tswitch(state->config_->validate()) {\n> +\tcase CameraConfiguration::Valid:\n> +\t\tGST_DEBUG_OBJECT(self, \"Camera configuration is valid\");\n> +\t\tbreak;\n> +\tcase CameraConfiguration::Adjusted:\n> +\t{\n> +\t\t/*\n> +\t\t * The configuration has been adjusted and is now valid.\n> +\t\t * Parameters may have changed for any stream, and stream configurations may have been removed.\n> +\t\t * Log the adjusted configuration.\n> +\t\t */\n> +\t\tfor (gsize i = 0; i < state->config_->size(); i++) {\n> +\t\t\tconst StreamConfiguration &cfg = state->config_->at(i);\n> +\t\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted for stream %zu: %s\", i, cfg.toString().c_str());\n> +\t\t}\n> +\t\tstd::ostringstream oss;\n> +\t\toss << state->config_->orientation;\n> +\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted orientation: %s\", oss.str().c_str());\n> +\t\tself->orientation = state->config_->orientation;\n> +\t\tbreak;\n> +\t}\n> +\tcase CameraConfiguration::Invalid:\n> +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> +\t\t\t\t  (\"Camera configuration is not supported\"),\n> +\t\t\t\t  (\"CameraConfiguration::validate() returned Invalid\"));\n>  \t\treturn false;\n> +\t}\n>  \n>  \tint ret = state->cam_->configure(state->config_.get());\n>  \tif (ret) {\n> @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n>  \t\tg_free(self->camera_name);\n>  \t\tself->camera_name = g_value_dup_string(value);\n>  \t\tbreak;\n> +\tcase PROP_ORIENTATION:\n> +\t\tself->orientation = (libcamera::Orientation)g_value_get_enum(value);\n> +\t\tbreak;\n>  \tdefault:\n>  \t\tif (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))\n>  \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,\n>  \tcase PROP_CAMERA_NAME:\n>  \t\tg_value_set_string(value, self->camera_name);\n>  \t\tbreak;\n> +\tcase PROP_ORIENTATION:\n> +\t\tg_value_set_enum(value, (gint)self->orientation);\n> +\t\tbreak;\n>  \tdefault:\n>  \t\tif (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))\n>  \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>  \tgst_element_remove_pad(element, pad);\n>  }\n>  \n> +static GType\n> +gst_libcamera_orientation_get_type()\n> +{\n> +\tstatic GType type = 0;\n> +\tstatic const GEnumValue values[] = {\n> +\t\t{\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0),\n> +\t\t\t\"libcamera::Orientation::Rotate0\",\n> +\t\t\t\"rotate-0\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate0Mirror\",\n> +\t\t\t\"rotate-0-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180),\n> +\t\t\t\"libcamera::Orientation::Rotate180\",\n> +\t\t\t\"rotate-180\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate180Mirror\",\n> +\t\t\t\"rotate-180-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate90Mirror\",\n> +\t\t\t\"rotate-90-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270),\n> +\t\t\t\"libcamera::Orientation::Rotate270\",\n> +\t\t\t\"rotate-270\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270Mirror),\n> +\t\t\t\"libcamera::Orientation::Rotate270Mirror\",\n> +\t\t\t\"rotate-270-mirror\",\n> +\t\t}, {\n> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90),\n> +\t\t\t\"libcamera::Orientation::Rotate90\",\n> +\t\t\t\"rotate-90\",\n> +\t\t},\n> +\t\t{ 0, nullptr, nullptr }\n> +\t};\n> +\n> +\tif (!type)\n> +\t\ttype = g_enum_register_static(\"GstLibcameraOrientation\", values);\n> +\n> +\treturn type;\n> +}\n\nSo remain to debate is this enum. We have to decide around leaking libcamera API\ninto GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation\nenum and names. Its libcamera project choice, be aware that if we move it back\ninto GStreamer in the future, this will change, since we try really hard to stop\nleaking project specific enum values.\n\nNicolas\n\n> +\n>  static void\n>  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>  {\n> @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>  \t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n>  \tg_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);\n>  \n> +\t/* Register the orientation enum type. */\n> +\tGType orientation_type = gst_libcamera_orientation_get_type();\n> +\tspec = g_param_spec_enum(\"orientation\", \"Orientation\",\n> +\t\t\t\t\t       \"Select the orientation of the camera.\",\n> +\t\t\t\t\t       orientation_type,\n> +\t\t\t\t\t       static_cast<gint>(libcamera::Orientation::Rotate0),\n> +\t\t\t\t\t       (GParamFlags)(GST_PARAM_MUTABLE_READY\n> +\t\t\t\t\t\t\t     | G_PARAM_CONSTRUCT\n> +\t\t\t\t\t\t\t     | G_PARAM_READWRITE\n> +\t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n> +\tg_object_class_install_property(object_class, PROP_ORIENTATION, spec);\n> +\n>  \tGstCameraControls::installProperties(object_class, PROP_LAST);\n>  }\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EE1F3BDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 26 Jun 2025 13:55:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD1C068DF6;\n\tThu, 26 Jun 2025 15:55:19 +0200 (CEST)","from mail-qv1-xf2c.google.com (mail-qv1-xf2c.google.com\n\t[IPv6:2607:f8b0:4864:20::f2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B215D68DE8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Jun 2025 15:55:18 +0200 (CEST)","by mail-qv1-xf2c.google.com with SMTP id\n\t6a1803df08f44-6ecf99dd567so12026806d6.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Jun 2025 06:55:18 -0700 (PDT)","from ?IPv6:2606:6d00:17:b699::bad? ([2606:6d00:17:b699::bad])\n\tby smtp.gmail.com with ESMTPSA id\n\taf79cd13be357-7d4431346c4sm3933785a.19.2025.06.26.06.55.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 26 Jun 2025 06:55:16 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com\n\theader.b=\"M2WK8O24\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1750946117;\n\tx=1751550917; darn=lists.libcamera.org; \n\th=mime-version:user-agent:autocrypt:references:in-reply-to:date:to\n\t:from:subject:message-id:from:to:cc:subject:date:message-id:reply-to; \n\tbh=DDm0LB/jWNTHcmRxSZoNu6rZwXfaDNgjwIiLmuexExc=;\n\tb=M2WK8O24w+KQi194++R2p39MtXPB8i+lPtZYLtNX4vu02zfJKTLpTFxmYg1Oa91zuA\n\tk6B5OpY3xrstqAqbJHb3s5L5OFIg6B98ombyY6B5d+uXo6TFFOs93wXR1fYwyWGHOZ94\n\tDRVYyANdFj+BAYNiC8AZ+1JTmsI3IHWvemk//nMaWvLP7tTHzpGRSNiT4brqPh3t4MRv\n\tJ+2KFBPubVFBSA3cur9ZQ1TwwqxrmLfZ7xW7O3EHIUmhCmxatyPlS3PkqgIsQqto4CPF\n\teCRDirgbVJVvAzo1SOAnnim9A06Z8qBZxMBEdabVC8cbiWujtrZcKSu0EhGA9citgqXV\n\t76eQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750946117; x=1751550917;\n\th=mime-version:user-agent:autocrypt:references:in-reply-to:date:to\n\t:from:subject:message-id:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=DDm0LB/jWNTHcmRxSZoNu6rZwXfaDNgjwIiLmuexExc=;\n\tb=IiP6nODfWGsgju+aUdCn129TzIeXKZW9TMFW73KpHyzywFMfNHG3n7Wdcs8WYP+LCB\n\t2HAU8OPv1lC5mVRfjoyCeR+vnQCicUIDevxdy0P1HGbUelrBP+Nl89nKjU0vWbGgZf32\n\tzTI4XSH4amYsuz5TTm4EeK9WI2EhjtWEXTYyzbz8pKDWGF0nnOwsb+wDTx40180XHkHX\n\tfIm/R7Dj+FBzUZ+zcq2wy3vnDAUyeRSHJAzjEuzU5BYh9lVx+l9fizbJcNEJoNdN93UV\n\tPyr/cnblg1N73qOM+7oap3Jqr7xMJaPZXc3WoFNYJyV+up7LfEfaFc9LbfhflX59X8cb\n\tLtHQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCU8i5DWZhk3dEMEgilCHejMoLYEUY0ICAVWk4KXUZTEm3sl+Kd1d9lJPKxY46v5HCuyyIJyDtCgCRMYC+Ooun8=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YymlFTF2erehk6FiGSWM9DFZxBO883Aire7xtwbli5so0vcLFWR\n\tjtnzjJI6djfWV9L6kz6gUfkTQ28Bw+B1jv87GulSnEHGl0V2F98OEbDeLaepczlXMjhCECR1sSN\n\tkDTqB","X-Gm-Gg":"ASbGncuqSfNJFiNtoOzWXS0Ji7HHkErU8/VbTxkJGjn8qTgV025iV7TdE11CLm+Wxum\n\tTEhXpBCBI8ixZj0a+TD6LZsufNx73oTckai6xfFvOUzf1ZGK+tF8DztOu3pEDDy7vdHO+DJTxEw\n\tmIFSMSG2b5YSFHzoyyjVsxAWTLe17sLAsOfAVwYYyt0X9ZF4tER5/R/l1Qw5zBcG/h4HYq0qlOn\n\tR+4GWxpaWIoPJ8joTBCe3+psOGUeRMVbuVe6wpz2DgzFLW6ldYEeGiTTEOgBZ7IwILo7HmgQ9Xa\n\tT3COqe6bP/ZmC/4JYsfWWImWYvcoUvUVNu21TojobzHEP68z7Msd5GGmS60w0kvMlY0=","X-Google-Smtp-Source":"AGHT+IEIDC7O+wIYy67nLMLHkISQBYynq1IJL9jKU2bh0YgmHEAZjKOAVHWAEHOjKstk4LdQk7VBqQ==","X-Received":"by 2002:a05:6214:3286:b0:6fc:ff18:87c1 with SMTP id\n\t6a1803df08f44-6fd5ef51a1emr125034796d6.2.1750946117295; \n\tThu, 26 Jun 2025 06:55:17 -0700 (PDT)","Message-ID":"<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","Subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"Giacomo Cappellini <giacomo.cappellini.87@gmail.com>, \n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 26 Jun 2025 09:55:15 -0400","In-Reply-To":"<20250626101017.3358487-1-giacomo.cappellini.87@gmail.com>","References":"<20250626101017.3358487-1-giacomo.cappellini.87@gmail.com>","Autocrypt":"addr=nicolas@ndufresne.ca; prefer-encrypt=mutual;\n\tkeydata=mDMEaCN2ixYJKwYBBAHaRw8BAQdAM0EHepTful3JOIzcPv6ekHOenE1u0vDG1gdHFrChD\n\t/e0MU5pY29sYXMgRHVmcmVzbmUgPG5pY29sYXMuZHVmcmVzbmVAY29sbGFib3JhLmNvbT6ImQQTFg\n\toAQQIbAwULCQgHAgIiAgYVCgkICwIEFgIDAQIeBwIXgBYhBO8NUoEVxMPCGgRvEtlBlFEpYHL0BQJ\n\toLLLGBQkJZfd1AAoJENlBlFEpYHL0BEkA/3qkWYt99myYFSmTJUF8UB/7OroEm3vr1HRqXeQe9Qp2\n\tAP0bsoAe6KjEPa/pJfuJ2khrOPPHxvyt/PBNbI5BYcIABLQnTmljb2xhcyBEdWZyZXNuZSA8bmljb\n\t2xhc0BuZHVmcmVzbmUuY2E+iJkEExYKAEECGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4AWIQ\n\tTvDVKBFcTDwhoEbxLZQZRRKWBy9AUCaCyy+AUJCWX3dQAKCRDZQZRRKWBy9FJ5AQCNy8SX8DpHbLa\n\tcy58vgDwyIpB89mok9eWGGejY9mqpRwEAhHzs+/n5xlVlM3bqy1yHnAzJqVwqBE1D0jG0a9V6VQI=","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-u/QfDKvk4jUm5tvj01AJ\"","User-Agent":"Evolution 3.56.2 (3.56.2-1.fc42) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34670,"web_url":"https://patchwork.libcamera.org/comment/34670/","msgid":"<175094928267.2047029.5231616399803599473@ping.linuxembedded.co.uk>","date":"2025-06-26T14:48:02","subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Nicolas Dufresne (2025-06-26 14:55:15)\n> Hi,\n> \n> Le jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :\n> > - rotation: is a static camera property reported\n> >   through Camera::properties().\n> >   This is exposed in GstLibcameraDevice as an extra property.\n> >   You can read it with gst-device-monitor-1.0\n> >   or GstDevice::gst_device_get_properties.\n> > \n> > - orientation: is a configurable parameter of the camera that only\n> >   concerns with the actual images rotation.\n> >   This is exposed as a GST_PARAM_MUTABLE_READY parameter of\n> >   GstLibcameraSrc.\n> >   It gets validated via CameraConfiguation::validate().\n> >   If the camera doesn't support it and CameraConfiguration::Adjusted\n> >   is returned the adjusted configuration will be\n> >   logged as a warning.\n> > ---\n> >  src/gstreamer/gstlibcameraprovider.cpp |  5 ++\n> >  src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-\n> >  2 files changed, 102 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> > index 5da96ea3..d3384c57 100644\n> > --- a/src/gstreamer/gstlibcameraprovider.cpp\n> > +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> > @@ -10,6 +10,7 @@\n> >  \n> >  #include \"gstlibcameraprovider.h\"\n> >  \n> > +#include <libcamera/libcamera.h>\n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/camera_manager.h>\n> >  \n> > @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n> >       static const std::array roles{ StreamRole::VideoRecording };\n> >       g_autoptr(GstCaps) caps = gst_caps_new_empty();\n> >       const gchar *name = camera->id().c_str();\n> > +     const int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);\n> >  \n> >       std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n> >       if (!config || config->size() != roles.size()) {\n> > @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n> >                                      \"display-name\", name,\n> >                                      \"caps\", caps,\n> >                                      \"device-class\", \"Source/Video\",\n> > +                                        \"properties\", gst_structure_new(\"device-properties\",\n> > +                                                     \"rotation\", G_TYPE_INT, rotation,\n> \n> What do you think of calling this \"mounting-rotation\" or \"mounting-orientation\"\n> ? I'm also wondering if we really want to expose libcamera internal enum values\n> here. Speed is not a problem, I'd use static strings.\n> \n> > +                                                     NULL),\n> >                                      nullptr));\n> >  }\n> >  \n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 3aca4eed..fe3249ff 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -32,6 +32,7 @@\n> >  #include <tuple>\n> >  #include <utility>\n> >  #include <vector>\n> > +#include <sstream>\n> >  \n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/camera_manager.h>\n> > @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {\n> >       GstTask *task;\n> >  \n> >       gchar *camera_name;\n> > +     Orientation orientation;\n> >  \n> >       std::atomic<GstEvent *> pending_eos;\n> >  \n> > @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {\n> >  enum {\n> >       PROP_0,\n> >       PROP_CAMERA_NAME,\n> > +     PROP_ORIENTATION,\n> >       PROP_LAST\n> >  };\n> >  \n> > @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n> >               gst_libcamera_get_framerate_from_caps(caps, element_caps);\n> >       }\n> >  \n> > +     /* Set orientation control. */\n> > +     state->config_->orientation = self->orientation;\n> > +\n> >       /* Validate the configuration. */\n> > -     if (state->config_->validate() == CameraConfiguration::Invalid)\n> > +     switch(state->config_->validate()) {\n> > +     case CameraConfiguration::Valid:\n> > +             GST_DEBUG_OBJECT(self, \"Camera configuration is valid\");\n> > +             break;\n> > +     case CameraConfiguration::Adjusted:\n> > +     {\n> > +             /*\n> > +              * The configuration has been adjusted and is now valid.\n> > +              * Parameters may have changed for any stream, and stream configurations may have been removed.\n> > +              * Log the adjusted configuration.\n> > +              */\n> > +             for (gsize i = 0; i < state->config_->size(); i++) {\n> > +                     const StreamConfiguration &cfg = state->config_->at(i);\n> > +                     GST_WARNING_OBJECT(self, \"Camera configuration adjusted for stream %zu: %s\", i, cfg.toString().c_str());\n> > +             }\n> > +             std::ostringstream oss;\n> > +             oss << state->config_->orientation;\n> > +             GST_WARNING_OBJECT(self, \"Camera configuration adjusted orientation: %s\", oss.str().c_str());\n> > +             self->orientation = state->config_->orientation;\n> > +             break;\n> > +     }\n> > +     case CameraConfiguration::Invalid:\n> > +             GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > +                               (\"Camera configuration is not supported\"),\n> > +                               (\"CameraConfiguration::validate() returned Invalid\"));\n> >               return false;\n> > +     }\n> >  \n> >       int ret = state->cam_->configure(state->config_.get());\n> >       if (ret) {\n> > @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n> >               g_free(self->camera_name);\n> >               self->camera_name = g_value_dup_string(value);\n> >               break;\n> > +     case PROP_ORIENTATION:\n> > +             self->orientation = (libcamera::Orientation)g_value_get_enum(value);\n> > +             break;\n> >       default:\n> >               if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))\n> >                       G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> > @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,\n> >       case PROP_CAMERA_NAME:\n> >               g_value_set_string(value, self->camera_name);\n> >               break;\n> > +     case PROP_ORIENTATION:\n> > +             g_value_set_enum(value, (gint)self->orientation);\n> > +             break;\n> >       default:\n> >               if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))\n> >                       G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> > @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n> >       gst_element_remove_pad(element, pad);\n> >  }\n> >  \n> > +static GType\n> > +gst_libcamera_orientation_get_type()\n> > +{\n> > +     static GType type = 0;\n> > +     static const GEnumValue values[] = {\n> > +             {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate0),\n> > +                     \"libcamera::Orientation::Rotate0\",\n> > +                     \"rotate-0\",\n> > +             }, {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate0Mirror),\n> > +                     \"libcamera::Orientation::Rotate0Mirror\",\n> > +                     \"rotate-0-mirror\",\n> > +             }, {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate180),\n> > +                     \"libcamera::Orientation::Rotate180\",\n> > +                     \"rotate-180\",\n> > +             }, {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate180Mirror),\n> > +                     \"libcamera::Orientation::Rotate180Mirror\",\n> > +                     \"rotate-180-mirror\",\n> > +             }, {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate90Mirror),\n> > +                     \"libcamera::Orientation::Rotate90Mirror\",\n> > +                     \"rotate-90-mirror\",\n> > +             }, {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate270),\n> > +                     \"libcamera::Orientation::Rotate270\",\n> > +                     \"rotate-270\",\n> > +             }, {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate270Mirror),\n> > +                     \"libcamera::Orientation::Rotate270Mirror\",\n> > +                     \"rotate-270-mirror\",\n> > +             }, {\n> > +                     static_cast<gint>(libcamera::Orientation::Rotate90),\n> > +                     \"libcamera::Orientation::Rotate90\",\n> > +                     \"rotate-90\",\n> > +             },\n> > +             { 0, nullptr, nullptr }\n> > +     };\n> > +\n> > +     if (!type)\n> > +             type = g_enum_register_static(\"GstLibcameraOrientation\", values);\n> > +\n> > +     return type;\n> > +}\n> \n> So remain to debate is this enum. We have to decide around leaking libcamera API\n> into GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation\n> enum and names. Its libcamera project choice, be aware that if we move it back\n> into GStreamer in the future, this will change, since we try really hard to stop\n> leaking project specific enum values.\n\nThe whole point of the gstlibcamerasrc is to map gstreamer <-> libcamera\n- so if there's an existing means for gstreamer to define this with all\nother elements - this code should be mapping those (GstVideoOrientation?) to\nthe libcamera equivalent in my view.\n\n--\nKieran\n\n\n> \n> Nicolas\n> \n> > +\n> >  static void\n> >  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >  {\n> > @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >                                                            | G_PARAM_STATIC_STRINGS));\n> >       g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);\n> >  \n> > +     /* Register the orientation enum type. */\n> > +     GType orientation_type = gst_libcamera_orientation_get_type();\n> > +     spec = g_param_spec_enum(\"orientation\", \"Orientation\",\n> > +                                            \"Select the orientation of the camera.\",\n> > +                                            orientation_type,\n> > +                                            static_cast<gint>(libcamera::Orientation::Rotate0),\n> > +                                            (GParamFlags)(GST_PARAM_MUTABLE_READY\n> > +                                                          | G_PARAM_CONSTRUCT\n> > +                                                          | G_PARAM_READWRITE\n> > +                                                          | G_PARAM_STATIC_STRINGS));\n> > +     g_object_class_install_property(object_class, PROP_ORIENTATION, spec);\n> > +\n> >       GstCameraControls::installProperties(object_class, PROP_LAST);\n> >  }\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6ADEDBDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 26 Jun 2025 14:48:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 05C0668DF4;\n\tThu, 26 Jun 2025 16:48:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A7D968DE8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Jun 2025 16:48:06 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 039FF743;\n\tThu, 26 Jun 2025 16:47:46 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gOXw9IrV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750949267;\n\tbh=gDNK/r66Xcagoxj0ymu+yVsJEIbQOt+Lr7HDIQ5fjhw=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=gOXw9IrVV3nc886sDFbMbI83MDYxBG+onhB+9lSwQS/SliavDX7TpaE9H7veSV535\n\t6zcNmazhSPp7hB1fVD4LORXH65WyMoQ4xqcIdZKbjEdGIpbF+lTadN9acsvOYadYqe\n\t6BD2bLQinVIFpfim0axHKSP/CKCuBpLfY+SgkRo8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","References":"<20250626101017.3358487-1-giacomo.cappellini.87@gmail.com>\n\t<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","Subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Giacomo Cappellini <giacomo.cappellini.87@gmail.com>,\n\tNicolas Dufresne <nicolas@ndufresne.ca>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 26 Jun 2025 15:48:02 +0100","Message-ID":"<175094928267.2047029.5231616399803599473@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34671,"web_url":"https://patchwork.libcamera.org/comment/34671/","msgid":"<vos6v5t3tlrslbumarbzd3yol6wgoroe5yupnlfkjdpz2pash6@lqu5hevs5xjw>","date":"2025-06-26T15:08:07","subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Now that people which knows about gstreamer made the real comments,\nlet me do the nitpicking.\n\nOn Thu, Jun 26, 2025 at 09:55:15AM -0400, Nicolas Dufresne wrote:\n> Hi,\n>\n> Le jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :\n> > - rotation: is a static camera property reported\n> >   through Camera::properties().\n> >   This is exposed in GstLibcameraDevice as an extra property.\n> >   You can read it with gst-device-monitor-1.0\n> >   or GstDevice::gst_device_get_properties.\n> >\n> > - orientation: is a configurable parameter of the camera that only\n> >   concerns with the actual images rotation.\n> >   This is exposed as a GST_PARAM_MUTABLE_READY parameter of\n> >   GstLibcameraSrc.\n> >   It gets validated via CameraConfiguation::validate().\n> >   If the camera doesn't support it and CameraConfiguration::Adjusted\n> >   is returned the adjusted configuration will be\n> >   logged as a warning.\n\nPlease take into account the comments you have received on v1 and try\nto comply with the requirements here specified\nhttps://cbea.ms/git-commit/\n\n1) Reduce commit title length (the suggested 50 cols limits is very\nshort, try to stay in 80 at least)\n2) Use imperative style as suggested in v1\n3) Try to use a commit message style that matches the existing\n   history\n4) Missing Signed-off-by: as commented on v1\n\nThis patch should be broken in 2 separate patches, as Umang suggested\n\n- gstreamer: Report the camera mounting rotation\n- gstreamer: Add support for Orientation\n\nEach commit should go around the lines of:\n\n\"libcamera reports through the Rotation property the camera physical\nmounting rotation. Add an extra property to GstLibcameraDevice\nto report it.\"\n\n\"libcamera allows to control the images orientation through the\nCameraConfiguration::orientation field. Expose a GST_PARAM_MUTABLE_READY\nparameter in GstLibcameraSrc to control it.\n\nAs the requested orientation might be adjusted by\nCameraConfiguation::validate(), inspect the actually image orientation\nafter validate() and expose it.\"\n\n(I don't think you should warn, unless the same warning happens for\nother paramters that could be possibile adjusted)\n\n> > ---\n> >  src/gstreamer/gstlibcameraprovider.cpp |  5 ++\n> >  src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-\n> >  2 files changed, 102 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> > index 5da96ea3..d3384c57 100644\n> > --- a/src/gstreamer/gstlibcameraprovider.cpp\n> > +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> > @@ -10,6 +10,7 @@\n> >  \n> >  #include \"gstlibcameraprovider.h\"\n> >  \n> > +#include <libcamera/libcamera.h>\n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/camera_manager.h>\n> >  \n> > @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n> >  \tstatic const std::array roles{ StreamRole::VideoRecording };\n> >  \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n> >  \tconst gchar *name = camera->id().c_str();\n> > +\tconst int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);\n> >  \n> >  \tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n> >  \tif (!config || config->size() != roles.size()) {\n> > @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n> >  \t\t\t\t       \"display-name\", name,\n> >  \t\t\t\t       \"caps\", caps,\n> >  \t\t\t\t       \"device-class\", \"Source/Video\",\n> > +\t\t\t\t\t   \"properties\", gst_structure_new(\"device-properties\",\n> > +\t\t\t\t\t\t\t\"rotation\", G_TYPE_INT, rotation,\n>\n> What do you think of calling this \"mounting-rotation\" or \"mounting-orientation\"\n> ? I'm also wondering if we really want to expose libcamera internal enum values\n> here. Speed is not a problem, I'd use static strings.\n>\n> > +\t\t\t\t\t\t\tNULL),\n> >  \t\t\t\t       nullptr));\n> >  }\n> >  \n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 3aca4eed..fe3249ff 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -32,6 +32,7 @@\n> >  #include <tuple>\n> >  #include <utility>\n> >  #include <vector>\n> > +#include <sstream>\n> >  \n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/camera_manager.h>\n> > @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {\n> >  \tGstTask *task;\n> >  \n> >  \tgchar *camera_name;\n> > +\tOrientation orientation;\n> >  \n> >  \tstd::atomic<GstEvent *> pending_eos;\n> >  \n> > @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {\n> >  enum {\n> >  \tPROP_0,\n> >  \tPROP_CAMERA_NAME,\n> > +\tPROP_ORIENTATION,\n> >  \tPROP_LAST\n> >  };\n> >  \n> > @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n> >  \t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n> >  \t}\n> >  \n> > +\t/* Set orientation control. */\n> > +\tstate->config_->orientation = self->orientation;\n> > +\n> >  \t/* Validate the configuration. */\n> > -\tif (state->config_->validate() == CameraConfiguration::Invalid)\n> > +\tswitch(state->config_->validate()) {\n> > +\tcase CameraConfiguration::Valid:\n> > +\t\tGST_DEBUG_OBJECT(self, \"Camera configuration is valid\");\n> > +\t\tbreak;\n> > +\tcase CameraConfiguration::Adjusted:\n> > +\t{\n> > +\t\t/*\n> > +\t\t * The configuration has been adjusted and is now valid.\n> > +\t\t * Parameters may have changed for any stream, and stream configurations may have been removed.\n> > +\t\t * Log the adjusted configuration.\n> > +\t\t */\n> > +\t\tfor (gsize i = 0; i < state->config_->size(); i++) {\n> > +\t\t\tconst StreamConfiguration &cfg = state->config_->at(i);\n> > +\t\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted for stream %zu: %s\", i, cfg.toString().c_str());\n> > +\t\t}\n> > +\t\tstd::ostringstream oss;\n> > +\t\toss << state->config_->orientation;\n> > +\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted orientation: %s\", oss.str().c_str());\n> > +\t\tself->orientation = state->config_->orientation;\n> > +\t\tbreak;\n> > +\t}\n> > +\tcase CameraConfiguration::Invalid:\n> > +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > +\t\t\t\t  (\"Camera configuration is not supported\"),\n> > +\t\t\t\t  (\"CameraConfiguration::validate() returned Invalid\"));\n> >  \t\treturn false;\n> > +\t}\n> >  \n> >  \tint ret = state->cam_->configure(state->config_.get());\n> >  \tif (ret) {\n> > @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n> >  \t\tg_free(self->camera_name);\n> >  \t\tself->camera_name = g_value_dup_string(value);\n> >  \t\tbreak;\n> > +\tcase PROP_ORIENTATION:\n> > +\t\tself->orientation = (libcamera::Orientation)g_value_get_enum(value);\n> > +\t\tbreak;\n> >  \tdefault:\n> >  \t\tif (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))\n> >  \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> > @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,\n> >  \tcase PROP_CAMERA_NAME:\n> >  \t\tg_value_set_string(value, self->camera_name);\n> >  \t\tbreak;\n> > +\tcase PROP_ORIENTATION:\n> > +\t\tg_value_set_enum(value, (gint)self->orientation);\n> > +\t\tbreak;\n> >  \tdefault:\n> >  \t\tif (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))\n> >  \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> > @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n> >  \tgst_element_remove_pad(element, pad);\n> >  }\n> >  \n> > +static GType\n> > +gst_libcamera_orientation_get_type()\n> > +{\n> > +\tstatic GType type = 0;\n> > +\tstatic const GEnumValue values[] = {\n> > +\t\t{\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0),\n> > +\t\t\t\"libcamera::Orientation::Rotate0\",\n> > +\t\t\t\"rotate-0\",\n> > +\t\t}, {\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0Mirror),\n> > +\t\t\t\"libcamera::Orientation::Rotate0Mirror\",\n> > +\t\t\t\"rotate-0-mirror\",\n> > +\t\t}, {\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180),\n> > +\t\t\t\"libcamera::Orientation::Rotate180\",\n> > +\t\t\t\"rotate-180\",\n> > +\t\t}, {\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180Mirror),\n> > +\t\t\t\"libcamera::Orientation::Rotate180Mirror\",\n> > +\t\t\t\"rotate-180-mirror\",\n> > +\t\t}, {\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90Mirror),\n> > +\t\t\t\"libcamera::Orientation::Rotate90Mirror\",\n> > +\t\t\t\"rotate-90-mirror\",\n> > +\t\t}, {\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270),\n> > +\t\t\t\"libcamera::Orientation::Rotate270\",\n> > +\t\t\t\"rotate-270\",\n> > +\t\t}, {\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270Mirror),\n> > +\t\t\t\"libcamera::Orientation::Rotate270Mirror\",\n> > +\t\t\t\"rotate-270-mirror\",\n> > +\t\t}, {\n> > +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90),\n> > +\t\t\t\"libcamera::Orientation::Rotate90\",\n> > +\t\t\t\"rotate-90\",\n> > +\t\t},\n> > +\t\t{ 0, nullptr, nullptr }\n> > +\t};\n> > +\n> > +\tif (!type)\n> > +\t\ttype = g_enum_register_static(\"GstLibcameraOrientation\", values);\n> > +\n> > +\treturn type;\n> > +}\n>\n> So remain to debate is this enum. We have to decide around leaking libcamera API\n> into GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation\n> enum and names. Its libcamera project choice, be aware that if we move it back\n> into GStreamer in the future, this will change, since we try really hard to stop\n> leaking project specific enum values.\n>\n\nIf this is exposed through a gstreamer element, it should use\ngstreamer types, doesn't it ? I see no value in leaking libcamera\nnames into gstreamer, but again, not a gstreamer expert :)\n\n> Nicolas\n>\n> > +\n> >  static void\n> >  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >  {\n> > @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >  \t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n> >  \tg_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);\n> >  \n> > +\t/* Register the orientation enum type. */\n> > +\tGType orientation_type = gst_libcamera_orientation_get_type();\n> > +\tspec = g_param_spec_enum(\"orientation\", \"Orientation\",\n> > +\t\t\t\t\t       \"Select the orientation of the camera.\",\n> > +\t\t\t\t\t       orientation_type,\n> > +\t\t\t\t\t       static_cast<gint>(libcamera::Orientation::Rotate0),\n> > +\t\t\t\t\t       (GParamFlags)(GST_PARAM_MUTABLE_READY\n> > +\t\t\t\t\t\t\t     | G_PARAM_CONSTRUCT\n> > +\t\t\t\t\t\t\t     | G_PARAM_READWRITE\n> > +\t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n> > +\tg_object_class_install_property(object_class, PROP_ORIENTATION, spec);\n> > +\n> >  \tGstCameraControls::installProperties(object_class, PROP_LAST);\n> >  }\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5AE65C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 26 Jun 2025 15:08:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C0E0268DF4;\n\tThu, 26 Jun 2025 17:08:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A90CD68DE8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 26 Jun 2025 17:08:10 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7331263B;\n\tThu, 26 Jun 2025 17:07:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LGB5gkzg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750950471;\n\tbh=Wt45ujZeyZh5mnhiAmY1cws7uz8dP4Kv1Yrcud2ly0c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LGB5gkzg//FoaaXZB8alxRUhDlpXkWAJBdBRmyIMLDjR1JWnCG7YvY2UzOT1xuR7q\n\t7NFdUjJdKSMsQCNyKjdEph8T7RVxxUs2z2bO3YAUcu345RqiVgRJWWvZx6pMTEBKTa\n\tUId0Bx/BI42Ql1nYVs8grIzM6ZBrDQpNFaNhzKhI=","Date":"Thu, 26 Jun 2025 17:08:07 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"Giacomo Cappellini <giacomo.cappellini.87@gmail.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","Message-ID":"<vos6v5t3tlrslbumarbzd3yol6wgoroe5yupnlfkjdpz2pash6@lqu5hevs5xjw>","References":"<20250626101017.3358487-1-giacomo.cappellini.87@gmail.com>\n\t<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha512;\n\tprotocol=\"application/pgp-signature\"; boundary=\"eiwhd54wh2apuyqy\"","Content-Disposition":"inline","In-Reply-To":"<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34674,"web_url":"https://patchwork.libcamera.org/comment/34674/","msgid":"<0ccb442d-1c0c-4540-87ac-aed9ea9fa9e3@igalia.com>","date":"2025-06-27T06:25:09","subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Nicolas\n\nOn 6/26/25 7:25 PM, Nicolas Dufresne wrote:\n> Hi,\n>\n> Le jeudi 26 juin 2025 à 12:08 +0200, Giacomo Cappellini a écrit :\n>> - rotation: is a static camera property reported\n>>    through Camera::properties().\n>>    This is exposed in GstLibcameraDevice as an extra property.\n>>    You can read it with gst-device-monitor-1.0\n>>    or GstDevice::gst_device_get_properties.\n>>\n>> - orientation: is a configurable parameter of the camera that only\n>>    concerns with the actual images rotation.\n>>    This is exposed as a GST_PARAM_MUTABLE_READY parameter of\n>>    GstLibcameraSrc.\n>>    It gets validated via CameraConfiguation::validate().\n>>    If the camera doesn't support it and CameraConfiguration::Adjusted\n>>    is returned the adjusted configuration will be\n>>    logged as a warning.\n>> ---\n>>   src/gstreamer/gstlibcameraprovider.cpp |  5 ++\n>>   src/gstreamer/gstlibcamerasrc.cpp      | 98 +++++++++++++++++++++++++-\n>>   2 files changed, 102 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n>> index 5da96ea3..d3384c57 100644\n>> --- a/src/gstreamer/gstlibcameraprovider.cpp\n>> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n>> @@ -10,6 +10,7 @@\n>>   \n>>   #include \"gstlibcameraprovider.h\"\n>>   \n>> +#include <libcamera/libcamera.h>\n>>   #include <libcamera/camera.h>\n>>   #include <libcamera/camera_manager.h>\n>>   \n>> @@ -131,6 +132,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n>>   \tstatic const std::array roles{ StreamRole::VideoRecording };\n>>   \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n>>   \tconst gchar *name = camera->id().c_str();\n>> +\tconst int32_t rotation = camera->properties().get(libcamera::properties::Rotation).value_or(0);\n>>   \n>>   \tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n>>   \tif (!config || config->size() != roles.size()) {\n>> @@ -150,6 +152,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n>>   \t\t\t\t       \"display-name\", name,\n>>   \t\t\t\t       \"caps\", caps,\n>>   \t\t\t\t       \"device-class\", \"Source/Video\",\n>> +\t\t\t\t\t   \"properties\", gst_structure_new(\"device-properties\",\n>> +\t\t\t\t\t\t\t\"rotation\", G_TYPE_INT, rotation,\n> What do you think of calling this \"mounting-rotation\" or \"mounting-orientation\"\n> ? I'm also wondering if we really want to expose libcamera internal enum values\n> here. Speed is not a problem, I'd use static strings.\n\n\nMind that properties::rotation is not a enum Orientation. It is a int32_t.\n\nOnly way to get libcamera::Orientation enum from 'Rotation' is when you \nmake explicit call\nto orientationFromRotation(), which has not been done here.\n\nI agree with usage of static human-friendly strings to report the \nmounting-rotation.\n\n>\n>> +\t\t\t\t\t\t\tNULL),\n>>   \t\t\t\t       nullptr));\n>>   }\n>>   \n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 3aca4eed..fe3249ff 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -32,6 +32,7 @@\n>>   #include <tuple>\n>>   #include <utility>\n>>   #include <vector>\n>> +#include <sstream>\n>>   \n>>   #include <libcamera/camera.h>\n>>   #include <libcamera/camera_manager.h>\n>> @@ -146,6 +147,7 @@ struct _GstLibcameraSrc {\n>>   \tGstTask *task;\n>>   \n>>   \tgchar *camera_name;\n>> +\tOrientation orientation;\n>>   \n>>   \tstd::atomic<GstEvent *> pending_eos;\n>>   \n>> @@ -157,6 +159,7 @@ struct _GstLibcameraSrc {\n>>   enum {\n>>   \tPROP_0,\n>>   \tPROP_CAMERA_NAME,\n>> +\tPROP_ORIENTATION,\n>>   \tPROP_LAST\n>>   };\n>>   \n>> @@ -616,9 +619,37 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>>   \t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n>>   \t}\n>>   \n>> +\t/* Set orientation control. */\n>> +\tstate->config_->orientation = self->orientation;\n>> +\n>>   \t/* Validate the configuration. */\n>> -\tif (state->config_->validate() == CameraConfiguration::Invalid)\n>> +\tswitch(state->config_->validate()) {\n>> +\tcase CameraConfiguration::Valid:\n>> +\t\tGST_DEBUG_OBJECT(self, \"Camera configuration is valid\");\n>> +\t\tbreak;\n>> +\tcase CameraConfiguration::Adjusted:\n>> +\t{\n>> +\t\t/*\n>> +\t\t * The configuration has been adjusted and is now valid.\n>> +\t\t * Parameters may have changed for any stream, and stream configurations may have been removed.\n>> +\t\t * Log the adjusted configuration.\n>> +\t\t */\n>> +\t\tfor (gsize i = 0; i < state->config_->size(); i++) {\n>> +\t\t\tconst StreamConfiguration &cfg = state->config_->at(i);\n>> +\t\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted for stream %zu: %s\", i, cfg.toString().c_str());\n>> +\t\t}\n>> +\t\tstd::ostringstream oss;\n>> +\t\toss << state->config_->orientation;\n>> +\t\tGST_WARNING_OBJECT(self, \"Camera configuration adjusted orientation: %s\", oss.str().c_str());\n>> +\t\tself->orientation = state->config_->orientation;\n>> +\t\tbreak;\n>> +\t}\n>> +\tcase CameraConfiguration::Invalid:\n>> +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n>> +\t\t\t\t  (\"Camera configuration is not supported\"),\n>> +\t\t\t\t  (\"CameraConfiguration::validate() returned Invalid\"));\n>>   \t\treturn false;\n>> +\t}\n>>   \n>>   \tint ret = state->cam_->configure(state->config_.get());\n>>   \tif (ret) {\n>> @@ -926,6 +957,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n>>   \t\tg_free(self->camera_name);\n>>   \t\tself->camera_name = g_value_dup_string(value);\n>>   \t\tbreak;\n>> +\tcase PROP_ORIENTATION:\n>> +\t\tself->orientation = (libcamera::Orientation)g_value_get_enum(value);\n>> +\t\tbreak;\n>>   \tdefault:\n>>   \t\tif (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))\n>>   \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n>> @@ -945,6 +979,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,\n>>   \tcase PROP_CAMERA_NAME:\n>>   \t\tg_value_set_string(value, self->camera_name);\n>>   \t\tbreak;\n>> +\tcase PROP_ORIENTATION:\n>> +\t\tg_value_set_enum(value, (gint)self->orientation);\n>> +\t\tbreak;\n>>   \tdefault:\n>>   \t\tif (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))\n>>   \t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n>> @@ -1120,6 +1157,53 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>>   \tgst_element_remove_pad(element, pad);\n>>   }\n>>   \n>> +static GType\n>> +gst_libcamera_orientation_get_type()\n>> +{\n>> +\tstatic GType type = 0;\n>> +\tstatic const GEnumValue values[] = {\n>> +\t\t{\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0),\n>> +\t\t\t\"libcamera::Orientation::Rotate0\",\n>> +\t\t\t\"rotate-0\",\n>> +\t\t}, {\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate0Mirror),\n>> +\t\t\t\"libcamera::Orientation::Rotate0Mirror\",\n>> +\t\t\t\"rotate-0-mirror\",\n>> +\t\t}, {\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180),\n>> +\t\t\t\"libcamera::Orientation::Rotate180\",\n>> +\t\t\t\"rotate-180\",\n>> +\t\t}, {\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate180Mirror),\n>> +\t\t\t\"libcamera::Orientation::Rotate180Mirror\",\n>> +\t\t\t\"rotate-180-mirror\",\n>> +\t\t}, {\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90Mirror),\n>> +\t\t\t\"libcamera::Orientation::Rotate90Mirror\",\n>> +\t\t\t\"rotate-90-mirror\",\n>> +\t\t}, {\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270),\n>> +\t\t\t\"libcamera::Orientation::Rotate270\",\n>> +\t\t\t\"rotate-270\",\n>> +\t\t}, {\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate270Mirror),\n>> +\t\t\t\"libcamera::Orientation::Rotate270Mirror\",\n>> +\t\t\t\"rotate-270-mirror\",\n>> +\t\t}, {\n>> +\t\t\tstatic_cast<gint>(libcamera::Orientation::Rotate90),\n>> +\t\t\t\"libcamera::Orientation::Rotate90\",\n>> +\t\t\t\"rotate-90\",\n>> +\t\t},\n>> +\t\t{ 0, nullptr, nullptr }\n>> +\t};\n>> +\n>> +\tif (!type)\n>> +\t\ttype = g_enum_register_static(\"GstLibcameraOrientation\", values);\n>> +\n>> +\treturn type;\n>> +}\n> So remain to debate is this enum. We have to decide around leaking libcamera API\n> into GStreamer or using GStreamer 1:1 equivalent which is GstVideoOrientation\n> enum and names. Its libcamera project choice, be aware that if we move it back\n> into GStreamer in the future, this will change, since we try really hard to stop\n> leaking project specific enum values.\n>\n> Nicolas\n>\n>> +\n>>   static void\n>>   gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>>   {\n>> @@ -1154,6 +1238,18 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>>   \t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n>>   \tg_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);\n>>   \n>> +\t/* Register the orientation enum type. */\n>> +\tGType orientation_type = gst_libcamera_orientation_get_type();\n>> +\tspec = g_param_spec_enum(\"orientation\", \"Orientation\",\n>> +\t\t\t\t\t       \"Select the orientation of the camera.\",\n>> +\t\t\t\t\t       orientation_type,\n>> +\t\t\t\t\t       static_cast<gint>(libcamera::Orientation::Rotate0),\n>> +\t\t\t\t\t       (GParamFlags)(GST_PARAM_MUTABLE_READY\n>> +\t\t\t\t\t\t\t     | G_PARAM_CONSTRUCT\n>> +\t\t\t\t\t\t\t     | G_PARAM_READWRITE\n>> +\t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n>> +\tg_object_class_install_property(object_class, PROP_ORIENTATION, spec);\n>> +\n>>   \tGstCameraControls::installProperties(object_class, PROP_LAST);\n>>   }\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DF03CBDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Jun 2025 06:25:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9582B68DF4;\n\tFri, 27 Jun 2025 08:25:08 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 373C161528\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Jun 2025 08:25:06 +0200 (CEST)","from [49.36.69.141] (helo=[192.168.29.4])\n\tby fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128)\n\t(Exim) id 1uV2WG-009HFa-FA; Fri, 27 Jun 2025 08:25:04 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"Ue3wuRnI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:In-Reply-To:From:\n\tReferences:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To:Cc:\n\tContent-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:\n\tResent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:\n\tList-Subscribe:List-Post:List-Owner:List-Archive;\n\tbh=SqRWLg2COyBfIrbbHJvUnNsxpK5Ul/CYmxgVGyIj6GA=;\n\tb=Ue3wuRnIxYEba+6o4nLc0Mj61W\n\toqXQaNpL8aEGZ+JL6NLqW0btYhGQn1b+w+/XoL6EHSOv429ut8iVgAdBW0YOfTrQN1C6NgC0Sknce\n\tTaVFEH8qTr0NKsmAZbzqxEhOSypqSMguok2MpEeAjQJi3ZsfNXJrJnWys4rmMtghqEfkavJLQzi4Z\n\twZL3NPdSQo2Ow81kAErgoXQKCDrXLWFrEMVUZb9avrhqiZvwBtr1uWdFVrdLV7xalu5QqbyDB39pM\n\tosI2C7IJ2uKKSKFFVlzAVerA4JaAT0y77kLDxZjR6EoFDHf+dNWo9IdtQsh7aW0AVxp/D3u3X+0/B\n\t5O5hDRjg==;","Message-ID":"<0ccb442d-1c0c-4540-87ac-aed9ea9fa9e3@igalia.com>","Date":"Fri, 27 Jun 2025 11:55:09 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] gstreamer: Adding static rotation property to\n\tGstLibcameraDevice and mutable orientation property to\n\tGstLibcameraSrc","To":"Nicolas Dufresne <nicolas@ndufresne.ca>,\n\tGiacomo Cappellini <giacomo.cappellini.87@gmail.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250626101017.3358487-1-giacomo.cappellini.87@gmail.com>\n\t<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","Content-Language":"en-US","From":"Umang Jain <uajain@igalia.com>","In-Reply-To":"<3b0e2440a13c9ee2c67b7c5793eec82fbb5eecfa.camel@ndufresne.ca>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]