| Message ID | 20200129033210.278800-4-nicolas@ndufresne.ca |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Nicolas, Thank you for the patch. On Tue, Jan 28, 2020 at 10:31:50PM -0500, Nicolas Dufresne wrote: > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > This feature is used with GstDeviceMonitor in order to enumerate > and monitor devices to be used with the source element. The resulting > GstDevice implementation is also used by application to abstract the > configuration of the source element. > > Implementations notes: s/Implementations/Implementation/ > - libcamera does not support polling yet > - The device ID isn't unique in libcamera yet :-) > - The "name" property does not yet exist in libcamerasrc > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > --- > src/gstreamer/gstlibcamera.c | 10 +- > src/gstreamer/gstlibcameraprovider.cpp | 227 +++++++++++++++++++++++++ > src/gstreamer/gstlibcameraprovider.h | 23 +++ > src/gstreamer/meson.build | 1 + > 4 files changed, 259 insertions(+), 2 deletions(-) > create mode 100644 src/gstreamer/gstlibcameraprovider.cpp > create mode 100644 src/gstreamer/gstlibcameraprovider.h > > diff --git a/src/gstreamer/gstlibcamera.c b/src/gstreamer/gstlibcamera.c > index 953fb29..7356374 100644 > --- a/src/gstreamer/gstlibcamera.c > +++ b/src/gstreamer/gstlibcamera.c > @@ -7,12 +7,18 @@ > */ > > #include "gstlibcamerasrc.h" > +#include "gstlibcameraprovider.h" Could you please keep the headers alphabetically sorted ? Please see Documentation/coding-style.rst, section "Order of Includes". > > static gboolean > plugin_init(GstPlugin *plugin) > { > - return gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, > - GST_TYPE_LIBCAMERA_SRC); > + if (!gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, > + GST_TYPE_LIBCAMERA_SRC) || > + !gst_device_provider_register(plugin, "libcameraprovider", > + GST_RANK_PRIMARY, > + GST_TYPE_LIBCAMERA_PROVIDER)) > + return FALSE; > + > return TRUE; > } > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp > new file mode 100644 > index 0000000..3e27912 > --- /dev/null > +++ b/src/gstreamer/gstlibcameraprovider.cpp > @@ -0,0 +1,227 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2020, Collabora Ltd. > + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> > + * > + * gstlibcameraprovider.c - GStreamer Device Provider > + */ > + > +#include "gstlibcamera-utils.h" > +#include "gstlibcameraprovider.h" > +#include "gstlibcamerasrc.h" > + > +#include <libcamera/camera.h> > +#include <libcamera/camera_manager.h> > + > +using namespace libcamera; > + > +GST_DEBUG_CATEGORY_STATIC(provider_debug); > +#define GST_CAT_DEFAULT provider_debug > + > +/**************************************/ > +/* GstLibcameraDevice internal Object */ > +/**************************************/ Is this standard gstreamer coding style ? Would it make sense to instead use a doxygen-style comment with a bit of documentation to describe the object ? /** * \struct _GstLibcameraDevice * \brief ... * * This structure describes ... */ It could be useful for developers who are less familiar with gstreamer (that is, all the rest of us :-)). > + > +enum { > + PROP_DEVICE_NAME = 1, > +}; > + > +#define GST_TYPE_LIBCAMERA_DEVICE gst_libcamera_device_get_type() > +G_DECLARE_FINAL_TYPE(GstLibcameraDevice, gst_libcamera_device, > + GST_LIBCAMERA, DEVICE, GstDevice); > + > +struct _GstLibcameraDevice { > + GstDevice parent; > + gchar *name; > +}; > + > +G_DEFINE_TYPE(GstLibcameraDevice, gst_libcamera_device, GST_TYPE_DEVICE); > + > +static GstElement * > +gst_libcamera_device_create_element(GstDevice *device, const gchar *name) > +{ > + GstElement *source; > + > + source = gst_element_factory_make("libcamerasrc", name); > + g_assert(source); > + > + g_object_set(source, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); > + > + return source; > +} > + > +static gboolean > +gst_libcamera_device_reconfigure_element(GstDevice *device, > + GstElement *element) > +{ > + if (!GST_LIBCAMERA_IS_SRC(element)) > + return FALSE; > + > + g_object_set(element, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); > + > + return TRUE; > +} > + > +static void > +gst_libcamera_device_set_property(GObject *object, guint prop_id, > + const GValue *value, GParamSpec *pspec) > +{ > + GstLibcameraDevice *device; > + > + device = GST_LIBCAMERA_DEVICE(object); You could merge the two lines: GstLibcameraDevice *device = = GST_LIBCAMERA_DEVICE(object); > + > + switch (prop_id) { > + case PROP_DEVICE_NAME: > + device->name = g_value_dup_string(value); > + break; > + default: > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > + break; > + } > +} > + > +static void > +gst_libcamera_device_init(GstLibcameraDevice *self) > +{ > +} > + > +static void > +gst_libcamera_device_finalize(GObject *object) > +{ > + GstLibcameraDevice *self = GST_LIBCAMERA_DEVICE(object); > + gpointer klass = gst_libcamera_device_parent_class; > + > + g_free(self->name); > + > + G_OBJECT_GET_CLASS(klass)->finalize(object); > +} > + > +static void > +gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass) > +{ > + GParamSpec *pspec; Can't this be named just "spec" ? > + GstDeviceClass *device_class = (GstDeviceClass *)klass; Same question as for the previous patch, should this be deviceClass ? > + GObjectClass *object_class = (GObjectClass *)klass; > + > + device_class->create_element = gst_libcamera_device_create_element; > + device_class->reconfigure_element = gst_libcamera_device_reconfigure_element; > + > + object_class->set_property = gst_libcamera_device_set_property; > + object_class->finalize = gst_libcamera_device_finalize; > + > + pspec = g_param_spec_string("name", "Name", As this is C++, we usually don't declare all variables at the top of functions, so this would become GParamSpec *pspec = g_param_spec_string("name", "Name", Up to you. > + "The name of the camera device", "", > + (GParamFlags)(G_PARAM_STATIC_STRINGS | G_PARAM_WRITABLE | > + G_PARAM_CONSTRUCT_ONLY)); > + g_object_class_install_property(object_class, PROP_DEVICE_NAME, pspec); > +} > + > +static GstDevice * > +gst_libcamera_device_new(const std::shared_ptr<Camera> &camera) > +{ > + g_autoptr(GstCaps) caps = gst_caps_new_empty(); > + g_autoptr(GstStructure) props = NULL; As this is always null (as far as I can tell), does it need to be an auto pointer ? > + const gchar *name = camera->name().c_str(); > + StreamRoles roles; > + > + roles.push_back(StreamRole::VideoRecording); > + std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles); > + > + for (const StreamConfiguration &stream_cfg : *config) { > + GstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats()); > + if (sub_caps) > + gst_caps_append(caps, sub_caps); > + } > + > + return (GstDevice *)g_object_new(GST_TYPE_LIBCAMERA_DEVICE, C++ code should use C++ casts instead of C-style casts. This would use static_cast<GstDevice *> (assuming that the GObject is the first member of GstDevice). > + /* FIXME not sure name is unique */ > + "name", name, > + "display-name", name, > + "caps", caps, > + "device-class", "Source/Video", > + "properties", props, > + NULL); > +} > + > +/*************************************/ > +/* GstLibcameraDeviceProvider Object */ > +/*************************************/ > + > +struct _GstLibcameraProvider { > + GstDeviceProvider parent; > + CameraManager *cm; > +}; > + > +G_DEFINE_TYPE_WITH_CODE(GstLibcameraProvider, gst_libcamera_provider, > + GST_TYPE_DEVICE_PROVIDER, > + GST_DEBUG_CATEGORY_INIT(provider_debug, "libcamera-provider", 0, > + "LibCamera Device Provider")); > + > +static GList * > +gst_libcamera_provider_probe(GstDeviceProvider *provider) > +{ > + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider); > + CameraManager *cm = self->cm; > + GList *devices = NULL; > + gint ret; > + > + GST_INFO_OBJECT(self, "Probing cameras using LibCamera"); > + > + /* FIXME as long as the manager isn't able to handle hot-plug, we need to > + * cycle start/stop here in order to ensure wthat we have an up to date s/wthat/that/ > + * list */ This should be /* * FIXME as long as the manager isn't able to handle hot-plug, we need to * cycle start/stop here in order to ensure that we have an up to date * list. */ according to the coding style (period at the end, and opening and closing markers on a line of their own). Even better would be to replace FIXME with \todo as that's the marker we use through the library. > + ret = cm->start(); > + if (ret) { > + GST_ERROR_OBJECT(self, "Failed to retrieve device list: %s", > + g_strerror(-ret)); > + return NULL; > + } > + > + for (const std::shared_ptr<Camera> &camera : cm->cameras()) { > + GST_INFO_OBJECT(self, "Found camera '%s'", camera->name().c_str()); > + devices = g_list_append(devices, > + g_object_ref_sink(gst_libcamera_device_new(camera))); > + } > + > + cm->stop(); > + > + return devices; > +} > + > +static void > +gst_libcamera_provider_init(GstLibcameraProvider *self) > +{ > + GstDeviceProvider *provider = GST_DEVICE_PROVIDER(self); > + > + self->cm = new CameraManager(); > + > + /* Avoid devices being duplicated */ s/duplicated/duplicated./ (This applies to the rest of the series.) > + gst_device_provider_hide_provider(provider, "v4l2deviceprovider"); > +} > + > +static void > +gst_libcamera_provider_finalize(GObject *object) > +{ > + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(object); > + gpointer klass = gst_libcamera_provider_parent_class; > + > + delete self->cm; > + > + return G_OBJECT_GET_CLASS(klass)->finalize(object); > +} > + > +static void > +gst_libcamera_provider_class_init(GstLibcameraProviderClass *klass) > +{ > + GstDeviceProviderClass *provider_class = (GstDeviceProviderClass *)klass; > + GObjectClass *object_class = (GObjectClass *)klass; > + > + provider_class->probe = gst_libcamera_provider_probe; > + object_class->finalize = gst_libcamera_provider_finalize; > + > + gst_device_provider_class_set_metadata(provider_class, > + "LibCamera Device Provider", > + "Source/Video", > + "List camera device using LibCamera", > + "Nicolas Dufresne <nicolas.dufresne@collabora.com>"); > +} > diff --git a/src/gstreamer/gstlibcameraprovider.h b/src/gstreamer/gstlibcameraprovider.h > new file mode 100644 > index 0000000..6dd232d > --- /dev/null > +++ b/src/gstreamer/gstlibcameraprovider.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2020, Collabora Ltd. > + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> > + * > + * gstlibcameraprovider.h - GStreamer Device Provider > + */ > + > +#include <gst/gst.h> > + > +#ifndef __GST_LIBCAMERA_PROVIDER_H__ > +#define __GST_LIBCAMERA_PROVIDER_H__ > + > +G_BEGIN_DECLS > + > +#define GST_TYPE_LIBCAMERA_PROVIDER gst_libcamera_provider_get_type() > +G_DECLARE_FINAL_TYPE(GstLibcameraProvider, gst_libcamera_provider, > + GST_LIBCAMERA, PROVIDER, GstDeviceProvider) > + > +G_END_DECLS > + > +#endif /* __GST_LIBCAMERA_PROVIDER_H__ */ > + > diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build > index 39a34e7..7769b78 100644 > --- a/src/gstreamer/meson.build > +++ b/src/gstreamer/meson.build > @@ -2,6 +2,7 @@ libcamera_gst_sources = [ > 'gstlibcamera.c', > 'gstlibcamera-utils.cpp', > 'gstlibcamerasrc.cpp', > + 'gstlibcameraprovider.cpp', Alphabetically sorted here too ? > ] > > libcamera_gst_c_args = [
Le mardi 11 février 2020 à 01:28 +0200, Laurent Pinchart a écrit : > Hi Nicolas, > > Thank you for the patch. > > On Tue, Jan 28, 2020 at 10:31:50PM -0500, Nicolas Dufresne wrote: > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > This feature is used with GstDeviceMonitor in order to enumerate > > and monitor devices to be used with the source element. The resulting > > GstDevice implementation is also used by application to abstract the > > configuration of the source element. > > > > Implementations notes: > > s/Implementations/Implementation/ > > > - libcamera does not support polling yet > > - The device ID isn't unique in libcamera > > yet :-) > > > - The "name" property does not yet exist in libcamerasrc > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > --- > > src/gstreamer/gstlibcamera.c | 10 +- > > src/gstreamer/gstlibcameraprovider.cpp | 227 +++++++++++++++++++++++++ > > src/gstreamer/gstlibcameraprovider.h | 23 +++ > > src/gstreamer/meson.build | 1 + > > 4 files changed, 259 insertions(+), 2 deletions(-) > > create mode 100644 src/gstreamer/gstlibcameraprovider.cpp > > create mode 100644 src/gstreamer/gstlibcameraprovider.h > > > > diff --git a/src/gstreamer/gstlibcamera.c b/src/gstreamer/gstlibcamera.c > > index 953fb29..7356374 100644 > > --- a/src/gstreamer/gstlibcamera.c > > +++ b/src/gstreamer/gstlibcamera.c > > @@ -7,12 +7,18 @@ > > */ > > > > #include "gstlibcamerasrc.h" > > +#include "gstlibcameraprovider.h" > > Could you please keep the headers alphabetically sorted ? Please see > Documentation/coding-style.rst, section "Order of Includes". > > > > > static gboolean > > plugin_init(GstPlugin *plugin) > > { > > - return gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, > > - GST_TYPE_LIBCAMERA_SRC); > > + if (!gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, > > + GST_TYPE_LIBCAMERA_SRC) || > > + !gst_device_provider_register(plugin, "libcameraprovider", > > + GST_RANK_PRIMARY, > > + GST_TYPE_LIBCAMERA_PROVIDER)) > > + return FALSE; > > + > > return TRUE; > > } > > > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp > > new file mode 100644 > > index 0000000..3e27912 > > --- /dev/null > > +++ b/src/gstreamer/gstlibcameraprovider.cpp > > @@ -0,0 +1,227 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2020, Collabora Ltd. > > + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > + * > > + * gstlibcameraprovider.c - GStreamer Device Provider > > + */ > > + > > +#include "gstlibcamera-utils.h" > > +#include "gstlibcameraprovider.h" > > +#include "gstlibcamerasrc.h" > > + > > +#include <libcamera/camera.h> > > +#include <libcamera/camera_manager.h> > > + > > +using namespace libcamera; > > + > > +GST_DEBUG_CATEGORY_STATIC(provider_debug); > > +#define GST_CAT_DEFAULT provider_debug > > + > > +/**************************************/ > > +/* GstLibcameraDevice internal Object */ > > +/**************************************/ > > Is this standard gstreamer coding style ? Would it make sense to instead > use a doxygen-style comment with a bit of documentation to describe the > object ? > > /** > * \struct _GstLibcameraDevice > * \brief ... > * > * This structure describes ... > */ > > It could be useful for developers who are less familiar with gstreamer > (that is, all the rest of us :-)). > > > + > > +enum { > > + PROP_DEVICE_NAME = 1, > > +}; > > + > > +#define GST_TYPE_LIBCAMERA_DEVICE gst_libcamera_device_get_type() > > +G_DECLARE_FINAL_TYPE(GstLibcameraDevice, gst_libcamera_device, > > + GST_LIBCAMERA, DEVICE, GstDevice); > > + > > +struct _GstLibcameraDevice { > > + GstDevice parent; > > + gchar *name; > > +}; > > + > > +G_DEFINE_TYPE(GstLibcameraDevice, gst_libcamera_device, GST_TYPE_DEVICE); > > + > > +static GstElement * > > +gst_libcamera_device_create_element(GstDevice *device, const gchar *name) > > +{ > > + GstElement *source; > > + > > + source = gst_element_factory_make("libcamerasrc", name); > > + g_assert(source); > > + > > + g_object_set(source, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); > > + > > + return source; > > +} > > + > > +static gboolean > > +gst_libcamera_device_reconfigure_element(GstDevice *device, > > + GstElement *element) > > +{ > > + if (!GST_LIBCAMERA_IS_SRC(element)) > > + return FALSE; > > + > > + g_object_set(element, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); > > + > > + return TRUE; > > +} > > + > > +static void > > +gst_libcamera_device_set_property(GObject *object, guint prop_id, > > + const GValue *value, GParamSpec *pspec) > > +{ > > + GstLibcameraDevice *device; > > + > > + device = GST_LIBCAMERA_DEVICE(object); > > You could merge the two lines: > > GstLibcameraDevice *device = = GST_LIBCAMERA_DEVICE(object); > > > + > > + switch (prop_id) { > > + case PROP_DEVICE_NAME: > > + device->name = g_value_dup_string(value); > > + break; > > + default: > > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > > + break; > > + } > > +} > > + > > +static void > > +gst_libcamera_device_init(GstLibcameraDevice *self) > > +{ > > +} > > + > > +static void > > +gst_libcamera_device_finalize(GObject *object) > > +{ > > + GstLibcameraDevice *self = GST_LIBCAMERA_DEVICE(object); > > + gpointer klass = gst_libcamera_device_parent_class; > > + > > + g_free(self->name); > > + > > + G_OBJECT_GET_CLASS(klass)->finalize(object); > > +} > > + > > +static void > > +gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass) > > +{ > > + GParamSpec *pspec; > > Can't this be named just "spec" ? > > > + GstDeviceClass *device_class = (GstDeviceClass *)klass; > > Same question as for the previous patch, should this be deviceClass ? > > > + GObjectClass *object_class = (GObjectClass *)klass; > > + > > + device_class->create_element = gst_libcamera_device_create_element; > > + device_class->reconfigure_element = gst_libcamera_device_reconfigure_element; > > + > > + object_class->set_property = gst_libcamera_device_set_property; > > + object_class->finalize = gst_libcamera_device_finalize; > > + > > + pspec = g_param_spec_string("name", "Name", > > As this is C++, we usually don't declare all variables at the top of > functions, so this would become > > GParamSpec *pspec = g_param_spec_string("name", "Name", > > Up to you. > > > + "The name of the camera device", "", > > + (GParamFlags)(G_PARAM_STATIC_STRINGS | G_PARAM_WRITABLE | > > + G_PARAM_CONSTRUCT_ONLY)); > > + g_object_class_install_property(object_class, PROP_DEVICE_NAME, pspec); > > +} > > + > > +static GstDevice * > > +gst_libcamera_device_new(const std::shared_ptr<Camera> &camera) > > +{ > > + g_autoptr(GstCaps) caps = gst_caps_new_empty(); > > + g_autoptr(GstStructure) props = NULL; > > As this is always null (as far as I can tell), does it need to be an > auto pointer ? > > > + const gchar *name = camera->name().c_str(); > > + StreamRoles roles; > > + > > + roles.push_back(StreamRole::VideoRecording); > > + std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles); > > + > > + for (const StreamConfiguration &stream_cfg : *config) { > > + GstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats()); > > + if (sub_caps) > > + gst_caps_append(caps, sub_caps); > > + } > > + > > + return (GstDevice *)g_object_new(GST_TYPE_LIBCAMERA_DEVICE, > > C++ code should use C++ casts instead of C-style casts. This would use > static_cast<GstDevice *> (assuming that the GObject is the first member > of GstDevice). It seems that static_cast don't work for void* to something. I could use the type checking GST_LIBCAMERA_DEVICE() cast, it's overlay safe though. In C with gcc/clang we instrument the g_object_new() function so the compiler will warn if the implicit cast is wrong, but in C++ you are forced to cast, which forces us to make it type unsafe (or use runtime checks). static_cast also don't generally work, since you cannot downcast a type, and you cannot cast if the structure definition is not public. > > > + /* FIXME not sure name is unique */ > > + "name", name, > > + "display-name", name, > > + "caps", caps, > > + "device-class", "Source/Video", > > + "properties", props, > > + NULL); > > +} > > + > > +/*************************************/ > > +/* GstLibcameraDeviceProvider Object */ > > +/*************************************/ > > + > > +struct _GstLibcameraProvider { > > + GstDeviceProvider parent; > > + CameraManager *cm; > > +}; > > + > > +G_DEFINE_TYPE_WITH_CODE(GstLibcameraProvider, gst_libcamera_provider, > > + GST_TYPE_DEVICE_PROVIDER, > > + GST_DEBUG_CATEGORY_INIT(provider_debug, "libcamera-provider", 0, > > + "LibCamera Device Provider")); > > + > > +static GList * > > +gst_libcamera_provider_probe(GstDeviceProvider *provider) > > +{ > > + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider); > > + CameraManager *cm = self->cm; > > + GList *devices = NULL; > > + gint ret; > > + > > + GST_INFO_OBJECT(self, "Probing cameras using LibCamera"); > > + > > + /* FIXME as long as the manager isn't able to handle hot-plug, we need to > > + * cycle start/stop here in order to ensure wthat we have an up to date > > s/wthat/that/ > > > + * list */ > > This should be > > /* > * FIXME as long as the manager isn't able to handle hot-plug, we need to > * cycle start/stop here in order to ensure that we have an up to date > * list. > */ > > according to the coding style (period at the end, and opening and > closing markers on a line of their own). Even better would be to replace > FIXME with \todo as that's the marker we use through the library. > > > + ret = cm->start(); > > + if (ret) { > > + GST_ERROR_OBJECT(self, "Failed to retrieve device list: %s", > > + g_strerror(-ret)); > > + return NULL; > > + } > > + > > + for (const std::shared_ptr<Camera> &camera : cm->cameras()) { > > + GST_INFO_OBJECT(self, "Found camera '%s'", camera->name().c_str()); > > + devices = g_list_append(devices, > > + g_object_ref_sink(gst_libcamera_device_new(camera))); > > + } > > + > > + cm->stop(); > > + > > + return devices; > > +} > > + > > +static void > > +gst_libcamera_provider_init(GstLibcameraProvider *self) > > +{ > > + GstDeviceProvider *provider = GST_DEVICE_PROVIDER(self); > > + > > + self->cm = new CameraManager(); > > + > > + /* Avoid devices being duplicated */ > > s/duplicated/duplicated./ > > (This applies to the rest of the series.) > > > + gst_device_provider_hide_provider(provider, "v4l2deviceprovider"); > > +} > > + > > +static void > > +gst_libcamera_provider_finalize(GObject *object) > > +{ > > + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(object); > > + gpointer klass = gst_libcamera_provider_parent_class; > > + > > + delete self->cm; > > + > > + return G_OBJECT_GET_CLASS(klass)->finalize(object); > > +} > > + > > +static void > > +gst_libcamera_provider_class_init(GstLibcameraProviderClass *klass) > > +{ > > + GstDeviceProviderClass *provider_class = (GstDeviceProviderClass *)klass; > > + GObjectClass *object_class = (GObjectClass *)klass; > > + > > + provider_class->probe = gst_libcamera_provider_probe; > > + object_class->finalize = gst_libcamera_provider_finalize; > > + > > + gst_device_provider_class_set_metadata(provider_class, > > + "LibCamera Device Provider", > > + "Source/Video", > > + "List camera device using LibCamera", > > + "Nicolas Dufresne <nicolas.dufresne@collabora.com>"); > > +} > > diff --git a/src/gstreamer/gstlibcameraprovider.h b/src/gstreamer/gstlibcameraprovider.h > > new file mode 100644 > > index 0000000..6dd232d > > --- /dev/null > > +++ b/src/gstreamer/gstlibcameraprovider.h > > @@ -0,0 +1,23 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2020, Collabora Ltd. > > + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > + * > > + * gstlibcameraprovider.h - GStreamer Device Provider > > + */ > > + > > +#include <gst/gst.h> > > + > > +#ifndef __GST_LIBCAMERA_PROVIDER_H__ > > +#define __GST_LIBCAMERA_PROVIDER_H__ > > + > > +G_BEGIN_DECLS > > + > > +#define GST_TYPE_LIBCAMERA_PROVIDER gst_libcamera_provider_get_type() > > +G_DECLARE_FINAL_TYPE(GstLibcameraProvider, gst_libcamera_provider, > > + GST_LIBCAMERA, PROVIDER, GstDeviceProvider) > > + > > +G_END_DECLS > > + > > +#endif /* __GST_LIBCAMERA_PROVIDER_H__ */ > > + > > diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build > > index 39a34e7..7769b78 100644 > > --- a/src/gstreamer/meson.build > > +++ b/src/gstreamer/meson.build > > @@ -2,6 +2,7 @@ libcamera_gst_sources = [ > > 'gstlibcamera.c', > > 'gstlibcamera-utils.cpp', > > 'gstlibcamerasrc.cpp', > > + 'gstlibcameraprovider.cpp', > > Alphabetically sorted here too ? > > > ] > > > > libcamera_gst_c_args = [
Hi Nicolas, On Tue, Feb 25, 2020 at 12:39:52PM -0500, Nicolas Dufresne wrote: > Le mardi 11 février 2020 à 01:28 +0200, Laurent Pinchart a écrit : > > On Tue, Jan 28, 2020 at 10:31:50PM -0500, Nicolas Dufresne wrote: > > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > > This feature is used with GstDeviceMonitor in order to enumerate > > > and monitor devices to be used with the source element. The resulting > > > GstDevice implementation is also used by application to abstract the > > > configuration of the source element. > > > > > > Implementations notes: > > > > s/Implementations/Implementation/ > > > > > - libcamera does not support polling yet > > > - The device ID isn't unique in libcamera > > > > yet :-) > > > > > - The "name" property does not yet exist in libcamerasrc > > > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > --- > > > src/gstreamer/gstlibcamera.c | 10 +- > > > src/gstreamer/gstlibcameraprovider.cpp | 227 +++++++++++++++++++++++++ > > > src/gstreamer/gstlibcameraprovider.h | 23 +++ > > > src/gstreamer/meson.build | 1 + > > > 4 files changed, 259 insertions(+), 2 deletions(-) > > > create mode 100644 src/gstreamer/gstlibcameraprovider.cpp > > > create mode 100644 src/gstreamer/gstlibcameraprovider.h > > > > > > diff --git a/src/gstreamer/gstlibcamera.c b/src/gstreamer/gstlibcamera.c > > > index 953fb29..7356374 100644 > > > --- a/src/gstreamer/gstlibcamera.c > > > +++ b/src/gstreamer/gstlibcamera.c > > > @@ -7,12 +7,18 @@ > > > */ > > > > > > #include "gstlibcamerasrc.h" > > > +#include "gstlibcameraprovider.h" > > > > Could you please keep the headers alphabetically sorted ? Please see > > Documentation/coding-style.rst, section "Order of Includes". > > > > > > > > static gboolean > > > plugin_init(GstPlugin *plugin) > > > { > > > - return gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, > > > - GST_TYPE_LIBCAMERA_SRC); > > > + if (!gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, > > > + GST_TYPE_LIBCAMERA_SRC) || > > > + !gst_device_provider_register(plugin, "libcameraprovider", > > > + GST_RANK_PRIMARY, > > > + GST_TYPE_LIBCAMERA_PROVIDER)) > > > + return FALSE; > > > + > > > return TRUE; > > > } > > > > > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp > > > new file mode 100644 > > > index 0000000..3e27912 > > > --- /dev/null > > > +++ b/src/gstreamer/gstlibcameraprovider.cpp > > > @@ -0,0 +1,227 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2020, Collabora Ltd. > > > + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > + * > > > + * gstlibcameraprovider.c - GStreamer Device Provider > > > + */ > > > + > > > +#include "gstlibcamera-utils.h" > > > +#include "gstlibcameraprovider.h" > > > +#include "gstlibcamerasrc.h" > > > + > > > +#include <libcamera/camera.h> > > > +#include <libcamera/camera_manager.h> > > > + > > > +using namespace libcamera; > > > + > > > +GST_DEBUG_CATEGORY_STATIC(provider_debug); > > > +#define GST_CAT_DEFAULT provider_debug > > > + > > > +/**************************************/ > > > +/* GstLibcameraDevice internal Object */ > > > +/**************************************/ > > > > Is this standard gstreamer coding style ? Would it make sense to instead > > use a doxygen-style comment with a bit of documentation to describe the > > object ? > > > > /** > > * \struct _GstLibcameraDevice > > * \brief ... > > * > > * This structure describes ... > > */ > > > > It could be useful for developers who are less familiar with gstreamer > > (that is, all the rest of us :-)). > > > > > + > > > +enum { > > > + PROP_DEVICE_NAME = 1, > > > +}; > > > + > > > +#define GST_TYPE_LIBCAMERA_DEVICE gst_libcamera_device_get_type() > > > +G_DECLARE_FINAL_TYPE(GstLibcameraDevice, gst_libcamera_device, > > > + GST_LIBCAMERA, DEVICE, GstDevice); > > > + > > > +struct _GstLibcameraDevice { > > > + GstDevice parent; > > > + gchar *name; > > > +}; > > > + > > > +G_DEFINE_TYPE(GstLibcameraDevice, gst_libcamera_device, GST_TYPE_DEVICE); > > > + > > > +static GstElement * > > > +gst_libcamera_device_create_element(GstDevice *device, const gchar *name) > > > +{ > > > + GstElement *source; > > > + > > > + source = gst_element_factory_make("libcamerasrc", name); > > > + g_assert(source); > > > + > > > + g_object_set(source, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); > > > + > > > + return source; > > > +} > > > + > > > +static gboolean > > > +gst_libcamera_device_reconfigure_element(GstDevice *device, > > > + GstElement *element) > > > +{ > > > + if (!GST_LIBCAMERA_IS_SRC(element)) > > > + return FALSE; > > > + > > > + g_object_set(element, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); > > > + > > > + return TRUE; > > > +} > > > + > > > +static void > > > +gst_libcamera_device_set_property(GObject *object, guint prop_id, > > > + const GValue *value, GParamSpec *pspec) > > > +{ > > > + GstLibcameraDevice *device; > > > + > > > + device = GST_LIBCAMERA_DEVICE(object); > > > > You could merge the two lines: > > > > GstLibcameraDevice *device = = GST_LIBCAMERA_DEVICE(object); > > > > > + > > > + switch (prop_id) { > > > + case PROP_DEVICE_NAME: > > > + device->name = g_value_dup_string(value); > > > + break; > > > + default: > > > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > > > + break; > > > + } > > > +} > > > + > > > +static void > > > +gst_libcamera_device_init(GstLibcameraDevice *self) > > > +{ > > > +} > > > + > > > +static void > > > +gst_libcamera_device_finalize(GObject *object) > > > +{ > > > + GstLibcameraDevice *self = GST_LIBCAMERA_DEVICE(object); > > > + gpointer klass = gst_libcamera_device_parent_class; > > > + > > > + g_free(self->name); > > > + > > > + G_OBJECT_GET_CLASS(klass)->finalize(object); > > > +} > > > + > > > +static void > > > +gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass) > > > +{ > > > + GParamSpec *pspec; > > > > Can't this be named just "spec" ? > > > > > + GstDeviceClass *device_class = (GstDeviceClass *)klass; > > > > Same question as for the previous patch, should this be deviceClass ? > > > > > + GObjectClass *object_class = (GObjectClass *)klass; > > > + > > > + device_class->create_element = gst_libcamera_device_create_element; > > > + device_class->reconfigure_element = gst_libcamera_device_reconfigure_element; > > > + > > > + object_class->set_property = gst_libcamera_device_set_property; > > > + object_class->finalize = gst_libcamera_device_finalize; > > > + > > > + pspec = g_param_spec_string("name", "Name", > > > > As this is C++, we usually don't declare all variables at the top of > > functions, so this would become > > > > GParamSpec *pspec = g_param_spec_string("name", "Name", > > > > Up to you. > > > > > + "The name of the camera device", "", > > > + (GParamFlags)(G_PARAM_STATIC_STRINGS | G_PARAM_WRITABLE | > > > + G_PARAM_CONSTRUCT_ONLY)); > > > + g_object_class_install_property(object_class, PROP_DEVICE_NAME, pspec); > > > +} > > > + > > > +static GstDevice * > > > +gst_libcamera_device_new(const std::shared_ptr<Camera> &camera) > > > +{ > > > + g_autoptr(GstCaps) caps = gst_caps_new_empty(); > > > + g_autoptr(GstStructure) props = NULL; > > > > As this is always null (as far as I can tell), does it need to be an > > auto pointer ? > > > > > + const gchar *name = camera->name().c_str(); > > > + StreamRoles roles; > > > + > > > + roles.push_back(StreamRole::VideoRecording); > > > + std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles); > > > + > > > + for (const StreamConfiguration &stream_cfg : *config) { > > > + GstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats()); > > > + if (sub_caps) > > > + gst_caps_append(caps, sub_caps); > > > + } > > > + > > > + return (GstDevice *)g_object_new(GST_TYPE_LIBCAMERA_DEVICE, > > > > C++ code should use C++ casts instead of C-style casts. This would use > > static_cast<GstDevice *> (assuming that the GObject is the first member > > of GstDevice). > > It seems that static_cast don't work for void* to something. I could > use the type checking GST_LIBCAMERA_DEVICE() cast, it's overlay safe > though. In C with gcc/clang we instrument the g_object_new() function > so the compiler will warn if the implicit cast is wrong, but in C++ you > are forced to cast, which forces us to make it type unsafe (or use > runtime checks). > > static_cast also don't generally work, since you cannot downcast a > type, and you cannot cast if the structure definition is not public. static_cast indeed requires definitions of both types. You can use a GStreamer- or glib-specific casting macro if you have one, or alternatively use reinterpret_cast<GstDevice *> in this specific case. > > > + /* FIXME not sure name is unique */ > > > + "name", name, > > > + "display-name", name, > > > + "caps", caps, > > > + "device-class", "Source/Video", > > > + "properties", props, > > > + NULL); > > > +} > > > + > > > +/*************************************/ > > > +/* GstLibcameraDeviceProvider Object */ > > > +/*************************************/ > > > + > > > +struct _GstLibcameraProvider { > > > + GstDeviceProvider parent; > > > + CameraManager *cm; > > > +}; > > > + > > > +G_DEFINE_TYPE_WITH_CODE(GstLibcameraProvider, gst_libcamera_provider, > > > + GST_TYPE_DEVICE_PROVIDER, > > > + GST_DEBUG_CATEGORY_INIT(provider_debug, "libcamera-provider", 0, > > > + "LibCamera Device Provider")); > > > + > > > +static GList * > > > +gst_libcamera_provider_probe(GstDeviceProvider *provider) > > > +{ > > > + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider); > > > + CameraManager *cm = self->cm; > > > + GList *devices = NULL; > > > + gint ret; > > > + > > > + GST_INFO_OBJECT(self, "Probing cameras using LibCamera"); > > > + > > > + /* FIXME as long as the manager isn't able to handle hot-plug, we need to > > > + * cycle start/stop here in order to ensure wthat we have an up to date > > > > s/wthat/that/ > > > > > + * list */ > > > > This should be > > > > /* > > * FIXME as long as the manager isn't able to handle hot-plug, we need to > > * cycle start/stop here in order to ensure that we have an up to date > > * list. > > */ > > > > according to the coding style (period at the end, and opening and > > closing markers on a line of their own). Even better would be to replace > > FIXME with \todo as that's the marker we use through the library. > > > > > + ret = cm->start(); > > > + if (ret) { > > > + GST_ERROR_OBJECT(self, "Failed to retrieve device list: %s", > > > + g_strerror(-ret)); > > > + return NULL; > > > + } > > > + > > > + for (const std::shared_ptr<Camera> &camera : cm->cameras()) { > > > + GST_INFO_OBJECT(self, "Found camera '%s'", camera->name().c_str()); > > > + devices = g_list_append(devices, > > > + g_object_ref_sink(gst_libcamera_device_new(camera))); > > > + } > > > + > > > + cm->stop(); > > > + > > > + return devices; > > > +} > > > + > > > +static void > > > +gst_libcamera_provider_init(GstLibcameraProvider *self) > > > +{ > > > + GstDeviceProvider *provider = GST_DEVICE_PROVIDER(self); > > > + > > > + self->cm = new CameraManager(); > > > + > > > + /* Avoid devices being duplicated */ > > > > s/duplicated/duplicated./ > > > > (This applies to the rest of the series.) > > > > > + gst_device_provider_hide_provider(provider, "v4l2deviceprovider"); > > > +} > > > + > > > +static void > > > +gst_libcamera_provider_finalize(GObject *object) > > > +{ > > > + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(object); > > > + gpointer klass = gst_libcamera_provider_parent_class; > > > + > > > + delete self->cm; > > > + > > > + return G_OBJECT_GET_CLASS(klass)->finalize(object); > > > +} > > > + > > > +static void > > > +gst_libcamera_provider_class_init(GstLibcameraProviderClass *klass) > > > +{ > > > + GstDeviceProviderClass *provider_class = (GstDeviceProviderClass *)klass; > > > + GObjectClass *object_class = (GObjectClass *)klass; > > > + > > > + provider_class->probe = gst_libcamera_provider_probe; > > > + object_class->finalize = gst_libcamera_provider_finalize; > > > + > > > + gst_device_provider_class_set_metadata(provider_class, > > > + "LibCamera Device Provider", > > > + "Source/Video", > > > + "List camera device using LibCamera", > > > + "Nicolas Dufresne <nicolas.dufresne@collabora.com>"); > > > +} > > > diff --git a/src/gstreamer/gstlibcameraprovider.h b/src/gstreamer/gstlibcameraprovider.h > > > new file mode 100644 > > > index 0000000..6dd232d > > > --- /dev/null > > > +++ b/src/gstreamer/gstlibcameraprovider.h > > > @@ -0,0 +1,23 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2020, Collabora Ltd. > > > + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > + * > > > + * gstlibcameraprovider.h - GStreamer Device Provider > > > + */ > > > + > > > +#include <gst/gst.h> > > > + > > > +#ifndef __GST_LIBCAMERA_PROVIDER_H__ > > > +#define __GST_LIBCAMERA_PROVIDER_H__ > > > + > > > +G_BEGIN_DECLS > > > + > > > +#define GST_TYPE_LIBCAMERA_PROVIDER gst_libcamera_provider_get_type() > > > +G_DECLARE_FINAL_TYPE(GstLibcameraProvider, gst_libcamera_provider, > > > + GST_LIBCAMERA, PROVIDER, GstDeviceProvider) > > > + > > > +G_END_DECLS > > > + > > > +#endif /* __GST_LIBCAMERA_PROVIDER_H__ */ > > > + > > > diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build > > > index 39a34e7..7769b78 100644 > > > --- a/src/gstreamer/meson.build > > > +++ b/src/gstreamer/meson.build > > > @@ -2,6 +2,7 @@ libcamera_gst_sources = [ > > > 'gstlibcamera.c', > > > 'gstlibcamera-utils.cpp', > > > 'gstlibcamerasrc.cpp', > > > + 'gstlibcameraprovider.cpp', > > > > Alphabetically sorted here too ? > > > > > ] > > > > > > libcamera_gst_c_args = [
diff --git a/src/gstreamer/gstlibcamera.c b/src/gstreamer/gstlibcamera.c index 953fb29..7356374 100644 --- a/src/gstreamer/gstlibcamera.c +++ b/src/gstreamer/gstlibcamera.c @@ -7,12 +7,18 @@ */ #include "gstlibcamerasrc.h" +#include "gstlibcameraprovider.h" static gboolean plugin_init(GstPlugin *plugin) { - return gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, - GST_TYPE_LIBCAMERA_SRC); + if (!gst_element_register(plugin, "libcamerasrc", GST_RANK_PRIMARY, + GST_TYPE_LIBCAMERA_SRC) || + !gst_device_provider_register(plugin, "libcameraprovider", + GST_RANK_PRIMARY, + GST_TYPE_LIBCAMERA_PROVIDER)) + return FALSE; + return TRUE; } diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp new file mode 100644 index 0000000..3e27912 --- /dev/null +++ b/src/gstreamer/gstlibcameraprovider.cpp @@ -0,0 +1,227 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Collabora Ltd. + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> + * + * gstlibcameraprovider.c - GStreamer Device Provider + */ + +#include "gstlibcamera-utils.h" +#include "gstlibcameraprovider.h" +#include "gstlibcamerasrc.h" + +#include <libcamera/camera.h> +#include <libcamera/camera_manager.h> + +using namespace libcamera; + +GST_DEBUG_CATEGORY_STATIC(provider_debug); +#define GST_CAT_DEFAULT provider_debug + +/**************************************/ +/* GstLibcameraDevice internal Object */ +/**************************************/ + +enum { + PROP_DEVICE_NAME = 1, +}; + +#define GST_TYPE_LIBCAMERA_DEVICE gst_libcamera_device_get_type() +G_DECLARE_FINAL_TYPE(GstLibcameraDevice, gst_libcamera_device, + GST_LIBCAMERA, DEVICE, GstDevice); + +struct _GstLibcameraDevice { + GstDevice parent; + gchar *name; +}; + +G_DEFINE_TYPE(GstLibcameraDevice, gst_libcamera_device, GST_TYPE_DEVICE); + +static GstElement * +gst_libcamera_device_create_element(GstDevice *device, const gchar *name) +{ + GstElement *source; + + source = gst_element_factory_make("libcamerasrc", name); + g_assert(source); + + g_object_set(source, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); + + return source; +} + +static gboolean +gst_libcamera_device_reconfigure_element(GstDevice *device, + GstElement *element) +{ + if (!GST_LIBCAMERA_IS_SRC(element)) + return FALSE; + + g_object_set(element, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, NULL); + + return TRUE; +} + +static void +gst_libcamera_device_set_property(GObject *object, guint prop_id, + const GValue *value, GParamSpec *pspec) +{ + GstLibcameraDevice *device; + + device = GST_LIBCAMERA_DEVICE(object); + + switch (prop_id) { + case PROP_DEVICE_NAME: + device->name = g_value_dup_string(value); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + break; + } +} + +static void +gst_libcamera_device_init(GstLibcameraDevice *self) +{ +} + +static void +gst_libcamera_device_finalize(GObject *object) +{ + GstLibcameraDevice *self = GST_LIBCAMERA_DEVICE(object); + gpointer klass = gst_libcamera_device_parent_class; + + g_free(self->name); + + G_OBJECT_GET_CLASS(klass)->finalize(object); +} + +static void +gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass) +{ + GParamSpec *pspec; + GstDeviceClass *device_class = (GstDeviceClass *)klass; + GObjectClass *object_class = (GObjectClass *)klass; + + device_class->create_element = gst_libcamera_device_create_element; + device_class->reconfigure_element = gst_libcamera_device_reconfigure_element; + + object_class->set_property = gst_libcamera_device_set_property; + object_class->finalize = gst_libcamera_device_finalize; + + pspec = g_param_spec_string("name", "Name", + "The name of the camera device", "", + (GParamFlags)(G_PARAM_STATIC_STRINGS | G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY)); + g_object_class_install_property(object_class, PROP_DEVICE_NAME, pspec); +} + +static GstDevice * +gst_libcamera_device_new(const std::shared_ptr<Camera> &camera) +{ + g_autoptr(GstCaps) caps = gst_caps_new_empty(); + g_autoptr(GstStructure) props = NULL; + const gchar *name = camera->name().c_str(); + StreamRoles roles; + + roles.push_back(StreamRole::VideoRecording); + std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles); + + for (const StreamConfiguration &stream_cfg : *config) { + GstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats()); + if (sub_caps) + gst_caps_append(caps, sub_caps); + } + + return (GstDevice *)g_object_new(GST_TYPE_LIBCAMERA_DEVICE, + /* FIXME not sure name is unique */ + "name", name, + "display-name", name, + "caps", caps, + "device-class", "Source/Video", + "properties", props, + NULL); +} + +/*************************************/ +/* GstLibcameraDeviceProvider Object */ +/*************************************/ + +struct _GstLibcameraProvider { + GstDeviceProvider parent; + CameraManager *cm; +}; + +G_DEFINE_TYPE_WITH_CODE(GstLibcameraProvider, gst_libcamera_provider, + GST_TYPE_DEVICE_PROVIDER, + GST_DEBUG_CATEGORY_INIT(provider_debug, "libcamera-provider", 0, + "LibCamera Device Provider")); + +static GList * +gst_libcamera_provider_probe(GstDeviceProvider *provider) +{ + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider); + CameraManager *cm = self->cm; + GList *devices = NULL; + gint ret; + + GST_INFO_OBJECT(self, "Probing cameras using LibCamera"); + + /* FIXME as long as the manager isn't able to handle hot-plug, we need to + * cycle start/stop here in order to ensure wthat we have an up to date + * list */ + ret = cm->start(); + if (ret) { + GST_ERROR_OBJECT(self, "Failed to retrieve device list: %s", + g_strerror(-ret)); + return NULL; + } + + for (const std::shared_ptr<Camera> &camera : cm->cameras()) { + GST_INFO_OBJECT(self, "Found camera '%s'", camera->name().c_str()); + devices = g_list_append(devices, + g_object_ref_sink(gst_libcamera_device_new(camera))); + } + + cm->stop(); + + return devices; +} + +static void +gst_libcamera_provider_init(GstLibcameraProvider *self) +{ + GstDeviceProvider *provider = GST_DEVICE_PROVIDER(self); + + self->cm = new CameraManager(); + + /* Avoid devices being duplicated */ + gst_device_provider_hide_provider(provider, "v4l2deviceprovider"); +} + +static void +gst_libcamera_provider_finalize(GObject *object) +{ + GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(object); + gpointer klass = gst_libcamera_provider_parent_class; + + delete self->cm; + + return G_OBJECT_GET_CLASS(klass)->finalize(object); +} + +static void +gst_libcamera_provider_class_init(GstLibcameraProviderClass *klass) +{ + GstDeviceProviderClass *provider_class = (GstDeviceProviderClass *)klass; + GObjectClass *object_class = (GObjectClass *)klass; + + provider_class->probe = gst_libcamera_provider_probe; + object_class->finalize = gst_libcamera_provider_finalize; + + gst_device_provider_class_set_metadata(provider_class, + "LibCamera Device Provider", + "Source/Video", + "List camera device using LibCamera", + "Nicolas Dufresne <nicolas.dufresne@collabora.com>"); +} diff --git a/src/gstreamer/gstlibcameraprovider.h b/src/gstreamer/gstlibcameraprovider.h new file mode 100644 index 0000000..6dd232d --- /dev/null +++ b/src/gstreamer/gstlibcameraprovider.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Collabora Ltd. + * Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> + * + * gstlibcameraprovider.h - GStreamer Device Provider + */ + +#include <gst/gst.h> + +#ifndef __GST_LIBCAMERA_PROVIDER_H__ +#define __GST_LIBCAMERA_PROVIDER_H__ + +G_BEGIN_DECLS + +#define GST_TYPE_LIBCAMERA_PROVIDER gst_libcamera_provider_get_type() +G_DECLARE_FINAL_TYPE(GstLibcameraProvider, gst_libcamera_provider, + GST_LIBCAMERA, PROVIDER, GstDeviceProvider) + +G_END_DECLS + +#endif /* __GST_LIBCAMERA_PROVIDER_H__ */ + diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build index 39a34e7..7769b78 100644 --- a/src/gstreamer/meson.build +++ b/src/gstreamer/meson.build @@ -2,6 +2,7 @@ libcamera_gst_sources = [ 'gstlibcamera.c', 'gstlibcamera-utils.cpp', 'gstlibcamerasrc.cpp', + 'gstlibcameraprovider.cpp', ] libcamera_gst_c_args = [