[libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when clearing out Request buffers on stop
Naushir Patuck
naush at raspberrypi.com
Wed Jul 28 09:00:00 CEST 2021
Hi Laurent,
Thanks for the review.
On Wed, 28 Jul 2021 at 07:54, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> Hi Naush,
>
> Thank you for the patch.
>
> On Wed, Jul 21, 2021 at 10:28:00AM +0100, Naushir Patuck wrote:
> > When RPiCameraData::clearIncompleteRequests() clears out the request
> queue
> > during a stop condition, it unconditionally calls completeBuffer() on all
> > buffers in each request. This is wrong, as a buffer could have already
> been
> > completed as part of the current request, but the request itself may not
> yet
> > have completed.
> >
> > Fix this by checking if the buffers in the request have been completed
> before
> > cancelling them.
>
> This probably calls for improving the PipelineHandler, FrameBuffer and
> Request classes to simplify completion handling. For the time being,
> this patch looks good.
>
Agree. I think perhaps a Request::Cancel() that pipeline handlers could
call
might be a neat solution. I'd imagine the implementation would look pretty
similar to the code in this patch.
Regards,
Naush
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > Fixes: d372aaa10ddb ("pipeline: raspberrypi: Simplify
> RPiCameraData::clearIncompleteRequests()")
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index f821d8fe1b6c..0bab3bedd402 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1520,8 +1520,14 @@ void RPiCameraData::clearIncompleteRequests()
> >
> > for (auto &b : request->buffers()) {
> > FrameBuffer *buffer = b.second;
> > - buffer->cancel();
> > - pipe_->completeBuffer(request, buffer);
> > + /*
> > + * Has the buffer already been handed back to the
> > + * request? If not, do so now.
> > + */
> > + if (buffer->request()) {
> > + buffer->cancel();
> > + pipe_->completeBuffer(request, buffer);
> > + }
> > }
> >
> > pipe_->completeRequest(request);
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210728/e72732f6/attachment.htm>
More information about the libcamera-devel
mailing list