[libcamera-devel] [PATCH v7 04/13] Add Python bindings

[Tomi Valkeinen]

On 05/05/2022 16:51, Kieran Bingham wrote:

>> +       pyCamera
>> +               .def_property_readonly("id", &Camera::id)
>> +               .def("acquire", &Camera::acquire)
>> +               .def("release", &Camera::release)
>> +               .def("start", [](Camera &self) {
>> +                       self.requestCompleted.connect(handleRequestCompleted);
> 
> What happens if someone calls start() multiple times? Is that going to
> mean the signal / slot will be connected multiple times?
> 
> That could be problematic ... as I think it means it could then call the
> request completed handler multiple times - but it may also already be
> trapped by the recent work we did around there.
> 
> So for now I think this is fine - but may be something to test or
> consider later.

Good point. I'll look at it.

>> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap
>> new file mode 100644
>> index 00000000..2413e9ca
>> --- /dev/null
>> +++ b/subprojects/pybind11.wrap
>> @@ -0,0 +1,8 @@
>> +[wrap-git]
>> +url = https://github.com/pybind/pybind11.git
>> +revision = 82734801f23314b4c34d70a79509e060a2648e04
> 
> Great - this is now pointing directly to the pybind sources so I think
> that's much cleaner:
> 
> It's hard to do a dedicated review on so much that I honestly don't
> fully comprehend yet - but I think getting this in and working/building
> on top is better for everyone, and as far as I know - it's already quite
> well tested by RPi (albeit, an older version, so I hope David can rebase
> and test sometime soon).

Yes, I think David rebasing their work and testing it is an important 
step before we can consider merging this.

Hint, hint, David! ;)

Note that the API has changed a bit. The fb mmap is different, and the 
ColorSpace enums are nested. I think those are the only API changes.

Another thing, Python bindings are currently enabled by default if the 
dependencies are there. I was wondering if the bindings should be 
disabled by default until we're more confident that they build fine.

  Tomi