From patchwork Tue May 30 09:20:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 18657 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 EE908C3213 for ; Tue, 30 May 2023 09:20:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 56831627B7; Tue, 30 May 2023 11:20:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1685438457; bh=MojXN5LsK1M08AUQsQRtnbnFG2cVSxHBamwtr1V7GrE=; 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=Yd+UQFCQwG+CMIEU133f/tspJEqaxLRLJXRWArPfOmPytorJysNmMlDt1vGlWrsOQ aKW8JanbuntugSS9gVg+pzQWGbIIMN2/soYi/moVZkQUnQ59swTdgFThYb7EeZtMZY Qb+mQsMYiWlfWyBQB6+NvRh3T1JFgNlgnFymccYXzOHmhls6vWHXwRho1O950ZwaOV YhFIb7nd8cwVrWIQCb93wZC8aR/BYRmnxQKeFDPtquQ1Bm/PHLSs0GDDzahr+j9oBD j3OxC2PBrE8/L1tYG7EyLudwXZX+2dJR9kGDl5bLJ6KVR5lE8Xl8a+4DxJpxYpEUbu D32Sjfl3mvkMg== 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 CFFE760388 for ; Tue, 30 May 2023 11:20:55 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="L0LJm48U"; dkim-atps=neutral Received: from desky.lan (91-154-35-171.elisa-laajakaista.fi [91.154.35.171]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F0AE327C; Tue, 30 May 2023 11:20:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1685438435; bh=MojXN5LsK1M08AUQsQRtnbnFG2cVSxHBamwtr1V7GrE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=L0LJm48UCeJtu/36DAl1YeZsNk+ZAk6VMTzt6jSNx5XMPL2QBmuG+qsdVOKENRnZu UQXpRq8Vrj9O+eGred7xk0lNFf1qi7nr5GAHdScEMAb4lM3mOZ6Wc5imKLG9bVxcVB o3szmxqSAIPsD3C+egufs/0rfiPF8Zwj2GQC4cPE= To: libcamera-devel@lists.libcamera.org Date: Tue, 30 May 2023 12:20:12 +0300 Message-Id: <20230530092016.34953-2-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> References: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/5] py: Fix CameraManager.version property 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" The current CameraManager.version doesn't work at all (raises a TypeError), as that's not how you use expose C++ static methods as Python class methods. Fix it. Signed-off-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart --- src/py/libcamera/py_main.cpp | 2 +- test/py/unittests.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index d14e18e2..c55495cc 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -105,7 +105,7 @@ PYBIND11_MODULE(_libcamera, m) return cm; }) - .def_property_readonly("version", &PyCameraManager::version) + .def_property_readonly_static("version", [](py::object /* self */) { return PyCameraManager::version(); }) .def("get", &PyCameraManager::get, py::keep_alive<0, 1>()) .def_property_readonly("cameras", &PyCameraManager::cameras) diff --git a/test/py/unittests.py b/test/py/unittests.py index 6dea80fc..90b04330 100755 --- a/test/py/unittests.py +++ b/test/py/unittests.py @@ -68,6 +68,10 @@ class SimpleTestMethods(BaseTestCase): # I expected EBUSY, but looks like double release works fine self.assertZero(ret) + def test_version(self): + cm = libcam.CameraManager.singleton() + self.assertIsInstance(cm.version, str) + class CameraTesterBase(BaseTestCase): cm: typing.Any From patchwork Tue May 30 09:20:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 18658 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 0AEA4C328E for ; Tue, 30 May 2023 09:21:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D633B627DC; Tue, 30 May 2023 11:20:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1685438458; bh=nHSP+q/90+O1ZqQyXaOquYKMd+RtlacWXMkMwb0jX+8=; 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=rAKr/RPfbWm4FfyByFdDpGlzCAgrQ9IuJs2Y3gbUdRir0xp2N1vUaxFFXxuzjnbip bKiq6tC7ur633kJrhJttyma7nmEDN+e+gRkKAdDn2/EoADQRltlzbo0X02HRk8YEoM EFCZoeevA+unZyygYldOe0/ElBZd6ATPUq9YRe62p450q8okEBvvwosrK+PwqE6aI+ N7qn96xJFUUEM1X9S3ee+9RerW8E/M7ScN97xA9dUpRzap1nMz850KsA3SznFcaFDZ ve2OWDEMCGDUVR+IwQgkID3l2vWTUxWkE5Ct5VZwsN9bl29T8WZlY5diccRZlbzil/ G5P0CyqO0LOyA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6619660388 for ; Tue, 30 May 2023 11:20:56 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="nUcVveIl"; dkim-atps=neutral Received: from desky.lan (91-154-35-171.elisa-laajakaista.fi [91.154.35.171]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8FD177F3; Tue, 30 May 2023 11:20:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1685438435; bh=nHSP+q/90+O1ZqQyXaOquYKMd+RtlacWXMkMwb0jX+8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nUcVveIl7YXYqaWNr7oddErUjvooJCajqbk5dAbt/sZh3nPPIl1ZrnWShRkwVsZjg 5QfCQnLfCleNtBJzd4FIp175CxJ15HsQxkYRwRxth+oMYZXyBJVl9si90OGogs2N2t nUevNWAaOx7Bofp32KNRFxRJcqscLz6oWK5a2K/A= To: libcamera-devel@lists.libcamera.org Date: Tue, 30 May 2023 12:20:13 +0300 Message-Id: <20230530092016.34953-3-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> References: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/5] py: Move ColorSpace and Transform classes to separate files 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" Move ColorSpace and Transform classes to separate files from the main py_main.cpp, for clarity. Signed-off-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart --- src/py/libcamera/meson.build | 2 + src/py/libcamera/py_color_space.cpp | 70 +++++++++++++++++ src/py/libcamera/py_main.cpp | 115 ++-------------------------- src/py/libcamera/py_transform.cpp | 81 ++++++++++++++++++++ 4 files changed, 158 insertions(+), 110 deletions(-) create mode 100644 src/py/libcamera/py_color_space.cpp create mode 100644 src/py/libcamera/py_transform.cpp diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build index af19ffdd..f87b1b4d 100644 --- a/src/py/libcamera/meson.build +++ b/src/py/libcamera/meson.build @@ -14,10 +14,12 @@ pybind11_dep = pybind11_proj.get_variable('pybind11_dep') pycamera_sources = files([ 'py_camera_manager.cpp', + 'py_color_space.cpp', 'py_enums.cpp', 'py_geometry.cpp', 'py_helpers.cpp', 'py_main.cpp', + 'py_transform.cpp', ]) # Generate controls diff --git a/src/py/libcamera/py_color_space.cpp b/src/py/libcamera/py_color_space.cpp new file mode 100644 index 00000000..a8301e3d --- /dev/null +++ b/src/py/libcamera/py_color_space.cpp @@ -0,0 +1,70 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Tomi Valkeinen + * + * Python bindings - Color Space classes + */ + +#include +#include + +#include +#include +#include + +namespace py = pybind11; + +using namespace libcamera; + +void init_py_color_space(py::module &m) +{ + auto pyColorSpace = py::class_(m, "ColorSpace"); + auto pyColorSpacePrimaries = py::enum_(pyColorSpace, "Primaries"); + auto pyColorSpaceTransferFunction = py::enum_(pyColorSpace, "TransferFunction"); + auto pyColorSpaceYcbcrEncoding = py::enum_(pyColorSpace, "YcbcrEncoding"); + auto pyColorSpaceRange = py::enum_(pyColorSpace, "Range"); + + pyColorSpace + .def(py::init([](ColorSpace::Primaries primaries, + ColorSpace::TransferFunction transferFunction, + ColorSpace::YcbcrEncoding ycbcrEncoding, + ColorSpace::Range range) { + return ColorSpace(primaries, transferFunction, ycbcrEncoding, range); + }), py::arg("primaries"), py::arg("transferFunction"), + py::arg("ycbcrEncoding"), py::arg("range")) + .def(py::init([](ColorSpace &other) { return other; })) + .def("__str__", [](ColorSpace &self) { + return ""; + }) + .def_readwrite("primaries", &ColorSpace::primaries) + .def_readwrite("transferFunction", &ColorSpace::transferFunction) + .def_readwrite("ycbcrEncoding", &ColorSpace::ycbcrEncoding) + .def_readwrite("range", &ColorSpace::range) + .def_static("Raw", []() { return ColorSpace::Raw; }) + .def_static("Srgb", []() { return ColorSpace::Srgb; }) + .def_static("Sycc", []() { return ColorSpace::Sycc; }) + .def_static("Smpte170m", []() { return ColorSpace::Smpte170m; }) + .def_static("Rec709", []() { return ColorSpace::Rec709; }) + .def_static("Rec2020", []() { return ColorSpace::Rec2020; }); + + pyColorSpacePrimaries + .value("Raw", ColorSpace::Primaries::Raw) + .value("Smpte170m", ColorSpace::Primaries::Smpte170m) + .value("Rec709", ColorSpace::Primaries::Rec709) + .value("Rec2020", ColorSpace::Primaries::Rec2020); + + pyColorSpaceTransferFunction + .value("Linear", ColorSpace::TransferFunction::Linear) + .value("Srgb", ColorSpace::TransferFunction::Srgb) + .value("Rec709", ColorSpace::TransferFunction::Rec709); + + pyColorSpaceYcbcrEncoding + .value("Null", ColorSpace::YcbcrEncoding::None) + .value("Rec601", ColorSpace::YcbcrEncoding::Rec601) + .value("Rec709", ColorSpace::YcbcrEncoding::Rec709) + .value("Rec2020", ColorSpace::YcbcrEncoding::Rec2020); + + pyColorSpaceRange + .value("Full", ColorSpace::Range::Full) + .value("Limited", ColorSpace::Range::Limited); +} diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index c55495cc..56d0717e 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -40,11 +40,13 @@ LOG_DEFINE_CATEGORY(Python) */ static std::weak_ptr gCameraManager; -void init_py_enums(py::module &m); +void init_py_color_space(py::module &m); void init_py_controls_generated(py::module &m); +void init_py_enums(py::module &m); void init_py_formats_generated(py::module &m); void init_py_geometry(py::module &m); void init_py_properties_generated(py::module &m); +void init_py_transform(py::module &m); PYBIND11_MODULE(_libcamera, m) { @@ -52,6 +54,8 @@ PYBIND11_MODULE(_libcamera, m) init_py_controls_generated(m); init_py_geometry(m); init_py_properties_generated(m); + init_py_color_space(m); + init_py_transform(m); /* Forward declarations */ @@ -79,12 +83,6 @@ PYBIND11_MODULE(_libcamera, m) auto pyFrameMetadata = py::class_(m, "FrameMetadata"); auto pyFrameMetadataStatus = py::enum_(pyFrameMetadata, "Status"); auto pyFrameMetadataPlane = py::class_(pyFrameMetadata, "Plane"); - auto pyTransform = py::class_(m, "Transform"); - auto pyColorSpace = py::class_(m, "ColorSpace"); - auto pyColorSpacePrimaries = py::enum_(pyColorSpace, "Primaries"); - auto pyColorSpaceTransferFunction = py::enum_(pyColorSpace, "TransferFunction"); - auto pyColorSpaceYcbcrEncoding = py::enum_(pyColorSpace, "YcbcrEncoding"); - auto pyColorSpaceRange = py::enum_(pyColorSpace, "Range"); auto pyPixelFormat = py::class_(m, "PixelFormat"); init_py_formats_generated(m); @@ -388,109 +386,6 @@ PYBIND11_MODULE(_libcamera, m) pyFrameMetadataPlane .def_readwrite("bytes_used", &FrameMetadata::Plane::bytesused); - pyTransform - .def(py::init([](int rotation, bool hflip, bool vflip, bool transpose) { - bool ok; - - Transform t = transformFromRotation(rotation, &ok); - if (!ok) - throw std::invalid_argument("Invalid rotation"); - - if (hflip) - t ^= Transform::HFlip; - if (vflip) - t ^= Transform::VFlip; - if (transpose) - t ^= Transform::Transpose; - return t; - }), py::arg("rotation") = 0, py::arg("hflip") = false, - py::arg("vflip") = false, py::arg("transpose") = false) - .def(py::init([](Transform &other) { return other; })) - .def("__str__", [](Transform &self) { - return ""; - }) - .def_property("hflip", - [](Transform &self) { - return !!(self & Transform::HFlip); - }, - [](Transform &self, bool hflip) { - if (hflip) - self |= Transform::HFlip; - else - self &= ~Transform::HFlip; - }) - .def_property("vflip", - [](Transform &self) { - return !!(self & Transform::VFlip); - }, - [](Transform &self, bool vflip) { - if (vflip) - self |= Transform::VFlip; - else - self &= ~Transform::VFlip; - }) - .def_property("transpose", - [](Transform &self) { - return !!(self & Transform::Transpose); - }, - [](Transform &self, bool transpose) { - if (transpose) - self |= Transform::Transpose; - else - self &= ~Transform::Transpose; - }) - .def("inverse", [](Transform &self) { return -self; }) - .def("invert", [](Transform &self) { - self = -self; - }) - .def("compose", [](Transform &self, Transform &other) { - self = self * other; - }); - - pyColorSpace - .def(py::init([](ColorSpace::Primaries primaries, - ColorSpace::TransferFunction transferFunction, - ColorSpace::YcbcrEncoding ycbcrEncoding, - ColorSpace::Range range) { - return ColorSpace(primaries, transferFunction, ycbcrEncoding, range); - }), py::arg("primaries"), py::arg("transferFunction"), - py::arg("ycbcrEncoding"), py::arg("range")) - .def(py::init([](ColorSpace &other) { return other; })) - .def("__str__", [](ColorSpace &self) { - return ""; - }) - .def_readwrite("primaries", &ColorSpace::primaries) - .def_readwrite("transferFunction", &ColorSpace::transferFunction) - .def_readwrite("ycbcrEncoding", &ColorSpace::ycbcrEncoding) - .def_readwrite("range", &ColorSpace::range) - .def_static("Raw", []() { return ColorSpace::Raw; }) - .def_static("Srgb", []() { return ColorSpace::Srgb; }) - .def_static("Sycc", []() { return ColorSpace::Sycc; }) - .def_static("Smpte170m", []() { return ColorSpace::Smpte170m; }) - .def_static("Rec709", []() { return ColorSpace::Rec709; }) - .def_static("Rec2020", []() { return ColorSpace::Rec2020; }); - - pyColorSpacePrimaries - .value("Raw", ColorSpace::Primaries::Raw) - .value("Smpte170m", ColorSpace::Primaries::Smpte170m) - .value("Rec709", ColorSpace::Primaries::Rec709) - .value("Rec2020", ColorSpace::Primaries::Rec2020); - - pyColorSpaceTransferFunction - .value("Linear", ColorSpace::TransferFunction::Linear) - .value("Srgb", ColorSpace::TransferFunction::Srgb) - .value("Rec709", ColorSpace::TransferFunction::Rec709); - - pyColorSpaceYcbcrEncoding - .value("Null", ColorSpace::YcbcrEncoding::None) - .value("Rec601", ColorSpace::YcbcrEncoding::Rec601) - .value("Rec709", ColorSpace::YcbcrEncoding::Rec709) - .value("Rec2020", ColorSpace::YcbcrEncoding::Rec2020); - - pyColorSpaceRange - .value("Full", ColorSpace::Range::Full) - .value("Limited", ColorSpace::Range::Limited); - pyPixelFormat .def(py::init<>()) .def(py::init()) diff --git a/src/py/libcamera/py_transform.cpp b/src/py/libcamera/py_transform.cpp new file mode 100644 index 00000000..08783e29 --- /dev/null +++ b/src/py/libcamera/py_transform.cpp @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Tomi Valkeinen + * + * Python bindings - Transform class + */ + +#include +#include + +#include +#include +#include + +namespace py = pybind11; + +using namespace libcamera; + +void init_py_transform(py::module &m) +{ + auto pyTransform = py::class_(m, "Transform"); + + pyTransform + .def(py::init([](int rotation, bool hflip, bool vflip, bool transpose) { + bool ok; + + Transform t = transformFromRotation(rotation, &ok); + if (!ok) + throw std::invalid_argument("Invalid rotation"); + + if (hflip) + t ^= Transform::HFlip; + if (vflip) + t ^= Transform::VFlip; + if (transpose) + t ^= Transform::Transpose; + return t; + }), py::arg("rotation") = 0, py::arg("hflip") = false, + py::arg("vflip") = false, py::arg("transpose") = false) + .def(py::init([](Transform &other) { return other; })) + .def("__str__", [](Transform &self) { + return ""; + }) + .def_property("hflip", + [](Transform &self) { + return !!(self & Transform::HFlip); + }, + [](Transform &self, bool hflip) { + if (hflip) + self |= Transform::HFlip; + else + self &= ~Transform::HFlip; + }) + .def_property("vflip", + [](Transform &self) { + return !!(self & Transform::VFlip); + }, + [](Transform &self, bool vflip) { + if (vflip) + self |= Transform::VFlip; + else + self &= ~Transform::VFlip; + }) + .def_property("transpose", + [](Transform &self) { + return !!(self & Transform::Transpose); + }, + [](Transform &self, bool transpose) { + if (transpose) + self |= Transform::Transpose; + else + self &= ~Transform::Transpose; + }) + .def("inverse", [](Transform &self) { return -self; }) + .def("invert", [](Transform &self) { + self = -self; + }) + .def("compose", [](Transform &self, Transform &other) { + self = self * other; + }); +} From patchwork Tue May 30 09:20:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 18659 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 5F3B0C3213 for ; Tue, 30 May 2023 09:21:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0EA536038D; Tue, 30 May 2023 11:21:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1685438462; bh=vor/8VFXBSwcad861uC0Bh5PAebGtD5tQjB9N0YwC/4=; 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=Sd9QANuNaLpSe6Gf5ooUAXzmMmCMnCZ0dar8oiINwBgyUQqM2Gp/Jl5gAULb+wjTT VW8vbU81jM4o4+BZFFGe1EqoKWK3j60wVkSKgKhLnHT958ZMR97mgnbob69y40+/xv mD7GZ+oJia20A3Wq10orp4Uk9wjKsyFEffJRXgexamMyH512Ph4kNfubY3spkDydDD 5gMlFVKMO+wgplEnHZbcLqxGGhGmQjqQVZrGH3PHkGk7xcDjlTrr0bg4kwDgDQSYKA 29C47QRV47Vfi7j8Yt6nWSqWr+BmC6cNG2TG/sdW00ORYB9Ckp5odnKQS0uUUlfNvC +mzWv2vDz1Axg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AFCF5627D3 for ; Tue, 30 May 2023 11:20:57 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Bzm7nZWS"; dkim-atps=neutral Received: from desky.lan (91-154-35-171.elisa-laajakaista.fi [91.154.35.171]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F5AA9B4; Tue, 30 May 2023 11:20:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1685438436; bh=vor/8VFXBSwcad861uC0Bh5PAebGtD5tQjB9N0YwC/4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Bzm7nZWSi9IT+CwpRrrtxzvu5RmeeIF0OpvuBqJ8h+DjVX8uzI2vMzOjfvkUUN+cP dehQEvmscakbY8uWFgKLLqOf2ZNDIys1Wa9Mun3Q5ho5FPRyz+KADeM94btUuEEfgO /07z23nakxFvfNFPkQg3pSGEtp1CopWiV9ucRipE= To: libcamera-devel@lists.libcamera.org Date: Tue, 30 May 2023 12:20:14 +0300 Message-Id: <20230530092016.34953-4-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> References: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/5] py: Use exceptions instead of returning error codes 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" We have multiple methods which return an error code, mimicking the C++ API. Using exceptions is more natural in the Python API, so change all those methods to raise an Exception instead. Signed-off-by: Tomi Valkeinen Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/py/cam/cam.py | 16 +---- src/py/examples/simple-cam.py | 15 +--- src/py/examples/simple-capture.py | 21 ++---- src/py/examples/simple-continuous-capture.py | 21 ++---- src/py/libcamera/py_main.cpp | 60 ++++++++++++---- test/py/unittests.py | 76 +++++++------------- 6 files changed, 91 insertions(+), 118 deletions(-) diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py index 967a72f5..a2a115c1 100755 --- a/src/py/cam/cam.py +++ b/src/py/cam/cam.py @@ -158,9 +158,7 @@ class CameraContext: print('Camera configuration adjusted') - r = self.camera.configure(camconfig) - if r != 0: - raise Exception('Configure failed') + self.camera.configure(camconfig) self.stream_names = {} self.streams = [] @@ -175,12 +173,7 @@ class CameraContext: allocator = libcam.FrameBufferAllocator(self.camera) for stream in self.streams: - ret = allocator.allocate(stream) - if ret < 0: - print('Cannot allocate buffers') - exit(-1) - - allocated = len(allocator.buffers(stream)) + allocated = allocator.allocate(stream) print('{}-{}: Allocated {} buffers'.format(self.id, self.stream_names[stream], allocated)) @@ -205,10 +198,7 @@ class CameraContext: buffers = self.allocator.buffers(stream) buffer = buffers[buf_num] - ret = request.add_buffer(stream, buffer) - if ret < 0: - print('Can not set buffer for request') - exit(-1) + request.add_buffer(stream, buffer) requests.append(request) diff --git a/src/py/examples/simple-cam.py b/src/py/examples/simple-cam.py index b3e97ca7..1cd1019d 100755 --- a/src/py/examples/simple-cam.py +++ b/src/py/examples/simple-cam.py @@ -259,12 +259,7 @@ def main(): allocator = libcam.FrameBufferAllocator(camera) for cfg in config: - ret = allocator.allocate(cfg.stream) - if ret < 0: - print('Can\'t allocate buffers') - return -1 - - allocated = len(allocator.buffers(cfg.stream)) + allocated = allocator.allocate(cfg.stream) print(f'Allocated {allocated} buffers for stream') # -------------------------------------------------------------------- @@ -289,15 +284,9 @@ def main(): requests = [] for i in range(len(buffers)): request = camera.create_request() - if not request: - print('Can\'t create request') - return -1 buffer = buffers[i] - ret = request.add_buffer(stream, buffer) - if ret < 0: - print('Can\'t set buffer for request') - return -1 + request.add_buffer(stream, buffer) # Controls can be added to a request on a per frame basis. request.set_control(libcam.controls.Brightness, 0.5) diff --git a/src/py/examples/simple-capture.py b/src/py/examples/simple-capture.py index 5f93574f..4b85408f 100755 --- a/src/py/examples/simple-capture.py +++ b/src/py/examples/simple-capture.py @@ -43,8 +43,7 @@ def main(): # Acquire the camera for our use - ret = cam.acquire() - assert ret == 0 + cam.acquire() # Configure the camera @@ -60,8 +59,7 @@ def main(): w, h = [int(v) for v in args.size.split('x')] stream_config.size = libcam.Size(w, h) - ret = cam.configure(cam_config) - assert ret == 0 + cam.configure(cam_config) print(f'Capturing {TOTAL_FRAMES} frames with {stream_config}') @@ -83,15 +81,13 @@ def main(): req = cam.create_request(i) buffer = allocator.buffers(stream)[i] - ret = req.add_buffer(stream, buffer) - assert ret == 0 + req.add_buffer(stream, buffer) reqs.append(req) # Start the camera - ret = cam.start() - assert ret == 0 + cam.start() # frames_queued and frames_done track the number of frames queued and done @@ -101,8 +97,7 @@ def main(): # Queue the requests to the camera for req in reqs: - ret = cam.queue_request(req) - assert ret == 0 + cam.queue_request(req) frames_queued += 1 # The main loop. Wait for the queued Requests to complete, process them, @@ -155,13 +150,11 @@ def main(): # Stop the camera - ret = cam.stop() - assert ret == 0 + cam.stop() # Release the camera - ret = cam.release() - assert ret == 0 + cam.release() return 0 diff --git a/src/py/examples/simple-continuous-capture.py b/src/py/examples/simple-continuous-capture.py index 26a8060b..e1cb931e 100755 --- a/src/py/examples/simple-continuous-capture.py +++ b/src/py/examples/simple-continuous-capture.py @@ -28,8 +28,7 @@ class CameraCaptureContext: # Acquire the camera for our use - ret = cam.acquire() - assert ret == 0 + cam.acquire() # Configure the camera @@ -37,8 +36,7 @@ class CameraCaptureContext: stream_config = cam_config.at(0) - ret = cam.configure(cam_config) - assert ret == 0 + cam.configure(cam_config) stream = stream_config.stream @@ -62,8 +60,7 @@ class CameraCaptureContext: req = cam.create_request(idx) buffer = allocator.buffers(stream)[i] - ret = req.add_buffer(stream, buffer) - assert ret == 0 + req.add_buffer(stream, buffer) self.reqs.append(req) @@ -73,13 +70,11 @@ class CameraCaptureContext: def uninit_camera(self): # Stop the camera - ret = self.cam.stop() - assert ret == 0 + self.cam.stop() # Release the camera - ret = self.cam.release() - assert ret == 0 + self.cam.release() # A container class for our state @@ -145,8 +140,7 @@ class CaptureContext: for cam_ctx in self.camera_contexts: for req in cam_ctx.reqs: - ret = cam_ctx.cam.queue_request(req) - assert ret == 0 + cam_ctx.cam.queue_request(req) # Use Selector to wait for events from the camera and from the keyboard @@ -177,8 +171,7 @@ def main(): # Start the cameras for cam_ctx in ctx.camera_contexts: - ret = cam_ctx.cam.start() - assert ret == 0 + cam_ctx.cam.start() ctx.capture() diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index 56d0717e..c66b90cd 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -112,8 +112,18 @@ PYBIND11_MODULE(_libcamera, m) pyCamera .def_property_readonly("id", &Camera::id) - .def("acquire", &Camera::acquire) - .def("release", &Camera::release) + .def("acquire", [](Camera &self) { + int ret = self.acquire(); + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to acquire camera"); + }) + .def("release", [](Camera &self) { + int ret = self.release(); + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to release camera"); + }) .def("start", [](Camera &self, const std::unordered_map &controls) { /* \todo What happens if someone calls start() multiple times? */ @@ -133,20 +143,19 @@ PYBIND11_MODULE(_libcamera, m) int ret = self.start(&controlList); if (ret) { self.requestCompleted.disconnect(); - return ret; + throw std::system_error(-ret, std::generic_category(), + "Failed to start camera"); } - - return 0; }, py::arg("controls") = std::unordered_map()) .def("stop", [](Camera &self) { int ret = self.stop(); - if (ret) - return ret; self.requestCompleted.disconnect(); - return 0; + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to stop camera"); }) .def("__str__", [](Camera &self) { @@ -155,9 +164,20 @@ PYBIND11_MODULE(_libcamera, m) /* Keep the camera alive, as StreamConfiguration contains a Stream* */ .def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>()) - .def("configure", &Camera::configure) + .def("configure", [](Camera &self, CameraConfiguration *config) { + int ret = self.configure(config); + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to configure camera"); + }) - .def("create_request", &Camera::createRequest, py::arg("cookie") = 0) + .def("create_request", [](Camera &self, uint64_t cookie) { + std::unique_ptr req = self.createRequest(cookie); + if (!req) + throw std::system_error(ENOMEM, std::generic_category(), + "Failed to create request"); + return req; + }, py::arg("cookie") = 0) .def("queue_request", [](Camera &self, Request *req) { py::object py_req = py::cast(req); @@ -170,10 +190,11 @@ PYBIND11_MODULE(_libcamera, m) py_req.inc_ref(); int ret = self.queueRequest(req); - if (ret) + if (ret) { py_req.dec_ref(); - - return ret; + throw std::system_error(-ret, std::generic_category(), + "Failed to queue request"); + } }) .def_property_readonly("streams", [](Camera &self) { @@ -251,7 +272,13 @@ PYBIND11_MODULE(_libcamera, m) pyFrameBufferAllocator .def(py::init>(), py::keep_alive<1, 2>()) - .def("allocate", &FrameBufferAllocator::allocate) + .def("allocate", [](FrameBufferAllocator &self, Stream *stream) { + int ret = self.allocate(stream); + if (ret < 0) + throw std::system_error(-ret, std::generic_category(), + "Failed to allocate buffers"); + return ret; + }) .def_property_readonly("allocated", &FrameBufferAllocator::allocated) /* Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator */ .def("buffers", [](FrameBufferAllocator &self, Stream *stream) { @@ -328,7 +355,10 @@ PYBIND11_MODULE(_libcamera, m) pyRequest /* \todo Fence is not supported, so we cannot expose addBuffer() directly */ .def("add_buffer", [](Request &self, const Stream *stream, FrameBuffer *buffer) { - return self.addBuffer(stream, buffer); + int ret = self.addBuffer(stream, buffer); + if (ret) + throw std::system_error(-ret, std::generic_category(), + "Failed to add buffer"); }, py::keep_alive<1, 3>()) /* Request keeps Framebuffer alive */ .def_property_readonly("status", &Request::status) .def_property_readonly("buffers", &Request::buffers) diff --git a/test/py/unittests.py b/test/py/unittests.py index 90b04330..0057ec92 100755 --- a/test/py/unittests.py +++ b/test/py/unittests.py @@ -42,31 +42,26 @@ class SimpleTestMethods(BaseTestCase): cam = cm.get('platform/vimc.0 Sensor B') self.assertIsNotNone(cam) - ret = cam.acquire() - self.assertZero(ret) + cam.acquire() - ret = cam.release() - self.assertZero(ret) + cam.release() def test_double_acquire(self): cm = libcam.CameraManager.singleton() cam = cm.get('platform/vimc.0 Sensor B') self.assertIsNotNone(cam) - ret = cam.acquire() - self.assertZero(ret) + cam.acquire() libcam.log_set_level('Camera', 'FATAL') - ret = cam.acquire() - self.assertEqual(ret, -errno.EBUSY) + with self.assertRaises(RuntimeError): + cam.acquire() libcam.log_set_level('Camera', 'ERROR') - ret = cam.release() - self.assertZero(ret) + cam.release() - ret = cam.release() - # I expected EBUSY, but looks like double release works fine - self.assertZero(ret) + # I expected exception here, but looks like double release works fine + cam.release() def test_version(self): cm = libcam.CameraManager.singleton() @@ -84,11 +79,7 @@ class CameraTesterBase(BaseTestCase): self.cm = None self.skipTest('No vimc found') - ret = self.cam.acquire() - if ret != 0: - self.cam = None - self.cm = None - raise Exception('Failed to acquire camera') + self.cam.acquire() self.wr_cam = weakref.ref(self.cam) self.wr_cm = weakref.ref(self.cm) @@ -97,9 +88,7 @@ class CameraTesterBase(BaseTestCase): # If a test fails, the camera may be in running state. So always stop. self.cam.stop() - ret = self.cam.release() - if ret != 0: - raise Exception('Failed to release camera') + self.cam.release() self.cam = None self.cm = None @@ -119,8 +108,7 @@ class AllocatorTestMethods(CameraTesterBase): streamconfig = camconfig.at(0) wr_streamconfig = weakref.ref(streamconfig) - ret = cam.configure(camconfig) - self.assertZero(ret) + cam.configure(camconfig) stream = streamconfig.stream wr_stream = weakref.ref(stream) @@ -133,8 +121,8 @@ class AllocatorTestMethods(CameraTesterBase): self.assertIsNotNone(wr_streamconfig()) allocator = libcam.FrameBufferAllocator(cam) - ret = allocator.allocate(stream) - self.assertTrue(ret > 0) + num_bufs = allocator.allocate(stream) + self.assertTrue(num_bufs > 0) wr_allocator = weakref.ref(allocator) buffers = allocator.buffers(stream) @@ -177,14 +165,13 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(fmts) fmts = None - ret = cam.configure(camconfig) - self.assertZero(ret) + cam.configure(camconfig) stream = streamconfig.stream allocator = libcam.FrameBufferAllocator(cam) - ret = allocator.allocate(stream) - self.assertTrue(ret > 0) + num_bufs = allocator.allocate(stream) + self.assertTrue(num_bufs > 0) num_bufs = len(allocator.buffers(stream)) @@ -194,19 +181,16 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(req) buffer = allocator.buffers(stream)[i] - ret = req.add_buffer(stream, buffer) - self.assertZero(ret) + req.add_buffer(stream, buffer) reqs.append(req) buffer = None - ret = cam.start() - self.assertZero(ret) + cam.start() for req in reqs: - ret = cam.queue_request(req) - self.assertZero(ret) + cam.queue_request(req) reqs = None gc.collect() @@ -234,8 +218,7 @@ class SimpleCaptureMethods(CameraTesterBase): reqs = None gc.collect() - ret = cam.stop() - self.assertZero(ret) + cam.stop() def test_select(self): cm = self.cm @@ -249,14 +232,13 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(fmts) fmts = None - ret = cam.configure(camconfig) - self.assertZero(ret) + cam.configure(camconfig) stream = streamconfig.stream allocator = libcam.FrameBufferAllocator(cam) - ret = allocator.allocate(stream) - self.assertTrue(ret > 0) + num_bufs = allocator.allocate(stream) + self.assertTrue(num_bufs > 0) num_bufs = len(allocator.buffers(stream)) @@ -266,19 +248,16 @@ class SimpleCaptureMethods(CameraTesterBase): self.assertIsNotNone(req) buffer = allocator.buffers(stream)[i] - ret = req.add_buffer(stream, buffer) - self.assertZero(ret) + req.add_buffer(stream, buffer) reqs.append(req) buffer = None - ret = cam.start() - self.assertZero(ret) + cam.start() for req in reqs: - ret = cam.queue_request(req) - self.assertZero(ret) + cam.queue_request(req) reqs = None gc.collect() @@ -307,8 +286,7 @@ class SimpleCaptureMethods(CameraTesterBase): reqs = None gc.collect() - ret = cam.stop() - self.assertZero(ret) + cam.stop() # Recursively expand slist's objects into olist, using seen to track already From patchwork Tue May 30 09:20:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 18660 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 12A73C328E for ; Tue, 30 May 2023 09:21:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C2187627B1; Tue, 30 May 2023 11:21:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1685438462; bh=etUTNpglDs12cp0qIBTbbGRzSHyx4+IT4y7n3WHxxm8=; 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=kCbH1B0cTEkX3Dzl769I0UKXofizDzM0oyZYIZ/0qyJeAzc9ziqCpyaIwHkjshYWm JVOGBpfTWEdLd851Jru6J1z3TDIPrhvO9FDz+8MTs17aUz6K5RItVwps3tJaXwPrhE NY2fLYxqB6R36e8MeOs7GBQtnJtGezNan37SzOpUgQubuXHK8bAmw0CJt2lVWXKUGp ErhsQv0RlY/jvGKddmH3Tc4X8A1S9zPnB4s6C8aOTq8eWzVlr9ymSom1FtqrreUbvY eV5Z7grdPz/i62tPrXH8LUy1AmWG9foMcNejcYesKUwb5qH1F2ExhUFQ4d7ObYU10r BUu8H84QVDrJg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E9B06627DE for ; Tue, 30 May 2023 11:20:57 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="lAZ+XQ8M"; dkim-atps=neutral Received: from desky.lan (91-154-35-171.elisa-laajakaista.fi [91.154.35.171]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 02BB7AB; Tue, 30 May 2023 11:20:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1685438437; bh=etUTNpglDs12cp0qIBTbbGRzSHyx4+IT4y7n3WHxxm8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lAZ+XQ8MFSVDhp2KY9YmuIkGxgAmNvRn2BkGd5tDiYfJUlbWv5ERRAJ9p9/3Uz3lL Cj0HHaRPtjwtIpTcKuu2XorPkFRbbjWyhBlbFb+Hl/FcwpievyeFCXtjxTKBWkqZX+ vchiK/wp6gmeBIhaKi3Mxz3XQfDzkSadZYFw6FOE= To: libcamera-devel@lists.libcamera.org Date: Tue, 30 May 2023 12:20:15 +0300 Message-Id: <20230530092016.34953-5-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> References: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 4/5] py: unittests.py: Add weakref helpers and use del 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" Add helpers to check if a weakref or a list of weakrefs is alive or dead. Also use 'del' for local variables instead of setting the variable to None. This makes debugging the test easier as the locals will be gone from locals() dict. Signed-off-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart --- test/py/unittests.py | 60 ++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/test/py/unittests.py b/test/py/unittests.py index 0057ec92..fe1e8ef0 100755 --- a/test/py/unittests.py +++ b/test/py/unittests.py @@ -18,6 +18,18 @@ class BaseTestCase(unittest.TestCase): def assertZero(self, a, msg=None): self.assertEqual(a, 0, msg) + def assertIsAlive(self, wr, msg='object not alive'): + self.assertIsNotNone(wr(), msg) + + def assertIsDead(self, wr, msg='object not dead'): + self.assertIsNone(wr(), msg) + + def assertIsAllAlive(self, wr_list, msg='object not alive'): + self.assertTrue(all([wr() for wr in wr_list]), msg) + + def assertIsAllDead(self, wr_list, msg='object not dead'): + self.assertTrue(all([not wr() for wr in wr_list]), msg) + class SimpleTestMethods(BaseTestCase): def test_get_ref(self): @@ -28,14 +40,14 @@ class SimpleTestMethods(BaseTestCase): self.assertIsNotNone(cam) wr_cam = weakref.ref(cam) - cm = None + del cm gc.collect() - self.assertIsNotNone(wr_cm()) + self.assertIsAlive(wr_cm) - cam = None + del cam gc.collect() - self.assertIsNone(wr_cm()) - self.assertIsNone(wr_cam()) + self.assertIsDead(wr_cm) + self.assertIsDead(wr_cam) def test_acquire_release(self): cm = libcam.CameraManager.singleton() @@ -93,8 +105,8 @@ class CameraTesterBase(BaseTestCase): self.cam = None self.cm = None - self.assertIsNone(self.wr_cm()) - self.assertIsNone(self.wr_cam()) + self.assertIsDead(self.wr_cm) + self.assertIsDead(self.wr_cam) class AllocatorTestMethods(CameraTesterBase): @@ -114,11 +126,11 @@ class AllocatorTestMethods(CameraTesterBase): wr_stream = weakref.ref(stream) # stream should keep streamconfig and camconfig alive - streamconfig = None - camconfig = None + del streamconfig + del camconfig gc.collect() - self.assertIsNotNone(wr_camconfig()) - self.assertIsNotNone(wr_streamconfig()) + self.assertIsAlive(wr_camconfig) + self.assertIsAlive(wr_streamconfig) allocator = libcam.FrameBufferAllocator(cam) num_bufs = allocator.allocate(stream) @@ -127,29 +139,29 @@ class AllocatorTestMethods(CameraTesterBase): buffers = allocator.buffers(stream) self.assertIsNotNone(buffers) - buffers = None + del buffers buffer = allocator.buffers(stream)[0] self.assertIsNotNone(buffer) wr_buffer = weakref.ref(buffer) - allocator = None + del allocator gc.collect() - self.assertIsNotNone(wr_buffer()) - self.assertIsNotNone(wr_allocator()) - self.assertIsNotNone(wr_stream()) + self.assertIsAlive(wr_buffer) + self.assertIsAlive(wr_allocator) + self.assertIsAlive(wr_stream) - buffer = None + del buffer gc.collect() - self.assertIsNone(wr_buffer()) - self.assertIsNone(wr_allocator()) - self.assertIsNotNone(wr_stream()) + self.assertIsDead(wr_buffer) + self.assertIsDead(wr_allocator) + self.assertIsAlive(wr_stream) - stream = None + del stream gc.collect() - self.assertIsNone(wr_stream()) - self.assertIsNone(wr_camconfig()) - self.assertIsNone(wr_streamconfig()) + self.assertIsDead(wr_stream) + self.assertIsDead(wr_camconfig) + self.assertIsDead(wr_streamconfig) class SimpleCaptureMethods(CameraTesterBase): From patchwork Tue May 30 09:20:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 18661 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 B7B8BC3213 for ; Tue, 30 May 2023 09:21:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 787886287F; Tue, 30 May 2023 11:21:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1685438463; bh=PltyImOLae7tYoV0O4X3/qeWYKQH+YvzD+u+TI6Fqls=; 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=b6+HObCkXMbJFyKePIIuOhJr59bT7mZoQM+EvQ5iWvmIL0vDn1StKtAxUnB9h7D2x K3jtvKmhi5jSP6ACKfdlantEwbGMgRlcJuWEvinVDn1+7n6ksodsJ1omFVUOd6wGbd qUmlziU3EQNLYnzIFvFWX/LdLtbtbj38rRBCulQIP6aWragNzh4fpBrs1reoDNDMA1 xTYsb3FxDwptqyrCI+Fugt6fjNlZ6IUwSfiXtV9dA0qBFDUmvIoD0OFvNnlTqpFh25 Qsunj+dFKLD7HMVdfkstOjEUvsbtreX5DViczuyOI5fZe8YCOog4ULNrRrprNurt7x yhe0AkaIzQgkA== 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 B831A6287D for ; Tue, 30 May 2023 11:20:58 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="EFvzvG+K"; dkim-atps=neutral Received: from desky.lan (91-154-35-171.elisa-laajakaista.fi [91.154.35.171]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AC3A07F3; Tue, 30 May 2023 11:20:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1685438438; bh=PltyImOLae7tYoV0O4X3/qeWYKQH+YvzD+u+TI6Fqls=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EFvzvG+Kz9T9Hlb1Pm2/NCnmuHIwLFyymBJGkBGQvoUVLu4JQmlUoSZiJGqR4Jdwg K8/lccl/5K+xu7MratR5Ma2hb53BZchoqjE5QpUkWIOHw6CeJov5MgQZgSn6MSo5yp 6CO6ZQ6MsYkpsJ3KXp1FQOBHlkkf3c5nN2nrcZp0= To: libcamera-devel@lists.libcamera.org Date: Tue, 30 May 2023 12:20:16 +0300 Message-Id: <20230530092016.34953-6-tomi.valkeinen@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> References: <20230530092016.34953-1-tomi.valkeinen@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 5/5] py: Move to mainline pybind11 version 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" We are using pybind11 'smart_holder' branch to solve the Camera destructor issue (see the comment in this patch, or the commit that originally added Python bindings support). As it would be very nice to use the mainline pybind11 (which is packaged in distributions), this patch adds a workaround allowing us to move to the mainline pybind11 version. The workaround is simply creating a custom holder class, used only for the Camera, which wraps around the shared_ptr. This makes the compiler happy. Moving to mainline pybdin11 is achieved with: - Change the pybind11 wrap to point to the mainline pybdind11 version - Change the meson.build file to use a system-installed pybind11 if available, and use the pybind11 subproject as a fallback. The fallback is there as a convenience, as pybind11 may not be available in pre-packaged form in all environments. Signed-off-by: Tomi Valkeinen --- src/py/libcamera/meson.build | 11 ++-- src/py/libcamera/py_camera_manager.h | 2 +- src/py/libcamera/py_color_space.cpp | 2 +- src/py/libcamera/py_controls_generated.cpp.in | 2 +- src/py/libcamera/py_enums.cpp | 2 +- src/py/libcamera/py_formats_generated.cpp.in | 2 +- src/py/libcamera/py_geometry.cpp | 2 +- src/py/libcamera/py_helpers.h | 2 +- src/py/libcamera/py_main.cpp | 52 +++++++++++++++++-- .../libcamera/py_properties_generated.cpp.in | 2 +- src/py/libcamera/py_transform.cpp | 2 +- subprojects/.gitignore | 2 +- subprojects/pybind11.wrap | 18 ++++--- 13 files changed, 76 insertions(+), 25 deletions(-) diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build index f87b1b4d..9d86d8bf 100644 --- a/src/py/libcamera/meson.build +++ b/src/py/libcamera/meson.build @@ -7,10 +7,15 @@ if not py3_dep.found() subdir_done() endif -pycamera_enabled = true +pybind11_dep = dependency('pybind11', required : get_option('pycamera'), + fallback : ['pybind11', 'pybind11_dep']) + +if not pybind11_dep.found() + pycamera_enabled = false + subdir_done() +endif -pybind11_proj = subproject('pybind11') -pybind11_dep = pybind11_proj.get_variable('pybind11_dep') +pycamera_enabled = true pycamera_sources = files([ 'py_camera_manager.cpp', diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h index 3525057d..3574db23 100644 --- a/src/py/libcamera/py_camera_manager.h +++ b/src/py/libcamera/py_camera_manager.h @@ -9,7 +9,7 @@ #include -#include +#include using namespace libcamera; diff --git a/src/py/libcamera/py_color_space.cpp b/src/py/libcamera/py_color_space.cpp index a8301e3d..5201121a 100644 --- a/src/py/libcamera/py_color_space.cpp +++ b/src/py/libcamera/py_color_space.cpp @@ -9,7 +9,7 @@ #include #include -#include +#include #include namespace py = pybind11; diff --git a/src/py/libcamera/py_controls_generated.cpp.in b/src/py/libcamera/py_controls_generated.cpp.in index cb8442ba..18fa57d9 100644 --- a/src/py/libcamera/py_controls_generated.cpp.in +++ b/src/py/libcamera/py_controls_generated.cpp.in @@ -9,7 +9,7 @@ #include -#include +#include namespace py = pybind11; diff --git a/src/py/libcamera/py_enums.cpp b/src/py/libcamera/py_enums.cpp index 96d4beef..803c4e7e 100644 --- a/src/py/libcamera/py_enums.cpp +++ b/src/py/libcamera/py_enums.cpp @@ -7,7 +7,7 @@ #include -#include +#include namespace py = pybind11; diff --git a/src/py/libcamera/py_formats_generated.cpp.in b/src/py/libcamera/py_formats_generated.cpp.in index b88807f3..a3f7f94d 100644 --- a/src/py/libcamera/py_formats_generated.cpp.in +++ b/src/py/libcamera/py_formats_generated.cpp.in @@ -9,7 +9,7 @@ #include -#include +#include namespace py = pybind11; diff --git a/src/py/libcamera/py_geometry.cpp b/src/py/libcamera/py_geometry.cpp index 84b0cb08..5c2aeac4 100644 --- a/src/py/libcamera/py_geometry.cpp +++ b/src/py/libcamera/py_geometry.cpp @@ -11,7 +11,7 @@ #include #include -#include +#include #include namespace py = pybind11; diff --git a/src/py/libcamera/py_helpers.h b/src/py/libcamera/py_helpers.h index cd31e2cc..983969df 100644 --- a/src/py/libcamera/py_helpers.h +++ b/src/py/libcamera/py_helpers.h @@ -7,7 +7,7 @@ #include -#include +#include pybind11::object controlValueToPy(const libcamera::ControlValue &cv); libcamera::ControlValue pyToControlValue(const pybind11::object &ob, libcamera::ControlType type); diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index c66b90cd..bbae7a99 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -17,7 +17,7 @@ #include #include -#include +#include #include #include @@ -33,6 +33,50 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Python) } +/* + * This is a holder class used only for the Camera class, for the sole purpose + * of avoiding the compilation issue with Camera's private destructor. + * + * pybind11 requires a public destructor for classes held with shared_ptrs, even + * in cases where the public destructor is not strictly needed. The current + * understanding is that there are the following options to solve the problem: + * + * - Use pybind11 'smart_holder' branch. The downside is that 'smart_holder' + * is not the mainline branch, and not available in distributions. + * - https://github.com/pybind/pybind11/pull/2067 + * - Make the Camera destructor public + * - Something like the PyCameraSmartPtr here, which adds a layer, hiding the + * issue. + */ +template +class PyCameraSmartPtr +{ +public: + using element_type = T; + + PyCameraSmartPtr() + { + } + + explicit PyCameraSmartPtr(T *) + { + throw std::runtime_error("invalid SmartPtr constructor call"); + } + + explicit PyCameraSmartPtr(std::shared_ptr p) + : ptr_(p) + { + } + + T *get() const { return ptr_.get(); } + + operator std::shared_ptr() const { return ptr_; } + +private: + std::shared_ptr ptr_; +}; + +PYBIND11_DECLARE_HOLDER_TYPE(T, PyCameraSmartPtr); /* * Note: global C++ destructors can be ran on this before the py module is @@ -65,8 +109,8 @@ 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 pyCamera = py::class_(m, "Camera"); + auto pyCameraManager = py::class_>(m, "CameraManager"); + auto pyCamera = py::class_>(m, "Camera"); auto pyCameraConfiguration = py::class_(m, "CameraConfiguration"); auto pyCameraConfigurationStatus = py::enum_(pyCameraConfiguration, "Status"); auto pyStreamConfiguration = py::class_(m, "StreamConfiguration"); @@ -271,7 +315,7 @@ PYBIND11_MODULE(_libcamera, m) .def("range", &StreamFormats::range); pyFrameBufferAllocator - .def(py::init>(), py::keep_alive<1, 2>()) + .def(py::init>(), py::keep_alive<1, 2>()) .def("allocate", [](FrameBufferAllocator &self, Stream *stream) { int ret = self.allocate(stream); if (ret < 0) diff --git a/src/py/libcamera/py_properties_generated.cpp.in b/src/py/libcamera/py_properties_generated.cpp.in index 044b2b2a..e49b6e91 100644 --- a/src/py/libcamera/py_properties_generated.cpp.in +++ b/src/py/libcamera/py_properties_generated.cpp.in @@ -9,7 +9,7 @@ #include -#include +#include namespace py = pybind11; diff --git a/src/py/libcamera/py_transform.cpp b/src/py/libcamera/py_transform.cpp index 08783e29..f3a0bfaf 100644 --- a/src/py/libcamera/py_transform.cpp +++ b/src/py/libcamera/py_transform.cpp @@ -9,7 +9,7 @@ #include #include -#include +#include #include namespace py = pybind11; diff --git a/subprojects/.gitignore b/subprojects/.gitignore index 0a8fd3a6..ebe59479 100644 --- a/subprojects/.gitignore +++ b/subprojects/.gitignore @@ -4,4 +4,4 @@ /libyaml /libyuv /packagecache -/pybind11 +/pybind11-* diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap index dd02687b..96ec57a1 100644 --- a/subprojects/pybind11.wrap +++ b/subprojects/pybind11.wrap @@ -1,11 +1,13 @@ -# SPDX-License-Identifier: CC0-1.0 - -[wrap-git] -url = https://github.com/pybind/pybind11.git -# This is the head of 'smart_holder' branch -revision = aebdf00cd060b871c5a1e0c2cf4a333503dd0431 -depth = 1 -patch_directory = pybind11 +[wrap-file] +directory = pybind11-2.10.4 +source_url = https://github.com/pybind/pybind11/archive/refs/tags/v2.10.4.tar.gz +source_filename = pybind11-2.10.4.tar.gz +source_hash = 832e2f309c57da9c1e6d4542dedd34b24e4192ecb4d62f6f4866a737454c9970 +patch_filename = pybind11_2.10.4-1_patch.zip +patch_url = https://wrapdb.mesonbuild.com/v2/pybind11_2.10.4-1/get_patch +patch_hash = 9489d0cdc1244078a3108c52b4591a6f07f3dc30ca7299d3a3c42b84fa763396 +source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/pybind11_2.10.4-1/pybind11-2.10.4.tar.gz +wrapdb_version = 2.10.4-1 [provide] pybind11 = pybind11_dep