[libcamera-devel] [PATCH v2 1/2] libcamera: v4l2_videodevice: Add a dequeue timer

[Kieran Bingham]

Hi Naush,

Thanks - this looks pretty good to me.
But Doxygen is generating warnings on this:

[7/7] Generating doxygen with a custom command
/home/kbingham/iob/libcamera/libcamera/include/libcamera/internal/v4l2_videodevice.h:222: warning: Member dequeueTimeout (variable) of class libcamera::V4L2VideoDevice is not documented.
/home/kbingham/iob/libcamera/libcamera/src/libcamera/v4l2_videodevice.cpp:1864: warning: unable to resolve reference to 'V4L2VideoDevice::dequeueTimeout' for \ref command

Also it's missing a unit test. Fortunately I was curious enough to see
how it works, so I've just written the test, and I'll post it in reply
to this.

--
Kieran


Quoting Naushir Patuck via libcamera-devel (2022-03-24 09:34:08)
> Add a timer that gets reset on every buffer dequeue event. If the timeout
> expires, optionally call a slot in the pipeline handler to handle this
> condition. This may be useful in detecting and handling stalls in either the
> hardware or device driver.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/internal/v4l2_videodevice.h | 10 ++++
>  src/libcamera/v4l2_videodevice.cpp            | 49 +++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 2d2ccc477c91..c34cc6585612 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -9,6 +9,7 @@
>  
>  #include <array>
>  #include <atomic>
> +#include <chrono>
>  #include <memory>
>  #include <optional>
>  #include <stdint.h>
> @@ -20,6 +21,7 @@
>  #include <libcamera/base/class.h>
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/signal.h>
> +#include <libcamera/base/timer.h>
>  #include <libcamera/base/unique_fd.h>
>  
>  #include <libcamera/color_space.h>
> @@ -216,6 +218,9 @@ public:
>         int streamOn();
>         int streamOff();
>  
> +       void setDequeueTimeout(std::chrono::milliseconds msec);
> +       Signal<> dequeueTimeout;
> +
>         static std::unique_ptr<V4L2VideoDevice>
>         fromEntityName(const MediaDevice *media, const std::string &entity);
>  
> @@ -246,6 +251,8 @@ private:
>         void bufferAvailable();
>         FrameBuffer *dequeueBuffer();
>  
> +       void watchdogExpired();
> +
>         V4L2Capability caps_;
>         V4L2DeviceFormat format_;
>         const PixelFormatInfo *formatInfo_;
> @@ -259,6 +266,9 @@ private:
>         EventNotifier *fdBufferNotifier_;
>  
>         bool streaming_;
> +
> +       Timer watchdog_;
> +       std::chrono::milliseconds watchdogDuration_;
>  };
>  
>  class V4L2M2MDevice
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 5f36ee20710d..29225f3058b0 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -526,6 +526,7 @@ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)
>  V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)
>         : V4L2VideoDevice(entity->deviceNode())
>  {
> +       watchdog_.timeout.connect(this, &V4L2VideoDevice::watchdogExpired);
>  }
>  
>  V4L2VideoDevice::~V4L2VideoDevice()
> @@ -1695,6 +1696,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
>                 return nullptr;
>         }
>  
> +       if (watchdogDuration_.count())
> +               watchdog_.start(watchdogDuration_);
> +
>         cache_->put(buf.index);
>  
>         FrameBuffer *buffer = it->second;
> @@ -1797,6 +1801,8 @@ int V4L2VideoDevice::streamOn()
>         }
>  
>         streaming_ = true;
> +       if (watchdogDuration_.count())
> +               watchdog_.start(watchdogDuration_);
>  
>         return 0;
>  }
> @@ -1821,6 +1827,9 @@ int V4L2VideoDevice::streamOff()
>         if (!streaming_ && queuedBuffers_.empty())
>                 return 0;
>  
> +       if (watchdogDuration_.count())
> +               watchdog_.stop();
> +
>         ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
>         if (ret < 0) {
>                 LOG(V4L2, Error)
> @@ -1843,6 +1852,46 @@ int V4L2VideoDevice::streamOff()
>         return 0;
>  }
>  
> +/**
> + * \brief Set the dequeue timeout value
> + * \param[in] msec The timeout value to be used
> + *
> + * Sets a timeout value, given by \a msec, that will be used by a watchdog timer
> + * to ensure buffer dequeue events are periodically occurring when the device is
> + * streaming. The watchdog timer is only active when the device is streaming, so
> + * it is not necessary to disable it when the device stops streaming. The timeout
> + * value can be safely updated at any time.
> + *
> + * If the timer expires, the \ref V4L2VideoDevice::dequeueTimeout signal is
> + * emitted. This can typically be used by pipeline handlers to be notified of
> + * stalled devices.
> + *
> + * Set \a msec to 0 to disable the watchdog timer.
> + */
> +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds msec)
> +{
> +       watchdogDuration_ = msec;
> +
> +       watchdog_.stop();
> +       if (watchdogDuration_.count() && streaming_)
> +               watchdog_.start(msec);
> +}
> +
> +/**
> + * \brief Slot to handle an expired dequeue timer.
> + *
> + * When this slot is called, the time between successive dequeue events is over
> + * the required timeout. Emit the \ref V4L2VideoDevice::dequeueTimeout signal.
> + */
> +void V4L2VideoDevice::watchdogExpired()
> +{
> +       LOG(V4L2, Warning)
> +               << "Dequeue timer of " << watchdogDuration_.count()
> +               << " ms has expired!";
> +
> +       dequeueTimeout.emit();
> +}
> +
>  /**
>   * \brief Create a new video device instance from \a entity in media device
>   * \a media
> -- 
> 2.25.1
>