From 13357fec204fdb75eefbd936e4fb27fc97047738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sat, 3 Sep 2022 22:10:50 +0200 Subject: [PATCH] spa: libcamera: manage libcamera::CameraManager via a shared_ptr Using a shared_ptr removes the need for manually calling `libcamera_manager_release()` to drop the reference as it is done automatically whenever the shared_ptr is destroyed or reset. --- spa/plugins/libcamera/libcamera-device.cpp | 24 ++++----- spa/plugins/libcamera/libcamera-manager.cpp | 60 ++++++--------------- spa/plugins/libcamera/libcamera-manager.hpp | 9 ++-- spa/plugins/libcamera/libcamera-source.cpp | 23 ++++---- 4 files changed, 38 insertions(+), 78 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-device.cpp b/spa/plugins/libcamera/libcamera-device.cpp index c531254aa..0edd1a33c 100644 --- a/spa/plugins/libcamera/libcamera-device.cpp +++ b/spa/plugins/libcamera/libcamera-device.cpp @@ -66,11 +66,11 @@ struct impl { struct spa_hook_list hooks; - CameraManager *manager; + std::shared_ptr manager; std::shared_ptr camera; impl(spa_log *log, - CameraManager *manager, + std::shared_ptr manager, std::shared_ptr camera, std::string device_id); }; @@ -234,20 +234,18 @@ static int impl_get_interface(struct spa_handle *handle, const char *type, void static int impl_clear(struct spa_handle *handle) { - auto impl = reinterpret_cast(handle); - auto manager = impl->manager; - - std::destroy_at(impl); - libcamera_manager_release(manager); - + std::destroy_at(reinterpret_cast(handle)); return 0; } -impl::impl(spa_log *log, CameraManager *manager, std::shared_ptr camera, std::string device_id) +impl::impl(spa_log *log, + std::shared_ptr manager, + std::shared_ptr camera, + std::string device_id) : handle({ SPA_VERSION_HANDLE, impl_get_interface, impl_clear }), log(log), device_id(std::move(device_id)), - manager(manager), + manager(std::move(manager)), camera(std::move(camera)) { libcamera_log_topic_init(log); @@ -282,9 +280,8 @@ impl_init(const struct spa_handle_factory *factory, auto log = static_cast(spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log)); - auto manager = libcamera_manager_acquire(); + auto manager = libcamera_manager_acquire(res); if (!manager) { - res = -errno; spa_log_error(log, "can't start camera manager: %s", spa_strerror(res)); return res; } @@ -296,11 +293,10 @@ impl_init(const struct spa_handle_factory *factory, auto camera = manager->get(device_id); if (!camera) { spa_log_error(log, "unknown camera id %s", device_id.c_str()); - libcamera_manager_release(manager); return -ENOENT; } - new (handle) impl(log, manager, std::move(camera), std::move(device_id)); + new (handle) impl(log, std::move(manager), std::move(camera), std::move(device_id)); return 0; } diff --git a/spa/plugins/libcamera/libcamera-manager.cpp b/spa/plugins/libcamera/libcamera-manager.cpp index 0bdb7ffc6..3aaf65d99 100644 --- a/spa/plugins/libcamera/libcamera-manager.cpp +++ b/spa/plugins/libcamera/libcamera-manager.cpp @@ -56,13 +56,6 @@ using namespace libcamera; namespace { -struct global { - int ref; - CameraManager *manager; -}; - -static struct global global; - struct device { uint32_t id; std::shared_ptr camera; @@ -79,7 +72,7 @@ typedef struct impl { static constexpr uint64_t info_all = SPA_DEVICE_CHANGE_MASK_FLAGS | SPA_DEVICE_CHANGE_MASK_PROPS; struct spa_device_info info = SPA_DEVICE_INFO_INIT(); - CameraManager *manager = nullptr; + std::shared_ptr manager; void addCamera(std::shared_ptr camera); void removeCamera(std::shared_ptr camera); @@ -91,35 +84,20 @@ typedef struct impl { } -int libcamera_manager_release(CameraManager *manager) +static std::weak_ptr global_manager; + +std::shared_ptr libcamera_manager_acquire(int& res) { - if (global.manager != manager) - return -EINVAL; + if (auto manager = global_manager.lock()) + return manager; - if (--global.ref == 0) { - global.manager->stop(); - delete global.manager; - global.manager = NULL; - } - return 0; -} + auto manager = std::make_shared(); + if ((res = manager->start()) < 0) + return {}; -CameraManager *libcamera_manager_acquire(void) -{ - int res; + global_manager = manager; - if (global.ref++ == 0) { - global.manager = new CameraManager(); - if (global.manager == NULL) - return NULL; - - if ((res = global.manager->start()) < 0) { - libcamera_manager_release(global.manager); - errno = -res; - return NULL; - } - } - return global.manager; + return manager; } static struct device *add_device(struct impl *impl, std::shared_ptr camera) @@ -227,7 +205,7 @@ static int start_monitor(struct impl *impl) static int stop_monitor(struct impl *impl) { - if (impl->manager != NULL) { + if (impl->manager) { impl->manager->cameraAdded.disconnect(impl, &Impl::addCamera); impl->manager->cameraRemoved.disconnect(impl, &Impl::removeCamera); } @@ -269,9 +247,7 @@ static void impl_hook_removed(struct spa_hook *hook) struct impl *impl = (struct impl*)hook->priv; if (spa_hook_list_is_empty(&impl->hooks)) { stop_monitor(impl); - if (impl->manager) - libcamera_manager_release(impl->manager); - impl->manager = NULL; + impl->manager.reset(); } } @@ -286,9 +262,9 @@ impl_device_add_listener(void *object, struct spa_hook *listener, spa_return_val_if_fail(impl != NULL, -EINVAL); spa_return_val_if_fail(events != NULL, -EINVAL); - impl->manager = libcamera_manager_acquire(); - if (impl->manager == NULL) - return -errno; + impl->manager = libcamera_manager_acquire(res); + if (!impl->manager) + return res; spa_hook_list_isolate(&impl->hooks, &save, listener, events, data); @@ -333,14 +309,10 @@ static int impl_get_interface(struct spa_handle *handle, const char *type, void static int impl_clear(struct spa_handle *handle) { auto impl = reinterpret_cast(handle); - auto manager = impl->manager; stop_monitor(impl); std::destroy_at(impl); - if (manager) - libcamera_manager_release(manager); - return 0; } diff --git a/spa/plugins/libcamera/libcamera-manager.hpp b/spa/plugins/libcamera/libcamera-manager.hpp index 53e488bc5..4336a392e 100644 --- a/spa/plugins/libcamera/libcamera-manager.hpp +++ b/spa/plugins/libcamera/libcamera-manager.hpp @@ -22,11 +22,8 @@ * DEALINGS IN THE SOFTWARE. */ +#include + #include -#include - -using namespace libcamera; - -CameraManager *libcamera_manager_acquire(void); -int libcamera_manager_release(CameraManager *manager); +std::shared_ptr libcamera_manager_acquire(int& res); diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 2db7edbb1..d5cb8527e 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -60,6 +60,8 @@ #include "libcamera.h" #include "libcamera-manager.hpp" +using namespace libcamera; + namespace { #define MAX_BUFFERS 32 @@ -160,7 +162,7 @@ typedef struct impl { struct spa_io_position *position = nullptr; struct spa_io_clock *clock = nullptr; - CameraManager *manager; + std::shared_ptr manager; std::shared_ptr camera; FrameBufferAllocator *allocator = nullptr; @@ -178,7 +180,7 @@ typedef struct impl { bool acquired = false; impl(spa_log *log, spa_loop *data_loop, spa_system *system, - CameraManager *manager, std::shared_ptr camera, std::string device_id); + std::shared_ptr manager, std::shared_ptr camera, std::string device_id); } Impl; } @@ -915,24 +917,19 @@ static int impl_get_interface(struct spa_handle *handle, const char *type, void static int impl_clear(struct spa_handle *handle) { - auto impl = reinterpret_cast(handle); - auto manager = impl->manager; - - std::destroy_at(impl); - libcamera_manager_release(manager); - + std::destroy_at(reinterpret_cast(handle)); return 0; } impl::impl(spa_log *log, spa_loop *data_loop, spa_system *system, - CameraManager *manager, std::shared_ptr camera, std::string device_id) + std::shared_ptr manager, std::shared_ptr camera, std::string device_id) : handle({ SPA_VERSION_HANDLE, impl_get_interface, impl_clear }), log(log), data_loop(data_loop), system(system), device_id(std::move(device_id)), out_ports{{this}}, - manager(manager), + manager(std::move(manager)), camera(std::move(camera)) { libcamera_log_topic_init(log); @@ -987,9 +984,8 @@ impl_init(const struct spa_handle_factory *factory, return -EINVAL; } - auto manager = libcamera_manager_acquire(); + auto manager = libcamera_manager_acquire(res); if (!manager) { - res = -errno; spa_log_error(log, "can't start camera manager: %s", spa_strerror(res)); return res; } @@ -1001,12 +997,11 @@ impl_init(const struct spa_handle_factory *factory, auto camera = manager->get(device_id); if (!camera) { spa_log_error(log, "unknown camera id %s", device_id.c_str()); - libcamera_manager_release(manager); return -ENOENT; } new (handle) impl(log, data_loop, system, - manager, std::move(camera), std::move(device_id)); + std::move(manager), std::move(camera), std::move(device_id)); return 0; }