Patch Detail
Show a patch.
GET /api/1.1/patches/13495/?format=api
{ "id": 13495, "url": "https://patchwork.libcamera.org/api/1.1/patches/13495/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13495/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20210825211852.1207168-3-nicolas@ndufresne.ca>", "date": "2021-08-25T21:18:51", "name": "[libcamera-devel,v1,2/3] gstreamer: Fix concurrent access issues to CameraManager", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "7abbaa44145d5ad6bf63dac34e094c24f493b37d", "submitter": { "id": 30, "url": "https://patchwork.libcamera.org/api/1.1/people/30/?format=api", "name": "Nicolas Dufresne", "email": "nicolas@ndufresne.ca" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/13495/mbox/", "series": [ { "id": 2396, "url": "https://patchwork.libcamera.org/api/1.1/series/2396/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2396", "date": "2021-08-25T21:18:49", "name": "Fix Gnome Cheese and multiple camera", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2396/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13495/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13495/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 57469BD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 25 Aug 2021 21:19:16 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18BBC688C4;\n\tWed, 25 Aug 2021 23:19:16 +0200 (CEST)", "from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8105C60288\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 25 Aug 2021 23:19:15 +0200 (CEST)", "from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id C830D1F435D8" ], "From": "Nicolas Dufresne <nicolas@ndufresne.ca>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 25 Aug 2021 17:18:51 -0400", "Message-Id": "<20210825211852.1207168-3-nicolas@ndufresne.ca>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<20210825211852.1207168-1-nicolas@ndufresne.ca>", "References": "<20210825211852.1207168-1-nicolas@ndufresne.ca>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v1 2/3] gstreamer: Fix concurrent access\n\tissues to CameraManager", "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>", "Cc": "Nicolas Dufresne <nicolas.dufresne@collabora.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n\nIt's not allowed to have multiple instances of CameraManager. This\nrequirement is not easy for GStreamer were the device monitor and\nthe camerasrc, or two camerasrc instances don't usually have any\ninteraction between each other. This this by implementing a minimalist\nsingleton around CameraManager constructor and start()/stop()\noperations.\n\nSigned-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n---\n src/gstreamer/gstlibcamera-utils.cpp | 31 ++++++++++++++++++++++++++\n src/gstreamer/gstlibcamera-utils.h | 6 +++--\n src/gstreamer/gstlibcameraprovider.cpp | 22 ++----------------\n src/gstreamer/gstlibcamerasrc.cpp | 11 +++++----\n 4 files changed, 42 insertions(+), 28 deletions(-)", "diff": "diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\nindex 007d6a64..34b64c4a 100644\n--- a/src/gstreamer/gstlibcamera-utils.cpp\n+++ b/src/gstreamer/gstlibcamera-utils.cpp\n@@ -221,3 +221,34 @@ gst_libcamera_resume_task(GstTask *task)\n \t\tGST_TASK_SIGNAL(task);\n \t}\n }\n+\n+G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n+std::weak_ptr<CameraManager> cm_singleton_ptr;\n+\n+void _cm_deleter(CameraManager *cm)\n+{\n+\tg_print(\"Delete CameraManager\\n\");\n+\tcm->stop();\n+\tdelete cm;\n+}\n+\n+std::shared_ptr<CameraManager>\n+gst_libcamera_get_camera_mananger(int &ret)\n+{\n+\tstd::shared_ptr<CameraManager> cm;\n+\n+\tG_LOCK(cm_singleton_lock);\n+\n+\tcm = cm_singleton_ptr.lock();\n+\tif (cm_singleton_ptr.expired()) {\n+\t\tstd::shared_ptr<CameraManager> ptr(new CameraManager(), _cm_deleter);\n+\t\tcm_singleton_ptr = cm = ptr;\n+\t\tret = cm->start();\n+\t} else {\n+\t\tret = 0;\n+\t}\n+\n+\tG_UNLOCK(cm_singleton_lock);\n+\n+\treturn cm;\n+}\ndiff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\nindex 2b3f26b6..67a06db3 100644\n--- a/src/gstreamer/gstlibcamera-utils.h\n+++ b/src/gstreamer/gstlibcamera-utils.h\n@@ -9,16 +9,18 @@\n #ifndef __GST_LIBCAMERA_UTILS_H__\n #define __GST_LIBCAMERA_UTILS_H__\n \n+#include <libcamera/camera_manager.h>\n+#include <libcamera/stream.h>\n+\n #include <gst/gst.h>\n #include <gst/video/video.h>\n \n-#include <libcamera/stream.h>\n-\n GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &formats);\n GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n \t\t\t\t\t GstCaps *caps);\n void gst_libcamera_resume_task(GstTask *task);\n+std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_mananger(int &ret);\n \n /**\n * \\class GLibLocker\ndiff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\nindex 29da6c32..948ba0d1 100644\n--- a/src/gstreamer/gstlibcameraprovider.cpp\n+++ b/src/gstreamer/gstlibcameraprovider.cpp\n@@ -158,7 +158,6 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)\n \n struct _GstLibcameraProvider {\n \tGstDeviceProvider parent;\n-\tCameraManager *cm;\n };\n \n G_DEFINE_TYPE_WITH_CODE(GstLibcameraProvider, gst_libcamera_provider,\n@@ -170,7 +169,7 @@ static GList *\n gst_libcamera_provider_probe(GstDeviceProvider *provider)\n {\n \tGstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider);\n-\tCameraManager *cm = self->cm;\n+\tstd::shared_ptr<CameraManager> cm;\n \tGList *devices = nullptr;\n \tgint ret;\n \n@@ -181,7 +180,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\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+\tcm = gst_libcamera_get_camera_mananger(ret);\n \tif (ret) {\n \t\tGST_ERROR_OBJECT(self, \"Failed to retrieve device list: %s\",\n \t\t\t\t g_strerror(-ret));\n@@ -194,8 +193,6 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\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@@ -204,31 +201,16 @@ 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_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\",\ndiff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\nindex 4c7b36ae..bdd9f88c 100644\n--- a/src/gstreamer/gstlibcamerasrc.cpp\n+++ b/src/gstreamer/gstlibcamerasrc.cpp\n@@ -104,7 +104,7 @@ GstBuffer *RequestWrap::detachBuffer(Stream *stream)\n struct GstLibcameraSrcState {\n \tGstLibcameraSrc *src_;\n \n-\tstd::unique_ptr<CameraManager> cm_;\n+\tstd::shared_ptr<CameraManager> cm_;\n \tstd::shared_ptr<Camera> cam_;\n \tstd::unique_ptr<CameraConfiguration> config_;\n \tstd::vector<GstPad *> srcpads_;\n@@ -198,13 +198,13 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n static bool\n gst_libcamera_src_open(GstLibcameraSrc *self)\n {\n-\tstd::unique_ptr<CameraManager> cm = std::make_unique<CameraManager>();\n+\tstd::shared_ptr<CameraManager> cm;\n \tstd::shared_ptr<Camera> cam;\n-\tgint ret = 0;\n+\tgint ret;\n \n \tGST_DEBUG_OBJECT(self, \"Opening camera device ...\");\n \n-\tret = cm->start();\n+\tcm = gst_libcamera_get_camera_mananger(ret);\n \tif (ret) {\n \t\tGST_ELEMENT_ERROR(self, LIBRARY, INIT,\n \t\t\t\t (\"Failed listing cameras.\"),\n@@ -250,7 +250,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n \tcam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted);\n \n \t/* No need to lock here, we didn't start our threads yet. */\n-\tself->state->cm_ = std::move(cm);\n+\tself->state->cm_ = cm;\n \tself->state->cam_ = cam;\n \n \treturn true;\n@@ -517,7 +517,6 @@ gst_libcamera_src_close(GstLibcameraSrc *self)\n \t}\n \n \tstate->cam_.reset();\n-\tstate->cm_->stop();\n \tstate->cm_.reset();\n }\n \n", "prefixes": [ "libcamera-devel", "v1", "2/3" ] }