Patch Detail
Show a patch.
GET /api/1.1/patches/2725/?format=api
{ "id": 2725, "url": "https://patchwork.libcamera.org/api/1.1/patches/2725/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2725/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200122205723.8865-10-laurent.pinchart@ideasonboard.com>", "date": "2020-01-22T20:57:19", "name": "[libcamera-devel,v2,09/13] libcamera: camera_manager: Run the camera manager in a thread", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "6be4260e9ec61a7ca5407f77d7866f5488e95ea9", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2725/mbox/", "series": [ { "id": 645, "url": "https://patchwork.libcamera.org/api/1.1/series/645/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=645", "date": "2020-01-22T20:57:10", "name": "Initial libcamera threading model", "version": 2, "mbox": "https://patchwork.libcamera.org/series/645/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2725/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2725/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "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 BB9DF60883\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jan 2020 21:57:45 +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 6535D2F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jan 2020 21:57:45 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579726665;\n\tbh=8M1B515kSSEKe77KbFMO71pqYBDFHYuOtjx8kJJarMg=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=rbcAhGtL01Uy1x2z5q2m/O8iOCQ00ceJhLitPK2q3r/1jcF+TECuKT8n+3i0+rrc8\n\t3Tk8dcwGyL+4UaiV85UnamomubMEHFRIvmsuWB4IMgi+N4xw+E+uOLlz5LbtOG37J6\n\tjY5ZG0gfW6CZScyuo3c6XujKdGffy05r2qW05Qrg=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 22 Jan 2020 22:57:19 +0200", "Message-Id": "<20200122205723.8865-10-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200122205723.8865-1-laurent.pinchart@ideasonboard.com>", "References": "<20200122205723.8865-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 09/13] libcamera: camera_manager: Run\n\tthe camera manager in a thread", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Wed, 22 Jan 2020 20:57:48 -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\nThe event dispatcher interface is still exposed to applications, to\nenable cross-thread signal delivery if desired.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nChanges since v1:\n\n- Move \"libcamera: camera_manager: Move private data members to private\n implementation\" and \"libcamera: camera_manager: Return a copy of the\n vector from cameras()\" to separate patches\n---\n src/libcamera/camera_manager.cpp | 94 ++++++++++++++++++++++++++++++--\n 1 file changed, 88 insertions(+), 6 deletions(-)", "diff": "diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 5fc1bba974c6..11c333ccb4e0 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -7,6 +7,7 @@\n \n #include <libcamera/camera_manager.h>\n \n+#include <condition_variable>\n #include <map>\n \n #include <libcamera/camera.h>\n@@ -28,33 +29,89 @@ namespace libcamera {\n \n LOG_DEFINE_CATEGORY(Camera)\n \n-class CameraManager::Private\n+class CameraManager::Private : public Thread\n {\n public:\n \tPrivate(CameraManager *cm);\n \n \tint start();\n-\tvoid stop();\n-\n \tvoid addCamera(std::shared_ptr<Camera> &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<std::shared_ptr<Camera>> cameras_;\n \tstd::map<dev_t, std::weak_ptr<Camera>> 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<std::shared_ptr<PipelineHandler>> pipes_;\n \tstd::unique_ptr<DeviceEnumerator> enumerator_;\n };\n \n CameraManager::Private::Private(CameraManager *cm)\n-\t: cm_(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::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@@ -89,7 +146,7 @@ int CameraManager::Private::start()\n \treturn 0;\n }\n \n-void CameraManager::Private::stop()\n+void CameraManager::Private::cleanup()\n {\n \t/* TODO: unregister hot-plug callback here */\n \n@@ -107,6 +164,8 @@ void CameraManager::Private::stop()\n void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,\n \t\t\t\t dev_t devnum)\n {\n+\tMutexLocker locker(mutex_);\n+\n \tfor (std::shared_ptr<Camera> c : cameras_) {\n \t\tif (c->name() == camera->name()) {\n \t\t\tLOG(Camera, Warning)\n@@ -126,6 +185,8 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,\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<Camera> &c) {\n \t\t\t\t\t return c.get() == camera;\n@@ -232,7 +293,8 @@ int CameraManager::start()\n */\n void CameraManager::stop()\n {\n-\tp_->stop();\n+\tp_->exit();\n+\tp_->wait();\n }\n \n /**\n@@ -242,10 +304,14 @@ 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<std::shared_ptr<Camera>> CameraManager::cameras() const\n {\n+\tMutexLocker locker(p_->mutex_);\n+\n \treturn p_->cameras_;\n }\n \n@@ -256,10 +322,14 @@ std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const\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<Camera> CameraManager::get(const std::string &name)\n {\n+\tMutexLocker locker(p_->mutex_);\n+\n \tfor (std::shared_ptr<Camera> camera : p_->cameras_) {\n \t\tif (camera->name() == name)\n \t\t\treturn camera;\n@@ -279,11 +349,15 @@ std::shared_ptr<Camera> 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<Camera> CameraManager::get(dev_t devnum)\n {\n+\tMutexLocker locker(p_->mutex_);\n+\n \tauto iter = p_->camerasByDevnum_.find(devnum);\n \tif (iter == p_->camerasByDevnum_.end())\n \t\treturn nullptr;\n@@ -302,9 +376,12 @@ std::shared_ptr<Camera> 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> camera, dev_t devnum)\n {\n+\tASSERT(Thread::current() == p_.get());\n \n \tp_->addCamera(camera, devnum);\n }\n@@ -316,15 +393,20 @@ void CameraManager::addCamera(std::shared_ptr<Camera> 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+\tASSERT(Thread::current() == p_.get());\n+\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", "v2", "09/13" ] }