[libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events support

[Tomi Valkeinen]

On 07/06/2023 10:31, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Mon, Jun 05, 2023 at 12:37:26PM +0300, Tomi Valkeinen wrote:
>> On 05/06/2023 12:10, Tomi Valkeinen wrote:
>>> On 05/06/2023 08:10, Laurent Pinchart wrote:
>>>> On Sat, Jun 03, 2023 at 10:56:06AM +0300, Tomi Valkeinen wrote:
>>>>> Convert cam.py to use the new event dispatching. In addition to handling
>>>>> the request-completed event, handle also disconnect, camera-added and
>>>>> camera-removed events (which only do a simple print).
>>>>>
>>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>>>>> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>>> ---
>>>>>    src/py/cam/cam.py | 27 +++++++++++++++++++++------
>>>>>    1 file changed, 21 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
>>>>> index a2a115c1..1e2d1f69 100755
>>>>> --- a/src/py/cam/cam.py
>>>>> +++ b/src/py/cam/cam.py
>>>>> @@ -230,11 +230,19 @@ class CaptureState:
>>>>>        # Called from renderer when there is a libcamera event
>>>>>        def event_handler(self):
>>>>>            try:
>>>>> -            reqs = self.cm.get_ready_requests()
>>>>> -
>>>>> -            for req in reqs:
>>>>> -                ctx = next(ctx for ctx in self.contexts if ctx.idx == req.cookie)
>>>>> -                self.__request_handler(ctx, req)
>>>>> +            for ev in self.cm.get_events():
>>>>> +                type = ev.type
>>>>> +
>>>>> +                if type == libcam.Event.Type.CameraAdded:
>>>>> +                    print(f'Camera {ev.camera} added')
>>>>> +                elif type == libcam.Event.Type.CameraRemoved:
>>>>> +                    print(f'Camera {ev.camera} removed')
>>>>> +                elif type == libcam.Event.Type.Disconnect:
>>>>> +                    print(f'Camera {ev.camera} disconnected')
>>>>> +                elif type == libcam.Event.Type.RequestCompleted:
>>>>> +                    self.__request_handler(ev.camera, ev.request)
>>>>> +                else:
>>>>> +                    raise RuntimeError("Bad event type")
>>>>
>>>> This will cause issues if we later add new event types. Wouldn't it be
>>>> better to ignore unknown event types, or possibly print a (one-time)
>>>> warning message ?
>>>
>>> Right, this error should never happen. Maybe this again shows that we
>>> have some unclear behavior in the bindings. If we make it so that all
>>> the events (including CameraManager's events) must be explicitly
>>> enabled, we can only handle the ones we have enabled, and raise an error
>>> for anything else (or assert). But if we do implicitly enable some
>>> events, we have to ignore any unhandled ones.
> 
> Agreed.
> 
>>> I'm leaning towards the explicit behavior, as these are somewhat low
>>> level bindings.
> 
> Aonther option would be to always emit all events. I think I would
> prefer that, an event subscription mechanism seems a bit over-engineered
> to me, it's additional complexity for unclear gains. Could we skip it
> for now and introduce it later if needed ?

Perhaps that's the best option. It does rub me the wrong way, though, 
doing at least a double amount of allocations (presuming the buffer 
completed event is not used), but in the bigger picture it's perhaps in 
the who-cares category.

  Tomi