[libcamera-devel] [PATCH v5 1/3] Add Python bindings
Tomi Valkeinen
tomi.valkeinen at ideasonboard.com
Wed Apr 13 11:10:45 CEST 2022
On 13/04/2022 10:57, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2022-04-12 18:49:53)
>> Hi Tomi,
>> I've been using the bindings in the last days, they're nice to
>> work! Great job!
>>
>> Once the question about a Request belonging to a Camera is
>> clarified I think we should merge these soon, even if incomplete, to
>> build on top.
>>
>> My understanding of python is very limited so I have just a few minor
>> comments and one larger question about controls.
>>
>> On Mon, Mar 14, 2022 at 05:46:31PM +0200, Tomi Valkeinen via libcamera-devel wrote:
>>> Add libcamera Python bindings. pybind11 is used to generate the C++ <->
>>> Python layer.
>>>
>>> We use pybind11 'smart_holder' version to avoid issues with private
>>> destructors and shared_ptr. There is also an alternative solution here:
>>>
>>> https://github.com/pybind/pybind11/pull/2067
>
> Why is the 'smart_holder' a branch of pybind11 ? I can't see a PR or
> anything such that indicates how or when it might get merged.
>
> Is there any other reference to the developments here?
There's a readme:
https://github.com/pybind/pybind11/blob/49f8f60ec4254e0f55db3c095c945210bcb43ad2/README_smart_holder.rst
I haven't studied the smart_holder discussions to find out if there's a
clear plan on how and if it gets merged to the main pybind11.
>>> Only a subset of libcamera classes are exposed. Implementing and testing
>>> the wrapper classes is challenging, and as such only classes that I have
>>> needed have been added so far.
>
> I think that's reasonable
>
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>>> ---
>>> meson.build | 1 +
>>> meson_options.txt | 5 +
>>> src/meson.build | 1 +
>>> src/py/libcamera/__init__.py | 10 +
>>> src/py/libcamera/meson.build | 43 ++++
>>> src/py/libcamera/pyenums.cpp | 53 ++++
>>> src/py/libcamera/pymain.cpp | 453 +++++++++++++++++++++++++++++++++++
>>> src/py/meson.build | 1 +
>>> subprojects/.gitignore | 3 +-
>>> subprojects/pybind11.wrap | 6 +
>>> 10 files changed, 575 insertions(+), 1 deletion(-)
>>> create mode 100644 src/py/libcamera/__init__.py
>>> create mode 100644 src/py/libcamera/meson.build
>>> create mode 100644 src/py/libcamera/pyenums.cpp
>>> create mode 100644 src/py/libcamera/pymain.cpp
>>> create mode 100644 src/py/meson.build
>>> create mode 100644 subprojects/pybind11.wrap
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 29d8542d..ff6c2ad6 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -179,6 +179,7 @@ summary({
>>> 'Tracing support': tracing_enabled,
>>> 'Android support': android_enabled,
>>> 'GStreamer support': gst_enabled,
>>> + 'Python bindings': pycamera_enabled,
>>> 'V4L2 emulation support': v4l2_enabled,
>>> 'cam application': cam_enabled,
>>> 'qcam application': qcam_enabled,
>>> diff --git a/meson_options.txt b/meson_options.txt
>>> index 2c80ad8b..ca00c78e 100644
>>> --- a/meson_options.txt
>>> +++ b/meson_options.txt
>>> @@ -58,3 +58,8 @@ option('v4l2',
>>> type : 'boolean',
>>> value : false,
>>> description : 'Compile the V4L2 compatibility layer')
>>> +
>>> +option('pycamera',
>>> + type : 'feature',
>>> + value : 'auto',
>>> + description : 'Enable libcamera Python bindings (experimental)')
>>> diff --git a/src/meson.build b/src/meson.build
>>> index e0ea9c35..34663a6f 100644
>>> --- a/src/meson.build
>>> +++ b/src/meson.build
>>> @@ -37,4 +37,5 @@ subdir('cam')
>>> subdir('qcam')
>>>
>>> subdir('gstreamer')
>>> +subdir('py')
>>> subdir('v4l2')
>>> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py
>>> new file mode 100644
>>> index 00000000..b30bf33a
>>> --- /dev/null
>>> +++ b/src/py/libcamera/__init__.py
>>> @@ -0,0 +1,10 @@
>>> +# SPDX-License-Identifier: LGPL-2.1-or-later
>>> +# Copyright (C) 2021, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>>> +
>>> +from ._libcamera import *
>>> +import mmap
>>> +
>>> +def __FrameBuffer__mmap(self, plane):
>>> + return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)
>>> +
>>> +FrameBuffer.mmap = __FrameBuffer__mmap
>
> This seems like an odd place to have this. Is it a workaround ? Can it
> live with other FrameBuffer code? or a comment to say why it's here?
>
> Should it live in something like framebuffer.py?
Yes, I should have added a comment. It was a quick way to get mmap
working. I don't see why it couldn't be done in the cpp code, but I
recall having some issues figuring out how to do that:
- If the cpp code would use native mmap, then it would also need to wrap
it somehow so that the mmaped area is usable in the python side
- Using python's mmap (like I do above) is possible from the cpp code,
but I think I didn't figure it out right away, so I just did it with python.
>>> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
>>> new file mode 100644
>>> index 00000000..82388efb
>>> --- /dev/null
>>> +++ b/src/py/libcamera/meson.build
>>> @@ -0,0 +1,43 @@
>>> +# SPDX-License-Identifier: CC0-1.0
>>> +
>>> +py3_dep = dependency('python3', required : get_option('pycamera'))
>>> +
>>> +if not py3_dep.found()
>>> + pycamera_enabled = false
>>> + subdir_done()
>>> +endif
>>> +
>>> +pycamera_enabled = true
>>> +
>>> +pybind11_proj = subproject('pybind11')
>>> +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')
>>> +
>>> +pycamera_sources = files([
>>> + 'pymain.cpp',
>>> + 'pyenums.cpp',
>>> +])
>>> +
>>> +pycamera_deps = [
>>> + libcamera_public,
>>> + py3_dep,
>>> + pybind11_dep,
>>> +]
>>> +
>>> +pycamera_args = ['-fvisibility=hidden']
>>> +pycamera_args += ['-Wno-shadow']
>>> +pycamera_args += ['-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT']
>>> +
>>> +destdir = get_option('libdir') + '/python' + py3_dep.version() + '/site-packages/libcamera'
>>> +
>>> +pycamera = shared_module('_libcamera',
>>> + pycamera_sources,
>>> + install : true,
>>> + install_dir : destdir,
>>> + name_prefix : '',
>>> + dependencies : pycamera_deps,
>>> + cpp_args : pycamera_args)
>>> +
>>> +run_command('ln', '-fsT', '../../../../src/py/libcamera/__init__.py',
>>> + meson.current_build_dir() / '__init__.py')
>
> Is this to support running from the build dir? or is it part of the
> requirements to build?
>
> A quick comment above could help make it clear.
For running from the build dir.
> I'm not sure I like the ../../../../ relative path ... is there a meson
> variable we can use to identify that location to build the correct /
> required path?
>
> I can look the other way on the ../../ path though...
I guess it depends, but usually a relative path is better with sym
links, so that they continue working even if the whole dir structure is
moved somewhere else.
>>> +
>>> +install_data(['__init__.py'], install_dir : destdir)
>>> diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp
>>> new file mode 100644
>>> index 00000000..af6151c8
>>> --- /dev/null
>>> +++ b/src/py/libcamera/pyenums.cpp
>>> @@ -0,0 +1,53 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2021, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>>> + *
>>> + * Python bindings
>>> + */
>>> +
>>> +#include <libcamera/libcamera.h>
>>> +
>>> +#include <pybind11/smart_holder.h>
>>> +
>>> +namespace py = pybind11;
>>> +
>>> +using namespace libcamera;
>>> +
>>> +void init_pyenums(py::module& m)
>>> +{
>>> + py::enum_<CameraConfiguration::Status>(m, "ConfigurationStatus")
>>> + .value("Valid", CameraConfiguration::Valid)
>>> + .value("Adjusted", CameraConfiguration::Adjusted)
>>> + .value("Invalid", CameraConfiguration::Invalid);
>>> +
>>> + py::enum_<StreamRole>(m, "StreamRole")
>>> + .value("StillCapture", StreamRole::StillCapture)
>>> + .value("Raw", StreamRole::Raw)
>>> + .value("VideoRecording", StreamRole::VideoRecording)
>>> + .value("Viewfinder", StreamRole::Viewfinder);
>>> +
>>> + py::enum_<Request::Status>(m, "RequestStatus")
>>> + .value("Pending", Request::RequestPending)
>>> + .value("Complete", Request::RequestComplete)
>>> + .value("Cancelled", Request::RequestCancelled);
>>> +
>>> + py::enum_<FrameMetadata::Status>(m, "FrameMetadataStatus")
>>> + .value("Success", FrameMetadata::FrameSuccess)
>>> + .value("Error", FrameMetadata::FrameError)
>>> + .value("Cancelled", FrameMetadata::FrameCancelled);
>>> +
>>> + py::enum_<Request::ReuseFlag>(m, "ReuseFlag")
>>> + .value("Default", Request::ReuseFlag::Default)
>>> + .value("ReuseBuffers", Request::ReuseFlag::ReuseBuffers);
>>> +
>>> + py::enum_<ControlType>(m, "ControlType")
>>> + .value("None", ControlType::ControlTypeNone)
>>> + .value("Bool", ControlType::ControlTypeBool)
>>> + .value("Byte", ControlType::ControlTypeByte)
>>> + .value("Integer32", ControlType::ControlTypeInteger32)
>>> + .value("Integer64", ControlType::ControlTypeInteger64)
>>> + .value("Float", ControlType::ControlTypeFloat)
>>> + .value("String", ControlType::ControlTypeString)
>>> + .value("Rectangle", ControlType::ControlTypeRectangle)
>>> + .value("Size", ControlType::ControlTypeSize);
>>> +}
>>> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
>>> new file mode 100644
>>> index 00000000..7701da40
>>> --- /dev/null
>>> +++ b/src/py/libcamera/pymain.cpp
>>> @@ -0,0 +1,453 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2021, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>>> + *
>>> + * Python bindings
>>> + */
>>> +
>>> +/*
>>> + * To generate pylibcamera stubs:
>>> + * PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera
>>> + */
>>> +
>>> +#include <chrono>
>>> +#include <fcntl.h>
>>> +#include <mutex>
>>> +#include <sys/eventfd.h>
>>> +#include <sys/mman.h>
>>> +#include <thread>
>>> +#include <unistd.h>
>>> +
>>> +#include <libcamera/libcamera.h>
>>> +
>>> +#include <pybind11/smart_holder.h>
>>> +#include <pybind11/functional.h>
>>> +#include <pybind11/stl.h>
>>> +#include <pybind11/stl_bind.h>
>
> sort order? Did checkstyle not catch this?
> (Do you have the post-commit hook enabled?)
We have a checkstyle? =) Yes, I see a script in utils. I'll try it out.
>>> +
>>> +namespace py = pybind11;
>>> +
>>> +using namespace std;
>>> +using namespace libcamera;
>>> +
>>> +template<typename T>
>>> +static py::object ValueOrTuple(const ControlValue &cv)
>>> +{
>>> + if (cv.isArray()) {
>>> + const T *v = reinterpret_cast<const T *>(cv.data().data());
>>> + auto t = py::tuple(cv.numElements());
>>> +
>>> + for (size_t i = 0; i < cv.numElements(); ++i)
>>> + t[i] = v[i];
>>> +
>>> + return t;
>>> + }
>>> +
>>> + return py::cast(cv.get<T>());
>>> +}
>>> +
>>> +static py::object ControlValueToPy(const ControlValue &cv)
>>> +{
>>> + switch (cv.type()) {
>>> + case ControlTypeBool:
>>> + return ValueOrTuple<bool>(cv);
>>> + case ControlTypeByte:
>>> + return ValueOrTuple<uint8_t>(cv);
>>> + case ControlTypeInteger32:
>>> + return ValueOrTuple<int32_t>(cv);
>>> + case ControlTypeInteger64:
>>> + return ValueOrTuple<int64_t>(cv);
>>> + case ControlTypeFloat:
>>> + return ValueOrTuple<float>(cv);
>>> + case ControlTypeString:
>>> + return py::cast(cv.get<string>());
>>> + case ControlTypeRectangle: {
>>> + const Rectangle *v = reinterpret_cast<const Rectangle *>(cv.data().data());
>>> + return py::make_tuple(v->x, v->y, v->width, v->height);
>>> + }
>>> + case ControlTypeSize: {
>>> + const Size *v = reinterpret_cast<const Size *>(cv.data().data());
>>> + return py::make_tuple(v->width, v->height);
>>> + }
>>> + case ControlTypeNone:
>>> + default:
>>> + throw runtime_error("Unsupported ControlValue type");
>>> + }
>>> +}
>>> +
>>> +static ControlValue PyToControlValue(const py::object &ob, ControlType type)
>>> +{
>>> + switch (type) {
>>> + case ControlTypeBool:
>>> + return ControlValue(ob.cast<bool>());
>>> + case ControlTypeByte:
>>> + return ControlValue(ob.cast<uint8_t>());
>>> + case ControlTypeInteger32:
>>> + return ControlValue(ob.cast<int32_t>());
>>> + case ControlTypeInteger64:
>>> + return ControlValue(ob.cast<int64_t>());
>>> + case ControlTypeFloat:
>>> + return ControlValue(ob.cast<float>());
>>> + case ControlTypeString:
>>> + return ControlValue(ob.cast<string>());
>>> + case ControlTypeRectangle:
>>> + case ControlTypeSize:
>>> + case ControlTypeNone:
>>> + default:
>>> + throw runtime_error("Control type not implemented");
>>> + }
>>> +}
>>> +
>>> +static weak_ptr<CameraManager> g_camera_manager;
>>> +static int g_eventfd;
>>> +static mutex g_reqlist_mutex;
>>> +static vector<Request *> g_reqlist;
>>> +
>>> +static void handleRequestCompleted(Request *req)
>>> +{
>>> + {
>>> + lock_guard guard(g_reqlist_mutex);
>>> + g_reqlist.push_back(req);
>>> + }
>>> +
>>> + uint64_t v = 1;
>>> + write(g_eventfd, &v, 8);
>>> +}
>>> +
>>> +void init_pyenums(py::module& m);
>>> +
>>> +PYBIND11_MODULE(_libcamera, m)
>>> +{
>
> Do these all have to be in a single definition file to maintain this
> scope? Or could the classes be broken down to one file per class? I
> could imagine these growing rapidly - so maintaining them would likely
> be easier to parse if they matched the file hierarchy of the
> corresponding libcamera impelemttion.
These can be split, as I have done with the enums (see the
init_pyenums() call below).
I don't know if file per class makes sense, as there's an overhead. But
it is possible. When using pybind11 I've done a split when I feel the
bindings grow too big, and there's a clear set of code to separate.
>>> + init_pyenums(m);
>>> +
>>> + /* Forward declarations */
>>> +
>>> + /*
>>> + * We need to declare all the classes here so that Python docstrings
>>> + * can be generated correctly.
>>> + * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings
>>> + */
>>> +
>>> + auto pyCameraManager = py::class_<CameraManager>(m, "CameraManager");
>>> + auto pyCamera = py::class_<Camera>(m, "Camera");
>>> + auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
>>> + auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
>>> + auto pyStreamFormats = py::class_<StreamFormats>(m, "StreamFormats");
>>> + auto pyFrameBufferAllocator = py::class_<FrameBufferAllocator>(m, "FrameBufferAllocator");
>>> + auto pyFrameBuffer = py::class_<FrameBuffer>(m, "FrameBuffer");
>>> + auto pyStream = py::class_<Stream>(m, "Stream");
>>> + auto pyControlId = py::class_<ControlId>(m, "ControlId");
>>> + auto pyRequest = py::class_<Request>(m, "Request");
>>> + auto pyFrameMetadata = py::class_<FrameMetadata>(m, "FrameMetadata");
>>> +
>>> + /* Global functions */
>>> + m.def("logSetLevel", &logSetLevel);
>>> +
>>> + /* Classes */
>>> + pyCameraManager
>>> + .def_static("singleton", []() {
>>> + shared_ptr<CameraManager> cm = g_camera_manager.lock();
>>> + if (cm)
>>> + return cm;
>>> +
>>> + int fd = eventfd(0, 0);
>>> + if (fd == -1)
>>> + throw std::system_error(errno, std::generic_category(), "Failed to create eventfd");
>>> +
>>> + cm = shared_ptr<CameraManager>(new CameraManager, [](auto p) {
>>> + close(g_eventfd);
>>> + g_eventfd = -1;
>>> + delete p;
>>> + });
>>> +
>>> + g_eventfd = fd;
>>> + g_camera_manager = cm;
>>> +
>>> + int ret = cm->start();
>>> + if (ret)
>>> + throw std::system_error(-ret, std::generic_category(), "Failed to start CameraManager");
>>> +
>>> + return cm;
>>> + })
>>> +
>>> + .def_property_readonly("version", &CameraManager::version)
>>> +
>>> + .def_property_readonly("efd", [](CameraManager &) {
>>> + return g_eventfd;
>>> + })
>>> +
>>> + .def("getReadyRequests", [](CameraManager &) {
>>> + vector<Request *> v;
>>> +
>>> + {
>>> + lock_guard guard(g_reqlist_mutex);
>>> + swap(v, g_reqlist);
>>> + }
>>> +
>>> + vector<py::object> ret;
>>> +
>>> + for (Request *req : v) {
>>> + py::object o = py::cast(req);
>>> + /* decrease the ref increased in Camera::queueRequest() */
>>> + o.dec_ref();
>>> + ret.push_back(o);
>>> + }
>>> +
>>> + return ret;
>>> + })
>>> +
>>> + .def("get", py::overload_cast<const string &>(&CameraManager::get), py::keep_alive<0, 1>())
>>> +
>>> + .def("find", [](CameraManager &self, string str) {
>>> + std::transform(str.begin(), str.end(), str.begin(), ::tolower);
>>> +
>>> + for (auto c : self.cameras()) {
>>> + string id = c->id();
>>> +
>>> + std::transform(id.begin(), id.end(), id.begin(), ::tolower);
>>> +
>>> + if (id.find(str) != string::npos)
>>> + return c;
>>> + }
>>> +
>>> + return shared_ptr<Camera>();
>>> + }, py::keep_alive<0, 1>())
>>> +
>>> + /* Create a list of Cameras, where each camera has a keep-alive to CameraManager */
>>> + .def_property_readonly("cameras", [](CameraManager &self) {
>>> + py::list l;
>>> +
>>> + for (auto &c : self.cameras()) {
>>> + py::object py_cm = py::cast(self);
>>> + py::object py_cam = py::cast(c);
>>> + py::detail::keep_alive_impl(py_cam, py_cm);
>>> + l.append(py_cam);
>>> + }
>>> +
>>> + return l;
>>> + });
>>> +
>>> + pyCamera
>>> + .def_property_readonly("id", &Camera::id)
>>> + .def("acquire", &Camera::acquire)
>>> + .def("release", &Camera::release)
>>> + .def("start", [](Camera &self) {
>>> + self.requestCompleted.connect(handleRequestCompleted);
>>> +
>>> + int ret = self.start();
>>> + if (ret)
>>> + self.requestCompleted.disconnect(handleRequestCompleted);
>>> +
>>> + return ret;
>>> + })
>>> +
>>> + .def("stop", [](Camera &self) {
>>> + int ret = self.stop();
>>> + if (!ret)
>>> + self.requestCompleted.disconnect(handleRequestCompleted);
>>> +
>>> + return ret;
>>> + })
>>> +
>>> + .def("__repr__", [](Camera &self) {
>>> + return "<libcamera.Camera '" + self.id() + "'>";
>>> + })
>>> +
>>> + /* Keep the camera alive, as StreamConfiguration contains a Stream* */
>>> + .def("generateConfiguration", &Camera::generateConfiguration, py::keep_alive<0, 1>())
>>> + .def("configure", &Camera::configure)
>>> +
>>> + .def("createRequest", &Camera::createRequest, py::arg("cookie") = 0)
>>> +
>>> + .def("queueRequest", [](Camera &self, Request *req) {
>>> + py::object py_req = py::cast(req);
>>> +
>>> + py_req.inc_ref();
>>> +
>>> + int ret = self.queueRequest(req);
>>> + if (ret)
>>> + py_req.dec_ref();
>>> +
>>> + return ret;
>>> + })
>>> +
>>> + .def_property_readonly("streams", [](Camera &self) {
>>> + py::set set;
>>> + for (auto &s : self.streams()) {
>>> + py::object py_self = py::cast(self);
>>> + py::object py_s = py::cast(s);
>>> + py::detail::keep_alive_impl(py_s, py_self);
>>> + set.add(py_s);
>>> + }
>>> + return set;
>>> + })
>>> +
>>> + .def("find_control", [](Camera &self, const string &name) {
>>> + const auto &controls = self.controls();
>>> +
>>> + auto it = find_if(controls.begin(), controls.end(),
>>> + [&name](const auto &kvp) { return kvp.first->name() == name; });
>>> +
>>> + if (it == controls.end())
>>> + throw runtime_error("Control not found");
>>> +
>>> + return it->first;
>>> + }, py::return_value_policy::reference_internal)
>>> +
>>> + .def_property_readonly("controls", [](Camera &self) {
>>> + py::dict ret;
>>> +
>>> + for (const auto &[id, ci] : self.controls()) {
>>> + ret[id->name().c_str()] = make_tuple<py::object>(ControlValueToPy(ci.min()),
>>> + ControlValueToPy(ci.max()),
>>> + ControlValueToPy(ci.def()));
>>> + }
>>> +
>>> + return ret;
>>> + })
>>> +
>>> + .def_property_readonly("properties", [](Camera &self) {
>>> + py::dict ret;
>>> +
>>> + for (const auto &[key, cv] : self.properties()) {
>>> + const ControlId *id = properties::properties.at(key);
>>> + py::object ob = ControlValueToPy(cv);
>>> +
>>> + ret[id->name().c_str()] = ob;
>>> + }
>>> +
>>> + return ret;
>>> + });
>>> +
>>> + pyCameraConfiguration
>>> + .def("__iter__", [](CameraConfiguration &self) {
>>> + return py::make_iterator<py::return_value_policy::reference_internal>(self);
>>> + }, py::keep_alive<0, 1>())
>>> + .def("__len__", [](CameraConfiguration &self) {
>>> + return self.size();
>>> + })
>>> + .def("validate", &CameraConfiguration::validate)
>>> + .def("at", py::overload_cast<unsigned int>(&CameraConfiguration::at), py::return_value_policy::reference_internal)
>>> + .def_property_readonly("size", &CameraConfiguration::size)
>>> + .def_property_readonly("empty", &CameraConfiguration::empty);
>>> +
>>> + pyStreamConfiguration
>>> + .def("toString", &StreamConfiguration::toString)
>>> + .def_property_readonly("stream", &StreamConfiguration::stream, py::return_value_policy::reference_internal)
>>> + .def_property(
>>> + "size",
>>> + [](StreamConfiguration &self) { return make_tuple(self.size.width, self.size.height); },
>>> + [](StreamConfiguration &self, tuple<uint32_t, uint32_t> size) { self.size.width = get<0>(size); self.size.height = get<1>(size); })
>>> + .def_property(
>>> + "pixelFormat",
>>> + [](StreamConfiguration &self) { return self.pixelFormat.toString(); },
>>> + [](StreamConfiguration &self, string fmt) { self.pixelFormat = PixelFormat::fromString(fmt); })
>>> + .def_readwrite("stride", &StreamConfiguration::stride)
>>> + .def_readwrite("frameSize", &StreamConfiguration::frameSize)
>>> + .def_readwrite("bufferCount", &StreamConfiguration::bufferCount)
>>> + .def_property_readonly("formats", &StreamConfiguration::formats, py::return_value_policy::reference_internal);
>>> + ;
>
> Is this a stray ; ?
>
> How does checkstyle cope with all of this? does clang-format get
> anywhere close?
>
> I expect the syntax and chaining probably breaks a lot of assumptions
> used by clang-format :-(
Right... I have never gotten clang-format to do exactly what I want. But
this ; looks extra. I think I originally used a style where the chain
ends with a ; on its own line like here.
>>> +
>>> + pyStreamFormats
>>> + .def_property_readonly("pixelFormats", [](StreamFormats &self) {
>>> + vector<string> fmts;
>>> + for (auto &fmt : self.pixelformats())
>>> + fmts.push_back(fmt.toString());
>>> + return fmts;
>>> + })
>>> + .def("sizes", [](StreamFormats &self, const string &pixelFormat) {
>>> + auto fmt = PixelFormat::fromString(pixelFormat);
>>> + vector<tuple<uint32_t, uint32_t>> fmts;
>>> + for (const auto &s : self.sizes(fmt))
>>> + fmts.push_back(make_tuple(s.width, s.height));
>>> + return fmts;
>>> + })
>>> + .def("range", [](StreamFormats &self, const string &pixelFormat) {
>>> + auto fmt = PixelFormat::fromString(pixelFormat);
>>> + const auto &range = self.range(fmt);
>>> + return make_tuple(make_tuple(range.hStep, range.vStep),
>>> + make_tuple(range.min.width, range.min.height),
>>> + make_tuple(range.max.width, range.max.height));
>>> + });
>>> +
>>> + pyFrameBufferAllocator
>>> + .def(py::init<shared_ptr<Camera>>(), py::keep_alive<1, 2>())
>>> + .def("allocate", &FrameBufferAllocator::allocate)
>>> + .def_property_readonly("allocated", &FrameBufferAllocator::allocated)
>>> + /* Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator */
>>> + .def("buffers", [](FrameBufferAllocator &self, Stream *stream) {
>>> + py::object py_self = py::cast(self);
>>> + py::list l;
>>> + for (auto &ub : self.buffers(stream)) {
>>> + py::object py_buf = py::cast(ub.get(), py::return_value_policy::reference_internal, py_self);
>>> + l.append(py_buf);
>>> + }
>>> + return l;
>>> + });
>>> +
>>> + pyFrameBuffer
>>> + /* TODO: implement FrameBuffer::Plane properly */
>>> + .def(py::init([](vector<tuple<int, unsigned int>> planes, unsigned int cookie) {
>>> + vector<FrameBuffer::Plane> v;
>>> + for (const auto &t : planes)
>>> + v.push_back({ SharedFD(get<0>(t)), FrameBuffer::Plane::kInvalidOffset, get<1>(t) });
>>> + return new FrameBuffer(v, cookie);
>>> + }))
>>> + .def_property_readonly("metadata", &FrameBuffer::metadata, py::return_value_policy::reference_internal)
>>> + .def("length", [](FrameBuffer &self, uint32_t idx) {
>>> + const FrameBuffer::Plane &plane = self.planes()[idx];
>>> + return plane.length;
>>> + })
>>> + .def("fd", [](FrameBuffer &self, uint32_t idx) {
>>> + const FrameBuffer::Plane &plane = self.planes()[idx];
>>> + return plane.fd.get();
>>> + })
>>> + .def_property("cookie", &FrameBuffer::cookie, &FrameBuffer::setCookie);
>>> +
>>> + pyStream
>>> + .def_property_readonly("configuration", &Stream::configuration);
>>> +
>>> + pyControlId
>>> + .def_property_readonly("id", &ControlId::id)
>>> + .def_property_readonly("name", &ControlId::name)
>>> + .def_property_readonly("type", &ControlId::type);
>>> +
>>> + pyRequest
>>> + /* Fence is not supported, so we cannot expose addBuffer() directly */
>>> + .def("addBuffer", [](Request &self, const Stream *stream, FrameBuffer *buffer) {
>>> + return self.addBuffer(stream, buffer);
>>> + }, py::keep_alive<1, 3>()) /* Request keeps Framebuffer alive */
>>> + .def_property_readonly("status", &Request::status)
>>> + .def_property_readonly("buffers", &Request::buffers)
>>> + .def_property_readonly("cookie", &Request::cookie)
>>> + .def_property_readonly("hasPendingBuffers", &Request::hasPendingBuffers)
>>> + .def("set_control", [](Request &self, ControlId& id, py::object value) {
>>> + self.controls().set(id.id(), PyToControlValue(value, id.type()));
>>> + })
>>
>> I see a mixture of camel case ("addBuffer") and snake case
>> ("set_controls"). If there's a preferred coding style for Python
>> should we uniform on it ?
>>
>> Minor comments apart, there's a thing which is missing:
>> support for setting controls that accept a Span<> of values.
>>
>> I tried several solutions and I got the following to compile
>>
>> .def("set_control_array", [](Request &self, ControlId& id, py::tuple &values) {
>> py::bytearray bytes = py::bytearray(values);
>> py::buffer_info info(py::buffer(bytes).request());
>>
>> const int *data = reinterpret_cast<const int *>(info.ptr);
>> size_t length = static_cast<size_t>(info.size);
>>
>> self.controls().set(id.id(), Span<const int>{data, length});
>> })
>>
>> (All types in casts<> should depend on id.type(), but that's for later).
>>
>> Unfortunately, while length is correct, the values I access with
>> 'data' seems invalid, which makes me think
>>
>> py::bytearray bytes = py::bytearray(values);
>>
>> Dosn't do what I think it does, or maybe Tuple in Python are simply
>> not backed by a contigous memory buffer , hence there's no way to wrap
>> their memory in a Span<>).
>>
>> Please note I also tried to instrument:
>>
>> .def("set_control_array", [](Request &self, ControlId& id, vector<int> values)
>>
>> Relying on pybind11 type casting, and it seems to work, but I see two issues:
>>
>> 1) Converting from python to C++ types goes through a copy.
>> Performances reasons apart, the vector lifetime is limited to the
>> function scope. This isn't be a problem for now, as control
>> values are copied in the Request's control list, but that would
>> prevent passing controls values as pointers, if a control transports a
>> large chunk of data (ie gamma tables). Not sure we'll ever want this
>> (I don't think so as it won't play well with serialization between the
>> IPA and the pipeline handler) but you never know.
>
> I don't think we could ever let an application pass a pointer to an
> arbitrary location into libcamera for us to parse.
>
>> 2) my understanding is that python does not support methods signature
>> overloading, hence we would have
>>
>> .def("set_control_array_int", [](Request &self, ControlId& id, vector<int> values)
>> .def("set_control_array_float", [](Request &self, ControlId& id, vector<float> values)
>> ....
>>
>> there are surely smart ways to handle this, but in my quick experiment
>> I haven't found one yet :)
>
> Given how dynamically typed python is I would expect there would be some
> form of wrapping here that could be done?
But the C++ templates are not dynamic, so we have to have all the
possible Span<T> variations here somehow so that we get the code for them.
>> David: Is the issue with Span<> controls addressed by picamera2 ?
>>
>> Thanks, I hope we can merge this soon!
>
> +1 ..
>
>>
>>
>>> + .def_property_readonly("metadata", [](Request &self) {
>>> + py::dict ret;
>>> +
>>> + for (const auto &[key, cv] : self.metadata()) {
>>> + const ControlId *id = controls::controls.at(key);
>>> + py::object ob = ControlValueToPy(cv);
>>> +
>>> + ret[id->name().c_str()] = ob;
>>> + }
>>> +
>>> + return ret;
>>> + })
>>> + /* As we add a keep_alive to the fb in addBuffers(), we can only allow reuse with ReuseBuffers. */
>>> + .def("reuse", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });
>>> +
>>> + pyFrameMetadata
>>> + .def_readonly("status", &FrameMetadata::status)
>>> + .def_readonly("sequence", &FrameMetadata::sequence)
>>> + .def_readonly("timestamp", &FrameMetadata::timestamp)
>>> + /* temporary helper, to be removed */
>>> + .def_property_readonly("bytesused", [](FrameMetadata &self) {
>>> + vector<unsigned int> v;
>>> + v.resize(self.planes().size());
>>> + transform(self.planes().begin(), self.planes().end(), v.begin(), [](const auto &p) { return p.bytesused; });
>>> + return v;
>>> + });
>>> +}
>>> diff --git a/src/py/meson.build b/src/py/meson.build
>>> new file mode 100644
>>> index 00000000..4ce9668c
>>> --- /dev/null
>>> +++ b/src/py/meson.build
>>> @@ -0,0 +1 @@
>>> +subdir('libcamera')
>>> diff --git a/subprojects/.gitignore b/subprojects/.gitignore
>>> index 391fde2c..757bb072 100644
>>> --- a/subprojects/.gitignore
>>> +++ b/subprojects/.gitignore
>>> @@ -1,3 +1,4 @@
>>> /googletest-release*
>>> /libyuv
>>> -/packagecache
>>> \ No newline at end of file
>>> +/packagecache
>>> +/pybind11*/
>>> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap
>>> new file mode 100644
>>> index 00000000..ebf942ff
>>> --- /dev/null
>>> +++ b/subprojects/pybind11.wrap
>>> @@ -0,0 +1,6 @@
>>> +[wrap-git]
>>> +url = https://github.com/tomba/pybind11.git
>>> +revision = smart_holder
>
> Can this point to the upstream branch? or have you made local
> modifications to the branch?
I add meson build support. Possibly that could be done with a suitable
meson wrap file to fetch the upstream smart_holder branch as the main
repo, and get the patch from my repo.
Tomi
More information about the libcamera-devel
mailing list