[libcamera-devel] [RFC v1 5/7] py: Add 'nonblocking' argument to get_ready_requests()

[Laurent Pinchart]

On Mon, Jun 27, 2022 at 12:37:10PM +0300, Tomi Valkeinen wrote:
> On 24/06/2022 13:13, Kieran Bingham wrote:
> > Quoting Tomi Valkeinen (2022-06-23 15:47:34)
> >> Add 'nonblocking' argument to get_ready_requests() which allows the user
> >> to ensure get_ready_requests() never blocks. This can be used e.g. after
> >> calling camera.stop(), to process or discard any ready or cancelled
> >> Requests.
> > 
> > I guess I needed to read ahead for the comments I posted on the previous
> > patch ;-)
> > 
> >>
> >> In fact, it probably should always be used after stopping the cameras,
> >> unless you have made sure that there are no unprocessed Requests. If you
> >> start the camera again, and you have left Requests unprocessed, you will
> >> get those "old" Requests when you expect to get the new Requests.
> >>
> >> It may be good to call this even if your script exits after stopping
> >> the cameras, as unprocessed Requests will keep the Cameras and related
> >> objects alive, and thus they won't be freed. As your script is exiting
> >> it's strictly speaking not an issue, but it does make tracking other
> >> "real" memory leaks more difficult.
> >>
> >> Perhaps the camera.start() should go and discard any old Requests
> >> related to that camera. For the exit issue I don't see any automatic
> >> solution.
> > 
> > At the end of camera.stop() we should validate and ensure that all
> > Requests are released to the application. We should hold no internal
> > requests when camera.stop() completes.
> > 
> > I.e. ... if we have things to discard at camera.start() - that's a bug I
> > believe.
> > 
> > ahhh but here perhaps the issue is that the python code is the one that
> > has to 'retrieve' those, while in C++ they are returned via a signal?
> 
> Right. All events are queued by the bindings when they happen, and the 
> Python code can then separately handle the events in the Python thread.
> 
> > Is there any harm in discarding the requests at the end of camera.stop()
> > such that there is simply 'nothing left to process' after? Or does the
> > python application have to end up owning the requests to correctly
> > release them?
> 
> Discarding automatically at camera.stop() sounds bad to me. There could 
> be normally completed requests in the list, not only cancelled requests. 
> And you could well design an app so that the app expects each queued 
> request to be "given back" via event handling.

If requests are in flight when stop() is called, an application has no
way to know if any particular request will complete successfully or not,
so it shouldn't rely on that. Dropping all successful completion events
would then just hardcode one specific end result, which applications
must be ready to handle.

I however agree that an application can be designed to assume each
queued request will complete. The libcamera C++ API guarantees that, and
I think it makes sense to do the same in the Python bindings.

> We could make the camera.stop() call the event handlers, so that it 
> would automatically dispatch the events. But that doesn't sound very 
> good either, although perhaps better than discarding the events 
> automatically.

I like this option :-)

-- 
Regards,

Laurent Pinchart