[{"id":21692,"web_url":"https://patchwork.libcamera.org/comment/21692/","msgid":"<163904389093.2066819.16202667063304401995@Monstersaurus>","date":"2021-12-09T09:58:10","subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Tomi Valkeinen (2021-12-09 09:29:05)\n> Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n> Python layer.\n> \n> Only a subset of libcamera classes are exposed.\n> \n> Signed-off-by: Tomi Valkeinen \n> ---\n> .gitignore | 1 +\n> meson.build | 1 +\n> meson_options.txt | 5 +\n> src/meson.build | 1 +\n> src/py/meson.build | 1 +\n> src/py/pycamera/__init__.py | 10 +\n> src/py/pycamera/meson.build | 43 ++++\n> src/py/pycamera/pymain.cpp | 424 ++++++++++++++++++++++++++++++++++++\n> subprojects/pybind11.wrap | 12 +\n> 9 files changed, 498 insertions(+)\n> create mode 100644 src/py/meson.build\n> create mode 100644 src/py/pycamera/__init__.py\n> create mode 100644 src/py/pycamera/meson.build\n> create mode 100644 src/py/pycamera/pymain.cpp\n> create mode 100644 subprojects/pybind11.wrap\n> \n> diff --git a/.gitignore b/.gitignore\n> index cca829fa..aae56b2d 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -3,6 +3,7 @@\n> __pycache__/\n> build/\n> patches/\n> +subprojects/pybind11-2.*/\n> *.patch\n> *.pyc\n> .cache\n> diff --git a/meson.build b/meson.build\n> index a20cc29e..0f885c14 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -181,6 +181,7 @@ summary({\n> 'qcam application': qcam_enabled,\n> 'lc-compliance application': lc_compliance_enabled,\n> 'Unit tests': test_enabled,\n> + 'Python bindings': pycamera_enabled,\n> },\n> section : 'Configuration',\n> bool_yn : true)\n> diff --git a/meson_options.txt b/meson_options.txt\n> index 2c80ad8b..ca00c78e 100644\n> --- a/meson_options.txt\n> +++ b/meson_options.txt\n> @@ -58,3 +58,8 @@ option('v4l2',\n> type : 'boolean',\n> value : false,\n> description : 'Compile the V4L2 compatibility layer')\n> +\n> +option('pycamera',\n> + type : 'feature',\n> + value : 'auto',\n> + description : 'Enable libcamera Python bindings (experimental)')\n> diff --git a/src/meson.build b/src/meson.build\n> index e0ea9c35..4a1e3cb0 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -38,3 +38,4 @@ subdir('qcam')\n> \n> subdir('gstreamer')\n> subdir('v4l2')\n> +subdir('py')\n> diff --git a/src/py/meson.build b/src/py/meson.build\n> new file mode 100644\n> index 00000000..42ffa221\n> --- /dev/null\n> +++ b/src/py/meson.build\n> @@ -0,0 +1 @@\n> +subdir('pycamera')\n> diff --git a/src/py/pycamera/__init__.py b/src/py/pycamera/__init__.py\n> new file mode 100644\n> index 00000000..a5c198dc\n> --- /dev/null\n> +++ b/src/py/pycamera/__init__.py\n> @@ -0,0 +1,10 @@\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +# Copyright (C) 2021, Tomi Valkeinen \n> +\n> +from .pycamera import *\n> +import mmap\n> +\n> +def __FrameBuffer__mmap(self, plane):\n> + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)\n> +\n> +FrameBuffer.mmap = __FrameBuffer__mmap\n> diff --git a/src/py/pycamera/meson.build b/src/py/pycamera/meson.build\n> new file mode 100644\n> index 00000000..c490a18d\n> --- /dev/null\n> +++ b/src/py/pycamera/meson.build\n> @@ -0,0 +1,43 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +py3_dep = dependency('python3', required : get_option('pycamera'))\n> +\n> +if not py3_dep.found()\n> + pycamera_enabled = false\n> + subdir_done()\n> +endif\n> +\n> +pycamera_enabled = true\n> +\n> +pybind11_proj = subproject('pybind11')\n> +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')\n> +\n> +pycamera_sources = files([\n> + 'pymain.cpp',\n> +])\n> +\n> +pycamera_deps = [\n> + libcamera_public,\n> + py3_dep,\n> + pybind11_dep,\n> +]\n> +\n> +pycamera_args = [ '-fvisibility=hidden' ]\n> +pycamera_args += [ '-Wno-shadow' ]\n\nDoes python shadow variables explicitly? Does this lead to any potential\nbugs?\n\nI recall putting -Wshadow in explicitly because I hit a nasty bug due to\nshadowing, so in my eyes, shadowed variables are a bad-thing (tm) :-)\n\nYou have to be sure you know which variable you are writing to,\nand it might not always be clear to a reader, or cause potential\nconfusion..?\n\n(of course subclassing, and having functions with the same name is\nessentially 'shadowing' the function names I guess...)\n\n\n> +\n> +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/pycamera'\n> +\n> +pycamera = shared_module('pycamera',\n> + pycamera_sources,\n> + install : true,\n> + install_dir : destdir,\n> + name_prefix : '',\n> + dependencies : pycamera_deps,\n> + cpp_args : pycamera_args)\n> +\n> +# XXX > You could also create a symlink. There's an example in the top-level\n> +# > meson.build.\n> +# Copy __init__.py to build dir so that we can run without installing\n> +configure_file(input: '__init__.py', output: '__init__.py', copy: true)\n\nI think a symlink would be better too. \n\n> +\n> +install_data(['__init__.py'], install_dir : destdir)\n> diff --git a/src/py/pycamera/pymain.cpp b/src/py/pycamera/pymain.cpp\n> new file mode 100644\n> index 00000000..0088e133\n> --- /dev/null\n> +++ b/src/py/pycamera/pymain.cpp\n> @@ -0,0 +1,424 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Tomi Valkeinen \n> + *\n> + * Python bindings\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +namespace py = pybind11;\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +static py::object ControlValueToPy(const ControlValue &cv)\n> +{\n> + //assert(!cv.isArray());\n> + //assert(cv.numElements() == 1);\n\nAre these asserts not necessary? Is it better to keep them in, in a way\nthat they are used on debug buidls and compiled out on release builds?\n\nI think we have ASSERT() for that, but that's an 'internal' helper I\nthink and I expect the src/py is building on top of the public api..\n\n\n> +\n> + switch (cv.type()) {\n> + case ControlTypeBool:\n> + return py::cast(cv.get());\n> + case ControlTypeByte:\n> + return py::cast(cv.get());\n> + case ControlTypeInteger32:\n> + return py::cast(cv.get());\n> + case ControlTypeInteger64:\n> + return py::cast(cv.get());\n> + case ControlTypeFloat:\n> + return py::cast(cv.get());\n> + case ControlTypeString:\n> + return py::cast(cv.get());\n> + case ControlTypeRectangle: {\n> + const Rectangle *v = reinterpret_cast(cv.data().data());\n> + return py::make_tuple(v->x, v->y, v->width, v->height);\n> + }\n> + case ControlTypeSize: {\n> + const Size *v = reinterpret_cast(cv.data().data());\n> + return py::make_tuple(v->width, v->height);\n> + }\n> + case ControlTypeNone:\n> + default:\n> + throw runtime_error(\"Unsupported ControlValue type\");\n> + }\n> +}\n> +\n> +static ControlValue PyToControlValue(const py::object &ob, ControlType type)\n> +{\n> + switch (type) {\n> + case ControlTypeBool:\n> + return ControlValue(ob.cast());\n> + case ControlTypeByte:\n> + return ControlValue(ob.cast());\n> + case ControlTypeInteger32:\n> + return ControlValue(ob.cast());\n> + case ControlTypeInteger64:\n> + return ControlValue(ob.cast());\n> + case ControlTypeFloat:\n> + return ControlValue(ob.cast());\n> + case ControlTypeString:\n> + return ControlValue(ob.cast());\n> + case ControlTypeRectangle:\n> + case ControlTypeSize:\n> + case ControlTypeNone:\n> + default:\n> + throw runtime_error(\"Control type not implemented\");\n> + }\n> +}\n> +\n> +static weak_ptr g_camera_manager;\n> +static int g_eventfd;\n> +static mutex g_reqlist_mutex;\n> +static vector g_reqlist;\n> +\n> +static void handle_request_completed(Request *req)\n> +{\n> + {\n> + lock_guard guard(g_reqlist_mutex);\n> + g_reqlist.push_back(req);\n> + }\n> +\n> + uint64_t v = 1;\n> + write(g_eventfd, &v, 8);\n> +}\n> +\n> +PYBIND11_MODULE(pycamera, m)\n> +{\n> + m.def(\"logSetLevel\", &logSetLevel);\n> +\n> + py::class_>(m, \"CameraManager\")\n> + .def_static(\"singleton\", []() {\n> + shared_ptr cm = g_camera_manager.lock();\n> + if (cm)\n> + return cm;\n> +\n> + int fd = eventfd(0, 0);\n> + if (fd == -1)\n> + throw std::system_error(errno, std::generic_category(), \"Failed to create eventfd\");\n> +\n> + cm = shared_ptr(new CameraManager, [](auto p) {\n> + close(g_eventfd);\n> + g_eventfd = -1;\n> + delete p;\n> + });\n> +\n> + g_eventfd = fd;\n> + g_camera_manager = cm;\n> +\n> + int ret = cm->start();\n> + if (ret)\n> + throw std::system_error(-ret, std::generic_category(), \"Failed to start CameraManager\");\n> +\n> + return cm;\n> + })\n> +\n> + .def_property_readonly(\"version\", &CameraManager::version)\n> +\n> + .def_property_readonly(\"efd\", [](CameraManager &) {\n> + return g_eventfd;\n> + })\n> +\n> + .def(\"getReadyRequests\", [](CameraManager &) {\n> + vector v;\n> +\n> + {\n> + lock_guard guard(g_reqlist_mutex);\n> + swap(v, g_reqlist);\n> + }\n> +\n> + vector ret;\n> +\n> + for (Request *req : v) {\n> + py::object o = py::cast(req);\n> + // decrease the ref increased in Camera::queueRequest()\n> + o.dec_ref();\n> + ret.push_back(o);\n> + }\n> +\n> + return ret;\n> + })\n> +\n> + .def(\"get\", py::overload_cast(&CameraManager::get), py::keep_alive<0, 1>())\n> +\n> + .def(\"find\", [](CameraManager &self, string str) {\n> + std::transform(str.begin(), str.end(), str.begin(), ::tolower);\n> +\n> + for (auto c : self.cameras()) {\n> + string id = c->id();\n> +\n> + std::transform(id.begin(), id.end(), id.begin(), ::tolower);\n> +\n> + if (id.find(str) != string::npos)\n> + return c;\n> + }\n> +\n> + return shared_ptr();\n> + }, py::keep_alive<0, 1>())\n> +\n> + // Create a list of Cameras, where each camera has a keep-alive to CameraManager\n> + .def_property_readonly(\"cameras\", [](CameraManager &self) {\n> + py::list l;\n> +\n> + for (auto &c : self.cameras()) {\n> + py::object py_cm = py::cast(self);\n> + py::object py_cam = py::cast(c);\n> + py::detail::keep_alive_impl(py_cam, py_cm);\n> + l.append(py_cam);\n> + }\n> +\n> + return l;\n> + });\n> +\n> + py::class_>(m, \"Camera\")\n> + .def_property_readonly(\"id\", &Camera::id)\n> + .def(\"acquire\", &Camera::acquire)\n> + .def(\"release\", &Camera::release)\n> + .def(\"start\", [](shared_ptr &self) {\n> + self->requestCompleted.connect(handle_request_completed);\n> +\n> + int ret = self->start();\n> + if (ret)\n> + self->requestCompleted.disconnect(handle_request_completed);\n> +\n> + return ret;\n> + })\n> +\n> + .def(\"stop\", [](shared_ptr &self) {\n> + int ret = self->stop();\n> + if (!ret)\n> + self->requestCompleted.disconnect(handle_request_completed);\n> +\n> + return ret;\n> + })\n> +\n> + .def(\"__repr__\", [](shared_ptr &self) {\n> + return \"id() + \"'>\";\n> + })\n> +\n> + // Keep the camera alive, as StreamConfiguration contains a Stream*\n> + .def(\"generateConfiguration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> + .def(\"configure\", &Camera::configure)\n> +\n> + .def(\"createRequest\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> +\n> + .def(\"queueRequest\", [](Camera &self, Request *req) {\n> + py::object py_req = py::cast(req);\n> +\n> + py_req.inc_ref();\n> +\n> + int ret = self.queueRequest(req);\n> + if (ret)\n> + py_req.dec_ref();\n> +\n> + return ret;\n> + })\n> +\n> + .def_property_readonly(\"streams\", [](Camera &self) {\n> + py::set set;\n> + for (auto &s : self.streams()) {\n> + py::object py_self = py::cast(self);\n> + py::object py_s = py::cast(s);\n> + py::detail::keep_alive_impl(py_s, py_self);\n> + set.add(py_s);\n> + }\n> + return set;\n> + })\n> +\n> + .def_property_readonly(\"controls\", [](Camera &self) {\n> + py::dict ret;\n> +\n> + for (const auto &[id, ci] : self.controls()) {\n> + ret[id->name().c_str()] = make_tuple(ControlValueToPy(ci.min()),\n> + ControlValueToPy(ci.max()),\n> + ControlValueToPy(ci.def()));\n> + }\n> +\n> + return ret;\n> + })\n> +\n> + .def_property_readonly(\"properties\", [](Camera &self) {\n> + py::dict ret;\n> +\n> + for (const auto &[key, cv] : self.properties()) {\n> + const ControlId *id = properties::properties.at(key);\n> + py::object ob = ControlValueToPy(cv);\n> +\n> + ret[id->name().c_str()] = ob;\n> + }\n> +\n> + return ret;\n> + });\n> +\n> + py::enum_(m, \"ConfigurationStatus\")\n> + .value(\"Valid\", CameraConfiguration::Valid)\n> + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n> + .value(\"Invalid\", CameraConfiguration::Invalid);\n> +\n> + py::class_(m, \"CameraConfiguration\")\n> + .def(\"__iter__\", [](CameraConfiguration &self) {\n> + return py::make_iterator(self);\n> + }, py::keep_alive<0, 1>())\n> + .def(\"__len__\", [](CameraConfiguration &self) {\n> + return self.size();\n> + })\n> + .def(\"validate\", &CameraConfiguration::validate)\n> + .def(\"at\", py::overload_cast(&CameraConfiguration::at), py::return_value_policy::reference_internal)\n> + .def_property_readonly(\"size\", &CameraConfiguration::size)\n> + .def_property_readonly(\"empty\", &CameraConfiguration::empty);\n> +\n> + py::class_(m, \"StreamConfiguration\")\n> + .def(\"toString\", &StreamConfiguration::toString)\n> + .def_property_readonly(\"stream\", &StreamConfiguration::stream, py::return_value_policy::reference_internal)\n> + .def_property(\n> + \"size\",\n> + [](StreamConfiguration &self) { return make_tuple(self.size.width, self.size.height); },\n> + [](StreamConfiguration &self, tuple size) { self.size.width = get<0>(size); self.size.height = get<1>(size); })\n> + .def_property(\n> + \"fmt\",\n> + [](StreamConfiguration &self) { return self.pixelFormat.toString(); },\n> + [](StreamConfiguration &self, string fmt) { self.pixelFormat = PixelFormat::fromString(fmt); })\n> + .def_readwrite(\"stride\", &StreamConfiguration::stride)\n> + .def_readwrite(\"frameSize\", &StreamConfiguration::frameSize)\n> + .def_readwrite(\"bufferCount\", &StreamConfiguration::bufferCount)\n> + .def_property_readonly(\"formats\", &StreamConfiguration::formats, py::return_value_policy::reference_internal);\n> + ;\n> +\n> + py::class_(m, \"StreamFormats\")\n> + .def_property_readonly(\"pixelFormats\", [](StreamFormats &self) {\n> + vector fmts;\n> + for (auto &fmt : self.pixelformats())\n> + fmts.push_back(fmt.toString());\n> + return fmts;\n> + })\n> + .def(\"sizes\", [](StreamFormats &self, const string &pixelFormat) {\n> + auto fmt = PixelFormat::fromString(pixelFormat);\n> + vector> fmts;\n> + for (const auto &s : self.sizes(fmt))\n> + fmts.push_back(make_tuple(s.width, s.height));\n> + return fmts;\n> + })\n> + .def(\"range\", [](StreamFormats &self, const string &pixelFormat) {\n> + auto fmt = PixelFormat::fromString(pixelFormat);\n> + const auto &range = self.range(fmt);\n> + return make_tuple(make_tuple(range.hStep, range.vStep),\n> + make_tuple(range.min.width, range.min.height),\n> + make_tuple(range.max.width, range.max.height));\n> + });\n> +\n> + py::enum_(m, \"StreamRole\")\n> + .value(\"StillCapture\", StreamRole::StillCapture)\n> + .value(\"Raw\", StreamRole::Raw)\n> + .value(\"VideoRecording\", StreamRole::VideoRecording)\n> + .value(\"Viewfinder\", StreamRole::Viewfinder);\n> +\n> + py::class_(m, \"FrameBufferAllocator\")\n> + .def(py::init>(), py::keep_alive<1, 2>())\n> + .def(\"allocate\", &FrameBufferAllocator::allocate)\n> + .def_property_readonly(\"allocated\", &FrameBufferAllocator::allocated)\n> + // Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator\n> + .def(\"buffers\", [](FrameBufferAllocator &self, Stream *stream) {\n> + py::object py_self = py::cast(self);\n> + py::list l;\n> + for (auto &ub : self.buffers(stream)) {\n> + py::object py_buf = py::cast(ub.get(), py::return_value_policy::reference_internal, py_self);\n> + l.append(py_buf);\n> + }\n> + return l;\n> + });\n> +\n> + py::class_(m, \"FrameBuffer\")\n> + // TODO: implement FrameBuffer::Plane properly\n> + .def(py::init([](vector> planes, unsigned int cookie) {\n> + vector v;\n> + for (const auto &t : planes)\n> + v.push_back({ SharedFD(get<0>(t)), FrameBuffer::Plane::kInvalidOffset, get<1>(t) });\n> + return new FrameBuffer(v, cookie);\n> + }))\n> + .def_property_readonly(\"metadata\", &FrameBuffer::metadata, py::return_value_policy::reference_internal)\n> + .def(\"length\", [](FrameBuffer &self, uint32_t idx) {\n> + const FrameBuffer::Plane &plane = self.planes()[idx];\n> + return plane.length;\n> + })\n> + .def(\"fd\", [](FrameBuffer &self, uint32_t idx) {\n> + const FrameBuffer::Plane &plane = self.planes()[idx];\n> + return plane.fd.get();\n> + })\n> + .def_property(\"cookie\", &FrameBuffer::cookie, &FrameBuffer::setCookie);\n> +\n> + py::class_(m, \"Stream\")\n> + .def_property_readonly(\"configuration\", &Stream::configuration);\n> +\n> + py::enum_(m, \"ReuseFlag\")\n> + .value(\"Default\", Request::ReuseFlag::Default)\n> + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n> +\n> + py::class_(m, \"Request\")\n> + .def_property_readonly(\"camera\", &Request::camera)\n> + .def(\"addBuffer\", &Request::addBuffer, py::keep_alive<1, 3>()) // Request keeps Framebuffer alive\n> + .def_property_readonly(\"status\", &Request::status)\n> + .def_property_readonly(\"buffers\", &Request::buffers)\n> + .def_property_readonly(\"cookie\", &Request::cookie)\n> + .def_property_readonly(\"hasPendingBuffers\", &Request::hasPendingBuffers)\n> + .def(\"set_control\", [](Request &self, string &control, py::object value) {\n> + const auto &controls = self.camera()->controls();\n> +\n> + auto it = find_if(controls.begin(), controls.end(),\n> + [&control](const auto &kvp) { return kvp.first->name() == control; });\n> +\n> + if (it == controls.end())\n> + throw runtime_error(\"Control not found\");\n> +\n> + const auto &id = it->first;\n> +\n> + self.controls().set(id->id(), PyToControlValue(value, id->type()));\n> + })\n> + .def_property_readonly(\"metadata\", [](Request &self) {\n> + py::dict ret;\n> +\n> + for (const auto &[key, cv] : self.metadata()) {\n> + const ControlId *id = controls::controls.at(key);\n> + py::object ob = ControlValueToPy(cv);\n> +\n> + ret[id->name().c_str()] = ob;\n> + }\n> +\n> + return ret;\n> + })\n> + // As we add a keep_alive to the fb in addBuffers(), we can only allow reuse with ReuseBuffers.\n> + .def(\"reuse\", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });\n> +\n> + py::enum_(m, \"RequestStatus\")\n> + .value(\"Pending\", Request::RequestPending)\n> + .value(\"Complete\", Request::RequestComplete)\n> + .value(\"Cancelled\", Request::RequestCancelled);\n> +\n> + py::enum_(m, \"FrameMetadataStatus\")\n> + .value(\"Success\", FrameMetadata::FrameSuccess)\n> + .value(\"Error\", FrameMetadata::FrameError)\n> + .value(\"Cancelled\", FrameMetadata::FrameCancelled);\n> +\n> + py::class_(m, \"FrameMetadata\")\n> + .def_readonly(\"status\", &FrameMetadata::status)\n> + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n> + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n> + .def_property_readonly(\"bytesused\", [](FrameMetadata &self) {\n> + vector v;\n> + v.resize(self.planes().size());\n> + transform(self.planes().begin(), self.planes().end(), v.begin(), [](const auto &p) { return p.bytesused; });\n> + return v;\n> + });\n> +}\n> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> new file mode 100644\n> index 00000000..9d6e7acb\n> --- /dev/null\n> +++ b/subprojects/pybind11.wrap\n> @@ -0,0 +1,12 @@\n> +[wrap-file]\n> +directory = pybind11-2.6.1\n> +source_url = https://github.com/pybind/pybind11/archive/v2.6.1.tar.gz\n> +source_filename = pybind11-2.6.1.tar.gz\n> +source_hash = cdbe326d357f18b83d10322ba202d69f11b2f49e2d87ade0dc2be0c5c34f8e2a\n> +patch_url = https://wrapdb.mesonbuild.com/v2/pybind11_2.6.1-1/get_patch\n> +patch_filename = pybind11-2.6.1-1-wrap.zip\n> +patch_hash = 6de5477598b56c8a2e609196420c783ac35b79a31d6622121602e6ade6b3cee8\n> +\n> +[provide]\n> +pybind11 = pybind11_dep\n> +\n> -- \n> 2.25.1\n>","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 D9374BDB13\n\tfor ;\n\tThu, 9 Dec 2021 09:58:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08EED60882;\n\tThu, 9 Dec 2021 10:58:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A4E7360224\n\tfor ;\n\tThu, 9 Dec 2021 10:58:13 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 37EDD5B0;\n\tThu, 9 Dec 2021 10:58:13 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L+yNyclK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639043893;\n\tbh=I+ijOIAz8yCFxLQYC1CPFsv3viZX0vaMR7i/P0CXKuc=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=L+yNyclKbNKkN7S/jwQIgr5wKrKX61TW6JJctXz8ut8OKqpz4kFBbRXyYHNc14BFi\n\tS51igfxqNmJUdqOq+040s3Kr1ZXj/b0iUvxEFouhuTWnw9giP9mBhXxyfj7MJfELr9\n\tqcPer2+BFoZWadRoNmWDnZeVCdtEUsNgznvRaQ8g=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>","References":"<20211209092906.37303-1-tomi.valkeinen@ideasonboard.com>\n\t<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>","From":"Kieran Bingham ","To":"Tomi Valkeinen ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 09 Dec 2021 09:58:10 +0000","Message-ID":"<163904389093.2066819.16202667063304401995@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21707,"web_url":"https://patchwork.libcamera.org/comment/21707/","msgid":"<078365ca-da00-6d70-77cd-431dbaf04d8e@ideasonboard.com>","date":"2021-12-09T11:10:01","subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 09/12/2021 11:58, Kieran Bingham wrote:\n\n>> +pycamera_args = [ '-fvisibility=hidden' ]\n>> +pycamera_args += [ '-Wno-shadow' ]\n> \n> Does python shadow variables explicitly? Does this lead to any potential\n> bugs?\n\nYes, it has many cases of:\n\nclass Foo\n{\n\tint bar;\n\n\tFoo(int bar)\n\t\t: bar(bar)\n\t{ }\n}\n\n Tomi","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 EDAFBBDB13\n\tfor ;\n\tThu, 9 Dec 2021 11:10:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 256656087A;\n\tThu, 9 Dec 2021 12:10:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D9E30607DE\n\tfor ;\n\tThu, 9 Dec 2021 12:10:04 +0100 (CET)","from [192.168.1.111] (91-156-85-209.elisa-laajakaista.fi\n\t[91.156.85.209])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5CC42501;\n\tThu, 9 Dec 2021 12:10:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"M6yVPvU5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639048204;\n\tbh=bYadNX4T/1ZfY+QFfYN7P9NmuCUKSzC6omdEWCY2S08=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=M6yVPvU5Td/ryzV7PonD8qVnNkzvDvsjjWExKS3uuLdizYphVdcUeo9bHvZoAra88\n\t5oV16lVNPrVQeQ6rxRWghZJprwZD6H0yZMPeW3YHKGKLbp14xGc6sNE4OfYzj/+YqH\n\tjvFIHVB1TwDSK5On+ZDfw8ILunNQT5krbIsaAPuk=","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211209092906.37303-1-tomi.valkeinen@ideasonboard.com>\n\t<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>\n\t<163904389093.2066819.16202667063304401995@Monstersaurus>","From":"Tomi Valkeinen ","Message-ID":"<078365ca-da00-6d70-77cd-431dbaf04d8e@ideasonboard.com>","Date":"Thu, 9 Dec 2021 13:10:01 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.14.0","MIME-Version":"1.0","In-Reply-To":"<163904389093.2066819.16202667063304401995@Monstersaurus>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21708,"web_url":"https://patchwork.libcamera.org/comment/21708/","msgid":"","date":"2021-12-09T11:16:17","subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Tomi\n\nThanks for this patch!\n\nOn Thu, 9 Dec 2021 at 09:58, Kieran Bingham\n wrote:\n>\n> Quoting Tomi Valkeinen (2021-12-09 09:29:05)\n> > Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n> > Python layer.\n> >\n> > Only a subset of libcamera classes are exposed.\n> >\n> > Signed-off-by: Tomi Valkeinen \n> > ---\n> > .gitignore | 1 +\n> > meson.build | 1 +\n> > meson_options.txt | 5 +\n> > src/meson.build | 1 +\n> > src/py/meson.build | 1 +\n> > src/py/pycamera/__init__.py | 10 +\n> > src/py/pycamera/meson.build | 43 ++++\n> > src/py/pycamera/pymain.cpp | 424 ++++++++++++++++++++++++++++++++++++\n> > subprojects/pybind11.wrap | 12 +\n> > 9 files changed, 498 insertions(+)\n> > create mode 100644 src/py/meson.build\n> > create mode 100644 src/py/pycamera/__init__.py\n> > create mode 100644 src/py/pycamera/meson.build\n> > create mode 100644 src/py/pycamera/pymain.cpp\n> > create mode 100644 subprojects/pybind11.wrap\n> >\n> > diff --git a/.gitignore b/.gitignore\n> > index cca829fa..aae56b2d 100644\n> > --- a/.gitignore\n> > +++ b/.gitignore\n> > @@ -3,6 +3,7 @@\n> > __pycache__/\n> > build/\n> > patches/\n> > +subprojects/pybind11-2.*/\n> > *.patch\n> > *.pyc\n> > .cache\n> > diff --git a/meson.build b/meson.build\n> > index a20cc29e..0f885c14 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -181,6 +181,7 @@ summary({\n> > 'qcam application': qcam_enabled,\n> > 'lc-compliance application': lc_compliance_enabled,\n> > 'Unit tests': test_enabled,\n> > + 'Python bindings': pycamera_enabled,\n> > },\n> > section : 'Configuration',\n> > bool_yn : true)\n> > diff --git a/meson_options.txt b/meson_options.txt\n> > index 2c80ad8b..ca00c78e 100644\n> > --- a/meson_options.txt\n> > +++ b/meson_options.txt\n> > @@ -58,3 +58,8 @@ option('v4l2',\n> > type : 'boolean',\n> > value : false,\n> > description : 'Compile the V4L2 compatibility layer')\n> > +\n> > +option('pycamera',\n> > + type : 'feature',\n> > + value : 'auto',\n> > + description : 'Enable libcamera Python bindings (experimental)')\n> > diff --git a/src/meson.build b/src/meson.build\n> > index e0ea9c35..4a1e3cb0 100644\n> > --- a/src/meson.build\n> > +++ b/src/meson.build\n> > @@ -38,3 +38,4 @@ subdir('qcam')\n> >\n> > subdir('gstreamer')\n> > subdir('v4l2')\n> > +subdir('py')\n> > diff --git a/src/py/meson.build b/src/py/meson.build\n> > new file mode 100644\n> > index 00000000..42ffa221\n> > --- /dev/null\n> > +++ b/src/py/meson.build\n> > @@ -0,0 +1 @@\n> > +subdir('pycamera')\n> > diff --git a/src/py/pycamera/__init__.py b/src/py/pycamera/__init__.py\n> > new file mode 100644\n> > index 00000000..a5c198dc\n> > --- /dev/null\n> > +++ b/src/py/pycamera/__init__.py\n> > @@ -0,0 +1,10 @@\n> > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > +# Copyright (C) 2021, Tomi Valkeinen \n> > +\n> > +from .pycamera import *\n> > +import mmap\n> > +\n> > +def __FrameBuffer__mmap(self, plane):\n> > + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)\n> > +\n> > +FrameBuffer.mmap = __FrameBuffer__mmap\n> > diff --git a/src/py/pycamera/meson.build b/src/py/pycamera/meson.build\n> > new file mode 100644\n> > index 00000000..c490a18d\n> > --- /dev/null\n> > +++ b/src/py/pycamera/meson.build\n> > @@ -0,0 +1,43 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +py3_dep = dependency('python3', required : get_option('pycamera'))\n> > +\n> > +if not py3_dep.found()\n> > + pycamera_enabled = false\n> > + subdir_done()\n> > +endif\n> > +\n> > +pycamera_enabled = true\n> > +\n> > +pybind11_proj = subproject('pybind11')\n> > +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')\n> > +\n> > +pycamera_sources = files([\n> > + 'pymain.cpp',\n> > +])\n> > +\n> > +pycamera_deps = [\n> > + libcamera_public,\n> > + py3_dep,\n> > + pybind11_dep,\n> > +]\n> > +\n> > +pycamera_args = [ '-fvisibility=hidden' ]\n> > +pycamera_args += [ '-Wno-shadow' ]\n>\n> Does python shadow variables explicitly? Does this lead to any potential\n> bugs?\n>\n> I recall putting -Wshadow in explicitly because I hit a nasty bug due to\n> shadowing, so in my eyes, shadowed variables are a bad-thing (tm) :-)\n>\n> You have to be sure you know which variable you are writing to,\n> and it might not always be clear to a reader, or cause potential\n> confusion..?\n>\n> (of course subclassing, and having functions with the same name is\n> essentially 'shadowing' the function names I guess...)\n>\n>\n> > +\n> > +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/pycamera'\n> > +\n> > +pycamera = shared_module('pycamera',\n> > + pycamera_sources,\n> > + install : true,\n> > + install_dir : destdir,\n> > + name_prefix : '',\n> > + dependencies : pycamera_deps,\n> > + cpp_args : pycamera_args)\n> > +\n> > +# XXX > You could also create a symlink. There's an example in the top-level\n> > +# > meson.build.\n> > +# Copy __init__.py to build dir so that we can run without installing\n> > +configure_file(input: '__init__.py', output: '__init__.py', copy: true)\n>\n> I think a symlink would be better too.\n>\n> > +\n> > +install_data(['__init__.py'], install_dir : destdir)\n> > diff --git a/src/py/pycamera/pymain.cpp b/src/py/pycamera/pymain.cpp\n> > new file mode 100644\n> > index 00000000..0088e133\n> > --- /dev/null\n> > +++ b/src/py/pycamera/pymain.cpp\n\npylibcamera?? :)\n\n> > @@ -0,0 +1,424 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Tomi Valkeinen \n> > + *\n> > + * Python bindings\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace py = pybind11;\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +static py::object ControlValueToPy(const ControlValue &cv)\n> > +{\n> > + //assert(!cv.isArray());\n> > + //assert(cv.numElements() == 1);\n>\n> Are these asserts not necessary? Is it better to keep them in, in a way\n> that they are used on debug buidls and compiled out on release builds?\n>\n> I think we have ASSERT() for that, but that's an 'internal' helper I\n> think and I expect the src/py is building on top of the public api..\n>\n>\n> > +\n> > + switch (cv.type()) {\n> > + case ControlTypeBool:\n> > + return py::cast(cv.get());\n> > + case ControlTypeByte:\n> > + return py::cast(cv.get());\n> > + case ControlTypeInteger32:\n> > + return py::cast(cv.get());\n> > + case ControlTypeInteger64:\n> > + return py::cast(cv.get());\n> > + case ControlTypeFloat:\n> > + return py::cast(cv.get());\n> > + case ControlTypeString:\n> > + return py::cast(cv.get());\n> > + case ControlTypeRectangle: {\n> > + const Rectangle *v = reinterpret_cast(cv.data().data());\n> > + return py::make_tuple(v->x, v->y, v->width, v->height);\n> > + }\n> > + case ControlTypeSize: {\n> > + const Size *v = reinterpret_cast(cv.data().data());\n> > + return py::make_tuple(v->width, v->height);\n> > + }\n> > + case ControlTypeNone:\n> > + default:\n> > + throw runtime_error(\"Unsupported ControlValue type\");\n> > + }\n> > +}\n> > +\n> > +static ControlValue PyToControlValue(const py::object &ob, ControlType type)\n> > +{\n> > + switch (type) {\n> > + case ControlTypeBool:\n> > + return ControlValue(ob.cast());\n> > + case ControlTypeByte:\n> > + return ControlValue(ob.cast());\n> > + case ControlTypeInteger32:\n> > + return ControlValue(ob.cast());\n> > + case ControlTypeInteger64:\n> > + return ControlValue(ob.cast());\n> > + case ControlTypeFloat:\n> > + return ControlValue(ob.cast());\n> > + case ControlTypeString:\n> > + return ControlValue(ob.cast());\n> > + case ControlTypeRectangle:\n> > + case ControlTypeSize:\n> > + case ControlTypeNone:\n> > + default:\n> > + throw runtime_error(\"Control type not implemented\");\n> > + }\n> > +}\n\nI wonder whether we should be supporting array controls even in the\ninitial version? Some of the other things (like transforms) simply\nmanifest themselves as \"features that you don't get\", but I found that\nany attempts to use metadata basically resulted in failures until I\nbodged the array controls to work.\n\nWhat do you thinK?\n\n> > +\n> > +static weak_ptr g_camera_manager;\n> > +static int g_eventfd;\n> > +static mutex g_reqlist_mutex;\n> > +static vector g_reqlist;\n> > +\n> > +static void handle_request_completed(Request *req)\n> > +{\n> > + {\n> > + lock_guard guard(g_reqlist_mutex);\n> > + g_reqlist.push_back(req);\n> > + }\n> > +\n> > + uint64_t v = 1;\n> > + write(g_eventfd, &v, 8);\n> > +}\n> > +\n> > +PYBIND11_MODULE(pycamera, m)\n> > +{\n> > + m.def(\"logSetLevel\", &logSetLevel);\n> > +\n> > + py::class_>(m, \"CameraManager\")\n> > + .def_static(\"singleton\", []() {\n> > + shared_ptr cm = g_camera_manager.lock();\n> > + if (cm)\n> > + return cm;\n> > +\n> > + int fd = eventfd(0, 0);\n> > + if (fd == -1)\n> > + throw std::system_error(errno, std::generic_category(), \"Failed to create eventfd\");\n> > +\n> > + cm = shared_ptr(new CameraManager, [](auto p) {\n> > + close(g_eventfd);\n> > + g_eventfd = -1;\n> > + delete p;\n> > + });\n> > +\n> > + g_eventfd = fd;\n> > + g_camera_manager = cm;\n> > +\n> > + int ret = cm->start();\n> > + if (ret)\n> > + throw std::system_error(-ret, std::generic_category(), \"Failed to start CameraManager\");\n> > +\n> > + return cm;\n> > + })\n> > +\n> > + .def_property_readonly(\"version\", &CameraManager::version)\n> > +\n> > + .def_property_readonly(\"efd\", [](CameraManager &) {\n> > + return g_eventfd;\n> > + })\n> > +\n> > + .def(\"getReadyRequests\", [](CameraManager &) {\n> > + vector v;\n> > +\n> > + {\n> > + lock_guard guard(g_reqlist_mutex);\n> > + swap(v, g_reqlist);\n> > + }\n> > +\n> > + vector ret;\n> > +\n> > + for (Request *req : v) {\n> > + py::object o = py::cast(req);\n> > + // decrease the ref increased in Camera::queueRequest()\n> > + o.dec_ref();\n> > + ret.push_back(o);\n> > + }\n> > +\n> > + return ret;\n> > + })\n> > +\n> > + .def(\"get\", py::overload_cast(&CameraManager::get), py::keep_alive<0, 1>())\n> > +\n> > + .def(\"find\", [](CameraManager &self, string str) {\n> > + std::transform(str.begin(), str.end(), str.begin(), ::tolower);\n> > +\n> > + for (auto c : self.cameras()) {\n> > + string id = c->id();\n> > +\n> > + std::transform(id.begin(), id.end(), id.begin(), ::tolower);\n> > +\n> > + if (id.find(str) != string::npos)\n> > + return c;\n> > + }\n> > +\n> > + return shared_ptr();\n> > + }, py::keep_alive<0, 1>())\n> > +\n> > + // Create a list of Cameras, where each camera has a keep-alive to CameraManager\n> > + .def_property_readonly(\"cameras\", [](CameraManager &self) {\n> > + py::list l;\n> > +\n> > + for (auto &c : self.cameras()) {\n> > + py::object py_cm = py::cast(self);\n> > + py::object py_cam = py::cast(c);\n> > + py::detail::keep_alive_impl(py_cam, py_cm);\n> > + l.append(py_cam);\n> > + }\n> > +\n> > + return l;\n> > + });\n> > +\n> > + py::class_>(m, \"Camera\")\n> > + .def_property_readonly(\"id\", &Camera::id)\n> > + .def(\"acquire\", &Camera::acquire)\n> > + .def(\"release\", &Camera::release)\n> > + .def(\"start\", [](shared_ptr &self) {\n\nThis is the one that needs to take a dictionary of controls, but we\ncan add that later!\n\n> > + self->requestCompleted.connect(handle_request_completed);\n> > +\n> > + int ret = self->start();\n> > + if (ret)\n> > + self->requestCompleted.disconnect(handle_request_completed);\n> > +\n> > + return ret;\n> > + })\n> > +\n> > + .def(\"stop\", [](shared_ptr &self) {\n> > + int ret = self->stop();\n> > + if (!ret)\n> > + self->requestCompleted.disconnect(handle_request_completed);\n> > +\n> > + return ret;\n> > + })\n> > +\n> > + .def(\"__repr__\", [](shared_ptr &self) {\n> > + return \"id() + \"'>\";\n> > + })\n> > +\n> > + // Keep the camera alive, as StreamConfiguration contains a Stream*\n> > + .def(\"generateConfiguration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n\nI was sometimes finding generateConfiguration a bit awkward because\nit's the only way to create a CameraConfiguration (I think, maybe I'm\nwrong?), and it always needs a list of stream roles. So I seem to\nspend a lot of my time converting Python dicts (what the user sees) to\nCameraConfigurations and vice versa. I guess I'm not sure what I'm\nasking for, perhaps I just need to try and tidy up my translation code\nwhich has become a bit too twisty.\n\n> > + .def(\"configure\", &Camera::configure)\n> > +\n> > + .def(\"createRequest\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> > +\n> > + .def(\"queueRequest\", [](Camera &self, Request *req) {\n> > + py::object py_req = py::cast(req);\n> > +\n> > + py_req.inc_ref();\n> > +\n> > + int ret = self.queueRequest(req);\n> > + if (ret)\n> > + py_req.dec_ref();\n> > +\n> > + return ret;\n> > + })\n> > +\n> > + .def_property_readonly(\"streams\", [](Camera &self) {\n> > + py::set set;\n> > + for (auto &s : self.streams()) {\n> > + py::object py_self = py::cast(self);\n> > + py::object py_s = py::cast(s);\n> > + py::detail::keep_alive_impl(py_s, py_self);\n> > + set.add(py_s);\n> > + }\n> > + return set;\n> > + })\n> > +\n> > + .def_property_readonly(\"controls\", [](Camera &self) {\n> > + py::dict ret;\n> > +\n> > + for (const auto &[id, ci] : self.controls()) {\n> > + ret[id->name().c_str()] = make_tuple(ControlValueToPy(ci.min()),\n> > + ControlValueToPy(ci.max()),\n> > + ControlValueToPy(ci.def()));\n> > + }\n> > +\n> > + return ret;\n> > + })\n> > +\n> > + .def_property_readonly(\"properties\", [](Camera &self) {\n> > + py::dict ret;\n> > +\n> > + for (const auto &[key, cv] : self.properties()) {\n> > + const ControlId *id = properties::properties.at(key);\n> > + py::object ob = ControlValueToPy(cv);\n> > +\n> > + ret[id->name().c_str()] = ob;\n> > + }\n> > +\n> > + return ret;\n> > + });\n> > +\n> > + py::enum_(m, \"ConfigurationStatus\")\n> > + .value(\"Valid\", CameraConfiguration::Valid)\n> > + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n> > + .value(\"Invalid\", CameraConfiguration::Invalid);\n> > +\n> > + py::class_(m, \"CameraConfiguration\")\n> > + .def(\"__iter__\", [](CameraConfiguration &self) {\n> > + return py::make_iterator(self);\n> > + }, py::keep_alive<0, 1>())\n> > + .def(\"__len__\", [](CameraConfiguration &self) {\n> > + return self.size();\n> > + })\n> > + .def(\"validate\", &CameraConfiguration::validate)\n> > + .def(\"at\", py::overload_cast(&CameraConfiguration::at), py::return_value_policy::reference_internal)\n> > + .def_property_readonly(\"size\", &CameraConfiguration::size)\n> > + .def_property_readonly(\"empty\", &CameraConfiguration::empty);\n> > +\n> > + py::class_(m, \"StreamConfiguration\")\n> > + .def(\"toString\", &StreamConfiguration::toString)\n> > + .def_property_readonly(\"stream\", &StreamConfiguration::stream, py::return_value_policy::reference_internal)\n> > + .def_property(\n> > + \"size\",\n> > + [](StreamConfiguration &self) { return make_tuple(self.size.width, self.size.height); },\n> > + [](StreamConfiguration &self, tuple size) { self.size.width = get<0>(size); self.size.height = get<1>(size); })\n> > + .def_property(\n> > + \"fmt\",\n> > + [](StreamConfiguration &self) { return self.pixelFormat.toString(); },\n> > + [](StreamConfiguration &self, string fmt) { self.pixelFormat = PixelFormat::fromString(fmt); })\n> > + .def_readwrite(\"stride\", &StreamConfiguration::stride)\n> > + .def_readwrite(\"frameSize\", &StreamConfiguration::frameSize)\n> > + .def_readwrite(\"bufferCount\", &StreamConfiguration::bufferCount)\n> > + .def_property_readonly(\"formats\", &StreamConfiguration::formats, py::return_value_policy::reference_internal);\n> > + ;\n> > +\n> > + py::class_(m, \"StreamFormats\")\n> > + .def_property_readonly(\"pixelFormats\", [](StreamFormats &self) {\n> > + vector fmts;\n> > + for (auto &fmt : self.pixelformats())\n> > + fmts.push_back(fmt.toString());\n> > + return fmts;\n> > + })\n> > + .def(\"sizes\", [](StreamFormats &self, const string &pixelFormat) {\n> > + auto fmt = PixelFormat::fromString(pixelFormat);\n> > + vector> fmts;\n> > + for (const auto &s : self.sizes(fmt))\n> > + fmts.push_back(make_tuple(s.width, s.height));\n> > + return fmts;\n> > + })\n> > + .def(\"range\", [](StreamFormats &self, const string &pixelFormat) {\n> > + auto fmt = PixelFormat::fromString(pixelFormat);\n> > + const auto &range = self.range(fmt);\n> > + return make_tuple(make_tuple(range.hStep, range.vStep),\n> > + make_tuple(range.min.width, range.min.height),\n> > + make_tuple(range.max.width, range.max.height));\n> > + });\n> > +\n> > + py::enum_(m, \"StreamRole\")\n> > + .value(\"StillCapture\", StreamRole::StillCapture)\n> > + .value(\"Raw\", StreamRole::Raw)\n> > + .value(\"VideoRecording\", StreamRole::VideoRecording)\n> > + .value(\"Viewfinder\", StreamRole::Viewfinder);\n> > +\n> > + py::class_(m, \"FrameBufferAllocator\")\n> > + .def(py::init>(), py::keep_alive<1, 2>())\n> > + .def(\"allocate\", &FrameBufferAllocator::allocate)\n> > + .def_property_readonly(\"allocated\", &FrameBufferAllocator::allocated)\n> > + // Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator\n> > + .def(\"buffers\", [](FrameBufferAllocator &self, Stream *stream) {\n> > + py::object py_self = py::cast(self);\n> > + py::list l;\n> > + for (auto &ub : self.buffers(stream)) {\n> > + py::object py_buf = py::cast(ub.get(), py::return_value_policy::reference_internal, py_self);\n> > + l.append(py_buf);\n> > + }\n> > + return l;\n> > + });\n> > +\n> > + py::class_(m, \"FrameBuffer\")\n> > + // TODO: implement FrameBuffer::Plane properly\n> > + .def(py::init([](vector> planes, unsigned int cookie) {\n> > + vector v;\n> > + for (const auto &t : planes)\n> > + v.push_back({ SharedFD(get<0>(t)), FrameBuffer::Plane::kInvalidOffset, get<1>(t) });\n> > + return new FrameBuffer(v, cookie);\n> > + }))\n> > + .def_property_readonly(\"metadata\", &FrameBuffer::metadata, py::return_value_policy::reference_internal)\n> > + .def(\"length\", [](FrameBuffer &self, uint32_t idx) {\n> > + const FrameBuffer::Plane &plane = self.planes()[idx];\n> > + return plane.length;\n> > + })\n> > + .def(\"fd\", [](FrameBuffer &self, uint32_t idx) {\n> > + const FrameBuffer::Plane &plane = self.planes()[idx];\n> > + return plane.fd.get();\n> > + })\n> > + .def_property(\"cookie\", &FrameBuffer::cookie, &FrameBuffer::setCookie);\n> > +\n> > + py::class_(m, \"Stream\")\n> > + .def_property_readonly(\"configuration\", &Stream::configuration);\n> > +\n> > + py::enum_(m, \"ReuseFlag\")\n> > + .value(\"Default\", Request::ReuseFlag::Default)\n> > + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n> > +\n> > + py::class_(m, \"Request\")\n> > + .def_property_readonly(\"camera\", &Request::camera)\n> > + .def(\"addBuffer\", &Request::addBuffer, py::keep_alive<1, 3>()) // Request keeps Framebuffer alive\n> > + .def_property_readonly(\"status\", &Request::status)\n> > + .def_property_readonly(\"buffers\", &Request::buffers)\n> > + .def_property_readonly(\"cookie\", &Request::cookie)\n> > + .def_property_readonly(\"hasPendingBuffers\", &Request::hasPendingBuffers)\n> > + .def(\"set_control\", [](Request &self, string &control, py::object value) {\n\nI found this one just a bit clunky to use, as I always have a\ndictionary of control values and then have to loop through them\nmyself. If it took the dictionary (like my modified start method) that\nwould be more convenient. And I suppose we'd call it set_controls, so\nwe could actually have both.\n\nBut these are all minor points, these bindings, perhaps with the\nexception of array controls, are already working very well for me.\n\nThanks!\n\nDavid\n\n> > + const auto &controls = self.camera()->controls();\n> > +\n> > + auto it = find_if(controls.begin(), controls.end(),\n> > + [&control](const auto &kvp) { return kvp.first->name() == control; });\n> > +\n> > + if (it == controls.end())\n> > + throw runtime_error(\"Control not found\");\n> > +\n> > + const auto &id = it->first;\n> > +\n> > + self.controls().set(id->id(), PyToControlValue(value, id->type()));\n> > + })\n> > + .def_property_readonly(\"metadata\", [](Request &self) {\n> > + py::dict ret;\n> > +\n> > + for (const auto &[key, cv] : self.metadata()) {\n> > + const ControlId *id = controls::controls.at(key);\n> > + py::object ob = ControlValueToPy(cv);\n> > +\n> > + ret[id->name().c_str()] = ob;\n> > + }\n> > +\n> > + return ret;\n> > + })\n> > + // As we add a keep_alive to the fb in addBuffers(), we can only allow reuse with ReuseBuffers.\n> > + .def(\"reuse\", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });\n> > +\n> > + py::enum_(m, \"RequestStatus\")\n> > + .value(\"Pending\", Request::RequestPending)\n> > + .value(\"Complete\", Request::RequestComplete)\n> > + .value(\"Cancelled\", Request::RequestCancelled);\n> > +\n> > + py::enum_(m, \"FrameMetadataStatus\")\n> > + .value(\"Success\", FrameMetadata::FrameSuccess)\n> > + .value(\"Error\", FrameMetadata::FrameError)\n> > + .value(\"Cancelled\", FrameMetadata::FrameCancelled);\n> > +\n> > + py::class_(m, \"FrameMetadata\")\n> > + .def_readonly(\"status\", &FrameMetadata::status)\n> > + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n> > + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n> > + .def_property_readonly(\"bytesused\", [](FrameMetadata &self) {\n> > + vector v;\n> > + v.resize(self.planes().size());\n> > + transform(self.planes().begin(), self.planes().end(), v.begin(), [](const auto &p) { return p.bytesused; });\n> > + return v;\n> > + });\n> > +}\n> > diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> > new file mode 100644\n> > index 00000000..9d6e7acb\n> > --- /dev/null\n> > +++ b/subprojects/pybind11.wrap\n> > @@ -0,0 +1,12 @@\n> > +[wrap-file]\n> > +directory = pybind11-2.6.1\n> > +source_url = https://github.com/pybind/pybind11/archive/v2.6.1.tar.gz\n> > +source_filename = pybind11-2.6.1.tar.gz\n> > +source_hash = cdbe326d357f18b83d10322ba202d69f11b2f49e2d87ade0dc2be0c5c34f8e2a\n> > +patch_url = https://wrapdb.mesonbuild.com/v2/pybind11_2.6.1-1/get_patch\n> > +patch_filename = pybind11-2.6.1-1-wrap.zip\n> > +patch_hash = 6de5477598b56c8a2e609196420c783ac35b79a31d6622121602e6ade6b3cee8\n> > +\n> > +[provide]\n> > +pybind11 = pybind11_dep\n> > +\n> > --\n> > 2.25.1\n> >","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 1E974BF415\n\tfor ;\n\tThu, 9 Dec 2021 11:16:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 481B760882;\n\tThu, 9 Dec 2021 12:16:30 +0100 (CET)","from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com\n\t[IPv6:2a00:1450:4864:20::42e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CFBA7607DE\n\tfor ;\n\tThu, 9 Dec 2021 12:16:28 +0100 (CET)","by mail-wr1-x42e.google.com with SMTP id a18so9070919wrn.6\n\tfor ;\n\tThu, 09 Dec 2021 03:16:28 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"kVTHtqrq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=aFcKpLAvK2q5GtV6H467mYaFgsp/hhruS3/IaP119JU=;\n\tb=kVTHtqrqGYtsP5kROucIBvNMTfWkEJ83Q0HgePhXjyC5vOll/yXceqP6cTK8idpt3D\n\t0YoYb753h1+9HxkakjKZvSOIuMB/5OQl+yfxNMOtaBPTabUtyD7FTjHfqEoXIpWSy1QK\n\tmsAXNi2FU47h9uaRfthxs02YUAOGaCesFWAL5YgGz3v6QAJbExbYp2vCWs3v9Ep34kjI\n\tOjI0gxN3LrkqDwUo+5aWdvpwbS79EV04u8iJuwk/agID4pPEqHUVfOl+kbHK2NWx4sYr\n\t2Xl6sAcHvYNjZHg4G1Y22Api7sqf01HegOZ7B7qzMWB5f0p0qlo7r56LJvQjwfLCPkbT\n\tC5YA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=aFcKpLAvK2q5GtV6H467mYaFgsp/hhruS3/IaP119JU=;\n\tb=WxiFM08DbTGo5yBtrlgQ71Drp2A+A0wATnT2pdlofD0J9dcRqivnIqkomkv+qgSPH9\n\tMTVRHNORO32rqQsc/OTI1caZ2NjVc2G3DKm5GNNBxRnx/WCRQQNVFpZQlL7ksMawiufx\n\tfaxirmNVXTsE5kloqoritzl2Tz7+oJm00B7bAD8b+0JRhzum6lqa36Az4Rfqm2h8SwV0\n\tExIE+mcBkQ/6DJjErY/bzMGjj2M38cLskmzp2KnGJn9SlRjNcftRglMM0s81ESMx5sSI\n\t8Ehoma0z6CmQQRWjxj+mr3KEyJ2gU7V7BlvLEskau9AHJgeRxl+n/uuXpbCQ4maVPxjC\n\t3CQA==","X-Gm-Message-State":"AOAM532nl7ciU0plkHyoL/jKA/c2qvPiKGBnRONjOrcF8tGShZYFP0Gr\n\teX6Jf7jyK+0XdiLEHyySRyJtmTlsRx2TtfjDeEMX5QNIWe7x7Q==","X-Google-Smtp-Source":"ABdhPJzTkjuxCQg8/x4N5AOeQPTStq12ZN5Z5xlZ1dy0sO6qkPHmKsjJAMGwxKs6b8RQ52QA+B5NktDLB20rXMzNFF8=","X-Received":"by 2002:adf:e390:: with SMTP id\n\te16mr5603878wrm.494.1639048588174; \n\tThu, 09 Dec 2021 03:16:28 -0800 (PST)","MIME-Version":"1.0","References":"<20211209092906.37303-1-tomi.valkeinen@ideasonboard.com>\n\t<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>\n\t<163904389093.2066819.16202667063304401995@Monstersaurus>","In-Reply-To":"<163904389093.2066819.16202667063304401995@Monstersaurus>","From":"David Plowman ","Date":"Thu, 9 Dec 2021 11:16:17 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21710,"web_url":"https://patchwork.libcamera.org/comment/21710/","msgid":"<163905088555.2066819.3887909202891654895@Monstersaurus>","date":"2021-12-09T11:54:45","subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2021-12-09 11:16:17)\n> Hi Tomi\n> \n> Thanks for this patch!\n> \n> On Thu, 9 Dec 2021 at 09:58, Kieran Bingham\n> wrote:\n> >\n> > Quoting Tomi Valkeinen (2021-12-09 09:29:05)\n> > > Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n> > > Python layer.\n> > >\n> > > Only a subset of libcamera classes are exposed.\n> > >\n> > > Signed-off-by: Tomi Valkeinen \n> > > ---\n> > > .gitignore | 1 +\n> > > meson.build | 1 +\n> > > meson_options.txt | 5 +\n> > > src/meson.build | 1 +\n> > > src/py/meson.build | 1 +\n> > > src/py/pycamera/__init__.py | 10 +\n> > > src/py/pycamera/meson.build | 43 ++++\n> > > src/py/pycamera/pymain.cpp | 424 ++++++++++++++++++++++++++++++++++++\n> > > subprojects/pybind11.wrap | 12 +\n> > > 9 files changed, 498 insertions(+)\n> > > create mode 100644 src/py/meson.build\n> > > create mode 100644 src/py/pycamera/__init__.py\n> > > create mode 100644 src/py/pycamera/meson.build\n> > > create mode 100644 src/py/pycamera/pymain.cpp\n> > > create mode 100644 subprojects/pybind11.wrap\n> > >\n> > > diff --git a/.gitignore b/.gitignore\n> > > index cca829fa..aae56b2d 100644\n> > > --- a/.gitignore\n> > > +++ b/.gitignore\n> > > @@ -3,6 +3,7 @@\n> > > __pycache__/\n> > > build/\n> > > patches/\n> > > +subprojects/pybind11-2.*/\n> > > *.patch\n> > > *.pyc\n> > > .cache\n> > > diff --git a/meson.build b/meson.build\n> > > index a20cc29e..0f885c14 100644\n> > > --- a/meson.build\n> > > +++ b/meson.build\n> > > @@ -181,6 +181,7 @@ summary({\n> > > 'qcam application': qcam_enabled,\n> > > 'lc-compliance application': lc_compliance_enabled,\n> > > 'Unit tests': test_enabled,\n> > > + 'Python bindings': pycamera_enabled,\n> > > },\n> > > section : 'Configuration',\n> > > bool_yn : true)\n> > > diff --git a/meson_options.txt b/meson_options.txt\n> > > index 2c80ad8b..ca00c78e 100644\n> > > --- a/meson_options.txt\n> > > +++ b/meson_options.txt\n> > > @@ -58,3 +58,8 @@ option('v4l2',\n> > > type : 'boolean',\n> > > value : false,\n> > > description : 'Compile the V4L2 compatibility layer')\n> > > +\n> > > +option('pycamera',\n> > > + type : 'feature',\n> > > + value : 'auto',\n> > > + description : 'Enable libcamera Python bindings (experimental)')\n> > > diff --git a/src/meson.build b/src/meson.build\n> > > index e0ea9c35..4a1e3cb0 100644\n> > > --- a/src/meson.build\n> > > +++ b/src/meson.build\n> > > @@ -38,3 +38,4 @@ subdir('qcam')\n> > >\n> > > subdir('gstreamer')\n> > > subdir('v4l2')\n> > > +subdir('py')\n> > > diff --git a/src/py/meson.build b/src/py/meson.build\n> > > new file mode 100644\n> > > index 00000000..42ffa221\n> > > --- /dev/null\n> > > +++ b/src/py/meson.build\n> > > @@ -0,0 +1 @@\n> > > +subdir('pycamera')\n> > > diff --git a/src/py/pycamera/__init__.py b/src/py/pycamera/__init__.py\n> > > new file mode 100644\n> > > index 00000000..a5c198dc\n> > > --- /dev/null\n> > > +++ b/src/py/pycamera/__init__.py\n> > > @@ -0,0 +1,10 @@\n> > > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > > +# Copyright (C) 2021, Tomi Valkeinen \n> > > +\n> > > +from .pycamera import *\n> > > +import mmap\n> > > +\n> > > +def __FrameBuffer__mmap(self, plane):\n> > > + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)\n> > > +\n> > > +FrameBuffer.mmap = __FrameBuffer__mmap\n> > > diff --git a/src/py/pycamera/meson.build b/src/py/pycamera/meson.build\n> > > new file mode 100644\n> > > index 00000000..c490a18d\n> > > --- /dev/null\n> > > +++ b/src/py/pycamera/meson.build\n> > > @@ -0,0 +1,43 @@\n> > > +# SPDX-License-Identifier: CC0-1.0\n> > > +\n> > > +py3_dep = dependency('python3', required : get_option('pycamera'))\n> > > +\n> > > +if not py3_dep.found()\n> > > + pycamera_enabled = false\n> > > + subdir_done()\n> > > +endif\n> > > +\n> > > +pycamera_enabled = true\n> > > +\n> > > +pybind11_proj = subproject('pybind11')\n> > > +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')\n> > > +\n> > > +pycamera_sources = files([\n> > > + 'pymain.cpp',\n> > > +])\n> > > +\n> > > +pycamera_deps = [\n> > > + libcamera_public,\n> > > + py3_dep,\n> > > + pybind11_dep,\n> > > +]\n> > > +\n> > > +pycamera_args = [ '-fvisibility=hidden' ]\n> > > +pycamera_args += [ '-Wno-shadow' ]\n> >\n> > Does python shadow variables explicitly? Does this lead to any potential\n> > bugs?\n> >\n> > I recall putting -Wshadow in explicitly because I hit a nasty bug due to\n> > shadowing, so in my eyes, shadowed variables are a bad-thing (tm) :-)\n> >\n> > You have to be sure you know which variable you are writing to,\n> > and it might not always be clear to a reader, or cause potential\n> > confusion..?\n> >\n> > (of course subclassing, and having functions with the same name is\n> > essentially 'shadowing' the function names I guess...)\n> >\n> >\n> > > +\n> > > +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/pycamera'\n> > > +\n> > > +pycamera = shared_module('pycamera',\n> > > + pycamera_sources,\n> > > + install : true,\n> > > + install_dir : destdir,\n> > > + name_prefix : '',\n> > > + dependencies : pycamera_deps,\n> > > + cpp_args : pycamera_args)\n> > > +\n> > > +# XXX > You could also create a symlink. There's an example in the top-level\n> > > +# > meson.build.\n> > > +# Copy __init__.py to build dir so that we can run without installing\n> > > +configure_file(input: '__init__.py', output: '__init__.py', copy: true)\n> >\n> > I think a symlink would be better too.\n> >\n> > > +\n> > > +install_data(['__init__.py'], install_dir : destdir)\n> > > diff --git a/src/py/pycamera/pymain.cpp b/src/py/pycamera/pymain.cpp\n> > > new file mode 100644\n> > > index 00000000..0088e133\n> > > --- /dev/null\n> > > +++ b/src/py/pycamera/pymain.cpp\n> \n> pylibcamera?? :)\n> \n> > > @@ -0,0 +1,424 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2021, Tomi Valkeinen \n> > > + *\n> > > + * Python bindings\n> > > + */\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +namespace py = pybind11;\n> > > +\n> > > +using namespace std;\n> > > +using namespace libcamera;\n> > > +\n> > > +static py::object ControlValueToPy(const ControlValue &cv)\n> > > +{\n> > > + //assert(!cv.isArray());\n> > > + //assert(cv.numElements() == 1);\n> >\n> > Are these asserts not necessary? Is it better to keep them in, in a way\n> > that they are used on debug buidls and compiled out on release builds?\n> >\n> > I think we have ASSERT() for that, but that's an 'internal' helper I\n> > think and I expect the src/py is building on top of the public api..\n> >\n> >\n> > > +\n> > > + switch (cv.type()) {\n> > > + case ControlTypeBool:\n> > > + return py::cast(cv.get());\n> > > + case ControlTypeByte:\n> > > + return py::cast(cv.get());\n> > > + case ControlTypeInteger32:\n> > > + return py::cast(cv.get());\n> > > + case ControlTypeInteger64:\n> > > + return py::cast(cv.get());\n> > > + case ControlTypeFloat:\n> > > + return py::cast(cv.get());\n> > > + case ControlTypeString:\n> > > + return py::cast(cv.get());\n> > > + case ControlTypeRectangle: {\n> > > + const Rectangle *v = reinterpret_cast(cv.data().data());\n> > > + return py::make_tuple(v->x, v->y, v->width, v->height);\n> > > + }\n> > > + case ControlTypeSize: {\n> > > + const Size *v = reinterpret_cast(cv.data().data());\n> > > + return py::make_tuple(v->width, v->height);\n> > > + }\n> > > + case ControlTypeNone:\n> > > + default:\n> > > + throw runtime_error(\"Unsupported ControlValue type\");\n> > > + }\n> > > +}\n> > > +\n> > > +static ControlValue PyToControlValue(const py::object &ob, ControlType type)\n> > > +{\n> > > + switch (type) {\n> > > + case ControlTypeBool:\n> > > + return ControlValue(ob.cast());\n> > > + case ControlTypeByte:\n> > > + return ControlValue(ob.cast());\n> > > + case ControlTypeInteger32:\n> > > + return ControlValue(ob.cast());\n> > > + case ControlTypeInteger64:\n> > > + return ControlValue(ob.cast());\n> > > + case ControlTypeFloat:\n> > > + return ControlValue(ob.cast());\n> > > + case ControlTypeString:\n> > > + return ControlValue(ob.cast());\n> > > + case ControlTypeRectangle:\n> > > + case ControlTypeSize:\n> > > + case ControlTypeNone:\n> > > + default:\n> > > + throw runtime_error(\"Control type not implemented\");\n> > > + }\n> > > +}\n> \n> I wonder whether we should be supporting array controls even in the\n> initial version? Some of the other things (like transforms) simply\n> manifest themselves as \"features that you don't get\", but I found that\n> any attempts to use metadata basically resulted in failures until I\n> bodged the array controls to work.\n> \n> What do you thinK?\n> \n> > > +\n> > > +static weak_ptr g_camera_manager;\n> > > +static int g_eventfd;\n> > > +static mutex g_reqlist_mutex;\n> > > +static vector g_reqlist;\n> > > +\n> > > +static void handle_request_completed(Request *req)\n> > > +{\n> > > + {\n> > > + lock_guard guard(g_reqlist_mutex);\n> > > + g_reqlist.push_back(req);\n> > > + }\n> > > +\n> > > + uint64_t v = 1;\n> > > + write(g_eventfd, &v, 8);\n> > > +}\n> > > +\n> > > +PYBIND11_MODULE(pycamera, m)\n> > > +{\n> > > + m.def(\"logSetLevel\", &logSetLevel);\n> > > +\n> > > + py::class_>(m, \"CameraManager\")\n> > > + .def_static(\"singleton\", []() {\n> > > + shared_ptr cm = g_camera_manager.lock();\n> > > + if (cm)\n> > > + return cm;\n> > > +\n> > > + int fd = eventfd(0, 0);\n> > > + if (fd == -1)\n> > > + throw std::system_error(errno, std::generic_category(), \"Failed to create eventfd\");\n> > > +\n> > > + cm = shared_ptr(new CameraManager, [](auto p) {\n> > > + close(g_eventfd);\n> > > + g_eventfd = -1;\n> > > + delete p;\n> > > + });\n> > > +\n> > > + g_eventfd = fd;\n> > > + g_camera_manager = cm;\n> > > +\n> > > + int ret = cm->start();\n> > > + if (ret)\n> > > + throw std::system_error(-ret, std::generic_category(), \"Failed to start CameraManager\");\n> > > +\n> > > + return cm;\n> > > + })\n> > > +\n> > > + .def_property_readonly(\"version\", &CameraManager::version)\n> > > +\n> > > + .def_property_readonly(\"efd\", [](CameraManager &) {\n> > > + return g_eventfd;\n> > > + })\n> > > +\n> > > + .def(\"getReadyRequests\", [](CameraManager &) {\n> > > + vector v;\n> > > +\n> > > + {\n> > > + lock_guard guard(g_reqlist_mutex);\n> > > + swap(v, g_reqlist);\n> > > + }\n> > > +\n> > > + vector ret;\n> > > +\n> > > + for (Request *req : v) {\n> > > + py::object o = py::cast(req);\n> > > + // decrease the ref increased in Camera::queueRequest()\n> > > + o.dec_ref();\n> > > + ret.push_back(o);\n> > > + }\n> > > +\n> > > + return ret;\n> > > + })\n> > > +\n> > > + .def(\"get\", py::overload_cast(&CameraManager::get), py::keep_alive<0, 1>())\n> > > +\n> > > + .def(\"find\", [](CameraManager &self, string str) {\n> > > + std::transform(str.begin(), str.end(), str.begin(), ::tolower);\n> > > +\n> > > + for (auto c : self.cameras()) {\n> > > + string id = c->id();\n> > > +\n> > > + std::transform(id.begin(), id.end(), id.begin(), ::tolower);\n> > > +\n> > > + if (id.find(str) != string::npos)\n> > > + return c;\n> > > + }\n> > > +\n> > > + return shared_ptr();\n> > > + }, py::keep_alive<0, 1>())\n> > > +\n> > > + // Create a list of Cameras, where each camera has a keep-alive to CameraManager\n> > > + .def_property_readonly(\"cameras\", [](CameraManager &self) {\n> > > + py::list l;\n> > > +\n> > > + for (auto &c : self.cameras()) {\n> > > + py::object py_cm = py::cast(self);\n> > > + py::object py_cam = py::cast(c);\n> > > + py::detail::keep_alive_impl(py_cam, py_cm);\n> > > + l.append(py_cam);\n> > > + }\n> > > +\n> > > + return l;\n> > > + });\n> > > +\n> > > + py::class_>(m, \"Camera\")\n> > > + .def_property_readonly(\"id\", &Camera::id)\n> > > + .def(\"acquire\", &Camera::acquire)\n> > > + .def(\"release\", &Camera::release)\n> > > + .def(\"start\", [](shared_ptr &self) {\n> \n> This is the one that needs to take a dictionary of controls, but we\n> can add that later!\n> \n> > > + self->requestCompleted.connect(handle_request_completed);\n> > > +\n> > > + int ret = self->start();\n> > > + if (ret)\n> > > + self->requestCompleted.disconnect(handle_request_completed);\n> > > +\n> > > + return ret;\n> > > + })\n> > > +\n> > > + .def(\"stop\", [](shared_ptr &self) {\n> > > + int ret = self->stop();\n> > > + if (!ret)\n> > > + self->requestCompleted.disconnect(handle_request_completed);\n> > > +\n> > > + return ret;\n> > > + })\n> > > +\n> > > + .def(\"__repr__\", [](shared_ptr &self) {\n> > > + return \"id() + \"'>\";\n> > > + })\n> > > +\n> > > + // Keep the camera alive, as StreamConfiguration contains a Stream*\n> > > + .def(\"generateConfiguration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> \n> I was sometimes finding generateConfiguration a bit awkward because\n> it's the only way to create a CameraConfiguration (I think, maybe I'm\n> wrong?), and it always needs a list of stream roles. So I seem to\n\nIt is the only way to 'create' a CameraConfiguration, as it has to be\nowned by the Camera I believe, but it shouldn't require a list of\nStreamRoles.\n\nThe stream roles can be empty, and the streams added (or removed?)\nmanually after.\n\nAnd you should be able to reuse it while negotiating with the Camera -\nyou wouldn't hve to recreate new ones each time do you? So it's not\nsomething I'd expect to happen a lot...\n\n> spend a lot of my time converting Python dicts (what the user sees) to\n> CameraConfigurations and vice versa. I guess I'm not sure what I'm\n> asking for, perhaps I just need to try and tidy up my translation code\n> which has become a bit too twisty.\n\nOr perhaps this is more refering to how the CameraConfiguration is being\nfilled in by your layer ...?\n\n> > > + .def(\"configure\", &Camera::configure)\n> > > +\n> > > + .def(\"createRequest\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> > > +\n> > > + .def(\"queueRequest\", [](Camera &self, Request *req) {\n> > > + py::object py_req = py::cast(req);\n> > > +\n> > > + py_req.inc_ref();\n> > > +\n> > > + int ret = self.queueRequest(req);\n> > > + if (ret)\n> > > + py_req.dec_ref();\n> > > +\n> > > + return ret;\n> > > + })\n> > > +\n> > > + .def_property_readonly(\"streams\", [](Camera &self) {\n> > > + py::set set;\n> > > + for (auto &s : self.streams()) {\n> > > + py::object py_self = py::cast(self);\n> > > + py::object py_s = py::cast(s);\n> > > + py::detail::keep_alive_impl(py_s, py_self);\n> > > + set.add(py_s);\n> > > + }\n> > > + return set;\n> > > + })\n> > > +\n> > > + .def_property_readonly(\"controls\", [](Camera &self) {\n> > > + py::dict ret;\n> > > +\n> > > + for (const auto &[id, ci] : self.controls()) {\n> > > + ret[id->name().c_str()] = make_tuple(ControlValueToPy(ci.min()),\n> > > + ControlValueToPy(ci.max()),\n> > > + ControlValueToPy(ci.def()));\n> > > + }\n> > > +\n> > > + return ret;\n> > > + })\n> > > +\n> > > + .def_property_readonly(\"properties\", [](Camera &self) {\n> > > + py::dict ret;\n> > > +\n> > > + for (const auto &[key, cv] : self.properties()) {\n> > > + const ControlId *id = properties::properties.at(key);\n> > > + py::object ob = ControlValueToPy(cv);\n> > > +\n> > > + ret[id->name().c_str()] = ob;\n> > > + }\n> > > +\n> > > + return ret;\n> > > + });\n> > > +\n> > > + py::enum_(m, \"ConfigurationStatus\")\n> > > + .value(\"Valid\", CameraConfiguration::Valid)\n> > > + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n> > > + .value(\"Invalid\", CameraConfiguration::Invalid);\n> > > +\n> > > + py::class_(m, \"CameraConfiguration\")\n> > > + .def(\"__iter__\", [](CameraConfiguration &self) {\n> > > + return py::make_iterator(self);\n> > > + }, py::keep_alive<0, 1>())\n> > > + .def(\"__len__\", [](CameraConfiguration &self) {\n> > > + return self.size();\n> > > + })\n> > > + .def(\"validate\", &CameraConfiguration::validate)\n> > > + .def(\"at\", py::overload_cast(&CameraConfiguration::at), py::return_value_policy::reference_internal)\n> > > + .def_property_readonly(\"size\", &CameraConfiguration::size)\n> > > + .def_property_readonly(\"empty\", &CameraConfiguration::empty);\n> > > +\n> > > + py::class_(m, \"StreamConfiguration\")\n> > > + .def(\"toString\", &StreamConfiguration::toString)\n> > > + .def_property_readonly(\"stream\", &StreamConfiguration::stream, py::return_value_policy::reference_internal)\n> > > + .def_property(\n> > > + \"size\",\n> > > + [](StreamConfiguration &self) { return make_tuple(self.size.width, self.size.height); },\n> > > + [](StreamConfiguration &self, tuple size) { self.size.width = get<0>(size); self.size.height = get<1>(size); })\n> > > + .def_property(\n> > > + \"fmt\",\n> > > + [](StreamConfiguration &self) { return self.pixelFormat.toString(); },\n> > > + [](StreamConfiguration &self, string fmt) { self.pixelFormat = PixelFormat::fromString(fmt); })\n> > > + .def_readwrite(\"stride\", &StreamConfiguration::stride)\n> > > + .def_readwrite(\"frameSize\", &StreamConfiguration::frameSize)\n> > > + .def_readwrite(\"bufferCount\", &StreamConfiguration::bufferCount)\n> > > + .def_property_readonly(\"formats\", &StreamConfiguration::formats, py::return_value_policy::reference_internal);\n> > > + ;\n> > > +\n> > > + py::class_(m, \"StreamFormats\")\n> > > + .def_property_readonly(\"pixelFormats\", [](StreamFormats &self) {\n> > > + vector fmts;\n> > > + for (auto &fmt : self.pixelformats())\n> > > + fmts.push_back(fmt.toString());\n> > > + return fmts;\n> > > + })\n> > > + .def(\"sizes\", [](StreamFormats &self, const string &pixelFormat) {\n> > > + auto fmt = PixelFormat::fromString(pixelFormat);\n> > > + vector> fmts;\n> > > + for (const auto &s : self.sizes(fmt))\n> > > + fmts.push_back(make_tuple(s.width, s.height));\n> > > + return fmts;\n> > > + })\n> > > + .def(\"range\", [](StreamFormats &self, const string &pixelFormat) {\n> > > + auto fmt = PixelFormat::fromString(pixelFormat);\n> > > + const auto &range = self.range(fmt);\n> > > + return make_tuple(make_tuple(range.hStep, range.vStep),\n> > > + make_tuple(range.min.width, range.min.height),\n> > > + make_tuple(range.max.width, range.max.height));\n> > > + });\n> > > +\n> > > + py::enum_(m, \"StreamRole\")\n> > > + .value(\"StillCapture\", StreamRole::StillCapture)\n> > > + .value(\"Raw\", StreamRole::Raw)\n> > > + .value(\"VideoRecording\", StreamRole::VideoRecording)\n> > > + .value(\"Viewfinder\", StreamRole::Viewfinder);\n> > > +\n> > > + py::class_(m, \"FrameBufferAllocator\")\n> > > + .def(py::init>(), py::keep_alive<1, 2>())\n> > > + .def(\"allocate\", &FrameBufferAllocator::allocate)\n> > > + .def_property_readonly(\"allocated\", &FrameBufferAllocator::allocated)\n> > > + // Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator\n> > > + .def(\"buffers\", [](FrameBufferAllocator &self, Stream *stream) {\n> > > + py::object py_self = py::cast(self);\n> > > + py::list l;\n> > > + for (auto &ub : self.buffers(stream)) {\n> > > + py::object py_buf = py::cast(ub.get(), py::return_value_policy::reference_internal, py_self);\n> > > + l.append(py_buf);\n> > > + }\n> > > + return l;\n> > > + });\n> > > +\n> > > + py::class_(m, \"FrameBuffer\")\n> > > + // TODO: implement FrameBuffer::Plane properly\n> > > + .def(py::init([](vector> planes, unsigned int cookie) {\n> > > + vector v;\n> > > + for (const auto &t : planes)\n> > > + v.push_back({ SharedFD(get<0>(t)), FrameBuffer::Plane::kInvalidOffset, get<1>(t) });\n> > > + return new FrameBuffer(v, cookie);\n> > > + }))\n> > > + .def_property_readonly(\"metadata\", &FrameBuffer::metadata, py::return_value_policy::reference_internal)\n> > > + .def(\"length\", [](FrameBuffer &self, uint32_t idx) {\n> > > + const FrameBuffer::Plane &plane = self.planes()[idx];\n> > > + return plane.length;\n> > > + })\n> > > + .def(\"fd\", [](FrameBuffer &self, uint32_t idx) {\n> > > + const FrameBuffer::Plane &plane = self.planes()[idx];\n> > > + return plane.fd.get();\n> > > + })\n> > > + .def_property(\"cookie\", &FrameBuffer::cookie, &FrameBuffer::setCookie);\n> > > +\n> > > + py::class_(m, \"Stream\")\n> > > + .def_property_readonly(\"configuration\", &Stream::configuration);\n> > > +\n> > > + py::enum_(m, \"ReuseFlag\")\n> > > + .value(\"Default\", Request::ReuseFlag::Default)\n> > > + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n> > > +\n> > > + py::class_(m, \"Request\")\n> > > + .def_property_readonly(\"camera\", &Request::camera)\n> > > + .def(\"addBuffer\", &Request::addBuffer, py::keep_alive<1, 3>()) // Request keeps Framebuffer alive\n> > > + .def_property_readonly(\"status\", &Request::status)\n> > > + .def_property_readonly(\"buffers\", &Request::buffers)\n> > > + .def_property_readonly(\"cookie\", &Request::cookie)\n> > > + .def_property_readonly(\"hasPendingBuffers\", &Request::hasPendingBuffers)\n> > > + .def(\"set_control\", [](Request &self, string &control, py::object value) {\n> \n> I found this one just a bit clunky to use, as I always have a\n> dictionary of control values and then have to loop through them\n> myself. If it took the dictionary (like my modified start method) that\n> would be more convenient. And I suppose we'd call it set_controls, so\n> we could actually have both.\n> \n> But these are all minor points, these bindings, perhaps with the\n> exception of array controls, are already working very well for me.\n> \n> Thanks!\n> \n> David\n> \n> > > + const auto &controls = self.camera()->controls();\n> > > +\n> > > + auto it = find_if(controls.begin(), controls.end(),\n> > > + [&control](const auto &kvp) { return kvp.first->name() == control; });\n> > > +\n> > > + if (it == controls.end())\n> > > + throw runtime_error(\"Control not found\");\n> > > +\n> > > + const auto &id = it->first;\n> > > +\n> > > + self.controls().set(id->id(), PyToControlValue(value, id->type()));\n> > > + })\n> > > + .def_property_readonly(\"metadata\", [](Request &self) {\n> > > + py::dict ret;\n> > > +\n> > > + for (const auto &[key, cv] : self.metadata()) {\n> > > + const ControlId *id = controls::controls.at(key);\n> > > + py::object ob = ControlValueToPy(cv);\n> > > +\n> > > + ret[id->name().c_str()] = ob;\n> > > + }\n> > > +\n> > > + return ret;\n> > > + })\n> > > + // As we add a keep_alive to the fb in addBuffers(), we can only allow reuse with ReuseBuffers.\n> > > + .def(\"reuse\", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });\n> > > +\n> > > + py::enum_(m, \"RequestStatus\")\n> > > + .value(\"Pending\", Request::RequestPending)\n> > > + .value(\"Complete\", Request::RequestComplete)\n> > > + .value(\"Cancelled\", Request::RequestCancelled);\n> > > +\n> > > + py::enum_(m, \"FrameMetadataStatus\")\n> > > + .value(\"Success\", FrameMetadata::FrameSuccess)\n> > > + .value(\"Error\", FrameMetadata::FrameError)\n> > > + .value(\"Cancelled\", FrameMetadata::FrameCancelled);\n> > > +\n> > > + py::class_(m, \"FrameMetadata\")\n> > > + .def_readonly(\"status\", &FrameMetadata::status)\n> > > + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n> > > + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n> > > + .def_property_readonly(\"bytesused\", [](FrameMetadata &self) {\n> > > + vector v;\n> > > + v.resize(self.planes().size());\n> > > + transform(self.planes().begin(), self.planes().end(), v.begin(), [](const auto &p) { return p.bytesused; });\n> > > + return v;\n> > > + });\n> > > +}\n> > > diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> > > new file mode 100644\n> > > index 00000000..9d6e7acb\n> > > --- /dev/null\n> > > +++ b/subprojects/pybind11.wrap\n> > > @@ -0,0 +1,12 @@\n> > > +[wrap-file]\n> > > +directory = pybind11-2.6.1\n> > > +source_url = https://github.com/pybind/pybind11/archive/v2.6.1.tar.gz\n> > > +source_filename = pybind11-2.6.1.tar.gz\n> > > +source_hash = cdbe326d357f18b83d10322ba202d69f11b2f49e2d87ade0dc2be0c5c34f8e2a\n> > > +patch_url = https://wrapdb.mesonbuild.com/v2/pybind11_2.6.1-1/get_patch\n> > > +patch_filename = pybind11-2.6.1-1-wrap.zip\n> > > +patch_hash = 6de5477598b56c8a2e609196420c783ac35b79a31d6622121602e6ade6b3cee8\n> > > +\n> > > +[provide]\n> > > +pybind11 = pybind11_dep\n> > > +\n> > > --\n> > > 2.25.1\n> > >","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 11EC7BF415\n\tfor ;\n\tThu, 9 Dec 2021 11:54:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 516636087A;\n\tThu, 9 Dec 2021 12:54:50 +0100 (CET)","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 76539607DE\n\tfor ;\n\tThu, 9 Dec 2021 12:54:48 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F3817501;\n\tThu, 9 Dec 2021 12:54:47 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PL+M3RZY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639050888;\n\tbh=MSbRwvDuorYyVkZ9jXftsTrHtpftL4Z7EMTOzfdjQK4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=PL+M3RZY7VYDaJ1Siyg5KWidppu+43frLR/hO/mrYy6jIY70Up+kM8diMwzkBytQO\n\tPHb5WkpTasHAzgQxQXhcib2XXKM9VWuc5XCRko/cwz2XwQkqzCyzvz1Kg9fZ8OoUdJ\n\tCan2f1KZOnjd8qqGX6EFaf81bEqNUEDHh49aOJuI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211209092906.37303-1-tomi.valkeinen@ideasonboard.com>\n\t<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>\n\t<163904389093.2066819.16202667063304401995@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"David Plowman ","Date":"Thu, 09 Dec 2021 11:54:45 +0000","Message-ID":"<163905088555.2066819.3887909202891654895@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21719,"web_url":"https://patchwork.libcamera.org/comment/21719/","msgid":"","date":"2021-12-09T19:28:33","subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nThank you for the patch.\n\nOn Thu, Dec 09, 2021 at 11:54:45AM +0000, Kieran Bingham wrote:\n> Quoting David Plowman (2021-12-09 11:16:17)\n> > On Thu, 9 Dec 2021 at 09:58, Kieran Bingham wrote:\n> > > Quoting Tomi Valkeinen (2021-12-09 09:29:05)\n> > > > Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n> > > > Python layer.\n> > > >\n> > > > Only a subset of libcamera classes are exposed.\n\nCould you elaborate a little bit on your plans in this area ? Was this\ndone to keep the development effort limited for a first version ? Are\nthere any particular design issues that would make support for the rest\nof the API more difficult, or is it \"just\" a matter of plumbind the rest\n?\n\n> > > > Signed-off-by: Tomi Valkeinen \n> > > > ---\n> > > > .gitignore | 1 +\n> > > > meson.build | 1 +\n> > > > meson_options.txt | 5 +\n> > > > src/meson.build | 1 +\n> > > > src/py/meson.build | 1 +\n> > > > src/py/pycamera/__init__.py | 10 +\n> > > > src/py/pycamera/meson.build | 43 ++++\n> > > > src/py/pycamera/pymain.cpp | 424 ++++++++++++++++++++++++++++++++++++\n> > > > subprojects/pybind11.wrap | 12 +\n> > > > 9 files changed, 498 insertions(+)\n> > > > create mode 100644 src/py/meson.build\n> > > > create mode 100644 src/py/pycamera/__init__.py\n> > > > create mode 100644 src/py/pycamera/meson.build\n> > > > create mode 100644 src/py/pycamera/pymain.cpp\n> > > > create mode 100644 subprojects/pybind11.wrap\n> > > >\n> > > > diff --git a/.gitignore b/.gitignore\n> > > > index cca829fa..aae56b2d 100644\n> > > > --- a/.gitignore\n> > > > +++ b/.gitignore\n> > > > @@ -3,6 +3,7 @@\n> > > > __pycache__/\n> > > > build/\n> > > > patches/\n> > > > +subprojects/pybind11-2.*/\n\nWe have a .gitignore in subprojects/ that can be used for this.\n\n> > > > *.patch\n> > > > *.pyc\n> > > > .cache\n> > > > diff --git a/meson.build b/meson.build\n> > > > index a20cc29e..0f885c14 100644\n> > > > --- a/meson.build\n> > > > +++ b/meson.build\n> > > > @@ -181,6 +181,7 @@ summary({\n> > > > 'qcam application': qcam_enabled,\n> > > > 'lc-compliance application': lc_compliance_enabled,\n> > > > 'Unit tests': test_enabled,\n> > > > + 'Python bindings': pycamera_enabled,\n\nCould you please move this with the adaptation layers, between GStreamer\nand V4L2 ?\n\n> > > > },\n> > > > section : 'Configuration',\n> > > > bool_yn : true)\n> > > > diff --git a/meson_options.txt b/meson_options.txt\n> > > > index 2c80ad8b..ca00c78e 100644\n> > > > --- a/meson_options.txt\n> > > > +++ b/meson_options.txt\n> > > > @@ -58,3 +58,8 @@ option('v4l2',\n> > > > type : 'boolean',\n> > > > value : false,\n> > > > description : 'Compile the V4L2 compatibility layer')\n> > > > +\n> > > > +option('pycamera',\n> > > > + type : 'feature',\n> > > > + value : 'auto',\n> > > > + description : 'Enable libcamera Python bindings (experimental)')\n> > > > diff --git a/src/meson.build b/src/meson.build\n> > > > index e0ea9c35..4a1e3cb0 100644\n> > > > --- a/src/meson.build\n> > > > +++ b/src/meson.build\n> > > > @@ -38,3 +38,4 @@ subdir('qcam')\n> > > >\n> > > > subdir('gstreamer')\n> > > > subdir('v4l2')\n> > > > +subdir('py')\n\nAlphabetical order please.\n\n> > > > diff --git a/src/py/meson.build b/src/py/meson.build\n> > > > new file mode 100644\n> > > > index 00000000..42ffa221\n> > > > --- /dev/null\n> > > > +++ b/src/py/meson.build\n> > > > @@ -0,0 +1 @@\n> > > > +subdir('pycamera')\n> > > > diff --git a/src/py/pycamera/__init__.py b/src/py/pycamera/__init__.py\n> > > > new file mode 100644\n> > > > index 00000000..a5c198dc\n> > > > --- /dev/null\n> > > > +++ b/src/py/pycamera/__init__.py\n> > > > @@ -0,0 +1,10 @@\n> > > > +# SPDX-License-Identifier: GPL-2.0-or-later\n\nShouldn't this be LGPL, as src/py/pycamera/pymain.cpp ?\n\n> > > > +# Copyright (C) 2021, Tomi Valkeinen \n> > > > +\n> > > > +from .pycamera import *\n> > > > +import mmap\n> > > > +\n> > > > +def __FrameBuffer__mmap(self, plane):\n> > > > + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)\n\nNo need to validate plane ?\n\nI'm also not sure extending the FrameBuffer class API this way is the\nbest option (there are reasons why the C++ class doesn't implement\nmmap()). It's fine for now as the Python bindings are work in progress,\nbut among the things that will need to be handled if we extend the\nPython API beyond what the C++ API provides is documentation.\n\n> > > > +\n> > > > +FrameBuffer.mmap = __FrameBuffer__mmap\n> > > > diff --git a/src/py/pycamera/meson.build b/src/py/pycamera/meson.build\n> > > > new file mode 100644\n> > > > index 00000000..c490a18d\n> > > > --- /dev/null\n> > > > +++ b/src/py/pycamera/meson.build\n> > > > @@ -0,0 +1,43 @@\n> > > > +# SPDX-License-Identifier: CC0-1.0\n> > > > +\n> > > > +py3_dep = dependency('python3', required : get_option('pycamera'))\n> > > > +\n> > > > +if not py3_dep.found()\n> > > > + pycamera_enabled = false\n> > > > + subdir_done()\n> > > > +endif\n> > > > +\n> > > > +pycamera_enabled = true\n> > > > +\n> > > > +pybind11_proj = subproject('pybind11')\n> > > > +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')\n> > > > +\n> > > > +pycamera_sources = files([\n> > > > + 'pymain.cpp',\n> > > > +])\n> > > > +\n> > > > +pycamera_deps = [\n> > > > + libcamera_public,\n> > > > + py3_dep,\n> > > > + pybind11_dep,\n> > > > +]\n> > > > +\n> > > > +pycamera_args = [ '-fvisibility=hidden' ]\n\nNo space after [ and before ] (that's the meson coding style AFAIK).\n\n> > > > +pycamera_args += [ '-Wno-shadow' ]\n> > >\n> > > Does python shadow variables explicitly? Does this lead to any potential\n> > > bugs?\n> > >\n> > > I recall putting -Wshadow in explicitly because I hit a nasty bug due to\n> > > shadowing, so in my eyes, shadowed variables are a bad-thing (tm) :-)\n> > >\n> > > You have to be sure you know which variable you are writing to,\n> > > and it might not always be clear to a reader, or cause potential\n> > > confusion..?\n> > >\n> > > (of course subclassing, and having functions with the same name is\n> > > essentially 'shadowing' the function names I guess...)\n> > >\n> > > > +\n> > > > +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/pycamera'\n> > > > +\n> > > > +pycamera = shared_module('pycamera',\n> > > > + pycamera_sources,\n> > > > + install : true,\n> > > > + install_dir : destdir,\n> > > > + name_prefix : '',\n> > > > + dependencies : pycamera_deps,\n> > > > + cpp_args : pycamera_args)\n> > > > +\n> > > > +# XXX > You could also create a symlink. There's an example in the top-level\n> > > > +# > meson.build.\n> > > > +# Copy __init__.py to build dir so that we can run without installing\n> > > > +configure_file(input: '__init__.py', output: '__init__.py', copy: true)\n> > >\n> > > I think a symlink would be better too.\n\nWhat do meson-based Python projects usually do ? I could imagine having\nmore .py files in the future, do we need to copy or symlink them all\nmanually, could there be another option ?\n\n> > > > +\n> > > > +install_data(['__init__.py'], install_dir : destdir)\n> > > > diff --git a/src/py/pycamera/pymain.cpp b/src/py/pycamera/pymain.cpp\n> > > > new file mode 100644\n> > > > index 00000000..0088e133\n> > > > --- /dev/null\n> > > > +++ b/src/py/pycamera/pymain.cpp\n> > \n> > pylibcamera?? :)\n\nI've replied to the naming question in a different patch of this series,\nlet's continue there.\n\n> > > > @@ -0,0 +1,424 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2021, Tomi Valkeinen \n> > > > + *\n> > > > + * Python bindings\n> > > > + */\n> > > > +\n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +\n> > > > +#include \n> > > > +\n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +\n> > > > +namespace py = pybind11;\n> > > > +\n> > > > +using namespace std;\n\nIf it's not too much churn, I'd prefer limiting usage of \"using\nnamespace\" and use the std:: prefix explicitly.\n\n> > > > +using namespace libcamera;\n> > > > +\n> > > > +static py::object ControlValueToPy(const ControlValue &cv)\n> > > > +{\n> > > > + //assert(!cv.isArray());\n> > > > + //assert(cv.numElements() == 1);\n> > >\n> > > Are these asserts not necessary? Is it better to keep them in, in a way\n> > > that they are used on debug buidls and compiled out on release builds?\n\nThey should be enabled or removed in any case :-)\n\n> > > I think we have ASSERT() for that, but that's an 'internal' helper I\n> > > think and I expect the src/py is building on top of the public api..\n\nASSERT() integrates with the logging infrastructure and the backtrace\ngeneration, so it's nice to use, but I can also imagine and accept that\nlanguage bindings may need specific rules.\n\n> > > > +\n> > > > + switch (cv.type()) {\n> > > > + case ControlTypeBool:\n> > > > + return py::cast(cv.get());\n> > > > + case ControlTypeByte:\n> > > > + return py::cast(cv.get());\n> > > > + case ControlTypeInteger32:\n> > > > + return py::cast(cv.get());\n> > > > + case ControlTypeInteger64:\n> > > > + return py::cast(cv.get());\n> > > > + case ControlTypeFloat:\n> > > > + return py::cast(cv.get());\n> > > > + case ControlTypeString:\n> > > > + return py::cast(cv.get());\n> > > > + case ControlTypeRectangle: {\n> > > > + const Rectangle *v = reinterpret_cast(cv.data().data());\n> > > > + return py::make_tuple(v->x, v->y, v->width, v->height);\n> > > > + }\n> > > > + case ControlTypeSize: {\n> > > > + const Size *v = reinterpret_cast(cv.data().data());\n> > > > + return py::make_tuple(v->width, v->height);\n> > > > + }\n> > > > + case ControlTypeNone:\n> > > > + default:\n> > > > + throw runtime_error(\"Unsupported ControlValue type\");\n> > > > + }\n> > > > +}\n> > > > +\n> > > > +static ControlValue PyToControlValue(const py::object &ob, ControlType type)\n> > > > +{\n> > > > + switch (type) {\n> > > > + case ControlTypeBool:\n> > > > + return ControlValue(ob.cast());\n> > > > + case ControlTypeByte:\n> > > > + return ControlValue(ob.cast());\n> > > > + case ControlTypeInteger32:\n> > > > + return ControlValue(ob.cast());\n> > > > + case ControlTypeInteger64:\n> > > > + return ControlValue(ob.cast());\n> > > > + case ControlTypeFloat:\n> > > > + return ControlValue(ob.cast());\n> > > > + case ControlTypeString:\n> > > > + return ControlValue(ob.cast());\n> > > > + case ControlTypeRectangle:\n> > > > + case ControlTypeSize:\n> > > > + case ControlTypeNone:\n> > > > + default:\n> > > > + throw runtime_error(\"Control type not implemented\");\n> > > > + }\n> > > > +}\n> > \n> > I wonder whether we should be supporting array controls even in the\n> > initial version? Some of the other things (like transforms) simply\n> > manifest themselves as \"features that you don't get\", but I found that\n> > any attempts to use metadata basically resulted in failures until I\n> > bodged the array controls to work.\n> > \n> > What do you thinK?\n\nPatches are welcome :-)\n\n> > > > +\n> > > > +static weak_ptr g_camera_manager;\n> > > > +static int g_eventfd;\n> > > > +static mutex g_reqlist_mutex;\n> > > > +static vector g_reqlist;\n> > > > +\n> > > > +static void handle_request_completed(Request *req)\n\nhandleRequestCompleted to match the libcamera coding style ?\n\n> > > > +{\n> > > > + {\n> > > > + lock_guard guard(g_reqlist_mutex);\n> > > > + g_reqlist.push_back(req);\n> > > > + }\n> > > > +\n> > > > + uint64_t v = 1;\n> > > > + write(g_eventfd, &v, 8);\n> > > > +}\n> > > > +\n> > > > +PYBIND11_MODULE(pycamera, m)\n> > > > +{\n> > > > + m.def(\"logSetLevel\", &logSetLevel);\n> > > > +\n> > > > + py::class_>(m, \"CameraManager\")\n> > > > + .def_static(\"singleton\", []() {\n> > > > + shared_ptr cm = g_camera_manager.lock();\n> > > > + if (cm)\n> > > > + return cm;\n> > > > +\n> > > > + int fd = eventfd(0, 0);\n> > > > + if (fd == -1)\n> > > > + throw std::system_error(errno, std::generic_category(), \"Failed to create eventfd\");\n> > > > +\n> > > > + cm = shared_ptr(new CameraManager, [](auto p) {\n> > > > + close(g_eventfd);\n> > > > + g_eventfd = -1;\n> > > > + delete p;\n> > > > + });\n> > > > +\n> > > > + g_eventfd = fd;\n> > > > + g_camera_manager = cm;\n> > > > +\n> > > > + int ret = cm->start();\n> > > > + if (ret)\n> > > > + throw std::system_error(-ret, std::generic_category(), \"Failed to start CameraManager\");\n> > > > +\n> > > > + return cm;\n> > > > + })\n> > > > +\n> > > > + .def_property_readonly(\"version\", &CameraManager::version)\n> > > > +\n> > > > + .def_property_readonly(\"efd\", [](CameraManager &) {\n> > > > + return g_eventfd;\n> > > > + })\n\nI understand why this is needed (documentation is required though), but\nwhy is the eventfd exposed by the CameraManager class and not the Camera\nclass ? Moving it to Camera would also allow moving the requests lists\nthere, and making them member variables instead of global variables.\n\nI'm also tempted to look at asyncio and other frameworks, as it seems\nthat callbacks from a different thread are a common enough issue that it\nshould have generic solutions. There are more signals than request\ncompletion that need to be exposed (buffer completion is another\nimportant one, so is camera hotplug), and I'm sure we'll have even more\nthe future.\n\nA completely generic solution isn't a blocker to get this merged (it's\nwork in progress, the API isn't stable) but will likely be required at\nsome point.\n\n> > > > +\n> > > > + .def(\"getReadyRequests\", [](CameraManager &) {\n\nI need to bring this up:\nhttps://www.python.org/dev/peps/pep-0008/#method-names-and-instance-variables.\nShould we follow the recommended Python convention of snake_case names\nfor methods, or stick to the libcamera naming convention ? I have a\nfeeling we'll end up with the latter.\n\n> > > > + vector v;\n> > > > +\n> > > > + {\n> > > > + lock_guard guard(g_reqlist_mutex);\n> > > > + swap(v, g_reqlist);\n> > > > + }\n> > > > +\n> > > > + vector ret;\n> > > > +\n> > > > + for (Request *req : v) {\n> > > > + py::object o = py::cast(req);\n> > > > + // decrease the ref increased in Camera::queueRequest()\n> > > > + o.dec_ref();\n> > > > + ret.push_back(o);\n> > > > + }\n> > > > +\n> > > > + return ret;\n> > > > + })\n> > > > +\n> > > > + .def(\"get\", py::overload_cast(&CameraManager::get), py::keep_alive<0, 1>())\n> > > > +\n> > > > + .def(\"find\", [](CameraManager &self, string str) {\n> > > > + std::transform(str.begin(), str.end(), str.begin(), ::tolower);\n> > > > +\n> > > > + for (auto c : self.cameras()) {\n> > > > + string id = c->id();\n> > > > +\n> > > > + std::transform(id.begin(), id.end(), id.begin(), ::tolower);\n> > > > +\n> > > > + if (id.find(str) != string::npos)\n> > > > + return c;\n> > > > + }\n> > > > +\n> > > > + return shared_ptr();\n> > > > + }, py::keep_alive<0, 1>())\n\nThe C++ camera enumeration API isn't the greatest, if there are\nimprovements that could be made there that would also benefit the Python\nAPI, feedback and proposals are welcome.\n\n> > > > +\n> > > > + // Create a list of Cameras, where each camera has a keep-alive to CameraManager\n\nLet's keep the comment style consistent with libcamera please :-)\n\n> > > > + .def_property_readonly(\"cameras\", [](CameraManager &self) {\n> > > > + py::list l;\n> > > > +\n> > > > + for (auto &c : self.cameras()) {\n> > > > + py::object py_cm = py::cast(self);\n> > > > + py::object py_cam = py::cast(c);\n> > > > + py::detail::keep_alive_impl(py_cam, py_cm);\n> > > > + l.append(py_cam);\n> > > > + }\n> > > > +\n> > > > + return l;\n> > > > + });\n> > > > +\n> > > > + py::class_>(m, \"Camera\")\n\nAlso discussed in a different mail in this thread is whether or not we\nshould create a custom wrapper class for std::shared_ptr. Let's\ncontinue the discussion there.\n\n> > > > + .def_property_readonly(\"id\", &Camera::id)\n> > > > + .def(\"acquire\", &Camera::acquire)\n> > > > + .def(\"release\", &Camera::release)\n> > > > + .def(\"start\", [](shared_ptr &self) {\n> > \n> > This is the one that needs to take a dictionary of controls, but we\n> > can add that later!\n\nIf it's not too difficult (and especially as David has an implementation\nalready), it could be done in v4 (as there will be a v4 to address\nmiscellaneous small review comments).\n\n> > > > + self->requestCompleted.connect(handle_request_completed);\n> > > > +\n> > > > + int ret = self->start();\n> > > > + if (ret)\n> > > > + self->requestCompleted.disconnect(handle_request_completed);\n> > > > +\n> > > > + return ret;\n> > > > + })\n> > > > +\n> > > > + .def(\"stop\", [](shared_ptr &self) {\n> > > > + int ret = self->stop();\n> > > > + if (!ret)\n> > > > + self->requestCompleted.disconnect(handle_request_completed);\n> > > > +\n> > > > + return ret;\n> > > > + })\n> > > > +\n> > > > + .def(\"__repr__\", [](shared_ptr &self) {\n> > > > + return \"id() + \"'>\";\n> > > > + })\n> > > > +\n> > > > + // Keep the camera alive, as StreamConfiguration contains a Stream*\n> > > > + .def(\"generateConfiguration\", &Camera::generateConfiguration, py::keep_alive<0, 1>())\n> > \n> > I was sometimes finding generateConfiguration a bit awkward because\n> > it's the only way to create a CameraConfiguration (I think, maybe I'm\n> > wrong?), and it always needs a list of stream roles. So I seem to\n> \n> It is the only way to 'create' a CameraConfiguration, as it has to be\n> owned by the Camera I believe, but it shouldn't require a list of\n> StreamRoles.\n> \n> The stream roles can be empty, and the streams added (or removed?)\n> manually after.\n> \n> And you should be able to reuse it while negotiating with the Camera -\n> you wouldn't hve to recreate new ones each time do you? So it's not\n> something I'd expect to happen a lot...\n> \n> > spend a lot of my time converting Python dicts (what the user sees) to\n> > CameraConfigurations and vice versa. I guess I'm not sure what I'm\n> > asking for, perhaps I just need to try and tidy up my translation code\n> > which has become a bit too twisty.\n> \n> Or perhaps this is more refering to how the CameraConfiguration is being\n> filled in by your layer ...?\n\nThe configuration API is in need of a big rework. Seeing the trouble\nthat Python code has with the current API will be useful.\n\n> > > > + .def(\"configure\", &Camera::configure)\n> > > > +\n> > > > + .def(\"createRequest\", &Camera::createRequest, py::arg(\"cookie\") = 0)\n> > > > +\n> > > > + .def(\"queueRequest\", [](Camera &self, Request *req) {\n> > > > + py::object py_req = py::cast(req);\n> > > > +\n> > > > + py_req.inc_ref();\n> > > > +\n> > > > + int ret = self.queueRequest(req);\n> > > > + if (ret)\n> > > > + py_req.dec_ref();\n> > > > +\n> > > > + return ret;\n> > > > + })\n> > > > +\n> > > > + .def_property_readonly(\"streams\", [](Camera &self) {\n> > > > + py::set set;\n> > > > + for (auto &s : self.streams()) {\n> > > > + py::object py_self = py::cast(self);\n> > > > + py::object py_s = py::cast(s);\n> > > > + py::detail::keep_alive_impl(py_s, py_self);\n> > > > + set.add(py_s);\n> > > > + }\n> > > > + return set;\n> > > > + })\n> > > > +\n> > > > + .def_property_readonly(\"controls\", [](Camera &self) {\n> > > > + py::dict ret;\n> > > > +\n> > > > + for (const auto &[id, ci] : self.controls()) {\n> > > > + ret[id->name().c_str()] = make_tuple(ControlValueToPy(ci.min()),\n> > > > + ControlValueToPy(ci.max()),\n> > > > + ControlValueToPy(ci.def()));\n> > > > + }\n> > > > +\n> > > > + return ret;\n> > > > + })\n> > > > +\n> > > > + .def_property_readonly(\"properties\", [](Camera &self) {\n> > > > + py::dict ret;\n> > > > +\n> > > > + for (const auto &[key, cv] : self.properties()) {\n> > > > + const ControlId *id = properties::properties.at(key);\n> > > > + py::object ob = ControlValueToPy(cv);\n> > > > +\n> > > > + ret[id->name().c_str()] = ob;\n> > > > + }\n> > > > +\n> > > > + return ret;\n> > > > + });\n> > > > +\n> > > > + py::enum_(m, \"ConfigurationStatus\")\n> > > > + .value(\"Valid\", CameraConfiguration::Valid)\n> > > > + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n> > > > + .value(\"Invalid\", CameraConfiguration::Invalid);\n> > > > +\n> > > > + py::class_(m, \"CameraConfiguration\")\n> > > > + .def(\"__iter__\", [](CameraConfiguration &self) {\n> > > > + return py::make_iterator(self);\n> > > > + }, py::keep_alive<0, 1>())\n> > > > + .def(\"__len__\", [](CameraConfiguration &self) {\n> > > > + return self.size();\n> > > > + })\n> > > > + .def(\"validate\", &CameraConfiguration::validate)\n> > > > + .def(\"at\", py::overload_cast(&CameraConfiguration::at), py::return_value_policy::reference_internal)\n> > > > + .def_property_readonly(\"size\", &CameraConfiguration::size)\n> > > > + .def_property_readonly(\"empty\", &CameraConfiguration::empty);\n> > > > +\n> > > > + py::class_(m, \"StreamConfiguration\")\n> > > > + .def(\"toString\", &StreamConfiguration::toString)\n> > > > + .def_property_readonly(\"stream\", &StreamConfiguration::stream, py::return_value_policy::reference_internal)\n> > > > + .def_property(\n> > > > + \"size\",\n> > > > + [](StreamConfiguration &self) { return make_tuple(self.size.width, self.size.height); },\n> > > > + [](StreamConfiguration &self, tuple size) { self.size.width = get<0>(size); self.size.height = get<1>(size); })\n> > > > + .def_property(\n> > > > + \"fmt\",\n\n\"pixelFormat\" for consistency ?\n\n> > > > + [](StreamConfiguration &self) { return self.pixelFormat.toString(); },\n> > > > + [](StreamConfiguration &self, string fmt) { self.pixelFormat = PixelFormat::fromString(fmt); })\n> > > > + .def_readwrite(\"stride\", &StreamConfiguration::stride)\n> > > > + .def_readwrite(\"frameSize\", &StreamConfiguration::frameSize)\n> > > > + .def_readwrite(\"bufferCount\", &StreamConfiguration::bufferCount)\n> > > > + .def_property_readonly(\"formats\", &StreamConfiguration::formats, py::return_value_policy::reference_internal);\n> > > > + ;\n> > > > +\n> > > > + py::class_(m, \"StreamFormats\")\n> > > > + .def_property_readonly(\"pixelFormats\", [](StreamFormats &self) {\n> > > > + vector fmts;\n> > > > + for (auto &fmt : self.pixelformats())\n> > > > + fmts.push_back(fmt.toString());\n> > > > + return fmts;\n> > > > + })\n> > > > + .def(\"sizes\", [](StreamFormats &self, const string &pixelFormat) {\n> > > > + auto fmt = PixelFormat::fromString(pixelFormat);\n> > > > + vector> fmts;\n> > > > + for (const auto &s : self.sizes(fmt))\n> > > > + fmts.push_back(make_tuple(s.width, s.height));\n> > > > + return fmts;\n> > > > + })\n> > > > + .def(\"range\", [](StreamFormats &self, const string &pixelFormat) {\n> > > > + auto fmt = PixelFormat::fromString(pixelFormat);\n> > > > + const auto &range = self.range(fmt);\n> > > > + return make_tuple(make_tuple(range.hStep, range.vStep),\n> > > > + make_tuple(range.min.width, range.min.height),\n> > > > + make_tuple(range.max.width, range.max.height));\n> > > > + });\n> > > > +\n> > > > + py::enum_(m, \"StreamRole\")\n> > > > + .value(\"StillCapture\", StreamRole::StillCapture)\n> > > > + .value(\"Raw\", StreamRole::Raw)\n> > > > + .value(\"VideoRecording\", StreamRole::VideoRecording)\n> > > > + .value(\"Viewfinder\", StreamRole::Viewfinder);\n> > > > +\n> > > > + py::class_(m, \"FrameBufferAllocator\")\n> > > > + .def(py::init>(), py::keep_alive<1, 2>())\n> > > > + .def(\"allocate\", &FrameBufferAllocator::allocate)\n> > > > + .def_property_readonly(\"allocated\", &FrameBufferAllocator::allocated)\n> > > > + // Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator\n> > > > + .def(\"buffers\", [](FrameBufferAllocator &self, Stream *stream) {\n> > > > + py::object py_self = py::cast(self);\n> > > > + py::list l;\n> > > > + for (auto &ub : self.buffers(stream)) {\n> > > > + py::object py_buf = py::cast(ub.get(), py::return_value_policy::reference_internal, py_self);\n> > > > + l.append(py_buf);\n> > > > + }\n> > > > + return l;\n> > > > + });\n> > > > +\n> > > > + py::class_(m, \"FrameBuffer\")\n> > > > + // TODO: implement FrameBuffer::Plane properly\n> > > > + .def(py::init([](vector> planes, unsigned int cookie) {\n> > > > + vector v;\n> > > > + for (const auto &t : planes)\n> > > > + v.push_back({ SharedFD(get<0>(t)), FrameBuffer::Plane::kInvalidOffset, get<1>(t) });\n> > > > + return new FrameBuffer(v, cookie);\n> > > > + }))\n> > > > + .def_property_readonly(\"metadata\", &FrameBuffer::metadata, py::return_value_policy::reference_internal)\n> > > > + .def(\"length\", [](FrameBuffer &self, uint32_t idx) {\n> > > > + const FrameBuffer::Plane &plane = self.planes()[idx];\n> > > > + return plane.length;\n> > > > + })\n> > > > + .def(\"fd\", [](FrameBuffer &self, uint32_t idx) {\n> > > > + const FrameBuffer::Plane &plane = self.planes()[idx];\n> > > > + return plane.fd.get();\n> > > > + })\n> > > > + .def_property(\"cookie\", &FrameBuffer::cookie, &FrameBuffer::setCookie);\n> > > > +\n> > > > + py::class_(m, \"Stream\")\n> > > > + .def_property_readonly(\"configuration\", &Stream::configuration);\n> > > > +\n> > > > + py::enum_(m, \"ReuseFlag\")\n> > > > + .value(\"Default\", Request::ReuseFlag::Default)\n> > > > + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n> > > > +\n> > > > + py::class_(m, \"Request\")\n> > > > + .def_property_readonly(\"camera\", &Request::camera)\n> > > > + .def(\"addBuffer\", &Request::addBuffer, py::keep_alive<1, 3>()) // Request keeps Framebuffer alive\n> > > > + .def_property_readonly(\"status\", &Request::status)\n> > > > + .def_property_readonly(\"buffers\", &Request::buffers)\n> > > > + .def_property_readonly(\"cookie\", &Request::cookie)\n> > > > + .def_property_readonly(\"hasPendingBuffers\", &Request::hasPendingBuffers)\n> > > > + .def(\"set_control\", [](Request &self, string &control, py::object value) {\n> > \n> > I found this one just a bit clunky to use, as I always have a\n> > dictionary of control values and then have to loop through them\n> > myself. If it took the dictionary (like my modified start method) that\n> > would be more convenient. And I suppose we'd call it set_controls, so\n> > we could actually have both.\n\nShould we expose the ControlList class in the Python API ?\n\n> > But these are all minor points, these bindings, perhaps with the\n> > exception of array controls, are already working very well for me.\n> > \n> > > > + const auto &controls = self.camera()->controls();\n> > > > +\n> > > > + auto it = find_if(controls.begin(), controls.end(),\n> > > > + [&control](const auto &kvp) { return kvp.first->name() == control; });\n> > > > +\n> > > > + if (it == controls.end())\n> > > > + throw runtime_error(\"Control not found\");\n> > > > +\n> > > > + const auto &id = it->first;\n> > > > +\n> > > > + self.controls().set(id->id(), PyToControlValue(value, id->type()));\n> > > > + })\n> > > > + .def_property_readonly(\"metadata\", [](Request &self) {\n> > > > + py::dict ret;\n> > > > +\n> > > > + for (const auto &[key, cv] : self.metadata()) {\n> > > > + const ControlId *id = controls::controls.at(key);\n> > > > + py::object ob = ControlValueToPy(cv);\n> > > > +\n> > > > + ret[id->name().c_str()] = ob;\n> > > > + }\n> > > > +\n> > > > + return ret;\n> > > > + })\n> > > > + // As we add a keep_alive to the fb in addBuffers(), we can only allow reuse with ReuseBuffers.\n> > > > + .def(\"reuse\", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });\n> > > > +\n> > > > + py::enum_(m, \"RequestStatus\")\n\nCould this be a member of the Python Request class, or would it be\nfrowned upon from a Python coding style point of view ? Same for\nFrameMetadataStatus bekiwn abd ReuseFlag above.\n\n> > > > + .value(\"Pending\", Request::RequestPending)\n> > > > + .value(\"Complete\", Request::RequestComplete)\n> > > > + .value(\"Cancelled\", Request::RequestCancelled);\n> > > > +\n> > > > + py::enum_(m, \"FrameMetadataStatus\")\n> > > > + .value(\"Success\", FrameMetadata::FrameSuccess)\n> > > > + .value(\"Error\", FrameMetadata::FrameError)\n> > > > + .value(\"Cancelled\", FrameMetadata::FrameCancelled);\n> > > > +\n> > > > + py::class_(m, \"FrameMetadata\")\n> > > > + .def_readonly(\"status\", &FrameMetadata::status)\n> > > > + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n> > > > + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n> > > > + .def_property_readonly(\"bytesused\", [](FrameMetadata &self) {\n\nWill this function go away when implementing support for planes ?\nThere's a todo comment above about that, let's add one here too.\n\n> > > > + vector v;\n> > > > + v.resize(self.planes().size());\n> > > > + transform(self.planes().begin(), self.planes().end(), v.begin(), [](const auto &p) { return p.bytesused; });\n> > > > + return v;\n> > > > + });\n> > > > +}\n> > > > diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> > > > new file mode 100644\n> > > > index 00000000..9d6e7acb\n> > > > --- /dev/null\n> > > > +++ b/subprojects/pybind11.wrap\n> > > > @@ -0,0 +1,12 @@\n> > > > +[wrap-file]\n> > > > +directory = pybind11-2.6.1\n> > > > +source_url = https://github.com/pybind/pybind11/archive/v2.6.1.tar.gz\n> > > > +source_filename = pybind11-2.6.1.tar.gz\n> > > > +source_hash = cdbe326d357f18b83d10322ba202d69f11b2f49e2d87ade0dc2be0c5c34f8e2a\n> > > > +patch_url = https://wrapdb.mesonbuild.com/v2/pybind11_2.6.1-1/get_patch\n> > > > +patch_filename = pybind11-2.6.1-1-wrap.zip\n> > > > +patch_hash = 6de5477598b56c8a2e609196420c783ac35b79a31d6622121602e6ade6b3cee8\n> > > > +\n> > > > +[provide]\n> > > > +pybind11 = pybind11_dep\n> > > > +\n\nExtra blank line.","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 BD183BF415\n\tfor ;\n\tThu, 9 Dec 2021 19:29:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6EEB60882;\n\tThu, 9 Dec 2021 20:29:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F85D607DE\n\tfor ;\n\tThu, 9 Dec 2021 20:29:03 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E03E1501;\n\tThu, 9 Dec 2021 20:29:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"b9LNzRqy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639078143;\n\tbh=mgGQ3MOCCLGJUUFSWHPODIX7zKkR/F304O0FwG/u5fQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b9LNzRqy59hhS8KshRFXnEan9CY5sUVo5GnRdpNWmzR3eGK3o7nvu2z9fwFBNpS8g\n\tXQ6LEEmNJt+7p4MZyaOM+lLcm13wIzNus02N1EBZ5fbRqJ+IkECzNPplErFP//mM5D\n\tRzlWRvhKwWsvSgBe8N+yU494tK3qXldRVPwwU4Ik=","Date":"Thu, 9 Dec 2021 21:28:33 +0200","From":"Laurent Pinchart ","To":"Tomi Valkeinen ","Message-ID":"","References":"<20211209092906.37303-1-tomi.valkeinen@ideasonboard.com>\n\t<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>\n\t<163904389093.2066819.16202667063304401995@Monstersaurus>\n\t\n\t<163905088555.2066819.3887909202891654895@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163905088555.2066819.3887909202891654895@Monstersaurus>","Subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21741,"web_url":"https://patchwork.libcamera.org/comment/21741/","msgid":"","date":"2021-12-10T12:27:59","subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 09/12/2021 21:28, Laurent Pinchart wrote:\n\n>>>>> + .def_property_readonly(\"efd\", [](CameraManager &) {\n>>>>> + return g_eventfd;\n>>>>> + })\n> \n> I understand why this is needed (documentation is required though), but\n> why is the eventfd exposed by the CameraManager class and not the Camera\n> class ? Moving it to Camera would also allow moving the requests lists\n> there, and making them member variables instead of global variables.\n\nBecause g_eventfd is a global variable and CameraManager is a singleton.\n\nWe don't have CameraManger or Camera classes in the python bindings, in \nthe sense that we could add fields to them. There's no \npython-bindings-specific-state for class instances. In other words, \neventfd is not part of CameraManager, it's just accessed via it.\n\nI've struggled with this multiple times, and I haven't figured out a \nsimple solution.\n\nWe can build new C++ classes that wrap the libcamera C++ classes, say, \nPyCamera for Camera, but then that affects all the places in the \nbindings where Camera instance is handled, producing possibly quite a \nbit of extra code. I haven't tried this out, but it's been in my mind as \nit would solve some problems.\n\n Tomi","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 6131FBF415\n\tfor ;\n\tFri, 10 Dec 2021 12:28:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8063260890;\n\tFri, 10 Dec 2021 13:28:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42C4760868\n\tfor ;\n\tFri, 10 Dec 2021 13:28:03 +0100 (CET)","from [192.168.1.111] (91-156-85-209.elisa-laajakaista.fi\n\t[91.156.85.209])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B8CF9F84;\n\tFri, 10 Dec 2021 13:28:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NOV6eVWY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639139283;\n\tbh=gvpIBdQuLXl+3w5GI4NYSaCbd2e2khvCnyj05Tkwa1k=;\n\th=To:Cc:References:From:Subject:Date:In-Reply-To:From;\n\tb=NOV6eVWYEqmuZUzegcgDGOzomCt8IDGTH3AVf2MQk+0qm5S7UuPFaJliqilj5HQQa\n\tU1aZv4V4nX8cPJ1vJd/pGWRM6yrN3EHwtohPtBjoz+xsNMbPE6Cyx6lVYWnTIzhplB\n\toUu2GaqIhpza/sMnT4aindgcw0vz/oU4Xvk/Xkms=","To":"Laurent Pinchart ","References":"<20211209092906.37303-1-tomi.valkeinen@ideasonboard.com>\n\t<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>\n\t<163904389093.2066819.16202667063304401995@Monstersaurus>\n\t\n\t<163905088555.2066819.3887909202891654895@Monstersaurus>\n\t","From":"Tomi Valkeinen ","Message-ID":"","Date":"Fri, 10 Dec 2021 14:27:59 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.14.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21750,"web_url":"https://patchwork.libcamera.org/comment/21750/","msgid":"","date":"2021-12-10T13:15:31","subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nOn Fri, Dec 10, 2021 at 02:27:59PM +0200, Tomi Valkeinen wrote:\n> On 09/12/2021 21:28, Laurent Pinchart wrote:\n> \n> >>>>> + .def_property_readonly(\"efd\", [](CameraManager &) {\n> >>>>> + return g_eventfd;\n> >>>>> + })\n> > \n> > I understand why this is needed (documentation is required though), but\n> > why is the eventfd exposed by the CameraManager class and not the Camera\n> > class ? Moving it to Camera would also allow moving the requests lists\n> > there, and making them member variables instead of global variables.\n> \n> Because g_eventfd is a global variable and CameraManager is a singleton.\n> \n> We don't have CameraManger or Camera classes in the python bindings, in \n> the sense that we could add fields to them. There's no \n> python-bindings-specific-state for class instances. In other words, \n> eventfd is not part of CameraManager, it's just accessed via it.\n> \n> I've struggled with this multiple times, and I haven't figured out a \n> simple solution.\n> \n> We can build new C++ classes that wrap the libcamera C++ classes, say, \n> PyCamera for Camera, but then that affects all the places in the \n> bindings where Camera instance is handled, producing possibly quite a \n> bit of extra code. I haven't tried this out, but it's been in my mind as \n> it would solve some problems.\n\nI have to say I lack experience in this area, but PyCamera C++ wrapper class\nsounds like it could really help. I expect it would expose a function to\nretrieve the Camera pointer, and I wonder if exposing that using a\ncustom `operator Camera *()` could help interfacing with code that\nexpects a Camera pointer, or if there are better ways. I'm sure we're\nnot the only ones dealing with this.","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 6C1FCBDB13\n\tfor ;\n\tFri, 10 Dec 2021 13:16:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6C546087E;\n\tFri, 10 Dec 2021 14:16:02 +0100 (CET)","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 81FE660868\n\tfor ;\n\tFri, 10 Dec 2021 14:16:00 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E1E5D1C03;\n\tFri, 10 Dec 2021 14:15:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"a91iU2mH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639142160;\n\tbh=oeKgMxwOE/SWMb6SaZA0P+SI7M1p95G9DuCo/x3Ck+4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=a91iU2mHAKxuZ6IZqAKD9BlhsvuH4u5lrjuY5/2Xo/zb41bNa/sSxYdrOinLWp0M6\n\t03FSzwDOn/n5bvdlbsZ6SHOPzd0LTkl4Rcp+fqjk4sdzXQ/KDDDMu17OHWqlcUFeNH\n\tLCAtWjvUEuRAdxeN4ReBZQqKprpTzVBqMYFKhBUg=","Date":"Fri, 10 Dec 2021 15:15:31 +0200","From":"Laurent Pinchart ","To":"Tomi Valkeinen ","Message-ID":"","References":"<20211209092906.37303-1-tomi.valkeinen@ideasonboard.com>\n\t<20211209092906.37303-5-tomi.valkeinen@ideasonboard.com>\n\t<163904389093.2066819.16202667063304401995@Monstersaurus>\n\t\n\t<163905088555.2066819.3887909202891654895@Monstersaurus>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC v3 4/5] Add Python bindings","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]