[libcamera-devel] [PATCH v1 6/6] libcamera: v4l2_videodevice: Empty the V4L2 buffer cache on streamOff()

[David Plowman]

Hi Naush

Thanks for this patch!

On Mon, 7 Mar 2022 at 12:46, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> When streamOff() is called, ensure the cache entires for the remaining queued
> buffers are freed since this will not happen via the dequeueBuffer() mechanism.
>
> Additionally, add a V4L2BufferCache::isEmpty() function and assert that the
> cache is empty at the end of the streamOff() call.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/internal/v4l2_videodevice.h |  1 +
>  src/libcamera/v4l2_videodevice.cpp            | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 2d2ccc477c91..37747c0b2f27 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -126,6 +126,7 @@ public:
>
>         int get(const FrameBuffer &buffer);
>         void put(unsigned int index);
> +       bool isEmpty() const;
>
>  private:
>         class Entry
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 5f36ee20710d..9da82697e7f0 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -262,6 +262,19 @@ void V4L2BufferCache::put(unsigned int index)
>         cache_[index].free_ = true;
>  }
>
> +/**
> + * \brief Check if all the entries in the cache are unused
> + */
> +bool V4L2BufferCache::isEmpty() const
> +{
> +       for (auto const &entry : cache_) {
> +               if (!entry.free_)
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
>  V4L2BufferCache::Entry::Entry()
>         : free_(true), lastUsed_(0)
>  {
> @@ -1832,10 +1845,13 @@ int V4L2VideoDevice::streamOff()
>         for (auto it : queuedBuffers_) {
>                 FrameBuffer *buffer = it.second;
>
> +               cache_->put(it.first);
>                 buffer->metadata_.status = FrameMetadata::FrameCancelled;
>                 bufferReady.emit(buffer);
>         }
>
> +       ASSERT(cache_->isEmpty());
> +
>         queuedBuffers_.clear();
>         fdBufferNotifier_->setEnabled(false);
>         streaming_ = false;
> --
> 2.25.1
>

Not sure I'm entirely qualified to give a very strong opinion on the
correctness of this, but nonetheless we've been using it for some time
so:

Tested-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David