
<html>Tested-by: Ashok Sidipotu <ashok.sidipotu@collabora.com><br /><br />On Monday, May 15, 2023 18:15 IST, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote:<br /> <blockquote type="cite" cite="20230515124550.3601128-4-kieran.bingham@ideasonboard.com">The CameraManager exposes addCamera and removeCamera as public API<br />calls, while they should never be called from an application. These<br />calls are only expected to be used by PipelineHandlers to update the<br />CameraManager that a new Camera has been created and allow the Camera<br />Manager to expose it to applications.<br /><br />Remove the public calls and update the private implementations such that<br />they can be used directly by the PipelineHandler through the internal<br />CameraManager::Private provided by the Extensible class.<br /><br />Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com><br />Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com><br />---<br />include/libcamera/camera_manager.h | 4 -<br />include/libcamera/internal/camera_manager.h | 2 +-<br />src/libcamera/camera_manager.cpp | 87 +++++++++------------<br />src/libcamera/pipeline_handler.cpp | 6 +-<br />4 files changed, 43 insertions(+), 56 deletions(-)<br /><br />diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h<br />index 4b1fb7568e83..9767acc42c89 100644<br />--- a/include/libcamera/camera_manager.h<br />+++ b/include/libcamera/camera_manager.h<br />@@ -34,10 +34,6 @@ public:<br />std::shared_ptr<Camera> get(const std::string &id);<br />std::shared_ptr<Camera> get(dev_t devnum);<br /><br />- void addCamera(std::shared_ptr<Camera> camera,<br />- const std::vector<dev_t> &devnums);<br />- void removeCamera(std::shared_ptr<Camera> camera);<br />-<br />static const std::string &version() { return version_; }<br /><br />Signal<std::shared_ptr<Camera>> cameraAdded;<br />diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h<br />index 05a1e4df8add..885bb2825687 100644<br />--- a/include/libcamera/internal/camera_manager.h<br />+++ b/include/libcamera/internal/camera_manager.h<br />@@ -31,7 +31,7 @@ public:<br />int start();<br />void addCamera(std::shared_ptr<Camera> camera,<br />const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);<br />- void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);<br />+ void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);<br /><br />/*<br />* This mutex protects<br />diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp<br />index d3c297b888d8..70eb4e455e54 100644<br />--- a/src/libcamera/camera_manager.cpp<br />+++ b/src/libcamera/camera_manager.cpp<br />@@ -147,9 +147,25 @@ void CameraManager::Private::cleanup()<br />enumerator_.reset(nullptr);<br />}<br /><br />+/**<br />+ * \brief Add a camera to the camera manager<br />+ * \param[in] camera The camera to be added<br />+ * \param[in] devnums The device numbers to associate with \a camera<br />+ *<br />+ * This function is called by pipeline handlers to register the cameras they<br />+ * handle with the camera manager. Registered cameras are immediately made<br />+ * available to the system.<br />+ *<br />+ * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes<br />+ * to Camera instances.<br />+ *<br />+ * \context This function shall be called from the CameraManager thread.<br />+ */<br />void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,<br />const std::vector<dev_t> &devnums)<br />{<br />+ ASSERT(Thread::current() == this);<br />+<br />MutexLocker locker(mutex_);<br /><br />for (const std::shared_ptr<Camera> &c : cameras_) {<br />@@ -166,15 +182,31 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,<br />unsigned int index = cameras_.size() - 1;<br />for (dev_t devnum : devnums)<br />camerasByDevnum_[devnum] = cameras_[index];<br />+<br />+ /* Report the addition to the public signal */<br />+ CameraManager *const o = LIBCAMERA_O_PTR();<br />+ o->cameraAdded.emit(cameras_[index]);<br />}<br /><br />-void CameraManager::Private::removeCamera(Camera *camera)<br />+/**<br />+ * \brief Remove a camera from the camera manager<br />+ * \param[in] camera The camera to be removed<br />+ *<br />+ * This function is called by pipeline handlers to unregister cameras from the<br />+ * camera manager. Unregistered cameras won't be reported anymore by the<br />+ * cameras() and get() calls, but references may still exist in applications.<br />+ *<br />+ * \context This function shall be called from the CameraManager thread.<br />+ */<br />+void CameraManager::Private::removeCamera(std::shared_ptr<Camera> camera)<br />{<br />+ ASSERT(Thread::current() == this);<br />+<br />MutexLocker locker(mutex_);<br /><br />auto iter = std::find_if(cameras_.begin(), cameras_.end(),<br />[camera](std::shared_ptr<Camera> &c) {<br />- return c.get() == camera;<br />+ return c.get() == camera.get();<br />});<br />if (iter == cameras_.end())<br />return;<br />@@ -184,12 +216,16 @@ void CameraManager::Private::removeCamera(Camera *camera)<br /><br />auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),<br />[camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {<br />- return p.second.lock().get() == camera;<br />+ return p.second.lock().get() == camera.get();<br />});<br />if (iter_d != camerasByDevnum_.end())<br />camerasByDevnum_.erase(iter_d);<br /><br />cameras_.erase(iter);<br />+<br />+ /* Report the removal to the public signal */<br />+ CameraManager *const o = LIBCAMERA_O_PTR();<br />+ o->cameraRemoved.emit(camera);<br />}<br /><br />/**<br />@@ -382,51 +418,6 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)<br />* perform any blocking operation.<br />*/<br /><br />-/**<br />- * \brief Add a camera to the camera manager<br />- * \param[in] camera The camera to be added<br />- * \param[in] devnums The device numbers to associate with \a camera<br />- *<br />- * This function is called by pipeline handlers to register the cameras they<br />- * handle with the camera manager. Registered cameras are immediately made<br />- * available to the system.<br />- *<br />- * \a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes<br />- * to Camera instances.<br />- *<br />- * \context This function shall be called from the CameraManager thread.<br />- */<br />-void CameraManager::addCamera(std::shared_ptr<Camera> camera,<br />- const std::vector<dev_t> &devnums)<br />-{<br />- Private *const d = _d();<br />-<br />- ASSERT(Thread::current() == d);<br />-<br />- d->addCamera(camera, devnums);<br />- cameraAdded.emit(camera);<br />-}<br />-<br />-/**<br />- * \brief Remove a camera from the camera manager<br />- * \param[in] camera The camera to be removed<br />- *<br />- * This function is called by pipeline handlers to unregister cameras from the<br />- * camera manager. Unregistered cameras won't be reported anymore by the<br />- * cameras() and get() calls, but references may still exist in applications.<br />- *<br />- * \context This function shall be called from the CameraManager thread.<br />- */<br />-void CameraManager::removeCamera(std::shared_ptr<Camera> camera)<br />-{<br />- Private *const d = _d();<br />-<br />- ASSERT(Thread::current() == d);<br />-<br />- d->removeCamera(camera.get());<br />- cameraRemoved.emit(camera);<br />-}<br />-<br />/**<br />* \fn const std::string &CameraManager::version()<br />* \brief Retrieve the libcamera version string<br />diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp<br />index f72613b8e515..49092ea88a58 100644<br />--- a/src/libcamera/pipeline_handler.cpp<br />+++ b/src/libcamera/pipeline_handler.cpp<br />@@ -16,10 +16,10 @@<br />#include <libcamera/base/utils.h><br /><br />#include <libcamera/camera.h><br />-#include <libcamera/camera_manager.h><br />#include <libcamera/framebuffer.h><br /><br />#include "libcamera/internal/camera.h"<br />+#include "libcamera/internal/camera_manager.h"<br />#include "libcamera/internal/device_enumerator.h"<br />#include "libcamera/internal/framebuffer.h"<br />#include "libcamera/internal/media_device.h"<br />@@ -624,7 +624,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)<br />}<br />}<br /><br />- manager_->addCamera(std::move(camera), devnums);<br />+ manager_->_d()->addCamera(std::move(camera), devnums);<br />}<br /><br />/**<br />@@ -691,7 +691,7 @@ void PipelineHandler::disconnect()<br />continue;<br /><br />camera->disconnect();<br />- manager_->removeCamera(camera);<br />+ manager_->_d()->removeCamera(camera);<br />}<br />}<br /><br />--<br />2.34.1<br /> </blockquote><br /><br /> </html>