From patchwork Thu Aug 26 13:23:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Dufresne X-Patchwork-Id: 13512 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id ACB38C3241 for ; Thu, 26 Aug 2021 13:24:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 28FAD6892C; Thu, 26 Aug 2021 15:24:02 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 45E9F6890C for ; Thu, 26 Aug 2021 15:24:01 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nicolas) with ESMTPSA id 90A801F44333 From: Nicolas Dufresne To: libcamera-devel@lists.libcamera.org Date: Thu, 26 Aug 2021 09:23:45 -0400 Message-Id: <20210826132346.1238420-3-nicolas@ndufresne.ca> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210826132346.1238420-1-nicolas@ndufresne.ca> References: <20210826132346.1238420-1-nicolas@ndufresne.ca> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/3] gstreamer: Fix concurrent access issues to CameraManager X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Nicolas Dufresne Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nicolas Dufresne It's not allowed to have multiple instances of CameraManager. This requirement is not easy for GStreamer were the device monitor and the camerasrc, or two camerasrc instances don't usually have any interaction between each other. Fix this by implementing a minimalist singleton around CameraManager constructor and start()/stop() operations. Signed-off-by: Nicolas Dufresne Reviewed-by: Laurent Pinchart --- src/gstreamer/gstlibcamera-utils.cpp | 24 ++++++++++++++++++++++++ src/gstreamer/gstlibcamera-utils.h | 6 ++++-- src/gstreamer/gstlibcameraprovider.cpp | 22 ++-------------------- src/gstreamer/gstlibcamerasrc.cpp | 11 +++++------ 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp index 007d6a64..029f031f 100644 --- a/src/gstreamer/gstlibcamera-utils.cpp +++ b/src/gstreamer/gstlibcamera-utils.cpp @@ -221,3 +221,27 @@ gst_libcamera_resume_task(GstTask *task) GST_TASK_SIGNAL(task); } } + +G_LOCK_DEFINE_STATIC(cm_singleton_lock); +std::weak_ptr cm_singleton_ptr; + +std::shared_ptr +gst_libcamera_get_camera_mananger(int &ret) +{ + std::shared_ptr cm; + + G_LOCK(cm_singleton_lock); + + cm = cm_singleton_ptr.lock(); + if (!cm) { + cm = std::make_shared(); + cm_singleton_ptr = cm; + ret = cm->start(); + } else { + ret = 0; + } + + G_UNLOCK(cm_singleton_lock); + + return cm; +} diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h index 2b3f26b6..67a06db3 100644 --- a/src/gstreamer/gstlibcamera-utils.h +++ b/src/gstreamer/gstlibcamera-utils.h @@ -9,16 +9,18 @@ #ifndef __GST_LIBCAMERA_UTILS_H__ #define __GST_LIBCAMERA_UTILS_H__ +#include +#include + #include #include -#include - GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &formats); GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg); void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg, GstCaps *caps); void gst_libcamera_resume_task(GstTask *task); +std::shared_ptr gst_libcamera_get_camera_mananger(int &ret); /** * \class GLibLocker diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp index 29da6c32..948ba0d1 100644 --- a/src/gstreamer/gstlibcameraprovider.cpp +++ b/src/gstreamer/gstlibcameraprovider.cpp @@ -158,7 +158,6 @@ gst_libcamera_device_new(const std::shared_ptr &camera) struct _GstLibcameraProvider { GstDeviceProvider parent; - CameraManager *cm; }; G_DEFINE_TYPE_WITH_CODE(GstLibcameraProvider, gst_libcamera_provider, @@ -170,7 +169,7 @@ static GList * gst_libcamera_provider_probe(GstDeviceProvider *provider) { GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider); - CameraManager *cm = self->cm; + std::shared_ptr cm; GList *devices = nullptr; gint ret; @@ -181,7 +180,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider) * gains monitoring support. Meanwhile we need to cycle start()/stop() * to ensure every probe() calls return the latest list. */ - ret = cm->start(); + cm = gst_libcamera_get_camera_mananger(ret); if (ret) { GST_ERROR_OBJECT(self, "Failed to retrieve device list: %s", g_strerror(-ret)); @@ -194,8 +193,6 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider) g_object_ref_sink(gst_libcamera_device_new(camera))); } - cm->stop(); - return devices; } @@ -204,31 +201,16 @@ 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_CLASS(klass)->finalize(object); -} - static void gst_libcamera_provider_class_init(GstLibcameraProviderClass *klass) { GstDeviceProviderClass *provider_class = GST_DEVICE_PROVIDER_CLASS(klass); - GObjectClass *object_class = G_OBJECT_CLASS(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", diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 4c7b36ae..bdd9f88c 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -104,7 +104,7 @@ GstBuffer *RequestWrap::detachBuffer(Stream *stream) struct GstLibcameraSrcState { GstLibcameraSrc *src_; - std::unique_ptr cm_; + std::shared_ptr cm_; std::shared_ptr cam_; std::unique_ptr config_; std::vector srcpads_; @@ -198,13 +198,13 @@ GstLibcameraSrcState::requestCompleted(Request *request) static bool gst_libcamera_src_open(GstLibcameraSrc *self) { - std::unique_ptr cm = std::make_unique(); + std::shared_ptr cm; std::shared_ptr cam; - gint ret = 0; + gint ret; GST_DEBUG_OBJECT(self, "Opening camera device ..."); - ret = cm->start(); + cm = gst_libcamera_get_camera_mananger(ret); if (ret) { GST_ELEMENT_ERROR(self, LIBRARY, INIT, ("Failed listing cameras."), @@ -250,7 +250,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self) cam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted); /* No need to lock here, we didn't start our threads yet. */ - self->state->cm_ = std::move(cm); + self->state->cm_ = cm; self->state->cam_ = cam; return true; @@ -517,7 +517,6 @@ gst_libcamera_src_close(GstLibcameraSrc *self) } state->cam_.reset(); - state->cm_->stop(); state->cm_.reset(); }