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

[Naushir Patuck]

On Mon, 21 Mar 2022 at 13:34, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> Quoting Naushir Patuck (2022-03-21 10:27:02)
> > 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>
> > Tested-by: David Plowman <david.plowman at raspberrypi.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Ohh I love an assert addition. It catches things. I wonder if these are
> good things or bad things to catch:
>

Hmmm... How can this be possible?
Presumably on streamOff(), all buffers must have been dequeued out of the
device
driver, and the cache has to be empty.  Is that an invalid assumption?

Naush


>
> Processed 31 frames
> Buffer received
> Buffer received
> Buffer received
> Buffer received
> Buffer received
> Buffer received
> Buffer received
> Buffer received
> stderr:
> [339:43:53.887319781] [3693294]  WARN CameraSensorProperties
> camera_sensor_properties.cpp:148 No static properties available for 'Sensor
> A'
> [339:43:53.887388240] [3693294]  WARN CameraSensorProperties
> camera_sensor_properties.cpp:150 Please consider updating the camera sensor
> properties database
> [339:43:53.887416152] [3693294]  WARN CameraSensor camera_sensor.cpp:411
> 'Sensor A': Failed to retrieve the camera location
> [339:43:54.491916393] [3693294] FATAL default v4l2_videodevice.cpp:1854
> /dev/video6[7:cap]: assertion "cache_->isEmpty()" failed in streamOff()
> Backtrace:
> libcamera::V4L2VideoDevice::streamOff()+0x14c2
> (../../src/libcamera/src/libcamera/v4l2_videodevice.cpp:1854)
> CaptureAsyncTest::run()+0x2dbb
> (../../src/libcamera/test/v4l2_videodevice/capture_async.cpp:82)
> Test::execute()+0x472 (../../src/libcamera/test/libtest/test.cpp:33)
> main+0x2d2 (../../src/libcamera/test/v4l2_videodevice/capture_async.cpp:93)
> __libc_start_call_main+0x80 (../sysdeps/nptl/libc_start_call_main.h:58)
> __libc_start_main@@GLIBC_2.34+0x7d (../csu/libc-start.c:128)
> _start+0x25
> (/home/kbingham/iob/libcamera/ci/integrator/builds/unit-tests/test/v4l2_videodevice/capture_async
> [0x000055a86d271fb5])
>
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
>
> 38/67 libcamera:v4l2_videodevice / buffer_sharing                OK
>       6.31s
> 39/67 libcamera:v4l2_videodevice / v4l2_m2mdevice                FAIL
>       1.58s   killed by signal 6 SIGABRT
> >>> MALLOC_PERTURB_=171
> /home/kbingham/iob/libcamera/ci/integrator/builds/unit-tests/test/v4l2_videodevice/v4l2_m2mdevice
> ――――――――――――――――――――――――――――――――――――― ✀
> ―――――――――――――――――――――――――――――――――――――
> stdout:
>
>
>
> Received capture buffer
> Received capture buffer
> Received capture buffer
> Received capture buffer
> stderr:
> Output 31 frames
> Captured 31 frames
> [339:44:02.406962689] [3693463] FATAL default v4l2_videodevice.cpp:1854
> /dev/video9[9:cap]: assertion "cache_->isEmpty()" failed in streamOff()
> Backtrace:
> libcamera::V4L2VideoDevice::streamOff()+0x14c2
> (../../src/libcamera/src/libcamera/v4l2_videodevice.cpp:1854)
> V4L2M2MDeviceTest::run()+0x523d
> (../../src/libcamera/test/v4l2_videodevice/v4l2_m2mdevice.cpp:173)
> Test::execute()+0x472 (../../src/libcamera/test/libtest/test.cpp:33)
> main+0x2bb
> (../../src/libcamera/test/v4l2_videodevice/v4l2_m2mdevice.cpp:205)
> __libc_start_call_main+0x80 (../sysdeps/nptl/libc_start_call_main.h:58)
> __libc_start_main@@GLIBC_2.34+0x7d (../csu/libc-start.c:128)
> _start+0x25
> (/home/kbingham/iob/libcamera/ci/integrator/builds/unit-tests/test/v4l2_videodevice/v4l2_m2mdevice
> [0x000055b106567015])
>
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
>
> --
> Kieran
>
>
>
> > ---
> >  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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220321/ab35a59b/attachment-0001.htm>