From patchwork Fri Jan 18 23:26:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 278 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 8D0BD60C9B for ; Sat, 19 Jan 2019 00:26:22 +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 3267553E for ; Sat, 19 Jan 2019 00:26:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547853982; bh=/5MlneqZLpR0AfZBUo1V/Snk2MBQ0uCwkLao+/Lrndo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=KwbiUCocVgtTtC2BcbblXP18+yHwAcR5DbTHysVba1qagp0ctIXmwC+f2SQizFW8U IdDjPMgEKI1gdgt/t/1no7zGnLMTp+J4hI1qYEd4gyL2Kh2XOwfUHN1vWqgbUjvRso xorxH5fI/aoOB/E3WnkoFSd5JvtVYJKcS44Zug9U= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 19 Jan 2019 01:26:17 +0200 Message-Id: <20190118232617.14631-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190118232617.14631-1-laurent.pinchart@ideasonboard.com> References: <20190118232617.14631-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 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: Fri, 18 Jan 2019 23:26:23 -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. This API change prevents pipeline handlers from subclassing the Camera class. This isn't deemed to be an issue. Mark the class final to make this explicit. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- Changes since v1: - Make the Camera class final - Make the Camera::Camera(const std::string &) constructor explicit - Pass the std::shared_ptr<> by value to CameraManager::addCamera() --- include/libcamera/camera.h | 15 ++++++---- include/libcamera/camera_manager.h | 8 +++--- src/libcamera/camera.cpp | 46 +++++++++++++++++------------- src/libcamera/camera_manager.cpp | 24 ++++++++-------- src/libcamera/pipeline/vimc.cpp | 4 +-- test/list-cameras.cpp | 2 +- 6 files changed, 54 insertions(+), 45 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 9a7579d61fa3..2ea1a6883311 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -7,22 +7,25 @@ #ifndef __LIBCAMERA_CAMERA_H__ #define __LIBCAMERA_CAMERA_H__ +#include #include namespace libcamera { -class Camera +class Camera final { 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_; + explicit 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..9ade29d76692 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 3fa9f23c8e0d..d76eaa7ace86 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -40,9 +40,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. @@ -148,15 +150,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; } @@ -172,9 +172,9 @@ 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) { - for (Camera *c : cameras_) { + for (std::shared_ptr c : cameras_) { if (c->name() == camera->name()) { LOG(Warning) << "Registering camera with duplicate name '" << camera->name() << "'"; @@ -182,7 +182,7 @@ void CameraManager::addCamera(Camera *camera) } } - cameras_.push_back(camera); + cameras_.push_back(std::move(camera)); } /** diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 8742e0bae9a8..82b9237a3d7d 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -64,8 +64,8 @@ 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"); - manager->addCamera(camera); + std::shared_ptr camera = Camera::create("Dummy VIMC Camera"); + manager->addCamera(std::move(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++; }