[{"id":35040,"web_url":"https://patchwork.libcamera.org/comment/35040/","msgid":"","date":"2025-07-23T11:38:02","subject":"Re: [PATCH v2] gstreamer: Add support for Orientation","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hello Giacomo\n\nOn Wed, Jul 23, 2025 at 11:39:37AM +0200, Giacomo Cappellini wrote:\n> libcamera allows to control the images orientation through the\n> CameraConfiguration::orientation field, expose a GST_PARAM_MUTABLE_READY\n> parameter of type GstVideoOrientationMethod in GstLibcameraSrc to\n> control it. Parameter is mapped internally to libcamera::Orientation via\n> new gstlibcamera-utils functions:\n> - gst_video_orientation_to_libcamera_orientation\n> - libcamera_orientation_to_gst_video_orientation\n> \n> Update CameraConfiguration::Adjusted case in negotiation to warn about\n> changes in StreamConfiguration and SensorConfiguration, as well as\n> the new Orientation parameter.\n> \n> signed-off-by: Giacomo Cappellini \n> ---\n> src/gstreamer/gstlibcamera-utils.cpp | 32 +++++++\n> src/gstreamer/gstlibcamera-utils.h | 4 +\n> src/gstreamer/gstlibcamerasrc.cpp | 124 ++++++++++++++++++++++++++-\n> 3 files changed, 159 insertions(+), 1 deletion(-)\n> \n\nPlease run checkstyle for your patch before sending. There are many\nissues with this patch\n($) ./utils/checkstyle.py HEAD\n\n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index a548b0c1..fc29d19d 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -10,6 +10,8 @@\n> \n> #include \n> #include \n> +#include \n> +#include \n> \n> using namespace libcamera;\n> \n> @@ -659,3 +661,33 @@ gst_libcamera_get_camera_manager(int &ret)\n> \n> \treturn cm;\n> }\n> +\n> +Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)\n> +{\n> + switch (method) {\n> + case GST_VIDEO_ORIENTATION_IDENTITY: return Orientation::Rotate0;\n> + case GST_VIDEO_ORIENTATION_90R: return Orientation::Rotate90;\n> + case GST_VIDEO_ORIENTATION_180: return Orientation::Rotate180;\n> + case GST_VIDEO_ORIENTATION_90L: return Orientation::Rotate270;\n> + case GST_VIDEO_ORIENTATION_HORIZ: return Orientation::Rotate0Mirror;\n> + case GST_VIDEO_ORIENTATION_VERT: return Orientation::Rotate180Mirror;\n> + case GST_VIDEO_ORIENTATION_UL_LR: return Orientation::Rotate90Mirror;\n> + case GST_VIDEO_ORIENTATION_UR_LL: return Orientation::Rotate270Mirror;\n> + default: return Orientation::Rotate0;\n> + }\n> +}\n> +\n> +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(Orientation orientation)\n> +{\n> + switch (orientation) {\n> + case Orientation::Rotate0: return GST_VIDEO_ORIENTATION_IDENTITY;\n> + case Orientation::Rotate90: return GST_VIDEO_ORIENTATION_90R;\n> + case Orientation::Rotate180: return GST_VIDEO_ORIENTATION_180;\n> + case Orientation::Rotate270: return GST_VIDEO_ORIENTATION_90L;\n> + case Orientation::Rotate0Mirror: return GST_VIDEO_ORIENTATION_HORIZ;\n> + case Orientation::Rotate180Mirror: return GST_VIDEO_ORIENTATION_VERT;\n> + case Orientation::Rotate90Mirror: return GST_VIDEO_ORIENTATION_UL_LR;\n> + case Orientation::Rotate270Mirror: return GST_VIDEO_ORIENTATION_UR_LL;\n> + default: return GST_VIDEO_ORIENTATION_IDENTITY;\n> + }\n> +}\n\nWe could something similar to buffer_map[] and format_map[] in the same\nfile. This way the mappings are stated only once and would be easier\nto update later just in case.\n\nstatic const struct { \n Orientation orientation; \n GstVideoOrientationMethod method; \n} orientation_map[]{ \n { Orientation::Rotate0, GST_VIDEO_ORIENTATION_IDENTITY }, \n { Orientation::Rotate90, GST_VIDEO_ORIENTATION_90R }, \n { Orientation::Rotate180, GST_VIDEO_ORIENTATION_180 }, \n { Orientation::Rotate270, GST_VIDEO_ORIENTATION_90L }, \n { Orientation::Rotate0Mirror, GST_VIDEO_ORIENTATION_HORIZ }, \n { Orientation::Rotate180Mirror, GST_VIDEO_ORIENTATION_VERT }, \n { Orientation::Rotate90Mirror, GST_VIDEO_ORIENTATION_UL_LR }, \n { Orientation::Rotate270Mirror, GST_VIDEO_ORIENTATION_UR_LL }, \n}; \n \nOrientation \ngst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)\n{ \n for (auto &b : orientation_map) { \n if (b.method == method) \n return b.orientation; \n } \n \n return Orientation::Rotate0; \n \n} \n \nGstVideoOrientationMethod \nlibcamera_orientation_to_gst_video_orientation(Orientation orientation) \n{ \n for (auto &a : orientation_map) { \n if (a.orientation == orientation) \n return a.method; \n } \n \n return GST_VIDEO_ORIENTATION_IDENTITY; \n} \n\n> \\ No newline at end of file\n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index 5f4e8a0f..3d4b049f 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -92,3 +93,6 @@ public:\n> private:\n> \tGRecMutex *mutex_;\n> };\n> +\n> +libcamera::Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method);\n> +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(libcamera::Orientation orientation);\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 3aca4eed..03d389aa 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -32,12 +32,14 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> #include \n> \n> #include \n> +#include \n> \n> #include \"gstlibcamera-controls.h\"\n> #include \"gstlibcamera-utils.h\"\n> @@ -146,6 +148,7 @@ struct _GstLibcameraSrc {\n> \tGstTask *task;\n> \n> \tgchar *camera_name;\n> +\tGstVideoOrientationMethod orientation;\n> \n> \tstd::atomic pending_eos;\n> \n> @@ -157,6 +160,7 @@ struct _GstLibcameraSrc {\n> enum {\n> \tPROP_0,\n> \tPROP_CAMERA_NAME,\n> +\tPROP_ORIENTATION,\n> \tPROP_LAST\n> };\n> \n> @@ -616,9 +620,110 @@ 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 = gst_video_orientation_to_libcamera_orientation(self->orientation);\n> +\n> +\t/* Save original configuration for comparison after validation */\n> +\tstd::vector orig_stream_cfgs;\n> +\tfor (gsize i = 0; i < state->config_->size(); i++)\n> +\t\torig_stream_cfgs.push_back(state->config_->at(i));\n> +\tstd::optional orig_sensor_cfg = state->config_->sensorConfig;\n> +\tOrientation orig_orientation = state->config_->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\nDid you happen to see: https://patchwork.libcamera.org/patch/23898/ ?\n\n> +\t{\t\n> +\t\tbool warned = false;\n> +\t\t// Warn if number of StreamConfigurations changed\n> +\t\tif (orig_stream_cfgs.size() != state->config_->size()) {\n> +\t\t\tGST_WARNING_OBJECT(self, \"Number of StreamConfiguration elements changed: requested=%zu, actual=%zu\",\n> +\t\t\t\torig_stream_cfgs.size(), state->config_->size());\n> +\t\t\twarned = true;\n> +\t\t}\n> +\t\t// Warn about changes in each StreamConfiguration\n> +\t\t// TODO implement diffing in StreamConfiguration \n> +\t\tfor (gsize i = 0; i < std::min(orig_stream_cfgs.size(), state->config_->size()); i++) {\n> +\t\t\tif (orig_stream_cfgs[i].toString() != state->config_->at(i).toString()) {\n> +\t\t\t\tGST_WARNING_OBJECT(self, \"StreamConfiguration %zu changed: %s -> %s\",\n> +\t\t\t\t\ti, orig_stream_cfgs[i].toString().c_str(),\n> +\t\t\t\t\tstate->config_->at(i).toString().c_str());\n> +\t\t\t\twarned = true;\n> +\t\t\t}\n> +\t\t}\n> +\t\t// Warn about SensorConfiguration changes\n> +\t\t// TODO implement diffing in SensorConfiguration\n> +\t\tif (orig_sensor_cfg.has_value() || state->config_->sensorConfig.has_value()) {\n> +\t\t\tconst SensorConfiguration *orig = orig_sensor_cfg.has_value() ? &orig_sensor_cfg.value() : nullptr;\n> +\t\t\tconst SensorConfiguration *curr = state->config_->sensorConfig.has_value() ? &state->config_->sensorConfig.value() : nullptr;\n> +\t\t\tbool sensor_changed = false;\n> +\t\t\tstd::ostringstream diff;\n> +\t\t\tif ((orig == nullptr) != (curr == nullptr)) {\n> +\t\t\t\tdiff << \"SensorConfiguration presence changed: \"\n> +\t\t\t\t << (orig ? \"was present\" : \"was absent\")\n> +\t\t\t\t << \" -> \"\n> +\t\t\t\t << (curr ? \"present\" : \"absent\");\n> +\t\t\t\tsensor_changed = true;\n> +\t\t\t} else if (orig && curr) {\n> +\t\t\t\tif (orig->bitDepth != curr->bitDepth) {\n> +\t\t\t\t\tdiff << \"bitDepth: \" << orig->bitDepth << \" -> \" << curr->bitDepth << \"; \";\n> +\t\t\t\t\tsensor_changed = true;\n> +\t\t\t\t}\n> +\t\t\t\tif (orig->analogCrop != curr->analogCrop) {\n> +\t\t\t\t\tdiff << \"analogCrop: \" << orig->analogCrop.toString() << \" -> \" << curr->analogCrop.toString() << \"; \";\n> +\t\t\t\t\tsensor_changed = true;\n> +\t\t\t\t}\n> +\t\t\t\tif (orig->binning.binX != curr->binning.binX ||\n> +\t\t\t\t orig->binning.binY != curr->binning.binY) {\n> +\t\t\t\t\tdiff << \"binning: (\" << orig->binning.binX << \",\" << orig->binning.binY << \") -> (\"\n> +\t\t\t\t\t << curr->binning.binX << \",\" << curr->binning.binY << \"); \";\n> +\t\t\t\t\tsensor_changed = true;\n> +\t\t\t\t}\n> +\t\t\t\tif (orig->skipping.xOddInc != curr->skipping.xOddInc ||\n> +\t\t\t\t orig->skipping.xEvenInc != curr->skipping.xEvenInc ||\n> +\t\t\t\t orig->skipping.yOddInc != curr->skipping.yOddInc ||\n> +\t\t\t\t orig->skipping.yEvenInc != curr->skipping.yEvenInc) {\n> +\t\t\t\t\tdiff << \"skipping: (\"\n> +\t\t\t\t\t << orig->skipping.xOddInc << \",\" << orig->skipping.xEvenInc << \",\"\n> +\t\t\t\t\t << orig->skipping.yOddInc << \",\" << orig->skipping.yEvenInc << \") -> (\"\n> +\t\t\t\t\t << curr->skipping.xOddInc << \",\" << curr->skipping.xEvenInc << \",\"\n> +\t\t\t\t\t << curr->skipping.yOddInc << \",\" << curr->skipping.yEvenInc << \"); \";\n> +\t\t\t\t\tsensor_changed = true;\n> +\t\t\t\t}\n> +\t\t\t\tif (orig->outputSize != curr->outputSize) {\n> +\t\t\t\t\tdiff << \"outputSize: \" << orig->outputSize.toString() << \" -> \" << curr->outputSize.toString() << \"; \";\n> +\t\t\t\t\tsensor_changed = true;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\tif (sensor_changed) {\n> +\t\t\t\tGST_WARNING_OBJECT(self, \"SensorConfiguration changed: %s\", diff.str().c_str());\n> +\t\t\t\twarned = true;\n> +\t\t\t}\n> +\t\t}\n> +\t\t// Warn about orientation change\n> +\t\tif (orig_orientation != state->config_->orientation) {\n> +\t\t\tGEnumClass *enum_class = (GEnumClass *)g_type_class_ref(GST_TYPE_VIDEO_ORIENTATION_METHOD);\n> +\t\t\tconst char *orig_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(orig_orientation))->value_nick;\n> +\t\t\tconst char *new_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(state->config_->orientation))->value_nick;\n> +\t\t\tGST_WARNING_OBJECT(self, \"Orientation changed: %s -> %s\", orig_orientation_str, new_orientation_str);\n> +\t\t\twarned = true;\n> +\t\t}\n> +\t\tif (!warned) {\n> +\t\t\tGST_DEBUG_OBJECT(self, \"Camera configuration adjusted, but no significant changes detected.\");\n> +\t\t}\n> +\t\t// Update Gst orientation property to match adjusted config\n> +\t\tself->orientation = libcamera_orientation_to_gst_video_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 +1031,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 = (GstVideoOrientationMethod)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 +1053,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> @@ -1154,6 +1265,17 @@ 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> +\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 GST_TYPE_VIDEO_ORIENTATION_METHOD,\n> +\t\t\t\t\t GST_VIDEO_ORIENTATION_IDENTITY,\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> \n> -- \n> 2.43.0\n>","headers":{"Return-Path":"","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 8F5DEC3237\n\tfor ;\n\tWed, 23 Jul 2025 11:38:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9665A69029;\n\tWed, 23 Jul 2025 13:38:00 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D167B614ED\n\tfor ;\n\tWed, 23 Jul 2025 13:37:58 +0200 (CEST)","from [49.36.71.87] (helo=uajain) by fanzine2.igalia.com with\n\tesmtpsa \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1ueXnI-002aQy-3q; Wed, 23 Jul 2025 13:37:56 +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=\"qucIZOQT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:\n\tSubject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=tCOtLf3ba8u3blc/FZCAvfH/MpXpVPbUM7VdlGdL+pI=;\n\tb=qucIZOQTnDt12ikyrM6a7qMvnt\n\tVMvA5TGCM0Ng0sA6EOMLgwwuVduG9g2Rv7WcoJLSc4g7iDlDrwgS0NesUY+3QOXFjAn+Hy3xjPkZI\n\tGo1uE1DpTt3elpjASEXU77YIcutleIoHlnp/KCwjNk2RcY935sX6sPW98XlgdzEs3WXYL39UWLCsP\n\tn12BJxi+/GHJFJtELTUh/KScVWMYkTl+ZU5ysdRGYhdlWrzQoTgtRmSJpcca4BoBW24PXTQOOJc4y\n\tyX6PBLATeJz2qRnVKQKrgW0SlLJ6YImM0Pptx1yaWRFjJ5+fX6xh5slBDxAqjXaevaOcgrzx30vbN\n\tzE3/7pkg==;","Date":"Wed, 23 Jul 2025 17:08:02 +0530","From":"Umang Jain ","To":"Giacomo Cappellini ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] gstreamer: Add support for Orientation","Message-ID":"","References":"<20250723093954.599102-1-giacomo.cappellini.87@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250723093954.599102-1-giacomo.cappellini.87@gmail.com>","User-Agent":"NeoMutt/20250510-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35041,"web_url":"https://patchwork.libcamera.org/comment/35041/","msgid":"","date":"2025-07-23T14:07:50","subject":"Re: [PATCH v2] gstreamer: Add support for Orientation","submitter":{"id":231,"url":"https://patchwork.libcamera.org/api/people/231/","name":"Giacomo Cappellini","email":"giacomo.cappellini.87@gmail.com"},"content":"Thanks for the review.\n\nI was not aware of https://patchwork.libcamera.org/patch/23898/, we\nmade changes to gst_libcamera_src_negotiate at same time/day.\nI see my contribution brings a more compete diffing for the\nCameraConfiguration::Adjusted case, but my code lacks the check if\ndownstream can accept this new stream configuration and if\nnot return a not-negotiated error.\n\nIn v3 I've fixed the style errors and replaced old maps with\norientation_map as you suggested.\n\nG.C.\n\nIl giorno mer 23 lug 2025 alle ore 13:37 Umang Jain\n ha scritto:\n>\n> Hello Giacomo\n>\n> On Wed, Jul 23, 2025 at 11:39:37AM +0200, Giacomo Cappellini wrote:\n> > libcamera allows to control the images orientation through the\n> > CameraConfiguration::orientation field, expose a GST_PARAM_MUTABLE_READY\n> > parameter of type GstVideoOrientationMethod in GstLibcameraSrc to\n> > control it. Parameter is mapped internally to libcamera::Orientation via\n> > new gstlibcamera-utils functions:\n> > - gst_video_orientation_to_libcamera_orientation\n> > - libcamera_orientation_to_gst_video_orientation\n> >\n> > Update CameraConfiguration::Adjusted case in negotiation to warn about\n> > changes in StreamConfiguration and SensorConfiguration, as well as\n> > the new Orientation parameter.\n> >\n> > signed-off-by: Giacomo Cappellini \n> > ---\n> > src/gstreamer/gstlibcamera-utils.cpp | 32 +++++++\n> > src/gstreamer/gstlibcamera-utils.h | 4 +\n> > src/gstreamer/gstlibcamerasrc.cpp | 124 ++++++++++++++++++++++++++-\n> > 3 files changed, 159 insertions(+), 1 deletion(-)\n> >\n>\n> Please run checkstyle for your patch before sending. There are many\n> issues with this patch\n> ($) ./utils/checkstyle.py HEAD\n>\n> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > index a548b0c1..fc29d19d 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > @@ -10,6 +10,8 @@\n> >\n> > #include \n> > #include \n> > +#include \n> > +#include \n> >\n> > using namespace libcamera;\n> >\n> > @@ -659,3 +661,33 @@ gst_libcamera_get_camera_manager(int &ret)\n> >\n> > return cm;\n> > }\n> > +\n> > +Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)\n> > +{\n> > + switch (method) {\n> > + case GST_VIDEO_ORIENTATION_IDENTITY: return Orientation::Rotate0;\n> > + case GST_VIDEO_ORIENTATION_90R: return Orientation::Rotate90;\n> > + case GST_VIDEO_ORIENTATION_180: return Orientation::Rotate180;\n> > + case GST_VIDEO_ORIENTATION_90L: return Orientation::Rotate270;\n> > + case GST_VIDEO_ORIENTATION_HORIZ: return Orientation::Rotate0Mirror;\n> > + case GST_VIDEO_ORIENTATION_VERT: return Orientation::Rotate180Mirror;\n> > + case GST_VIDEO_ORIENTATION_UL_LR: return Orientation::Rotate90Mirror;\n> > + case GST_VIDEO_ORIENTATION_UR_LL: return Orientation::Rotate270Mirror;\n> > + default: return Orientation::Rotate0;\n> > + }\n> > +}\n> > +\n> > +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(Orientation orientation)\n> > +{\n> > + switch (orientation) {\n> > + case Orientation::Rotate0: return GST_VIDEO_ORIENTATION_IDENTITY;\n> > + case Orientation::Rotate90: return GST_VIDEO_ORIENTATION_90R;\n> > + case Orientation::Rotate180: return GST_VIDEO_ORIENTATION_180;\n> > + case Orientation::Rotate270: return GST_VIDEO_ORIENTATION_90L;\n> > + case Orientation::Rotate0Mirror: return GST_VIDEO_ORIENTATION_HORIZ;\n> > + case Orientation::Rotate180Mirror: return GST_VIDEO_ORIENTATION_VERT;\n> > + case Orientation::Rotate90Mirror: return GST_VIDEO_ORIENTATION_UL_LR;\n> > + case Orientation::Rotate270Mirror: return GST_VIDEO_ORIENTATION_UR_LL;\n> > + default: return GST_VIDEO_ORIENTATION_IDENTITY;\n> > + }\n> > +}\n>\n> We could something similar to buffer_map[] and format_map[] in the same\n> file. This way the mappings are stated only once and would be easier\n> to update later just in case.\n>\n> static const struct {\n> Orientation orientation;\n> GstVideoOrientationMethod method;\n> } orientation_map[]{\n> { Orientation::Rotate0, GST_VIDEO_ORIENTATION_IDENTITY },\n> { Orientation::Rotate90, GST_VIDEO_ORIENTATION_90R },\n> { Orientation::Rotate180, GST_VIDEO_ORIENTATION_180 },\n> { Orientation::Rotate270, GST_VIDEO_ORIENTATION_90L },\n> { Orientation::Rotate0Mirror, GST_VIDEO_ORIENTATION_HORIZ },\n> { Orientation::Rotate180Mirror, GST_VIDEO_ORIENTATION_VERT },\n> { Orientation::Rotate90Mirror, GST_VIDEO_ORIENTATION_UL_LR },\n> { Orientation::Rotate270Mirror, GST_VIDEO_ORIENTATION_UR_LL },\n> };\n>\n> Orientation\n> gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)\n> {\n> for (auto &b : orientation_map) {\n> if (b.method == method)\n> return b.orientation;\n> }\n>\n> return Orientation::Rotate0;\n>\n> }\n>\n> GstVideoOrientationMethod\n> libcamera_orientation_to_gst_video_orientation(Orientation orientation)\n> {\n> for (auto &a : orientation_map) {\n> if (a.orientation == orientation)\n> return a.method;\n> }\n>\n> return GST_VIDEO_ORIENTATION_IDENTITY;\n> }\n>\n> > \\ No newline at end of file\n> > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> > index 5f4e8a0f..3d4b049f 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.h\n> > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > @@ -11,6 +11,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> >\n> > #include \n> > #include \n> > @@ -92,3 +93,6 @@ public:\n> > private:\n> > GRecMutex *mutex_;\n> > };\n> > +\n> > +libcamera::Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method);\n> > +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(libcamera::Orientation orientation);\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 3aca4eed..03d389aa 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -32,12 +32,14 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> >\n> > #include \n> > #include \n> > #include \n> >\n> > #include \n> > +#include \n> >\n> > #include \"gstlibcamera-controls.h\"\n> > #include \"gstlibcamera-utils.h\"\n> > @@ -146,6 +148,7 @@ struct _GstLibcameraSrc {\n> > GstTask *task;\n> >\n> > gchar *camera_name;\n> > + GstVideoOrientationMethod orientation;\n> >\n> > std::atomic pending_eos;\n> >\n> > @@ -157,6 +160,7 @@ struct _GstLibcameraSrc {\n> > enum {\n> > PROP_0,\n> > PROP_CAMERA_NAME,\n> > + PROP_ORIENTATION,\n> > PROP_LAST\n> > };\n> >\n> > @@ -616,9 +620,110 @@ 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 = gst_video_orientation_to_libcamera_orientation(self->orientation);\n> > +\n> > + /* Save original configuration for comparison after validation */\n> > + std::vector orig_stream_cfgs;\n> > + for (gsize i = 0; i < state->config_->size(); i++)\n> > + orig_stream_cfgs.push_back(state->config_->at(i));\n> > + std::optional orig_sensor_cfg = state->config_->sensorConfig;\n> > + Orientation orig_orientation = state->config_->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> Did you happen to see: https://patchwork.libcamera.org/patch/23898/ ?\n>\n> > + {\n> > + bool warned = false;\n> > + // Warn if number of StreamConfigurations changed\n> > + if (orig_stream_cfgs.size() != state->config_->size()) {\n> > + GST_WARNING_OBJECT(self, \"Number of StreamConfiguration elements changed: requested=%zu, actual=%zu\",\n> > + orig_stream_cfgs.size(), state->config_->size());\n> > + warned = true;\n> > + }\n> > + // Warn about changes in each StreamConfiguration\n> > + // TODO implement diffing in StreamConfiguration\n> > + for (gsize i = 0; i < std::min(orig_stream_cfgs.size(), state->config_->size()); i++) {\n> > + if (orig_stream_cfgs[i].toString() != state->config_->at(i).toString()) {\n> > + GST_WARNING_OBJECT(self, \"StreamConfiguration %zu changed: %s -> %s\",\n> > + i, orig_stream_cfgs[i].toString().c_str(),\n> > + state->config_->at(i).toString().c_str());\n> > + warned = true;\n> > + }\n> > + }\n> > + // Warn about SensorConfiguration changes\n> > + // TODO implement diffing in SensorConfiguration\n> > + if (orig_sensor_cfg.has_value() || state->config_->sensorConfig.has_value()) {\n> > + const SensorConfiguration *orig = orig_sensor_cfg.has_value() ? &orig_sensor_cfg.value() : nullptr;\n> > + const SensorConfiguration *curr = state->config_->sensorConfig.has_value() ? &state->config_->sensorConfig.value() : nullptr;\n> > + bool sensor_changed = false;\n> > + std::ostringstream diff;\n> > + if ((orig == nullptr) != (curr == nullptr)) {\n> > + diff << \"SensorConfiguration presence changed: \"\n> > + << (orig ? \"was present\" : \"was absent\")\n> > + << \" -> \"\n> > + << (curr ? \"present\" : \"absent\");\n> > + sensor_changed = true;\n> > + } else if (orig && curr) {\n> > + if (orig->bitDepth != curr->bitDepth) {\n> > + diff << \"bitDepth: \" << orig->bitDepth << \" -> \" << curr->bitDepth << \"; \";\n> > + sensor_changed = true;\n> > + }\n> > + if (orig->analogCrop != curr->analogCrop) {\n> > + diff << \"analogCrop: \" << orig->analogCrop.toString() << \" -> \" << curr->analogCrop.toString() << \"; \";\n> > + sensor_changed = true;\n> > + }\n> > + if (orig->binning.binX != curr->binning.binX ||\n> > + orig->binning.binY != curr->binning.binY) {\n> > + diff << \"binning: (\" << orig->binning.binX << \",\" << orig->binning.binY << \") -> (\"\n> > + << curr->binning.binX << \",\" << curr->binning.binY << \"); \";\n> > + sensor_changed = true;\n> > + }\n> > + if (orig->skipping.xOddInc != curr->skipping.xOddInc ||\n> > + orig->skipping.xEvenInc != curr->skipping.xEvenInc ||\n> > + orig->skipping.yOddInc != curr->skipping.yOddInc ||\n> > + orig->skipping.yEvenInc != curr->skipping.yEvenInc) {\n> > + diff << \"skipping: (\"\n> > + << orig->skipping.xOddInc << \",\" << orig->skipping.xEvenInc << \",\"\n> > + << orig->skipping.yOddInc << \",\" << orig->skipping.yEvenInc << \") -> (\"\n> > + << curr->skipping.xOddInc << \",\" << curr->skipping.xEvenInc << \",\"\n> > + << curr->skipping.yOddInc << \",\" << curr->skipping.yEvenInc << \"); \";\n> > + sensor_changed = true;\n> > + }\n> > + if (orig->outputSize != curr->outputSize) {\n> > + diff << \"outputSize: \" << orig->outputSize.toString() << \" -> \" << curr->outputSize.toString() << \"; \";\n> > + sensor_changed = true;\n> > + }\n> > + }\n> > + if (sensor_changed) {\n> > + GST_WARNING_OBJECT(self, \"SensorConfiguration changed: %s\", diff.str().c_str());\n> > + warned = true;\n> > + }\n> > + }\n> > + // Warn about orientation change\n> > + if (orig_orientation != state->config_->orientation) {\n> > + GEnumClass *enum_class = (GEnumClass *)g_type_class_ref(GST_TYPE_VIDEO_ORIENTATION_METHOD);\n> > + const char *orig_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(orig_orientation))->value_nick;\n> > + const char *new_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(state->config_->orientation))->value_nick;\n> > + GST_WARNING_OBJECT(self, \"Orientation changed: %s -> %s\", orig_orientation_str, new_orientation_str);\n> > + warned = true;\n> > + }\n> > + if (!warned) {\n> > + GST_DEBUG_OBJECT(self, \"Camera configuration adjusted, but no significant changes detected.\");\n> > + }\n> > + // Update Gst orientation property to match adjusted config\n> > + self->orientation = libcamera_orientation_to_gst_video_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 +1031,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 = (GstVideoOrientationMethod)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 +1053,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> > @@ -1154,6 +1265,17 @@ 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> > + spec = g_param_spec_enum(\"orientation\", \"Orientation\",\n> > + \"Select the orientation of the camera.\",\n> > + GST_TYPE_VIDEO_ORIENTATION_METHOD,\n> > + GST_VIDEO_ORIENTATION_IDENTITY,\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> >\n> > --\n> > 2.43.0\n> >","headers":{"Return-Path":"","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 79A29C3237\n\tfor ;\n\tWed, 23 Jul 2025 14:08:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 450BD69053;\n\tWed, 23 Jul 2025 16:08:05 +0200 (CEST)","from mail-il1-x130.google.com (mail-il1-x130.google.com\n\t[IPv6:2607:f8b0:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6B868614ED\n\tfor ;\n\tWed, 23 Jul 2025 16:08:03 +0200 (CEST)","by mail-il1-x130.google.com with SMTP id\n\te9e14a558f8ab-3de18fde9cfso27804875ab.3\n\tfor ;\n\tWed, 23 Jul 2025 07:08:03 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"gwP2+jNC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1753279682; x=1753884482;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=dSK1ZSQPxqG6Yxisuu06Hsuyh3jJFMNSQS2xqIMsCBI=;\n\tb=gwP2+jNCYEdK8FTpyQZJfmhTVHKPeDeSwRnchRWSqknmfuF72z43+0KhoAwBShlpjN\n\tMHf8Uj2ZRpbCjivihN9ASA0UA1SZNahxCeOWQaBg5ZP4/cfJClLv7ayMqTKWnS7xU04y\n\tR49lU7XEpTr45iMfFljYl4h3k98FFBmlrE8xZX5MaviqLswSOG0t8+VFQiMC1sFr/fqy\n\tO+/g2nrwL/LrqW3FbkbRF7gjv2ppbwuSFnni/Z9uI7djC4a1Dh0dJOU605A5tJIflW3U\n\t2+OPUoLJIkZa1WTJZOy8ctRaz730FUxw0H+7WvsmMt4Qt2DlI1cUglOMZL27Mptk5+6i\n\tG5mQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753279682; x=1753884482;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=dSK1ZSQPxqG6Yxisuu06Hsuyh3jJFMNSQS2xqIMsCBI=;\n\tb=sKsHEG0mIzqsFrWXB+u944CnO4Effiz1NTI4dgYbsaLq7JsrMRSbfJvHUxFSTR+w3c\n\tDzov34pyXQ5aMrBbYJTXtEQu56BduIOLHUFeGIPUfiavQuP1JWk4FUbUVfh1+kK3mZfU\n\txrUJRlJsiS/7inGOaH2Z9LMeQvYHbaD4c7FO3knZqXn3H/9Dy/1XoIuc1h839u7T74NJ\n\tuDSbNUh3/YcIF71KLI5yXINdj2JqOyqpkZ0TyE/SrqmEBZiFYp3ALpXhyXTEySb/7rPe\n\tMgG1LNhgXC4Uqqwl80Z9rF71sWwDe1iUc4vkAv0pxn0f6pcq5e/M+gelprizw+Jz2hxq\n\tAgEA==","X-Gm-Message-State":"AOJu0YxGl6707Y6hW7ESbpZeSRbtlEcFObVYzzy+U+NsCp4WH9LGqEjx\n\tgteUvg2ikRy/mi3PFNoP6Buf11AwKoq6UB465wH12I/28v5pH527KAEI61idmdr/SxkKzlBZdLQ\n\tGHsG/TURITW9nEVphBlEukGWfCgsRUw4=","X-Gm-Gg":"ASbGnctuNyjmvI5PpjttR9EWXgtY13sJQIy6a6A2ys9hUzmim88Eik5dbFJQI0TMj9W\n\t3hadXKA47MnNYeXrOYV8QpVMj/+K5rlT+j9IEp0XwoCwUHHIV4DrZfOtSB897DEZTkFW1eqBhld\n\tksrXXbe8TTgJK3YlReBd6KQgD63nL9J3KNFAtXVH742DQeiCnViIV5ChuHNsBkk/SBQUiQz8+l0\n\ttEmO83d","X-Google-Smtp-Source":"AGHT+IEWb/0a/guzh0JXbrrOvC6RkdI6acvxoqO68Ijvxan5HXMIzTuQLyypGh+bIm266nz82Xvrgi4SQ2l1aQt/RoQ=","X-Received":"by 2002:a05:6e02:3091:b0:3df:3271:6aba with SMTP id\n\te9e14a558f8ab-3e34559e360mr50197355ab.15.1753279681669;\n\tWed, 23 Jul 2025 07:08:01 -0700 (PDT)","MIME-Version":"1.0","References":"<20250723093954.599102-1-giacomo.cappellini.87@gmail.com>\n\t","In-Reply-To":"","From":"Giacomo Cappellini ","Date":"Wed, 23 Jul 2025 16:07:50 +0200","X-Gm-Features":"Ac12FXx4yvKBHco1BNdKQvxP7qc1flsaRH1sM25q13IJ6L6ETzQ6UXoXdP1Axjc","Message-ID":"","Subject":"Re: [PATCH v2] gstreamer: Add support for Orientation","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]