[libcamera-devel] Request and Pipeline::Stop() behavior

[Naushir Patuck]

Hi Laurent.

On Thu, 26 Mar 2020 at 15:32, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Naush,
>
> Replying to a bit of an old thread.
>
> On Thu, Feb 27, 2020 at 10:00:58AM +0000, Naushir Patuck wrote:
> > On Wed, 26 Feb 2020 at 15:13, Naushir Patuck <naush at raspberrypi.com> wrote:
> > > On Wed, 26 Feb 2020 at 14:24, Kieran Bingham wrote:
> >
> >  <snip>
> >
> > > Yes, but I thought that mechanism was only to mark the buffers
> > > FrameMetadata::FrameCancelled, and causes Request::complete() to set
> > > the appropriate stats
> > >
> > > void Request::complete()
> > > {
> > >    ASSERT(!hasPendingBuffers());
> > >    status_ = cancelled_ ? RequestCancelled : RequestComplete;
> > > }
> > >
> > > > And once all buffers have completed (with their status as cancelled) the
> > > > Request will (I think) automatically be cancelled too.
> > >
> > > I did not see where this might be happening.  From my brief
> > > investigation, CameraData::queuedRequests_ maintains an internal list
> > > of pending requests.  The only place this list gets cleared is in
> > > PipelineHandler::completeRequest.  So I was under the impression that
> > > my pipeline handler should also maintain a list of Requests queued,
> > > and call PipelineHandler::completeReques() on all those elements
> > > during a Stop().  By the way, I noticed the UVC pipeline handler does
> > > not do this, so I could be completely wrong about this!
> >
> > So after looking at the  UVC pipeline handler, I think I understand
> > better.  In V4L2VideoDevice::streamOff(), we call
> > bufferReady.emit(buffer) - the UVC pipeline handler does an explicit
> > completeBuffer and completeRequest call and the queue are emptied and
> > all is good.
> >
> > The reason I am seeing these problems is, unlike the UVC pipeline
> > handler, when I get a request in (via
> > PipelineHandlerRPi::queueRequestDevice()), I do not queue the stream
> > buffer immediately.  Rather, I was holding onto the request (and any
> > subsequent requests) until I can action them appropriately one-by-one.
> > The reason for this was to synchronise the Request (and associated
> > buffers) with the Controls applied for the Request, and Metadata
> > returned back for the Request.  Since my Request buffers were not
> > queued, I never got the bufferReady.emit(buffer) signal for them.
> >
> > I suppose there is no harm in queueing my buffers as soon as the
> > Request arrives and still maintaining my existing Request list to sync
> > the Controls and Metadata.
>
> If there is no harm in doing this that's a totally fine solution.
> Otherwise, it's also fully valid to manually complete all the requests
> you have stored in your internal queue when PipelineHandler::stop() is
> called.
>

As it turns out, there is some harm in queuing all buffers up-front.
In doing so, we cannot easily drop frames in the pipeline handler (if
for example, sensors produce N rubbish frames when they initially
start streaming) and reuse the buffer for a valid frame to return out
with the request.

Not to worry, doing the manual complete on all requests works well and
is not too much code.


> --
> Regards,
>
> Laurent Pinchart

Regards,
Naush