[{"id":22533,"web_url":"https://patchwork.libcamera.org/comment/22533/","msgid":"<164873252223.15275.6903933217787986958@Monstersaurus>","date":"2022-03-31T13:15:22","subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2022-03-29 12:29:27)\n> Add a timer that gets reset on every buffer dequeue event. If the timeout\n> expires, optionally call a slot in the pipeline handler to handle this\n> condition. This may be useful in detecting and handling stalls in either the\n> hardware or device driver.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> ---\n>  include/libcamera/internal/v4l2_videodevice.h | 10 ++++\n>  src/libcamera/v4l2_videodevice.cpp            | 54 +++++++++++++++++++\n>  2 files changed, 64 insertions(+)\n> \n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index cfeae7bd6c52..2a9ba1fe5c71 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -9,6 +9,7 @@\n>  \n>  #include <array>\n>  #include <atomic>\n> +#include <chrono>\n>  #include <memory>\n>  #include <optional>\n>  #include <stdint.h>\n> @@ -20,6 +21,7 @@\n>  #include <libcamera/base/class.h>\n>  #include <libcamera/base/log.h>\n>  #include <libcamera/base/signal.h>\n> +#include <libcamera/base/timer.h>\n>  #include <libcamera/base/unique_fd.h>\n>  \n>  #include <libcamera/color_space.h>\n> @@ -217,6 +219,9 @@ public:\n>         int streamOn();\n>         int streamOff();\n>  \n> +       void setDequeueTimeout(std::chrono::milliseconds msec);\n> +       Signal<> dequeueTimeout;\n> +\n>         static std::unique_ptr<V4L2VideoDevice>\n>         fromEntityName(const MediaDevice *media, const std::string &entity);\n>  \n> @@ -253,6 +258,8 @@ private:\n>         void bufferAvailable();\n>         FrameBuffer *dequeueBuffer();\n>  \n> +       void watchdogExpired();\n> +\n>         V4L2Capability caps_;\n>         V4L2DeviceFormat format_;\n>         const PixelFormatInfo *formatInfo_;\n> @@ -266,6 +273,9 @@ private:\n>         EventNotifier *fdBufferNotifier_;\n>  \n>         State state_;\n> +\n> +       Timer watchdog_;\n> +       std::chrono::milliseconds watchdogDuration_;\n>  };\n>  \n>  class V4L2M2MDevice\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 009f6d55610f..22191cb9de4d 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -539,6 +539,7 @@ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)\n>  V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)\n>         : V4L2VideoDevice(entity->deviceNode())\n>  {\n> +       watchdog_.timeout.connect(this, &V4L2VideoDevice::watchdogExpired);\n>  }\n>  \n>  V4L2VideoDevice::~V4L2VideoDevice()\n> @@ -1723,6 +1724,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n>                 return nullptr;\n>         }\n>  \n> +       if (watchdogDuration_.count())\n> +               watchdog_.start(watchdogDuration_);\n> +\n>         cache_->put(buf.index);\n>  \n>         FrameBuffer *buffer = it->second;\n> @@ -1825,6 +1829,8 @@ int V4L2VideoDevice::streamOn()\n>         }\n>  \n>         state_ = State::Streaming;\n> +       if (watchdogDuration_.count())\n> +               watchdog_.start(watchdogDuration_);\n>  \n>         return 0;\n>  }\n> @@ -1849,6 +1855,9 @@ int V4L2VideoDevice::streamOff()\n>         if (state_ != State::Streaming && queuedBuffers_.empty())\n>                 return 0;\n>  \n> +       if (watchdogDuration_.count())\n> +               watchdog_.stop();\n> +\n>         ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);\n>         if (ret < 0) {\n>                 LOG(V4L2, Error)\n> @@ -1876,6 +1885,51 @@ int V4L2VideoDevice::streamOff()\n>         return 0;\n>  }\n>  \n> +/**\n> + * \\brief Set the dequeue timeout value\n> + * \\param[in] msec The timeout value to be used\n> + *\n> + * Sets a timeout value, given by \\a msec, that will be used by a watchdog timer\n> + * to ensure buffer dequeue events are periodically occurring when the device is\n> + * streaming. The watchdog timer is only active when the device is streaming, so\n> + * it is not necessary to disable it when the device stops streaming. The timeout\n> + * value can be safely updated at any time.\n> + *\n> + * If the timer expires, the \\ref V4L2VideoDevice::dequeueTimeout signal is\n> + * emitted. This can typically be used by pipeline handlers to be notified of\n> + * stalled devices.\n> + *\n> + * Set \\a msec to 0 to disable the watchdog timer.\n> + */\n> +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds msec)\n> +{\n> +       watchdogDuration_ = msec;\n> +\n> +       watchdog_.stop();\n> +       if (watchdogDuration_.count() && state_ == State::Streaming)\n> +               watchdog_.start(msec);\n> +}\n> +\n> +/**\n> + * \\var V4L2VideoDevice::dequeueTimeout\n> + * \\brief A Signal emitted when the dequeue watchdog timer expires\n> + */\n> +\n> +/**\n> + * \\brief Slot to handle an expired dequeue timer.\n> + *\n> + * When this slot is called, the time between successive dequeue events is over\n> + * the required timeout. Emit the \\ref V4L2VideoDevice::dequeueTimeout signal.\n> + */\n> +void V4L2VideoDevice::watchdogExpired()\n> +{\n> +       LOG(V4L2, Warning)\n> +               << \"Dequeue timer of \" << watchdogDuration_.count()\n> +               << \" ms has expired!\";\n> +\n> +       dequeueTimeout.emit();\n> +}\n> +\n>  /**\n>   * \\brief Create a new video device instance from \\a entity in media device\n>   * \\a media\n> -- \n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BE4E1C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 31 Mar 2022 13:15:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1239C65634;\n\tThu, 31 Mar 2022 15:15:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 82CE065631\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Mar 2022 15:15:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 292408E6;\n\tThu, 31 Mar 2022 15:15:24 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648732526;\n\tbh=yNDnKwXEjXjtZjKbfyjq2u13qWhziffDDmVWNpwXIMU=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=psO+avzjXMPMtiwkGbabZvPKZFTSpEaCuXONMTNS4FVOA5MbrtjfExQCwHOfyvqXo\n\tekZ2G57qYn67JGHZ7oS/bE7GRBfYDhNjgQ9oiV9r/o6zEPCryKHA3zqG/tI9Y6laOJ\n\tHjxVER/geS747pYpWX0l/aYGmy8bF5fAvDeLX/NvjENUvLV1O6j+JbuYUhOv1JP6Z1\n\tg0p2BNTfqQruSTl/1wdPxVTrF65RmAOHkokLP/i/yGUz239I3DubHtiIMA4n0yfvxT\n\t8pCnQWn7fZTr37+XL/QAh/th2AyeaoBnjkwmCxZe+aqphofhSbh4VgMhrioY79mtfL\n\tIcDgYLRUIy09A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648732524;\n\tbh=yNDnKwXEjXjtZjKbfyjq2u13qWhziffDDmVWNpwXIMU=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=CbGu9L56ZFzuCrWr7ZE/i5cC5JbBGlmxLPuZqIfKU8iqxowGauPyTdSiXedp6ZTQw\n\t+AluaOz+f6lCpQfxNp7Mtw5kKaunF+PmifiWYlQe7M4WCEw6/ZRvbJ01qpqY2tEaL3\n\tekMX5GUv6vAyEm6XbgYnBK6uePBklyBEoRe8kBck="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CbGu9L56\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220329112929.465434-1-naush@raspberrypi.com>","References":"<20220329112929.465434-1-naush@raspberrypi.com>","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 31 Mar 2022 14:15:22 +0100","Message-ID":"<164873252223.15275.6903933217787986958@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22565,"web_url":"https://patchwork.libcamera.org/comment/22565/","msgid":"<2aa4c63c-a1df-f70c-7546-0c128d82705a@ideasonboard.com>","date":"2022-04-05T13:56:40","subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Naushir,\n\nJust a drive-by comment,\n\nOn 3/29/22 16:59, Naushir Patuck via libcamera-devel wrote:\n> Add a timer that gets reset on every buffer dequeue event. If the timeout\n> expires, optionally call a slot in the pipeline handler to handle this\n> condition. This may be useful in detecting and handling stalls in either the\n> hardware or device driver.\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>   include/libcamera/internal/v4l2_videodevice.h | 10 ++++\n>   src/libcamera/v4l2_videodevice.cpp            | 54 +++++++++++++++++++\n>   2 files changed, 64 insertions(+)\n>\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index cfeae7bd6c52..2a9ba1fe5c71 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -9,6 +9,7 @@\n>   \n>   #include <array>\n>   #include <atomic>\n> +#include <chrono>\n>   #include <memory>\n>   #include <optional>\n>   #include <stdint.h>\n> @@ -20,6 +21,7 @@\n>   #include <libcamera/base/class.h>\n>   #include <libcamera/base/log.h>\n>   #include <libcamera/base/signal.h>\n> +#include <libcamera/base/timer.h>\n>   #include <libcamera/base/unique_fd.h>\n>   \n>   #include <libcamera/color_space.h>\n> @@ -217,6 +219,9 @@ public:\n>   \tint streamOn();\n>   \tint streamOff();\n>   \n> +\tvoid setDequeueTimeout(std::chrono::milliseconds msec);\n> +\tSignal<> dequeueTimeout;\n> +\n>   \tstatic std::unique_ptr<V4L2VideoDevice>\n>   \tfromEntityName(const MediaDevice *media, const std::string &entity);\n>   \n> @@ -253,6 +258,8 @@ private:\n>   \tvoid bufferAvailable();\n>   \tFrameBuffer *dequeueBuffer();\n>   \n> +\tvoid watchdogExpired();\n> +\n>   \tV4L2Capability caps_;\n>   \tV4L2DeviceFormat format_;\n>   \tconst PixelFormatInfo *formatInfo_;\n> @@ -266,6 +273,9 @@ private:\n>   \tEventNotifier *fdBufferNotifier_;\n>   \n>   \tState state_;\n> +\n> +\tTimer watchdog_;\n> +\tstd::chrono::milliseconds watchdogDuration_;\n\n\nAny particular reason this is std::chrono::miiliseconds instead of using \nutils::Duration ?\n\n>   };\n>   \n>   class V4L2M2MDevice\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 009f6d55610f..22191cb9de4d 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -539,6 +539,7 @@ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)\n>   V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)\n>   \t: V4L2VideoDevice(entity->deviceNode())\n>   {\n> +\twatchdog_.timeout.connect(this, &V4L2VideoDevice::watchdogExpired);\n>   }\n>   \n>   V4L2VideoDevice::~V4L2VideoDevice()\n> @@ -1723,6 +1724,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n>   \t\treturn nullptr;\n>   \t}\n>   \n> +\tif (watchdogDuration_.count())\n> +\t\twatchdog_.start(watchdogDuration_);\n> +\n>   \tcache_->put(buf.index);\n>   \n>   \tFrameBuffer *buffer = it->second;\n> @@ -1825,6 +1829,8 @@ int V4L2VideoDevice::streamOn()\n>   \t}\n>   \n>   \tstate_ = State::Streaming;\n> +\tif (watchdogDuration_.count())\n> +\t\twatchdog_.start(watchdogDuration_);\n>   \n>   \treturn 0;\n>   }\n> @@ -1849,6 +1855,9 @@ int V4L2VideoDevice::streamOff()\n>   \tif (state_ != State::Streaming && queuedBuffers_.empty())\n>   \t\treturn 0;\n>   \n> +\tif (watchdogDuration_.count())\n> +\t\twatchdog_.stop();\n> +\n>   \tret = ioctl(VIDIOC_STREAMOFF, &bufferType_);\n>   \tif (ret < 0) {\n>   \t\tLOG(V4L2, Error)\n> @@ -1876,6 +1885,51 @@ int V4L2VideoDevice::streamOff()\n>   \treturn 0;\n>   }\n>   \n> +/**\n> + * \\brief Set the dequeue timeout value\n> + * \\param[in] msec The timeout value to be used\n> + *\n> + * Sets a timeout value, given by \\a msec, that will be used by a watchdog timer\n> + * to ensure buffer dequeue events are periodically occurring when the device is\n> + * streaming. The watchdog timer is only active when the device is streaming, so\n> + * it is not necessary to disable it when the device stops streaming. The timeout\n> + * value can be safely updated at any time.\n> + *\n> + * If the timer expires, the \\ref V4L2VideoDevice::dequeueTimeout signal is\n> + * emitted. This can typically be used by pipeline handlers to be notified of\n> + * stalled devices.\n> + *\n> + * Set \\a msec to 0 to disable the watchdog timer.\n> + */\n> +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds msec)\n\n\nSame here as well. I guess you take in a `utils::Duration msec` and \nassign to watchdogDuration_ below (given you have utils::Duration \nwatchdogDuration_ too).\n\n> +{\n> +\twatchdogDuration_ = msec;\n> +\n> +\twatchdog_.stop();\n> +\tif (watchdogDuration_.count() && state_ == State::Streaming)\n> +\t\twatchdog_.start(msec);\n> +}\n> +\n> +/**\n> + * \\var V4L2VideoDevice::dequeueTimeout\n> + * \\brief A Signal emitted when the dequeue watchdog timer expires\n> + */\n> +\n> +/**\n> + * \\brief Slot to handle an expired dequeue timer.\n> + *\n> + * When this slot is called, the time between successive dequeue events is over\n> + * the required timeout. Emit the \\ref V4L2VideoDevice::dequeueTimeout signal.\n> + */\n> +void V4L2VideoDevice::watchdogExpired()\n> +{\n> +\tLOG(V4L2, Warning)\n> +\t\t<< \"Dequeue timer of \" << watchdogDuration_.count()\n> +\t\t<< \" ms has expired!\";\n> +\n> +\tdequeueTimeout.emit();\n> +}\n> +\n>   /**\n>    * \\brief Create a new video device instance from \\a entity in media device\n>    * \\a media","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0C015C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Apr 2022 13:56:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8709565642;\n\tTue,  5 Apr 2022 15:56:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 23393604BB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Apr 2022 15:56:45 +0200 (CEST)","from [192.168.1.110] (unknown [27.57.186.178])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 361385D;\n\tTue,  5 Apr 2022 15:56:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649167006;\n\tbh=CDlUyvPNV3PKXKpwwDBn4/3qhkaGaAnQeqwoJGiYH9Y=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=zaUw+KDGp5gPma56Vjc3D9gkQaWDWxVOIj33qhnl+DxKoIxgLKO+tpxbmnIQfAzTU\n\tfiROq5uuguqrXtD8/aSkB2dzu7TDRDcWKLMGlJJMJr5/vtRkBVU01/v7syOiOrVrVG\n\t2/ESUXsXV9GaZvaFCascRphOLeKf2zyxmKe4VggVjJA42W5PKDV/Qh055zjsO8sec4\n\tpK6uwCtL4zULiy2BOLULxpvraqstjDSqes1kqJmAu6fYkXTYJB+hurOyhjUIpjsYno\n\tdgmVwIxBgjf1gzWIrUmLPmef26/3SuV1Snb3ZqKe7NoxyJzKwb78eMDtjEn3sncrG6\n\tYHiv80SU/phrQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649167004;\n\tbh=CDlUyvPNV3PKXKpwwDBn4/3qhkaGaAnQeqwoJGiYH9Y=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=EUs6XP6AgP3UOl2RtOIVKnF03iUQXLigBWkERXo/T0I8qjB6fqC3dXgm/Mh9v74+Q\n\tZ98TmLhG+81F9syGEGGXWpaJyGRsVP2a4DWBSvlrDElAW2V11IxUVBIdxiKY7UmXHv\n\tqkRYSZrfk0Dt/PM7GF5EkPRVYuzz8boKfR9d8L6g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EUs6XP6A\"; dkim-atps=neutral","Message-ID":"<2aa4c63c-a1df-f70c-7546-0c128d82705a@ideasonboard.com>","Date":"Tue, 5 Apr 2022 19:26:40 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220329112929.465434-1-naush@raspberrypi.com>","In-Reply-To":"<20220329112929.465434-1-naush@raspberrypi.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22568,"web_url":"https://patchwork.libcamera.org/comment/22568/","msgid":"<CAEmqJPq-KRjt-fvc+toGSx60AjFTwoPYWrp8nnt7j0dfTT0+Og@mail.gmail.com>","date":"2022-04-05T14:19:29","subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Umang,\n\nOn Tue, 5 Apr 2022 at 14:56, Umang Jain <umang.jain@ideasonboard.com> wrote:\n\n> Hi Naushir,\n>\n> Just a drive-by comment,\n>\n> On 3/29/22 16:59, Naushir Patuck via libcamera-devel wrote:\n> > Add a timer that gets reset on every buffer dequeue event. If the timeout\n> > expires, optionally call a slot in the pipeline handler to handle this\n> > condition. This may be useful in detecting and handling stalls in either\n> the\n> > hardware or device driver.\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >   include/libcamera/internal/v4l2_videodevice.h | 10 ++++\n> >   src/libcamera/v4l2_videodevice.cpp            | 54 +++++++++++++++++++\n> >   2 files changed, 64 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_videodevice.h\n> b/include/libcamera/internal/v4l2_videodevice.h\n> > index cfeae7bd6c52..2a9ba1fe5c71 100644\n> > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > @@ -9,6 +9,7 @@\n> >\n> >   #include <array>\n> >   #include <atomic>\n> > +#include <chrono>\n> >   #include <memory>\n> >   #include <optional>\n> >   #include <stdint.h>\n> > @@ -20,6 +21,7 @@\n> >   #include <libcamera/base/class.h>\n> >   #include <libcamera/base/log.h>\n> >   #include <libcamera/base/signal.h>\n> > +#include <libcamera/base/timer.h>\n> >   #include <libcamera/base/unique_fd.h>\n> >\n> >   #include <libcamera/color_space.h>\n> > @@ -217,6 +219,9 @@ public:\n> >       int streamOn();\n> >       int streamOff();\n> >\n> > +     void setDequeueTimeout(std::chrono::milliseconds msec);\n> > +     Signal<> dequeueTimeout;\n> > +\n> >       static std::unique_ptr<V4L2VideoDevice>\n> >       fromEntityName(const MediaDevice *media, const std::string\n> &entity);\n> >\n> > @@ -253,6 +258,8 @@ private:\n> >       void bufferAvailable();\n> >       FrameBuffer *dequeueBuffer();\n> >\n> > +     void watchdogExpired();\n> > +\n> >       V4L2Capability caps_;\n> >       V4L2DeviceFormat format_;\n> >       const PixelFormatInfo *formatInfo_;\n> > @@ -266,6 +273,9 @@ private:\n> >       EventNotifier *fdBufferNotifier_;\n> >\n> >       State state_;\n> > +\n> > +     Timer watchdog_;\n> > +     std::chrono::milliseconds watchdogDuration_;\n>\n>\n> Any particular reason this is std::chrono::miiliseconds instead of using\n> utils::Duration ?\n>\n\nIn the previous revision, I did use the utils::Duration type for this\nvariable.\nHowever, Laurent suggested that we might want to avoid that as he intends to\nmove utils::Duration to the \"public\" API, and would not be available to use\nin this case.  So I switched to std::chrono::milliseconds.\n\nRegards,\nNaush\n\n\n>\n> >   };\n> >\n> >   class V4L2M2MDevice\n> > diff --git a/src/libcamera/v4l2_videodevice.cpp\n> b/src/libcamera/v4l2_videodevice.cpp\n> > index 009f6d55610f..22191cb9de4d 100644\n> > --- a/src/libcamera/v4l2_videodevice.cpp\n> > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > @@ -539,6 +539,7 @@ V4L2VideoDevice::V4L2VideoDevice(const std::string\n> &deviceNode)\n> >   V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)\n> >       : V4L2VideoDevice(entity->deviceNode())\n> >   {\n> > +     watchdog_.timeout.connect(this, &V4L2VideoDevice::watchdogExpired);\n> >   }\n> >\n> >   V4L2VideoDevice::~V4L2VideoDevice()\n> > @@ -1723,6 +1724,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n> >               return nullptr;\n> >       }\n> >\n> > +     if (watchdogDuration_.count())\n> > +             watchdog_.start(watchdogDuration_);\n> > +\n> >       cache_->put(buf.index);\n> >\n> >       FrameBuffer *buffer = it->second;\n> > @@ -1825,6 +1829,8 @@ int V4L2VideoDevice::streamOn()\n> >       }\n> >\n> >       state_ = State::Streaming;\n> > +     if (watchdogDuration_.count())\n> > +             watchdog_.start(watchdogDuration_);\n> >\n> >       return 0;\n> >   }\n> > @@ -1849,6 +1855,9 @@ int V4L2VideoDevice::streamOff()\n> >       if (state_ != State::Streaming && queuedBuffers_.empty())\n> >               return 0;\n> >\n> > +     if (watchdogDuration_.count())\n> > +             watchdog_.stop();\n> > +\n> >       ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);\n> >       if (ret < 0) {\n> >               LOG(V4L2, Error)\n> > @@ -1876,6 +1885,51 @@ int V4L2VideoDevice::streamOff()\n> >       return 0;\n> >   }\n> >\n> > +/**\n> > + * \\brief Set the dequeue timeout value\n> > + * \\param[in] msec The timeout value to be used\n> > + *\n> > + * Sets a timeout value, given by \\a msec, that will be used by a\n> watchdog timer\n> > + * to ensure buffer dequeue events are periodically occurring when the\n> device is\n> > + * streaming. The watchdog timer is only active when the device is\n> streaming, so\n> > + * it is not necessary to disable it when the device stops streaming.\n> The timeout\n> > + * value can be safely updated at any time.\n> > + *\n> > + * If the timer expires, the \\ref V4L2VideoDevice::dequeueTimeout\n> signal is\n> > + * emitted. This can typically be used by pipeline handlers to be\n> notified of\n> > + * stalled devices.\n> > + *\n> > + * Set \\a msec to 0 to disable the watchdog timer.\n> > + */\n> > +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds msec)\n>\n>\n> Same here as well. I guess you take in a `utils::Duration msec` and\n> assign to watchdogDuration_ below (given you have utils::Duration\n> watchdogDuration_ too).\n>\n> > +{\n> > +     watchdogDuration_ = msec;\n> > +\n> > +     watchdog_.stop();\n> > +     if (watchdogDuration_.count() && state_ == State::Streaming)\n> > +             watchdog_.start(msec);\n> > +}\n> > +\n> > +/**\n> > + * \\var V4L2VideoDevice::dequeueTimeout\n> > + * \\brief A Signal emitted when the dequeue watchdog timer expires\n> > + */\n> > +\n> > +/**\n> > + * \\brief Slot to handle an expired dequeue timer.\n> > + *\n> > + * When this slot is called, the time between successive dequeue events\n> is over\n> > + * the required timeout. Emit the \\ref V4L2VideoDevice::dequeueTimeout\n> signal.\n> > + */\n> > +void V4L2VideoDevice::watchdogExpired()\n> > +{\n> > +     LOG(V4L2, Warning)\n> > +             << \"Dequeue timer of \" << watchdogDuration_.count()\n> > +             << \" ms has expired!\";\n> > +\n> > +     dequeueTimeout.emit();\n> > +}\n> > +\n> >   /**\n> >    * \\brief Create a new video device instance from \\a entity in media\n> device\n> >    * \\a media\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2D2FEC0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Apr 2022 14:19:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E36746563F;\n\tTue,  5 Apr 2022 16:19:49 +0200 (CEST)","from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D764D604BB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Apr 2022 16:19:47 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id q68so5287810ljb.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 05 Apr 2022 07:19:47 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649168389;\n\tbh=5gxfQx6onEveSlR5hjkBVK5NNMqhQWar4TABpjkmGvA=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=syYGCck/mgefFOKOmuGvwX1D0kMqTg83k9dJc5W7lIZwYYr1C8kHHRAjbhDd4rPUx\n\ts1iNbybzTbU69wKpmJfUAhYefDVjcrLhXXX2OvWC/iZnhaj8NA4jUV/sHc13ADFTZQ\n\teZ52xoFaG7EMDYbbURJwD9UdtSrRn+qr//x2ztkfqBpKq6tc3J3X4sMMftfb9bp6RY\n\tNRGorD3hh/8YIIyJyVcBkifgalq01d/HE19K65Vpz+Oe4p1xr1D4vBXE7rUYg8H2+p\n\tmS+tbFL+NvIq2feGNmAljX6EhP6aV3BEWf29qJrKzhSh5QMgN9QdsRY38N4WFHB40l\n\tThTzvNWAdMP+Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ADo+m4kl9+jC/3kDtsSlWc6b+5PsOa43OlI6j6/1Jcs=;\n\tb=ElAzwAPYlM/wD1RGjj2wAEfYCg0ZEU4xPJ0L4rlLdB76MTJV5Ml0RaL712RgOkIiwz\n\teQknhHgzRS619fQnB6jVOeGlqEvB7g1CLYUFTlS60F2PvCQbhr/CKuHkVKTcT0SPBKhT\n\tcESKxNJCIl5EVXyu4upgSGPww9BmXQMGznM1CwvEynA+T04GKBc7+BemiqFWnxmZVf0v\n\t/UrOMIKZNEm6h+urQT37LoxiAlEhHWsEMT3kOmJgk4e/BmzrcZvkc+dZqqAAPMknowms\n\tOAuvOrzI2hOiOQv3+aUOCQ1j08yKghaSiyZSyvJw4Oon0jh48yxnGmFH0/fpjZKOJr/k\n\tr0sw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"ElAzwAPY\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ADo+m4kl9+jC/3kDtsSlWc6b+5PsOa43OlI6j6/1Jcs=;\n\tb=yGegaX6q+d8PK1BIzFw7jwriXVZgVO0Ke84adImb8+35i0T88g+ispdknsTr62Wg0z\n\tXmyPcT1As8wiD6710juKLTGeasxpYnb8cDZQnlzeV09p06aCGU84lumIOzkOnCO/uaLB\n\tt8LHPRpIT/Q2ldTjQouBuIV8wyCqSgf2hkVRDIc5i+cfAhViIGDudYOXSBWK3JpjXhUE\n\tcYPagvo7+Ubo4nzQS2peXwYQWkOZ+mMq5PosDt+aqM/MgGj3JyasOa/+W98rt/tfRtER\n\t96RUaO1MThub04InIwp1QoHwr2GA1QxquukzYU1hQcBNDnsaJfDW6aAWhA87ZFTU40PI\n\tHoaQ==","X-Gm-Message-State":"AOAM5333Xq+jjV78EkSFWZjMNgSidPyW22M9+KQal+Xh1b31lHycVi2z\n\ttDSCftsXdQcFiqe4Der+UEG2nZKH6ZmaXZEpAnVKfbebbzQ=","X-Google-Smtp-Source":"ABdhPJxcY0EucyI/r9XloRDkh9AqJleimIvXkCiysfF6+D31IGT5T94eHzvhISAbXo7vCyfICCGUpY/gFJ5a6T3jJKQ=","X-Received":"by 2002:a2e:a54d:0:b0:249:8dd1:9da1 with SMTP id\n\te13-20020a2ea54d000000b002498dd19da1mr2357531ljn.372.1649168385671;\n\tTue, 05 Apr 2022 07:19:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20220329112929.465434-1-naush@raspberrypi.com>\n\t<2aa4c63c-a1df-f70c-7546-0c128d82705a@ideasonboard.com>","In-Reply-To":"<2aa4c63c-a1df-f70c-7546-0c128d82705a@ideasonboard.com>","Date":"Tue, 5 Apr 2022 15:19:29 +0100","Message-ID":"<CAEmqJPq-KRjt-fvc+toGSx60AjFTwoPYWrp8nnt7j0dfTT0+Og@mail.gmail.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000cd986105dbe8ef00\"","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22571,"web_url":"https://patchwork.libcamera.org/comment/22571/","msgid":"<ed4e7a02-e768-d279-4396-549fbb65d3d3@ideasonboard.com>","date":"2022-04-05T14:24:51","subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi,\n\nOn 4/5/22 19:49, Naushir Patuck wrote:\n> Hi Umang,\n>\n> On Tue, 5 Apr 2022 at 14:56, Umang Jain <umang.jain@ideasonboard.com> wrote:\n>\n>> Hi Naushir,\n>>\n>> Just a drive-by comment,\n>>\n>> On 3/29/22 16:59, Naushir Patuck via libcamera-devel wrote:\n>>> Add a timer that gets reset on every buffer dequeue event. If the timeout\n>>> expires, optionally call a slot in the pipeline handler to handle this\n>>> condition. This may be useful in detecting and handling stalls in either\n>> the\n>>> hardware or device driver.\n>>>\n>>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n>>> ---\n>>>    include/libcamera/internal/v4l2_videodevice.h | 10 ++++\n>>>    src/libcamera/v4l2_videodevice.cpp            | 54 +++++++++++++++++++\n>>>    2 files changed, 64 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/internal/v4l2_videodevice.h\n>> b/include/libcamera/internal/v4l2_videodevice.h\n>>> index cfeae7bd6c52..2a9ba1fe5c71 100644\n>>> --- a/include/libcamera/internal/v4l2_videodevice.h\n>>> +++ b/include/libcamera/internal/v4l2_videodevice.h\n>>> @@ -9,6 +9,7 @@\n>>>\n>>>    #include <array>\n>>>    #include <atomic>\n>>> +#include <chrono>\n>>>    #include <memory>\n>>>    #include <optional>\n>>>    #include <stdint.h>\n>>> @@ -20,6 +21,7 @@\n>>>    #include <libcamera/base/class.h>\n>>>    #include <libcamera/base/log.h>\n>>>    #include <libcamera/base/signal.h>\n>>> +#include <libcamera/base/timer.h>\n>>>    #include <libcamera/base/unique_fd.h>\n>>>\n>>>    #include <libcamera/color_space.h>\n>>> @@ -217,6 +219,9 @@ public:\n>>>        int streamOn();\n>>>        int streamOff();\n>>>\n>>> +     void setDequeueTimeout(std::chrono::milliseconds msec);\n>>> +     Signal<> dequeueTimeout;\n>>> +\n>>>        static std::unique_ptr<V4L2VideoDevice>\n>>>        fromEntityName(const MediaDevice *media, const std::string\n>> &entity);\n>>> @@ -253,6 +258,8 @@ private:\n>>>        void bufferAvailable();\n>>>        FrameBuffer *dequeueBuffer();\n>>>\n>>> +     void watchdogExpired();\n>>> +\n>>>        V4L2Capability caps_;\n>>>        V4L2DeviceFormat format_;\n>>>        const PixelFormatInfo *formatInfo_;\n>>> @@ -266,6 +273,9 @@ private:\n>>>        EventNotifier *fdBufferNotifier_;\n>>>\n>>>        State state_;\n>>> +\n>>> +     Timer watchdog_;\n>>> +     std::chrono::milliseconds watchdogDuration_;\n>>\n>> Any particular reason this is std::chrono::miiliseconds instead of using\n>> utils::Duration ?\n>>\n> In the previous revision, I did use the utils::Duration type for this\n> variable.\n> However, Laurent suggested that we might want to avoid that as he intends to\n> move utils::Duration to the \"public\" API, and would not be available to use\n> in this case.  So I switched to std::chrono::milliseconds.\n\n\nerr, didn't read the previous iterations, sorry for the noise then :S\n\n>\n> Regards,\n> Naush\n>\n>\n>>>    };\n>>>\n>>>    class V4L2M2MDevice\n>>> diff --git a/src/libcamera/v4l2_videodevice.cpp\n>> b/src/libcamera/v4l2_videodevice.cpp\n>>> index 009f6d55610f..22191cb9de4d 100644\n>>> --- a/src/libcamera/v4l2_videodevice.cpp\n>>> +++ b/src/libcamera/v4l2_videodevice.cpp\n>>> @@ -539,6 +539,7 @@ V4L2VideoDevice::V4L2VideoDevice(const std::string\n>> &deviceNode)\n>>>    V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)\n>>>        : V4L2VideoDevice(entity->deviceNode())\n>>>    {\n>>> +     watchdog_.timeout.connect(this, &V4L2VideoDevice::watchdogExpired);\n>>>    }\n>>>\n>>>    V4L2VideoDevice::~V4L2VideoDevice()\n>>> @@ -1723,6 +1724,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n>>>                return nullptr;\n>>>        }\n>>>\n>>> +     if (watchdogDuration_.count())\n>>> +             watchdog_.start(watchdogDuration_);\n>>> +\n>>>        cache_->put(buf.index);\n>>>\n>>>        FrameBuffer *buffer = it->second;\n>>> @@ -1825,6 +1829,8 @@ int V4L2VideoDevice::streamOn()\n>>>        }\n>>>\n>>>        state_ = State::Streaming;\n>>> +     if (watchdogDuration_.count())\n>>> +             watchdog_.start(watchdogDuration_);\n>>>\n>>>        return 0;\n>>>    }\n>>> @@ -1849,6 +1855,9 @@ int V4L2VideoDevice::streamOff()\n>>>        if (state_ != State::Streaming && queuedBuffers_.empty())\n>>>                return 0;\n>>>\n>>> +     if (watchdogDuration_.count())\n>>> +             watchdog_.stop();\n>>> +\n>>>        ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);\n>>>        if (ret < 0) {\n>>>                LOG(V4L2, Error)\n>>> @@ -1876,6 +1885,51 @@ int V4L2VideoDevice::streamOff()\n>>>        return 0;\n>>>    }\n>>>\n>>> +/**\n>>> + * \\brief Set the dequeue timeout value\n>>> + * \\param[in] msec The timeout value to be used\n>>> + *\n>>> + * Sets a timeout value, given by \\a msec, that will be used by a\n>> watchdog timer\n>>> + * to ensure buffer dequeue events are periodically occurring when the\n>> device is\n>>> + * streaming. The watchdog timer is only active when the device is\n>> streaming, so\n>>> + * it is not necessary to disable it when the device stops streaming.\n>> The timeout\n>>> + * value can be safely updated at any time.\n>>> + *\n>>> + * If the timer expires, the \\ref V4L2VideoDevice::dequeueTimeout\n>> signal is\n>>> + * emitted. This can typically be used by pipeline handlers to be\n>> notified of\n>>> + * stalled devices.\n>>> + *\n>>> + * Set \\a msec to 0 to disable the watchdog timer.\n>>> + */\n>>> +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds msec)\n>>\n>> Same here as well. I guess you take in a `utils::Duration msec` and\n>> assign to watchdogDuration_ below (given you have utils::Duration\n>> watchdogDuration_ too).\n>>\n>>> +{\n>>> +     watchdogDuration_ = msec;\n>>> +\n>>> +     watchdog_.stop();\n>>> +     if (watchdogDuration_.count() && state_ == State::Streaming)\n>>> +             watchdog_.start(msec);\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\var V4L2VideoDevice::dequeueTimeout\n>>> + * \\brief A Signal emitted when the dequeue watchdog timer expires\n>>> + */\n>>> +\n>>> +/**\n>>> + * \\brief Slot to handle an expired dequeue timer.\n>>> + *\n>>> + * When this slot is called, the time between successive dequeue events\n>> is over\n>>> + * the required timeout. Emit the \\ref V4L2VideoDevice::dequeueTimeout\n>> signal.\n>>> + */\n>>> +void V4L2VideoDevice::watchdogExpired()\n>>> +{\n>>> +     LOG(V4L2, Warning)\n>>> +             << \"Dequeue timer of \" << watchdogDuration_.count()\n>>> +             << \" ms has expired!\";\n>>> +\n>>> +     dequeueTimeout.emit();\n>>> +}\n>>> +\n>>>    /**\n>>>     * \\brief Create a new video device instance from \\a entity in media\n>> device\n>>>     * \\a media","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3DFF5C3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Apr 2022 14:24:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC25665640;\n\tTue,  5 Apr 2022 16:24:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BB8F604BB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Apr 2022 16:24:56 +0200 (CEST)","from [192.168.1.110] (unknown [27.57.186.178])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BF5925D;\n\tTue,  5 Apr 2022 16:24:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649168697;\n\tbh=tkj50nYoijjIA4DWu4A/Lx7X+DXidZGbtSVjE7IN9z8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=i20V6ADr/7uwNsjzsK4wE3mYFEdw+Es+EkqILNUT0HOAT8k4yyktuSh9QbsPGI/Bn\n\tqmSZwy1u1G4jdOE86d33WulsWfgPuffw2hXTyTMTIuhpM4eCW2qDlpbj+4svIVhSEv\n\tLpkD7mk5qN8j2C8YTdRuGTRTVIEQourWejSwW2m3xLYQeiASj59FgFjYwLb65DQ2o0\n\tOlFxhZvb8n4q+yS7UvlQeF/AqqDkGvHptDwzpayy1bGNZ4aoPiOK9FRPYEzsxTCRUz\n\tR3FkA4kwdsx5dq5MGG26aSu4fkOt2XpMMXDQYUNNbYHa8ajATGwGQ9ZZRmK5RLHfUE\n\tKju4/kVjtaF/g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649168696;\n\tbh=tkj50nYoijjIA4DWu4A/Lx7X+DXidZGbtSVjE7IN9z8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=S0ic45VyZ1GKd5iVV0V0HRTqUqjlVQKDowLlf48dRNWmk2Jd4isBOGiu0nOEV/NIw\n\t61G0m/yZJ9xBlh53XETmNXD+pkbZp/NcsqQWmqWevZHA85tTIT3wuK7dykXveUpJGh\n\t79kLu2tps97Gg8dHuL4dDpzfaUBxO6loDKAFJHK4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"S0ic45Vy\"; dkim-atps=neutral","Message-ID":"<ed4e7a02-e768-d279-4396-549fbb65d3d3@ideasonboard.com>","Date":"Tue, 5 Apr 2022 19:54:51 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Naushir Patuck <naush@raspberrypi.com>","References":"<20220329112929.465434-1-naush@raspberrypi.com>\n\t<2aa4c63c-a1df-f70c-7546-0c128d82705a@ideasonboard.com>\n\t<CAEmqJPq-KRjt-fvc+toGSx60AjFTwoPYWrp8nnt7j0dfTT0+Og@mail.gmail.com>","In-Reply-To":"<CAEmqJPq-KRjt-fvc+toGSx60AjFTwoPYWrp8nnt7j0dfTT0+Og@mail.gmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22573,"web_url":"https://patchwork.libcamera.org/comment/22573/","msgid":"<YkxfOG0Jxdx2JlGK@pendragon.ideasonboard.com>","date":"2022-04-05T15:24:40","subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Tue, Apr 05, 2022 at 03:19:29PM +0100, Naushir Patuck wrote:\n> On Tue, 5 Apr 2022 at 14:56, Umang Jain wrote:\n> > On 3/29/22 16:59, Naushir Patuck via libcamera-devel wrote:\n> > > Add a timer that gets reset on every buffer dequeue event. If the timeout\n> > > expires, optionally call a slot in the pipeline handler to handle this\n> > > condition. This may be useful in detecting and handling stalls in either the\n> > > hardware or device driver.\n> > >\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > ---\n> > >   include/libcamera/internal/v4l2_videodevice.h | 10 ++++\n> > >   src/libcamera/v4l2_videodevice.cpp            | 54 +++++++++++++++++++\n> > >   2 files changed, 64 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> > > index cfeae7bd6c52..2a9ba1fe5c71 100644\n> > > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > > @@ -9,6 +9,7 @@\n> > >\n> > >   #include <array>\n> > >   #include <atomic>\n> > > +#include <chrono>\n> > >   #include <memory>\n> > >   #include <optional>\n> > >   #include <stdint.h>\n> > > @@ -20,6 +21,7 @@\n> > >   #include <libcamera/base/class.h>\n> > >   #include <libcamera/base/log.h>\n> > >   #include <libcamera/base/signal.h>\n> > > +#include <libcamera/base/timer.h>\n> > >   #include <libcamera/base/unique_fd.h>\n> > >\n> > >   #include <libcamera/color_space.h>\n> > > @@ -217,6 +219,9 @@ public:\n> > >       int streamOn();\n> > >       int streamOff();\n> > >\n> > > +     void setDequeueTimeout(std::chrono::milliseconds msec);\n> > > +     Signal<> dequeueTimeout;\n> > > +\n> > >       static std::unique_ptr<V4L2VideoDevice>\n> > >       fromEntityName(const MediaDevice *media, const std::string &entity);\n> > >\n> > > @@ -253,6 +258,8 @@ private:\n> > >       void bufferAvailable();\n> > >       FrameBuffer *dequeueBuffer();\n> > >\n> > > +     void watchdogExpired();\n> > > +\n> > >       V4L2Capability caps_;\n> > >       V4L2DeviceFormat format_;\n> > >       const PixelFormatInfo *formatInfo_;\n> > > @@ -266,6 +273,9 @@ private:\n> > >       EventNotifier *fdBufferNotifier_;\n> > >\n> > >       State state_;\n> > > +\n> > > +     Timer watchdog_;\n> > > +     std::chrono::milliseconds watchdogDuration_;\n> >\n> > Any particular reason this is std::chrono::miiliseconds instead of using\n> > utils::Duration ?\n> \n> In the previous revision, I did use the utils::Duration type for this variable.\n> However, Laurent suggested that we might want to avoid that as he intends to\n> move utils::Duration to the \"public\" API, and would not be available to use\n> in this case.  So I switched to std::chrono::milliseconds.\n\nThat was about usage of utils::Duration in the libcamera-base layer\nitself (in the Timer class if I recall correctly). You can use it in\nhere if desired, types defined in the libcamera API are accessible to\nlibcamera internals.\n\n> > >   };\n> > >\n> > >   class V4L2M2MDevice\n> > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > > index 009f6d55610f..22191cb9de4d 100644\n> > > --- a/src/libcamera/v4l2_videodevice.cpp\n> > > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > > @@ -539,6 +539,7 @@ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)\n> > >   V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)\n> > >       : V4L2VideoDevice(entity->deviceNode())\n> > >   {\n> > > +     watchdog_.timeout.connect(this, &V4L2VideoDevice::watchdogExpired);\n> > >   }\n> > >\n> > >   V4L2VideoDevice::~V4L2VideoDevice()\n> > > @@ -1723,6 +1724,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n> > >               return nullptr;\n> > >       }\n> > >\n> > > +     if (watchdogDuration_.count())\n> > > +             watchdog_.start(watchdogDuration_);\n> > > +\n> > >       cache_->put(buf.index);\n> > >\n> > >       FrameBuffer *buffer = it->second;\n> > > @@ -1825,6 +1829,8 @@ int V4L2VideoDevice::streamOn()\n> > >       }\n> > >\n> > >       state_ = State::Streaming;\n> > > +     if (watchdogDuration_.count())\n> > > +             watchdog_.start(watchdogDuration_);\n> > >\n> > >       return 0;\n> > >   }\n> > > @@ -1849,6 +1855,9 @@ int V4L2VideoDevice::streamOff()\n> > >       if (state_ != State::Streaming && queuedBuffers_.empty())\n> > >               return 0;\n> > >\n> > > +     if (watchdogDuration_.count())\n> > > +             watchdog_.stop();\n> > > +\n> > >       ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);\n> > >       if (ret < 0) {\n> > >               LOG(V4L2, Error)\n> > > @@ -1876,6 +1885,51 @@ int V4L2VideoDevice::streamOff()\n> > >       return 0;\n> > >   }\n> > >\n> > > +/**\n> > > + * \\brief Set the dequeue timeout value\n> > > + * \\param[in] msec The timeout value to be used\n\nI'd call the parameter timeout instead of msec, the units are specified\nby the chrono type.\n\n> > > + *\n> > > + * Sets a timeout value, given by \\a msec, that will be used by a watchdog timer\n> > > + * to ensure buffer dequeue events are periodically occurring when the device is\n> > > + * streaming. The watchdog timer is only active when the device is streaming, so\n> > > + * it is not necessary to disable it when the device stops streaming. The timeout\n> > > + * value can be safely updated at any time.\n> > > + *\n> > > + * If the timer expires, the \\ref V4L2VideoDevice::dequeueTimeout signal is\n> > > + * emitted. This can typically be used by pipeline handlers to be notified of\n> > > + * stalled devices.\n> > > + *\n> > > + * Set \\a msec to 0 to disable the watchdog timer.\n> > > + */\n> > > +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds msec)\n> >\n> > Same here as well. I guess you take in a `utils::Duration msec` and\n> > assign to watchdogDuration_ below (given you have utils::Duration\n> > watchdogDuration_ too).\n> >\n> > > +{\n> > > +     watchdogDuration_ = msec;\n> > > +\n> > > +     watchdog_.stop();\n> > > +     if (watchdogDuration_.count() && state_ == State::Streaming)\n> > > +             watchdog_.start(msec);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\var V4L2VideoDevice::dequeueTimeout\n> > > + * \\brief A Signal emitted when the dequeue watchdog timer expires\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Slot to handle an expired dequeue timer.\n\ns/.$//\n\n> > > + *\n> > > + * When this slot is called, the time between successive dequeue events is over\n> > > + * the required timeout. Emit the \\ref V4L2VideoDevice::dequeueTimeout signal.\n> > > + */\n> > > +void V4L2VideoDevice::watchdogExpired()\n> > > +{\n> > > +     LOG(V4L2, Warning)\n> > > +             << \"Dequeue timer of \" << watchdogDuration_.count()\n> > > +             << \" ms has expired!\";\n\nIf watchdogDuration_ was stored as a utils::Duration, you could write\n\n             << \"Dequeue timer of \" << watchdogDuration_ << \" has expired!\";\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nWould you like to submit a new version with utils::Duration here, or\nkeep std::chrono::milliseconds ?\n\n> > > +\n> > > +     dequeueTimeout.emit();\n> > > +}\n> > > +\n> > >   /**\n> > >    * \\brief Create a new video device instance from \\a entity in media device\n> > >    * \\a media","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B7A73C3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Apr 2022 15:24:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 22B0A65642;\n\tTue,  5 Apr 2022 17:24:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A6E04604BB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Apr 2022 17:24:44 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 278055D;\n\tTue,  5 Apr 2022 17:24:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649172286;\n\tbh=BMjEvHZj2vDfsr+D3u6+EhGvHXPGWH1vDex7FVDIhQM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=PH053mCi8H93l+dY6jZK+fH4iNodtvtmYTUINfZfsvo+80xr5JtKQzUtIdlt1/FIm\n\tWkVyASqpdzqu52KNnWg/9wRHGU3UNNPIULoArAr0y/S00GucblT0lVvqDz6FgdKXP7\n\tVR1xBrFzzCoaRgoyaErutGpKU/ckuE0zxeaRKrbxuV+WIHSCOx5maSosYpxw2bHb0U\n\tWfEbsXJNice1cqVU6a6Z8Ltzqs9HtYakzbSuGzvF6yJe8Hwf0rBkaDMCOlnWT5n60C\n\tqQfF8His/qfwerY3OJxUVkE+LqH/6GCg6+6T8NJpIc46cGUMhAHI4LQaxr9hxrK4M8\n\t7IiSod8AVTXqw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649172284;\n\tbh=BMjEvHZj2vDfsr+D3u6+EhGvHXPGWH1vDex7FVDIhQM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qgW1LKUb9Drx8pCSMIa1XfKvC3/bgSzo7CFW4KMutQx5PlClWvBkotqLTDV4LuRAX\n\ttUZA/xcWPJWDgQntWkgNUFMgq07n0T71LAhrr48IJvk+arV+w7cHjmb/LB5PB90YLl\n\tc+9UMMARhF7n2YIIG+lo78BLL7W8kVgEupIghaS8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qgW1LKUb\"; dkim-atps=neutral","Date":"Tue, 5 Apr 2022 18:24:40 +0300","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YkxfOG0Jxdx2JlGK@pendragon.ideasonboard.com>","References":"<20220329112929.465434-1-naush@raspberrypi.com>\n\t<2aa4c63c-a1df-f70c-7546-0c128d82705a@ideasonboard.com>\n\t<CAEmqJPq-KRjt-fvc+toGSx60AjFTwoPYWrp8nnt7j0dfTT0+Og@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPq-KRjt-fvc+toGSx60AjFTwoPYWrp8nnt7j0dfTT0+Og@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22606,"web_url":"https://patchwork.libcamera.org/comment/22606/","msgid":"<CAEmqJPqUQ_4pw=7Tz2AkUpipjkFrP3FLvQfYREJEyk6dTwnoKw@mail.gmail.com>","date":"2022-04-06T07:03:59","subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your feedback.\n\nOn Tue, 5 Apr 2022 at 16:24, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Tue, Apr 05, 2022 at 03:19:29PM +0100, Naushir Patuck wrote:\n> > On Tue, 5 Apr 2022 at 14:56, Umang Jain wrote:\n> > > On 3/29/22 16:59, Naushir Patuck via libcamera-devel wrote:\n> > > > Add a timer that gets reset on every buffer dequeue event. If the\n> timeout\n> > > > expires, optionally call a slot in the pipeline handler to handle\n> this\n> > > > condition. This may be useful in detecting and handling stalls in\n> either the\n> > > > hardware or device driver.\n> > > >\n> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > ---\n> > > >   include/libcamera/internal/v4l2_videodevice.h | 10 ++++\n> > > >   src/libcamera/v4l2_videodevice.cpp            | 54\n> +++++++++++++++++++\n> > > >   2 files changed, 64 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/internal/v4l2_videodevice.h\n> b/include/libcamera/internal/v4l2_videodevice.h\n> > > > index cfeae7bd6c52..2a9ba1fe5c71 100644\n> > > > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > > > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > > > @@ -9,6 +9,7 @@\n> > > >\n> > > >   #include <array>\n> > > >   #include <atomic>\n> > > > +#include <chrono>\n> > > >   #include <memory>\n> > > >   #include <optional>\n> > > >   #include <stdint.h>\n> > > > @@ -20,6 +21,7 @@\n> > > >   #include <libcamera/base/class.h>\n> > > >   #include <libcamera/base/log.h>\n> > > >   #include <libcamera/base/signal.h>\n> > > > +#include <libcamera/base/timer.h>\n> > > >   #include <libcamera/base/unique_fd.h>\n> > > >\n> > > >   #include <libcamera/color_space.h>\n> > > > @@ -217,6 +219,9 @@ public:\n> > > >       int streamOn();\n> > > >       int streamOff();\n> > > >\n> > > > +     void setDequeueTimeout(std::chrono::milliseconds msec);\n> > > > +     Signal<> dequeueTimeout;\n> > > > +\n> > > >       static std::unique_ptr<V4L2VideoDevice>\n> > > >       fromEntityName(const MediaDevice *media, const std::string\n> &entity);\n> > > >\n> > > > @@ -253,6 +258,8 @@ private:\n> > > >       void bufferAvailable();\n> > > >       FrameBuffer *dequeueBuffer();\n> > > >\n> > > > +     void watchdogExpired();\n> > > > +\n> > > >       V4L2Capability caps_;\n> > > >       V4L2DeviceFormat format_;\n> > > >       const PixelFormatInfo *formatInfo_;\n> > > > @@ -266,6 +273,9 @@ private:\n> > > >       EventNotifier *fdBufferNotifier_;\n> > > >\n> > > >       State state_;\n> > > > +\n> > > > +     Timer watchdog_;\n> > > > +     std::chrono::milliseconds watchdogDuration_;\n> > >\n> > > Any particular reason this is std::chrono::miiliseconds instead of\n> using\n> > > utils::Duration ?\n> >\n> > In the previous revision, I did use the utils::Duration type for this\n> variable.\n> > However, Laurent suggested that we might want to avoid that as he\n> intends to\n> > move utils::Duration to the \"public\" API, and would not be available to\n> use\n> > in this case.  So I switched to std::chrono::milliseconds.\n>\n> That was about usage of utils::Duration in the libcamera-base layer\n> itself (in the Timer class if I recall correctly). You can use it in\n> here if desired, types defined in the libcamera API are accessible to\n> libcamera internals.\n>\n\nAh, sorry that was my misunderstanding.  I'll switch to using\nutils::Duration here then.\n\n\n>\n> > > >   };\n> > > >\n> > > >   class V4L2M2MDevice\n> > > > diff --git a/src/libcamera/v4l2_videodevice.cpp\n> b/src/libcamera/v4l2_videodevice.cpp\n> > > > index 009f6d55610f..22191cb9de4d 100644\n> > > > --- a/src/libcamera/v4l2_videodevice.cpp\n> > > > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > > > @@ -539,6 +539,7 @@ V4L2VideoDevice::V4L2VideoDevice(const\n> std::string &deviceNode)\n> > > >   V4L2VideoDevice::V4L2VideoDevice(const MediaEntity *entity)\n> > > >       : V4L2VideoDevice(entity->deviceNode())\n> > > >   {\n> > > > +     watchdog_.timeout.connect(this,\n> &V4L2VideoDevice::watchdogExpired);\n> > > >   }\n> > > >\n> > > >   V4L2VideoDevice::~V4L2VideoDevice()\n> > > > @@ -1723,6 +1724,9 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n> > > >               return nullptr;\n> > > >       }\n> > > >\n> > > > +     if (watchdogDuration_.count())\n> > > > +             watchdog_.start(watchdogDuration_);\n> > > > +\n> > > >       cache_->put(buf.index);\n> > > >\n> > > >       FrameBuffer *buffer = it->second;\n> > > > @@ -1825,6 +1829,8 @@ int V4L2VideoDevice::streamOn()\n> > > >       }\n> > > >\n> > > >       state_ = State::Streaming;\n> > > > +     if (watchdogDuration_.count())\n> > > > +             watchdog_.start(watchdogDuration_);\n> > > >\n> > > >       return 0;\n> > > >   }\n> > > > @@ -1849,6 +1855,9 @@ int V4L2VideoDevice::streamOff()\n> > > >       if (state_ != State::Streaming && queuedBuffers_.empty())\n> > > >               return 0;\n> > > >\n> > > > +     if (watchdogDuration_.count())\n> > > > +             watchdog_.stop();\n> > > > +\n> > > >       ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);\n> > > >       if (ret < 0) {\n> > > >               LOG(V4L2, Error)\n> > > > @@ -1876,6 +1885,51 @@ int V4L2VideoDevice::streamOff()\n> > > >       return 0;\n> > > >   }\n> > > >\n> > > > +/**\n> > > > + * \\brief Set the dequeue timeout value\n> > > > + * \\param[in] msec The timeout value to be used\n>\n> I'd call the parameter timeout instead of msec, the units are specified\n> by the chrono type.\n>\n> > > > + *\n> > > > + * Sets a timeout value, given by \\a msec, that will be used by a\n> watchdog timer\n> > > > + * to ensure buffer dequeue events are periodically occurring when\n> the device is\n> > > > + * streaming. The watchdog timer is only active when the device is\n> streaming, so\n> > > > + * it is not necessary to disable it when the device stops\n> streaming. The timeout\n> > > > + * value can be safely updated at any time.\n> > > > + *\n> > > > + * If the timer expires, the \\ref V4L2VideoDevice::dequeueTimeout\n> signal is\n> > > > + * emitted. This can typically be used by pipeline handlers to be\n> notified of\n> > > > + * stalled devices.\n> > > > + *\n> > > > + * Set \\a msec to 0 to disable the watchdog timer.\n> > > > + */\n> > > > +void V4L2VideoDevice::setDequeueTimeout(std::chrono::milliseconds\n> msec)\n> > >\n> > > Same here as well. I guess you take in a `utils::Duration msec` and\n> > > assign to watchdogDuration_ below (given you have utils::Duration\n> > > watchdogDuration_ too).\n> > >\n> > > > +{\n> > > > +     watchdogDuration_ = msec;\n> > > > +\n> > > > +     watchdog_.stop();\n> > > > +     if (watchdogDuration_.count() && state_ == State::Streaming)\n> > > > +             watchdog_.start(msec);\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\var V4L2VideoDevice::dequeueTimeout\n> > > > + * \\brief A Signal emitted when the dequeue watchdog timer expires\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\brief Slot to handle an expired dequeue timer.\n>\n> s/.$//\n>\n> > > > + *\n> > > > + * When this slot is called, the time between successive dequeue\n> events is over\n> > > > + * the required timeout. Emit the \\ref\n> V4L2VideoDevice::dequeueTimeout signal.\n> > > > + */\n> > > > +void V4L2VideoDevice::watchdogExpired()\n> > > > +{\n> > > > +     LOG(V4L2, Warning)\n> > > > +             << \"Dequeue timer of \" << watchdogDuration_.count()\n> > > > +             << \" ms has expired!\";\n>\n> If watchdogDuration_ was stored as a utils::Duration, you could write\n>\n>              << \"Dequeue timer of \" << watchdogDuration_ << \" has\n> expired!\";\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n> Would you like to submit a new version with utils::Duration here, or\n> keep std::chrono::milliseconds ?\n>\n\nI'll submit a new version with utils::Duration + the other suggestions!\n\nRegards,\nNaush\n\n\n>\n> > > > +\n> > > > +     dequeueTimeout.emit();\n> > > > +}\n> > > > +\n> > > >   /**\n> > > >    * \\brief Create a new video device instance from \\a entity in\n> media device\n> > > >    * \\a media\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A5F8EC0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 Apr 2022 07:04:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 089BF65642;\n\tWed,  6 Apr 2022 09:04:18 +0200 (CEST)","from mail-lf1-x132.google.com (mail-lf1-x132.google.com\n\t[IPv6:2a00:1450:4864:20::132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0960D604B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 Apr 2022 09:04:16 +0200 (CEST)","by mail-lf1-x132.google.com with SMTP id m3so2412111lfj.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 06 Apr 2022 00:04:15 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649228658;\n\tbh=FoR5u0T/6pZGga3dPv4Sx9M07zlEgR2ohDYTyNaP4k8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=irtz9ig/PKrnnsGb1qiWCiEhT7e0htlZdDCBfd3iA4OxdY4SRIta/Fec3Cb+Egkkj\n\tgGFTpp7GjqXZKBdJ427laFjzxWPdvYJ6yuzN3uK/Ne6c7o0yY+DfEtJLhl3sdxzWTW\n\tT9irGZ0nQPdBSPaNdR+1iSyndWxkA04jbCxJW6OxB6a8P1Y3oti4IegVO/iEZdiDnK\n\tVgUD60fZ2wmMXyv+oeGMp+AQh0kEXNo+DJUYAzDjWX1nlH54mZPeOckdL8g7Qq+Qpe\n\tIhfxLscGJdph73v5PXdsH0XvD8ZoXwUgrT5ZePurI243tYSi4Y14tx649+2lnMtTSX\n\tIfwMqpokKizJA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=mita5ctTizisruC/VjqntwkY7fxg5r4vANAqH1COsSw=;\n\tb=LrvXP4fphcI2BicKWxjXW9VFryJw/tL2B0RCg3hd4Y/hf2mR/wx5YTXs5SJsjF8xCM\n\tQWHjxYNSYVyNQapVyS3NWAS33YFDsGLIBQBAGuwCCR/KOyCBS6mHdaTx+JMWiWjy11lb\n\t7FNNu8kcFxgbkdexI047ktJsoPaImNKlaX0rnwypSOaNyThcqssWLSazq6zxTHq+q2iV\n\tfcCJMNsy7IP86Tq601XX2Side85yN+6oj6SY7ZKS71aB8/j0MCjG+u7uAbK6zjzosd4S\n\tiPIbDWbs6tePg8uW7WEiqYYSMTRoGyKJqP70pVdXlQ3TmNDAgpCt7pLAGvs82LiG/KLo\n\teY3g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"LrvXP4fp\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=mita5ctTizisruC/VjqntwkY7fxg5r4vANAqH1COsSw=;\n\tb=4mRCo5QjH15Xi0PTxm09O5xKt/Kq8nDKJSaLa+BzDOHVWb6PlgR+UI9m8F/X1mdI3j\n\tcl0VXoE6R806lzWuSfPqYHdIjvzOdKeUkk7u5nAEJWn7M04ZZDGTlL/NYsvAT6BF+uk8\n\tyvZtwRKYNe1Kr+1y0141eeBblh1xn4wKJRpfCaNmNVYw/PACr8CugFlEl1l/tILnyzwx\n\ta0YPzMspDNK3J1PJgGWXaRLs8pVhwYH+LgBuXyZMNafXsFjXLVJh7gScdIlaWRwisVFf\n\tMyAm0yRWkVUSejwI1ZIzHTuAfifIjyD421Q1sAgGGzwlEcPNBCW3BVu3YdTyQLWNuTqr\n\t9Eog==","X-Gm-Message-State":"AOAM533WkXBlllDqluJKR3PfYr+b6sS8LxjSaKmMzpQpufQejSRlMIIT\n\tiCLAtE0CF8JYmKSNgBSNFbZ5kXYAC74CMO14h04iVQ==","X-Google-Smtp-Source":"ABdhPJzoJesw/dLwH2FZ3/kxQNoKiaPooCUpt7OP98emv5QfVkzgEpqVrUx076k0DqtZw98tJKxTdHS6OgXY0ML6CtM=","X-Received":"by 2002:a05:6512:25a:b0:44a:3f55:1746 with SMTP id\n\tb26-20020a056512025a00b0044a3f551746mr5026186lfo.122.1649228655377;\n\tWed, 06 Apr 2022 00:04:15 -0700 (PDT)","MIME-Version":"1.0","References":"<20220329112929.465434-1-naush@raspberrypi.com>\n\t<2aa4c63c-a1df-f70c-7546-0c128d82705a@ideasonboard.com>\n\t<CAEmqJPq-KRjt-fvc+toGSx60AjFTwoPYWrp8nnt7j0dfTT0+Og@mail.gmail.com>\n\t<YkxfOG0Jxdx2JlGK@pendragon.ideasonboard.com>","In-Reply-To":"<YkxfOG0Jxdx2JlGK@pendragon.ideasonboard.com>","Date":"Wed, 6 Apr 2022 08:03:59 +0100","Message-ID":"<CAEmqJPqUQ_4pw=7Tz2AkUpipjkFrP3FLvQfYREJEyk6dTwnoKw@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"0000000000002850ea05dbf6f88b\"","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] libcamera: v4l2_videodevice:\n\tAdd a dequeue timer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]