[libcamera-devel,v2,00/14] Python bindings event handling

16406 mbox series

Hi,

v2 of the event handling series. Plenty of changes to v1:

- (Hopefully) most of the review comments addressed
- PyCameraManager no longer inherits CameraManager. This also fixes the
  camera added/removed events.
- 7 new patches
- API CHANGE: The eventfd is now always non-blocking. This causes an API
  change only if you relied on the blocking read.
- API CHANGE: many of the methods exposed to Python no longer return an
  error code, but raise an exception on error.

The patches up to and including "py: Switch to non-blocking eventfd" can
be considered for merging.

The new event handling still needs more thought, I believe. I'm still
not sure if we should go with the dispatching method implemented now, or
instead returning a list of events which the user needs to process (a
bit more like what we have now with the get_ready_requests). I'm also
not quite sure how to go with the camera.stop(), but there's an attempt
to solve that in this series.

 Tomi

Tomi Valkeinen (14):
  py: cam.py: Fix multi camera capture without -C
  py: Add Python logging category
  py: Move ControlValue helpers to py_helpers.cpp
  py: cam.py: Remove todo comment
  py: Create PyCameraManager
  py: Use UniqueFD
  py: Set EFD_CLOEXEC on eventfd to avoid fd leaking
  py: Use libcamera's Mutex classes
  py: Use exceptions instead of returning error codes
  py: Switch to non-blocking eventfd
  py: New event handling
  py: cam.py: Use new events support
  py: Discard/Dispatch Request events on camera.stop()
  py: Add hotplug-monitor.py

 src/py/cam/cam.py                            |  57 +--
 src/py/examples/hotplug-monitor.py           |  38 ++
 src/py/examples/simple-cam.py                |  20 +-
 src/py/examples/simple-capture.py            |  33 +-
 src/py/examples/simple-continuous-capture.py |  26 +-
 src/py/libcamera/meson.build                 |   2 +
 src/py/libcamera/py_camera_manager.cpp       | 375 +++++++++++++++++++
 src/py/libcamera/py_camera_manager.h         | 100 +++++
 src/py/libcamera/py_helpers.cpp              |  97 +++++
 src/py/libcamera/py_helpers.h                |  13 +
 src/py/libcamera/py_main.cpp                 | 347 ++++++++---------
 src/py/libcamera/py_main.h                   |  14 +
 test/py/unittests.py                         |  83 ++--
 13 files changed, 904 insertions(+), 301 deletions(-)
 create mode 100644 src/py/examples/hotplug-monitor.py
 create mode 100644 src/py/libcamera/py_camera_manager.cpp
 create mode 100644 src/py/libcamera/py_camera_manager.h
 create mode 100644 src/py/libcamera/py_helpers.cpp
 create mode 100644 src/py/libcamera/py_helpers.h
 create mode 100644 src/py/libcamera/py_main.h

On 29/06/2022 10:04, Tomi Valkeinen wrote:
> Hi,
> 
> v2 of the event handling series. Plenty of changes to v1:
> 
> - (Hopefully) most of the review comments addressed
> - PyCameraManager no longer inherits CameraManager. This also fixes the
>    camera added/removed events.
> - 7 new patches
> - API CHANGE: The eventfd is now always non-blocking. This causes an API
>    change only if you relied on the blocking read.
> - API CHANGE: many of the methods exposed to Python no longer return an
>    error code, but raise an exception on error.
> 
> The patches up to and including "py: Switch to non-blocking eventfd" can
> be considered for merging.
> 
> The new event handling still needs more thought, I believe. I'm still
> not sure if we should go with the dispatching method implemented now, or
> instead returning a list of events which the user needs to process (a
> bit more like what we have now with the get_ready_requests). I'm also
> not quite sure how to go with the camera.stop(), but there's an attempt
> to solve that in this series.

One more thing about the camera.stop(). I considered implementing the 
feature on top of the of the current get_ready_request, without the new 
event handling, so that we could get it merged before the new event 
handling. But that has a few issues/questions:

- I'd need to rewrite the request handling somewhat, as currently we 
only store the Request, and we don't know for which camera it is. This 
change wouldn't be a difficult change, but it would be dropped when we 
move to the new event handling.

- A similar question to the camera.stop() with the new event handling: 
what should it do? 1) Discard Request related events for that camera 2) 
Return the Requests for that camera 3) Return all requests (similar to 
get_ready_requests)

- If the camera.stop() would return Requests, moving to the new event 
handling would change the API for camera.stop().

  Tomi