From patchwork Thu Jan 24 10:16:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 356 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6424360C78 for ; Thu, 24 Jan 2019 11:16:57 +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 D02B72F6; Thu, 24 Jan 2019 11:16:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325017; bh=assAIgFS7yxI5tahqSAH/spUZDdGW1KgSnrmxsr4QNs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HfOyU+Ol4vF6mcQ96Lp0dQr+WXo7hzWs1f0if5SuINkbtsoE6mJ1FYZ53fmRJpelH YFgmA6f/3Drr79s+BAPx6crwfYLLWA3Ix4z2KDag9ojCkzNr3tC5TdrqeMtl4p5EuP YhJrmPKNoR0cXswSrAR2gAMCI6XL36mtWL5Yoc9c= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:42 +0200 Message-Id: <20190124101651.9993-2-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 01/10] libcamera: pipeline_handler: Store the camera manager pointer 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:57 -0000 Instead of passing the camera manager pointer to the match() function, and later to more PipelineHandler functions, store it in the PipelineHandler::manager_ member variable at construction time and access it from there. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/camera_manager.cpp | 4 +-- src/libcamera/include/pipeline_handler.h | 20 ++++++++----- src/libcamera/pipeline/ipu3/ipu3.cpp | 18 ++++++------ src/libcamera/pipeline/uvcvideo.cpp | 12 ++++---- src/libcamera/pipeline/vimc.cpp | 12 ++++---- src/libcamera/pipeline_handler.cpp | 36 ++++++++++++++++++++---- 6 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 4ea7ed44cc31..14410d4dcda7 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -98,8 +98,8 @@ int CameraManager::start() * all pipelines it can provide. */ while (1) { - PipelineHandler *pipe = factory->create(); - if (!pipe->match(this, enumerator_.get())) { + PipelineHandler *pipe = factory->create(this); + if (!pipe->match(enumerator_.get())) { delete pipe; break; } diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index b03217d5bff8..7bb07d1ec5c7 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -19,9 +19,13 @@ class DeviceEnumerator; class PipelineHandler { public: - virtual ~PipelineHandler() { }; + PipelineHandler(CameraManager *manager); + virtual ~PipelineHandler(); - virtual bool match(CameraManager *manager, DeviceEnumerator *enumerator) = 0; + virtual bool match(DeviceEnumerator *enumerator) = 0; + +protected: + CameraManager *manager_; }; class PipelineHandlerFactory @@ -30,7 +34,7 @@ public: PipelineHandlerFactory(const char *name); virtual ~PipelineHandlerFactory() { }; - virtual PipelineHandler *create() = 0; + virtual PipelineHandler *create(CameraManager *manager) = 0; const std::string &name() const { return name_; } @@ -42,11 +46,13 @@ private: }; #define REGISTER_PIPELINE_HANDLER(handler) \ -class handler##Factory : public PipelineHandlerFactory { \ +class handler##Factory : public PipelineHandlerFactory \ +{ \ public: \ - handler##Factory() : PipelineHandlerFactory(#handler) { } \ - PipelineHandler *create() final { \ - return new handler(); \ + handler##Factory() : PipelineHandlerFactory(#handler) {} \ + PipelineHandler *create(CameraManager *manager) final \ + { \ + return new handler(manager); \ } \ }; \ static handler##Factory global_##handler##Factory; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8cbc10acfbb5..589b3078f301 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -23,20 +23,20 @@ LOG_DEFINE_CATEGORY(IPU3) class PipelineHandlerIPU3 : public PipelineHandler { public: - PipelineHandlerIPU3(); + PipelineHandlerIPU3(CameraManager *manager); ~PipelineHandlerIPU3(); - bool match(CameraManager *manager, DeviceEnumerator *enumerator); + bool match(DeviceEnumerator *enumerator); private: MediaDevice *cio2_; MediaDevice *imgu_; - void registerCameras(CameraManager *manager); + void registerCameras(); }; -PipelineHandlerIPU3::PipelineHandlerIPU3() - : cio2_(nullptr), imgu_(nullptr) +PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) + : PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr) { } @@ -52,7 +52,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3() imgu_ = nullptr; } -bool PipelineHandlerIPU3::match(CameraManager *manager, DeviceEnumerator *enumerator) +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) { DeviceMatch cio2_dm("ipu3-cio2"); cio2_dm.add("ipu3-csi2 0"); @@ -106,7 +106,7 @@ bool PipelineHandlerIPU3::match(CameraManager *manager, DeviceEnumerator *enumer if (cio2_->disableLinks()) goto error_close_cio2; - registerCameras(manager); + registerCameras(); cio2_->close(); @@ -127,7 +127,7 @@ error_release_mdev: * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four * CIO2 CSI-2 receivers. */ -void PipelineHandlerIPU3::registerCameras(CameraManager *manager) +void PipelineHandlerIPU3::registerCameras() { /* * For each CSI-2 receiver on the IPU3, create a Camera if an @@ -172,7 +172,7 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager) std::string cameraName = sensor->name() + " " + std::to_string(id); std::shared_ptr camera = Camera::create(cameraName); - manager->addCamera(std::move(camera)); + manager_->addCamera(std::move(camera)); LOG(IPU3, Info) << "Registered Camera[" << numCameras << "] \"" diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 3ba69da8b775..92b23da73901 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -17,17 +17,17 @@ namespace libcamera { class PipelineHandlerUVC : public PipelineHandler { public: - PipelineHandlerUVC(); + PipelineHandlerUVC(CameraManager *manager); ~PipelineHandlerUVC(); - bool match(CameraManager *manager, DeviceEnumerator *enumerator); + bool match(DeviceEnumerator *enumerator); private: MediaDevice *dev_; }; -PipelineHandlerUVC::PipelineHandlerUVC() - : dev_(nullptr) +PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) + : PipelineHandler(manager), dev_(nullptr) { } @@ -37,7 +37,7 @@ PipelineHandlerUVC::~PipelineHandlerUVC() dev_->release(); } -bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumerator) +bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) { DeviceMatch dm("uvcvideo"); @@ -49,7 +49,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera dev_->acquire(); std::shared_ptr camera = Camera::create(dev_->model()); - manager->addCamera(std::move(camera)); + manager_->addCamera(std::move(camera)); return true; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 82b9237a3d7d..f12d007cd956 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -17,17 +17,17 @@ namespace libcamera { class PipeHandlerVimc : public PipelineHandler { public: - PipeHandlerVimc(); + PipeHandlerVimc(CameraManager *manager); ~PipeHandlerVimc(); - bool match(CameraManager *manager, DeviceEnumerator *enumerator); + bool match(DeviceEnumerator *enumerator); private: MediaDevice *dev_; }; -PipeHandlerVimc::PipeHandlerVimc() - : dev_(nullptr) +PipeHandlerVimc::PipeHandlerVimc(CameraManager *manager) + : PipelineHandler(manager), dev_(nullptr) { } @@ -37,7 +37,7 @@ PipeHandlerVimc::~PipeHandlerVimc() dev_->release(); } -bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator) +bool PipeHandlerVimc::match(DeviceEnumerator *enumerator) { DeviceMatch dm("vimc"); @@ -65,7 +65,7 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator * object is modeled. */ std::shared_ptr camera = Camera::create("Dummy VIMC Camera"); - manager->addCamera(std::move(camera)); + manager_->addCamera(std::move(camera)); return true; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index c24feeafc503..45788487b5c0 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -34,18 +34,30 @@ LOG_DEFINE_CATEGORY(Pipeline) * with the pipelines it supports and creates corresponding Camera devices. */ +/** + * \brief Construct a PipelineHandler instance + * \param[in] manager The camera manager + */ +PipelineHandler::PipelineHandler(CameraManager *manager) + : manager_(manager) +{ +} + +PipelineHandler::~PipelineHandler() +{ +} + /** * \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 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. + * 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, create one or multiple Camera instances and register them with the + * camera 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 @@ -66,6 +78,15 @@ LOG_DEFINE_CATEGORY(Pipeline) * created, or false otherwise */ +/** + * \var PipelineHandler::manager_ + * \brief The Camera manager associated with the pipeline handler + * + * The camera manager pointer is stored in the pipeline handler for the + * convenience of pipeline handler implementations. It remains valid and + * constant for the whole lifetime of the pipeline handler. + */ + /** * \class PipelineHandlerFactory * \brief Registration of PipelineHandler classes and creation of instances @@ -96,8 +117,11 @@ PipelineHandlerFactory::PipelineHandlerFactory(const char *name) /** * \fn PipelineHandlerFactory::create() * \brief Create an instance of the PipelineHandler corresponding to the factory + * \param[in] manager The camera manager * - * This virtual function is implemented by the REGISTER_PIPELINE_HANDLER() macro. + * This virtual function is implemented by the REGISTER_PIPELINE_HANDLER() + * macro. It creates a pipeline handler instance associated with the camera + * \a manager. * * \return a pointer to a newly constructed instance of the PipelineHandler * subclass corresponding to the factory From patchwork Thu Jan 24 10:16:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 357 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A0EDC60C78 for ; Thu, 24 Jan 2019 11:16:57 +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 35B3A2F7; 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=qq4iBObw1ANHFHTo6gmT0APE6NV95bwtWEDhW1jtsb8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XTWtJz4SUajONPuabJd9WXKSoE1m6xM45IDrUwkRrSBXv2XzcKXj6e3lJnRkvSMNf lmwz+yorZsaWFSC3za1v3o+J2uwglIYkW7fcE0jVEttJim6C8x+7tsdZfA9YPPL3Es yNyX6doKgeUDYG0RUnXnhlG+xrC8XHXQ4GEwI86c= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:43 +0200 Message-Id: <20190124101651.9993-3-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 02/10] libcamera: pipeline_handler: Declare factory children classes as final 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:57 -0000 Nothing should inherit from the factory classes created by the REGISTER_PIPELINE_HANDLER() macro. Declare them as final instead of only declaring their create() method final. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/pipeline_handler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 7bb07d1ec5c7..1da6dc758ca6 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -46,11 +46,11 @@ private: }; #define REGISTER_PIPELINE_HANDLER(handler) \ -class handler##Factory : public PipelineHandlerFactory \ +class handler##Factory final : public PipelineHandlerFactory \ { \ public: \ handler##Factory() : PipelineHandlerFactory(#handler) {} \ - PipelineHandler *create(CameraManager *manager) final \ + PipelineHandler *create(CameraManager *manager) \ { \ return new handler(manager); \ } \ 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) From patchwork Thu Jan 24 10:16:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 359 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6132C60C78 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 E5E6E2F6; 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=1548325018; bh=IclIqyBCrpuH2EWaUHZdqmltq/6gEkpA7l356d5T3sc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XmnUou7/3oMQpueZpA/PSEi2tCdl0UxGgnS+2JarqBes/eBdJF3nll3saGIcnO1qo viZskZrCANHsB120GgBvEJwSu9bD+kpOpPDYhzr9ddOv4XJXZgpWcqgFVqlD/NpBe/ 2aydOMFetYaNZ7hXilC0aurCAhbXIIIFZguMddcc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:45 +0200 Message-Id: <20190124101651.9993-5-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 04/10] libcamera: device_enumerator: Reference-count MediaDevice instances 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 The MediaDevice class will be the entry point to hot-unplug, as it corresponds to the kernel devices that will report device removal events. The class will signal media device disconnection to pipeline handlers, which will clean up resources as a result. This can't be performed synchronously as references may exist to the related Camera objects in applications. The MediaDevice object thus needs to be reference-counted in order to support unplugging, as otherwise pipeline handlers would be required to drop all the references to the media device they have borrowed synchronously with the disconnection signal handler, which would be very error prone (if even possible at all in a sane way). Handle MedieDevice instances with std::shared_ptr<> to support this. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/device_enumerator.cpp | 23 ++++++++++---------- src/libcamera/include/device_enumerator.h | 4 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 11 ++++------ src/libcamera/pipeline/uvcvideo.cpp | 4 ++-- src/libcamera/pipeline/vimc.cpp | 4 ++-- test/media_device/media_device_link_test.cpp | 2 +- test/pipeline/ipu3/ipu3_pipeline_test.cpp | 2 +- 7 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index 8100972a8d04..149ffbf9aea6 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -166,12 +166,10 @@ std::unique_ptr DeviceEnumerator::create() DeviceEnumerator::~DeviceEnumerator() { - for (MediaDevice *dev : devices_) { - if (dev->busy()) + for (std::shared_ptr media : devices_) { + if (media->busy()) LOG(DeviceEnumerator, Error) << "Removing media device while still in use"; - - delete dev; } } @@ -211,7 +209,7 @@ DeviceEnumerator::~DeviceEnumerator() */ int DeviceEnumerator::addDevice(const std::string &deviceNode) { - MediaDevice *media = new MediaDevice(deviceNode); + std::shared_ptr media = std::make_shared(deviceNode); int ret = media->open(); if (ret < 0) @@ -243,9 +241,10 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) return ret; } - devices_.push_back(media); media->close(); + devices_.push_back(std::move(media)); + return 0; } @@ -260,17 +259,17 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) * * \return pointer to the matching MediaDevice, or nullptr if no match is found */ -MediaDevice *DeviceEnumerator::search(const DeviceMatch &dm) +std::shared_ptr DeviceEnumerator::search(const DeviceMatch &dm) { - for (MediaDevice *dev : devices_) { - if (dev->busy()) + for (std::shared_ptr media : devices_) { + if (media->busy()) continue; - if (dm.match(dev)) { + if (dm.match(media.get())) { LOG(DeviceEnumerator, Debug) << "Successful match for media device \"" - << dev->driver() << "\""; - return dev; + << media->driver() << "\""; + return std::move(media); } } diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h index 40c7750b49fc..3f87a6255303 100644 --- a/src/libcamera/include/device_enumerator.h +++ b/src/libcamera/include/device_enumerator.h @@ -42,13 +42,13 @@ public: virtual int init() = 0; virtual int enumerate() = 0; - MediaDevice *search(const DeviceMatch &dm); + std::shared_ptr search(const DeviceMatch &dm); protected: int addDevice(const std::string &deviceNode); private: - std::vector devices_; + std::vector> devices_; virtual std::string lookupDeviceNode(int major, int minor) = 0; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 13ff7da4c99e..9831f74fe53f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -29,8 +29,8 @@ public: bool match(DeviceEnumerator *enumerator); private: - MediaDevice *cio2_; - MediaDevice *imgu_; + std::shared_ptr cio2_; + std::shared_ptr imgu_; void registerCameras(); }; @@ -47,9 +47,6 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3() if (imgu_) imgu_->release(); - - cio2_ = nullptr; - imgu_ = nullptr; } bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) @@ -78,11 +75,11 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) imgu_dm.add("ipu3-imgu 1 viewfinder"); imgu_dm.add("ipu3-imgu 1 3a stat"); - cio2_ = enumerator->search(cio2_dm); + cio2_ = std::move(enumerator->search(cio2_dm)); if (!cio2_) return false; - imgu_ = enumerator->search(imgu_dm); + imgu_ = std::move(enumerator->search(imgu_dm)); if (!imgu_) return false; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 3ebc074093ab..73bad6714bb4 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -23,7 +23,7 @@ public: bool match(DeviceEnumerator *enumerator); private: - MediaDevice *dev_; + std::shared_ptr dev_; }; PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) @@ -41,7 +41,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) { DeviceMatch dm("uvcvideo"); - dev_ = enumerator->search(dm); + dev_ = std::move(enumerator->search(dm)); if (!dev_) return false; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 68bfe9b12ab6..521b20d3a120 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -23,7 +23,7 @@ public: bool match(DeviceEnumerator *enumerator); private: - MediaDevice *dev_; + std::shared_ptr dev_; }; PipeHandlerVimc::PipeHandlerVimc(CameraManager *manager) @@ -51,7 +51,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator) dm.add("RGB/YUV Input"); dm.add("Scaler"); - dev_ = enumerator->search(dm); + dev_ = std::move(enumerator->search(dm)); if (!dev_) return false; diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp index ac5b632f8ed1..58a55cdfee63 100644 --- a/test/media_device/media_device_link_test.cpp +++ b/test/media_device/media_device_link_test.cpp @@ -240,7 +240,7 @@ class MediaDeviceLinkTest : public Test private: unique_ptr enumerator; - MediaDevice *dev_; + shared_ptr dev_; }; TEST_REGISTER(MediaDeviceLinkTest); diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp index 482c12499a86..953f0233cde4 100644 --- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp +++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp @@ -65,7 +65,7 @@ int IPU3PipelineTest::init() return TestSkip; } - MediaDevice *cio2 = enumerator->search(cio2_dm); + std::shared_ptr cio2 = enumerator->search(cio2_dm); if (!cio2) { cerr << "Failed to find IPU3 CIO2: test skip" << endl; return TestSkip; From patchwork Thu Jan 24 10:16:46 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 360 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B198260C78 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 4832E23A; Thu, 24 Jan 2019 11:16:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325018; bh=jiQk+fOuv06fpKtb2EcywkAZQi4ALqLUV3NMZst7JDA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JR5YIoO4JRFlcblEcgxwEnKvE9zUhVO5vubEjvIZm3JUX7z9Nn1jvDb3NuRoOvXXx sD/R1ZVyi/2K6aQP/tDnS2aLpHI1x6I4SQxguptO0/st1jZlYUguRvyShgMs7WByRb gXy8LuCEOhCBTBDtAzJqFamHj0W0rp4zrzlheTJ8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:46 +0200 Message-Id: <20190124101651.9993-6-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 05/10] libcamera: media_device: Add disconnected signal 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 The signal is emitted when the hardware device corresponding to the media device is unplugged. This will trigger the full unplug handling chain. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/media_device.h | 4 ++++ src/libcamera/media_device.cpp | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index 8a7b9489faa9..27a2b46a4392 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -14,6 +14,8 @@ #include +#include + #include "media_object.h" namespace libcamera { @@ -48,6 +50,8 @@ public: MediaLink *link(const MediaPad *source, const MediaPad *sink); int disableLinks(); + Signal disconnected; + private: std::string driver_; std::string deviceNode_; diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 51e5088ebdd9..be81bd8c4c23 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -427,6 +427,16 @@ int MediaDevice::disableLinks() return 0; } +/** + * \var MediaDevice::disconnected + * \brief Signal emitted when the media device is disconnected from the system + * + * This signal is emitted when the device enumerator detects that the media + * device has been removed from the system. For hot-pluggable devices this is + * usually caused by physical device disconnection, but can also result from + * driver unloading for most devices. The media device is passed as a parameter. + */ + /** * \var MediaDevice::objects_ * \brief Global map of media objects (entities, pads, links) keyed by their From patchwork Thu Jan 24 10:16:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 361 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 1434D60C78 for ; Thu, 24 Jan 2019 11:16:59 +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 9F93E2F6; Thu, 24 Jan 2019 11:16:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325018; bh=dXJtiLCzeEV9BXGfV2fAU3jxX0gG+Iy4PAbEtXvexUA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=P8Nls5QbgheKbdTElALSRjRjGXnUzc8vUVyxLg6lq0I16yBj6yeI8X7PMNzRGAPpa vHbX82WpzHt4Q/CYPkwRSX5fMn0zP470JdqLDFSNhgopwgqIq2iaGCZfZkhaXibHzy w93JKBqYvlPC29Sv0vsUcPR0ZYKW5f29JnPjDPBY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:47 +0200 Message-Id: <20190124101651.9993-7-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 06/10] libcamera: camera: Add disconnection notification 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:17:00 -0000 From: Niklas Söderlund As camera object have the potential to outlive the hardware they represent, there is a need to inform the camera that the underlying device has been disconnected, and in turn to notify applications. Implement a disconnection notification mechanism that can be used by pipeline handlers to notify the camera of disconnection. The camera then block all new API calls and emit the disconnected signal. Signed-off-by: Niklas Söderlund Signed-off-by: Laurent Pinchart --- include/libcamera/camera.h | 7 +++++++ src/libcamera/camera.cpp | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index efafb9e28c56..a2ded62de948 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -10,6 +10,8 @@ #include #include +#include + namespace libcamera { class PipelineHandler; @@ -25,10 +27,15 @@ public: const std::string &name() const; + Signal disconnected; + private: Camera(PipelineHandler *pipe, const std::string &name); ~Camera(); + friend class PipelineHandler; + void disconnect(); + std::shared_ptr pipe_; std::string name_; }; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 3a531c7e4d8f..9cec289282e4 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -34,6 +34,8 @@ namespace libcamera { +LOG_DECLARE_CATEGORY(Camera) + /** * \class Camera * \brief Camera device @@ -87,6 +89,18 @@ const std::string &Camera::name() const return name_; } +/** + * \var Camera::disconnected + * \brief Signal emitted when the camera is disconnected from the system + * + * This signal is emitted when libcamera detects that the cameera has been + * removed from the system. For hot-pluggable devices this is usually caused by + * physical device disconnection. The media device is passed as a parameter. + * + * As soon as this signal is emitted the camera instance will refuse all new + * application API calls by returning errors immediately. + */ + Camera::Camera(PipelineHandler *pipe, const std::string &name) : pipe_(pipe->shared_from_this()), name_(name) { @@ -96,4 +110,21 @@ Camera::~Camera() { } +/** + * \brief Notify camera disconnection + * + * This method is used to notify the camera instance that the underlying + * hardware has been unplugged. In response to the disconnection the camera + * instance notifies the application by emitting the #disconnected signal, and + * ensures that all new calls to the application-facing Camera API return an + * error immediately. + */ +void Camera::disconnect() +{ + LOG(Camera, Debug) << "Disconnecting camera " << name_; + + /** \todo Block API calls when they will be implemented. */ + disconnected.emit(this); +} + } /* namespace libcamera */ From patchwork Thu Jan 24 10:16:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 362 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6ADC660C80 for ; Thu, 24 Jan 2019 11:16:59 +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 01E4723A; Thu, 24 Jan 2019 11:16:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325019; bh=L6Pj4vgVp1yJwUojTA335EVrDk1aMDHwLJvuo0r7JQk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qT8cygdgIbS4xGjJUEkkeAG49W7qD+wh0tMHtCo30nPE4wuEvnzZhInSGcp9gG3eZ 5vdlJhx4UWDlSEdghBqNGqJW+yfNnL82cuhsd361vpPwGZXgBkUDZ6FSfFOsbl28vR RwM6S2y4qvDAv2VwJjZ7J/98oo2akEdAvioCM4P4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:48 +0200 Message-Id: <20190124101651.9993-8-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 07/10] libcamera: camera_manager: Add method to unregister a camera 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:17:00 -0000 The new removeCamera() method is meant to be used by pipeline handlers to unregister a camera in case of device disconnection. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera_manager.h | 1 + src/libcamera/camera_manager.cpp | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h index 45e72df0ef65..56dbd26f64db 100644 --- a/include/libcamera/camera_manager.h +++ b/include/libcamera/camera_manager.h @@ -28,6 +28,7 @@ public: std::shared_ptr get(const std::string &name); void addCamera(std::shared_ptr camera); + void removeCamera(Camera *camera); static CameraManager *instance(); diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 3eccf20c4ce9..f90201ade18b 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -191,6 +191,27 @@ void CameraManager::addCamera(std::shared_ptr camera) cameras_.push_back(std::move(camera)); } +/** + * \brief Remove a camera from the camera manager + * \param[in] camera The camera to be removed + * + * This function is called by pipeline handlers to unregister cameras from the + * camera manager. Unregistered cameras won't be reported anymore by the + * cameras() and get() calls, but references may still exist in applications. + */ +void CameraManager::removeCamera(Camera *camera) +{ + for (auto iter = cameras_.begin(); iter != cameras_.end(); ++iter) { + if (iter->get() == camera) { + LOG(Camera, Debug) + << "Unregistering camera '" + << camera->name() << "'"; + cameras_.erase(iter); + return; + } + } +} + /** * \brief Retrieve the camera manager instance * From patchwork Thu Jan 24 10:16:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 363 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 C513B60CA4 for ; Thu, 24 Jan 2019 11:16:59 +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 5946C2F6; Thu, 24 Jan 2019 11:16:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325019; bh=aQGQ8RhvH7WMn6uwvrDcTO+ad1MMOuufb7zWhDXlJyk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wne1UQNDzU9EQdXJgXQDJyqCkdjFEGqeam7r0zwkDPYcfhW7qJMQxQeZoUsG3RnD1 x/luYYFF9m/+TJBhqyTLloDzo/SLZEvHADwhhK2pLzT7rbWoqoC0UFz1Kwz/8hqp07 Vi25xAYhOzzHSI0LAE4cNCQCgiiaf+J9vXyvbfgI= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:49 +0200 Message-Id: <20190124101651.9993-9-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 08/10] libcamera: pipeline_handler: Add camera disconnection support 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:17:01 -0000 From: Niklas Söderlund Pipeline handlers are responsible for creating camera instances, but also for destroying them when devices are unplugged. As camera objects are reference-counted this isn't a straightforward operation and involves the camera manager and camera object itself. Add two helper methods in the PipelineHandler base class to register a camera and to register a media device with the pipeline handler. When registering a camera, the registerCamera() helper method will add it to the camera manager. When registering a media device, the registerMediaDevice() helper method will listen to device disconnection events, and disconnect all cameras created by the pipeline handler as a response. Under the hood the PipelineHandler class needs to keep track of registered cameras in order to handle disconnection. They can't be stored as shared pointers as this would create a circular dependency (the Camera class owns a shared pointer to the pipeline handler). Store them as weak pointers instead. This is safe as a reference to the camera is stored in the camera manager, and doesn't get removed until the camera is unregistered from the manager by the PipelineHandler. Signed-off-by: Niklas Söderlund Signed-off-by: Laurent Pinchart --- src/libcamera/include/pipeline_handler.h | 10 ++++ src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +- src/libcamera/pipeline/uvcvideo.cpp | 3 +- src/libcamera/pipeline/vimc.cpp | 3 +- src/libcamera/pipeline_handler.cpp | 71 ++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 6 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index e1d6369eb0c4..804cce4807ee 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -16,6 +16,7 @@ namespace libcamera { class CameraManager; class DeviceEnumerator; +class MediaDevice; class PipelineHandler : public std::enable_shared_from_this { @@ -27,6 +28,15 @@ public: protected: CameraManager *manager_; + + void registerCamera(std::shared_ptr camera); + void hotplugMediaDevice(MediaDevice *media); + +private: + virtual void disconnect(); + void mediaDeviceDisconnected(MediaDevice *media); + + std::vector> cameras_; }; class PipelineHandlerFactory diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 9831f74fe53f..3161e71420ed 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -9,7 +9,6 @@ #include #include -#include #include "device_enumerator.h" #include "log.h" @@ -169,7 +168,7 @@ void PipelineHandlerIPU3::registerCameras() std::string cameraName = sensor->name() + " " + std::to_string(id); std::shared_ptr camera = Camera::create(this, cameraName); - manager_->addCamera(std::move(camera)); + registerCamera(std::move(camera)); LOG(IPU3, Info) << "Registered Camera[" << numCameras << "] \"" diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 73bad6714bb4..c8f1bf553bfe 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -6,7 +6,6 @@ */ #include -#include #include "device_enumerator.h" #include "media_device.h" @@ -49,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) dev_->acquire(); std::shared_ptr camera = Camera::create(this, dev_->model()); - manager_->addCamera(std::move(camera)); + registerCamera(std::move(camera)); return true; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 521b20d3a120..b714a07688e9 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -6,7 +6,6 @@ */ #include -#include #include "device_enumerator.h" #include "media_device.h" @@ -58,7 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator) dev_->acquire(); std::shared_ptr camera = Camera::create(this, "Dummy VIMC Camera"); - manager_->addCamera(std::move(camera)); + registerCamera(std::move(camera)); return true; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 3850ea8fadb5..f0aa2f8022c2 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -5,7 +5,11 @@ * pipeline_handler.cpp - Pipeline handler infrastructure */ +#include +#include + #include "log.h" +#include "media_device.h" #include "pipeline_handler.h" /** @@ -97,6 +101,73 @@ PipelineHandler::~PipelineHandler() * constant for the whole lifetime of the pipeline handler. */ +/** + * \brief Register a camera to the camera manager and pipeline handler + * \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. + */ +void PipelineHandler::registerCamera(std::shared_ptr camera) +{ + cameras_.push_back(camera); + manager_->addCamera(std::move(camera)); +} + +/** + * \brief Handle hotplugging of a media device + * \param[in] media The media device + * + * This function enables hotplug handling, and especially hot-unplug handling, + * of the \a media device. It shall be called by pipeline handlers for all the + * media devices that can be disconnected. + * + * When a media device passed to this function is later unplugged, the pipeline + * handler gets notified and automatically disconnects all the cameras it has + * registered without requiring any manual intervention. + */ +void PipelineHandler::hotplugMediaDevice(MediaDevice *media) +{ + media->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected); +} + +/** + * \brief Device disconnection handler + * + * This virtual function is called to notify the pipeline handler that the + * device it handles has been disconnected. It notifies all cameras created by + * the pipeline handler that they have been disconnected, and unregisters them + * from the camera manager. + * + * The function can be overloaded by pipeline handlers to perform custom + * operations at disconnection time. Any overloaded version shall call the + * PipelineHandler::disconnect() base function for proper hot-unplug operation. + */ +void PipelineHandler::disconnect() +{ + for (std::weak_ptr ptr : cameras_) { + std::shared_ptr camera = ptr.lock(); + if (!camera) + continue; + + camera->disconnect(); + manager_->removeCamera(camera.get()); + } + + cameras_.clear(); +} + +/** + * \brief Slot for the MediaDevice disconnected signal + */ +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) +{ + if (cameras_.empty()) + return; + + disconnect(); +} + /** * \class PipelineHandlerFactory * \brief Registration of PipelineHandler classes and creation of instances From patchwork Thu Jan 24 10:16:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 364 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A67460CA5 for ; Thu, 24 Jan 2019 11:17:00 +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 B390B23A; Thu, 24 Jan 2019 11:16:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325019; bh=wuiCFL94jMZ7GB3MS6fK6XHOyx0EidxESTbr0aEWsa0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Gt2j0IqF3C8tqm2HCFpgx8g0gmZAalcaO8i3wdusm6pBpqNjUzvsX8rUDsmPwD0ul Fz05ZC8PwD/TYkwkNlPsGDn4dAqWveE3zpbDTNZSSWKtCPiHsoW8vujMlPbW7PK9J6 ukR1URUYxS8W0dfstKM1J6pYdRpXZn8NHrHXyv5k= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:50 +0200 Message-Id: <20190124101651.9993-10-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 09/10] libcamera: pipeline: uvc: Mark the media device as hotpluggable 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:17:01 -0000 UVC devices can be hot-unplugged. Mark the corresponding media device as hotpluggable to ensure proper disconnection support. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/uvcvideo.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index c8f1bf553bfe..f84fa41515f1 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -49,6 +49,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) std::shared_ptr camera = Camera::create(this, dev_->model()); registerCamera(std::move(camera)); + hotplugMediaDevice(dev_.get()); return true; } From patchwork Thu Jan 24 10:16:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 365 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 8B13660C83 for ; Thu, 24 Jan 2019 11:17:00 +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 15AEB2F6; Thu, 24 Jan 2019 11:17:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325020; bh=tEpCdsWCcinodfYHjd5vdST7yxot6qVoCMBXfxD8qqc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KQffT/pA0Dtd95bp4su86TtQBb4SVMq1KR178lT5h5hLbueS1F6aKUKTApvKU6bKD Xe98G0snownxu1CgDBombCdf6foU1tbpUvjE0obN7r+hPJ/TmiUaJE6XQ0gp2FtSU/ mwFSewpgCFCCh2xvY3JxFPDJWS0E4kSUD8ZgMGH0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 24 Jan 2019 12:16:51 +0200 Message-Id: <20190124101651.9993-11-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 10/10] libcamera: device_enumerator: Add hotplug support 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:17:01 -0000 Create a udev_monitor in the udev device enumerator to listen to media device disconnection, and emit the corresponding media device's disconnect signal in response. Signed-off-by: Laurent Pinchart --- src/libcamera/device_enumerator.cpp | 83 ++++++++++++++++++++++- src/libcamera/include/device_enumerator.h | 6 ++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index 149ffbf9aea6..703b03dd418a 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -11,6 +11,8 @@ #include #include +#include + #include "device_enumerator.h" #include "log.h" #include "media_device.h" @@ -243,11 +245,47 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) media->close(); + LOG(DeviceEnumerator, Debug) + << "Added device " << deviceNode << ": " << media->driver(); + devices_.push_back(std::move(media)); return 0; } +/** + * \brief Remove a media device from the enumerator + * \param[in] deviceNode Path to the media device to remove + * + * Remove the media device identified by \a deviceNode previously added to the + * enumerator with addDevice(). The media device's MediaDevice::disconnected + * signal is emitted. + */ +void DeviceEnumerator::removeDevice(const std::string &deviceNode) +{ + std::shared_ptr media; + + for (auto iter = devices_.begin(); iter != devices_.end(); ++iter) { + if ((*iter)->deviceNode() == deviceNode) { + media = std::move(*iter); + devices_.erase(iter); + break; + } + } + + if (!media) { + LOG(DeviceEnumerator, Warning) + << "Media device for node " << deviceNode + << " not found"; + return; + } + + LOG(DeviceEnumerator, Debug) + << "Media device for node " << deviceNode << " removed."; + + media->disconnected.emit(media.get()); +} + /** * \brief Search available media devices for a pattern match * \param[in] dm Search pattern @@ -301,12 +339,18 @@ DeviceEnumeratorUdev::DeviceEnumeratorUdev() DeviceEnumeratorUdev::~DeviceEnumeratorUdev() { + delete notifier_; + + if (monitor_) + udev_monitor_unref(monitor_); if (udev_) udev_unref(udev_); } int DeviceEnumeratorUdev::init() { + int ret; + if (udev_) return -EBUSY; @@ -314,6 +358,15 @@ int DeviceEnumeratorUdev::init() if (!udev_) return -ENODEV; + monitor_ = udev_monitor_new_from_netlink(udev_, "udev"); + if (!monitor_) + return -ENODEV; + + ret = udev_monitor_filter_add_match_subsystem_devtype(monitor_, "media", + nullptr); + if (ret < 0) + return ret; + return 0; } @@ -365,7 +418,18 @@ int DeviceEnumeratorUdev::enumerate() } done: udev_enumerate_unref(udev_enum); - return ret >= 0 ? 0 : ret; + if (ret < 0) + return ret; + + ret = udev_monitor_enable_receiving(monitor_); + if (ret < 0) + return ret; + + int fd = udev_monitor_get_fd(monitor_); + notifier_ = new EventNotifier(fd, EventNotifier::Read); + notifier_->activated.connect(this, &DeviceEnumeratorUdev::udevNotify); + + return 0; } std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor) @@ -389,4 +453,21 @@ std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor) return deviceNode; } +void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier) +{ + struct udev_device *dev = udev_monitor_receive_device(monitor_); + std::string action(udev_device_get_action(dev)); + std::string deviceNode(udev_device_get_devnode(dev)); + + LOG(Debug) << action << " device " << udev_device_get_devnode(dev); + + if (action == "add") { + addDevice(deviceNode); + } else if (action == "remove") { + removeDevice(deviceNode); + } + + udev_device_unref(dev); +} + } /* namespace libcamera */ diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h index 3f87a6255303..22ed8dedcb06 100644 --- a/src/libcamera/include/device_enumerator.h +++ b/src/libcamera/include/device_enumerator.h @@ -16,6 +16,7 @@ namespace libcamera { +class EventNotifier; class MediaDevice; class DeviceMatch @@ -46,6 +47,7 @@ public: protected: int addDevice(const std::string &deviceNode); + void removeDevice(const std::string &deviceNode); private: std::vector> devices_; @@ -64,8 +66,12 @@ public: private: struct udev *udev_; + struct udev_monitor *monitor_; + EventNotifier *notifier_; std::string lookupDeviceNode(int major, int minor) final; + + void udevNotify(EventNotifier *notifier); }; } /* namespace libcamera */