From patchwork Thu Jan 17 23:59:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 267 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E04260C98 for ; Fri, 18 Jan 2019 00:59:20 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 213CAD4B for ; Fri, 18 Jan 2019 00:59:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547769560; bh=do84Lty8iKl/q6ouiHDarTfhWNEcG7ZNcrkdQ8kkUj8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Ka7AAMjdVc14NeJvHGGuDkaj8nni/X7P1YbcvRBLwsIxvstAR9jHvy5QQ1bAmlQGz ntvh9ljR+5egxAf7qlWJzSi8JqgxsRrK1rIHgTlW3Grmcy+55YuD6yeqwF5TJEYPT5 xujcKYDekGDYPHcX9ykg5qeFfa4SQwCwH9nm9Yrc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 18 Jan 2019 01:59:13 +0200 Message-Id: <20190117235916.1906-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> References: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/4] Documentation: coding_style: Add object ownership rules X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jan 2019 23:59:20 -0000 Object ownership is a complex topic that can lead to many issues, from memory leak to crashes. Document the rules that libcamera enforces to make object ownership tracking explicit. This is a first version of the rules and is expected to be expanded as the library is developed. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Documentation/coding-style.rst | 62 ++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst index f8d2fdfeda8e..f77325239bfa 100644 --- a/Documentation/coding-style.rst +++ b/Documentation/coding-style.rst @@ -81,6 +81,68 @@ C++-11-specific features: overused. * Variadic class and function templates +Object Ownership +~~~~~~~~~~~~~~~~ + +libcamera creates and destroys many objects at runtime, for both objects +internal to the library and objects exposed to the user. To guarantee proper +operation without use after free, double free or memory leaks, knowing who owns +each object at any time is crucial. The project has enacted a set of rules to +make object ownership tracking as explicit and fool-proof as possible. + +In the context of this section, the terms object and instance are used +interchangeably and both refer to an instance of a class. The term reference +refers to both C++ references and C++ pointers in their capacity to refer to an +object. Passing a reference means offering a way to a callee to obtain a +reference to an object that the caller has a valid reference to. Borrowing a +reference means using a reference passed by a caller without ownership transfer +based on the assumption that the caller guarantees the validate of the +reference for the duration of the operation that borrows the reference. + +#. Single Owner Objects + + * By default an object has a single owner at any time. + * References to a single owner object can be borrowed by passing them from + the owner to the borrower, providing that + + * the owner guarantees the validity of the reference for the whole duration + of borrowing, and + * the borrower doesn't access the reference after the end of the borrowing. + + When borrowing from caller to callee for the duration of a function call, + this implies that the callee shall not keep any stored reference after it + returns. These rules applies to the callee and all the functions it calls, + directly or indirectly. + * When the object doesn't need to be modified and may not be null, borrowed + references are passed as 'const &'. + * When the object may be modified or can be null, borrowed references are + passed as pointers. Unless otherwise specified, pointers passed to + functions are considered as borrowed references valid for the duration of + the function only. + * Single ownership is emphasized as much as possible by storing the unique + reference as a std::unique_ptr<>. + * Ownership is transfered by passing the reference as a std::unique_ptr<>. + +#. Shared Objects + + * Objects that may have multiple owners at a given time are called shared + objects. They are reference-counted and live as long as any references to + the object exist. + * Shared objects are created with std::make_shared<> or + std::allocate_shared<> an stored in an std::shared_ptr<>. + * Borrowed references to shared objects are passed with the same rules as for + single owner objects. + * Ownership is shared by creating and passing copies of any valid + std::shared_ptr<> reference. Ownership is released by destroying the + corresponding std::shared_ptr<>. + +.. attention:: Long term borrowing of single owner objects is allowed. Example + use cases are implementation of the singleton pattern (where the singleton + guarantees the validity of the reference forever), or returning references + to global objects whose lifetime matches the lifetime of the application. As + long term borrowing isn't marked through language constructs, it shall be + documented explicitly in details in the API. + Tools ----- From patchwork Thu Jan 17 23:59:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 268 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C533860C98 for ; Fri, 18 Jan 2019 00:59:20 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 64211D50 for ; Fri, 18 Jan 2019 00:59:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547769560; bh=Yje3AtUt21PQk4uW1VFQ9J+pN5npOrqgOgINPDRdSro=; h=From:To:Subject:Date:In-Reply-To:References:From; b=G2jyw+FGOZ5+hNBIdlSa97m+1Z9PVqfQ+EzjHup+w0Xt2WXI09n/3J7wGdH+z/cNd 0y2Fa7UP1t9VLdLc1vk1NYBYdh7oGMLqYOiJgUpIGyTqtrvhzEqdhYzVHf9FXrQ5Vy GlWZYU32blKncBL9QzSlF+yJwSfhBp7onW9hDTgU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 18 Jan 2019 01:59:14 +0200 Message-Id: <20190117235916.1906-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> References: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/4] libcamera: camera_manager: Use std::unique_ptr to store event dispatcher X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jan 2019 23:59:20 -0000 The CameraManager takes ownership of the dispatcher passed to the setEventDispatcher() function. Enforces this by using std::unique_ptr<>. Signed-off-by: Laurent Pinchart --- include/libcamera/camera_manager.h | 4 ++-- src/libcamera/camera_manager.cpp | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 6cfcba3c3933..b82a8ce95b9f 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -29,7 +29,7 @@ public: static CameraManager *instance(); - void setEventDispatcher(EventDispatcher *dispatcher); + void setEventDispatcher(std::unique_ptr dispatcher); EventDispatcher *eventDispatcher(); private: @@ -41,7 +41,7 @@ private: std::unique_ptr enumerator_; std::vector pipes_; - EventDispatcher *dispatcher_; + std::unique_ptr dispatcher_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 9f554be57e33..9806e399f92b 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -58,7 +58,6 @@ CameraManager::CameraManager() CameraManager::~CameraManager() { - delete dispatcher_; } /** @@ -209,14 +208,14 @@ CameraManager *CameraManager::instance() * The CameraManager takes ownership of the event dispatcher and will delete it * when the application terminates. */ -void CameraManager::setEventDispatcher(EventDispatcher *dispatcher) +void CameraManager::setEventDispatcher(std::unique_ptr dispatcher) { if (dispatcher_) { LOG(Warning) << "Event dispatcher is already set"; return; } - dispatcher_ = dispatcher; + dispatcher_ = std::move(dispatcher); } /** @@ -226,14 +225,16 @@ void CameraManager::setEventDispatcher(EventDispatcher *dispatcher) * If no dispatcher has been set, a default poll-based implementation is created * and returned, and no custom event dispatcher may be installed anymore. * + * The returned event dispatcher is valid until the camera manager is destroyed. + * * \return Pointer to the event dispatcher */ EventDispatcher *CameraManager::eventDispatcher() { if (!dispatcher_) - dispatcher_ = new EventDispatcherPoll(); + dispatcher_.reset(new EventDispatcherPoll()); - return dispatcher_; + return dispatcher_.get(); } } /* namespace libcamera */ From patchwork Thu Jan 17 23:59:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 269 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1475360C98 for ; Fri, 18 Jan 2019 00:59:21 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AA5F8558 for ; Fri, 18 Jan 2019 00:59:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547769560; bh=BCmQvI98I0BXOsQ8vk/ummh4rX9zomnw65nmhCAHLOs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=OOVxhQoiPWGnx6+y5kgs/g9kyPP6VBZYM2MNwo4zSusTTaoqF0FxNghohXclUTfeI vE/fWGxSOso7uIpw2cwSgEj+sq7zI9je163bNrikKlc4/smdjSg/4ueIxgf4x6x0x7 Qeh33hp81l5DqyU0eZk/F86NPgW6KxdqT8x4IEFU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 18 Jan 2019 01:59:15 +0200 Message-Id: <20190117235916.1906-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> References: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/4] libcamera: camera_manager: Register cameras with the camera manager X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jan 2019 23:59:21 -0000 Cameras are listed through a double indirection, first iterating over all available pipeline handlers, and then listing the cameras they each support. To simplify the API make the pipeline handlers register the cameras with the camera manager directly, which lets the camera manager easily expose the list of all available cameras. The PipelineHandler API gets simplified as the handlers don't need to expose the list of cameras they have created. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera_manager.h | 5 ++- src/libcamera/camera_manager.cpp | 44 +++++++++++------------- src/libcamera/include/pipeline_handler.h | 6 ++-- src/libcamera/pipeline/vimc.cpp | 30 ++++------------ src/libcamera/pipeline_handler.cpp | 21 +++-------- test/list-cameras.cpp | 5 +-- 6 files changed, 40 insertions(+), 71 deletions(-) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index b82a8ce95b9f..4b941fd9183b 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -24,9 +24,11 @@ public: int start(); void stop(); - std::vector list() const; + const std::vector &cameras() const { return cameras_; } Camera *get(const std::string &name); + void addCamera(Camera *camera); + static CameraManager *instance(); void setEventDispatcher(std::unique_ptr dispatcher); @@ -40,6 +42,7 @@ private: std::unique_ptr enumerator_; std::vector pipes_; + std::vector cameras_; std::unique_ptr dispatcher_; }; diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 9806e399f92b..852e5ed70fe3 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -93,7 +93,7 @@ int CameraManager::start() */ while (1) { PipelineHandler *pipe = factory->create(); - if (!pipe->match(enumerator_.get())) { + if (!pipe->match(this, enumerator_.get())) { delete pipe; break; } @@ -132,26 +132,14 @@ void CameraManager::stop() } /** - * \brief List all detected cameras + * \fn CameraManager::cameras() + * \brief Retrieve all available cameras * * Before calling this function the caller is responsible for ensuring that * the camera manger is running. * - * \return List of names for all detected cameras + * \return List of all available cameras */ -std::vector CameraManager::list() const -{ - std::vector list; - - for (PipelineHandler *pipe : pipes_) { - for (unsigned int i = 0; i < pipe->count(); i++) { - Camera *cam = pipe->camera(i); - list.push_back(cam->name()); - } - } - - return list; -} /** * \brief Get a camera based on name @@ -167,19 +155,27 @@ std::vector CameraManager::list() const */ Camera *CameraManager::get(const std::string &name) { - for (PipelineHandler *pipe : pipes_) { - for (unsigned int i = 0; i < pipe->count(); i++) { - Camera *cam = pipe->camera(i); - if (cam->name() == name) { - cam->get(); - return cam; - } - } + for (Camera *camera : cameras_) { + if (camera->name() == name) + return camera; } return nullptr; } +/** + * \brief Add a camera to the camera manager + * \param[in] camera The camera to be added + * + * This function is called by pipeline handlers to register the cameras they + * handle with the camera manager. Registered cameras are immediately made + * available to the system. + */ +void CameraManager::addCamera(Camera *camera) +{ + cameras_.push_back(camera); +} + /** * \brief Retrieve the camera manager instance * diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index e976aaa13546..f05f201f7ca8 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -15,6 +15,7 @@ namespace libcamera { +class CameraManager; class DeviceEnumerator; class PipelineHandler @@ -22,10 +23,7 @@ class PipelineHandler public: virtual ~PipelineHandler() { }; - virtual bool match(DeviceEnumerator *enumerator) = 0; - - virtual unsigned int count() = 0; - virtual Camera *camera(unsigned int id) = 0; + virtual bool match(CameraManager *manager, DeviceEnumerator *enumerator) = 0; }; class PipelineHandlerFactory diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 720d9c2031c9..8742e0bae9a8 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -6,6 +6,7 @@ */ #include +#include #include "device_enumerator.h" #include "media_device.h" @@ -19,44 +20,24 @@ public: PipeHandlerVimc(); ~PipeHandlerVimc(); - bool match(DeviceEnumerator *enumerator); - - unsigned int count(); - Camera *camera(unsigned int id) final; + bool match(CameraManager *manager, DeviceEnumerator *enumerator); private: MediaDevice *dev_; - Camera *camera_; }; PipeHandlerVimc::PipeHandlerVimc() - : dev_(nullptr), camera_(nullptr) + : dev_(nullptr) { } PipeHandlerVimc::~PipeHandlerVimc() { - if (camera_) - camera_->put(); - if (dev_) dev_->release(); } -unsigned int PipeHandlerVimc::count() -{ - return 1; -} - -Camera *PipeHandlerVimc::camera(unsigned int id) -{ - if (id != 0) - return nullptr; - - return camera_; -} - -bool PipeHandlerVimc::match(DeviceEnumerator *enumerator) +bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator) { DeviceMatch dm("vimc"); @@ -83,7 +64,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator) * will be chosen depends on how the Camera * object is modeled. */ - camera_ = new Camera("Dummy VIMC Camera"); + Camera *camera = new Camera("Dummy VIMC Camera"); + manager->addCamera(camera); return true; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index c19ab94f1774..f2e08a6a7315 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -35,13 +35,15 @@ namespace libcamera { /** * \fn PipelineHandler::match(DeviceEnumerator *enumerator) * \brief Match media devices and create camera instances + * \param manager The camera manager * \param enumerator The enumerator providing all media devices found in the * system * * This function is the main entry point of the pipeline handler. It is called - * by the camera manager with the \a enumerator passed as an argument. It - * shall acquire from the \a enumerator all the media devices it needs for a - * single pipeline and create one or multiple Camera instances. + * by the camera manager with the \a manager and \a enumerator passed as + * arguments. It shall acquire from the \a enumerator all the media devices it + * needs for a single pipeline, create one or multiple Camera instances and + * register them with the \a manager. * * If all media devices needed by the pipeline handler are found, they must all * be acquired by a call to MediaDevice::acquire(). This function shall then @@ -62,19 +64,6 @@ namespace libcamera { * created, or false otherwise */ -/** - * \fn PipelineHandler::count() - * \brief Retrieve the number of cameras handled by this pipeline handler - * \return the number of cameras that were created by the match() function - */ - -/** - * \fn PipelineHandler::camera(unsigned int id) - * \brief Retrieve one of the cameras handled by this pipeline handler - * \param[in] id the camera index - * \return a pointer to the Camera identified by \a id - */ - /** * \class PipelineHandlerFactory * \brief Registration of PipelineHandler classes and creation of instances diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp index e2026c99c5b8..fdbbda0957b2 100644 --- a/test/list-cameras.cpp +++ b/test/list-cameras.cpp @@ -7,6 +7,7 @@ #include +#include #include #include "test.h" @@ -29,8 +30,8 @@ protected: { unsigned int count = 0; - for (auto name : cm->list()) { - cout << "- " << name << endl; + for (Camera *camera : cm->cameras()) { + cout << "- " << camera->name() << endl; count++; } From patchwork Thu Jan 17 23:59:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 270 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 549B460C98 for ; Fri, 18 Jan 2019 00:59:21 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EDE51D4B for ; Fri, 18 Jan 2019 00:59:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547769561; bh=7vXCoRKGVW4oRqThZ/cG4QU+DohssURNKKDdIE4Ss44=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ctvBKotT4Cd5c6ioEgjC4S8Qx6xFgnEmZL1TODs5ywEDYnRHksnuD3MaStcsR2seI /uu5SrXiM1w/uDVO3txd8a+8dicTpsipIdt+Z8x31njfQ3pfhgay5OxZQkN45hpqOe 4JD8FcmVarAkcPUkdIq+HaX6I2J055deiO67+KFk= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 18 Jan 2019 01:59:16 +0200 Message-Id: <20190117235916.1906-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> References: <20190117235916.1906-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 4/4] libcamera: camera: Handle camera objects through shared pointers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jan 2019 23:59:21 -0000 The Camera class is explicitly reference-counted to manage the lifetime of camera objects. Replace this open-coded implementation with usage of the std::shared_ptr<> class. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/camera.h | 13 +++++---- include/libcamera/camera_manager.h | 8 +++--- src/libcamera/camera.cpp | 46 +++++++++++++++++------------- src/libcamera/camera_manager.cpp | 20 ++++++------- src/libcamera/pipeline/vimc.cpp | 2 +- test/list-cameras.cpp | 2 +- 6 files changed, 50 insertions(+), 41 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 9a7579d61fa3..ef0a794e3a82 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_CAMERA_H__ #define __LIBCAMERA_CAMERA_H__ +#include #include namespace libcamera { @@ -14,15 +15,17 @@ namespace libcamera { class Camera { public: - Camera(const std::string &name); + static std::shared_ptr create(const std::string &name); + + Camera(const Camera &) = delete; + void operator=(const Camera &) = delete; const std::string &name() const; - void get(); - void put(); private: - virtual ~Camera() { }; - int ref_; + Camera(const std::string &name); + ~Camera(); + std::string name_; }; diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 4b941fd9183b..738b13f4cfb1 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -24,10 +24,10 @@ public: int start(); void stop(); - const std::vector &cameras() const { return cameras_; } - Camera *get(const std::string &name); + const std::vector> &cameras() const { return cameras_; } + std::shared_ptr get(const std::string &name); - void addCamera(Camera *camera); + void addCamera(std::shared_ptr &camera); static CameraManager *instance(); @@ -42,7 +42,7 @@ private: std::unique_ptr enumerator_; std::vector pipes_; - std::vector cameras_; + std::vector> cameras_; std::unique_ptr dispatcher_; }; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 6da2b20137d4..acf912bee95c 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -41,19 +41,36 @@ namespace libcamera { * streams from a single image source. It provides the main interface to * configuring and controlling the device, and capturing image streams. It is * the central object exposed by libcamera. + * + * To support the central nature of Camera objects, libcamera manages the + * lifetime of camera instances with std::shared_ptr<>. Instances shall be + * created with the create() function which returns a shared pointer. The + * Camera constructors and destructor are private, to prevent instances from + * being constructed and destroyed manually. */ /** - * \brief Construct a named camera device + * \brief Create a camera instance + * \param[in] name The name of the camera device * - * \param[in] name The name to set on the camera device + * The caller is responsible for guaranteeing unicity of the camera name. * - * The caller is responsible for guaranteeing unicity of the camera - * device name. + * \return A shared pointer to the newly created camera object */ -Camera::Camera(const std::string &name) - : ref_(1), name_(name) +std::shared_ptr Camera::create(const std::string &name) { + struct Allocator : std::allocator { + void construct(void *p, const std::string &name) + { + ::new(p) Camera(name); + } + void destroy(Camera *p) + { + p->~Camera(); + } + }; + + return std::allocate_shared(Allocator(), name); } /** @@ -66,24 +83,13 @@ const std::string &Camera::name() const return name_; } -/** - * \brief Acquire a reference to the camera - */ -void Camera::get() +Camera::Camera(const std::string &name) + : name_(name) { - ref_++; } -/** - * \brief Release a reference to the camera - * - * When the last reference is released the camera device is deleted. Callers - * shall not access the camera device after calling this function. - */ -void Camera::put() +Camera::~Camera() { - if (--ref_ == 0) - delete this; } } /* namespace libcamera */ diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 852e5ed70fe3..d12bd42001ae 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -39,9 +39,11 @@ namespace libcamera { * This will enumerate all the cameras present in the system, which can then be * listed with list() and retrieved with get(). * - * Cameras are reference-counted, and shall be returned to the camera manager - * with Camera::put() after being used. Once all cameras have been returned to - * the manager, it can be stopped with stop(). + * Cameras are shared through std::shared_ptr<>, ensuring that a camera will + * stay valid until the last reference is released without requiring any special + * action from the application. Once the application has released all the + * references it held to cameras, the camera manager can be stopped with + * stop(). * * \todo Add ability to add and remove media devices based on hot-(un)plug * events coming from the device enumerator. @@ -147,15 +149,13 @@ void CameraManager::stop() * \param[in] name Name of camera to get * * Before calling this function the caller is responsible for ensuring that - * the camera manger is running. A camera fetched this way shall be - * released by the user with the put() method of the Camera object once - * it is done using the camera. + * the camera manger is running. * - * \return Pointer to Camera object or nullptr if camera not found + * \return Shared pointer to Camera object or nullptr if camera not found */ -Camera *CameraManager::get(const std::string &name) +std::shared_ptr CameraManager::get(const std::string &name) { - for (Camera *camera : cameras_) { + for (std::shared_ptr camera : cameras_) { if (camera->name() == name) return camera; } @@ -171,7 +171,7 @@ Camera *CameraManager::get(const std::string &name) * handle with the camera manager. Registered cameras are immediately made * available to the system. */ -void CameraManager::addCamera(Camera *camera) +void CameraManager::addCamera(std::shared_ptr &camera) { cameras_.push_back(camera); } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 8742e0bae9a8..306a5b85cd60 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -64,7 +64,7 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator * will be chosen depends on how the Camera * object is modeled. */ - Camera *camera = new Camera("Dummy VIMC Camera"); + std::shared_ptr camera = Camera::create("Dummy VIMC Camera"); manager->addCamera(camera); return true; diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp index fdbbda0957b2..070cbf2be977 100644 --- a/test/list-cameras.cpp +++ b/test/list-cameras.cpp @@ -30,7 +30,7 @@ protected: { unsigned int count = 0; - for (Camera *camera : cm->cameras()) { + for (const std::shared_ptr &camera : cm->cameras()) { cout << "- " << camera->name() << endl; count++; }