[libcamera-devel] [PATCH v4 4/6] libcamera: pipeline: ipu3: Cancel unused buffers

[Naushir Patuck]

Hi Kieran,


On Tue, 20 Apr 2021 at 14:07, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> When the CIO2 returns a cancelled buffer, we will not queue buffers
> to the IMGU.
>
> These buffers should be explicitly marked as cancelled to ensure
> the application knows there is no valid metadata or frame data
> provided in the buffer.
>
> Provide a cancel() method on the FrameBuffer to allow explicitly
> cancelling a buffer.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/buffer.h           | 2 ++
>  src/libcamera/buffer.cpp             | 8 ++++++++
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 7 +++++--
>  3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 620f8a66f6a2..e0af00900409 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -53,6 +53,8 @@ public:
>         unsigned int cookie() const { return cookie_; }
>         void setCookie(unsigned int cookie) { cookie_ = cookie; }
>
> +       void cancel() { metadata_.status = FrameMetadata::FrameCancelled; }
> +
>  private:
>         LIBCAMERA_DISABLE_COPY_AND_MOVE(FrameBuffer)
>
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index 75b2693281a7..7635226b9045 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -226,6 +226,14 @@ FrameBuffer::FrameBuffer(const std::vector<Plane>
> &planes, unsigned int cookie)
>   * core never modifies the buffer cookie.
>   */
>
> +/**
> + * \fn FrameBuffer::cancel()
> + * \brief Marks the buffer as cancelled
> + *
> + * If a buffer is not used by a request, it shall be marked as cancelled
> to
> + * indicate that the metadata is invalid.
> + */
> +
>  /**
>   * \class MappedBuffer
>   * \brief Provide an interface to support managing memory mapped buffers
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 51446fcf5bc1..73306cea6b37 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1257,8 +1257,11 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer
> *buffer)
>
>         /* If the buffer is cancelled force a complete of the whole
> request. */
>         if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> -               for (auto it : request->buffers())
> -                       pipe_->completeBuffer(request, it.second);
> +               for (auto it : request->buffers()) {
> +                       FrameBuffer *b = it.second;
> +                       b->cancel();
> +                       pipe_->completeBuffer(request, b);
> +               }
>

Would you consider adding a Request::cancel() method to the API that
pipeline
handlers can invoke to essentially perform the above logic?  I found it a
bit
confusing having to call completeBuffer on a cancelled buffer, and
completeRequest
to complete cancelling the request if you see what I mean?  This is
probably a bit
more relevant to the Raspberry Pi pipeline handler where we queue up
requests
if required.

Regards,
Naush


>                 frameInfos_.remove(info);
>                 pipe_->completeRequest(request);
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210420/ed3b250d/attachment.htm>