<div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 24 Mar 2022 at 12:53, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Thanks - this looks pretty good to me.<br>
But Doxygen is generating warnings on this:<br>
<br>
[7/7] Generating doxygen with a custom command<br>
/home/kbingham/iob/libcamera/libcamera/include/libcamera/internal/v4l2_videodevice.h:222: warning: Member dequeueTimeout (variable) of class libcamera::V4L2VideoDevice is not documented.<br>
/home/kbingham/iob/libcamera/libcamera/src/libcamera/v4l2_videodevice.cpp:1864: warning: unable to resolve reference to 'V4L2VideoDevice::dequeueTimeout' for \ref command<br></blockquote><div><br></div><div>Sorry, I missed those! Now fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Also it's missing a unit test. Fortunately I was curious enough to see<br>
how it works, so I've just written the test, and I'll post it in reply<br>
to this.<br></blockquote><div><br></div><div>Thanks! I'll add this to the next revision of the patch.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
--<br>
Kieran<br>
<br>
<br>
Quoting Naushir Patuck via libcamera-devel (2022-03-24 09:34:08)<br>
> Add a timer that gets reset on every buffer dequeue event. If the timeout<br>
> expires, optionally call a slot in the pipeline handler to handle this<br>
> condition. This may be useful in detecting and handling stalls in either the<br>
> hardware or device driver.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> include/libcamera/internal/v4l2_videodevice.h | 10 ++++<br>
> src/libcamera/v4l2_videodevice.cpp | 49 +++++++++++++++++++<br>
> 2 files changed, 59 insertions(+)<br>
> <br>
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h<br>
> index 2d2ccc477c91..c34cc6585612 100644<br>
> --- a/include/libcamera/internal/v4l2_videodevice.h<br>
> +++ b/include/libcamera/internal/v4l2_videodevice.h<br>
> @@ -9,6 +9,7 @@<br>
> <br>
> #include <array><br>
> #include <atomic><br>
> +#include <chrono><br>
> #include <memory><br>
> #include <optional><br>
> #include <stdint.h><br>
> @@ -20,6 +21,7 @@<br>
> #include <libcamera/base/class.h><br>
> #include <libcamera/base/log.h><br>
> #include <libcamera/base/signal.h><br>
> +#include <libcamera/base/timer.h><br>
> #include <libcamera/base/unique_fd.h><br>
> <br>
> #include <libcamera/color_space.h><br>
> @@ -216,6 +218,9 @@ public:<br>
> int streamOn();<br>
> int streamOff();<br>
> <br>
> + void setDequeueTimeout(std::chrono::milliseconds msec);<br>
> + Signal<> dequeueTimeout;<br>
> +<br>
> static std::unique_ptr<V4L2VideoDevice><br>
> fromEntityName(const MediaDevice *media, const std::string &entity);<br>
> <br>
> @@ -246,6 +251,8 @@ private:<br>
> void bufferAvailable();<br>
> FrameBuffer *dequeueBuffer();<br>
> <br>
> + void watchdogExpired();<br>
> +<br>
> V4L2Capability caps_;<br>
> V4L2DeviceFormat format_;<br>
> const PixelFormatInfo *formatInfo_;<br>
> @@ -259,6 +266,9 @@ private:<br>
> EventNotifier *fdBufferNotifier_;<br>
> <br>
> bool streaming_;<br>
> +<br>
> + Timer watchdog_;<br>
> + std::chrono::milliseconds watchdogDuration_;<br>
> };<br>
> <br>
> class V4L2M2MDevice<br>
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp<br>
> index 5f36ee20710d..29225f3058b0 100644<br>
> --- a/src/libcamera/v4l2_videodevice.cpp<br>
> +++ b/src/libcamera/v4l2_videodevice.cpp<br>
> @@ -526,6 +526,7 @@ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)<br>
> V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)<br>
> : V4L2VideoDevice(entity->deviceNode())<br>
> {<br>
> + watchdog_.timeout.connect(this, &V4L2VideoDevice::watchdogExpired);<br>
> }<br>
> <br>
> V4L2VideoDevice::~V4L2VideoDevice()<br>
> @@ -1695,6 +1696,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()<br>
> return nullptr;<br>
> }<br>
> <br>
> + if (watchdogDuration_.count())<br>
> + watchdog_.start(watchdogDuration_);<br>
> +<br>
> cache_->put(buf.index);<br>
> <br>
> FrameBuffer *buffer = it->second;<br>
> @@ -1797,6 +1801,8 @@ int V4L2VideoDevice::streamOn()<br>
> }<br>
> <br>
> streaming_ = true;<br>
> + if (watchdogDuration_.count())<br>
> + watchdog_.start(watchdogDuration_);<br>
> <br>
> return 0;<br>
> }<br>
> @@ -1821,6 +1827,9 @@ int V4L2VideoDevice::streamOff()<br>
> if (!streaming_ && queuedBuffers_.empty())<br>
> return 0;<br>
> <br>
> + if (watchdogDuration_.count())<br>
> + watchdog_.stop();<br>
> +<br>
> ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);<br>
> if (ret < 0) {<br>
> LOG(V4L2, Error)<br>
> @@ -1843,6 +1852,46 @@ int V4L2VideoDevice::streamOff()<br>
> return 0;<br>
> }<br>
> <br>
> +/**<br>
> + * \brief Set the dequeue timeout value<br>
> + * \param[in] msec The timeout value to be used<br>
> + *<br>
> + * Sets a timeout value, given by \a msec, that will be used by a watchdog timer<br>
> + * to ensure buffer dequeue events are periodically occurring when the device is<br>
> + * streaming. The watchdog timer is only active when the device is streaming, so<br>
> + * it is not necessary to disable it when the device stops streaming. The timeout<br>
> + * value can be safely updated at any time.<br>
> + *<br>
> + * If the timer expires, the \ref V4L2VideoDevice::dequeueTimeout signal is<br>
> + * emitted. This can typically be used by pipeline handlers to be notified of<br>
> + * stalled devices.<br>
> + *<br>
> + * Set \a msec to 0 to disable the watchdog timer.<br>
> + */<br>
> +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds msec)<br>
> +{<br>
> + watchdogDuration_ = msec;<br>
> +<br>
> + watchdog_.stop();<br>
> + if (watchdogDuration_.count() && streaming_)<br>
> + watchdog_.start(msec);<br>
> +}<br>
> +<br>
> +/**<br>
> + * \brief Slot to handle an expired dequeue timer.<br>
> + *<br>
> + * When this slot is called, the time between successive dequeue events is over<br>
> + * the required timeout. Emit the \ref V4L2VideoDevice::dequeueTimeout signal.<br>
> + */<br>
> +void V4L2VideoDevice::watchdogExpired()<br>
> +{<br>
> + LOG(V4L2, Warning)<br>
> + << "Dequeue timer of " << watchdogDuration_.count()<br>
> + << " ms has expired!";<br>
> +<br>
> + dequeueTimeout.emit();<br>
> +}<br>
> +<br>
> /**<br>
> * \brief Create a new video device instance from \a entity in media device<br>
> * \a media<br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>