From patchwork Fri Jul 1 08:45:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 16497 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 76322BD808 for ; Fri, 1 Jul 2022 08:45:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EB0356569E; Fri, 1 Jul 2022 10:45:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1656665148; bh=FDESrgPL0gxNa+Z4XsJ2JAUY5xvJcK9lZM+SU5xx8Xw=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=RmbNu61OLccvlsqaXnSr6GyRr/X5PG/ESM/eXTwpjrmwobimEP7X/mSDqY+8pOmIx J+LZFE+upUnh2aejxcEb3JJZUQeZacs1sRalG1kw7uRRlUwe4lhvtDrp7yAMDl4oS2 9WkwnkLI4ONCvSUTr9L4aOTVolAV4GffGSQsKqnnVGCtCGuSv9TsvsebT4yFr1hF0L j/SmspIygeBPtCWBFzH7+6OOMacl1i3ujo2Vl8BM253ipqzTgEobV+1F/XF6+WvIlp j5V2zKDOAeu5YUIYELiE1ZSGjuSseYLovgS7gqhP8gWqfUzfDVn8bk6G0U4wWd+IbL +KZMyuN/x7LuQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 34D2465657 for ; Fri, 1 Jul 2022 10:45:43 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="mRLyU1a9"; dkim-atps=neutral Received: from deskari.lan (91-158-154-79.elisa-laajakaista.fi [91.158.154.79]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8B8F625C; Fri, 1 Jul 2022 10:45:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1656665142; bh=FDESrgPL0gxNa+Z4XsJ2JAUY5xvJcK9lZM+SU5xx8Xw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mRLyU1a9/+aZjyNAIcH+VbVzopNsoOOLYt+poXwXViQswfw/5mJj0+GkFKCwknSWj WIz9VTCx6Sj5u/I1hs0LzQ9+VGjrHWfDwWUH+ki/CH+Fu//cMmeZZbgSt//e1iBcpi KYBGORLCTpGkWvO0T+RgxDthYWTpkSc5fA9DrYlo= To: libcamera-devel@lists.libcamera.org, David Plowman , Kieran Bingham , Laurent Pinchart , Jacopo Mondi Date: Fri, 1 Jul 2022 11:45:09 +0300 Message-Id: <20220701084521.31831-6-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220701084521.31831-1-tomi.valkeinen@ideasonboard.com> References: <20220701084521.31831-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 05/17] py: Create PyCameraManager X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Tomi Valkeinen via libcamera-devel From: Tomi Valkeinen Reply-To: Tomi Valkeinen Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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. Signed-off-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart --- src/py/libcamera/meson.build | 1 + src/py/libcamera/py_camera_manager.cpp | 125 +++++++++++++++++++++++++ src/py/libcamera/py_camera_manager.h | 44 +++++++++ src/py/libcamera/py_main.cpp | 117 +++++------------------ 4 files changed, 193 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..ad47271d --- /dev/null +++ b/src/py/libcamera/py_camera_manager.cpp @@ -0,0 +1,125 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Tomi Valkeinen + */ + +#include "py_camera_manager.h" + +#include +#include +#include +#include + +#include "py_main.h" + +namespace py = pybind11; + +using namespace libcamera; + +PyCameraManager::PyCameraManager() +{ + LOG(Python, Debug) << "PyCameraManager()"; + + cameraManager_ = std::make_unique(); + + int fd = eventfd(0, 0); + if (fd == -1) + throw std::system_error(errno, std::generic_category(), + "Failed to create eventfd"); + + eventFd_ = fd; + + int ret = cameraManager_->start(); + if (ret) { + close(fd); + eventFd_ = -1; + throw std::system_error(-ret, std::generic_category(), + "Failed to start CameraManager"); + } +} + +PyCameraManager::~PyCameraManager() +{ + LOG(Python, Debug) << "~PyCameraManager()"; + + if (eventFd_ != -1) { + close(eventFd_); + eventFd_ = -1; + } +} + +py::list PyCameraManager::cameras() +{ + /* + * Create a list of Cameras, where each camera has a keep-alive to + * CameraManager. + */ + py::list l; + + for (auto &camera : cameraManager_->cameras()) { + py::object py_cm = py::cast(this); + py::object py_cam = py::cast(camera); + py::detail::keep_alive_impl(py_cam, py_cm); + l.append(py_cam); + } + + return l; +} + +std::vector PyCameraManager::getReadyRequests() +{ + readFd(); + + std::vector ret; + + for (Request *request : getCompletedRequests()) { + py::object o = py::cast(request); + /* 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 log a fatal error. + */ + 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(completedRequestsMutex_); + completedRequests_.push_back(req); +} + +std::vector PyCameraManager::getCompletedRequests() +{ + std::vector v; + std::lock_guard guard(completedRequestsMutex_); + swap(v, completedRequests_); + return v; +} diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h new file mode 100644 index 00000000..9c15f814 --- /dev/null +++ b/src/py/libcamera/py_camera_manager.h @@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Tomi Valkeinen + */ + +#pragma once + +#include + +#include + +#include + +using namespace libcamera; + +class PyCameraManager +{ +public: + PyCameraManager(); + ~PyCameraManager(); + + pybind11::list cameras(); + std::shared_ptr get(const std::string &name) { return cameraManager_->get(name); } + + static const std::string &version() { return CameraManager::version(); } + + int eventFd() const { return eventFd_; } + + std::vector getReadyRequests(); + + void handleRequestCompleted(Request *req); + +private: + std::unique_ptr cameraManager_; + + int eventFd_ = -1; + std::mutex completedRequestsMutex_; + std::vector completedRequests_; + + void writeFd(); + void readFd(); + void pushRequest(Request *req); + std::vector getCompletedRequests(); +}; diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index e652837f..e7d078b5 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -7,10 +7,7 @@ #include "py_main.h" -#include #include -#include -#include #include @@ -21,6 +18,7 @@ #include #include +#include "py_camera_manager.h" #include "py_helpers.h" namespace py = pybind11; @@ -33,27 +31,11 @@ LOG_DEFINE_CATEGORY(Python) } -static std::weak_ptr gCameraManager; -static int gEventfd; -static std::mutex gReqlistMutex; -static std::vector 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 log a fatal error. - */ - if (s != 8) - LOG(Python, Fatal) << "Unable to write to eventfd"; -} +/* + * Note: global C++ destructors can be ran on this before the py module is + * destructed. + */ +static std::weak_ptr gCameraManager; void init_py_enums(py::module &m); void init_py_controls_generated(py::module &m); @@ -76,7 +58,7 @@ PYBIND11_MODULE(_libcamera, m) * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings */ - auto pyCameraManager = py::class_(m, "CameraManager"); + auto pyCameraManager = py::class_(m, "CameraManager"); auto pyCamera = py::class_(m, "Camera"); auto pyCameraConfiguration = py::class_(m, "CameraConfiguration"); auto pyCameraConfigurationStatus = py::enum_(pyCameraConfiguration, "Status"); @@ -110,78 +92,22 @@ PYBIND11_MODULE(_libcamera, m) /* Classes */ pyCameraManager .def_static("singleton", []() { - std::shared_ptr 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(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) + std::shared_ptr cm = gCameraManager.lock(); - .def_property_readonly("event_fd", [](CameraManager &) { - return gEventfd; - }) - - .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 v; - - { - std::lock_guard guard(gReqlistMutex); - swap(v, gReqList); + if (!cm) { + cm = std::make_shared(); + gCameraManager = cm; } - std::vector 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(&CameraManager::get), py::keep_alive<0, 1>()) - - /* 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); - } + .def_property_readonly("version", &PyCameraManager::version) + .def("get", &PyCameraManager::get, py::keep_alive<0, 1>()) + .def_property_readonly("cameras", &PyCameraManager::cameras) - return l; - }); + .def_property_readonly("event_fd", &PyCameraManager::eventFd) + .def("get_ready_requests", &PyCameraManager::getReadyRequests); pyCamera .def_property_readonly("id", &Camera::id) @@ -191,7 +117,10 @@ PYBIND11_MODULE(_libcamera, m) const std::unordered_map &controls) { /* \todo What happens if someone calls start() multiple times? */ - self.requestCompleted.connect(handleRequestCompleted); + auto cm = gCameraManager.lock(); + ASSERT(cm); + + self.requestCompleted.connect(cm.get(), &PyCameraManager::handleRequestCompleted); ControlList controlList(self.controls()); @@ -202,7 +131,7 @@ PYBIND11_MODULE(_libcamera, m) int ret = self.start(&controlList); if (ret) { - self.requestCompleted.disconnect(handleRequestCompleted); + self.requestCompleted.disconnect(); return ret; } @@ -214,7 +143,7 @@ PYBIND11_MODULE(_libcamera, m) if (ret) return ret; - self.requestCompleted.disconnect(handleRequestCompleted); + self.requestCompleted.disconnect(); return 0; })