[{"id":22339,"web_url":"https://patchwork.libcamera.org/comment/22339/","msgid":"<164786843265.1484799.450675013157466896@Monstersaurus>","date":"2022-03-21T13:13:52","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Add a sensor\n\tdequeue timeout","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nQuoting Naushir Patuck via libcamera-devel (2022-03-21 12:48:28)\n> Add a timer to the pipeline handler that get set on start(), and subsequently\n> reset on each frame dequeue from the sensor. If the timeout expires, log an\n> error message indicating so. This is useful for debugging sensor failures were\n> the device just stops streaming frames to Unicam, or the pipeline handler has\n> failed to queue buffers to the Unicam device driver.\n> \n> The timeout is calculated as 2x the maximum frame length possible for a given\n> mode.\n> \n\nThis is an interesting idea, and I think will prove useful for\nidentifying stalls in streams.\n\nWould it make sense to make this a feature of the V4L2VideoDevice? Then\nit would (automatically?) cover ISP hangs as well as sensor hangs...\n\nPerhaps the hard part is in making sure the correct timeout is\ndetermined or set?\n\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n>  src/ipa/raspberrypi/raspberrypi.cpp           |  7 ++++++\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 +++++++++++++++++++\n>  3 files changed, 30 insertions(+)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index acd3cafe6c91..33d2a97ca916 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -41,6 +41,7 @@ struct IPAConfig {\n>  struct StartConfig {\n>         libcamera.ControlList controls;\n>         int32 dropFrameCount;\n> +       uint32 sensorTimeoutMs;\n>  };\n>  \n>  interface IPARPiInterface {\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index fd8fecb07f81..983d6e998b4c 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -281,6 +281,13 @@ void IPARPi::start(const ControlList &controls, ipa::RPi::StartConfig *startConf\n>  \n>         startConfig->dropFrameCount = dropFrameCount_;\n>  \n> +       /*\n> +        * Set the pipeline handler's sensor timeout to 2x the maximum possible\n> +        * frame duration for this mode.\n> +        */\n> +       const Duration maxSensorFrameDuration = mode_.max_frame_length * mode_.line_length;\n> +       startConfig->sensorTimeoutMs = 2 * maxSensorFrameDuration.get<std::milli>();\n> +\n>         firstStart_ = false;\n>         lastRunTimestamp_ = 0;\n>  }\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index c2230199fed7..86d952b52aed 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -15,6 +15,7 @@\n>  #include <utility>\n>  \n>  #include <libcamera/base/shared_fd.h>\n> +#include <libcamera/base/timer.h>\n>  #include <libcamera/base/utils.h>\n>  \n>  #include <libcamera/camera.h>\n> @@ -202,6 +203,7 @@ public:\n>         void setIspControls(const ControlList &controls);\n>         void setDelayedControls(const ControlList &controls);\n>         void setSensorControls(ControlList &controls);\n> +       void sensorTimeout();\n>  \n>         /* bufferComplete signal handlers. */\n>         void unicamBufferDequeue(FrameBuffer *buffer);\n> @@ -279,6 +281,10 @@ public:\n>          */\n>         std::optional<int32_t> notifyGainsUnity_;\n>  \n> +       /* Timer to ensure the sensor is producing frames for the pipeline handler. */\n> +       Timer sensorTimeout_;\n> +       std::chrono::milliseconds sensorTimeoutDuration_;\n> +\n>  private:\n>         void checkRequestCompleted();\n>         void fillRequestMetadata(const ControlList &bufferControls,\n> @@ -1032,6 +1038,11 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n>                 }\n>         }\n>  \n> +       LOG(RPI, Debug) << \"Setting sensor timeout to \" << startConfig.sensorTimeoutMs << \" ms\";\n> +       data->sensorTimeoutDuration_ = std::chrono::milliseconds(startConfig.sensorTimeoutMs);\n> +       data->sensorTimeout_.start(data->sensorTimeoutDuration_);\n> +       data->sensorTimeout_.timeout.connect(data, &RPiCameraData::sensorTimeout);\n> +\n>         return 0;\n>  }\n>  \n> @@ -1040,6 +1051,8 @@ void PipelineHandlerRPi::stopDevice(Camera *camera)\n>         RPiCameraData *data = cameraData(camera);\n>  \n>         data->state_ = RPiCameraData::State::Stopped;\n> +       data->sensorTimeout_.timeout.disconnect();\n> +       data->sensorTimeout_.stop();\n>  \n>         /* Disable SOF event generation. */\n>         data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n> @@ -1757,6 +1770,12 @@ void RPiCameraData::setSensorControls(ControlList &controls)\n>         sensor_->setControls(&controls);\n>  }\n>  \n> +void RPiCameraData::sensorTimeout()\n> +{\n> +       LOG(RPI, Error) << \"Sensor has timed out after \"\n> +                       << sensorTimeoutDuration_.count() << \" ms!\";\n> +}\n> +\n>  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n>  {\n>         RPi::Stream *stream = nullptr;\n> @@ -1792,6 +1811,9 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n>                  */\n>                 ctrl.set(controls::SensorTimestamp, buffer->metadata().timestamp);\n>                 bayerQueue_.push({ buffer, std::move(ctrl) });\n> +\n> +               /* Restart the sensor timer. */\n> +               sensorTimeout_.start(sensorTimeoutDuration_);\n>         } else {\n>                 embeddedQueue_.push(buffer);\n>         }\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 C6D9FBD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Mar 2022 13:13:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 87258604D4;\n\tMon, 21 Mar 2022 14:13:56 +0100 (CET)","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 536A8604C6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Mar 2022 14:13:55 +0100 (CET)","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 0DF1D291;\n\tMon, 21 Mar 2022 14:13:55 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647868436;\n\tbh=UhmVfigsO//m1suQM08uvVni3hR+EjLHaEQ7YzOpX+4=;\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=2dMyVz8k3EsVjDVHZTw59G6df49MQfuMrMJLJnWbJZ11M9hmUFftNZY347ulg3ET9\n\ttWzppl0BTFRsgbcomEsU6//91MsdvqZ1RnRLPyqP0/yl3ZrWf9MD6k9mqLaWizTcG4\n\tB/F76WxH84TAvjK3T7MwThd63E8WkTt8N19FnotYx1xE4kB+yPAA5+nAboabCX7jin\n\tTQSns6iDqzXiSF7OWBFs7+7uUptLoet6BF/psxFSzSU9lySTLbSc0WvPOsuIQAf/tz\n\tsaBA6ID5IjCw5plvT+H10VsU1VgJlG6UENoRRncvnL2mtZ+YX5jWScFLfGyEYVwB+0\n\tGgt+4v/l5S1Sg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1647868435;\n\tbh=UhmVfigsO//m1suQM08uvVni3hR+EjLHaEQ7YzOpX+4=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=gQ0/u2/XtAroB2iqikuTqD2qh0Nm6gBfr2hhGrgdIXQLiLPw30kPyftocVqlianSb\n\tHNz5fgXr3JOZM/Qer4v3G/m/AGx0gHWgYBdGYMOxrxjI8vKq3Oe0m23430qHIuhRUf\n\tr7t3sM36UNrFXoMCRzQFJyP955wIvItleefK59Lk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gQ0/u2/X\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220321124828.1845058-1-naush@raspberrypi.com>","References":"<20220321124828.1845058-1-naush@raspberrypi.com>","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 21 Mar 2022 13:13:52 +0000","Message-ID":"<164786843265.1484799.450675013157466896@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Add a sensor\n\tdequeue timeout","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":22340,"web_url":"https://patchwork.libcamera.org/comment/22340/","msgid":"<CAEmqJPpvyhm038jW_rKFyLOfiMK+Yg8YdSwJtjs1d+i1qG+W1A@mail.gmail.com>","date":"2022-03-21T13:22:43","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Add a sensor\n\tdequeue timeout","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nOn Mon, 21 Mar 2022 at 13:13, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Quoting Naushir Patuck via libcamera-devel (2022-03-21 12:48:28)\n> > Add a timer to the pipeline handler that get set on start(), and\n> subsequently\n> > reset on each frame dequeue from the sensor. If the timeout expires, log\n> an\n> > error message indicating so. This is useful for debugging sensor\n> failures were\n> > the device just stops streaming frames to Unicam, or the pipeline\n> handler has\n> > failed to queue buffers to the Unicam device driver.\n> >\n> > The timeout is calculated as 2x the maximum frame length possible for a\n> given\n> > mode.\n> >\n>\n> This is an interesting idea, and I think will prove useful for\n> identifying stalls in streams.\n>\n> Would it make sense to make this a feature of the V4L2VideoDevice? Then\n> it would (automatically?) cover ISP hangs as well as sensor hangs...\n>\n\nIt would be nice to add this to a level above, and allow all pipeline\nhandlers\nto gain this functionality!  I can look to do that if we want to go this\nway.\n\n\n>\n> Perhaps the hard part is in making sure the correct timeout is\n> determined or set?\n>\n\nWell, I've already done it for the sensor in this patch :-) Adding it for\nthe\nISP is a bit simpler, as it is (normally) not mode dependent.\n\nRegards,\nNaush\n\n\n>\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n> >  src/ipa/raspberrypi/raspberrypi.cpp           |  7 ++++++\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 +++++++++++++++++++\n> >  3 files changed, 30 insertions(+)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> > index acd3cafe6c91..33d2a97ca916 100644\n> > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > @@ -41,6 +41,7 @@ struct IPAConfig {\n> >  struct StartConfig {\n> >         libcamera.ControlList controls;\n> >         int32 dropFrameCount;\n> > +       uint32 sensorTimeoutMs;\n> >  };\n> >\n> >  interface IPARPiInterface {\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index fd8fecb07f81..983d6e998b4c 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -281,6 +281,13 @@ void IPARPi::start(const ControlList &controls,\n> ipa::RPi::StartConfig *startConf\n> >\n> >         startConfig->dropFrameCount = dropFrameCount_;\n> >\n> > +       /*\n> > +        * Set the pipeline handler's sensor timeout to 2x the maximum\n> possible\n> > +        * frame duration for this mode.\n> > +        */\n> > +       const Duration maxSensorFrameDuration = mode_.max_frame_length *\n> mode_.line_length;\n> > +       startConfig->sensorTimeoutMs = 2 *\n> maxSensorFrameDuration.get<std::milli>();\n> > +\n> >         firstStart_ = false;\n> >         lastRunTimestamp_ = 0;\n> >  }\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index c2230199fed7..86d952b52aed 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -15,6 +15,7 @@\n> >  #include <utility>\n> >\n> >  #include <libcamera/base/shared_fd.h>\n> > +#include <libcamera/base/timer.h>\n> >  #include <libcamera/base/utils.h>\n> >\n> >  #include <libcamera/camera.h>\n> > @@ -202,6 +203,7 @@ public:\n> >         void setIspControls(const ControlList &controls);\n> >         void setDelayedControls(const ControlList &controls);\n> >         void setSensorControls(ControlList &controls);\n> > +       void sensorTimeout();\n> >\n> >         /* bufferComplete signal handlers. */\n> >         void unicamBufferDequeue(FrameBuffer *buffer);\n> > @@ -279,6 +281,10 @@ public:\n> >          */\n> >         std::optional<int32_t> notifyGainsUnity_;\n> >\n> > +       /* Timer to ensure the sensor is producing frames for the\n> pipeline handler. */\n> > +       Timer sensorTimeout_;\n> > +       std::chrono::milliseconds sensorTimeoutDuration_;\n> > +\n> >  private:\n> >         void checkRequestCompleted();\n> >         void fillRequestMetadata(const ControlList &bufferControls,\n> > @@ -1032,6 +1038,11 @@ int PipelineHandlerRPi::start(Camera *camera,\n> const ControlList *controls)\n> >                 }\n> >         }\n> >\n> > +       LOG(RPI, Debug) << \"Setting sensor timeout to \" <<\n> startConfig.sensorTimeoutMs << \" ms\";\n> > +       data->sensorTimeoutDuration_ =\n> std::chrono::milliseconds(startConfig.sensorTimeoutMs);\n> > +       data->sensorTimeout_.start(data->sensorTimeoutDuration_);\n> > +       data->sensorTimeout_.timeout.connect(data,\n> &RPiCameraData::sensorTimeout);\n> > +\n> >         return 0;\n> >  }\n> >\n> > @@ -1040,6 +1051,8 @@ void PipelineHandlerRPi::stopDevice(Camera *camera)\n> >         RPiCameraData *data = cameraData(camera);\n> >\n> >         data->state_ = RPiCameraData::State::Stopped;\n> > +       data->sensorTimeout_.timeout.disconnect();\n> > +       data->sensorTimeout_.stop();\n> >\n> >         /* Disable SOF event generation. */\n> >         data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n> > @@ -1757,6 +1770,12 @@ void RPiCameraData::setSensorControls(ControlList\n> &controls)\n> >         sensor_->setControls(&controls);\n> >  }\n> >\n> > +void RPiCameraData::sensorTimeout()\n> > +{\n> > +       LOG(RPI, Error) << \"Sensor has timed out after \"\n> > +                       << sensorTimeoutDuration_.count() << \" ms!\";\n> > +}\n> > +\n> >  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >  {\n> >         RPi::Stream *stream = nullptr;\n> > @@ -1792,6 +1811,9 @@ void\n> RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >                  */\n> >                 ctrl.set(controls::SensorTimestamp,\n> buffer->metadata().timestamp);\n> >                 bayerQueue_.push({ buffer, std::move(ctrl) });\n> > +\n> > +               /* Restart the sensor timer. */\n> > +               sensorTimeout_.start(sensorTimeoutDuration_);\n> >         } else {\n> >                 embeddedQueue_.push(buffer);\n> >         }\n> > --\n> > 2.25.1\n> >\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 663DDBDE17\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Mar 2022 13:23:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9EC0E604E7;\n\tMon, 21 Mar 2022 14:23:01 +0100 (CET)","from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com\n\t[IPv6:2a00:1450:4864:20::12d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78C3D604C6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Mar 2022 14:23:00 +0100 (CET)","by mail-lf1-x12d.google.com with SMTP id d5so5369443lfj.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Mar 2022 06:23:00 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647868981;\n\tbh=mf1cLy9HiCJvHlZz5j7C95xGGC+1LKYwyxwYV8fx4nk=;\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=XHHq58MjA6ekEm3KJ/AF4TF3rm7BiWhE8v8tmnLeOEjJ/acbvmdmtH6sXlwSwwZkE\n\tqfLmV+hGb+Y2GgcPiereU3xv6YIby2amqphHiA1ETjNOr2jeSDZW0n5XhfbCJ5U8ov\n\t3vAhbIoveJwTbrcKWltKaynLVZXTg8ZJ7R0G+OtQMezOEFolJ4Zkg2mFqGwB/udmcR\n\tMrPWeLxWfY1xknvUwaHggu+n9MVpK1RVEL2rfauoon6vC2YcUq2RsHejIDT3Qi2o8J\n\tT9fB4j9bxBEhOWFrfOw1EPHBnBJaKGvgd99Eum68VvVn7lCzZmvmHQntznOIY6eq8p\n\tU8XuwR7pf0Wtw==","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=+vW8/YtLXeLIXcFNZ+1AeerKT0vrx6yQlIM3qj9OVLQ=;\n\tb=nxnavxNlVRirFH7/b9Fbo/+pu5nm394TdQzVxuoj5bxQrE6ShhR3a3jYJzfb0nYGVx\n\tSIxXuAgq7LMVLFtBCUmsAaNcEMFtcXmB+L2BgFixyInaK5aZpUtL3XOdPQDSGZZ4LOCI\n\t3JWkANXPCHorYBGZwuUeaIPqnnNkzyKd3+bZrF8qKb22BZ61egqZgjfDhdjKhzUogEUV\n\tJ1layHCiN7rUjTYAH+jEzwSVfRLKeeIF6OdkxS0hr7rD0nk4FvD/JkcuJiszSQ91IErh\n\tf6HM5TF863ogKarmR9r8txzpLpdlQHCQrQXULw0EEC2cw3y1JcKZKOpBoQpw1FkuOSZs\n\tAx1g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"nxnavxNl\"; 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=+vW8/YtLXeLIXcFNZ+1AeerKT0vrx6yQlIM3qj9OVLQ=;\n\tb=hNRBtFqjMelozbquNy4weplGRoIkKif/v9BASJByKBkI51c/D3oa/Y6IQNnoWEziE+\n\tYD2j1YeQbvCs0CzgN5nqcH+njmSdqig1tYLN+aphxkNvH+WbxTQmQBzMI+rkfE5qTez0\n\te5hqBBlEaYYQ8X1OhIUiAxsnj/akXXJHuiQ3Cu0LaXAGgoDCluc5JLl4ysgL9ST0nqGj\n\taAMJ4IZu9MHO2wVEf3mHcvRvABA0ESaKG0Fy0urIpnlbFiwt4nK3cHzLpwXTe1TF65It\n\tX/SGKOtpi/X6dU4gcxMvXpVbY+RyjSESEi2v0Q2jyU+HLCvRIuZQzV3uVEB6xgV3kZMg\n\tQlwA==","X-Gm-Message-State":"AOAM533v7JPQyYJDjDlLH6bZtHIfvOSuiejpBzmj5vN+SuenbqaB/tQL\n\tIcrefhBTgv581DuTqxVJWxLOqxU/trFkHf5bvYaGpfECPdg=","X-Google-Smtp-Source":"ABdhPJzHXgK03/fnRp9cejHa5MhjAYUwzkjs6nQ5KT2Z7NS0Opi8hZBJPwZv9oyXqP1k+BhVSYKTIcVBMClV/+F3+u0=","X-Received":"by 2002:a05:6512:12c8:b0:448:7c3f:60f8 with SMTP id\n\tp8-20020a05651212c800b004487c3f60f8mr15386689lfg.79.1647868979765;\n\tMon, 21 Mar 2022 06:22:59 -0700 (PDT)","MIME-Version":"1.0","References":"<20220321124828.1845058-1-naush@raspberrypi.com>\n\t<164786843265.1484799.450675013157466896@Monstersaurus>","In-Reply-To":"<164786843265.1484799.450675013157466896@Monstersaurus>","Date":"Mon, 21 Mar 2022 13:22:43 +0000","Message-ID":"<CAEmqJPpvyhm038jW_rKFyLOfiMK+Yg8YdSwJtjs1d+i1qG+W1A@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"0000000000002cf48005daba6596\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Add a sensor\n\tdequeue timeout","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":22345,"web_url":"https://patchwork.libcamera.org/comment/22345/","msgid":"<YjiE+AWPt4B+IjJZ@pendragon.ideasonboard.com>","date":"2022-03-21T14:00:24","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Add a sensor\n\tdequeue timeout","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Mon, Mar 21, 2022 at 01:22:43PM +0000, Naushir Patuck via libcamera-devel wrote:\n> On Mon, 21 Mar 2022 at 13:13, Kieran Bingham wrote:\n> > Quoting Naushir Patuck via libcamera-devel (2022-03-21 12:48:28)\n> > > Add a timer to the pipeline handler that get set on start(), and subsequently\n> > > reset on each frame dequeue from the sensor. If the timeout expires, log an\n> > > error message indicating so. This is useful for debugging sensor failures were\n> > > the device just stops streaming frames to Unicam, or the pipeline handler has\n> > > failed to queue buffers to the Unicam device driver.\n> > >\n> > > The timeout is calculated as 2x the maximum frame length possible for a given\n> > > mode.\n> >\n> > This is an interesting idea, and I think will prove useful for\n> > identifying stalls in streams.\n> >\n> > Would it make sense to make this a feature of the V4L2VideoDevice? Then\n> > it would (automatically?) cover ISP hangs as well as sensor hangs...\n> \n> It would be nice to add this to a level above, and allow all pipeline handlers\n> to gain this functionality!  I can look to do that if we want to go this way.\n\nThat would be nice indeed. Let's be careful not to generate false\npositives though. I suppose the mechanism would need to be enabled by\nthe V4L2VideoDevice user by setting a timeout, so the risk shouldn't be\ntoo high.\n\n> > Perhaps the hard part is in making sure the correct timeout is\n> > determined or set?\n> \n> Well, I've already done it for the sensor in this patch :-) Adding it for the\n> ISP is a bit simpler, as it is (normally) not mode dependent.\n> \n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > ---\n> > >  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n> > >  src/ipa/raspberrypi/raspberrypi.cpp           |  7 ++++++\n> > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 +++++++++++++++++++\n> > >  3 files changed, 30 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> > > index acd3cafe6c91..33d2a97ca916 100644\n> > > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > > @@ -41,6 +41,7 @@ struct IPAConfig {\n> > >  struct StartConfig {\n> > >         libcamera.ControlList controls;\n> > >         int32 dropFrameCount;\n> > > +       uint32 sensorTimeoutMs;\n> > >  };\n> > >\n> > >  interface IPARPiInterface {\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index fd8fecb07f81..983d6e998b4c 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -281,6 +281,13 @@ void IPARPi::start(const ControlList &controls, ipa::RPi::StartConfig *startConf\n> > >\n> > >         startConfig->dropFrameCount = dropFrameCount_;\n> > >\n> > > +       /*\n> > > +        * Set the pipeline handler's sensor timeout to 2x the maximum possible\n> > > +        * frame duration for this mode.\n> > > +        */\n> > > +       const Duration maxSensorFrameDuration = mode_.max_frame_length * mode_.line_length;\n> > > +       startConfig->sensorTimeoutMs = 2 * maxSensorFrameDuration.get<std::milli>();\n\nI don't like having the IPA instruct the pipeline handler. It may be\nconsidered as nitpicking, but I'd rather expose data from the API, (such\nas the frame duration for instance), and let the pipeline handler\ndetermine the timeout. Doesn't the pipeline handler already have access\nto the frame duration, as it's configurable by applications through\nlibcamera controls ?\n\n> > > +\n> > >         firstStart_ = false;\n> > >         lastRunTimestamp_ = 0;\n> > >  }\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index c2230199fed7..86d952b52aed 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -15,6 +15,7 @@\n> > >  #include <utility>\n> > >\n> > >  #include <libcamera/base/shared_fd.h>\n> > > +#include <libcamera/base/timer.h>\n> > >  #include <libcamera/base/utils.h>\n> > >\n> > >  #include <libcamera/camera.h>\n> > > @@ -202,6 +203,7 @@ public:\n> > >         void setIspControls(const ControlList &controls);\n> > >         void setDelayedControls(const ControlList &controls);\n> > >         void setSensorControls(ControlList &controls);\n> > > +       void sensorTimeout();\n> > >\n> > >         /* bufferComplete signal handlers. */\n> > >         void unicamBufferDequeue(FrameBuffer *buffer);\n> > > @@ -279,6 +281,10 @@ public:\n> > >          */\n> > >         std::optional<int32_t> notifyGainsUnity_;\n> > >\n> > > +       /* Timer to ensure the sensor is producing frames for the pipeline handler. */\n> > > +       Timer sensorTimeout_;\n> > > +       std::chrono::milliseconds sensorTimeoutDuration_;\n> > > +\n> > >  private:\n> > >         void checkRequestCompleted();\n> > >         void fillRequestMetadata(const ControlList &bufferControls,\n> > > @@ -1032,6 +1038,11 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> > >                 }\n> > >         }\n> > >\n> > > +       LOG(RPI, Debug) << \"Setting sensor timeout to \" << startConfig.sensorTimeoutMs << \" ms\";\n> > > +       data->sensorTimeoutDuration_ = std::chrono::milliseconds(startConfig.sensorTimeoutMs);\n> > > +       data->sensorTimeout_.start(data->sensorTimeoutDuration_);\n> > > +       data->sensorTimeout_.timeout.connect(data, &RPiCameraData::sensorTimeout);\n> > > +\n> > >         return 0;\n> > >  }\n> > >\n> > > @@ -1040,6 +1051,8 @@ void PipelineHandlerRPi::stopDevice(Camera *camera)\n> > >         RPiCameraData *data = cameraData(camera);\n> > >\n> > >         data->state_ = RPiCameraData::State::Stopped;\n> > > +       data->sensorTimeout_.timeout.disconnect();\n\nYou don't have to connect/disconnect the signal when starting and\nstopping the camera, you can connect it when creating the RPiCameraData\ninstance and keep it connected.\n\n> > > +       data->sensorTimeout_.stop();\n> > >\n> > >         /* Disable SOF event generation. */\n> > >         data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);\n> > > @@ -1757,6 +1770,12 @@ void RPiCameraData::setSensorControls(ControlList &controls)\n> > >         sensor_->setControls(&controls);\n> > >  }\n> > >\n> > > +void RPiCameraData::sensorTimeout()\n> > > +{\n> > > +       LOG(RPI, Error) << \"Sensor has timed out after \"\n> > > +                       << sensorTimeoutDuration_.count() << \" ms!\";\n> > > +}\n> > > +\n> > >  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> > >  {\n> > >         RPi::Stream *stream = nullptr;\n> > > @@ -1792,6 +1811,9 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> > >                  */\n> > >                 ctrl.set(controls::SensorTimestamp, buffer->metadata().timestamp);\n> > >                 bayerQueue_.push({ buffer, std::move(ctrl) });\n> > > +\n> > > +               /* Restart the sensor timer. */\n> > > +               sensorTimeout_.start(sensorTimeoutDuration_);\n> > >         } else {\n> > >                 embeddedQueue_.push(buffer);\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 A3299BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Mar 2022 14:00:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04077604DC;\n\tMon, 21 Mar 2022 15:00:43 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 186B1604C6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Mar 2022 15:00:42 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 851EC291;\n\tMon, 21 Mar 2022 15:00:41 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647871243;\n\tbh=RIRscb5stiS++As3YWykPFCXmr5iSK1kDqvjmE30sEI=;\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=YrJDkxIa7TBhuzFL5DDFZ5A0Z6Z1bGxDctFxClqfHYetQDbs6YGgVsTJ+rp8nBTPC\n\tPrmS96dsB6fZpnbbmHzT/NW3Eer7KeSdyS+qU95OwdbOE+3ZOPwcwiQNaI2LoACBv5\n\tD60YwG29kyzU3qRXVuoWSiVqLms48oGhtup/LiVkPH9b1EDajMFeM5gW2zOrDYhGkq\n\tsJDOfnb7lAQp5Gz6DwqJ8V3v19geMiUrblaxzhUfq4aF51EyS+3nCMgZUw803TlLrN\n\tbDP6fUuh/WyvJaM4jtXnJAoflGEWF/bPxDPe8yYWO0clD11+OmJp+XIVkmslEBVa4O\n\tCi3iGzD4ka0gg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1647871241;\n\tbh=RIRscb5stiS++As3YWykPFCXmr5iSK1kDqvjmE30sEI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HosZCWCKVcluLx1/M4WsFn9789v/9Cejp5QG4B+Yi1GH7/3ozB/JALOPF2FkMhxWm\n\tX55RoT5c703kz0tiwafqJDYVzvhTnGCLx8FviEMrf1iwSx6e9Z/iTtSp5/NBGcjIYI\n\t1560z+kUZjTeSqgMTc+en+xmYl/Xmup/ev2SnJks="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HosZCWCK\"; dkim-atps=neutral","Date":"Mon, 21 Mar 2022 16:00:24 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YjiE+AWPt4B+IjJZ@pendragon.ideasonboard.com>","References":"<20220321124828.1845058-1-naush@raspberrypi.com>\n\t<164786843265.1484799.450675013157466896@Monstersaurus>\n\t<CAEmqJPpvyhm038jW_rKFyLOfiMK+Yg8YdSwJtjs1d+i1qG+W1A@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPpvyhm038jW_rKFyLOfiMK+Yg8YdSwJtjs1d+i1qG+W1A@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Add a sensor\n\tdequeue timeout","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>"}}]