[libcamera-devel] [PATCH v5 1/3] Add Python bindings
Tomi Valkeinen
tomi.valkeinen at ideasonboard.com
Wed Apr 13 12:01:39 CEST 2022
On 13/04/2022 12:53, Jacopo Mondi wrote:
> 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 ?
I really don't like having exceptions thrown when everything goes fine.
Unfortunately I haven't had time to look at how to do this properly.
Tomi
More information about the libcamera-devel
mailing list