[{"id":3860,"web_url":"https://patchwork.libcamera.org/comment/3860/","msgid":"<20200229133046.GG18738@pendragon.ideasonboard.com>","date":"2020-02-29T13:30:46","subject":"Re: [libcamera-devel] [PATCH v2 03/27] gst: Add initial device\n\tprovider","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the patch.\n\nOn Thu, Feb 27, 2020 at 03:03:43PM -0500, Nicolas Dufresne wrote:\n> This feature is used with GstDeviceMonitor in order to enumerate\n> and monitor devices to be used with the source element. The resulting\n> GstDevice implementation is also used by application to abstract the\n> configuration of the source element.\n> \n> Implementation notes:\n>   - libcamera does not support polling yet\n>   - The device ID isn't unique in libcamera yet\n>   - The \"name\" property does not yet exist in libcamerasrc yet\n> \n> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> ---\n>  src/gstreamer/gstlibcamera.c           |  10 +-\n>  src/gstreamer/gstlibcameraprovider.cpp | 235 +++++++++++++++++++++++++\n>  src/gstreamer/gstlibcameraprovider.h   |  23 +++\n>  src/gstreamer/meson.build              |   1 +\n>  4 files changed, 267 insertions(+), 2 deletions(-)\n>  create mode 100644 src/gstreamer/gstlibcameraprovider.cpp\n>  create mode 100644 src/gstreamer/gstlibcameraprovider.h\n> \n> diff --git a/src/gstreamer/gstlibcamera.c b/src/gstreamer/gstlibcamera.c\n> index 7dd94ca..81c7bb1 100644\n> --- a/src/gstreamer/gstlibcamera.c\n> +++ b/src/gstreamer/gstlibcamera.c\n> @@ -6,13 +6,19 @@\n>   * gstlibcamera.c - GStreamer plugin\n>   */\n>  \n> +#include \"gstlibcameraprovider.h\"\n>  #include \"gstlibcamerasrc.h\"\n>  \n>  static gboolean\n>  plugin_init(GstPlugin *plugin)\n>  {\n> -\treturn gst_element_register(plugin, \"libcamerasrc\", GST_RANK_PRIMARY,\n> -\t\t\t\t    GST_TYPE_LIBCAMERA_SRC);\n> +\tif (!gst_element_register(plugin, \"libcamerasrc\", GST_RANK_PRIMARY,\n> +\t\t\t\t  GST_TYPE_LIBCAMERA_SRC) ||\n> +\t    !gst_device_provider_register(plugin, \"libcameraprovider\",\n> +\t\t\t\t\t  GST_RANK_PRIMARY,\n> +\t\t\t\t\t  GST_TYPE_LIBCAMERA_PROVIDER))\n> +\t\treturn FALSE;\n> +\n>  \treturn TRUE;\n>  }\n>  \n> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> new file mode 100644\n> index 0000000..3dcac59\n> --- /dev/null\n> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> @@ -0,0 +1,235 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Collabora Ltd.\n> + *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> + *\n> + * gstlibcameraprovider.c - GStreamer Device Provider\n> + */\n> +\n> +#include \"gstlibcamera-utils.h\"\n> +#include \"gstlibcameraprovider.h\"\n> +#include \"gstlibcamerasrc.h\"\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/camera_manager.h>\n> +\n> +using namespace libcamera;\n> +\n> +GST_DEBUG_CATEGORY_STATIC(provider_debug);\n> +#define GST_CAT_DEFAULT provider_debug\n> +\n> +/**\n> + * \\struct _GstLibcameraDevice\n> + * \\brief libcamera GstDevice implementation\n> + *\n> + * This object will be used by GstLibcameraProder to abstract a libcamera\n\ns/GstLibcameraProder/GstLibcameraProvider/\n\nI would already write \"is used\" instead of \"will be used\", otherwise\nyou'll have to change that later.\n\n> + * device. It also provides helpers to create and configure the\n> + * libcamerasrc GstElement to be used with this device. The implementation is\n> + * private to the plugin.\n> + */\n\nThanks for providing documentation for the structures, it's very useful.\n\n> +\n> +enum {\n> +\tPROP_DEVICE_NAME = 1,\n> +};\n> +\n> +#define GST_TYPE_LIBCAMERA_DEVICE gst_libcamera_device_get_type()\n> +G_DECLARE_FINAL_TYPE(GstLibcameraDevice, gst_libcamera_device,\n> +\t\t     GST_LIBCAMERA, DEVICE, GstDevice);\n> +\n> +struct _GstLibcameraDevice {\n> +\tGstDevice parent;\n> +\tgchar *name;\n> +};\n> +\n> +G_DEFINE_TYPE(GstLibcameraDevice, gst_libcamera_device, GST_TYPE_DEVICE);\n> +\n> +static GstElement *\n> +gst_libcamera_device_create_element(GstDevice *device, const gchar *name)\n> +{\n> +\tGstElement *source = gst_element_factory_make(\"libcamerasrc\", name);\n> +\n> +\t/* Provider and source lives in the same plugin, so making the source\n> +\t * should never fail. */\n\nThe comment style should be\n\n\t/*\n\t * Provider and source lives in the same plugin, so making the source\n\t * should never fail.\n\t */\n\nI'll try to nerd-snipe Kieran again to add this to checkstyle.py :-)\n\n> +\tg_assert(source);\n> +\n> +\tg_object_set(source, \"camera-name\", GST_LIBCAMERA_DEVICE(device)->name, nullptr);\n> +\n> +\treturn source;\n> +}\n> +\n> +static gboolean\n> +gst_libcamera_device_reconfigure_element(GstDevice *device,\n> +\t\t\t\t\t GstElement *element)\n> +{\n> +\tif (!GST_LIBCAMERA_IS_SRC(element))\n> +\t\treturn FALSE;\n> +\n> +\tg_object_set(element, \"camera-name\", GST_LIBCAMERA_DEVICE(device)->name, nullptr);\n> +\n> +\treturn TRUE;\n> +}\n> +\n> +static void\n> +gst_libcamera_device_set_property(GObject *object, guint prop_id,\n> +\t\t\t\t  const GValue *value, GParamSpec *pspec)\n> +{\n> +\tGstLibcameraDevice *device = GST_LIBCAMERA_DEVICE(object);\n> +\n> +\tswitch (prop_id) {\n> +\tcase PROP_DEVICE_NAME:\n> +\t\tdevice->name = g_value_dup_string(value);\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> +static void\n> +gst_libcamera_device_init(GstLibcameraDevice *self)\n> +{\n> +}\n> +\n> +static void\n> +gst_libcamera_device_finalize(GObject *object)\n> +{\n> +\tGstLibcameraDevice *self = GST_LIBCAMERA_DEVICE(object);\n> +\tgpointer klass = gst_libcamera_device_parent_class;\n> +\n> +\tg_free(self->name);\n> +\n> +\tG_OBJECT_GET_CLASS(klass)->finalize(object);\n> +}\n> +\n> +static void\n> +gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass)\n> +{\n> +\tGstDeviceClass *device_class = GST_DEVICE_CLASS(klass);\n> +\tGObjectClass *object_class = G_OBJECT_CLASS(klass);\n> +\n> +\tdevice_class->create_element = gst_libcamera_device_create_element;\n> +\tdevice_class->reconfigure_element = gst_libcamera_device_reconfigure_element;\n> +\n> +\tobject_class->set_property = gst_libcamera_device_set_property;\n> +\tobject_class->finalize = gst_libcamera_device_finalize;\n> +\n> +\tGParamSpec *pspec = g_param_spec_string(\"name\", \"Name\",\n> +\t\t\t\t\t\t\"The name of the camera device\", \"\",\n> +\t\t\t\t\t\t(GParamFlags)(G_PARAM_STATIC_STRINGS | G_PARAM_WRITABLE |\n> +\t\t\t\t\t\t\t      G_PARAM_CONSTRUCT_ONLY));\n> +\tg_object_class_install_property(object_class, PROP_DEVICE_NAME, pspec);\n> +}\n> +\n> +static GstDevice *\n> +gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n> +{\n> +\tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n> +\tconst gchar *name = camera->name().c_str();\n> +\tStreamRoles roles;\n> +\n> +\troles.push_back(StreamRole::VideoRecording);\n> +\tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n> +\n> +\tfor (const StreamConfiguration &stream_cfg : *config) {\n> +\t\tGstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> +\t\tif (sub_caps)\n> +\t\t\tgst_caps_append(caps, sub_caps);\n> +\t}\n> +\n> +\treturn GST_DEVICE(g_object_new(GST_TYPE_LIBCAMERA_DEVICE,\n> +\t\t\t\t       /* \\todo Use a unique identifier instead of camera name. */\n> +\t\t\t\t       \"name\", name,\n> +\t\t\t\t       \"display-name\", name,\n> +\t\t\t\t       \"caps\", caps,\n> +\t\t\t\t       \"device-class\", \"Source/Video\",\n> +\t\t\t\t       nullptr));\n> +}\n> +\n> +/**\n> + * \\struct _GstLibcameraProvider\n> + * \\brief libcamera GstDeviceProvider implementation\n> + *\n> + * This GstFeature will be used by GstDeviceMonitor to probe the available\n\ns/will be used/is used/\n\n> + * libcamera devices.  The implementation is private to the plugin.\n\ns/  / /\n\n> + */\n> +\n> +struct _GstLibcameraProvider {\n> +\tGstDeviceProvider parent;\n> +\tCameraManager *cm;\n> +};\n> +\n> +G_DEFINE_TYPE_WITH_CODE(GstLibcameraProvider, gst_libcamera_provider,\n> +\t\t\tGST_TYPE_DEVICE_PROVIDER,\n> +\t\t\tGST_DEBUG_CATEGORY_INIT(provider_debug, \"libcamera-provider\", 0,\n> +\t\t\t\t\t\t\"libcamera Device Provider\"));\n> +\n> +static GList *\n> +gst_libcamera_provider_probe(GstDeviceProvider *provider)\n> +{\n> +\tGstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider);\n> +\tCameraManager *cm = self->cm;\n> +\tGList *devices = nullptr;\n> +\tgint ret;\n> +\n> +\tGST_INFO_OBJECT(self, \"Probing cameras using libcamera\");\n> +\n> +\t/* \\todo Move the CameraMananger start()/stop() calls into\n> +\t * GstDeviceProfiler start()/stop() virtual function when CameraMananger\n\ns/GstDeviceProfiler/GstDeviceProvider/\n\nWith those small issues fixed,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t * gains monitoring support. Meanwhile we need to cycle start()/stop()\n> +\t * to ensure every probe() calls return the latest list.\n> +\t */\n> +\tret = cm->start();\n> +\tif (ret) {\n> +\t\tGST_ERROR_OBJECT(self, \"Failed to retrieve device list: %s\",\n> +\t\t\t\t g_strerror(-ret));\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tfor (const std::shared_ptr<Camera> &camera : cm->cameras()) {\n> +\t\tGST_INFO_OBJECT(self, \"Found camera '%s'\", camera->name().c_str());\n> +\t\tdevices = g_list_append(devices,\n> +\t\t\t\t\tg_object_ref_sink(gst_libcamera_device_new(camera)));\n> +\t}\n> +\n> +\tcm->stop();\n> +\n> +\treturn devices;\n> +}\n> +\n> +static void\n> +gst_libcamera_provider_init(GstLibcameraProvider *self)\n> +{\n> +\tGstDeviceProvider *provider = GST_DEVICE_PROVIDER(self);\n> +\n> +\tself->cm = new CameraManager();\n> +\n> +\t/* Avoid devices being duplicated. */\n> +\tgst_device_provider_hide_provider(provider, \"v4l2deviceprovider\");\n> +}\n> +\n> +static void\n> +gst_libcamera_provider_finalize(GObject *object)\n> +{\n> +\tGstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(object);\n> +\tgpointer klass = gst_libcamera_provider_parent_class;\n> +\n> +\tdelete self->cm;\n> +\n> +\treturn G_OBJECT_GET_CLASS(klass)->finalize(object);\n> +}\n> +\n> +static void\n> +gst_libcamera_provider_class_init(GstLibcameraProviderClass *klass)\n> +{\n> +\tGstDeviceProviderClass *provider_class = GST_DEVICE_PROVIDER_CLASS(klass);\n> +\tGObjectClass *object_class = G_OBJECT_CLASS(klass);\n> +\n> +\tprovider_class->probe = gst_libcamera_provider_probe;\n> +\tobject_class->finalize = gst_libcamera_provider_finalize;\n> +\n> +\tgst_device_provider_class_set_metadata(provider_class,\n> +\t\t\t\t\t       \"libcamera Device Provider\",\n> +\t\t\t\t\t       \"Source/Video\",\n> +\t\t\t\t\t       \"List camera device using libcamera\",\n> +\t\t\t\t\t       \"Nicolas Dufresne <nicolas.dufresne@collabora.com>\");\n> +}\n> diff --git a/src/gstreamer/gstlibcameraprovider.h b/src/gstreamer/gstlibcameraprovider.h\n> new file mode 100644\n> index 0000000..bdd19db\n> --- /dev/null\n> +++ b/src/gstreamer/gstlibcameraprovider.h\n> @@ -0,0 +1,23 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Collabora Ltd.\n> + *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> + *\n> + * gstlibcameraprovider.h - GStreamer Device Provider\n> + */\n> +\n> +#ifndef __GST_LIBCAMERA_PROVIDER_H__\n> +#define __GST_LIBCAMERA_PROVIDER_H__\n> +\n> +#include <gst/gst.h>\n> +\n> +G_BEGIN_DECLS\n> +\n> +#define GST_TYPE_LIBCAMERA_PROVIDER gst_libcamera_provider_get_type()\n> +G_DECLARE_FINAL_TYPE(GstLibcameraProvider, gst_libcamera_provider,\n> +\t\t     GST_LIBCAMERA, PROVIDER, GstDeviceProvider)\n> +\n> +G_END_DECLS\n> +\n> +#endif /* __GST_LIBCAMERA_PROVIDER_H__ */\n> +\n> diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build\n> index f409107..a57dd85 100644\n> --- a/src/gstreamer/meson.build\n> +++ b/src/gstreamer/meson.build\n> @@ -1,6 +1,7 @@\n>  libcamera_gst_sources = [\n>      'gstlibcamera-utils.cpp',\n>      'gstlibcamera.c',\n> +    'gstlibcameraprovider.cpp',\n>      'gstlibcamerasrc.cpp',\n>  ]\n>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2B6F862689\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 29 Feb 2020 14:31:10 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D5992AF;\n\tSat, 29 Feb 2020 14:31:09 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582983069;\n\tbh=51d6bqC3Xb5bU12PT1bUkms7fsaV5yYn/2ld8otHM+c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=g8z7m0weTtAogvZnogxCQhUr+l+v5PCHaIwmaWmyozyRrTNXAobWvMVOFb4w/ayhw\n\t35v+5kB6njDsnBG1b/nDQ+hVoCWH8EZoGlajINp2rsyAZp1UkFevzIRxI6FdDuibMw\n\toIfi+JTaI6exAravT1bfEMuqkbkH64zAHyPfXM0Y=","Date":"Sat, 29 Feb 2020 15:30:46 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200229133046.GG18738@pendragon.ideasonboard.com>","References":"<20200227200407.490616-1-nicolas.dufresne@collabora.com>\n\t<20200227200407.490616-4-nicolas.dufresne@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200227200407.490616-4-nicolas.dufresne@collabora.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 03/27] gst: Add initial device\n\tprovider","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sat, 29 Feb 2020 13:31:10 -0000"}}]