From patchwork Thu Jan 24 10:16:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 358 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 035E760C78 for ; Thu, 24 Jan 2019 11:16:58 +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 8D94623A; Thu, 24 Jan 2019 11:16:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325017; bh=6gV9aidWQeYC32QKe+bYH03vMjAHzkuR/hfxsbBAwoA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=g0blyj8aHBAmVs5BwTA8+IeCpDndUF0OztU7t0HFRO4+cRfEXJt75tmy4JjkrzfoU 6bQbBlbwV4WTPGqH3AMvvpfNEEGyvIlkww+r0vESWTdE867IIUbcEcbhtDebdfS2Is IC3O4BBt2dHsZjgYX/Z65mT4aniZL5mTTjSzBNAc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:44 +0200 Message-Id: <20190124101651.9993-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190124101651.9993-1-laurent.pinchart@ideasonboard.com> References: <20190124101651.9993-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 03/10] libcamera: camera: Associate cameras with their pipeline handler 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, 24 Jan 2019 10:16:58 -0000 From: Niklas Söderlund The PipelineHandler which creates a Camera is responsible for serving any operation requested by the user. In order forward the public API calls, the camera needs to store a reference to its pipeline handler. Signed-off-by: Niklas Söderlund Signed-off-by: Laurent Pinchart --- Changes since v1: - Create pipeline handlers is shared pointers, make them inherit from std::enable_shared_from_this<> and stored them in shared pointers. --- include/libcamera/camera.h | 8 ++++++-- include/libcamera/camera_manager.h | 2 +- src/libcamera/camera.cpp | 16 ++++++++++------ src/libcamera/camera_manager.cpp | 17 +++++++++-------- src/libcamera/include/pipeline_handler.h | 9 +++++---- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 9 +-------- src/libcamera/pipeline_handler.cpp | 10 ++++++++++ 9 files changed, 44 insertions(+), 31 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 2ea1a6883311..efafb9e28c56 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -12,10 +12,13 @@ namespace libcamera { +class PipelineHandler; + class Camera final { public: - static std::shared_ptr create(const std::string &name); + static std::shared_ptr create(PipelineHandler *pipe, + const std::string &name); Camera(const Camera &) = delete; void operator=(const Camera &) = delete; @@ -23,9 +26,10 @@ public: const std::string &name() const; private: - explicit Camera(const std::string &name); + Camera(PipelineHandler *pipe, const std::string &name); ~Camera(); + std::shared_ptr pipe_; std::string name_; }; diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 9ade29d76692..45e72df0ef65 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -41,7 +41,7 @@ private: ~CameraManager(); std::unique_ptr enumerator_; - std::vector pipes_; + std::vector> pipes_; std::vector> cameras_; std::unique_ptr dispatcher_; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index acf912bee95c..3a531c7e4d8f 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -8,6 +8,7 @@ #include #include "log.h" +#include "pipeline_handler.h" /** * \file camera.h @@ -52,17 +53,20 @@ namespace libcamera { /** * \brief Create a camera instance * \param[in] name The name of the camera device + * \param[in] pipe The pipeline handler responsible for the camera device * * The caller is responsible for guaranteeing unicity of the camera name. * * \return A shared pointer to the newly created camera object */ -std::shared_ptr Camera::create(const std::string &name) +std::shared_ptr Camera::create(PipelineHandler *pipe, + const std::string &name) { struct Allocator : std::allocator { - void construct(void *p, const std::string &name) + void construct(void *p, PipelineHandler *pipe, + const std::string &name) { - ::new(p) Camera(name); + ::new(p) Camera(pipe, name); } void destroy(Camera *p) { @@ -70,7 +74,7 @@ std::shared_ptr Camera::create(const std::string &name) } }; - return std::allocate_shared(Allocator(), name); + return std::allocate_shared(Allocator(), pipe, name); } /** @@ -83,8 +87,8 @@ const std::string &Camera::name() const return name_; } -Camera::Camera(const std::string &name) - : name_(name) +Camera::Camera(PipelineHandler *pipe, const std::string &name) + : pipe_(pipe->shared_from_this()), name_(name) { } diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 14410d4dcda7..3eccf20c4ce9 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -98,16 +98,14 @@ int CameraManager::start() * all pipelines it can provide. */ while (1) { - PipelineHandler *pipe = factory->create(this); - if (!pipe->match(enumerator_.get())) { - delete pipe; + std::shared_ptr pipe = factory->create(this); + if (!pipe->match(enumerator_.get())) break; - } LOG(Camera, Debug) << "Pipeline handler \"" << factory->name() << "\" matched"; - pipes_.push_back(pipe); + pipes_.push_back(std::move(pipe)); } } @@ -130,10 +128,13 @@ void CameraManager::stop() { /* TODO: unregister hot-plug callback here */ - for (PipelineHandler *pipe : pipes_) - delete pipe; - + /* + * Release all references to cameras and pipeline handlers to ensure + * they all get destroyed before the device enumerator deletes the + * media devices. + */ pipes_.clear(); + cameras_.clear(); enumerator_.reset(nullptr); } diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 1da6dc758ca6..e1d6369eb0c4 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -8,6 +8,7 @@ #define __LIBCAMERA_PIPELINE_HANDLER_H__ #include +#include #include #include @@ -16,7 +17,7 @@ namespace libcamera { class CameraManager; class DeviceEnumerator; -class PipelineHandler +class PipelineHandler : public std::enable_shared_from_this { public: PipelineHandler(CameraManager *manager); @@ -34,7 +35,7 @@ public: PipelineHandlerFactory(const char *name); virtual ~PipelineHandlerFactory() { }; - virtual PipelineHandler *create(CameraManager *manager) = 0; + virtual std::shared_ptr create(CameraManager *manager) = 0; const std::string &name() const { return name_; } @@ -50,9 +51,9 @@ class handler##Factory final : public PipelineHandlerFactory \ { \ public: \ handler##Factory() : PipelineHandlerFactory(#handler) {} \ - PipelineHandler *create(CameraManager *manager) \ + std::shared_ptr create(CameraManager *manager) \ { \ - return new handler(manager); \ + return std::make_shared(manager); \ } \ }; \ static handler##Factory global_##handler##Factory; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 589b3078f301..13ff7da4c99e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -171,7 +171,7 @@ void PipelineHandlerIPU3::registerCameras() continue; std::string cameraName = sensor->name() + " " + std::to_string(id); - std::shared_ptr camera = Camera::create(cameraName); + std::shared_ptr camera = Camera::create(this, cameraName); manager_->addCamera(std::move(camera)); LOG(IPU3, Info) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 92b23da73901..3ebc074093ab 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) dev_->acquire(); - std::shared_ptr camera = Camera::create(dev_->model()); + std::shared_ptr camera = Camera::create(this, dev_->model()); manager_->addCamera(std::move(camera)); return true; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index f12d007cd956..68bfe9b12ab6 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -57,14 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator) dev_->acquire(); - /* - * NOTE: A more complete Camera implementation could - * be passed the MediaDevice(s) it controls here or - * a reference to the PipelineHandler. Which method - * will be chosen depends on how the Camera - * object is modeled. - */ - std::shared_ptr camera = Camera::create("Dummy VIMC Camera"); + std::shared_ptr camera = Camera::create(this, "Dummy VIMC Camera"); manager_->addCamera(std::move(camera)); return true; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 45788487b5c0..3850ea8fadb5 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -32,11 +32,21 @@ LOG_DEFINE_CATEGORY(Pipeline) * * The PipelineHandler matches the media devices provided by a DeviceEnumerator * with the pipelines it supports and creates corresponding Camera devices. + * + * Pipeline handler instances are reference-counted through std::shared_ptr<>. + * They implement std::enable_shared_from_this<> in order to create new + * std::shared_ptr<> in code paths originating from member functions of the + * PipelineHandler class where only the 'this' pointer is available. */ /** * \brief Construct a PipelineHandler instance * \param[in] manager The camera manager + * + * In order to honour the std::enable_shared_from_this<> contract, + * PipelineHandler instances shall never be constructed manually, but always + * through the PipelineHandlerFactory::create() method implemented by the + * respective factories. */ PipelineHandler::PipelineHandler(CameraManager *manager) : manager_(manager)