{"id":16342,"url":"https://patchwork.libcamera.org/api/patches/16342/?format=json","web_url":"https://patchwork.libcamera.org/patch/16342/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/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":"<20220623144736.78537-5-tomi.valkeinen@ideasonboard.com>","date":"2022-06-23T14:47:33","name":"[libcamera-devel,RFC,v1,4/7] py: Create PyCameraManager","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"259b80ccb10bcf913dfc0d8816660aab61ff6646","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/?format=json","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/16342/mbox/","series":[{"id":3213,"url":"https://patchwork.libcamera.org/api/series/3213/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3213","date":"2022-06-23T14:47:29","name":"Python bindings event handling","version":1,"mbox":"https://patchwork.libcamera.org/series/3213/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/16342/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/16342/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D6472BD808\n\tfor ;\n\tThu, 23 Jun 2022 14:48:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E9F865646;\n\tThu, 23 Jun 2022 16:48:05 +0200 (CEST)","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 7C3CC6048F\n\tfor ;\n\tThu, 23 Jun 2022 16:47:59 +0200 (CEST)","from deskari.lan (91-158-154-79.elisa-laajakaista.fi\n\t[91.158.154.79])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D57EC6BB;\n\tThu, 23 Jun 2022 16:47:58 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655995685;\n\tbh=udNMIcTVClz269HEeNat9aXGvxRVuCxQIK1mJk1zZVo=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=wl+RhzTtzm1Tidhgx+HLBYE4crPwE5+FWEHgm5NBmAL0x+oZY++D9m5jteGvPKz1L\n\tMOAnGk/jiCGunUxMwgnbpUHL8PnhGQ8cBPgtL35aS3r3vEokVsgeM1eukbpI833aP5\n\tUDNh3nFdUjnVcytv7kKhxAT0qrbW1D/2PG+XvAGxPC06l/CjDnLpfCQcNcTQ0Lb3q+\n\tKwuMv8ILRAYYTLKL0n6jDsU6D2l6deFOIIhvVIIPAaNxLo46llE8n5LwbuhYtjzS0D\n\tiix5Uwm9VKqYplHQRUXR5qxN9kk5weTl1JKIsvX7QSVofqGYyzppoV2xnFetl+z6Mm\n\tKh7vZqNbKgb6Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655995679;\n\tbh=udNMIcTVClz269HEeNat9aXGvxRVuCxQIK1mJk1zZVo=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=hlxPQvgzuC9qBYqWn+yFkHeHee4kS0SisJ31eKl/p9otN+CTMYgiCejKaZYua+TeF\n\tZw1dDWL/Ia7R82ee3jfkuMVHeJk5wmuGMloZVtg8+YsBakRVAPJjJccgghXlPvhK5G\n\tHjduWM6CYvOxFYXpQtjemgknRe/xt4fAORvzH4Lo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hlxPQvgz\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org,\n\tDavid Plowman ,\n\tKieran Bingham ,\n\tLaurent Pinchart ,\n\tJacopo Mondi ","Date":"Thu, 23 Jun 2022 17:47:33 +0300","Message-Id":"<20220623144736.78537-5-tomi.valkeinen@ideasonboard.com>","X-Mailer":"git-send-email 2.34.1","In-Reply-To":"<20220623144736.78537-1-tomi.valkeinen@ideasonboard.com>","References":"<20220623144736.78537-1-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [RFC v1 4/7] py: Create PyCameraManager","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","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Wrap the CameraManager with a PyCameraManager class and move the related\ncode inside the new class.\n\nThis helps understanding the life times of the used-to-be global\nvariables, gets rid of static handleRequestCompleted function, and\nallows us to simplify the binding code as the more complex pieces are\ninside the class.\n\nThere should be no user visible functional changes.\n\nNote that attaching to requestCompleted signal is a bit funny, as\napparently the only way to use a lambda with a signal is to provide an\nobject instance pointer as the first parameter, even if there's really\nno instance.\n\nSigned-off-by: Tomi Valkeinen \n---\n src/py/libcamera/meson.build | 1 +\n src/py/libcamera/py_camera_manager.cpp | 115 ++++++++++++++++++++++++\n src/py/libcamera/py_camera_manager.h | 39 ++++++++\n src/py/libcamera/py_main.cpp | 120 ++++++-------------------\n 4 files changed, 181 insertions(+), 94 deletions(-)\n create mode 100644 src/py/libcamera/py_camera_manager.cpp\n create mode 100644 src/py/libcamera/py_camera_manager.h","diff":"diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\nindex 04578bac..ad2a6858 100644\n--- a/src/py/libcamera/meson.build\n+++ b/src/py/libcamera/meson.build\n@@ -13,6 +13,7 @@ pybind11_proj = subproject('pybind11')\n pybind11_dep = pybind11_proj.get_variable('pybind11_dep')\n \n pycamera_sources = files([\n+ 'py_camera_manager.cpp',\n 'py_enums.cpp',\n 'py_geometry.cpp',\n 'py_helpers.cpp',\ndiff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\nnew file mode 100644\nindex 00000000..c9e5a99c\n--- /dev/null\n+++ b/src/py/libcamera/py_camera_manager.cpp\n@@ -0,0 +1,115 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2022, Tomi Valkeinen \n+ */\n+\n+#include \"py_camera_manager.h\"\n+\n+#include \n+#include \n+\n+#include \"py_main.h\"\n+\n+namespace py = pybind11;\n+\n+using namespace libcamera;\n+\n+PyCameraManager::PyCameraManager()\n+{\n+\tint fd = eventfd(0, 0);\n+\tif (fd == -1)\n+\t\tthrow std::system_error(errno, std::generic_category(),\n+\t\t\t\t\t\"Failed to create eventfd\");\n+\n+\teventFd_ = fd;\n+\n+\tint ret = start();\n+\tif (ret) {\n+\t\tclose(fd);\n+\t\teventFd_ = -1;\n+\t\tthrow std::system_error(-ret, std::generic_category(),\n+\t\t\t\t\t\"Failed to start CameraManager\");\n+\t}\n+}\n+\n+PyCameraManager::~PyCameraManager()\n+{\n+\tif (eventFd_ != -1) {\n+\t\tclose(eventFd_);\n+\t\teventFd_ = -1;\n+\t}\n+}\n+\n+py::list PyCameraManager::getCameras()\n+{\n+\t/* Create a list of Cameras, where each camera has a keep-alive to CameraManager */\n+\tpy::list l;\n+\n+\tfor (auto &c : cameras()) {\n+\t\tpy::object py_cm = py::cast(this);\n+\t\tpy::object py_cam = py::cast(c);\n+\t\tpy::detail::keep_alive_impl(py_cam, py_cm);\n+\t\tl.append(py_cam);\n+\t}\n+\n+\treturn l;\n+}\n+\n+std::vector PyCameraManager::getReadyRequests()\n+{\n+\treadFd();\n+\n+\tstd::vector v;\n+\tgetRequests(v);\n+\n+\tstd::vector ret;\n+\n+\tfor (Request *req : v) {\n+\t\tpy::object o = py::cast(req);\n+\t\t/* Decrease the ref increased in Camera.queue_request() */\n+\t\to.dec_ref();\n+\t\tret.push_back(o);\n+\t}\n+\n+\treturn ret;\n+}\n+\n+/* Note: Called from another thread */\n+void PyCameraManager::handleRequestCompleted(Request *req)\n+{\n+\tpushRequest(req);\n+\twriteFd();\n+}\n+\n+void PyCameraManager::writeFd()\n+{\n+\tuint64_t v = 1;\n+\n+\tsize_t s = write(eventFd_, &v, 8);\n+\t/*\n+\t * We should never fail, and have no simple means to manage the error,\n+\t * so let's use LOG(Python, Fatal).\n+ */\n+\tif (s != 8)\n+\t\tLOG(Python, Fatal) << \"Unable to write to eventfd\";\n+}\n+\n+void PyCameraManager::readFd()\n+{\n+\tuint8_t buf[8];\n+\n+\tif (read(eventFd_, buf, 8) != 8)\n+\t\tthrow std::system_error(errno, std::generic_category());\n+}\n+\n+void PyCameraManager::pushRequest(Request *req)\n+{\n+\tstd::lock_guard guard(reqlist_mutex_);\n+\treqList_.push_back(req);\n+}\n+\n+void PyCameraManager::getRequests(std::vector &v)\n+{\n+\tstd::lock_guard guard(reqlist_mutex_);\n+\tswap(v, reqList_);\n+}\ndiff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h\nnew file mode 100644\nindex 00000000..b0b971ad\n--- /dev/null\n+++ b/src/py/libcamera/py_camera_manager.h\n@@ -0,0 +1,39 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2022, Tomi Valkeinen \n+ */\n+\n+#pragma once\n+\n+#include \n+\n+#include \n+\n+#include \n+\n+using namespace libcamera;\n+\n+class PyCameraManager : public CameraManager\n+{\n+public:\n+\tPyCameraManager();\n+\tvirtual ~PyCameraManager();\n+\n+\tpybind11::list getCameras();\n+\n+\tint eventFd() const { return eventFd_; }\n+\n+\tstd::vector getReadyRequests();\n+\n+\tvoid handleRequestCompleted(Request *req);\n+\n+private:\n+\tint eventFd_ = -1;\n+\tstd::mutex reqlist_mutex_;\n+\tstd::vector reqList_;\n+\n+\tvoid writeFd();\n+\tvoid readFd();\n+\tvoid pushRequest(Request *req);\n+\tvoid getRequests(std::vector &v);\n+};\ndiff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\nindex 5a423ece..23018288 100644\n--- a/src/py/libcamera/py_main.cpp\n+++ b/src/py/libcamera/py_main.cpp\n@@ -7,10 +7,7 @@\n \n #include \"py_main.h\"\n \n-#include \n #include \n-#include \n-#include \n \n #include \n \n@@ -21,6 +18,7 @@\n #include \n #include \n \n+#include \"py_camera_manager.h\"\n #include \"py_helpers.h\"\n \n namespace py = pybind11;\n@@ -29,27 +27,11 @@ using namespace libcamera;\n \n LOG_DEFINE_CATEGORY(Python)\n \n-static std::weak_ptr gCameraManager;\n-static int gEventfd;\n-static std::mutex gReqlistMutex;\n-static std::vector gReqList;\n-\n-static void handleRequestCompleted(Request *req)\n-{\n-\t{\n-\t\tstd::lock_guard guard(gReqlistMutex);\n-\t\tgReqList.push_back(req);\n-\t}\n-\n-\tuint64_t v = 1;\n-\tsize_t s = write(gEventfd, &v, 8);\n-\t/*\n-\t * We should never fail, and have no simple means to manage the error,\n-\t * so let's use LOG(Python, Fatal).\n-\t */\n-\tif (s != 8)\n-\t\tLOG(Python, Fatal) << \"Unable to write to eventfd\";\n-}\n+/*\n+ * XXX Note: global C++ destructors can be ran on this before the py module is\n+ * destructed.\n+ */\n+static std::weak_ptr gCameraManager;\n \n void init_py_enums(py::module &m);\n void init_py_controls_generated(py::module &m);\n@@ -72,7 +54,7 @@ PYBIND11_MODULE(_libcamera, m)\n \t * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n \t */\n \n-\tauto pyCameraManager = py::class_(m, \"CameraManager\");\n+\tauto pyCameraManager = py::class_(m, \"CameraManager\");\n \tauto pyCamera = py::class_(m, \"Camera\");\n \tauto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n \tauto pyCameraConfigurationStatus = py::enum_(pyCameraConfiguration, \"Status\");\n@@ -106,78 +88,22 @@ PYBIND11_MODULE(_libcamera, m)\n \t/* Classes */\n \tpyCameraManager\n \t\t.def_static(\"singleton\", []() {\n-\t\t\tstd::shared_ptr cm = gCameraManager.lock();\n-\t\t\tif (cm)\n-\t\t\t\treturn cm;\n-\n-\t\t\tint fd = eventfd(0, 0);\n-\t\t\tif (fd == -1)\n-\t\t\t\tthrow std::system_error(errno, std::generic_category(),\n-\t\t\t\t\t\t\t\"Failed to create eventfd\");\n-\n-\t\t\tcm = std::shared_ptr(new CameraManager, [](auto p) {\n-\t\t\t\tclose(gEventfd);\n-\t\t\t\tgEventfd = -1;\n-\t\t\t\tdelete p;\n-\t\t\t});\n-\n-\t\t\tgEventfd = fd;\n-\t\t\tgCameraManager = cm;\n-\n-\t\t\tint ret = cm->start();\n-\t\t\tif (ret)\n-\t\t\t\tthrow std::system_error(-ret, std::generic_category(),\n-\t\t\t\t\t\t\t\"Failed to start CameraManager\");\n-\n-\t\t\treturn cm;\n-\t\t})\n-\n-\t\t.def_property_readonly(\"version\", &CameraManager::version)\n-\n-\t\t.def_property_readonly(\"event_fd\", [](CameraManager &) {\n-\t\t\treturn gEventfd;\n-\t\t})\n+\t\t\tstd::shared_ptr cm = gCameraManager.lock();\n \n-\t\t.def(\"get_ready_requests\", [](CameraManager &) {\n-\t\t\tuint8_t buf[8];\n-\n-\t\t\tif (read(gEventfd, buf, 8) != 8)\n-\t\t\t\tthrow std::system_error(errno, std::generic_category());\n-\n-\t\t\tstd::vector v;\n-\n-\t\t\t{\n-\t\t\t\tstd::lock_guard guard(gReqlistMutex);\n-\t\t\t\tswap(v, gReqList);\n+\t\t\tif (!cm) {\n+\t\t\t\tcm = std::make_shared();\n+\t\t\t\tgCameraManager = cm;\n \t\t\t}\n \n-\t\t\tstd::vector ret;\n-\n-\t\t\tfor (Request *req : v) {\n-\t\t\t\tpy::object o = py::cast(req);\n-\t\t\t\t/* Decrease the ref increased in Camera.queue_request() */\n-\t\t\t\to.dec_ref();\n-\t\t\t\tret.push_back(o);\n-\t\t\t}\n-\n-\t\t\treturn ret;\n+\t\t\treturn cm;\n \t\t})\n \n-\t\t.def(\"get\", py::overload_cast(&CameraManager::get), py::keep_alive<0, 1>())\n+\t\t.def_property_readonly(\"version\", &PyCameraManager::version)\n+\t\t.def(\"get\", py::overload_cast(&PyCameraManager::get), py::keep_alive<0, 1>())\n+\t\t.def_property_readonly(\"cameras\", &PyCameraManager::getCameras)\n \n-\t\t/* Create a list of Cameras, where each camera has a keep-alive to CameraManager */\n-\t\t.def_property_readonly(\"cameras\", [](CameraManager &self) {\n-\t\t\tpy::list l;\n-\n-\t\t\tfor (auto &c : self.cameras()) {\n-\t\t\t\tpy::object py_cm = py::cast(self);\n-\t\t\t\tpy::object py_cam = py::cast(c);\n-\t\t\t\tpy::detail::keep_alive_impl(py_cam, py_cm);\n-\t\t\t\tl.append(py_cam);\n-\t\t\t}\n-\n-\t\t\treturn l;\n-\t\t});\n+\t\t.def_property_readonly(\"event_fd\", &PyCameraManager::eventFd)\n+\t\t.def(\"get_ready_requests\", &PyCameraManager::getReadyRequests);\n \n \tpyCamera\n \t\t.def_property_readonly(\"id\", &Camera::id)\n@@ -187,7 +113,13 @@ PYBIND11_MODULE(_libcamera, m)\n \t\t const std::unordered_map &controls) {\n \t\t\t/* \\todo What happens if someone calls start() multiple times? */\n \n-\t\t\tself.requestCompleted.connect(handleRequestCompleted);\n+\t\t\tauto cm = gCameraManager.lock();\n+\t\t\tassert(cm);\n+\n+\t\t\tself.requestCompleted.connect(&self, [cm=std::move(cm)](Request *req) {\n+\n+\t\t\t\tcm->handleRequestCompleted(req);\n+\t\t\t});\n \n \t\t\tControlList controlList(self.controls());\n \n@@ -198,7 +130,7 @@ PYBIND11_MODULE(_libcamera, m)\n \n \t\t\tint ret = self.start(&controlList);\n \t\t\tif (ret) {\n-\t\t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n+\t\t\t\tself.requestCompleted.disconnect();\n \t\t\t\treturn ret;\n \t\t\t}\n \n@@ -210,7 +142,7 @@ PYBIND11_MODULE(_libcamera, m)\n \t\t\tif (ret)\n \t\t\t\treturn ret;\n \n-\t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n+\t\t\tself.requestCompleted.disconnect();\n \n \t\t\treturn 0;\n \t\t})\n","prefixes":["libcamera-devel","RFC","v1","4/7"]}