From 5b02e03199b79165086135236d8fb9b2c673aae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20S=C3=B6derlund?= Date: Tue, 22 Jan 2019 16:31:39 +0100 Subject: libcamera: camera: Associate cameras with their pipeline handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 2ea1a688..efafb9e2 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 9ade29d7..45e72df0 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 acf912be..3a531c7e 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 14410d4d..3eccf20c 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 1da6dc75..e1d6369e 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 589b3078..13ff7da4 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 92b23da7..3ebc0740 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 f12d007c..68bfe9b1 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 45788487..3850ea8f 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) -- cgit v1.2.1