[{"id":26549,"web_url":"https://patchwork.libcamera.org/comment/26549/","msgid":"<20230305113828.wt5yvvlp2ai32upc@uno.localdomain>","date":"2023-03-05T11:38:28","subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Thu, Mar 02, 2023 at 01:54:29PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Date: Thu, 2 Mar 2023 13:54:29 +0000\n> From: Naushir Patuck \n> To: libcamera-devel@lists.libcamera.org\n> Subject: [PATCH v2 3/3] pipeline: raspberrypi: Add a Unicam timeout\n> override config options\n> X-Mailer: git-send-email 2.34.1\n>\n> Add a new parameter to the pipeline handler config file named\n> \"unicam_timeout_value_ms\" to allow users to override the automiatically\n> computed Unicam timeout value.\n>\n> This value is given in milliseconds, and setting a value of 0 (the\n> default value) disables the override.\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n\nMy understanding is that emitting a signal and have no slot connected\nis safe.\nReviewed-by: Jacopo Mondi \n\n> ---\n> .../pipeline/raspberrypi/data/example.yaml | 11 ++++++++++-\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++++++++++++++\n> 2 files changed, 24 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> index ad5f2344384f..5d4ca997b7a0 100644\n> --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> @@ -32,6 +32,15 @@\n> # Override any request from the IPA to drop a number of startup\n> # frames.\n> #\n> - # \"disable_startup_frame_drops\": false\n> + # \"disable_startup_frame_drops\": false,\n> +\n> + # Custom timeout value (in ms) for Unicam to use. This overrides\n> + # the value computed by the pipeline handler based on frame\n> + # durations.\n> + #\n> + # Set this value to 0 to use the pipeline handler computed\n> + # timeout value.\n> + #\n> + # \"unicam_timeout_value_ms\": 0\n> }\n> }\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 3d04842a2440..6b0880c579eb 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -319,6 +319,11 @@ public:\n> \t\t * frames.\n> \t\t */\n> \t\tbool disableStartupFrameDrops;\n> +\t\t/*\n> +\t\t * Override the Unicam timeout value calculated by the IPA based\n> +\t\t * on frame durations.\n> +\t\t */\n> +\t\tunsigned int unicamTimeoutValue;\n> \t};\n>\n> \tConfig config_;\n> @@ -1715,6 +1720,7 @@ int RPiCameraData::loadPipelineConfiguration()\n> \t\t.minUnicamBuffers = 2,\n> \t\t.minTotalUnicamBuffers = 4,\n> \t\t.disableStartupFrameDrops = false,\n> +\t\t.unicamTimeoutValue = 0,\n> \t};\n>\n> \tchar const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> @@ -1750,6 +1756,14 @@ int RPiCameraData::loadPipelineConfiguration()\n> \t\tphConfig[\"min_total_unicam_buffers\"].get(config_.minTotalUnicamBuffers);\n> \tconfig_.disableStartupFrameDrops =\n> \t\tphConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> +\tconfig_.unicamTimeoutValue =\n> +\t\tphConfig[\"unicam_timeout_value_ms\"].get(config_.unicamTimeoutValue);\n> +\n> +\tif (config_.unicamTimeoutValue) {\n> +\t\t/* Disable the IPA signal to control timeout and set the user requested value. */\n> +\t\tunicam_[Unicam::Image].dev()->dequeueTimeout.disconnect();\n> +\t\tunicam_[Unicam::Image].dev()->setDequeueTimeout(config_.unicamTimeoutValue * 1ms);\n> +\t}\n>\n> \tif (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> \t\tLOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","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 E6B39BE080\n\tfor ;\n\tSun, 5 Mar 2023 11:38:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B448D6265A;\n\tSun, 5 Mar 2023 12:38:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 299726261A\n\tfor ;\n\tSun, 5 Mar 2023 12:38:31 +0100 (CET)","from ideasonboard.com (host-79-47-54-87.retail.telecomitalia.it\n\t[79.47.54.87])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A9454904;\n\tSun, 5 Mar 2023 12:38:30 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678016312;\n\tbh=LZN39Eu0X+aBZwermqGlP4D9D3gT6bSg1ZEpAbBEYy0=;\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=Ek5ZPfYRdsdubQCvCAQ99QDLfc8z0zVs7svajmd14/oaXLZMD3E3th+nhRNEzpSVw\n\tWgLNgApLNPTqvDiuIESCajwjzBeAUWTQWXkBmOo9vdp40jyLMp3aw5zsn5fqjTfkqn\n\tWV+yCWhLtA0vAInynabCkE2GvxmSW3snZglJS6/sGTCMAUr/0fnSydTCRfsJ/IEGlV\n\tKK1Ybx1xKg5/otihyz9OT2OcUK1rEK5gminHiVidUUSLHC2ebFDATtzsVoRKwBP2Iy\n\tWbKwmf04Edi6M9ZbItVqA1THJ5fybDGIqlIJ3Rnm/JbxeYUiHMkGdVKUCCLlhKDCSH\n\tSSm+rHAuGWi/A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678016310;\n\tbh=LZN39Eu0X+aBZwermqGlP4D9D3gT6bSg1ZEpAbBEYy0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cCa86TAnRfUuKDpcHpMVMaXfYMJIZOPoiZpGGiMrbWMoOFAOnto4tgmiTEGFxBUfs\n\toBvbhhYKvYUSIHMAmpPNTt/VCO8btyTWbttlcuiPXDZnb/vsLikfllIRMEGc7S167B\n\tn/EogmjvuXtx2WP174tyPIYFV4LqD+59ivcm5Yr8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cCa86TAn\"; dkim-atps=neutral","Date":"Sun, 5 Mar 2023 12:38:28 +0100","To":"Naushir Patuck ","Message-ID":"<20230305113828.wt5yvvlp2ai32upc@uno.localdomain>","References":"<20230302135429.23821-1-naush@raspberrypi.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26566,"web_url":"https://patchwork.libcamera.org/comment/26566/","msgid":"<20230306172832.GI20909@pendragon.ideasonboard.com>","date":"2023-03-06T17:28:32","subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Thu, Mar 02, 2023 at 01:54:29PM +0000, Naushir Patuck via libcamera-devel wrote:\n> \n> Add a new parameter to the pipeline handler config file named\n> \"unicam_timeout_value_ms\" to allow users to override the automiatically\n\ns/automiatically/automatically/\n\n> computed Unicam timeout value.\n> \n> This value is given in milliseconds, and setting a value of 0 (the\n> default value) disables the override.\n\nCould you explain the use case for setting a custom timeout ?\n\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> ---\n> .../pipeline/raspberrypi/data/example.yaml | 11 ++++++++++-\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++++++++++++++\n> 2 files changed, 24 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> index ad5f2344384f..5d4ca997b7a0 100644\n> --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> @@ -32,6 +32,15 @@\n> # Override any request from the IPA to drop a number of startup\n> # frames.\n> #\n> - # \"disable_startup_frame_drops\": false\n> + # \"disable_startup_frame_drops\": false,\n> +\n> + # Custom timeout value (in ms) for Unicam to use. This overrides\n> + # the value computed by the pipeline handler based on frame\n> + # durations.\n> + #\n> + # Set this value to 0 to use the pipeline handler computed\n> + # timeout value.\n> + #\n> + # \"unicam_timeout_value_ms\": 0\n\nWhile at it, I'd add a trailing comma here to minimize the diff next\ntime you will add a parameter.\n\n> }\n> }\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 3d04842a2440..6b0880c579eb 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -319,6 +319,11 @@ public:\n> \t\t * frames.\n> \t\t */\n> \t\tbool disableStartupFrameDrops;\n> +\t\t/*\n> +\t\t * Override the Unicam timeout value calculated by the IPA based\n> +\t\t * on frame durations.\n> +\t\t */\n> +\t\tunsigned int unicamTimeoutValue;\n> \t};\n> \n> \tConfig config_;\n> @@ -1715,6 +1720,7 @@ int RPiCameraData::loadPipelineConfiguration()\n> \t\t.minUnicamBuffers = 2,\n> \t\t.minTotalUnicamBuffers = 4,\n> \t\t.disableStartupFrameDrops = false,\n> +\t\t.unicamTimeoutValue = 0,\n> \t};\n> \n> \tchar const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> @@ -1750,6 +1756,14 @@ int RPiCameraData::loadPipelineConfiguration()\n> \t\tphConfig[\"min_total_unicam_buffers\"].get(config_.minTotalUnicamBuffers);\n> \tconfig_.disableStartupFrameDrops =\n> \t\tphConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> +\tconfig_.unicamTimeoutValue =\n> +\t\tphConfig[\"unicam_timeout_value_ms\"].get(config_.unicamTimeoutValue);\n> +\n> +\tif (config_.unicamTimeoutValue) {\n> +\t\t/* Disable the IPA signal to control timeout and set the user requested value. */\n> +\t\tunicam_[Unicam::Image].dev()->dequeueTimeout.disconnect();\n\nDidn't you mean to disconnect the signal emitted by the IPA module to\nset the timeout, not the signal emitted by the V4L2VideoDevice when a\ntimeout occurs ?\n\n> +\t\tunicam_[Unicam::Image].dev()->setDequeueTimeout(config_.unicamTimeoutValue * 1ms);\n> +\t}\n> \n> \tif (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> \t\tLOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";","headers":{"Return-Path":"","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 5D90BBE080\n\tfor ;\n\tMon, 6 Mar 2023 17:28:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1E6762698;\n\tMon, 6 Mar 2023 18:28:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DCA4A603B1\n\tfor ;\n\tMon, 6 Mar 2023 18:28:29 +0100 (CET)","from pendragon.ideasonboard.com\n\t(153.162-64-87.adsl-dyn.isp.belgacom.be [87.64.162.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 58AF4308;\n\tMon, 6 Mar 2023 18:28:29 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678123711;\n\tbh=LH34k1RNIhoneHylM8Atzn9utjgzGZUA5S+s04dU5NM=;\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=j6HKTjc1bEHCYwVlN/+GpAY+6ot9UmU8r/ZRDVFSkaTad9PLjpF/48KEReYwE7oqz\n\tMVATDgFDyR1PQ58WcKibj/pFtWn4z4Y1wRA/w8CUuflWHQ+KVFAfFHacr9qoWMiJ1O\n\t0xUMA53k9cQheO6EBZhjiuOuXgfTufPw0WXzRHz4EIYq8kJ82pjGLIlCe5BNGIVRtT\n\tDWQazOUd3jq/olCqBm0TDH58wd1bF+nvuG8q57mPOs5Xtp+SRg4DzFEzphb+vLFh4b\n\tSoPkw0qzfcxx9nQS9f0h301BvPeNO0XS51HRfYtLgrYrKGzq0+XYA4ymP7InZUslVZ\n\tulDYTl5qezSTw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678123709;\n\tbh=LH34k1RNIhoneHylM8Atzn9utjgzGZUA5S+s04dU5NM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wNjlrbK/m5NLSxYW8nqWef4fUhnvhsljHhJzhH6qWSjEcooZENZ78kQmEQnel2imR\n\t4nrqJHxIA37o9A6Lmp00bRgwZdSuLNBA2vR5BNd/K4u5k4A62pM1tUlMzNSgxJPqPQ\n\tWrmDXITmzF6Ub52wO+FX6vqnxeLsIEDSPWGNX2mk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"wNjlrbK/\"; dkim-atps=neutral","Date":"Mon, 6 Mar 2023 19:28:32 +0200","To":"Naushir Patuck ","Message-ID":"<20230306172832.GI20909@pendragon.ideasonboard.com>","References":"<20230302135429.23821-1-naush@raspberrypi.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26571,"web_url":"https://patchwork.libcamera.org/comment/26571/","msgid":"","date":"2023-03-07T08:50:42","subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","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 Mon, 6 Mar 2023 at 17:28, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Thu, Mar 02, 2023 at 01:54:29PM +0000, Naushir Patuck via\n> libcamera-devel wrote:\n> >\n> > Add a new parameter to the pipeline handler config file named\n> > \"unicam_timeout_value_ms\" to allow users to override the automiatically\n>\n> s/automiatically/automatically/\n>\n> > computed Unicam timeout value.\n> >\n> > This value is given in milliseconds, and setting a value of 0 (the\n> > default value) disables the override.\n>\n> Could you explain the use case for setting a custom timeout ?\n>\n\nThis is partially from a user requested use case.\n\nAssume an app is configured a RAW stream, and provides buffers for the\nstream on\nevery request (i.e. our mandatory stream hint). If the application holds\noff on\nsending requests for whatever reason (the dreaded timelapse comes to mind\nagin),\nthen we will possibly hit the watchdog timeout as it is only a small\nmultiple of\nthe frame length. This override allows an application to select a larger\nvalue\nwith the knowledge that it may space requests longer than the calculated\ntimeout\nvalue.\n\n\n>\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > ---\n> > .../pipeline/raspberrypi/data/example.yaml | 11 ++++++++++-\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++++++++++++++\n> > 2 files changed, 24 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > index ad5f2344384f..5d4ca997b7a0 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > @@ -32,6 +32,15 @@\n> > # Override any request from the IPA to drop a number of\n> startup\n> > # frames.\n> > #\n> > - # \"disable_startup_frame_drops\": false\n> > + # \"disable_startup_frame_drops\": false,\n> > +\n> > + # Custom timeout value (in ms) for Unicam to use. This\n> overrides\n> > + # the value computed by the pipeline handler based on\n> frame\n> > + # durations.\n> > + #\n> > + # Set this value to 0 to use the pipeline handler\n> computed\n> > + # timeout value.\n> > + #\n> > + # \"unicam_timeout_value_ms\": 0\n>\n> While at it, I'd add a trailing comma here to minimize the diff next\n> time you will add a parameter.\n>\n> > }\n> > }\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 3d04842a2440..6b0880c579eb 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -319,6 +319,11 @@ public:\n> > * frames.\n> > */\n> > bool disableStartupFrameDrops;\n> > + /*\n> > + * Override the Unicam timeout value calculated by the IPA\n> based\n> > + * on frame durations.\n> > + */\n> > + unsigned int unicamTimeoutValue;\n> > };\n> >\n> > Config config_;\n> > @@ -1715,6 +1720,7 @@ int RPiCameraData::loadPipelineConfiguration()\n> > .minUnicamBuffers = 2,\n> > .minTotalUnicamBuffers = 4,\n> > .disableStartupFrameDrops = false,\n> > + .unicamTimeoutValue = 0,\n> > };\n> >\n> > char const *configFromEnv =\n> utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > @@ -1750,6 +1756,14 @@ int RPiCameraData::loadPipelineConfiguration()\n> > phConfig[\"min_total_unicam_buffers\"].get int>(config_.minTotalUnicamBuffers);\n> > config_.disableStartupFrameDrops =\n> >\n> phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> > + config_.unicamTimeoutValue =\n> > + phConfig[\"unicam_timeout_value_ms\"].get int>(config_.unicamTimeoutValue);\n> > +\n> > + if (config_.unicamTimeoutValue) {\n> > + /* Disable the IPA signal to control timeout and set the\n> user requested value. */\n> > + unicam_[Unicam::Image].dev()->dequeueTimeout.disconnect();\n>\n> Didn't you mean to disconnect the signal emitted by the IPA module to\n> set the timeout, not the signal emitted by the V4L2VideoDevice when a\n> timeout occurs ?\n>\n\nYou are right, and annoyingly I was certain I fixed this before submitting\n:-(\nI will fix up for the next revision!\n\nRegards,\nNaush\n\n\n\n>\n> > +\n> unicam_[Unicam::Image].dev()->setDequeueTimeout(config_.unicamTimeoutValue\n> * 1ms);\n> > + }\n> >\n> > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > LOG(RPI, Error) << \"Invalid configuration:\n> min_total_unicam_buffers must be >= min_unicam_buffers\";\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 16A64BE080\n\tfor ;\n\tTue, 7 Mar 2023 08:50:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 87C36626B1;\n\tTue, 7 Mar 2023 09:50:52 +0100 (CET)","from mail-yb1-xb36.google.com (mail-yb1-xb36.google.com\n\t[IPv6:2607:f8b0:4864:20::b36])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D91376265E\n\tfor ;\n\tTue, 7 Mar 2023 09:50:50 +0100 (CET)","by mail-yb1-xb36.google.com with SMTP id c80so3694115ybf.5\n\tfor ;\n\tTue, 07 Mar 2023 00:50:50 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678179052;\n\tbh=yr3A8FlvX2vqXusiNDgtpyt7eIuwVhUSB7ufEhRRSQc=;\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=cEbFOsJCvpDjmlle9LxwJ02GfrKEBaHIF43hDEEqg1hxMivLbouTc9d0p9lQGPO+g\n\txMcPWY/0zK+rMQEBzz4BLV5T4BIEKV50OD5e5Gdn8zAx0rhOUSADpPcLcARuqYGAi5\n\tcOaoqJklywox6zdAmWCCVBbq8XMYItrW8haPW6F8JIQLTgDYJ7YzvOnfhTRE/76hAF\n\t/ihxE8ZKZbje2NS9k8V8Po2JjkldMN4AhDDT54b3+NmZf5yjyOeAI4VicWY6U+IPhW\n\tYkKPfT+Z7Ul/+mau5YUzgx6UcbThRfy6vtA0nHReLiuyoByLXXcERI2hz/QLrzIbQV\n\tNgda5ufTzflOA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1678179049;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=80mXY83lKAhyH8YMnsIanvLdMHh5Jgfb3wHJ8zuj9K0=;\n\tb=mceDrWPr5o1SSWoqTTG5kTj6pP12we0abZyq1XKozGCeqkrFPNwlZlB3TpHNXr2I7J\n\t+zB830+r+d4CFUbZAAM1KXR0q2wE+VqBf/YNKyqnUDDT29MdrmJi9AjRkm8TyIhuBonO\n\ttLKoi3s5DQWQcx4oWc7HJ5IhBUi1s2NweZsGXWcOZIDGsKkWbIbqMFRQq0tuZDG0bWNY\n\txjn206p8pqX45LwYQlNStoR+tAw+wsdvS/eyqacDGZMVJgzQcO505nw6LYltpLLUQlid\n\tnCW/hyPuVOC4aLD5hOHBjjYg1pl1/LL4pIe4UpcatunMqBFUGzdC+0xa2fKfSZLCoKeP\n\toBQw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"mceDrWPr\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1678179049;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=80mXY83lKAhyH8YMnsIanvLdMHh5Jgfb3wHJ8zuj9K0=;\n\tb=NELIg8yFKeVdBiQV5PFaSypmB3EhKYmsjYmOSLomI9jYC8vMm8LTCaWxjw+qPVEgMx\n\tpyIhY0EptP0K29CnkoOzr/a0qrcPNbymGlyPinxAaM+vD/WEZegh5bQPpJ8RWtwpgpvh\n\tAmNxAEEVDywySCk8zteG+YiN6Anai8dadB4VZor9NrOyqi45015KJphDq2Y/nGo0dcjZ\n\t47J6Y08KRUOr7PMB0Q4Tqxsq43MGIE17xOg2VbPpFhz8pRnTrCxLxAXdpzlN6wZPvSXN\n\tX9uID1ehiwWlXtcKT2ZZGJ3c/rhPJCWr7e7ItgH7qCSjODmvuYngephPB2WVWvdzQzlU\n\tVsCQ==","X-Gm-Message-State":"AO0yUKWrlu+BxMrdyZFK71XPCDb9NJVGvLaAGSaz3GGbk13ymZnpLCoL\n\tMHjbzUgFjtaPjA6TYY/Htf2jTy/muX7Nu+ho+MI9FiWqXqY6/wwqo4OCrA==","X-Google-Smtp-Source":"AK7set9u35gpthz9PQS8jzXe1tAIZqWbn9eWV/gdCHQJ7F2YMK4GXgLkuj1oO3efa2c7md1DXKFmtOonZS6UzkDvl0g=","X-Received":"by 2002:a5b:e92:0:b0:ab8:1ed9:cfc9 with SMTP id\n\tz18-20020a5b0e92000000b00ab81ed9cfc9mr8144308ybr.6.1678179049756;\n\tTue, 07 Mar 2023 00:50:49 -0800 (PST)","MIME-Version":"1.0","References":"<20230302135429.23821-1-naush@raspberrypi.com>\n\t\n\t<20230306172832.GI20909@pendragon.ideasonboard.com>","In-Reply-To":"<20230306172832.GI20909@pendragon.ideasonboard.com>","Date":"Tue, 7 Mar 2023 08:50:42 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000021720d05f64b82db\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26580,"web_url":"https://patchwork.libcamera.org/comment/26580/","msgid":"<20230307100900.GC22827@pendragon.ideasonboard.com>","date":"2023-03-07T10:09:00","subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","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, Mar 07, 2023 at 08:50:42AM +0000, Naushir Patuck wrote:\n> On Mon, 6 Mar 2023 at 17:28, Laurent Pinchart wrote:\n> > On Thu, Mar 02, 2023 at 01:54:29PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > >\n> > > Add a new parameter to the pipeline handler config file named\n> > > \"unicam_timeout_value_ms\" to allow users to override the automiatically\n> >\n> > s/automiatically/automatically/\n> >\n> > > computed Unicam timeout value.\n> > >\n> > > This value is given in milliseconds, and setting a value of 0 (the\n> > > default value) disables the override.\n> >\n> > Could you explain the use case for setting a custom timeout ?\n> \n> This is partially from a user requested use case.\n> \n> Assume an app is configured a RAW stream, and provides buffers for the stream on\n> every request (i.e. our mandatory stream hint). If the application holds off on\n> sending requests for whatever reason (the dreaded timelapse comes to mind agin),\n> then we will possibly hit the watchdog timeout as it is only a small multiple of\n> the frame length. This override allows an application to select a larger value\n> with the knowledge that it may space requests longer than the calculated timeout\n> value.\n\nOK. Could you record that at least in the commit message ?\n\n> > > Signed-off-by: Naushir Patuck \n> > > Reviewed-by: David Plowman \n> > > ---\n> > > .../pipeline/raspberrypi/data/example.yaml | 11 ++++++++++-\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++++++++++++++\n> > > 2 files changed, 24 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > index ad5f2344384f..5d4ca997b7a0 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > @@ -32,6 +32,15 @@\n> > > # Override any request from the IPA to drop a number of startup\n> > > # frames.\n> > > #\n> > > - # \"disable_startup_frame_drops\": false\n> > > + # \"disable_startup_frame_drops\": false,\n> > > +\n> > > + # Custom timeout value (in ms) for Unicam to use. This overrides\n> > > + # the value computed by the pipeline handler based on frame\n> > > + # durations.\n> > > + #\n> > > + # Set this value to 0 to use the pipeline handler computed\n> > > + # timeout value.\n> > > + #\n> > > + # \"unicam_timeout_value_ms\": 0\n> >\n> > While at it, I'd add a trailing comma here to minimize the diff next\n> > time you will add a parameter.\n> >\n> > > }\n> > > }\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 3d04842a2440..6b0880c579eb 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -319,6 +319,11 @@ public:\n> > > * frames.\n> > > */\n> > > bool disableStartupFrameDrops;\n> > > + /*\n> > > + * Override the Unicam timeout value calculated by the IPA based\n> > > + * on frame durations.\n> > > + */\n> > > + unsigned int unicamTimeoutValue;\n> > > };\n> > >\n> > > Config config_;\n> > > @@ -1715,6 +1720,7 @@ int RPiCameraData::loadPipelineConfiguration()\n> > > .minUnicamBuffers = 2,\n> > > .minTotalUnicamBuffers = 4,\n> > > .disableStartupFrameDrops = false,\n> > > + .unicamTimeoutValue = 0,\n> > > };\n> > >\n> > > char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > > @@ -1750,6 +1756,14 @@ int RPiCameraData::loadPipelineConfiguration()\n> > > phConfig[\"min_total_unicam_buffers\"].get(config_.minTotalUnicamBuffers);\n> > > config_.disableStartupFrameDrops =\n> > > phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> > > + config_.unicamTimeoutValue =\n> > > + phConfig[\"unicam_timeout_value_ms\"].get(config_.unicamTimeoutValue);\n> > > +\n> > > + if (config_.unicamTimeoutValue) {\n> > > + /* Disable the IPA signal to control timeout and set the user requested value. */\n> > > + unicam_[Unicam::Image].dev()->dequeueTimeout.disconnect();\n> >\n> > Didn't you mean to disconnect the signal emitted by the IPA module to\n> > set the timeout, not the signal emitted by the V4L2VideoDevice when a\n> > timeout occurs ?\n> \n> You are right, and annoyingly I was certain I fixed this before submitting :-(\n> I will fix up for the next revision!\n> \n> > > + unicam_[Unicam::Image].dev()->setDequeueTimeout(config_.unicamTimeoutValue * 1ms);\n> > > + }\n> > >\n> > > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > > LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";","headers":{"Return-Path":"","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 4C058BE080\n\tfor ;\n\tTue, 7 Mar 2023 10:08:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D732626B2;\n\tTue, 7 Mar 2023 11:08:57 +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 9326B6266A\n\tfor ;\n\tTue, 7 Mar 2023 11:08:56 +0100 (CET)","from pendragon.ideasonboard.com\n\t(153.162-64-87.adsl-dyn.isp.belgacom.be [87.64.162.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 09A6D4AD;\n\tTue, 7 Mar 2023 11:08:56 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678183737;\n\tbh=HtQUUnFtBsKs8w+PBsIhI7dgjyqFvU3fky7mPleC/No=;\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=DNX+fUqkCI2YJ/3owinvBbXryr/j3fIIAy+DaBVmBkrBCHB7ayuwyp212M4AF7ciO\n\ti1asugjT2MSDTG/IBC2tPENj9OkoNjZ9ddQz/bXdCvmxjbepBr+yo702AXxOpzOz54\n\tqtBzIJ/Zzq6L/TP1Y2/sxf7wjRJMDnMUGyEkD/s+3tNxxxIcbFOqX8T4ykKcGhSstP\n\tU2aGUuDViym2TewjRCue00OBgiSZVWebnuHPwHzp2EsCcn4I9oGENzFi5LM4KUKQrg\n\tMZxwHbc89VNloZCTZSYWWd9i1j77+FMEVqNmOoo5y6NOCLdqfFlEIymWF4IvtAkzNg\n\tZs/cRaLHx6VWw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678183736;\n\tbh=HtQUUnFtBsKs8w+PBsIhI7dgjyqFvU3fky7mPleC/No=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SmV6aMikNLBwpTUqVuf0/wVlPbXvxMjh/tPDHiLtQ19d51csgJhDrqM91aAajdVtA\n\tSl8ZU0yExKlFDQwAwHD+OBFFifgwg8WYs7JCm4Q0qz7sU6iS9qkpaqszmFdbmI1Wc/\n\ta3COawnLCsohxPasozacJ/4MZUhd5nWINz3YOiB0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SmV6aMik\"; dkim-atps=neutral","Date":"Tue, 7 Mar 2023 12:09:00 +0200","To":"Naushir Patuck ","Message-ID":"<20230307100900.GC22827@pendragon.ideasonboard.com>","References":"<20230302135429.23821-1-naush@raspberrypi.com>\n\t\n\t<20230306172832.GI20909@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]