[libcamera-devel] [PATCH v2 3/5] libcamera: camera_manager: Introduce signals when a camera is added/removed
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon May 18 17:44:17 CEST 2020
Hi Umang,
On 13/05/2020 18:29, Umang Jain wrote:
> Emit 'cameraAdded' and 'cameraRemoved' from CameraManager to enable
> hotplug and hot-unplug support in appplications like QCam.
s/appplications/applications/
Hrm ... seeing cameraAdded and cameraRemoved, makes me think back to my
earlier signal name bikeshedding ;-) Perhaps that one should be
deviceAdded and deviceRemoved (was newDevicesFound?)
Anyway, let the bikeshedding happen on that patch instead ;-)
> Signed-off-by: Umang Jain <email at uajain.com>
> ---
> include/libcamera/camera_manager.h | 6 +++++-
> src/libcamera/camera_manager.cpp | 34 +++++++++++++++++++++++++++---
> src/libcamera/pipeline_handler.cpp | 2 +-
> 3 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 079f848..366fa07 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -13,6 +13,7 @@
> #include <vector>
>
> #include <libcamera/object.h>
> +#include <libcamera/signal.h>
>
> namespace libcamera {
>
> @@ -35,13 +36,16 @@ public:
> std::shared_ptr<Camera> get(dev_t devnum);
>
> void addCamera(std::shared_ptr<Camera> camera, dev_t devnum);
> - void removeCamera(Camera *camera);
> + void removeCamera(std::shared_ptr<Camera> camera);
>
> static const std::string &version() { return version_; }
>
> void setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);
> EventDispatcher *eventDispatcher();
>
> + Signal<std::shared_ptr<Camera>> cameraAdded;
> + Signal<std::shared_ptr<Camera>> cameraRemoved;
> +
> private:
> static const std::string version_;
> static CameraManager *self_;
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index a13cfe1..b9d2496 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -186,7 +186,7 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,
> }
> }
>
> - cameras_.push_back(std::move(camera));
> + cameras_.push_back(camera);
>
> if (devnum) {
> unsigned int index = cameras_.size() - 1;
> @@ -376,6 +376,32 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
> return iter->second.lock();
> }
>
> +/**
> + * \brief Notify of a new camera added to the system
Missing the \var to reference the signal.
> + *
> + * This signal is emitted when a new camera is detected and successfully handled
> + * by the camera manager. The notification occurs alike for cameras detected
> + * when the manager is started with start() or when new cameras are later
> + * connected to the system. When the signal is emitted the new camera is already
> + * available from the list of cameras().
> + *
> + * The signal is emitted from the CameraManager thread. Applications shall
> + * minimize the time spent in the signal handler and shall in particular not
> + * perform any blocking operation.
> + */
> +
> +/**
And missing here too.
> + * \brief Notify of a new camera removed from the system
> + *
> + * This signal is emitted when a camera is removed from the system. When the
> + * signal is emitted the camera is not available from the list of cameras()
> + * anymore.
> + *
> + * The signal is emitted from the CameraManager thread. Applications shall
> + * minimize the time spent in the signal handler and shall in particular not
> + * perform any blocking operation.
> + */
> +
> /**
> * \brief Add a camera to the camera manager
> * \param[in] camera The camera to be added
> @@ -395,6 +421,7 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
> ASSERT(Thread::current() == p_.get());
>
> p_->addCamera(camera, devnum);
> + cameraAdded.emit(camera);
> }
>
> /**
> @@ -407,11 +434,12 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
> *
> * \context This function shall be called from the CameraManager thread.
> */
> -void CameraManager::removeCamera(Camera *camera)
> +void CameraManager::removeCamera(std::shared_ptr<Camera> camera)
> {
> ASSERT(Thread::current() == p_.get());
>
> - p_->removeCamera(camera);
> + p_->removeCamera(camera.get());
> + cameraRemoved.emit(camera);
Is there any race here, or potential problem emitting the signal with
the camera, after it's been removed from the CameraManager? (I
hope/expect not as it's using a shared_ptr ... But I haven't checked to
see if there are any implications caused by the removeCamera call)
> }
>
> /**
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 254d341..a9187e1 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -555,7 +555,7 @@ void PipelineHandler::disconnect()
> continue;
>
> camera->disconnect();
> - manager_->removeCamera(camera.get());
> + manager_->removeCamera(camera);
> }
>
> cameras_.clear();
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list