[libcamera-devel] [PATCH v5 1/3] Add Python bindings
Jacopo Mondi
jacopo at jmondi.org
Wed Apr 13 11:53:26 CEST 2022
Hi
On Wed, Apr 13, 2022 at 12:10:45PM +0300, Tomi Valkeinen wrote:
> 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.
> >
[snip]
> > > > +
> > > > + 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()));
> > > > + })
> > >
> > > 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 ?
I have tested with picamera2, and this commit allows the usage of
controls which accepts multiple values
https://github.com/raspberrypi/libcamera/commit/43cbfd5289aedafa40758312e944db13a5afdf14
and it works here.
It goes through a forceful ob.cast<vector>() which goes through a copy
https://pybind11.readthedocs.io/en/stable/advanced/cast/stl.html
I had looked a bit into using PYBIND11_MAKE_OPAQUE() to define a
custom caster, but if the raw memory of a Python Tuple cannot be
accessed as a contiguous block, it won't help much.
Is this something that can be brought in already in your next series
Tomi ?
Thanks
j
> > >
> > > 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