[v2] libcamera: gstreamer: Report mounting-rotation of camera sensor
diff mbox series

Message ID 20250703123647.21468-1-uajain@igalia.com
State New
Headers show
Series
  • [v2] libcamera: gstreamer: Report mounting-rotation of camera sensor
Related show

Commit Message

Umang Jain July 3, 2025, 12:36 p.m. UTC
Report mounting-rotation of the camera sensor by reading the
property::Rotation property from libcamera. This is reported
in device properties for libcamerasrc.

Signed-off-by: Umang Jain <uajain@igalia.com>
---
Changes in v2:
- Ditch reporting mounting-rotation as libcamera orientation string
- Use the property naming format as: "api.libcamera.<property>: <value>"
---
 src/gstreamer/gstlibcameraprovider.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Kieran Bingham July 3, 2025, 12:44 p.m. UTC | #1
Quoting Umang Jain (2025-07-03 13:36:47)
> Report mounting-rotation of the camera sensor by reading the
> property::Rotation property from libcamera. This is reported
> in device properties for libcamerasrc.
> 
> Signed-off-by: Umang Jain <uajain@igalia.com>
> ---
> Changes in v2:
> - Ditch reporting mounting-rotation as libcamera orientation string
> - Use the property naming format as: "api.libcamera.<property>: <value>"
> ---
>  src/gstreamer/gstlibcameraprovider.cpp | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index 5da96ea3..b0a7124f 100644
> --- a/src/gstreamer/gstlibcameraprovider.cpp
> +++ b/src/gstreamer/gstlibcameraprovider.cpp
> @@ -12,6 +12,7 @@
>  
>  #include <libcamera/camera.h>
>  #include <libcamera/camera_manager.h>
> +#include <libcamera/property_ids.h>
>  
>  #include "gstlibcamerasrc.h"
>  #include "gstlibcamera-utils.h"
> @@ -144,12 +145,19 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>                         gst_caps_append(caps, sub_caps);
>         }
>  
> +       const int32_t rotation = camera->properties().get(properties::Rotation).value_or(0);
> +       g_autoptr(GstStructure) props =
> +               gst_structure_new("camera-properties",
> +                                 "api.libcamera.mounting-rotation", G_TYPE_INT, rotation,

If we extend this to iterate over all properties and report them as
api.libcamera.$PROPERTY_NAME this would just be 'rotation' I think ?

Could you envisage iterating all the properties and including them with
their property name ?

Such as the way cam does it with "cam -c 1 -p"

Then this would handle all camera properties automatically ?

--
Kieran


> +                                 nullptr);
> +
>         return GST_DEVICE(g_object_new(GST_TYPE_LIBCAMERA_DEVICE,
>                                        /* \todo Use a unique identifier instead of camera name. */
>                                        "name", name,
>                                        "display-name", name,
>                                        "caps", caps,
>                                        "device-class", "Source/Video",
> +                                      "properties", props,
>                                        nullptr));
>  }
>  
> -- 
> 2.50.0
>
Kieran Bingham July 3, 2025, 1:37 p.m. UTC | #2
Quoting Kieran Bingham (2025-07-03 13:44:52)
> Quoting Umang Jain (2025-07-03 13:36:47)
> > Report mounting-rotation of the camera sensor by reading the
> > property::Rotation property from libcamera. This is reported
> > in device properties for libcamerasrc.
> > 
> > Signed-off-by: Umang Jain <uajain@igalia.com>
> > ---
> > Changes in v2:
> > - Ditch reporting mounting-rotation as libcamera orientation string
> > - Use the property naming format as: "api.libcamera.<property>: <value>"
> > ---
> >  src/gstreamer/gstlibcameraprovider.cpp | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> > index 5da96ea3..b0a7124f 100644
> > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > @@ -12,6 +12,7 @@
> >  
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> > +#include <libcamera/property_ids.h>
> >  
> >  #include "gstlibcamerasrc.h"
> >  #include "gstlibcamera-utils.h"
> > @@ -144,12 +145,19 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
> >                         gst_caps_append(caps, sub_caps);
> >         }
> >  
> > +       const int32_t rotation = camera->properties().get(properties::Rotation).value_or(0);
> > +       g_autoptr(GstStructure) props =
> > +               gst_structure_new("camera-properties",
> > +                                 "api.libcamera.mounting-rotation", G_TYPE_INT, rotation,
> 
> If we extend this to iterate over all properties and report them as
> api.libcamera.$PROPERTY_NAME this would just be 'rotation' I think ?
> 
> Could you envisage iterating all the properties and including them with
> their property name ?
> 
> Such as the way cam does it with "cam -c 1 -p"
> 
> Then this would handle all camera properties automatically ?

As Laurent asked if I'm asking for yak-shaving here ... to be clearer:

- If you want to just add this rotation property - I'd be fine - if it
matched the property name - so that for the bigger picture - handling
'all properties' could be handled on top, Otherwise someone will have to
special case the name of the rotation property.

Or handling all properties now is certainly a yak - and if you like
shaving yaks then that's fine too ;-)

--
Kieran


> 
> --
> Kieran
> 
> 
> > +                                 nullptr);
> > +
> >         return GST_DEVICE(g_object_new(GST_TYPE_LIBCAMERA_DEVICE,
> >                                        /* \todo Use a unique identifier instead of camera name. */
> >                                        "name", name,
> >                                        "display-name", name,
> >                                        "caps", caps,
> >                                        "device-class", "Source/Video",
> > +                                      "properties", props,
> >                                        nullptr));
> >  }
> >  
> > -- 
> > 2.50.0
> >
Umang Jain July 3, 2025, 2:28 p.m. UTC | #3
On Thu, Jul 03, 2025 at 02:37:13PM +0100, Kieran Bingham wrote:
> Quoting Kieran Bingham (2025-07-03 13:44:52)
> > Quoting Umang Jain (2025-07-03 13:36:47)
> > > Report mounting-rotation of the camera sensor by reading the
> > > property::Rotation property from libcamera. This is reported
> > > in device properties for libcamerasrc.
> > > 
> > > Signed-off-by: Umang Jain <uajain@igalia.com>
> > > ---
> > > Changes in v2:
> > > - Ditch reporting mounting-rotation as libcamera orientation string
> > > - Use the property naming format as: "api.libcamera.<property>: <value>"
> > > ---
> > >  src/gstreamer/gstlibcameraprovider.cpp | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> > > index 5da96ea3..b0a7124f 100644
> > > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > > @@ -12,6 +12,7 @@
> > >  
> > >  #include <libcamera/camera.h>
> > >  #include <libcamera/camera_manager.h>
> > > +#include <libcamera/property_ids.h>
> > >  
> > >  #include "gstlibcamerasrc.h"
> > >  #include "gstlibcamera-utils.h"
> > > @@ -144,12 +145,19 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
> > >                         gst_caps_append(caps, sub_caps);
> > >         }
> > >  
> > > +       const int32_t rotation = camera->properties().get(properties::Rotation).value_or(0);
> > > +       g_autoptr(GstStructure) props =
> > > +               gst_structure_new("camera-properties",
> > > +                                 "api.libcamera.mounting-rotation", G_TYPE_INT, rotation,
> > 
> > If we extend this to iterate over all properties and report them as
> > api.libcamera.$PROPERTY_NAME this would just be 'rotation' I think ?
> > 
> > Could you envisage iterating all the properties and including them with
> > their property name ?
> > 
> > Such as the way cam does it with "cam -c 1 -p"
> > 
> > Then this would handle all camera properties automatically ?
> 
> As Laurent asked if I'm asking for yak-shaving here ... to be clearer:
> 
> - If you want to just add this rotation property - I'd be fine - if it
> matched the property name - so that for the bigger picture - handling
> 'all properties' could be handled on top, Otherwise someone will have to
> special case the name of the rotation property.

Naming is hard.. we recently went through whole cycle of rotation,
mounting-rotation, orientaion, ... eh

> 
> Or handling all properties now is certainly a yak - and if you like
> shaving yaks then that's fine too ;-)

Should be do-able, I will take a look at some point.

> 
> --
> Kieran
> 
> 
> > 
> > --
> > Kieran
> > 
> > 
> > > +                                 nullptr);
> > > +
> > >         return GST_DEVICE(g_object_new(GST_TYPE_LIBCAMERA_DEVICE,
> > >                                        /* \todo Use a unique identifier instead of camera name. */
> > >                                        "name", name,
> > >                                        "display-name", name,
> > >                                        "caps", caps,
> > >                                        "device-class", "Source/Video",
> > > +                                      "properties", props,
> > >                                        nullptr));
> > >  }
> > >  
> > > -- 
> > > 2.50.0
> > >
Nicolas Dufresne July 4, 2025, 12:34 a.m. UTC | #4
Le jeudi 03 juillet 2025 à 13:44 +0100, Kieran Bingham a écrit :
> Quoting Umang Jain (2025-07-03 13:36:47)
> > Report mounting-rotation of the camera sensor by reading the
> > property::Rotation property from libcamera. This is reported
> > in device properties for libcamerasrc.
> > 
> > Signed-off-by: Umang Jain <uajain@igalia.com>
> > ---
> > Changes in v2:
> > - Ditch reporting mounting-rotation as libcamera orientation string
> > - Use the property naming format as: "api.libcamera.<property>: <value>"
> > ---
> >  src/gstreamer/gstlibcameraprovider.cpp | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp
> > b/src/gstreamer/gstlibcameraprovider.cpp
> > index 5da96ea3..b0a7124f 100644
> > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > @@ -12,6 +12,7 @@
> >  
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> > +#include <libcamera/property_ids.h>
> >  
> >  #include "gstlibcamerasrc.h"
> >  #include "gstlibcamera-utils.h"
> > @@ -144,12 +145,19 @@ gst_libcamera_device_new(const std::shared_ptr<Camera>
> > &camera)
> >                         gst_caps_append(caps, sub_caps);
> >         }
> >  
> > +       const int32_t rotation = camera-
> > >properties().get(properties::Rotation).value_or(0);
> > +       g_autoptr(GstStructure) props =
> > +               gst_structure_new("camera-properties",
> > +                                 "api.libcamera.mounting-rotation",
> > G_TYPE_INT, rotation,
> 
> If we extend this to iterate over all properties and report them as
> api.libcamera.$PROPERTY_NAME this would just be 'rotation' I think ?
> 
> Could you envisage iterating all the properties and including them with
> their property name ?
> 
> Such as the way cam does it with "cam -c 1 -p"
> 
> Then this would handle all camera properties automatically ?

Seems like a good suggestion to me, and with the api.libcamera namespace, we
know where to look for documentation.

Nicolas

> 
> --
> Kieran
> 
> 
> > +                                 nullptr);
> > +
> >         return GST_DEVICE(g_object_new(GST_TYPE_LIBCAMERA_DEVICE,
> >                                        /* \todo Use a unique identifier
> > instead of camera name. */
> >                                        "name", name,
> >                                        "display-name", name,
> >                                        "caps", caps,
> >                                        "device-class", "Source/Video",
> > +                                      "properties", props,
> >                                        nullptr));
> >  }
> >  
> > -- 
> > 2.50.0
> >

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
index 5da96ea3..b0a7124f 100644
--- a/src/gstreamer/gstlibcameraprovider.cpp
+++ b/src/gstreamer/gstlibcameraprovider.cpp
@@ -12,6 +12,7 @@ 
 
 #include <libcamera/camera.h>
 #include <libcamera/camera_manager.h>
+#include <libcamera/property_ids.h>
 
 #include "gstlibcamerasrc.h"
 #include "gstlibcamera-utils.h"
@@ -144,12 +145,19 @@  gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
 			gst_caps_append(caps, sub_caps);
 	}
 
+	const int32_t rotation = camera->properties().get(properties::Rotation).value_or(0);
+	g_autoptr(GstStructure) props =
+		gst_structure_new("camera-properties",
+				  "api.libcamera.mounting-rotation", G_TYPE_INT, rotation,
+				  nullptr);
+
 	return GST_DEVICE(g_object_new(GST_TYPE_LIBCAMERA_DEVICE,
 				       /* \todo Use a unique identifier instead of camera name. */
 				       "name", name,
 				       "display-name", name,
 				       "caps", caps,
 				       "device-class", "Source/Video",
+				       "properties", props,
 				       nullptr));
 }