[libcamera-devel] [RFC v1 4/7] py: Create PyCameraManager

Tomi Valkeinen tomi.valkeinen at ideasonboard.com
Thu Jun 23 16:47:33 CEST 2022


Wrap the CameraManager with a PyCameraManager class and move the related
code inside the new class.

This helps understanding the life times of the used-to-be global
variables, gets rid of static handleRequestCompleted function, and
allows us to simplify the binding code as the more complex pieces are
inside the class.

There should be no user visible functional changes.

Note that attaching to requestCompleted signal is a bit funny, as
apparently the only way to use a lambda with a signal is to provide an
object instance pointer as the first parameter, even if there's really
no instance.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
---
 src/py/libcamera/meson.build           |   1 +
 src/py/libcamera/py_camera_manager.cpp | 115 ++++++++++++++++++++++++
 src/py/libcamera/py_camera_manager.h   |  39 ++++++++
 src/py/libcamera/py_main.cpp           | 120 ++++++-------------------
 4 files changed, 181 insertions(+), 94 deletions(-)
 create mode 100644 src/py/libcamera/py_camera_manager.cpp
 create mode 100644 src/py/libcamera/py_camera_manager.h

diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
index 04578bac..ad2a6858 100644
--- a/src/py/libcamera/meson.build
+++ b/src/py/libcamera/meson.build
@@ -13,6 +13,7 @@ pybind11_proj = subproject('pybind11')
 pybind11_dep = pybind11_proj.get_variable('pybind11_dep')
 
 pycamera_sources = files([
+    'py_camera_manager.cpp',
     'py_enums.cpp',
     'py_geometry.cpp',
     'py_helpers.cpp',
diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
new file mode 100644
index 00000000..c9e5a99c
--- /dev/null
+++ b/src/py/libcamera/py_camera_manager.cpp
@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
+ */
+
+#include "py_camera_manager.h"
+
+#include <sys/eventfd.h>
+#include <unistd.h>
+
+#include "py_main.h"
+
+namespace py = pybind11;
+
+using namespace libcamera;
+
+PyCameraManager::PyCameraManager()
+{
+	int fd = eventfd(0, 0);
+	if (fd == -1)
+		throw std::system_error(errno, std::generic_category(),
+					"Failed to create eventfd");
+
+	eventFd_ = fd;
+
+	int ret = start();
+	if (ret) {
+		close(fd);
+		eventFd_ = -1;
+		throw std::system_error(-ret, std::generic_category(),
+					"Failed to start CameraManager");
+	}
+}
+
+PyCameraManager::~PyCameraManager()
+{
+	if (eventFd_ != -1) {
+		close(eventFd_);
+		eventFd_ = -1;
+	}
+}
+
+py::list PyCameraManager::getCameras()
+{
+	/* Create a list of Cameras, where each camera has a keep-alive to CameraManager */
+	py::list l;
+
+	for (auto &c : cameras()) {
+		py::object py_cm = py::cast(this);
+		py::object py_cam = py::cast(c);
+		py::detail::keep_alive_impl(py_cam, py_cm);
+		l.append(py_cam);
+	}
+
+	return l;
+}
+
+std::vector<py::object> PyCameraManager::getReadyRequests()
+{
+	readFd();
+
+	std::vector<Request *> v;
+	getRequests(v);
+
+	std::vector<py::object> ret;
+
+	for (Request *req : v) {
+		py::object o = py::cast(req);
+		/* Decrease the ref increased in Camera.queue_request() */
+		o.dec_ref();
+		ret.push_back(o);
+	}
+
+	return ret;
+}
+
+/* Note: Called from another thread */
+void PyCameraManager::handleRequestCompleted(Request *req)
+{
+	pushRequest(req);
+	writeFd();
+}
+
+void PyCameraManager::writeFd()
+{
+	uint64_t v = 1;
+
+	size_t s = write(eventFd_, &v, 8);
+	/*
+	 * We should never fail, and have no simple means to manage the error,
+	 * so let's use LOG(Python, Fatal).
+         */
+	if (s != 8)
+		LOG(Python, Fatal) << "Unable to write to eventfd";
+}
+
+void PyCameraManager::readFd()
+{
+	uint8_t buf[8];
+
+	if (read(eventFd_, buf, 8) != 8)
+		throw std::system_error(errno, std::generic_category());
+}
+
+void PyCameraManager::pushRequest(Request *req)
+{
+	std::lock_guard guard(reqlist_mutex_);
+	reqList_.push_back(req);
+}
+
+void PyCameraManager::getRequests(std::vector<Request *> &v)
+{
+	std::lock_guard guard(reqlist_mutex_);
+	swap(v, reqList_);
+}
diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h
new file mode 100644
index 00000000..b0b971ad
--- /dev/null
+++ b/src/py/libcamera/py_camera_manager.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
+ */
+
+#pragma once
+
+#include <mutex>
+
+#include <libcamera/libcamera.h>
+
+#include <pybind11/smart_holder.h>
+
+using namespace libcamera;
+
+class PyCameraManager : public CameraManager
+{
+public:
+	PyCameraManager();
+	virtual ~PyCameraManager();
+
+	pybind11::list getCameras();
+
+	int eventFd() const { return eventFd_; }
+
+	std::vector<pybind11::object> getReadyRequests();
+
+	void handleRequestCompleted(Request *req);
+
+private:
+	int eventFd_ = -1;
+	std::mutex reqlist_mutex_;
+	std::vector<Request *> reqList_;
+
+	void writeFd();
+	void readFd();
+	void pushRequest(Request *req);
+	void getRequests(std::vector<Request *> &v);
+};
diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
index 5a423ece..23018288 100644
--- a/src/py/libcamera/py_main.cpp
+++ b/src/py/libcamera/py_main.cpp
@@ -7,10 +7,7 @@
 
 #include "py_main.h"
 
-#include <mutex>
 #include <stdexcept>
-#include <sys/eventfd.h>
-#include <unistd.h>
 
 #include <libcamera/base/log.h>
 
@@ -21,6 +18,7 @@
 #include <pybind11/stl.h>
 #include <pybind11/stl_bind.h>
 
+#include "py_camera_manager.h"
 #include "py_helpers.h"
 
 namespace py = pybind11;
@@ -29,27 +27,11 @@ using namespace libcamera;
 
 LOG_DEFINE_CATEGORY(Python)
 
-static std::weak_ptr<CameraManager> gCameraManager;
-static int gEventfd;
-static std::mutex gReqlistMutex;
-static std::vector<Request *> gReqList;
-
-static void handleRequestCompleted(Request *req)
-{
-	{
-		std::lock_guard guard(gReqlistMutex);
-		gReqList.push_back(req);
-	}
-
-	uint64_t v = 1;
-	size_t s = write(gEventfd, &v, 8);
-	/*
-	 * We should never fail, and have no simple means to manage the error,
-	 * so let's use LOG(Python, Fatal).
-	 */
-	if (s != 8)
-		LOG(Python, Fatal) << "Unable to write to eventfd";
-}
+/*
+ * XXX Note: global C++ destructors can be ran on this before the py module is
+ * destructed.
+ */
+static std::weak_ptr<PyCameraManager> gCameraManager;
 
 void init_py_enums(py::module &m);
 void init_py_controls_generated(py::module &m);
@@ -72,7 +54,7 @@ PYBIND11_MODULE(_libcamera, m)
 	 * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings
 	 */
 
-	auto pyCameraManager = py::class_<CameraManager>(m, "CameraManager");
+	auto pyCameraManager = py::class_<PyCameraManager>(m, "CameraManager");
 	auto pyCamera = py::class_<Camera>(m, "Camera");
 	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
 	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
@@ -106,78 +88,22 @@ PYBIND11_MODULE(_libcamera, m)
 	/* Classes */
 	pyCameraManager
 		.def_static("singleton", []() {
-			std::shared_ptr<CameraManager> cm = gCameraManager.lock();
-			if (cm)
-				return cm;
-
-			int fd = eventfd(0, 0);
-			if (fd == -1)
-				throw std::system_error(errno, std::generic_category(),
-							"Failed to create eventfd");
-
-			cm = std::shared_ptr<CameraManager>(new CameraManager, [](auto p) {
-				close(gEventfd);
-				gEventfd = -1;
-				delete p;
-			});
-
-			gEventfd = fd;
-			gCameraManager = cm;
-
-			int ret = cm->start();
-			if (ret)
-				throw std::system_error(-ret, std::generic_category(),
-							"Failed to start CameraManager");
-
-			return cm;
-		})
-
-		.def_property_readonly("version", &CameraManager::version)
-
-		.def_property_readonly("event_fd", [](CameraManager &) {
-			return gEventfd;
-		})
+			std::shared_ptr<PyCameraManager> cm = gCameraManager.lock();
 
-		.def("get_ready_requests", [](CameraManager &) {
-			uint8_t buf[8];
-
-			if (read(gEventfd, buf, 8) != 8)
-				throw std::system_error(errno, std::generic_category());
-
-			std::vector<Request *> v;
-
-			{
-				std::lock_guard guard(gReqlistMutex);
-				swap(v, gReqList);
+			if (!cm) {
+				cm = std::make_shared<PyCameraManager>();
+				gCameraManager = cm;
 			}
 
-			std::vector<py::object> ret;
-
-			for (Request *req : v) {
-				py::object o = py::cast(req);
-				/* Decrease the ref increased in Camera.queue_request() */
-				o.dec_ref();
-				ret.push_back(o);
-			}
-
-			return ret;
+			return cm;
 		})
 
-		.def("get", py::overload_cast<const std::string &>(&CameraManager::get), py::keep_alive<0, 1>())
+		.def_property_readonly("version", &PyCameraManager::version)
+		.def("get", py::overload_cast<const std::string &>(&PyCameraManager::get), py::keep_alive<0, 1>())
+		.def_property_readonly("cameras", &PyCameraManager::getCameras)
 
-		/* Create a list of Cameras, where each camera has a keep-alive to CameraManager */
-		.def_property_readonly("cameras", [](CameraManager &self) {
-			py::list l;
-
-			for (auto &c : self.cameras()) {
-				py::object py_cm = py::cast(self);
-				py::object py_cam = py::cast(c);
-				py::detail::keep_alive_impl(py_cam, py_cm);
-				l.append(py_cam);
-			}
-
-			return l;
-		});
+		.def_property_readonly("event_fd", &PyCameraManager::eventFd)
+		.def("get_ready_requests", &PyCameraManager::getReadyRequests);
 
 	pyCamera
 		.def_property_readonly("id", &Camera::id)
@@ -187,7 +113,13 @@ PYBIND11_MODULE(_libcamera, m)
 		                 const std::unordered_map<const ControlId *, py::object> &controls) {
 			/* \todo What happens if someone calls start() multiple times? */
 
-			self.requestCompleted.connect(handleRequestCompleted);
+			auto cm = gCameraManager.lock();
+			assert(cm);
+
+			self.requestCompleted.connect(&self, [cm=std::move(cm)](Request *req) {
+
+				cm->handleRequestCompleted(req);
+			});
 
 			ControlList controlList(self.controls());
 
@@ -198,7 +130,7 @@ PYBIND11_MODULE(_libcamera, m)
 
 			int ret = self.start(&controlList);
 			if (ret) {
-				self.requestCompleted.disconnect(handleRequestCompleted);
+				self.requestCompleted.disconnect();
 				return ret;
 			}
 
@@ -210,7 +142,7 @@ PYBIND11_MODULE(_libcamera, m)
 			if (ret)
 				return ret;
 
-			self.requestCompleted.disconnect(handleRequestCompleted);
+			self.requestCompleted.disconnect();
 
 			return 0;
 		})
-- 
2.34.1



More information about the libcamera-devel mailing list