{"id":2700,"url":"https://patchwork.libcamera.org/api/1.1/patches/2700/?format=json","web_url":"https://patchwork.libcamera.org/patch/2700/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200120002437.6633-14-laurent.pinchart@ideasonboard.com>","date":"2020-01-20T00:24:31","name":"[libcamera-devel,13/19] libcamera: camera_manager: Run the camera manager in a thread","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"1e0b5bc860592065339e1bfcd425f1992bb083df","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/2700/mbox/","series":[{"id":641,"url":"https://patchwork.libcamera.org/api/1.1/series/641/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=641","date":"2020-01-20T00:24:19","name":"Initial libcamera threading model","version":1,"mbox":"https://patchwork.libcamera.org/series/641/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/2700/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/2700/checks/","tags":{},"headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 49D9F60454\n\tfor ;\n\tMon, 20 Jan 2020 01:24:47 +0100 (CET)","from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DEB76529\n\tfor ;\n\tMon, 20 Jan 2020 01:24:46 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579479887;\n\tbh=cE/6Q8cXp+khS6Q2njjhgxZTloE72CLePidnvjviz6c=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=rCEMeOKRnqRptrksxyHwepFfl+anwe9m6QFNuOMNSVsXQm+7QhKkjiAE1SDTrUl1+\n\tjuKZaBDTgMI42lJfodwnExI8+8qqoXp24iBnZavJFyQhR+VL6cnL7dFsRO0kO3HxD/\n\twdVSmZV4KwnJ6ZzwaptVEeDB6t6sRsqV3EHBIPVQ=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 20 Jan 2020 02:24:31 +0200","Message-Id":"<20200120002437.6633-14-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.24.1","In-Reply-To":"<20200120002437.6633-1-laurent.pinchart@ideasonboard.com>","References":"<20200120002437.6633-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 13/19] libcamera: camera_manager: Run the\n\tcamera manager in a thread","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 20 Jan 2020 00:24:47 -0000"},"content":"Relying on the application event loop to process all our internal events\nis a bad idea for multiple reasons. In many cases the user of libcamera\ncan't provide an event loop, for instance when running through one of\nthe adaptation layers. The Android camera HAL and V4L2 compatibility\nlayer create a thread for this reason, and the GStreamer element would\nneed to do so as well. Furthermore, relying on the application event\nloop pushes libcamera's realtime constraints to the application, which\nisn't manageable.\n\nFor these reasons it's desirable to always run the camera manager, the\npipeline handlers and the cameras in a separate thread. Doing so isn't\ntoo complicated, it only involves creating the thread internally when\nstarting the camera manager, and synchronizing a few methods of the\nCamera class. Do so as a first step towards defining the threading model\nof libcamera.\n\nMaking CameraManager::cameras() thread-safe requires returning a copy of\nthe cameras vector instead of a reference. This is also required for\nhot-plugging support and is thus desirable.\n\nThe event dispatcher interface is still exposed to applications, to\nenable cross-thread signal delivery if desired.\n\nSigned-off-by: Laurent Pinchart \n---\n Documentation/Doxyfile.in | 1 +\n include/libcamera/camera_manager.h | 13 +-\n src/libcamera/camera_manager.cpp | 295 +++++++++++++++++++++--------\n 3 files changed, 224 insertions(+), 85 deletions(-)","diff":"diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\nindex 734672ed15dc..1c46b04b3f7e 100644\n--- a/Documentation/Doxyfile.in\n+++ b/Documentation/Doxyfile.in\n@@ -879,6 +879,7 @@ EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \\\n libcamera::BoundMethodPackBase \\\n libcamera::BoundMethodStatic \\\n libcamera::SignalBase \\\n+ libcamera::*::Private \\\n std::*\n \n # The EXAMPLE_PATH tag can be used to specify one or more files or directories\ndiff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\nindex 094197668698..079f848aec79 100644\n--- a/include/libcamera/camera_manager.h\n+++ b/include/libcamera/camera_manager.h\n@@ -7,7 +7,6 @@\n #ifndef __LIBCAMERA_CAMERA_MANAGER_H__\n #define __LIBCAMERA_CAMERA_MANAGER_H__\n \n-#include \n #include \n #include \n #include \n@@ -18,9 +17,7 @@\n namespace libcamera {\n \n class Camera;\n-class DeviceEnumerator;\n class EventDispatcher;\n-class PipelineHandler;\n \n class CameraManager : public Object\n {\n@@ -33,7 +30,7 @@ public:\n \tint start();\n \tvoid stop();\n \n-\tconst std::vector> &cameras() const { return cameras_; }\n+\tstd::vector> cameras() const;\n \tstd::shared_ptr get(const std::string &name);\n \tstd::shared_ptr get(dev_t devnum);\n \n@@ -46,13 +43,11 @@ public:\n \tEventDispatcher *eventDispatcher();\n \n private:\n-\tstd::unique_ptr enumerator_;\n-\tstd::vector> pipes_;\n-\tstd::vector> cameras_;\n-\tstd::map> camerasByDevnum_;\n-\n \tstatic const std::string version_;\n \tstatic CameraManager *self_;\n+\n+\tclass Private;\n+\tstd::unique_ptr p_;\n };\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex a71caf8536bb..64aa89975701 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -7,6 +7,9 @@\n \n #include \n \n+#include \n+#include \n+\n #include \n #include \n \n@@ -26,6 +29,184 @@ namespace libcamera {\n \n LOG_DEFINE_CATEGORY(Camera)\n \n+class CameraManager::Private : public Thread\n+{\n+public:\n+\tPrivate(CameraManager *cm);\n+\n+\tint start();\n+\tvoid addCamera(std::shared_ptr &camera, dev_t devnum);\n+\tvoid removeCamera(Camera *camera);\n+\n+\t/*\n+\t * This mutex protects\n+\t *\n+\t * - initialized_ and status_ during initialization\n+\t * - cameras_ and camerasByDevnum_ after initialization\n+\t */\n+\tMutex mutex_;\n+\tstd::vector> cameras_;\n+\tstd::map> camerasByDevnum_;\n+\n+protected:\n+\tvoid run() override;\n+\n+private:\n+\tint init();\n+\tvoid cleanup();\n+\n+\tCameraManager *cm_;\n+\n+\tstd::condition_variable cv_;\n+\tbool initialized_;\n+\tint status_;\n+\n+\tstd::vector> pipes_;\n+\tstd::unique_ptr enumerator_;\n+};\n+\n+CameraManager::Private::Private(CameraManager *cm)\n+\t: cm_(cm), initialized_(false)\n+{\n+}\n+\n+int CameraManager::Private::start()\n+{\n+\t/* Start the thread and wait for initialization to complete. */\n+\tThread::start();\n+\n+\t{\n+\t\tMutexLocker locker(mutex_);\n+\t\tcv_.wait(locker, [&] { return initialized_; });\n+\t}\n+\n+\t/* If a failure happened during initialization, stop the thread. */\n+\tif (status_ < 0) {\n+\t\texit();\n+\t\twait();\n+\t\treturn status_;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+void CameraManager::Private::addCamera(std::shared_ptr &camera,\n+\t\t\t\t dev_t devnum)\n+{\n+\tMutexLocker locker(mutex_);\n+\n+\tfor (std::shared_ptr c : cameras_) {\n+\t\tif (c->name() == camera->name()) {\n+\t\t\tLOG(Camera, Warning)\n+\t\t\t\t<< \"Registering camera with duplicate name '\"\n+\t\t\t\t<< camera->name() << \"'\";\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tcameras_.push_back(std::move(camera));\n+\n+\tif (devnum) {\n+\t\tunsigned int index = cameras_.size() - 1;\n+\t\tcamerasByDevnum_[devnum] = cameras_[index];\n+\t}\n+}\n+\n+void CameraManager::Private::removeCamera(Camera *camera)\n+{\n+\tMutexLocker locker(mutex_);\n+\n+\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n+\t\t\t\t [camera](std::shared_ptr &c) {\n+\t\t\t\t\t return c.get() == camera;\n+\t\t\t\t });\n+\tif (iter == cameras_.end())\n+\t\treturn;\n+\n+\tLOG(Camera, Debug)\n+\t\t<< \"Unregistering camera '\" << camera->name() << \"'\";\n+\n+\tauto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),\n+\t\t\t\t [camera](const std::pair> &p) {\n+\t\t\t\t\t return p.second.lock().get() == camera;\n+\t\t\t\t });\n+\tif (iter_d != camerasByDevnum_.end())\n+\t\tcamerasByDevnum_.erase(iter_d);\n+\n+\tcameras_.erase(iter);\n+}\n+\n+void CameraManager::Private::run()\n+{\n+\tLOG(Camera, Debug) << \"Starting camera manager\";\n+\n+\tint ret = init();\n+\n+\tmutex_.lock();\n+\tstatus_ = ret;\n+\tinitialized_ = true;\n+\tmutex_.unlock();\n+\tcv_.notify_one();\n+\n+\tif (ret < 0)\n+\t\treturn;\n+\n+\t/* Now start processing events and messages. */\n+\texec();\n+\n+\tcleanup();\n+}\n+\n+int CameraManager::Private::init()\n+{\n+\tenumerator_ = DeviceEnumerator::create();\n+\tif (!enumerator_ || enumerator_->enumerate())\n+\t\treturn -ENODEV;\n+\n+\t/*\n+\t * TODO: Try to read handlers and order from configuration\n+\t * file and only fallback on all handlers if there is no\n+\t * configuration file.\n+\t */\n+\tstd::vector &factories = PipelineHandlerFactory::factories();\n+\n+\tfor (PipelineHandlerFactory *factory : factories) {\n+\t\t/*\n+\t\t * Try each pipeline handler until it exhaust\n+\t\t * all pipelines it can provide.\n+\t\t */\n+\t\twhile (1) {\n+\t\t\tstd::shared_ptr pipe = factory->create(cm_);\n+\t\t\tif (!pipe->match(enumerator_.get()))\n+\t\t\t\tbreak;\n+\n+\t\t\tLOG(Camera, Debug)\n+\t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n+\t\t\t\t<< \"\\\" matched\";\n+\t\t\tpipes_.push_back(std::move(pipe));\n+\t\t}\n+\t}\n+\n+\t/* TODO: register hot-plug callback here */\n+\n+\treturn 0;\n+}\n+\n+void CameraManager::Private::cleanup()\n+{\n+\t/* TODO: unregister hot-plug callback here */\n+\n+\t/*\n+\t * Release all references to cameras and pipeline handlers to ensure\n+\t * they all get destroyed before the device enumerator deletes the\n+\t * media devices.\n+\t */\n+\tpipes_.clear();\n+\tcameras_.clear();\n+\n+\tenumerator_.reset(nullptr);\n+}\n+\n /**\n * \\class CameraManager\n * \\brief Provide access and manage all cameras in the system\n@@ -62,7 +243,7 @@ LOG_DEFINE_CATEGORY(Camera)\n CameraManager *CameraManager::self_ = nullptr;\n \n CameraManager::CameraManager()\n-\t: enumerator_(nullptr)\n+\t: p_(new CameraManager::Private(this))\n {\n \tif (self_)\n \t\tLOG(Camera, Fatal)\n@@ -73,6 +254,8 @@ CameraManager::CameraManager()\n \n CameraManager::~CameraManager()\n {\n+\tstop();\n+\n \tself_ = nullptr;\n }\n \n@@ -88,41 +271,16 @@ CameraManager::~CameraManager()\n */\n int CameraManager::start()\n {\n-\tif (enumerator_)\n-\t\treturn -EBUSY;\n-\n \tLOG(Camera, Info) << \"libcamera \" << version_;\n \n-\tenumerator_ = DeviceEnumerator::create();\n-\tif (!enumerator_ || enumerator_->enumerate())\n-\t\treturn -ENODEV;\n-\n-\t/*\n-\t * TODO: Try to read handlers and order from configuration\n-\t * file and only fallback on all handlers if there is no\n-\t * configuration file.\n-\t */\n-\tstd::vector &factories = PipelineHandlerFactory::factories();\n+\tint ret = p_->start();\n+\tif (ret < 0) {\n+\t\tLOG(Camera, Error) << \"Failed to start camera manager: \"\n+\t\t\t\t << strerror(-ret);\n \n-\tfor (PipelineHandlerFactory *factory : factories) {\n-\t\t/*\n-\t\t * Try each pipeline handler until it exhaust\n-\t\t * all pipelines it can provide.\n-\t\t */\n-\t\twhile (1) {\n-\t\t\tstd::shared_ptr pipe = factory->create(this);\n-\t\t\tif (!pipe->match(enumerator_.get()))\n-\t\t\t\tbreak;\n-\n-\t\t\tLOG(Camera, Debug)\n-\t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n-\t\t\t\t<< \"\\\" matched\";\n-\t\t\tpipes_.push_back(std::move(pipe));\n-\t\t}\n+\t\treturn ret;\n \t}\n \n-\t/* TODO: register hot-plug callback here */\n-\n \treturn 0;\n }\n \n@@ -138,17 +296,8 @@ int CameraManager::start()\n */\n void CameraManager::stop()\n {\n-\t/* TODO: unregister hot-plug callback here */\n-\n-\t/*\n-\t * Release all references to cameras and pipeline handlers to ensure\n-\t * they all get destroyed before the device enumerator deletes the\n-\t * media devices.\n-\t */\n-\tpipes_.clear();\n-\tcameras_.clear();\n-\n-\tenumerator_.reset(nullptr);\n+\tp_->exit();\n+\tp_->wait();\n }\n \n /**\n@@ -158,8 +307,16 @@ void CameraManager::stop()\n * Before calling this function the caller is responsible for ensuring that\n * the camera manager is running.\n *\n+ * \\context This function is \\threadsafe.\n+ *\n * \\return List of all available cameras\n */\n+std::vector> CameraManager::cameras() const\n+{\n+\tMutexLocker locker(p_->mutex_);\n+\n+\treturn p_->cameras_;\n+}\n \n /**\n * \\brief Get a camera based on name\n@@ -168,11 +325,15 @@ void CameraManager::stop()\n * Before calling this function the caller is responsible for ensuring that\n * the camera manager is running.\n *\n+ * \\context This function is \\threadsafe.\n+ *\n * \\return Shared pointer to Camera object or nullptr if camera not found\n */\n std::shared_ptr CameraManager::get(const std::string &name)\n {\n-\tfor (std::shared_ptr camera : cameras_) {\n+\tMutexLocker locker(p_->mutex_);\n+\n+\tfor (std::shared_ptr camera : p_->cameras_) {\n \t\tif (camera->name() == name)\n \t\t\treturn camera;\n \t}\n@@ -191,13 +352,17 @@ std::shared_ptr CameraManager::get(const std::string &name)\n * Before calling this function the caller is responsible for ensuring that\n * the camera manager is running.\n *\n+ * \\context This function is \\threadsafe.\n+ *\n * \\return Shared pointer to Camera object, which is empty if the camera is\n * not found\n */\n std::shared_ptr CameraManager::get(dev_t devnum)\n {\n-\tauto iter = camerasByDevnum_.find(devnum);\n-\tif (iter == camerasByDevnum_.end())\n+\tMutexLocker locker(p_->mutex_);\n+\n+\tauto iter = p_->camerasByDevnum_.find(devnum);\n+\tif (iter == p_->camerasByDevnum_.end())\n \t\treturn nullptr;\n \n \treturn iter->second.lock();\n@@ -214,24 +379,14 @@ std::shared_ptr CameraManager::get(dev_t devnum)\n *\n * \\a devnum is used by the V4L2 compatibility layer to map V4L2 device nodes\n * to Camera instances.\n+ *\n+ * \\context This function shall be called from the CameraManager thread.\n */\n void CameraManager::addCamera(std::shared_ptr camera, dev_t devnum)\n {\n-\tfor (std::shared_ptr c : cameras_) {\n-\t\tif (c->name() == camera->name()) {\n-\t\t\tLOG(Camera, Warning)\n-\t\t\t\t<< \"Registering camera with duplicate name '\"\n-\t\t\t\t<< camera->name() << \"'\";\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n-\tcameras_.push_back(std::move(camera));\n+\tASSERT(Thread::current() == p_.get());\n \n-\tif (devnum) {\n-\t\tunsigned int index = cameras_.size() - 1;\n-\t\tcamerasByDevnum_[devnum] = cameras_[index];\n-\t}\n+\tp_->addCamera(camera, devnum);\n }\n \n /**\n@@ -241,32 +396,20 @@ void CameraManager::addCamera(std::shared_ptr camera, dev_t devnum)\n * This function is called by pipeline handlers to unregister cameras from the\n * camera manager. Unregistered cameras won't be reported anymore by the\n * cameras() and get() calls, but references may still exist in applications.\n+ *\n+ * \\context This function shall be called from the CameraManager thread.\n */\n void CameraManager::removeCamera(Camera *camera)\n {\n-\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n-\t\t\t\t [camera](std::shared_ptr &c) {\n-\t\t\t\t\t return c.get() == camera;\n-\t\t\t\t });\n-\tif (iter == cameras_.end())\n-\t\treturn;\n-\n-\tLOG(Camera, Debug)\n-\t\t<< \"Unregistering camera '\" << camera->name() << \"'\";\n-\n-\tauto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),\n-\t\t\t\t [camera](const std::pair> &p) {\n-\t\t\t\t\t return p.second.lock().get() == camera;\n-\t\t\t\t });\n-\tif (iter_d != camerasByDevnum_.end())\n-\t\tcamerasByDevnum_.erase(iter_d);\n+\tASSERT(Thread::current() == p_.get());\n \n-\tcameras_.erase(iter);\n+\tp_->removeCamera(camera);\n }\n \n /**\n * \\fn const std::string &CameraManager::version()\n * \\brief Retrieve the libcamera version string\n+ * \\context This function is \\a threadsafe.\n * \\return The libcamera version string\n */\n \n","prefixes":["libcamera-devel","13/19"]}