<div id="geary-body" dir="auto"><div>Hi Laurent,</div><div><br></div><div>Thanks for the feedback.</div></div><div id="geary-quote" dir="auto"><br>On Wed, May 6, 2020 at 23:53, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:<br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">Hi Umang,
Thank you for the patch.
On Wed, May 06, 2020 at 10:33:53AM +0000, Umang Jain wrote:
<blockquote> Emit 'cameraAdded' and 'cameraRemoved' from CameraManager to enable
hotplug and hot-unplug support in appplications like QCam.
Signed-off-by: Umang Jain <<a href="mailto:email@uajain.com">email@uajain.com</a>>
---
include/libcamera/camera_manager.h | 4 ++++
src/libcamera/camera_manager.cpp | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
index 079f848..558bb96 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 {
@@ -42,6 +43,9 @@ public:
void setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);
EventDispatcher *eventDispatcher();
+ Signal<Camera *> cameraAdded;
+ Signal<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 c75979a..6438f87 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -391,6 +391,23 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
return iter->second.lock();
}
+/**
+ * \var CameraManager::cameraAdded
+ * \brief Signal emitted when a new camera is added in CameraManager
+ *
+ * This signal is emitted when a new camera is added by the CameraManager
+ * in the list of cameras it manages. A pointer to the newly-added camera
+ * is passed as a parameter.
</blockquote>
I think we should detail here the relationship with
CameraManager::start() and CameraManager::cameras(), and note that the
signal handler should be fast (otherwise it would block operation of
other cameras). How about the following ?
* \brief Notify of a new camera added to the system
*
* 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.
<blockquote> + */
+
+/**
+ * \var CameraManager::cameraRemoved
+ * \brief Signal emitted when a camera is removed in CameraManager
+ *
+ * This signal is emitted when a camera is removed from the CameraManager.
+ * A pointer to the removed camera is passed as a parameter.
</blockquote>
And something similar here ?
* \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.
<blockquote> + */
+
/**
* \brief Add a camera to the camera manager
* \param[in] camera The camera to be added
@@ -409,7 +426,9 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
{
ASSERT(Thread::current() == p_.get());
+ Camera *cam = camera.get();
p_->addCamera(camera, devnum);
+ cameraAdded.emit(cam);
</blockquote>
You don't need a local cam variable as camera is a shared pointer, not a
unique pointer (and if it was a unique pointer and was given to
addCamera() with std::move, it would be potentially unsafe to use it
afterwards).
cameraAdded.emit(camera.get());
is fine.</div></blockquote><span style="white-space: pre-wrap;"><div><span style="white-space: pre-wrap;"><br></span></div>Yes, camera is a shared pointer here and is std::move in p_->addCamera(camera, devnum);</span><div><span style="white-space: pre-wrap;"> But I noticed that camera becomes nullptr after p_->addCamera() call and that's</span><div><span style="white-space: pre-wrap;">I would expect. I double-checked with running the codepath with your change,</span></div><div><span style="white-space: pre-wrap;">and plugging in a camera crashes with a `null` segfault backtrace. Am I missing something?</span></div><div><span style="white-space: pre-wrap;"><br></span><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">
<blockquote> }
/**
@@ -427,6 +446,7 @@ void CameraManager::removeCamera(Camera *camera)
ASSERT(Thread::current() == p_.get());
p_->removeCamera(camera);
+ cameraRemoved.emit(camera);
}
/**
</blockquote>
<div>--
</div>Regards,
Laurent Pinchart
</div></blockquote></div></div></div><img src="https://u15657259.ct.sendgrid.net/wf/open?upn=GCEip0g28ftA9O9fsCR2M7x08El53O4YVYtHuSI-2FrLwtytoSlmO-2FnSq-2B0q807R-2FEkpfNqlczdJaWdOJHav-2Fc-2BQMibjmx8xoMSh3SmoCMcc2VPDHftBCElhXW-2FlfUmVyOwQiOuqLSl30bMI36IXniuMsQ73v-2BCOtMXHr0cbRl-2Fp8VVKMJAKAOo1bxvk2iDwZd8G6hJmv0IMzSv07GWiwXyuFYFwkAhQdrMNn1xBgmf9rbifRh5IjQjX76ldxoQPOv" alt="" width="1" height="1" border="0" style="height:1px !important;width:1px !important;border-width:0 !important;margin-top:0 !important;margin-bottom:0 !important;margin-right:0 !important;margin-left:0 !important;padding-top:0 !important;padding-bottom:0 !important;padding-right:0 !important;padding-left:0 !important;"/>