[libcamera-devel] [PATCH] pipeline: raspberrypi: Simplify RPiCameraData::clearIncompleteRequests()

[Naushir Patuck]

Hi all,

Another gentle nudge.  This needs one more R-b tag to get merged.

Thanks,
Naush

On Thu, 3 Jun 2021 at 13:43, Naushir Patuck <naush at raspberrypi.com> wrote:

> With the addition of FrameBuffer::cancel(), the logic to clear and return
> pending requests can be simplified by not having to queue all the request
> buffers to the device before calling streamOff().
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 46 ++++---------------
>  1 file changed, 8 insertions(+), 38 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index a65b4568256c..887f8d0f7404 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -884,7 +884,9 @@ void PipelineHandlerRPi::stop(Camera *camera)
>         /* Disable SOF event generation. */
>         data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);
>
> -       /* This also stops the streams. */
> +       for (auto const stream : data->streams_)
> +               stream->dev()->streamOff();
> +
>         data->clearIncompleteRequests();
>         data->bayerQueue_ = {};
>         data->embeddedQueue_ = {};
> @@ -1477,49 +1479,17 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer
> *buffer)
>
>  void RPiCameraData::clearIncompleteRequests()
>  {
> -       /*
> -        * Queue up any buffers passed in the request.
> -        * This is needed because streamOff() will then mark the buffers as
> -        * cancelled.
> -        */
> -       for (auto const request : requestQueue_) {
> -               for (auto const stream : streams_) {
> -                       if (!stream->isExternal())
> -                               continue;
> -
> -                       FrameBuffer *buffer = request->findBuffer(stream);
> -                       if (buffer)
> -                               stream->queueBuffer(buffer);
> -               }
> -       }
> -
> -       /* Stop all streams. */
> -       for (auto const stream : streams_)
> -               stream->dev()->streamOff();
> -
>         /*
>          * All outstanding requests (and associated buffers) must be
> returned
> -        * back to the pipeline. The buffers would have been marked as
> -        * cancelled by the call to streamOff() earlier.
> +        * back to the pipeline.
>          */
>         while (!requestQueue_.empty()) {
>                 Request *request = requestQueue_.front();
> -               /*
> -                * A request could be partially complete,
> -                * i.e. we have returned some buffers, but still waiting
> -                * for others or waiting for metadata.
> -                */
> -               for (auto const stream : streams_) {
> -                       if (!stream->isExternal())
> -                               continue;
>
> -                       FrameBuffer *buffer = request->findBuffer(stream);
> -                       /*
> -                        * Has the buffer already been handed back to the
> -                        * request? If not, do so now.
> -                        */
> -                       if (buffer && buffer->request())
> -                               pipe_->completeBuffer(request, buffer);
> +               for (auto &b : request->buffers()) {
> +                       FrameBuffer *buffer = b.second;
> +                       buffer->cancel();
> +                       pipe_->completeBuffer(request, buffer);
>                 }
>
>                 pipe_->completeRequest(request);
> --
> 2.25.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210622/c5c9555d/attachment-0001.htm>