[libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler: Add camera disconnection support
Jacopo Mondi
jacopo at jmondi.org
Fri Jan 25 00:24:49 CET 2019
Hi Laurent, Niklas
a quite small things
On Thu, Jan 24, 2019 at 12:16:49PM +0200, Laurent Pinchart wrote:
> From: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> 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 <niklas.soderlund at ragnatech.se>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> 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<PipelineHandler>
> {
> @@ -27,6 +28,15 @@ public:
>
> protected:
> CameraManager *manager_;
> +
> + void registerCamera(std::shared_ptr<Camera> camera);
> + void hotplugMediaDevice(MediaDevice *media);
> +
> +private:
> + virtual void disconnect();
> + void mediaDeviceDisconnected(MediaDevice *media);
> +
> + std::vector<std::weak_ptr<Camera>> 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 <vector>
>
> #include <libcamera/camera.h>
> -#include <libcamera/camera_manager.h>
>
> #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 = 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 <libcamera/camera.h>
> -#include <libcamera/camera_manager.h>
>
> #include "device_enumerator.h"
> #include "media_device.h"
> @@ -49,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> dev_->acquire();
>
> std::shared_ptr<Camera> 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 <libcamera/camera.h>
> -#include <libcamera/camera_manager.h>
>
> #include "device_enumerator.h"
> #include "media_device.h"
> @@ -58,7 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
> dev_->acquire();
>
> std::shared_ptr<Camera> 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 <libcamera/camera.h>
> +#include <libcamera/camera_manager.h>
> +
> #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> camera)
> +{
> + cameras_.push_back(camera);
> + manager_->addCamera(std::move(camera));
> +}
> +
> +/**
> + * \brief Handle hotplugging of a media device
"Handle" here misleaded me. What about "Enable" ?
> + * \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<Camera> ptr : cameras_) {
> + std::shared_ptr<Camera> camera = ptr.lock();
> + if (!camera)
> + continue;
> +
I wonder if sub-classes need a disconnectCamera(*camera) to perform
per-camera operations before disconnect...
> + 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
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190125/929ff124/attachment.sig>
More information about the libcamera-devel
mailing list