[v3] libcamerasrc: Report camera properties as device properties
diff mbox series

Message ID 20250704053424.23718-1-uajain@igalia.com
State New
Headers show
Series
  • [v3] libcamerasrc: Report camera properties as device properties
Related show

Commit Message

Umang Jain July 4, 2025, 5:34 a.m. UTC
Iterate over all libcamera camera properties and report them as
device properties. The values are reported as strings, including the ones
which enum types, for better readability.

Signed-off-by: Umang Jain <uajain@igalia.com>
---
Changes in v3:
- Iterate over all camera properties and report everything.
---
 src/gstreamer/gstlibcameraprovider.cpp | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Laurent Pinchart July 4, 2025, 9:46 a.m. UTC | #1
Hi Umang,

Thank you for the patch.

On Fri, Jul 04, 2025 at 11:04:24AM +0530, Umang Jain wrote:
> Iterate over all libcamera camera properties and report them as
> device properties. The values are reported as strings, including the ones
> which enum types, for better readability.
> 
> Signed-off-by: Umang Jain <uajain@igalia.com>
> ---
> Changes in v3:
> - Iterate over all camera properties and report everything.
> ---
>  src/gstreamer/gstlibcameraprovider.cpp | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index 5da96ea3..237baeb2 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"
> @@ -130,6 +131,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>  {
>  	static const std::array roles{ StreamRole::VideoRecording };
>  	g_autoptr(GstCaps) caps = gst_caps_new_empty();
> +	g_autoptr(GstStructure) props = gst_structure_new_empty("camera-properties");

You can move this just before the for loop below.

>  	const gchar *name = camera->id().c_str();
>  
>  	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
> @@ -144,12 +146,34 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>  			gst_caps_append(caps, sub_caps);
>  	}
>  
> +	for (const auto &[key, value] : camera->properties()) {
> +		const ControlId *id = properties::properties.at(key);
> +
> +		g_autoptr(GString) prop_str = g_string_new("api.libcamera.");
> +		g_string_append(prop_str, id->name().c_str());
> +
> +		/* Use string names for enum values for better readability */

s/readability/readability./

> +		if (value.type() == ControlTypeInteger32) {
> +			int32_t val = value.get<int32_t>();
> +			const auto &it = id->enumerators().find(val);
> +			if (it != id->enumerators().end()) {
> +				gst_structure_set(props, prop_str->str, G_TYPE_STRING,
> +						  it->second.c_str(), nullptr);
> +				continue;
> +			}
> +		}
> +
> +		gst_structure_set(props, prop_str->str, G_TYPE_STRING,
> +				  value.toString().c_str(), nullptr);

Instead of using a libcamera-specific internal string representation of
the value, it should be converted to native GStreamer types. Numerical
properties should use a numerical type, rectangles should be converted
to the native GStreamer representation, ...

Nicolas, any opinion on this ?

> +	}
> +
>  	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));
>  }
>
Nicolas Dufresne July 4, 2025, 1:40 p.m. UTC | #2
Le vendredi 04 juillet 2025 à 12:46 +0300, Laurent Pinchart a écrit :
> Hi Umang,
> 
> Thank you for the patch.
> 
> On Fri, Jul 04, 2025 at 11:04:24AM +0530, Umang Jain wrote:
> > Iterate over all libcamera camera properties and report them as
> > device properties. The values are reported as strings, including the ones
> > which enum types, for better readability.
> > 
> > Signed-off-by: Umang Jain <uajain@igalia.com>
> > ---
> > Changes in v3:
> > - Iterate over all camera properties and report everything.

Here, outside of commit message, you could pate the output of gst-device-
monitor-1.0 Video/Source, that helps to see what kind of output we get.

> > ---
> >  src/gstreamer/gstlibcameraprovider.cpp | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp
> > b/src/gstreamer/gstlibcameraprovider.cpp
> > index 5da96ea3..237baeb2 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"
> > @@ -130,6 +131,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera>
> > &camera)
> >  {
> >  	static const std::array roles{ StreamRole::VideoRecording };
> >  	g_autoptr(GstCaps) caps = gst_caps_new_empty();
> > +	g_autoptr(GstStructure) props = gst_structure_new_empty("camera-
> > properties");
> 
> You can move this just before the for loop below.
> 
> >  	const gchar *name = camera->id().c_str();
> >  
> >  	std::unique_ptr<CameraConfiguration> config = camera-
> > >generateConfiguration(roles);
> > @@ -144,12 +146,34 @@ gst_libcamera_device_new(const std::shared_ptr<Camera>
> > &camera)
> >  			gst_caps_append(caps, sub_caps);
> >  	}
> >  
> > +	for (const auto &[key, value] : camera->properties()) {
> > +		const ControlId *id = properties::properties.at(key);
> > +
> > +		g_autoptr(GString) prop_str =
> > g_string_new("api.libcamera.");
> > +		g_string_append(prop_str, id->name().c_str());
> > +
> > +		/* Use string names for enum values for better readability
> > */
> 
> s/readability/readability./
> 
> > +		if (value.type() == ControlTypeInteger32) {
> > +			int32_t val = value.get<int32_t>();
> > +			const auto &it = id->enumerators().find(val);
> > +			if (it != id->enumerators().end()) {
> > +				gst_structure_set(props, prop_str->str,
> > G_TYPE_STRING,
> > +						  it->second.c_str(),
> > nullptr);
> > +				continue;
> > +			}
> > +		}
> > +
> > +		gst_structure_set(props, prop_str->str, G_TYPE_STRING,
> > +				  value.toString().c_str(), nullptr);
> 
> Instead of using a libcamera-specific internal string representation of
> the value, it should be converted to native GStreamer types. Numerical
> properties should use a numerical type, rectangles should be converted
> to the native GStreamer representation, ...
> 
> Nicolas, any opinion on this ?

This is the first time we have an API we don't have to fully map manually.
Looking around, only G_TYPE_UINT and G_TYPE_STRING is currently used by other
providers. Its likely a good idea for simplicity. Though, when I look at the
vulkan providers, what they did for flags and enum is to have both, e.g. flags +
flags_str (no compromise). If you link to Vulkan API, you will find handy the
value, and otherwise you have a string version which is human readable.

Now, I don't see an "enum" type in the list of libcamera control types. So can
this really be handled generically ? Here's that list I found:

	ControlTypeBool -> UINT
	ControlTypeByte --> BASE64 STRING
	ControlTypeUnsigned16 -> UINT
	ControlTypeUnsigned32 --> UINT
	ControlTypeInteger32 --> UINT
	ControlTypeInteger64 --> UINT64
	ControlTypeFloat --> DOUBLE ?
	ControlTypeString --> STRING
	ControlTypeRectangle --> No match, break in 4 _x, _y, _width, _height?
	ControlTypeSize --> UINT64
	ControlTypePoint --> No match, break in 2 _x, _y ?

GST_TYPE_LIST/ARRAY can be used if these comes as an array, you can define your
own types + serializers also if you want to have rec/point in a single field.
This is not a GStreamer but a GLib thing, GstStructure holds GValue, and offer
helper for the most common.

Stepping back, if we consider we need manual mapping, why not go back to just
exposing the rotation for now, and make it so its easy to map more later ?

> 
> > +	}
> > +
> >  	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));
> >  }
> >
Umang Jain July 5, 2025, 5:30 a.m. UTC | #3
On Fri, Jul 04, 2025 at 09:40:03AM -0400, Nicolas Dufresne wrote:
> Le vendredi 04 juillet 2025 à 12:46 +0300, Laurent Pinchart a écrit :
> > Hi Umang,
> > 
> > Thank you for the patch.
> > 
> > On Fri, Jul 04, 2025 at 11:04:24AM +0530, Umang Jain wrote:
> > > Iterate over all libcamera camera properties and report them as
> > > device properties. The values are reported as strings, including the ones
> > > which enum types, for better readability.
> > > 
> > > Signed-off-by: Umang Jain <uajain@igalia.com>
> > > ---
> > > Changes in v3:
> > > - Iterate over all camera properties and report everything.
> 
> Here, outside of commit message, you could pate the output of gst-device-
> monitor-1.0 Video/Source, that helps to see what kind of output we get.

[~]$ gst-device-monitor-1.0 
Probing devices...

[3:00:30.247078308] [64824]  INFO Camera camera_manager.cpp:326 libcamera v0.5.1+47-12a310d9

Device found:

	name  : \_SB_.PC00.XHCI.RHUB.HS04-4:1.0-04f2:b7e0
	class : Source/Video
	caps  : image/jpeg, width=640, height=360
	        image/jpeg, width=640, height=480
	        image/jpeg, width=848, height=480
	        image/jpeg, width=960, height=540
	        image/jpeg, width=1280, height=720
	        image/jpeg, width=1920, height=1080
	        video/x-raw, format=YUY2, width=640, height=360
	        video/x-raw, format=YUY2, width=640, height=480
	properties:
		api.libcamera.SystemDevices = [ 20736 ]
		api.libcamera.PixelArrayActiveAreas = [ (0, 0)/1920x1080 ]
		api.libcamera.PixelArraySize = 1920x1080
		api.libcamera.Location = CameraLocationFront
		api.libcamera.Model = Integrated Camera: Integrated C
	gst-launch-1.0 libcamerasrc camera-name="\\_SB_.PC00.XHCI.RHUB.HS04-4:1.0-04f2:b7e0" ! ...

Now I realise, rotation is not reported. Because UVC pipeline handler
doesn't set it. So camera->properties() will only report properties set
by the pipeline handler.

Q: Do we want to report *all* properties as per property_ids_core.yaml ?
And report "Unset" or equivant, in the device provider? 

> 
> > > ---
> > >  src/gstreamer/gstlibcameraprovider.cpp | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > > 
> > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp
> > > b/src/gstreamer/gstlibcameraprovider.cpp
> > > index 5da96ea3..237baeb2 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"
> > > @@ -130,6 +131,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera>
> > > &camera)
> > >  {
> > >  	static const std::array roles{ StreamRole::VideoRecording };
> > >  	g_autoptr(GstCaps) caps = gst_caps_new_empty();
> > > +	g_autoptr(GstStructure) props = gst_structure_new_empty("camera-
> > > properties");
> > 
> > You can move this just before the for loop below.
> > 
> > >  	const gchar *name = camera->id().c_str();
> > >  
> > >  	std::unique_ptr<CameraConfiguration> config = camera-
> > > >generateConfiguration(roles);
> > > @@ -144,12 +146,34 @@ gst_libcamera_device_new(const std::shared_ptr<Camera>
> > > &camera)
> > >  			gst_caps_append(caps, sub_caps);
> > >  	}
> > >  
> > > +	for (const auto &[key, value] : camera->properties()) {
> > > +		const ControlId *id = properties::properties.at(key);
> > > +
> > > +		g_autoptr(GString) prop_str =
> > > g_string_new("api.libcamera.");
> > > +		g_string_append(prop_str, id->name().c_str());
> > > +
> > > +		/* Use string names for enum values for better readability
> > > */
> > 
> > s/readability/readability./
> > 
> > > +		if (value.type() == ControlTypeInteger32) {
> > > +			int32_t val = value.get<int32_t>();
> > > +			const auto &it = id->enumerators().find(val);
> > > +			if (it != id->enumerators().end()) {
> > > +				gst_structure_set(props, prop_str->str,
> > > G_TYPE_STRING,
> > > +						  it->second.c_str(),
> > > nullptr);
> > > +				continue;
> > > +			}
> > > +		}
> > > +
> > > +		gst_structure_set(props, prop_str->str, G_TYPE_STRING,
> > > +				  value.toString().c_str(), nullptr);
> > 
> > Instead of using a libcamera-specific internal string representation of
> > the value, it should be converted to native GStreamer types. Numerical
> > properties should use a numerical type, rectangles should be converted
> > to the native GStreamer representation, ...
> > 
> > Nicolas, any opinion on this ?
> 
> This is the first time we have an API we don't have to fully map manually.
> Looking around, only G_TYPE_UINT and G_TYPE_STRING is currently used by other
> providers. Its likely a good idea for simplicity. Though, when I look at the
> vulkan providers, what they did for flags and enum is to have both, e.g. flags +
> flags_str (no compromise). If you link to Vulkan API, you will find handy the
> value, and otherwise you have a string version which is human readable.

yes, I thought of it initially where I suggested:
	mounting-rotation : <angle_value>
	mounting-rotation-str: "Rotate180" 

> 
> Now, I don't see an "enum" type in the list of libcamera control types. So can
> this really be handled generically ? Here's that list I found:

enums currently get mapped to ControlTypeInteger32 and  currently you cannot
know if a ControlValue is a enum, without going through
ControlId::enumerators() call, which returns a map <int, EnumString>.

> 
> 	ControlTypeBool -> UINT
> 	ControlTypeByte --> BASE64 STRING
> 	ControlTypeUnsigned16 -> UINT
> 	ControlTypeUnsigned32 --> UINT
> 	ControlTypeInteger32 --> UINT

This would be G_TYPE_INT no ?

> 	ControlTypeInteger64 --> UINT64
> 	ControlTypeFloat --> DOUBLE ?
> 	ControlTypeString --> STRING
> 	ControlTypeRectangle --> No match, break in 4 _x, _y, _width, _height?

Are you suggesting an array of 4 values or property breakup for e.g.
	
	api.libcamera.PixelArrayActiveAreaX
	api.libcamera.PixelArrayActiveAreaY
	api.libcamera.PixelArrayActiveAreaW
	api.libcamera.PixelArrayActiveAreaH

> 	ControlTypeSize --> UINT64

Ditto

> 	ControlTypePoint --> No match, break in 2 _x, _y ?
> 
> GST_TYPE_LIST/ARRAY can be used if these comes as an array, you can define your
> own types + serializers also if you want to have rec/point in a single field.
> This is not a GStreamer but a GLib thing, GstStructure holds GValue, and offer
> helper for the most common.

I wonder if we do endup with really complex ContolType* which 
1) won't exactly map to GStreamer/GLib values
2) Their string representation will be complex as well...

On the other hand, camera properties will be relatively simpler than
camera controls ?

> 
> Stepping back, if we consider we need manual mapping, why not go back to just
> exposing the rotation for now, and make it so its easy to map more later ?
> 
> > 
> > > +	}
> > > +
> > >  	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));
> > >  }
> > >
Kieran Bingham July 22, 2025, 12:01 p.m. UTC | #4
Quoting Umang Jain (2025-07-05 06:30:09)
> On Fri, Jul 04, 2025 at 09:40:03AM -0400, Nicolas Dufresne wrote:
> > Le vendredi 04 juillet 2025 à 12:46 +0300, Laurent Pinchart a écrit :
> > > Hi Umang,
> > > 
> > > Thank you for the patch.
> > > 
> > > On Fri, Jul 04, 2025 at 11:04:24AM +0530, Umang Jain wrote:
> > > > Iterate over all libcamera camera properties and report them as
> > > > device properties. The values are reported as strings, including the ones
> > > > which enum types, for better readability.
> > > > 
> > > > Signed-off-by: Umang Jain <uajain@igalia.com>
> > > > ---
> > > > Changes in v3:
> > > > - Iterate over all camera properties and report everything.
> > 
> > Here, outside of commit message, you could pate the output of gst-device-
> > monitor-1.0 Video/Source, that helps to see what kind of output we get.
> 
> [~]$ gst-device-monitor-1.0 
> Probing devices...
> 
> [3:00:30.247078308] [64824]  INFO Camera camera_manager.cpp:326 libcamera v0.5.1+47-12a310d9
> 
> Device found:
> 
>         name  : \_SB_.PC00.XHCI.RHUB.HS04-4:1.0-04f2:b7e0
>         class : Source/Video
>         caps  : image/jpeg, width=640, height=360
>                 image/jpeg, width=640, height=480
>                 image/jpeg, width=848, height=480
>                 image/jpeg, width=960, height=540
>                 image/jpeg, width=1280, height=720
>                 image/jpeg, width=1920, height=1080
>                 video/x-raw, format=YUY2, width=640, height=360
>                 video/x-raw, format=YUY2, width=640, height=480
>         properties:
>                 api.libcamera.SystemDevices = [ 20736 ]
>                 api.libcamera.PixelArrayActiveAreas = [ (0, 0)/1920x1080 ]
>                 api.libcamera.PixelArraySize = 1920x1080
>                 api.libcamera.Location = CameraLocationFront
>                 api.libcamera.Model = Integrated Camera: Integrated C
>         gst-launch-1.0 libcamerasrc camera-name="\\_SB_.PC00.XHCI.RHUB.HS04-4:1.0-04f2:b7e0" ! ...
> 
> Now I realise, rotation is not reported. Because UVC pipeline handler
> doesn't set it. So camera->properties() will only report properties set
> by the pipeline handler.
> 
> Q: Do we want to report *all* properties as per property_ids_core.yaml ?
> And report "Unset" or equivant, in the device provider? 

I would only expect to see properties that are set to be represented.

I suspect 'rotation' is a difficult case for UVC cameras - they can be
freely rotated if they're external?

> > > >  src/gstreamer/gstlibcameraprovider.cpp | 24 ++++++++++++++++++++++++
> > > >  1 file changed, 24 insertions(+)
> > > > 
> > > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp
> > > > b/src/gstreamer/gstlibcameraprovider.cpp
> > > > index 5da96ea3..237baeb2 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"
> > > > @@ -130,6 +131,7 @@ gst_libcamera_device_new(const std::shared_ptr<Camera>
> > > > &camera)
> > > >  {
> > > >   static const std::array roles{ StreamRole::VideoRecording };
> > > >   g_autoptr(GstCaps) caps = gst_caps_new_empty();
> > > > + g_autoptr(GstStructure) props = gst_structure_new_empty("camera-
> > > > properties");
> > > 
> > > You can move this just before the for loop below.
> > > 
> > > >   const gchar *name = camera->id().c_str();
> > > >  
> > > >   std::unique_ptr<CameraConfiguration> config = camera-
> > > > >generateConfiguration(roles);
> > > > @@ -144,12 +146,34 @@ gst_libcamera_device_new(const std::shared_ptr<Camera>
> > > > &camera)
> > > >                   gst_caps_append(caps, sub_caps);
> > > >   }
> > > >  
> > > > + for (const auto &[key, value] : camera->properties()) {
> > > > +         const ControlId *id = properties::properties.at(key);
> > > > +
> > > > +         g_autoptr(GString) prop_str =
> > > > g_string_new("api.libcamera.");
> > > > +         g_string_append(prop_str, id->name().c_str());
> > > > +
> > > > +         /* Use string names for enum values for better readability
> > > > */
> > > 
> > > s/readability/readability./
> > > 
> > > > +         if (value.type() == ControlTypeInteger32) {
> > > > +                 int32_t val = value.get<int32_t>();
> > > > +                 const auto &it = id->enumerators().find(val);

As with the cam patch - it's not if the type is integer that we should
look for enumerators.

It's if the enumerators is not empty - then we should parse them.


> > > > +                 if (it != id->enumerators().end()) {
> > > > +                         gst_structure_set(props, prop_str->str,
> > > > G_TYPE_STRING,
> > > > +                                           it->second.c_str(),
> > > > nullptr);
> > > > +                         continue;
> > > > +                 }
> > > > +         }
> > > > +
> > > > +         gst_structure_set(props, prop_str->str, G_TYPE_STRING,
> > > > +                           value.toString().c_str(), nullptr);
> > > 
> > > Instead of using a libcamera-specific internal string representation of
> > > the value, it should be converted to native GStreamer types. Numerical
> > > properties should use a numerical type, rectangles should be converted
> > > to the native GStreamer representation, ...
> > > 
> > > Nicolas, any opinion on this ?
> > 
> > This is the first time we have an API we don't have to fully map manually.
> > Looking around, only G_TYPE_UINT and G_TYPE_STRING is currently used by other
> > providers. Its likely a good idea for simplicity. Though, when I look at the
> > vulkan providers, what they did for flags and enum is to have both, e.g. flags +
> > flags_str (no compromise). If you link to Vulkan API, you will find handy the
> > value, and otherwise you have a string version which is human readable.
> 
> yes, I thought of it initially where I suggested:
>         mounting-rotation : <angle_value>
>         mounting-rotation-str: "Rotate180" 
> 
> > 
> > Now, I don't see an "enum" type in the list of libcamera control types. So can
> > this really be handled generically ? Here's that list I found:
> 
> enums currently get mapped to ControlTypeInteger32 and  currently you cannot
> know if a ControlValue is a enum, without going through
> ControlId::enumerators() call, which returns a map <int, EnumString>.

But isn't that simply what you should do?


> >       ControlTypeBool -> UINT
> >       ControlTypeByte --> BASE64 STRING
> >       ControlTypeUnsigned16 -> UINT
> >       ControlTypeUnsigned32 --> UINT
> >       ControlTypeInteger32 --> UINT
> 
> This would be G_TYPE_INT no ?
> 
> >       ControlTypeInteger64 --> UINT64
> >       ControlTypeFloat --> DOUBLE ?
> >       ControlTypeString --> STRING
> >       ControlTypeRectangle --> No match, break in 4 _x, _y, _width, _height?
> 
> Are you suggesting an array of 4 values or property breakup for e.g.
>         
>         api.libcamera.PixelArrayActiveAreaX
>         api.libcamera.PixelArrayActiveAreaY
>         api.libcamera.PixelArrayActiveAreaW
>         api.libcamera.PixelArrayActiveAreaH
> 
> >       ControlTypeSize --> UINT64
> 
> Ditto
> 
> >       ControlTypePoint --> No match, break in 2 _x, _y ?
> > 
> > GST_TYPE_LIST/ARRAY can be used if these comes as an array, you can define your
> > own types + serializers also if you want to have rec/point in a single field.
> > This is not a GStreamer but a GLib thing, GstStructure holds GValue, and offer
> > helper for the most common.
> 
> I wonder if we do endup with really complex ContolType* which 
> 1) won't exactly map to GStreamer/GLib values
> 2) Their string representation will be complex as well...
> 
> On the other hand, camera properties will be relatively simpler than
> camera controls ?
> 
> > 
> > Stepping back, if we consider we need manual mapping, why not go back to just
> > exposing the rotation for now, and make it so its easy to map more later ?
> > 
> > > 
> > > > + }
> > > > +
> > > >   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));
> > > >  }
> > > >  
> 
>

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
index 5da96ea3..237baeb2 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"
@@ -130,6 +131,7 @@  gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
 {
 	static const std::array roles{ StreamRole::VideoRecording };
 	g_autoptr(GstCaps) caps = gst_caps_new_empty();
+	g_autoptr(GstStructure) props = gst_structure_new_empty("camera-properties");
 	const gchar *name = camera->id().c_str();
 
 	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
@@ -144,12 +146,34 @@  gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
 			gst_caps_append(caps, sub_caps);
 	}
 
+	for (const auto &[key, value] : camera->properties()) {
+		const ControlId *id = properties::properties.at(key);
+
+		g_autoptr(GString) prop_str = g_string_new("api.libcamera.");
+		g_string_append(prop_str, id->name().c_str());
+
+		/* Use string names for enum values for better readability */
+		if (value.type() == ControlTypeInteger32) {
+			int32_t val = value.get<int32_t>();
+			const auto &it = id->enumerators().find(val);
+			if (it != id->enumerators().end()) {
+				gst_structure_set(props, prop_str->str, G_TYPE_STRING,
+						  it->second.c_str(), nullptr);
+				continue;
+			}
+		}
+
+		gst_structure_set(props, prop_str->str, G_TYPE_STRING,
+				  value.toString().c_str(), 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));
 }