[{"id":22856,"web_url":"https://patchwork.libcamera.org/comment/22856/","msgid":"<165175869453.1423360.6614915904429692992@Monstersaurus>","date":"2022-05-05T13:51:34","subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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 (2022-05-05 11:40:55)\n> Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n> Python layer.\n> \n> We use pybind11 'smart_holder' version to avoid issues with private\n> destructors and shared_ptr. There is also an alternative solution here:\n> \n> https://github.com/pybind/pybind11/pull/2067\n> \n> Only a subset of libcamera classes are exposed. Implementing and testing\n> the wrapper classes is challenging, and as such only classes that I have\n> needed have been added so far.\n> \n> Signed-off-by: Tomi Valkeinen \n> ---\n> meson.build | 1 +\n> meson_options.txt | 5 +\n> src/meson.build | 1 +\n> src/py/libcamera/__init__.py | 12 +\n> src/py/libcamera/meson.build | 44 ++\n> src/py/libcamera/pyenums.cpp | 53 ++\n> src/py/libcamera/pymain.cpp | 452 ++++++++++++++++++\n> src/py/meson.build | 1 +\n> subprojects/.gitignore | 3 +-\n> subprojects/packagefiles/pybind11/meson.build | 8 +\n> subprojects/pybind11.wrap | 8 +\n> 11 files changed, 587 insertions(+), 1 deletion(-)\n> create mode 100644 src/py/libcamera/__init__.py\n> create mode 100644 src/py/libcamera/meson.build\n> create mode 100644 src/py/libcamera/pyenums.cpp\n> create mode 100644 src/py/libcamera/pymain.cpp\n> create mode 100644 src/py/meson.build\n> create mode 100644 subprojects/packagefiles/pybind11/meson.build\n> create mode 100644 subprojects/pybind11.wrap\n> \n> diff --git a/meson.build b/meson.build\n> index 0124e7d3..60a911e0 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -177,6 +177,7 @@ summary({\n> 'Tracing support': tracing_enabled,\n> 'Android support': android_enabled,\n> 'GStreamer support': gst_enabled,\n> + 'Python bindings': pycamera_enabled,\n> 'V4L2 emulation support': v4l2_enabled,\n> 'cam application': cam_enabled,\n> 'qcam application': qcam_enabled,\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..34663a6f 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -37,4 +37,5 @@ subdir('cam')\n> subdir('qcam')\n> \n> subdir('gstreamer')\n> +subdir('py')\n> subdir('v4l2')\n> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py\n> new file mode 100644\n> index 00000000..cd7512a2\n> --- /dev/null\n> +++ b/src/py/libcamera/__init__.py\n> @@ -0,0 +1,12 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +# Copyright (C) 2021, Tomi Valkeinen \n> +\n> +from ._libcamera import *\n> +import mmap\n> +\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> +\n> +FrameBuffer.mmap = __FrameBuffer__mmap\n> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> new file mode 100644\n> index 00000000..e1f5cf21\n> --- /dev/null\n> +++ b/src/py/libcamera/meson.build\n> @@ -0,0 +1,44 @@\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> + 'pyenums.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> +pycamera_args += ['-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT']\n> +\n> +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/libcamera'\n> +\n> +pycamera = shared_module('_libcamera',\n> + pycamera_sources,\n> + install : true,\n> + install_dir : destdir,\n> + name_prefix : '',\n> + dependencies : pycamera_deps,\n> + cpp_args : pycamera_args)\n> +\n> +run_command('ln', '-fsT', '../../../../src/py/libcamera/__init__.py',\n> + meson.current_build_dir() / '__init__.py',\n> + check: true)\n> +\n> +install_data(['__init__.py'], install_dir : destdir)\n> diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp\n> new file mode 100644\n> index 00000000..39886656\n> --- /dev/null\n> +++ b/src/py/libcamera/pyenums.cpp\n> @@ -0,0 +1,53 @@\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> +\n> +#include \n> +\n> +namespace py = pybind11;\n> +\n> +using namespace libcamera;\n> +\n> +void init_pyenums(py::module &m)\n> +{\n> + py::enum_(m, \"ConfigurationStatus\")\n> + .value(\"Valid\", CameraConfiguration::Valid)\n> + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n> + .value(\"Invalid\", CameraConfiguration::Invalid);\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::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::enum_(m, \"ReuseFlag\")\n> + .value(\"Default\", Request::ReuseFlag::Default)\n> + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n> +\n> + py::enum_(m, \"ControlType\")\n> + .value(\"None\", ControlType::ControlTypeNone)\n> + .value(\"Bool\", ControlType::ControlTypeBool)\n> + .value(\"Byte\", ControlType::ControlTypeByte)\n> + .value(\"Integer32\", ControlType::ControlTypeInteger32)\n> + .value(\"Integer64\", ControlType::ControlTypeInteger64)\n> + .value(\"Float\", ControlType::ControlTypeFloat)\n> + .value(\"String\", ControlType::ControlTypeString)\n> + .value(\"Rectangle\", ControlType::ControlTypeRectangle)\n> + .value(\"Size\", ControlType::ControlTypeSize);\n> +}\n> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp\n> new file mode 100644\n> index 00000000..54674caf\n> --- /dev/null\n> +++ b/src/py/libcamera/pymain.cpp\n> @@ -0,0 +1,452 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Tomi Valkeinen \n> + *\n> + * Python bindings\n> + */\n> +\n> +/*\n> + * To generate pylibcamera stubs:\n> + * PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera\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> +template\n> +static py::object ValueOrTuple(const ControlValue &cv)\n> +{\n> + if (cv.isArray()) {\n> + const T *v = reinterpret_cast(cv.data().data());\n> + auto t = py::tuple(cv.numElements());\n> +\n> + for (size_t i = 0; i < cv.numElements(); ++i)\n> + t[i] = v[i];\n> +\n> + return t;\n> + }\n> +\n> + return py::cast(cv.get());\n> +}\n> +\n> +static py::object ControlValueToPy(const ControlValue &cv)\n> +{\n> + switch (cv.type()) {\n> + case ControlTypeBool:\n> + return ValueOrTuple(cv);\n> + case ControlTypeByte:\n> + return ValueOrTuple(cv);\n> + case ControlTypeInteger32:\n> + return ValueOrTuple(cv);\n> + case ControlTypeInteger64:\n> + return ValueOrTuple(cv);\n> + case ControlTypeFloat:\n> + return ValueOrTuple(cv);\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 handleRequestCompleted(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> +void init_pyenums(py::module &m);\n> +\n> +PYBIND11_MODULE(_libcamera, m)\n> +{\n> + init_pyenums(m);\n> +\n> + /* Forward declarations */\n> +\n> + /*\n> + * We need to declare all the classes here so that Python docstrings\n> + * can be generated correctly.\n> + * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n> + */\n> +\n> + auto pyCameraManager = py::class_(m, \"CameraManager\");\n> + auto pyCamera = py::class_(m, \"Camera\");\n> + auto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n> + auto pyStreamConfiguration = py::class_(m, \"StreamConfiguration\");\n> + auto pyStreamFormats = py::class_(m, \"StreamFormats\");\n> + auto pyFrameBufferAllocator = py::class_(m, \"FrameBufferAllocator\");\n> + auto pyFrameBuffer = py::class_(m, \"FrameBuffer\");\n> + auto pyStream = py::class_(m, \"Stream\");\n> + auto pyControlId = py::class_(m, \"ControlId\");\n> + auto pyRequest = py::class_(m, \"Request\");\n> + auto pyFrameMetadata = py::class_(m, \"FrameMetadata\");\n> +\n> + /* Global functions */\n> + m.def(\"logSetLevel\", &logSetLevel);\n> +\n> + /* Classes */\n> + pyCameraManager\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> + pyCamera\n> + .def_property_readonly(\"id\", &Camera::id)\n> + .def(\"acquire\", &Camera::acquire)\n> + .def(\"release\", &Camera::release)\n> + .def(\"start\", [](Camera &self) {\n> + self.requestCompleted.connect(handleRequestCompleted);\n\nWhat happens if someone calls start() multiple times? Is that going to\nmean the signal / slot will be connected multiple times?\n\nThat could be problematic ... as I think it means it could then call the\nrequest completed handler multiple times - but it may also already be\ntrapped by the recent work we did around there.\n\nSo for now I think this is fine - but may be something to test or\nconsider later.\n\n\n> +\n> + int ret = self.start();\n> + if (ret)\n> + self.requestCompleted.disconnect(handleRequestCompleted);\n> +\n> + return ret;\n> + })\n> +\n> + .def(\"stop\", [](Camera &self) {\n> + int ret = self.stop();\n> + if (!ret)\n> + self.requestCompleted.disconnect(handleRequestCompleted);\n> +\n> + return ret;\n> + })\n> +\n> + .def(\"__repr__\", [](Camera &self) {\n> + return \"\";\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(\"find_control\", [](Camera &self, const string &name) {\n> + const auto &controls = self.controls();\n> +\n> + auto it = find_if(controls.begin(), controls.end(),\n> + [&name](const auto &kvp) { return kvp.first->name() == name; });\n> +\n> + if (it == controls.end())\n> + throw runtime_error(\"Control not found\");\n> +\n> + return it->first;\n> + }, py::return_value_policy::reference_internal)\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> + pyCameraConfiguration\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> + pyStreamConfiguration\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> + \"pixelFormat\",\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> + pyStreamFormats\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> + pyFrameBufferAllocator\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> + pyFrameBuffer\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> + pyStream\n> + .def_property_readonly(\"configuration\", &Stream::configuration);\n> +\n> + pyControlId\n> + .def_property_readonly(\"id\", &ControlId::id)\n> + .def_property_readonly(\"name\", &ControlId::name)\n> + .def_property_readonly(\"type\", &ControlId::type);\n> +\n> + pyRequest\n> + /* Fence is not supported, so we cannot expose addBuffer() directly */\n> + .def(\"addBuffer\", [](Request &self, const Stream *stream, FrameBuffer *buffer) {\n> + return self.addBuffer(stream, buffer);\n> + }, 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, ControlId &id, py::object value) {\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> + pyFrameMetadata\n> + .def_readonly(\"status\", &FrameMetadata::status)\n> + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n> + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n> + /* temporary helper, to be removed */\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/src/py/meson.build b/src/py/meson.build\n> new file mode 100644\n> index 00000000..4ce9668c\n> --- /dev/null\n> +++ b/src/py/meson.build\n> @@ -0,0 +1 @@\n> +subdir('libcamera')\n> diff --git a/subprojects/.gitignore b/subprojects/.gitignore\n> index 391fde2c..0e194289 100644\n> --- a/subprojects/.gitignore\n> +++ b/subprojects/.gitignore\n> @@ -1,3 +1,4 @@\n> /googletest-release*\n> /libyuv\n> -/packagecache\n> \\ No newline at end of file\n> +/packagecache\n> +/pybind11\n> diff --git a/subprojects/packagefiles/pybind11/meson.build b/subprojects/packagefiles/pybind11/meson.build\n> new file mode 100644\n> index 00000000..67e89aec\n> --- /dev/null\n> +++ b/subprojects/packagefiles/pybind11/meson.build\n> @@ -0,0 +1,8 @@\n> +project('pybind11', 'cpp',\n> + version : '2.9.1',\n> + license : 'BSD-3-Clause')\n> +\n> +pybind11_incdir = include_directories('include')\n> +\n> +pybind11_dep = declare_dependency(\n> + include_directories : pybind11_incdir)\n> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> new file mode 100644\n> index 00000000..2413e9ca\n> --- /dev/null\n> +++ b/subprojects/pybind11.wrap\n> @@ -0,0 +1,8 @@\n> +[wrap-git]\n> +url = https://github.com/pybind/pybind11.git\n> +revision = 82734801f23314b4c34d70a79509e060a2648e04\n\nGreat - this is now pointing directly to the pybind sources so I think\nthat's much cleaner:\n\nIt's hard to do a dedicated review on so much that I honestly don't\nfully comprehend yet - but I think getting this in and working/building\non top is better for everyone, and as far as I know - it's already quite\nwell tested by RPi (albeit, an older version, so I hope David can rebase\nand test sometime soon).\n\n\nReviewed-by: Kieran Bingham \n\n\n\n> +depth = 1\n> +patch_directory = pybind11\n> +\n> +[provide]\n> +pybind11 = pybind11_dep\n> -- \n> 2.34.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 493AEC0F2A\n\tfor ;\n\tThu, 5 May 2022 13:51:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8F87265643;\n\tThu, 5 May 2022 15:51:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94B95603AB\n\tfor ;\n\tThu, 5 May 2022 15:51:37 +0200 (CEST)","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 DC638492;\n\tThu, 5 May 2022 15:51:36 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651758699;\n\tbh=yU5ZrHuJuDUqYIQG9gp9VcrbWfCAhEN2azgIzTJgxcU=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=E6wAfkDywCnOKKMn2lukhS2UWQZjcdEznHd/T4vFwk5INo1S+Zfcbmt1T33OPmlIX\n\tKcRhseSAQcz5lghpaxqviImSa9RczVEgoBaWdwLBXr6stefVq+/0qUlibNv5pM8OfP\n\thqjkZmjxvVtY56E96iKI1D38A94V2dRcvF0mQ1rOvvRDfz8cGQYlyWVyCuZkXM8hns\n\t54/sd1QlV2dtl2Ihlr4oA/zbzinC0jD9auGMDM5tlshWXiVnrJVsAaCdo4dxb4Wsmk\n\tZNZOpTiy59wVHL9lciGm2HRGRHCUwGaZ8uZ+ioOMYPnW3UCrFSU1Nq9zKDTufZCdAs\n\tzAMggsR7Eh9Bg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651758697;\n\tbh=yU5ZrHuJuDUqYIQG9gp9VcrbWfCAhEN2azgIzTJgxcU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=XGbRDDeO5Fbiq7aA7pp7rN2J2vCxxlltL5UTlzo0lzgoqXjsSJcnbnHj8m7ygfcM3\n\tEVncEvvtzf4qODfOuFmi/jmp2IT1ommaiAvkFA9Vw6LElS3Eh9EnSPn3rgI9NcjGPI\n\tVmSZRiyVXhWQl3Rd0VCQ+Pf85cnX6GDqo/ZSNT0Q="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XGbRDDeO\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220505104104.70841-5-tomi.valkeinen@ideasonboard.com>","References":"<20220505104104.70841-1-tomi.valkeinen@ideasonboard.com>\n\t<20220505104104.70841-5-tomi.valkeinen@ideasonboard.com>","To":"David Plowman ,\n\tJacopo Mondi ,\n\tLaurent Pinchart ,\n\tTomi Valkeinen ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 05 May 2022 14:51:34 +0100","Message-ID":"<165175869453.1423360.6614915904429692992@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22857,"web_url":"https://patchwork.libcamera.org/comment/22857/","msgid":"<67cf0db7-11dc-1212-1642-24504fa4ec5d@ideasonboard.com>","date":"2022-05-05T14:03:59","subject":"Re: [libcamera-devel] [PATCH v7 04/13] Add Python bindings","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 05/05/2022 16:51, Kieran Bingham wrote:\n\n>> + pyCamera\n>> + .def_property_readonly(\"id\", &Camera::id)\n>> + .def(\"acquire\", &Camera::acquire)\n>> + .def(\"release\", &Camera::release)\n>> + .def(\"start\", [](Camera &self) {\n>> + self.requestCompleted.connect(handleRequestCompleted);\n> \n> What happens if someone calls start() multiple times? Is that going to\n> mean the signal / slot will be connected multiple times?\n> \n> That could be problematic ... as I think it means it could then call the\n> request completed handler multiple times - but it may also already be\n> trapped by the recent work we did around there.\n> \n> So for now I think this is fine - but may be something to test or\n> consider later.\n\nGood point. I'll look at it.\n\n>> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n>> new file mode 100644\n>> index 00000000..2413e9ca\n>> --- /dev/null\n>> +++ b/subprojects/pybind11.wrap\n>> @@ -0,0 +1,8 @@\n>> +[wrap-git]\n>> +url = https://github.com/pybind/pybind11.git\n>> +revision = 82734801f23314b4c34d70a79509e060a2648e04\n> \n> Great - this is now pointing directly to the pybind sources so I think\n> that's much cleaner:\n> \n> It's hard to do a dedicated review on so much that I honestly don't\n> fully comprehend yet - but I think getting this in and working/building\n> on top is better for everyone, and as far as I know - it's already quite\n> well tested by RPi (albeit, an older version, so I hope David can rebase\n> and test sometime soon).\n\nYes, I think David rebasing their work and testing it is an important \nstep before we can consider merging this.\n\nHint, hint, David! ;)\n\nNote that the API has changed a bit. The fb mmap is different, and the \nColorSpace enums are nested. I think those are the only API changes.\n\nAnother thing, Python bindings are currently enabled by default if the \ndependencies are there. I was wondering if the bindings should be \ndisabled by default until we're more confident that they build fine.\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 ED2D8C3256\n\tfor ;\n\tThu, 5 May 2022 14:04:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6618465643;\n\tThu, 5 May 2022 16:04:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F2DA6603AB\n\tfor ;\n\tThu, 5 May 2022 16:04:02 +0200 (CEST)","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 22919492;\n\tThu, 5 May 2022 16:04:02 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651759444;\n\tbh=ZRdhhagqB50XJScC/z3RXp0ky8P08dQbBKCYDsh/ilU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=12wXFRuGs7QTaisGpUMm22qCcugd5Y1+VI6XzVbhyPh43mFeao7/LHbZDqfMny7ux\n\tj6iOSG9cSpy56EiLvqQMFELTKVQo8XhJIDalTArxCknKUBQGX/iZdmYwmtb9XU/yLa\n\tWLbpMp6Ca1ODTfe36JvYbSxBktNkyISv79PTYDIalCWgMaZmfLVZmboW6gIa0FzzoJ\n\ti+b3/yVIpKEl6PrRqFGsxuzVV0jGKS3MY7ANY8MZdRsOOYQ8EgEmHz/hcbSFG920nI\n\tTHk/gFO2SJc/OTmn7JD4+PWsS2o7q2tiKejN7malFVJnq3yJcnWV39untOfahSjFcd\n\ts4ZtHA4qHRV7A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651759442;\n\tbh=ZRdhhagqB50XJScC/z3RXp0ky8P08dQbBKCYDsh/ilU=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=tzksDyaOpjL4iayO9kix5LOZe2oAq9IOozhHFfhLpVstpRz0KcNFTKMEuGG5MHcpt\n\t+1RhcLKM8T7skUNYOtncwtoiT2NSU9r+WJx9ke+V2yxr7zn2RjgQTDJ7RXo1VYIB5i\n\tgisFvhPOB41kYjkS6j3GrxvplINEcArVieh2UwnE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"tzksDyaO\"; dkim-atps=neutral","Message-ID":"<67cf0db7-11dc-1212-1642-24504fa4ec5d@ideasonboard.com>","Date":"Thu, 5 May 2022 17:03:59 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.8.0","Content-Language":"en-US","To":"Kieran Bingham ,\n\tDavid Plowman ,\n\tJacopo Mondi ,\n\tLaurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220505104104.70841-1-tomi.valkeinen@ideasonboard.com>\n\t<20220505104104.70841-5-tomi.valkeinen@ideasonboard.com>\n\t<165175869453.1423360.6614915904429692992@Monstersaurus>","In-Reply-To":"<165175869453.1423360.6614915904429692992@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22859,"web_url":"https://patchwork.libcamera.org/comment/22859/","msgid":"<165176024324.1217485.14800148891900375799@Monstersaurus>","date":"2022-05-05T14:17:23","subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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 (2022-05-05 15:03:59)\n> On 05/05/2022 16:51, Kieran Bingham wrote:\n> \n> >> + pyCamera\n> >> + .def_property_readonly(\"id\", &Camera::id)\n> >> + .def(\"acquire\", &Camera::acquire)\n> >> + .def(\"release\", &Camera::release)\n> >> + .def(\"start\", [](Camera &self) {\n> >> + self.requestCompleted.connect(handleRequestCompleted);\n> > \n> > What happens if someone calls start() multiple times? Is that going to\n> > mean the signal / slot will be connected multiple times?\n> > \n> > That could be problematic ... as I think it means it could then call the\n> > request completed handler multiple times - but it may also already be\n> > trapped by the recent work we did around there.\n> > \n> > So for now I think this is fine - but may be something to test or\n> > consider later.\n> \n> Good point. I'll look at it.\n> \n> >> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> >> new file mode 100644\n> >> index 00000000..2413e9ca\n> >> --- /dev/null\n> >> +++ b/subprojects/pybind11.wrap\n> >> @@ -0,0 +1,8 @@\n> >> +[wrap-git]\n> >> +url = https://github.com/pybind/pybind11.git\n> >> +revision = 82734801f23314b4c34d70a79509e060a2648e04\n> > \n> > Great - this is now pointing directly to the pybind sources so I think\n> > that's much cleaner:\n> > \n> > It's hard to do a dedicated review on so much that I honestly don't\n> > fully comprehend yet - but I think getting this in and working/building\n> > on top is better for everyone, and as far as I know - it's already quite\n> > well tested by RPi (albeit, an older version, so I hope David can rebase\n> > and test sometime soon).\n> \n> Yes, I think David rebasing their work and testing it is an important \n> step before we can consider merging this.\n> \n> Hint, hint, David! ;)\n> \n> Note that the API has changed a bit. The fb mmap is different, and the \n> ColorSpace enums are nested. I think those are the only API changes.\n> \n> Another thing, Python bindings are currently enabled by default if the \n> dependencies are there. I was wondering if the bindings should be \n> disabled by default until we're more confident that they build fine.\n\nI'm fine with them building when dependencies are met. If that catches\nfailures early - that's better in my opinion.\n\n--\nKieran\n\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 BB8EFC3256\n\tfor ;\n\tThu, 5 May 2022 14:17:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7630D65643;\n\tThu, 5 May 2022 16:17:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B90D8603AB\n\tfor ;\n\tThu, 5 May 2022 16:17:25 +0200 (CEST)","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 75812492;\n\tThu, 5 May 2022 16:17:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651760246;\n\tbh=SEZWrlOoDbp2Yghmq1/WbPLxi9DaWTRgnXxiymWYTAk=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=YPyp20BYcjSiyQ0kbKlKWRBfjd5CkFAYn0trANQMO0qpOB+bJZtKDCl0Z8HON6AjE\n\tKcl2qdN+m5W0voRnqa2SnoqG0VQK+Hu00ESHKh0x/gjQGbKbaYiLHvpeYkwU1tTDbl\n\tCq80UYXIGPv3TK7nFyWSF/ls9i0+/iMfGZdzM0dXF7sHsLJ1bPRJBYPGvLrJEedvh+\n\tCMHoZXABne2TiNxZalCc+1APJHNXxcC3eYpl3JDwm/ZOTChZElhoettyt4Ecx7XRxc\n\t6w2gpklzGIIOBJNCFY9JPMBDG/7BX0V7E8AmEiWkAZYPM0RQzEfxZHKKTuv9gmrNIK\n\tmpUjBQ43w25YA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651760245;\n\tbh=SEZWrlOoDbp2Yghmq1/WbPLxi9DaWTRgnXxiymWYTAk=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=SgCITOZgTSEqoYXwrWRMErTR5oTg5uwRe3nyX89cXLtIwL5IIZX84zMjW9OB/w1J4\n\tKzUjqBqGLqG4bVnrd2hms9KGjwIaSmXTsnwplURpPWAvmFzULv94Z7+9jxEosFlruA\n\trwAw9+9i0oH9dzL/eiFTcc3CbFQ7epoZFUSv9daQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SgCITOZg\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<67cf0db7-11dc-1212-1642-24504fa4ec5d@ideasonboard.com>","References":"<20220505104104.70841-1-tomi.valkeinen@ideasonboard.com>\n\t<20220505104104.70841-5-tomi.valkeinen@ideasonboard.com>\n\t<165175869453.1423360.6614915904429692992@Monstersaurus>\n\t<67cf0db7-11dc-1212-1642-24504fa4ec5d@ideasonboard.com>","To":"David Plowman ,\n\tJacopo Mondi ,\n\tLaurent Pinchart ,\n\tTomi Valkeinen ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 05 May 2022 15:17:23 +0100","Message-ID":"<165176024324.1217485.14800148891900375799@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22863,"web_url":"https://patchwork.libcamera.org/comment/22863/","msgid":"","date":"2022-05-05T17:32:11","subject":"Re: [libcamera-devel] [PATCH v7 04/13] Add Python bindings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, May 05, 2022 at 02:51:34PM +0100, Kieran Bingham wrote:\n> Quoting Tomi Valkeinen (2022-05-05 11:40:55)\n> > Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n> > Python layer.\n> > \n> > We use pybind11 'smart_holder' version to avoid issues with private\n> > destructors and shared_ptr. There is also an alternative solution here:\n> > \n> > https://github.com/pybind/pybind11/pull/2067\n> > \n> > Only a subset of libcamera classes are exposed. Implementing and testing\n> > the wrapper classes is challenging, and as such only classes that I have\n> > needed have been added so far.\n> > \n> > Signed-off-by: Tomi Valkeinen \n> > ---\n> > meson.build | 1 +\n> > meson_options.txt | 5 +\n> > src/meson.build | 1 +\n> > src/py/libcamera/__init__.py | 12 +\n> > src/py/libcamera/meson.build | 44 ++\n> > src/py/libcamera/pyenums.cpp | 53 ++\n> > src/py/libcamera/pymain.cpp | 452 ++++++++++++++++++\n> > src/py/meson.build | 1 +\n> > subprojects/.gitignore | 3 +-\n> > subprojects/packagefiles/pybind11/meson.build | 8 +\n> > subprojects/pybind11.wrap | 8 +\n> > 11 files changed, 587 insertions(+), 1 deletion(-)\n> > create mode 100644 src/py/libcamera/__init__.py\n> > create mode 100644 src/py/libcamera/meson.build\n> > create mode 100644 src/py/libcamera/pyenums.cpp\n> > create mode 100644 src/py/libcamera/pymain.cpp\n> > create mode 100644 src/py/meson.build\n> > create mode 100644 subprojects/packagefiles/pybind11/meson.build\n> > create mode 100644 subprojects/pybind11.wrap\n> > \n> > diff --git a/meson.build b/meson.build\n> > index 0124e7d3..60a911e0 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -177,6 +177,7 @@ summary({\n> > 'Tracing support': tracing_enabled,\n> > 'Android support': android_enabled,\n> > 'GStreamer support': gst_enabled,\n> > + 'Python bindings': pycamera_enabled,\n> > 'V4L2 emulation support': v4l2_enabled,\n> > 'cam application': cam_enabled,\n> > 'qcam application': qcam_enabled,\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..34663a6f 100644\n> > --- a/src/meson.build\n> > +++ b/src/meson.build\n> > @@ -37,4 +37,5 @@ subdir('cam')\n> > subdir('qcam')\n> > \n> > subdir('gstreamer')\n> > +subdir('py')\n> > subdir('v4l2')\n> > diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py\n> > new file mode 100644\n> > index 00000000..cd7512a2\n> > --- /dev/null\n> > +++ b/src/py/libcamera/__init__.py\n> > @@ -0,0 +1,12 @@\n> > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > +# Copyright (C) 2021, Tomi Valkeinen \n\nYou may want to bump the copyright year :-) Same below.\n\n> > +\n> > +from ._libcamera import *\n> > +import mmap\n> > +\n> > +\n> > +def __FrameBuffer__mmap(self, plane):\n\nLet's record that more work is needed:\n\n # \\todo Support multi-planar formats\n\n> > + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)\n> > +\n> > +\n> > +FrameBuffer.mmap = __FrameBuffer__mmap\n> > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > new file mode 100644\n> > index 00000000..e1f5cf21\n> > --- /dev/null\n> > +++ b/src/py/libcamera/meson.build\n> > @@ -0,0 +1,44 @@\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> > + 'pyenums.cpp',\n\nAlphabetical order please.\n\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> > +pycamera_args += ['-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT']\n\nYou could write this as\n\npycamera_args = [\n '-fvisibility=hidden',\n '-Wno-shadow',\n '-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT',\n]\n\n> > +\n> > +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/libcamera'\n\ndestdir = get_option('libdir') / ('python' + py3_dep.version()) / 'site-packages' / 'libcamera'\n\n> > +\n> > +pycamera = shared_module('_libcamera',\n> > + pycamera_sources,\n> > + install : true,\n> > + install_dir : destdir,\n> > + name_prefix : '',\n> > + dependencies : pycamera_deps,\n> > + cpp_args : pycamera_args)\n> > +\n> > +run_command('ln', '-fsT', '../../../../src/py/libcamera/__init__.py',\n> > + meson.current_build_dir() / '__init__.py',\n> > + check: true)\n> > +\n> > +install_data(['__init__.py'], install_dir : destdir)\n> > diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp\n> > new file mode 100644\n> > index 00000000..39886656\n> > --- /dev/null\n> > +++ b/src/py/libcamera/pyenums.cpp\n> > @@ -0,0 +1,53 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Tomi Valkeinen \n> > + *\n> > + * Python bindings\n\nThis could be a bit more descriptive:\n\n * Python bindings - Enumerations\n\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +namespace py = pybind11;\n> > +\n> > +using namespace libcamera;\n> > +\n> > +void init_pyenums(py::module &m)\n> > +{\n> > + py::enum_(m, \"ConfigurationStatus\")\n> > + .value(\"Valid\", CameraConfiguration::Valid)\n> > + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n> > + .value(\"Invalid\", CameraConfiguration::Invalid);\n\nIt would be nice if C++ had some introspection capabilities that would\nallow this to be done automatically :-)\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::enum_(m, \"RequestStatus\")\n> > + .value(\"Pending\", Request::RequestPending)\n\nI wonder if the C++ enum should turn into an enum class, and the\nenumerators renamed to drop the Request prefix.\n\n> > + .value(\"Complete\", Request::RequestComplete)\n> > + .value(\"Cancelled\", Request::RequestCancelled);\n\nNothing that needs to be changed now, by what is more pythonic between\n\n Request.Status.Pending\n\nand\n\n RequestStatus.Pending\n\n?\n\n> > +\n> > + py::enum_(m, \"FrameMetadataStatus\")\n> > + .value(\"Success\", FrameMetadata::FrameSuccess)\n\nSame here, an enum class may make sense.\n\n> > + .value(\"Error\", FrameMetadata::FrameError)\n> > + .value(\"Cancelled\", FrameMetadata::FrameCancelled);\n> > +\n> > + py::enum_(m, \"ReuseFlag\")\n> > + .value(\"Default\", Request::ReuseFlag::Default)\n> > + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n> > +\n> > + py::enum_(m, \"ControlType\")\n> > + .value(\"None\", ControlType::ControlTypeNone)\n\nDitto.\n\n> > + .value(\"Bool\", ControlType::ControlTypeBool)\n> > + .value(\"Byte\", ControlType::ControlTypeByte)\n> > + .value(\"Integer32\", ControlType::ControlTypeInteger32)\n> > + .value(\"Integer64\", ControlType::ControlTypeInteger64)\n> > + .value(\"Float\", ControlType::ControlTypeFloat)\n> > + .value(\"String\", ControlType::ControlTypeString)\n> > + .value(\"Rectangle\", ControlType::ControlTypeRectangle)\n> > + .value(\"Size\", ControlType::ControlTypeSize);\n> > +}\n> > diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp\n> > new file mode 100644\n> > index 00000000..54674caf\n> > --- /dev/null\n> > +++ b/src/py/libcamera/pymain.cpp\n> > @@ -0,0 +1,452 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Tomi Valkeinen \n> > + *\n> > + * Python bindings\n> > + */\n> > +\n> > +/*\n> > + * To generate pylibcamera stubs:\n> > + * PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera\n\nWhat is this for ?\n\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\nYou're using the std:: namespace qualifier explicitly in several\nlocation below. Could we do that everywhere and drop the using namespace\ndirective ?\n\n> > +using namespace libcamera;\n> > +\n> > +template\n> > +static py::object ValueOrTuple(const ControlValue &cv)\n\nCould you rename this to valueOrTuple() to follow the libcamera coding\nstyle ? Or would that look awkward from a python bindings coding style\npoint of view ?\n\nSame for ControlValueToPy and PyToControlValue.\n\n> > +{\n> > + if (cv.isArray()) {\n> > + const T *v = reinterpret_cast(cv.data().data());\n> > + auto t = py::tuple(cv.numElements());\n> > +\n> > + for (size_t i = 0; i < cv.numElements(); ++i)\n> > + t[i] = v[i];\n> > +\n> > + return t;\n> > + }\n> > +\n> > + return py::cast(cv.get());\n> > +}\n> > +\n> > +static py::object ControlValueToPy(const ControlValue &cv)\n> > +{\n> > + switch (cv.type()) {\n> > + case ControlTypeBool:\n> > + return ValueOrTuple(cv);\n> > + case ControlTypeByte:\n> > + return ValueOrTuple(cv);\n> > + case ControlTypeInteger32:\n> > + return ValueOrTuple(cv);\n> > + case ControlTypeInteger64:\n> > + return ValueOrTuple(cv);\n> > + case ControlTypeFloat:\n> > + return ValueOrTuple(cv);\n> > + case ControlTypeString:\n> > + return py::cast(cv.get());\n> > + case ControlTypeRectangle: {\n\n\t/* \\todo Add geometry classes to the Python bindings */\n\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\nA todo comment would be nice here too.\n\n> > + case ControlTypeNone:\n> > + default:\n> > + throw runtime_error(\"Control type not implemented\");\n> > + }\n> > +}\n> > +\n> > +static weak_ptr g_camera_manager;\n\nCoding style here too, gCameraManager, or would that look too awkward ?\nSame below.\n\n> > +static int g_eventfd;\n> > +static mutex g_reqlist_mutex;\n> > +static vector g_reqlist;\n> > +\n> > +static void handleRequestCompleted(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> > +void init_pyenums(py::module &m);\n> > +\n> > +PYBIND11_MODULE(_libcamera, m)\n> > +{\n> > + init_pyenums(m);\n> > +\n> > + /* Forward declarations */\n> > +\n> > + /*\n> > + * We need to declare all the classes here so that Python docstrings\n> > + * can be generated correctly.\n> > + * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n> > + */\n> > +\n> > + auto pyCameraManager = py::class_(m, \"CameraManager\");\n> > + auto pyCamera = py::class_(m, \"Camera\");\n> > + auto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n> > + auto pyStreamConfiguration = py::class_(m, \"StreamConfiguration\");\n> > + auto pyStreamFormats = py::class_(m, \"StreamFormats\");\n> > + auto pyFrameBufferAllocator = py::class_(m, \"FrameBufferAllocator\");\n> > + auto pyFrameBuffer = py::class_(m, \"FrameBuffer\");\n> > + auto pyStream = py::class_(m, \"Stream\");\n> > + auto pyControlId = py::class_(m, \"ControlId\");\n> > + auto pyRequest = py::class_(m, \"Request\");\n> > + auto pyFrameMetadata = py::class_(m, \"FrameMetadata\");\n\nCould you please sort these alphabetically ?\n\n> > +\n> > + /* Global functions */\n> > + m.def(\"logSetLevel\", &logSetLevel);\n> > +\n> > + /* Classes */\n> > + pyCameraManager\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\nSome long lines like this one can be wrapped:\n\n\t\t\t\tthrow std::system_error(errno, std::generic_category(),\n\t\t\t\t\t\t\t\"Failed to create eventfd\");\n> > +\n> > + 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\nAs we don't usually prefix getters with \"get\", maybe just\n\"readyRequests\" ?\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\ns/decrease/Decrease/\n\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\nDo we really want to ignore the case in the comparison here ?\n\n> > +\n> > + if (id.find(str) != string::npos)\n\nActually, does this function really belong in the python bindings ? It\nseems to be used in unit tests only, where you could iterate over the\ncameras exposed by the cameras property instead.\n\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> > + pyCamera\n> > + .def_property_readonly(\"id\", &Camera::id)\n> > + .def(\"acquire\", &Camera::acquire)\n> > + .def(\"release\", &Camera::release)\n> > + .def(\"start\", [](Camera &self) {\n> > + self.requestCompleted.connect(handleRequestCompleted);\n> \n> What happens if someone calls start() multiple times? Is that going to\n> mean the signal / slot will be connected multiple times?\n> \n> That could be problematic ... as I think it means it could then call the\n> request completed handler multiple times - but it may also already be\n> trapped by the recent work we did around there.\n> \n> So for now I think this is fine - but may be something to test or\n> consider later.\n\nIndeed. A \\todo comment would be nice.\n\n> > +\n> > + int ret = self.start();\n> > + if (ret)\n> > + self.requestCompleted.disconnect(handleRequestCompleted);\n> > +\n> > + return ret;\n\n\t\t\tint ret = self.start();\n\t\t\tif (ret) {\n\t\t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n\t\t\t\treturn ret;\n\t\t\t}\n\n\t\t\treturn 0;\n\n> > + })\n> > +\n> > + .def(\"stop\", [](Camera &self) {\n> > + int ret = self.stop();\n> > + if (!ret)\n> > + self.requestCompleted.disconnect(handleRequestCompleted);\n> > +\n> > + return ret;\n\n\t\t\tint ret = self.stop();\n\t\t\tif (ret)\n\t\t\t\treturn ret;\n\n\t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n\t\t\treturn 0;\n\n> > + })\n> > +\n> > + .def(\"__repr__\", [](Camera &self) {\n> > + return \"\";\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\n\t\t\t/*\n\t\t\t * Increase the reference count, will be dropped in\n\t\t\t * getReadyRequests().\n\t\t\t */\n\nI wonder what happens if nobody calls getReadyRequests() though, for\ninstance for the last requesuts when stopping the camera. If there's an\nissue there, please add a \\todo comment somewhere.\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(\"find_control\", [](Camera &self, const string &name) {\n\n\"findControl\"\n\n> > + const auto &controls = self.controls();\n> > +\n> > + auto it = find_if(controls.begin(), controls.end(),\n> > + [&name](const auto &kvp) { return kvp.first->name() == name; });\n> > +\n> > + if (it == controls.end())\n> > + throw runtime_error(\"Control not found\");\n> > +\n> > + return it->first;\n> > + }, py::return_value_policy::reference_internal)\n> > +\n> > + .def_property_readonly(\"controls\", [](Camera &self) {\n> > + py::dict ret;\n> > +\n> > + for (const auto &[id, ci] : self.controls()) {\n\n\t\t\t\t/* \\todo Add bindings for the ControlInfo class */\n\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> > + pyCameraConfiguration\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\nLine wrap.\n\n> > + .def_property_readonly(\"size\", &CameraConfiguration::size)\n> > + .def_property_readonly(\"empty\", &CameraConfiguration::empty);\n> > +\n> > + pyStreamConfiguration\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\nThat's a very long line too.\n\n\t\t\t[](StreamConfiguration &self) {\n\t\t\t\treturn make_tuple(self.size.width, self.size.height);\n\t\t\t},\n\t\t\t[](StreamConfiguration &self, tuple size) {\n\t\t\t\tself.size.width = get<0>(size);\n\t\t\t\tself.size.height = get<1>(size);\n\t\t\t})\n\nSame where applicable.\n\n> > + .def_property(\n> > + \"pixelFormat\",\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> > + pyStreamFormats\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> > + pyFrameBufferAllocator\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> > + pyFrameBuffer\n> > + /* TODO: implement FrameBuffer::Plane properly */\n\ns/TODO: implement/\\\\todo Implement/\n\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> > + pyStream\n> > + .def_property_readonly(\"configuration\", &Stream::configuration);\n> > +\n> > + pyControlId\n> > + .def_property_readonly(\"id\", &ControlId::id)\n> > + .def_property_readonly(\"name\", &ControlId::name)\n> > + .def_property_readonly(\"type\", &ControlId::type);\n> > +\n> > + pyRequest\n> > + /* Fence is not supported, so we cannot expose addBuffer() directly */\n> > + .def(\"addBuffer\", [](Request &self, const Stream *stream, FrameBuffer *buffer) {\n> > + return self.addBuffer(stream, buffer);\n> > + }, 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, ControlId &id, py::object value) {\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\nThat will be an annoying limitation. Can you add a todo comment to fix\nit (and line wrap this comment) ?\n\n> > + .def(\"reuse\", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });\n> > +\n> > + pyFrameMetadata\n> > + .def_readonly(\"status\", &FrameMetadata::status)\n> > + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n> > + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n> > + /* temporary helper, to be removed */\n\n\t\t/* \\todo Temporary helper, to be removed */\n\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/src/py/meson.build b/src/py/meson.build\n> > new file mode 100644\n> > index 00000000..4ce9668c\n> > --- /dev/null\n> > +++ b/src/py/meson.build\n> > @@ -0,0 +1 @@\n> > +subdir('libcamera')\n> > diff --git a/subprojects/.gitignore b/subprojects/.gitignore\n> > index 391fde2c..0e194289 100644\n> > --- a/subprojects/.gitignore\n> > +++ b/subprojects/.gitignore\n> > @@ -1,3 +1,4 @@\n> > /googletest-release*\n> > /libyuv\n> > -/packagecache\n> > \\ No newline at end of file\n> > +/packagecache\n> > +/pybind11\n> > diff --git a/subprojects/packagefiles/pybind11/meson.build b/subprojects/packagefiles/pybind11/meson.build\n> > new file mode 100644\n> > index 00000000..67e89aec\n> > --- /dev/null\n> > +++ b/subprojects/packagefiles/pybind11/meson.build\n> > @@ -0,0 +1,8 @@\n> > +project('pybind11', 'cpp',\n> > + version : '2.9.1',\n> > + license : 'BSD-3-Clause')\n> > +\n> > +pybind11_incdir = include_directories('include')\n> > +\n> > +pybind11_dep = declare_dependency(\n> > + include_directories : pybind11_incdir)\n\n4 spaces for indentation.\n\n> > diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> > new file mode 100644\n> > index 00000000..2413e9ca\n> > --- /dev/null\n> > +++ b/subprojects/pybind11.wrap\n> > @@ -0,0 +1,8 @@\n> > +[wrap-git]\n> > +url = https://github.com/pybind/pybind11.git\n\nA comment here to mention this is the smart_holder branch could be nice.\n\n> > +revision = 82734801f23314b4c34d70a79509e060a2648e04\n> \n> Great - this is now pointing directly to the pybind sources so I think\n> that's much cleaner:\n> \n> It's hard to do a dedicated review on so much that I honestly don't\n> fully comprehend yet - but I think getting this in and working/building\n> on top is better for everyone, and as far as I know - it's already quite\n> well tested by RPi (albeit, an older version, so I hope David can rebase\n> and test sometime soon).\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > +depth = 1\n> > +patch_directory = pybind11\n\nDo we actually have any patch ?\n\nAll comments are minor, once addressed,\n\nReviewed-by: Laurent Pinchart \n\n> > +\n> > +[provide]\n> > +pybind11 = pybind11_dep","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 571BEC3256\n\tfor ;\n\tThu, 5 May 2022 17:32:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 99F1B65646;\n\tThu, 5 May 2022 19:32:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF0F7603AB\n\tfor ;\n\tThu, 5 May 2022 19:32:15 +0200 (CEST)","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 16DF9492;\n\tThu, 5 May 2022 19:32:15 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651771937;\n\tbh=rrmpEu2+0hrTqm40H35kc7l7WQDrbnzp8na/Iy1moT8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=c8RfE8HGfqZXRxy5uoVFaDFjIPAolmRlZlyOhZmlBLR32SqJu5hyjAE1xrspBNB+/\n\thKSf2DB3mVl1AOEeOsx1ItlYFwx/0NGZm3dzFgiShltCg2oknmu0J3gG8C9Ole6A50\n\tniYzx4BRcDr1590wQtvWjLZiREF+6ur426V2TYPsVazw0k0JArrRUdfYi1BH+axwmM\n\t0WtTeEfKbeA5JNf1Gm8gZR1fDZDIH9Ykn7aGxWL7fqF65Wa45a034xpelUC+/R8t9W\n\t2nfXeVBV42GbEjCI5fEe4aK9nWy6gUs3WB3VO7GWMqKeK176ZYfZKB7cxE72Tmw8bi\n\tAhQCsoLtMAvSw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651771935;\n\tbh=rrmpEu2+0hrTqm40H35kc7l7WQDrbnzp8na/Iy1moT8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oaVzB20VfueCz6aPXg5uolM6qSSWQRvCz0qSpGepQB0eK1RjsejlN1NzWHwLnfxPG\n\ta6uEEz90Rn9fPMHNioEgRiic27cYZkQvp5OM4EMt8bvYvVGrqdykd5PNfE1tMdagce\n\tFpHgtwZMoUaw0tYQa2cG40B44VtRrTKftkQKn1gQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"oaVzB20V\"; dkim-atps=neutral","Date":"Thu, 5 May 2022 20:32:11 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20220505104104.70841-1-tomi.valkeinen@ideasonboard.com>\n\t<20220505104104.70841-5-tomi.valkeinen@ideasonboard.com>\n\t<165175869453.1423360.6614915904429692992@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<165175869453.1423360.6614915904429692992@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22872,"web_url":"https://patchwork.libcamera.org/comment/22872/","msgid":"<046203e5-d966-7f3f-6491-d194b1a68753@ideasonboard.com>","date":"2022-05-06T08:11:46","subject":"Re: [libcamera-devel] [PATCH v7 04/13] Add Python bindings","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"Hi,\n\nOn 05/05/2022 20:32, Laurent Pinchart wrote:\n> On Thu, May 05, 2022 at 02:51:34PM +0100, Kieran Bingham wrote:\n>> Quoting Tomi Valkeinen (2022-05-05 11:40:55)\n>>> Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n>>> Python layer.\n>>>\n>>> We use pybind11 'smart_holder' version to avoid issues with private\n>>> destructors and shared_ptr. There is also an alternative solution here:\n>>>\n>>> https://github.com/pybind/pybind11/pull/2067\n>>>\n>>> Only a subset of libcamera classes are exposed. Implementing and testing\n>>> the wrapper classes is challenging, and as such only classes that I have\n>>> needed have been added so far.\n>>>\n>>> Signed-off-by: Tomi Valkeinen \n>>> ---\n>>> meson.build | 1 +\n>>> meson_options.txt | 5 +\n>>> src/meson.build | 1 +\n>>> src/py/libcamera/__init__.py | 12 +\n>>> src/py/libcamera/meson.build | 44 ++\n>>> src/py/libcamera/pyenums.cpp | 53 ++\n>>> src/py/libcamera/pymain.cpp | 452 ++++++++++++++++++\n>>> src/py/meson.build | 1 +\n>>> subprojects/.gitignore | 3 +-\n>>> subprojects/packagefiles/pybind11/meson.build | 8 +\n>>> subprojects/pybind11.wrap | 8 +\n>>> 11 files changed, 587 insertions(+), 1 deletion(-)\n>>> create mode 100644 src/py/libcamera/__init__.py\n>>> create mode 100644 src/py/libcamera/meson.build\n>>> create mode 100644 src/py/libcamera/pyenums.cpp\n>>> create mode 100644 src/py/libcamera/pymain.cpp\n>>> create mode 100644 src/py/meson.build\n>>> create mode 100644 subprojects/packagefiles/pybind11/meson.build\n>>> create mode 100644 subprojects/pybind11.wrap\n>>>\n>>> diff --git a/meson.build b/meson.build\n>>> index 0124e7d3..60a911e0 100644\n>>> --- a/meson.build\n>>> +++ b/meson.build\n>>> @@ -177,6 +177,7 @@ summary({\n>>> 'Tracing support': tracing_enabled,\n>>> 'Android support': android_enabled,\n>>> 'GStreamer support': gst_enabled,\n>>> + 'Python bindings': pycamera_enabled,\n>>> 'V4L2 emulation support': v4l2_enabled,\n>>> 'cam application': cam_enabled,\n>>> 'qcam application': qcam_enabled,\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..34663a6f 100644\n>>> --- a/src/meson.build\n>>> +++ b/src/meson.build\n>>> @@ -37,4 +37,5 @@ subdir('cam')\n>>> subdir('qcam')\n>>> \n>>> subdir('gstreamer')\n>>> +subdir('py')\n>>> subdir('v4l2')\n>>> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py\n>>> new file mode 100644\n>>> index 00000000..cd7512a2\n>>> --- /dev/null\n>>> +++ b/src/py/libcamera/__init__.py\n>>> @@ -0,0 +1,12 @@\n>>> +# SPDX-License-Identifier: LGPL-2.1-or-later\n>>> +# Copyright (C) 2021, Tomi Valkeinen \n> \n> You may want to bump the copyright year :-) Same below.\n> \n>>> +\n>>> +from ._libcamera import *\n>>> +import mmap\n>>> +\n>>> +\n>>> +def __FrameBuffer__mmap(self, plane):\n> \n> Let's record that more work is needed:\n> \n> # \\todo Support multi-planar formats\n\nThis is implemented in a later patch in the series. I didn't want to \nsquash to highlight the change.\n\n>>> + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)\n>>> +\n>>> +\n>>> +FrameBuffer.mmap = __FrameBuffer__mmap\n>>> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n>>> new file mode 100644\n>>> index 00000000..e1f5cf21\n>>> --- /dev/null\n>>> +++ b/src/py/libcamera/meson.build\n>>> @@ -0,0 +1,44 @@\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>>> + 'pyenums.cpp',\n> \n> Alphabetical order please.\n> \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>>> +pycamera_args += ['-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT']\n> \n> You could write this as\n> \n> pycamera_args = [\n> '-fvisibility=hidden',\n> '-Wno-shadow',\n> '-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT',\n> ]\n> \n>>> +\n>>> +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/libcamera'\n> \n> destdir = get_option('libdir') / ('python' + py3_dep.version()) / 'site-packages' / 'libcamera'\n> \n>>> +\n>>> +pycamera = shared_module('_libcamera',\n>>> + pycamera_sources,\n>>> + install : true,\n>>> + install_dir : destdir,\n>>> + name_prefix : '',\n>>> + dependencies : pycamera_deps,\n>>> + cpp_args : pycamera_args)\n>>> +\n>>> +run_command('ln', '-fsT', '../../../../src/py/libcamera/__init__.py',\n>>> + meson.current_build_dir() / '__init__.py',\n>>> + check: true)\n>>> +\n>>> +install_data(['__init__.py'], install_dir : destdir)\n>>> diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp\n>>> new file mode 100644\n>>> index 00000000..39886656\n>>> --- /dev/null\n>>> +++ b/src/py/libcamera/pyenums.cpp\n>>> @@ -0,0 +1,53 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2021, Tomi Valkeinen \n>>> + *\n>>> + * Python bindings\n> \n> This could be a bit more descriptive:\n> \n> * Python bindings - Enumerations\n> \n>>> + */\n>>> +\n>>> +#include \n>>> +\n>>> +#include \n>>> +\n>>> +namespace py = pybind11;\n>>> +\n>>> +using namespace libcamera;\n>>> +\n>>> +void init_pyenums(py::module &m)\n>>> +{\n>>> + py::enum_(m, \"ConfigurationStatus\")\n>>> + .value(\"Valid\", CameraConfiguration::Valid)\n>>> + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n>>> + .value(\"Invalid\", CameraConfiguration::Invalid);\n> \n> It would be nice if C++ had some introspection capabilities that would\n> allow this to be done automatically :-)\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::enum_(m, \"RequestStatus\")\n>>> + .value(\"Pending\", Request::RequestPending)\n> \n> I wonder if the C++ enum should turn into an enum class, and the\n> enumerators renamed to drop the Request prefix.\n\nHmm, what are you suggesting? Changing the C++ enum to enum class? Isn't \nthat kind of a drastic change? In some situations enums and enum classes \nbehave quite differently, and in any case it's a big API change.\n\nI'm not against it, I was surprised to see so many enums used in the C++ \nside, but... Maybe that's not part of thise series?\n\n>>> + .value(\"Complete\", Request::RequestComplete)\n>>> + .value(\"Cancelled\", Request::RequestCancelled);\n> \n> Nothing that needs to be changed now, by what is more pythonic between\n> \n> Request.Status.Pending\n> \n> and\n> \n> RequestStatus.Pending\n> \n> ?\n\nOr Request.RequestPending.\n\nI have no idea =). I like the first one best. I did add some of the more \nrecent enums inside the related class, but for these I just copied the \nC++ side style.\n\n>>> +\n>>> + py::enum_(m, \"FrameMetadataStatus\")\n>>> + .value(\"Success\", FrameMetadata::FrameSuccess)\n> \n> Same here, an enum class may make sense.\n> \n>>> + .value(\"Error\", FrameMetadata::FrameError)\n>>> + .value(\"Cancelled\", FrameMetadata::FrameCancelled);\n>>> +\n>>> + py::enum_(m, \"ReuseFlag\")\n>>> + .value(\"Default\", Request::ReuseFlag::Default)\n>>> + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n>>> +\n>>> + py::enum_(m, \"ControlType\")\n>>> + .value(\"None\", ControlType::ControlTypeNone)\n> \n> Ditto.\n> \n>>> + .value(\"Bool\", ControlType::ControlTypeBool)\n>>> + .value(\"Byte\", ControlType::ControlTypeByte)\n>>> + .value(\"Integer32\", ControlType::ControlTypeInteger32)\n>>> + .value(\"Integer64\", ControlType::ControlTypeInteger64)\n>>> + .value(\"Float\", ControlType::ControlTypeFloat)\n>>> + .value(\"String\", ControlType::ControlTypeString)\n>>> + .value(\"Rectangle\", ControlType::ControlTypeRectangle)\n>>> + .value(\"Size\", ControlType::ControlTypeSize);\n>>> +}\n>>> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp\n>>> new file mode 100644\n>>> index 00000000..54674caf\n>>> --- /dev/null\n>>> +++ b/src/py/libcamera/pymain.cpp\n>>> @@ -0,0 +1,452 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2021, Tomi Valkeinen \n>>> + *\n>>> + * Python bindings\n>>> + */\n>>> +\n>>> +/*\n>>> + * To generate pylibcamera stubs:\n>>> + * PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera\n> \n> What is this for ?\n\nOh, that's just a comment for myself on how to generate stubs for the \nlibrary. I need to add a bit more context here, or remove it.\n\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> \n> You're using the std:: namespace qualifier explicitly in several\n> location below. Could we do that everywhere and drop the using namespace\n> directive ?\n\nI'd prefer to use the 'using namespace' and drop the std:: in the code. \nThe few std:: uses are left-overs from copy pastes from examples...\n\n>>> +using namespace libcamera;\n>>> +\n>>> +template\n>>> +static py::object ValueOrTuple(const ControlValue &cv)\n> \n> Could you rename this to valueOrTuple() to follow the libcamera coding\n> style ? Or would that look awkward from a python bindings coding style\n> point of view ?\n\nNo, I think it's fine.\n\n> Same for ControlValueToPy and PyToControlValue.\n> \n>>> +{\n>>> + if (cv.isArray()) {\n>>> + const T *v = reinterpret_cast(cv.data().data());\n>>> + auto t = py::tuple(cv.numElements());\n>>> +\n>>> + for (size_t i = 0; i < cv.numElements(); ++i)\n>>> + t[i] = v[i];\n>>> +\n>>> + return t;\n>>> + }\n>>> +\n>>> + return py::cast(cv.get());\n>>> +}\n>>> +\n>>> +static py::object ControlValueToPy(const ControlValue &cv)\n>>> +{\n>>> + switch (cv.type()) {\n>>> + case ControlTypeBool:\n>>> + return ValueOrTuple(cv);\n>>> + case ControlTypeByte:\n>>> + return ValueOrTuple(cv);\n>>> + case ControlTypeInteger32:\n>>> + return ValueOrTuple(cv);\n>>> + case ControlTypeInteger64:\n>>> + return ValueOrTuple(cv);\n>>> + case ControlTypeFloat:\n>>> + return ValueOrTuple(cv);\n>>> + case ControlTypeString:\n>>> + return py::cast(cv.get());\n>>> + case ControlTypeRectangle: {\n> \n> \t/* \\todo Add geometry classes to the Python bindings */\n\nWhat's missing? The switch handles all the possible cases afaics.\n\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> \n> A todo comment would be nice here too.\n> \n>>> + case ControlTypeNone:\n>>> + default:\n>>> + throw runtime_error(\"Control type not implemented\");\n>>> + }\n>>> +}\n>>> +\n>>> +static weak_ptr g_camera_manager;\n> \n> Coding style here too, gCameraManager, or would that look too awkward ?\n> Same below.\n> \n>>> +static int g_eventfd;\n>>> +static mutex g_reqlist_mutex;\n>>> +static vector g_reqlist;\n>>> +\n>>> +static void handleRequestCompleted(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>>> +void init_pyenums(py::module &m);\n>>> +\n>>> +PYBIND11_MODULE(_libcamera, m)\n>>> +{\n>>> + init_pyenums(m);\n>>> +\n>>> + /* Forward declarations */\n>>> +\n>>> + /*\n>>> + * We need to declare all the classes here so that Python docstrings\n>>> + * can be generated correctly.\n>>> + * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n>>> + */\n>>> +\n>>> + auto pyCameraManager = py::class_(m, \"CameraManager\");\n>>> + auto pyCamera = py::class_(m, \"Camera\");\n>>> + auto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n>>> + auto pyStreamConfiguration = py::class_(m, \"StreamConfiguration\");\n>>> + auto pyStreamFormats = py::class_(m, \"StreamFormats\");\n>>> + auto pyFrameBufferAllocator = py::class_(m, \"FrameBufferAllocator\");\n>>> + auto pyFrameBuffer = py::class_(m, \"FrameBuffer\");\n>>> + auto pyStream = py::class_(m, \"Stream\");\n>>> + auto pyControlId = py::class_(m, \"ControlId\");\n>>> + auto pyRequest = py::class_(m, \"Request\");\n>>> + auto pyFrameMetadata = py::class_(m, \"FrameMetadata\");\n> \n> Could you please sort these alphabetically ?\n\nLater additions introduce classes that depend on each other, and cannot \nbe sorted. Also, these are in the same order as they are in the file \nbelow. So, I'd rather keep them as they are.\n\n>>> +\n>>> + /* Global functions */\n>>> + m.def(\"logSetLevel\", &logSetLevel);\n>>> +\n>>> + /* Classes */\n>>> + pyCameraManager\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> Some long lines like this one can be wrapped:\n> \n> \t\t\t\tthrow std::system_error(errno, std::generic_category(),\n> \t\t\t\t\t\t\t\"Failed to create eventfd\");\n>>> +\n>>> + 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> \n> As we don't usually prefix getters with \"get\", maybe just\n> \"readyRequests\" ?\n\nHmm, this is not a getter, at least not as I see a getter. A getter \nreturns the value of a field, and would be exposed as a property. This \nis a function that does work.\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> \n> s/decrease/Decrease/\n> \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> Do we really want to ignore the case in the comparison here ?\n> \n>>> +\n>>> + if (id.find(str) != string::npos)\n> \n> Actually, does this function really belong in the python bindings ? It\n> seems to be used in unit tests only, where you could iterate over the\n> cameras exposed by the cameras property instead.\n\nI think I agree. I probably added this quite early, when I didn't know \nhow to expose a list of cameras.\n\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>>> + pyCamera\n>>> + .def_property_readonly(\"id\", &Camera::id)\n>>> + .def(\"acquire\", &Camera::acquire)\n>>> + .def(\"release\", &Camera::release)\n>>> + .def(\"start\", [](Camera &self) {\n>>> + self.requestCompleted.connect(handleRequestCompleted);\n>>\n>> What happens if someone calls start() multiple times? Is that going to\n>> mean the signal / slot will be connected multiple times?\n>>\n>> That could be problematic ... as I think it means it could then call the\n>> request completed handler multiple times - but it may also already be\n>> trapped by the recent work we did around there.\n>>\n>> So for now I think this is fine - but may be something to test or\n>> consider later.\n> \n> Indeed. A \\todo comment would be nice.\n> \n>>> +\n>>> + int ret = self.start();\n>>> + if (ret)\n>>> + self.requestCompleted.disconnect(handleRequestCompleted);\n>>> +\n>>> + return ret;\n> \n> \t\t\tint ret = self.start();\n> \t\t\tif (ret) {\n> \t\t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n> \t\t\t\treturn ret;\n> \t\t\t}\n> \n> \t\t\treturn 0;\n> \n>>> + })\n>>> +\n>>> + .def(\"stop\", [](Camera &self) {\n>>> + int ret = self.stop();\n>>> + if (!ret)\n>>> + self.requestCompleted.disconnect(handleRequestCompleted);\n>>> +\n>>> + return ret;\n> \n> \t\t\tint ret = self.stop();\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \n> \t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n> \t\t\treturn 0;\n> \n>>> + })\n>>> +\n>>> + .def(\"__repr__\", [](Camera &self) {\n>>> + return \"\";\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> \n> \t\t\t/*\n> \t\t\t * Increase the reference count, will be dropped in\n> \t\t\t * getReadyRequests().\n> \t\t\t */\n> \n> I wonder what happens if nobody calls getReadyRequests() though, for\n> instance for the last requesuts when stopping the camera. If there's an\n> issue there, please add a \\todo comment somewhere.\n\nThe handled requests will then sit in the gReqList. What is the issue \nyou see? Memory leak? We \"leak\" in any case, as the CameraManager \nsingleton is always there, and the gReqList is essentially a field of \nthe camera manager singleton.\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(\"find_control\", [](Camera &self, const string &name) {\n> \n> \"findControl\"\n\nThis is the python API, so \"find_control\" is the correct one. That said, \nI mix up the styles in many places. To be honest, I don't know if a \ndirect binding should use the original names or pythonic names.\n\nBut I think I'll just go with the pythonic names.\n\n>>> + const auto &controls = self.controls();\n>>> +\n>>> + auto it = find_if(controls.begin(), controls.end(),\n>>> + [&name](const auto &kvp) { return kvp.first->name() == name; });\n>>> +\n>>> + if (it == controls.end())\n>>> + throw runtime_error(\"Control not found\");\n>>> +\n>>> + return it->first;\n>>> + }, py::return_value_policy::reference_internal)\n>>> +\n>>> + .def_property_readonly(\"controls\", [](Camera &self) {\n>>> + py::dict ret;\n>>> +\n>>> + for (const auto &[id, ci] : self.controls()) {\n> \n> \t\t\t\t/* \\todo Add bindings for the ControlInfo class */\n> \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>>> + pyCameraConfiguration\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> \n> Line wrap.\n> \n>>> + .def_property_readonly(\"size\", &CameraConfiguration::size)\n>>> + .def_property_readonly(\"empty\", &CameraConfiguration::empty);\n>>> +\n>>> + pyStreamConfiguration\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> \n> That's a very long line too.\n> \n> \t\t\t[](StreamConfiguration &self) {\n> \t\t\t\treturn make_tuple(self.size.width, self.size.height);\n> \t\t\t},\n> \t\t\t[](StreamConfiguration &self, tuple size) {\n> \t\t\t\tself.size.width = get<0>(size);\n> \t\t\t\tself.size.height = get<1>(size);\n> \t\t\t})\n> \n> Same where applicable.\n> \n>>> + .def_property(\n>>> + \"pixelFormat\",\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>>> + pyStreamFormats\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>>> + pyFrameBufferAllocator\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>>> + pyFrameBuffer\n>>> + /* TODO: implement FrameBuffer::Plane properly */\n> \n> s/TODO: implement/\\\\todo Implement/\n> \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>>> + pyStream\n>>> + .def_property_readonly(\"configuration\", &Stream::configuration);\n>>> +\n>>> + pyControlId\n>>> + .def_property_readonly(\"id\", &ControlId::id)\n>>> + .def_property_readonly(\"name\", &ControlId::name)\n>>> + .def_property_readonly(\"type\", &ControlId::type);\n>>> +\n>>> + pyRequest\n>>> + /* Fence is not supported, so we cannot expose addBuffer() directly */\n>>> + .def(\"addBuffer\", [](Request &self, const Stream *stream, FrameBuffer *buffer) {\n>>> + return self.addBuffer(stream, buffer);\n>>> + }, 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, ControlId &id, py::object value) {\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> \n> That will be an annoying limitation. Can you add a todo comment to fix\n> it (and line wrap this comment) ?\n> \n>>> + .def(\"reuse\", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });\n>>> +\n>>> + pyFrameMetadata\n>>> + .def_readonly(\"status\", &FrameMetadata::status)\n>>> + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n>>> + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n>>> + /* temporary helper, to be removed */\n> \n> \t\t/* \\todo Temporary helper, to be removed */\n> \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/src/py/meson.build b/src/py/meson.build\n>>> new file mode 100644\n>>> index 00000000..4ce9668c\n>>> --- /dev/null\n>>> +++ b/src/py/meson.build\n>>> @@ -0,0 +1 @@\n>>> +subdir('libcamera')\n>>> diff --git a/subprojects/.gitignore b/subprojects/.gitignore\n>>> index 391fde2c..0e194289 100644\n>>> --- a/subprojects/.gitignore\n>>> +++ b/subprojects/.gitignore\n>>> @@ -1,3 +1,4 @@\n>>> /googletest-release*\n>>> /libyuv\n>>> -/packagecache\n>>> \\ No newline at end of file\n>>> +/packagecache\n>>> +/pybind11\n>>> diff --git a/subprojects/packagefiles/pybind11/meson.build b/subprojects/packagefiles/pybind11/meson.build\n>>> new file mode 100644\n>>> index 00000000..67e89aec\n>>> --- /dev/null\n>>> +++ b/subprojects/packagefiles/pybind11/meson.build\n>>> @@ -0,0 +1,8 @@\n>>> +project('pybind11', 'cpp',\n>>> + version : '2.9.1',\n>>> + license : 'BSD-3-Clause')\n>>> +\n>>> +pybind11_incdir = include_directories('include')\n>>> +\n>>> +pybind11_dep = declare_dependency(\n>>> + include_directories : pybind11_incdir)\n> \n> 4 spaces for indentation.\n> \n>>> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n>>> new file mode 100644\n>>> index 00000000..2413e9ca\n>>> --- /dev/null\n>>> +++ b/subprojects/pybind11.wrap\n>>> @@ -0,0 +1,8 @@\n>>> +[wrap-git]\n>>> +url = https://github.com/pybind/pybind11.git\n> \n> A comment here to mention this is the smart_holder branch could be nice.\n> \n>>> +revision = 82734801f23314b4c34d70a79509e060a2648e04\n>>\n>> Great - this is now pointing directly to the pybind sources so I think\n>> that's much cleaner:\n>>\n>> It's hard to do a dedicated review on so much that I honestly don't\n>> fully comprehend yet - but I think getting this in and working/building\n>> on top is better for everyone, and as far as I know - it's already quite\n>> well tested by RPi (albeit, an older version, so I hope David can rebase\n>> and test sometime soon).\n>>\n>>\n>> Reviewed-by: Kieran Bingham \n>>\n>>> +depth = 1\n>>> +patch_directory = pybind11\n> \n> Do we actually have any patch ?\n\nWe have the meson.build file.\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 0B8C7C0F2A\n\tfor ;\n\tFri, 6 May 2022 08:11:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54C1F65643;\n\tFri, 6 May 2022 10:11:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5FC4E60421\n\tfor ;\n\tFri, 6 May 2022 10:11:50 +0200 (CEST)","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 981DA487;\n\tFri, 6 May 2022 10:11:49 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651824711;\n\tbh=XIb0GgbTwqHJ5e7iKwrrRLkyFlZ9b/LXtfadA/YRdlw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=yrkglAJhnfXu7qIuxUalUD+GpzAv54nTErM0n6GR9v/08DF0/YwftimrC0kDuP3ER\n\tN3UwzXy7J04vSREZDz61t3o86rZ6mWRCsJysKueyivGMpbPWotpcp6zlbJfMnMru6x\n\tTMnnkaYtvZoYIZEsyj1knJU8oNxOVtPtJZCSm4abqVJ5LPRP3vAbgs0JSFuRertiw6\n\tY6+CN3QAmhq8iYiJ/ezLYXdI7xqomufBCKpKVBzan09fcxT5YJeGmWp89sHCEga8tq\n\tVOzkwhFHjCIUN7v3meHDj+OcTxPaIeitMNxHCHXrTv3tK3nyxcntvcyZc5ph94Th3d\n\tLRiew/cSv44YA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651824709;\n\tbh=XIb0GgbTwqHJ5e7iKwrrRLkyFlZ9b/LXtfadA/YRdlw=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=nQsz1phuKfI45//+UfVluRBppRFg8Q+bqTJ036/G88ZmXkaMnyZfziTzzpbFTGtP8\n\tKoWa5QH9YPphqdQVgWvFpeD8V5PMYi0ujg2q5p7RNx4PpiTuBhrgNmOUHA9edsnHUv\n\tumyjLDYI8NTIA+gBOWbsMHD6/s1oOsOQX5mXJPE8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nQsz1phu\"; dkim-atps=neutral","Message-ID":"<046203e5-d966-7f3f-6491-d194b1a68753@ideasonboard.com>","Date":"Fri, 6 May 2022 11:11:46 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.8.0","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tKieran Bingham ","References":"<20220505104104.70841-1-tomi.valkeinen@ideasonboard.com>\n\t<20220505104104.70841-5-tomi.valkeinen@ideasonboard.com>\n\t<165175869453.1423360.6614915904429692992@Monstersaurus>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22874,"web_url":"https://patchwork.libcamera.org/comment/22874/","msgid":"","date":"2022-05-06T10:01:47","subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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, May 06, 2022 at 11:11:46AM +0300, Tomi Valkeinen wrote:\n> On 05/05/2022 20:32, Laurent Pinchart wrote:\n> > On Thu, May 05, 2022 at 02:51:34PM +0100, Kieran Bingham wrote:\n> >> Quoting Tomi Valkeinen (2022-05-05 11:40:55)\n> >>> Add libcamera Python bindings. pybind11 is used to generate the C++ <->\n> >>> Python layer.\n> >>>\n> >>> We use pybind11 'smart_holder' version to avoid issues with private\n> >>> destructors and shared_ptr. There is also an alternative solution here:\n> >>>\n> >>> https://github.com/pybind/pybind11/pull/2067\n> >>>\n> >>> Only a subset of libcamera classes are exposed. Implementing and testing\n> >>> the wrapper classes is challenging, and as such only classes that I have\n> >>> needed have been added so far.\n> >>>\n> >>> Signed-off-by: Tomi Valkeinen \n> >>> ---\n> >>> meson.build | 1 +\n> >>> meson_options.txt | 5 +\n> >>> src/meson.build | 1 +\n> >>> src/py/libcamera/__init__.py | 12 +\n> >>> src/py/libcamera/meson.build | 44 ++\n> >>> src/py/libcamera/pyenums.cpp | 53 ++\n> >>> src/py/libcamera/pymain.cpp | 452 ++++++++++++++++++\n> >>> src/py/meson.build | 1 +\n> >>> subprojects/.gitignore | 3 +-\n> >>> subprojects/packagefiles/pybind11/meson.build | 8 +\n> >>> subprojects/pybind11.wrap | 8 +\n> >>> 11 files changed, 587 insertions(+), 1 deletion(-)\n> >>> create mode 100644 src/py/libcamera/__init__.py\n> >>> create mode 100644 src/py/libcamera/meson.build\n> >>> create mode 100644 src/py/libcamera/pyenums.cpp\n> >>> create mode 100644 src/py/libcamera/pymain.cpp\n> >>> create mode 100644 src/py/meson.build\n> >>> create mode 100644 subprojects/packagefiles/pybind11/meson.build\n> >>> create mode 100644 subprojects/pybind11.wrap\n\n[snip]\n\n> >>> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py\n> >>> new file mode 100644\n> >>> index 00000000..cd7512a2\n> >>> --- /dev/null\n> >>> +++ b/src/py/libcamera/__init__.py\n> >>> @@ -0,0 +1,12 @@\n\n[snip]\n\n> >>> +def __FrameBuffer__mmap(self, plane):\n> > \n> > Let's record that more work is needed:\n> > \n> > # \\todo Support multi-planar formats\n> \n> This is implemented in a later patch in the series. I didn't want to \n> squash to highlight the change.\n\nI noticed that after reviewing this patch :-)\n\n> >>> + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)\n> >>> +\n> >>> +\n> >>> +FrameBuffer.mmap = __FrameBuffer__mmap\n\n[snip]\n\n> >>> diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp\n> >>> new file mode 100644\n> >>> index 00000000..39886656\n> >>> --- /dev/null\n> >>> +++ b/src/py/libcamera/pyenums.cpp\n> >>> @@ -0,0 +1,53 @@\n\n[snip]\n\n> >>> +void init_pyenums(py::module &m)\n> >>> +{\n> >>> + py::enum_(m, \"ConfigurationStatus\")\n> >>> + .value(\"Valid\", CameraConfiguration::Valid)\n> >>> + .value(\"Adjusted\", CameraConfiguration::Adjusted)\n> >>> + .value(\"Invalid\", CameraConfiguration::Invalid);\n> > \n> > It would be nice if C++ had some introspection capabilities that would\n> > allow this to be done automatically :-)\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::enum_(m, \"RequestStatus\")\n> >>> + .value(\"Pending\", Request::RequestPending)\n> > \n> > I wonder if the C++ enum should turn into an enum class, and the\n> > enumerators renamed to drop the Request prefix.\n> \n> Hmm, what are you suggesting? Changing the C++ enum to enum class? Isn't \n> that kind of a drastic change? In some situations enums and enum classes \n> behave quite differently, and in any case it's a big API change.\n> \n> I'm not against it, I was surprised to see so many enums used in the C++ \n> side, but... Maybe that's not part of thise series?\n\nSorry to have been unclear, I didn't mean doing it as part of this\nseries. It's an open question to get feedback on the idea.\n\n> >>> + .value(\"Complete\", Request::RequestComplete)\n> >>> + .value(\"Cancelled\", Request::RequestCancelled);\n> > \n> > Nothing that needs to be changed now, by what is more pythonic between\n> > \n> > Request.Status.Pending\n> > \n> > and\n> > \n> > RequestStatus.Pending\n> > \n> > ?\n> \n> Or Request.RequestPending.\n> \n> I have no idea =). I like the first one best. I did add some of the more \n> recent enums inside the related class, but for these I just copied the \n> C++ side style.\n> \n> >>> +\n> >>> + py::enum_(m, \"FrameMetadataStatus\")\n> >>> + .value(\"Success\", FrameMetadata::FrameSuccess)\n> > \n> > Same here, an enum class may make sense.\n> > \n> >>> + .value(\"Error\", FrameMetadata::FrameError)\n> >>> + .value(\"Cancelled\", FrameMetadata::FrameCancelled);\n> >>> +\n> >>> + py::enum_(m, \"ReuseFlag\")\n> >>> + .value(\"Default\", Request::ReuseFlag::Default)\n> >>> + .value(\"ReuseBuffers\", Request::ReuseFlag::ReuseBuffers);\n> >>> +\n> >>> + py::enum_(m, \"ControlType\")\n> >>> + .value(\"None\", ControlType::ControlTypeNone)\n> > \n> > Ditto.\n> > \n> >>> + .value(\"Bool\", ControlType::ControlTypeBool)\n> >>> + .value(\"Byte\", ControlType::ControlTypeByte)\n> >>> + .value(\"Integer32\", ControlType::ControlTypeInteger32)\n> >>> + .value(\"Integer64\", ControlType::ControlTypeInteger64)\n> >>> + .value(\"Float\", ControlType::ControlTypeFloat)\n> >>> + .value(\"String\", ControlType::ControlTypeString)\n> >>> + .value(\"Rectangle\", ControlType::ControlTypeRectangle)\n> >>> + .value(\"Size\", ControlType::ControlTypeSize);\n> >>> +}\n> >>> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp\n> >>> new file mode 100644\n> >>> index 00000000..54674caf\n> >>> --- /dev/null\n> >>> +++ b/src/py/libcamera/pymain.cpp\n> >>> @@ -0,0 +1,452 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2021, Tomi Valkeinen \n> >>> + *\n> >>> + * Python bindings\n> >>> + */\n> >>> +\n> >>> +/*\n> >>> + * To generate pylibcamera stubs:\n> >>> + * PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera\n> > \n> > What is this for ?\n> \n> Oh, that's just a comment for myself on how to generate stubs for the \n> library. I need to add a bit more context here, or remove it.\n\nI'm fine keeping it if the comment is expanded to explain why it's\nuseful :-)\n\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> > \n> > You're using the std:: namespace qualifier explicitly in several\n> > location below. Could we do that everywhere and drop the using namespace\n> > directive ?\n> \n> I'd prefer to use the 'using namespace' and drop the std:: in the code. \n> The few std:: uses are left-overs from copy pastes from examples...\n\nFor the libcamera namespace I agree as the name is long (although an\nalias with `using namespace cam = libcamera` may be a good idea), but\nfor std we try to make it explicit to avoid namespace clashes. Just\n`vector` looks quite alien compared to the rest of the libcamera code\nbase. On a related note, I have on my todo list to drop the `using\nnamespace std` from unit tests.\n\n> >>> +using namespace libcamera;\n> >>> +\n> >>> +template\n> >>> +static py::object ValueOrTuple(const ControlValue &cv)\n> > \n> > Could you rename this to valueOrTuple() to follow the libcamera coding\n> > style ? Or would that look awkward from a python bindings coding style\n> > point of view ?\n> \n> No, I think it's fine.\n> \n> > Same for ControlValueToPy and PyToControlValue.\n> > \n> >>> +{\n> >>> + if (cv.isArray()) {\n> >>> + const T *v = reinterpret_cast(cv.data().data());\n> >>> + auto t = py::tuple(cv.numElements());\n> >>> +\n> >>> + for (size_t i = 0; i < cv.numElements(); ++i)\n> >>> + t[i] = v[i];\n> >>> +\n> >>> + return t;\n> >>> + }\n> >>> +\n> >>> + return py::cast(cv.get());\n> >>> +}\n> >>> +\n> >>> +static py::object ControlValueToPy(const ControlValue &cv)\n> >>> +{\n> >>> + switch (cv.type()) {\n> >>> + case ControlTypeBool:\n> >>> + return ValueOrTuple(cv);\n> >>> + case ControlTypeByte:\n> >>> + return ValueOrTuple(cv);\n> >>> + case ControlTypeInteger32:\n> >>> + return ValueOrTuple(cv);\n> >>> + case ControlTypeInteger64:\n> >>> + return ValueOrTuple(cv);\n> >>> + case ControlTypeFloat:\n> >>> + return ValueOrTuple(cv);\n> >>> + case ControlTypeString:\n> >>> + return py::cast(cv.get());\n> >>> + case ControlTypeRectangle: {\n> > \n> > \t/* \\todo Add geometry classes to the Python bindings */\n> \n> What's missing? The switch handles all the possible cases afaics.\n\nI meant adding bindings for the geometry classes themselves. At the\nmoment they're translated to tuples.\n\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> > \n> > A todo comment would be nice here too.\n> > \n> >>> + case ControlTypeNone:\n> >>> + default:\n> >>> + throw runtime_error(\"Control type not implemented\");\n> >>> + }\n> >>> +}\n> >>> +\n> >>> +static weak_ptr g_camera_manager;\n> > \n> > Coding style here too, gCameraManager, or would that look too awkward ?\n> > Same below.\n> > \n> >>> +static int g_eventfd;\n> >>> +static mutex g_reqlist_mutex;\n> >>> +static vector g_reqlist;\n> >>> +\n> >>> +static void handleRequestCompleted(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> >>> +void init_pyenums(py::module &m);\n> >>> +\n> >>> +PYBIND11_MODULE(_libcamera, m)\n> >>> +{\n> >>> + init_pyenums(m);\n> >>> +\n> >>> + /* Forward declarations */\n> >>> +\n> >>> + /*\n> >>> + * We need to declare all the classes here so that Python docstrings\n> >>> + * can be generated correctly.\n> >>> + * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n> >>> + */\n> >>> +\n> >>> + auto pyCameraManager = py::class_(m, \"CameraManager\");\n> >>> + auto pyCamera = py::class_(m, \"Camera\");\n> >>> + auto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n> >>> + auto pyStreamConfiguration = py::class_(m, \"StreamConfiguration\");\n> >>> + auto pyStreamFormats = py::class_(m, \"StreamFormats\");\n> >>> + auto pyFrameBufferAllocator = py::class_(m, \"FrameBufferAllocator\");\n> >>> + auto pyFrameBuffer = py::class_(m, \"FrameBuffer\");\n> >>> + auto pyStream = py::class_(m, \"Stream\");\n> >>> + auto pyControlId = py::class_(m, \"ControlId\");\n> >>> + auto pyRequest = py::class_(m, \"Request\");\n> >>> + auto pyFrameMetadata = py::class_(m, \"FrameMetadata\");\n> > \n> > Could you please sort these alphabetically ?\n> \n> Later additions introduce classes that depend on each other, and cannot \n> be sorted. Also, these are in the same order as they are in the file \n> below. So, I'd rather keep them as they are.\n\nI would have proposed to change the order below too :-) But if there are\ndependencies, it's indeed an issue :-( Would it make sense to make them\nmostly sorted ?\n\n> >>> +\n> >>> + /* Global functions */\n> >>> + m.def(\"logSetLevel\", &logSetLevel);\n> >>> +\n> >>> + /* Classes */\n> >>> + pyCameraManager\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> > Some long lines like this one can be wrapped:\n> > \n> > \t\t\t\tthrow std::system_error(errno, std::generic_category(),\n> > \t\t\t\t\t\t\t\"Failed to create eventfd\");\n> >>> +\n> >>> + 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> > \n> > As we don't usually prefix getters with \"get\", maybe just\n> > \"readyRequests\" ?\n> \n> Hmm, this is not a getter, at least not as I see a getter. A getter \n> returns the value of a field, and would be exposed as a property. This \n> is a function that does work.\n\nOK.\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> > \n> > s/decrease/Decrease/\n> > \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> > Do we really want to ignore the case in the comparison here ?\n> > \n> >>> +\n> >>> + if (id.find(str) != string::npos)\n> > \n> > Actually, does this function really belong in the python bindings ? It\n> > seems to be used in unit tests only, where you could iterate over the\n> > cameras exposed by the cameras property instead.\n> \n> I think I agree. I probably added this quite early, when I didn't know \n> how to expose a list of cameras.\n> \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> >>> + pyCamera\n> >>> + .def_property_readonly(\"id\", &Camera::id)\n> >>> + .def(\"acquire\", &Camera::acquire)\n> >>> + .def(\"release\", &Camera::release)\n> >>> + .def(\"start\", [](Camera &self) {\n> >>> + self.requestCompleted.connect(handleRequestCompleted);\n> >>\n> >> What happens if someone calls start() multiple times? Is that going to\n> >> mean the signal / slot will be connected multiple times?\n> >>\n> >> That could be problematic ... as I think it means it could then call the\n> >> request completed handler multiple times - but it may also already be\n> >> trapped by the recent work we did around there.\n> >>\n> >> So for now I think this is fine - but may be something to test or\n> >> consider later.\n> > \n> > Indeed. A \\todo comment would be nice.\n> > \n> >>> +\n> >>> + int ret = self.start();\n> >>> + if (ret)\n> >>> + self.requestCompleted.disconnect(handleRequestCompleted);\n> >>> +\n> >>> + return ret;\n> > \n> > \t\t\tint ret = self.start();\n> > \t\t\tif (ret) {\n> > \t\t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n> > \t\t\t\treturn ret;\n> > \t\t\t}\n> > \n> > \t\t\treturn 0;\n> > \n> >>> + })\n> >>> +\n> >>> + .def(\"stop\", [](Camera &self) {\n> >>> + int ret = self.stop();\n> >>> + if (!ret)\n> >>> + self.requestCompleted.disconnect(handleRequestCompleted);\n> >>> +\n> >>> + return ret;\n> > \n> > \t\t\tint ret = self.stop();\n> > \t\t\tif (ret)\n> > \t\t\t\treturn ret;\n> > \n> > \t\t\tself.requestCompleted.disconnect(handleRequestCompleted);\n> > \t\t\treturn 0;\n> > \n> >>> + })\n> >>> +\n> >>> + .def(\"__repr__\", [](Camera &self) {\n> >>> + return \"\";\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> > \n> > \t\t\t/*\n> > \t\t\t * Increase the reference count, will be dropped in\n> > \t\t\t * getReadyRequests().\n> > \t\t\t */\n> > \n> > I wonder what happens if nobody calls getReadyRequests() though, for\n> > instance for the last requesuts when stopping the camera. If there's an\n> > issue there, please add a \\todo comment somewhere.\n> \n> The handled requests will then sit in the gReqList. What is the issue \n> you see? Memory leak? We \"leak\" in any case, as the CameraManager \n> singleton is always there, and the gReqList is essentially a field of \n> the camera manager singleton.\n\nYes, memory leaks, and possible crashes if we clean up things in the\nwrong order. This can be addressed later, but a comment that states the\nissue could be nice.\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(\"find_control\", [](Camera &self, const string &name) {\n> > \n> > \"findControl\"\n> \n> This is the python API, so \"find_control\" is the correct one. That said, \n> I mix up the styles in many places. To be honest, I don't know if a \n> direct binding should use the original names or pythonic names.\n\nI'm not sure either :-) What bothers me is that you have \"createRequest\"\nabove and \"find_control\" here. We should at least be consistent.\n\nI think I have a preference for native names, as that will make it\neasier to navigate documentation and code between C++ and Python.\n\n> But I think I'll just go with the pythonic names.\n> \n> >>> + const auto &controls = self.controls();\n> >>> +\n> >>> + auto it = find_if(controls.begin(), controls.end(),\n> >>> + [&name](const auto &kvp) { return kvp.first->name() == name; });\n> >>> +\n> >>> + if (it == controls.end())\n> >>> + throw runtime_error(\"Control not found\");\n> >>> +\n> >>> + return it->first;\n> >>> + }, py::return_value_policy::reference_internal)\n> >>> +\n> >>> + .def_property_readonly(\"controls\", [](Camera &self) {\n> >>> + py::dict ret;\n> >>> +\n> >>> + for (const auto &[id, ci] : self.controls()) {\n> > \n> > \t\t\t\t/* \\todo Add bindings for the ControlInfo class */\n> > \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> >>> + pyCameraConfiguration\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> > \n> > Line wrap.\n> > \n> >>> + .def_property_readonly(\"size\", &CameraConfiguration::size)\n> >>> + .def_property_readonly(\"empty\", &CameraConfiguration::empty);\n> >>> +\n> >>> + pyStreamConfiguration\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> > \n> > That's a very long line too.\n> > \n> > \t\t\t[](StreamConfiguration &self) {\n> > \t\t\t\treturn make_tuple(self.size.width, self.size.height);\n> > \t\t\t},\n> > \t\t\t[](StreamConfiguration &self, tuple size) {\n> > \t\t\t\tself.size.width = get<0>(size);\n> > \t\t\t\tself.size.height = get<1>(size);\n> > \t\t\t})\n> > \n> > Same where applicable.\n> > \n> >>> + .def_property(\n> >>> + \"pixelFormat\",\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> >>> + pyStreamFormats\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> >>> + pyFrameBufferAllocator\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> >>> + pyFrameBuffer\n> >>> + /* TODO: implement FrameBuffer::Plane properly */\n> > \n> > s/TODO: implement/\\\\todo Implement/\n> > \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> >>> + pyStream\n> >>> + .def_property_readonly(\"configuration\", &Stream::configuration);\n> >>> +\n> >>> + pyControlId\n> >>> + .def_property_readonly(\"id\", &ControlId::id)\n> >>> + .def_property_readonly(\"name\", &ControlId::name)\n> >>> + .def_property_readonly(\"type\", &ControlId::type);\n> >>> +\n> >>> + pyRequest\n> >>> + /* Fence is not supported, so we cannot expose addBuffer() directly */\n> >>> + .def(\"addBuffer\", [](Request &self, const Stream *stream, FrameBuffer *buffer) {\n> >>> + return self.addBuffer(stream, buffer);\n> >>> + }, 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, ControlId &id, py::object value) {\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> > \n> > That will be an annoying limitation. Can you add a todo comment to fix\n> > it (and line wrap this comment) ?\n> > \n> >>> + .def(\"reuse\", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });\n> >>> +\n> >>> + pyFrameMetadata\n> >>> + .def_readonly(\"status\", &FrameMetadata::status)\n> >>> + .def_readonly(\"sequence\", &FrameMetadata::sequence)\n> >>> + .def_readonly(\"timestamp\", &FrameMetadata::timestamp)\n> >>> + /* temporary helper, to be removed */\n> > \n> > \t\t/* \\todo Temporary helper, to be removed */\n> > \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/src/py/meson.build b/src/py/meson.build\n> >>> new file mode 100644\n> >>> index 00000000..4ce9668c\n> >>> --- /dev/null\n> >>> +++ b/src/py/meson.build\n> >>> @@ -0,0 +1 @@\n> >>> +subdir('libcamera')\n> >>> diff --git a/subprojects/.gitignore b/subprojects/.gitignore\n> >>> index 391fde2c..0e194289 100644\n> >>> --- a/subprojects/.gitignore\n> >>> +++ b/subprojects/.gitignore\n> >>> @@ -1,3 +1,4 @@\n> >>> /googletest-release*\n> >>> /libyuv\n> >>> -/packagecache\n> >>> \\ No newline at end of file\n> >>> +/packagecache\n> >>> +/pybind11\n> >>> diff --git a/subprojects/packagefiles/pybind11/meson.build b/subprojects/packagefiles/pybind11/meson.build\n> >>> new file mode 100644\n> >>> index 00000000..67e89aec\n> >>> --- /dev/null\n> >>> +++ b/subprojects/packagefiles/pybind11/meson.build\n> >>> @@ -0,0 +1,8 @@\n> >>> +project('pybind11', 'cpp',\n> >>> + version : '2.9.1',\n> >>> + license : 'BSD-3-Clause')\n> >>> +\n> >>> +pybind11_incdir = include_directories('include')\n> >>> +\n> >>> +pybind11_dep = declare_dependency(\n> >>> + include_directories : pybind11_incdir)\n> > \n> > 4 spaces for indentation.\n> > \n> >>> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\n> >>> new file mode 100644\n> >>> index 00000000..2413e9ca\n> >>> --- /dev/null\n> >>> +++ b/subprojects/pybind11.wrap\n> >>> @@ -0,0 +1,8 @@\n> >>> +[wrap-git]\n> >>> +url = https://github.com/pybind/pybind11.git\n> > \n> > A comment here to mention this is the smart_holder branch could be nice.\n> > \n> >>> +revision = 82734801f23314b4c34d70a79509e060a2648e04\n> >>\n> >> Great - this is now pointing directly to the pybind sources so I think\n> >> that's much cleaner:\n> >>\n> >> It's hard to do a dedicated review on so much that I honestly don't\n> >> fully comprehend yet - but I think getting this in and working/building\n> >> on top is better for everyone, and as far as I know - it's already quite\n> >> well tested by RPi (albeit, an older version, so I hope David can rebase\n> >> and test sometime soon).\n> >>\n> >>\n> >> Reviewed-by: Kieran Bingham \n> >>\n> >>> +depth = 1\n> >>> +patch_directory = pybind11\n> > \n> > Do we actually have any patch ?\n> \n> We have the meson.build file.\n\nI got confused by the fact that patch_directory points to an overlay\ndirectory, not a directory containing patches. My bad.","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 3C3BAC0F2A\n\tfor ;\n\tFri, 6 May 2022 10:01:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E0AC65648;\n\tFri, 6 May 2022 12:01:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 28D6B604A3\n\tfor ;\n\tFri, 6 May 2022 12:01:52 +0200 (CEST)","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 5986F487;\n\tFri, 6 May 2022 12:01:51 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651831313;\n\tbh=sixriGbSSWCf8odalK0GbNXlzj7L7Pz/tUn8W0De1QI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=X7DSE0dlngnKRvkb/GkEoZbiUi+G6YpBHoumPCd8Q3OxbMsG3xEL4Cdp92xStxGBH\n\t6nEwOReVhyHj+l12gO1GI+X7BXIgLI8NJgjZrS6lCNI15jMHsKAXMUoO6+89BzpKDV\n\t+8eYAImlVOIIeHu9AsXvxUMg4nb3isBnMC0N6I3yotAvMpIxGqu/BMzPmuzrRAR2L1\n\tJ/blZgXxoaKgPWar8MB1DdyBY+86EJKsg80bSQMsS1D0CPthYQV/Bw2XzkicHqXZDI\n\tEf26N+UJx24cIdnGZi6M4k8KDKKZuimU4cuKzXFH3jNFbh5Clm03HIF77l4Q/t8qvF\n\tm4QQQ8Vrm37+Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651831311;\n\tbh=sixriGbSSWCf8odalK0GbNXlzj7L7Pz/tUn8W0De1QI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ur+3kIbnrA5jdbMp6CzhPpfXX3KcHHIFdqEs43zfqoA7NlWA9oPoIvlDPk8vR0m/f\n\tqWn6Yvq636jh+h9r3/hDGTMEEjPZg271kBWn+0u3b5gEWiLppdyoxnM7KI0tfGjPt1\n\tuT2A4UnAwJB1DDIcZzLdXU6FAQLAywdPYKg7UCoQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ur+3kIbn\"; dkim-atps=neutral","Date":"Fri, 6 May 2022 13:01:47 +0300","To":"Tomi Valkeinen ","Message-ID":"","References":"<20220505104104.70841-1-tomi.valkeinen@ideasonboard.com>\n\t<20220505104104.70841-5-tomi.valkeinen@ideasonboard.com>\n\t<165175869453.1423360.6614915904429692992@Monstersaurus>\n\t\n\t<046203e5-d966-7f3f-6491-d194b1a68753@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<046203e5-d966-7f3f-6491-d194b1a68753@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v7 04/13] 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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]