[{"id":26476,"web_url":"https://patchwork.libcamera.org/comment/26476/","msgid":"","date":"2023-02-24T11:22:26","subject":"Re: [libcamera-devel] [PATCH v1 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch.\n\nOn Thu, 23 Feb 2023 at 12:49, Naushir Patuck via libcamera-devel\n 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>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/data/example.yaml | 11 ++++++-\n> .../pipeline/raspberrypi/raspberrypi.cpp | 29 ++++++++++++++-----\n> 2 files changed, 32 insertions(+), 8 deletions(-)\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..7c8c66129014 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> @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n>\n> data->state_ = RPiCameraData::State::Idle;\n>\n> + if (data->config_.unicamTimeoutValue)\n> + data->setCameraTimeout(data->config_.unicamTimeoutValue);\n> +\n> /* Start all streams. */\n> for (auto const stream : data->streams_) {\n> ret = stream->dev()->streamOn();\n> @@ -1715,6 +1723,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 +1759,8 @@ 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_.disableStartupFrameDrops);\n\nIs bool the right type here? Also is the appearance of\n\"disableStartupFrameDrops\" a copy/paste typo?\n\nApart from these minor things:\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n>\n> if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";\n> @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)\n>\n> void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)\n> {\n> - /*\n> - * Set the dequeue timeout to the larger of 5x the maximum reported\n> - * frame length advertised by the IPA over a number of frames. Allow\n> - * a minimum timeout value of 1s.\n> - */\n> - utils::Duration timeout =\n> - std::max(1s, 5 * maxFrameLengthMs * 1ms);\n> + utils::Duration timeout;\n> +\n> + if (!config_.unicamTimeoutValue) {\n> + /*\n> + * Set the dequeue timeout to the larger of 5x the maximum reported\n> + * frame length advertised by the IPA over a number of frames. Allow\n> + * a minimum timeout value of 1s.\n> + */\n> + timeout = std::max(1s, 5 * maxFrameLengthMs * 1ms);\n> + } else\n> + timeout = config_.unicamTimeoutValue * 1ms;\n>\n> LOG(RPI, Debug) << \"Setting Unicam timeout to \" << timeout;\n> unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> --\n> 2.25.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 31BD0BE080\n\tfor ;\n\tFri, 24 Feb 2023 11:22:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D94F56266C;\n\tFri, 24 Feb 2023 12:22:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677237758;\n\tbh=LJ6cOHCvC7UcFQlKN6X3JpPHyFBAM3ZI3P7WfW8bE48=;\n\th=References:In-Reply-To:Date:To:List-Id:List-Post:From:Cc:\n\tList-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help:\n\tSubject:From;\n\tb=SamiSyO89vvuGa4YM87DhhQ8dyiG/WFenpq7iLyGATYNjWpS+AnrziagcA/HvTIQr\n\tV4H2t8yvlQNncG/wrV3hFaXeFXe3m7oEvYOALQDuwfqO/mJp3iH1Nihu/X+KaJL2BX\n\tXnohzh12iAQI0AxZrc6i2J6GfM0qNKVwzZE+Yiq1by2BpaSGow+W0DCUkHd8RYcOGf\n\t2vPeQCsDCq+qixDHae4HIqcQr+RpsLztVzZr+Xyj4xdguW/sm1RQAJ7CekSvmisorY\n\ts/OzB4WpQhqv++OmocePjvVja+uNrUpVyR9KrKgdBy+uxeST8GDivt+qxGTqFU4k2P\n\tLYPojlGeHRljA==","References":"<20230223124957.11084-1-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Fri, 24 Feb 2023 11:22:26 +0000","To":"Naushir Patuck ","MIME-Version":"1.0","Message-ID":"","List-Id":"","List-Post":"","From":"David Plowman via libcamera-devel ","Precedence":"list","Cc":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","X-BeenThere":"libcamera-devel@lists.libcamera.org","List-Subscribe":",\n\t","List-Unsubscribe":",\n\t","List-Archive":"","Reply-To":"David Plowman ","List-Help":"","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","Content-Type":"message/rfc822","Content-Disposition":"inline","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26477,"web_url":"https://patchwork.libcamera.org/comment/26477/","msgid":"","date":"2023-02-24T11:30:07","subject":"Re: [libcamera-devel] [PATCH v1 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":"On Fri, 24 Feb 2023 at 11:22, David Plowman\n wrote:\n>\n> Hi Naush\n>\n> Thanks for the patch.\n>\n> On Thu, 23 Feb 2023 at 12:49, Naushir Patuck via libcamera-devel\n> 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> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/data/example.yaml | 11 ++++++-\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 29 ++++++++++++++-----\n> > 2 files changed, 32 insertions(+), 8 deletions(-)\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..7c8c66129014 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> > @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> >\n> > data->state_ = RPiCameraData::State::Idle;\n> >\n> > + if (data->config_.unicamTimeoutValue)\n> > + data->setCameraTimeout(data->config_.unicamTimeoutValue);\n> > +\n> > /* Start all streams. */\n> > for (auto const stream : data->streams_) {\n> > ret = stream->dev()->streamOn();\n> > @@ -1715,6 +1723,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 +1759,8 @@ 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_.disableStartupFrameDrops);\n>\n> Is bool the right type here? Also is the appearance of\n> \"disableStartupFrameDrops\" a copy/paste typo?\n\nOh no. Evidently I submitted the wrong patch :-( My local changes\nshow the correct code.\nI will fix this and resubmit.\n\nNaush\n\n>\n> Apart from these minor things:\n>\n> Reviewed-by: David Plowman \n>\n> Thanks!\n> David\n>\n> >\n> > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";\n> > @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)\n> >\n> > void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)\n> > {\n> > - /*\n> > - * Set the dequeue timeout to the larger of 5x the maximum reported\n> > - * frame length advertised by the IPA over a number of frames. Allow\n> > - * a minimum timeout value of 1s.\n> > - */\n> > - utils::Duration timeout =\n> > - std::max(1s, 5 * maxFrameLengthMs * 1ms);\n> > + utils::Duration timeout;\n> > +\n> > + if (!config_.unicamTimeoutValue) {\n> > + /*\n> > + * Set the dequeue timeout to the larger of 5x the maximum reported\n> > + * frame length advertised by the IPA over a number of frames. Allow\n> > + * a minimum timeout value of 1s.\n> > + */\n> > + timeout = std::max(1s, 5 * maxFrameLengthMs * 1ms);\n> > + } else\n> > + timeout = config_.unicamTimeoutValue * 1ms;\n> >\n> > LOG(RPI, Debug) << \"Setting Unicam timeout to \" << timeout;\n> > unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> > --\n> > 2.25.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 A5228BDCBF\n\tfor ;\n\tFri, 24 Feb 2023 11:30:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2061962652;\n\tFri, 24 Feb 2023 12:30:24 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677238224;\n\tbh=/E6U2hwPX7Qsb65boTPPHuB7v+rgPmIIzWAcQPw0ZvQ=;\n\th=References:In-Reply-To:Date:To:List-Id:List-Post:From:Cc:\n\tList-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help:\n\tSubject:From;\n\tb=XbjdWSlyxPYoTYlemnVoRxDdy+JC+ERDnWEQuiUuElRM8oRGn/HI+6cy+MDt1Vy4D\n\t4E3xEUDKG+3z66nLEF/T46so4VAFlh017jDBTHsfIUbm+dX10DAf+JO+eIUqYypchg\n\t9womYyQYIxCFz1lHya7n0orafOGW8JmeDHCbknvkiSJXMRPW53ncmckM2NQOxXL2rz\n\tzvw+MkrFmB6Jlp+EKe0douGYl0yk56+bUhx8GA14WTQ4il+iEik3H9orMbHHkc8uX8\n\t9mYDNDEpkCfjUvCKPWjVTrtkwRGTMA/csTK+YuxemngVx4E+xnms7YskiXnl5FjMDB\n\t6cZZd5WUC73Jg==","References":"<20230223124957.11084-1-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","Date":"Fri, 24 Feb 2023 11:30:07 +0000","To":"David Plowman ","MIME-Version":"1.0","Message-ID":"","List-Id":"","List-Post":"","From":"Naushir Patuck via libcamera-devel\n\t","Precedence":"list","Cc":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","X-BeenThere":"libcamera-devel@lists.libcamera.org","List-Subscribe":",\n\t","List-Unsubscribe":",\n\t","List-Archive":"","Reply-To":"Naushir Patuck ","List-Help":"","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","Content-Type":"message/rfc822","Content-Disposition":"inline","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26517,"web_url":"https://patchwork.libcamera.org/comment/26517/","msgid":"<20230301144721.bohfg7egujasjbo3@uno.localdomain>","date":"2023-03-01T15:17:21","subject":"Re: [libcamera-devel] [PATCH v1 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, Feb 23, 2023 at 12:49:57PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Date: Thu, 23 Feb 2023 12:49:57 +0000\n> From: Naushir Patuck \n> To: libcamera-devel@lists.libcamera.org\n> Subject: [PATCH v1 3/3] pipeline: raspberrypi: Add a Unicam timeout\n> override config options\n> X-Mailer: git-send-email 2.25.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> ---\n> .../pipeline/raspberrypi/data/example.yaml | 11 ++++++-\n> .../pipeline/raspberrypi/raspberrypi.cpp | 29 ++++++++++++++-----\n> 2 files changed, 32 insertions(+), 8 deletions(-)\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..7c8c66129014 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> @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n>\n> \tdata->state_ = RPiCameraData::State::Idle;\n>\n> +\tif (data->config_.unicamTimeoutValue)\n> +\t\tdata->setCameraTimeout(data->config_.unicamTimeoutValue);\n> +\n> \t/* Start all streams. */\n> \tfor (auto const stream : data->streams_) {\n> \t\tret = stream->dev()->streamOn();\n> @@ -1715,6 +1723,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 +1759,8 @@ 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_.disableStartupFrameDrops);\n>\n> \tif (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> \t\tLOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";\n> @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)\n>\n> void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)\n> {\n> -\t/*\n> -\t * Set the dequeue timeout to the larger of 5x the maximum reported\n> -\t * frame length advertised by the IPA over a number of frames. Allow\n> -\t * a minimum timeout value of 1s.\n> -\t */\n> -\tutils::Duration timeout =\n> -\t\tstd::max(1s, 5 * maxFrameLengthMs * 1ms);\n> +\tutils::Duration timeout;\n> +\n> +\tif (!config_.unicamTimeoutValue) {\n\nI wonder, when the value comes from configuration file, can it change\nat run-time ? If it can't is there any purpose in emitting the signal,\nhandling it here and setting the same value on the video device ? If\nthat's the case, I would try to figure out early if unicamTimeoutValue\n!= 0 and if that's the case set the value on the video device and not\nconnected (or disconnect) the signal\n\n> +\t\t/*\n> +\t\t * Set the dequeue timeout to the larger of 5x the maximum reported\n> +\t\t * frame length advertised by the IPA over a number of frames. Allow\n> +\t\t * a minimum timeout value of 1s.\n> +\t\t */\n> +\t\ttimeout = std::max(1s, 5 * maxFrameLengthMs * 1ms);\n> +\t} else\n> +\t\ttimeout = config_.unicamTimeoutValue * 1ms;\n>\n> \tLOG(RPI, Debug) << \"Setting Unicam timeout to \" << timeout;\n> \tunicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> --\n> 2.25.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 5A166BE080\n\tfor ;\n\tWed, 1 Mar 2023 15:17:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 169A1626B9;\n\tWed, 1 Mar 2023 16:17:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DCE1962666\n\tfor ;\n\tWed, 1 Mar 2023 16:17:24 +0100 (CET)","from ideasonboard.com (mob-5-90-142-222.net.vodafone.it\n\t[5.90.142.222])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6AF83890;\n\tWed, 1 Mar 2023 16:17:24 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677683846;\n\tbh=6RMXcqLOHj1sOje1zYDrkyCHrEwVvGXB/+ndu/XRmfs=;\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=pTx5xm/AZpYl2YzMfwU/xfVeoZzgwyc/r1paaNm2eSljZV5uF4D2S2+aUshGz5Xoy\n\t+IAt4+dZI07x/nANPSJkooiJ5aY/zulCZo1dA6PpTjgw4hBd31xUWPEl2uX+dtmOS3\n\taXOKlwv0ugH1GoX6uIuygF5xx4G/T5FbqcA+267hEx3U+tFFqyNk1UIa3yFr0shfWn\n\tDWCbZWT+o0xwlwmCE1ZLk4SzziwsGizEp5Xx8MbQ7Ob2/K49UTFo1H3543uOkAUF2j\n\tsKCX7zgSJfNh3Ous2KYHsUwRuxK53EHvfU3I4DgaSwJ34swRJbTMGVI5GSVTXdxHuX\n\tj9BjedaxB3lgg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677683844;\n\tbh=6RMXcqLOHj1sOje1zYDrkyCHrEwVvGXB/+ndu/XRmfs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wbS5/hCxgjr9sxLuryo5CJy7V/HcMyq2Fu00Ol9I0H9MngYdQ+KrYpJQh2V2Qwf3i\n\tN9ElqwHhUocmABiOG7dsZ3OXIH+Qho5qF8RDiwN2B1b26WPz26bqmWzSQKbDVU4ww2\n\tfr9TZdGKoe6YHJHex/n9xHmqOrwOFC1New0jgPKU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"wbS5/hCx\"; dkim-atps=neutral","Date":"Wed, 1 Mar 2023 16:17:21 +0100","To":"Naushir Patuck ","Message-ID":"<20230301144721.bohfg7egujasjbo3@uno.localdomain>","References":"<20230223124957.11084-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 v1 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":26522,"web_url":"https://patchwork.libcamera.org/comment/26522/","msgid":"","date":"2023-03-02T08:31:29","subject":"Re: [libcamera-devel] [PATCH v1 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 Jacopo,\n\nThank you for the feedback!\n\nOn Wed, 1 Mar 2023 at 15:17, Jacopo Mondi wrote:\n>\n> Hi Naush\n>\n> On Thu, Feb 23, 2023 at 12:49:57PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > Date: Thu, 23 Feb 2023 12:49:57 +0000\n> > From: Naushir Patuck \n> > To: libcamera-devel@lists.libcamera.org\n> > Subject: [PATCH v1 3/3] pipeline: raspberrypi: Add a Unicam timeout\n> > override config options\n> > X-Mailer: git-send-email 2.25.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> > ---\n> > .../pipeline/raspberrypi/data/example.yaml | 11 ++++++-\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 29 ++++++++++++++-----\n> > 2 files changed, 32 insertions(+), 8 deletions(-)\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..7c8c66129014 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> > @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> >\n> > data->state_ = RPiCameraData::State::Idle;\n> >\n> > + if (data->config_.unicamTimeoutValue)\n> > + data->setCameraTimeout(data->config_.unicamTimeoutValue);\n> > +\n> > /* Start all streams. */\n> > for (auto const stream : data->streams_) {\n> > ret = stream->dev()->streamOn();\n> > @@ -1715,6 +1723,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 +1759,8 @@ 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_.disableStartupFrameDrops);\n> >\n> > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";\n> > @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)\n> >\n> > void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)\n> > {\n> > - /*\n> > - * Set the dequeue timeout to the larger of 5x the maximum reported\n> > - * frame length advertised by the IPA over a number of frames. Allow\n> > - * a minimum timeout value of 1s.\n> > - */\n> > - utils::Duration timeout =\n> > - std::max(1s, 5 * maxFrameLengthMs * 1ms);\n> > + utils::Duration timeout;\n> > +\n> > + if (!config_.unicamTimeoutValue) {\n>\n> I wonder, when the value comes from configuration file, can it change\n> at run-time ? If it can't is there any purpose in emitting the signal,\n> handling it here and setting the same value on the video device ? If\n> that's the case, I would try to figure out early if unicamTimeoutValue\n> != 0 and if that's the case set the value on the video device and not\n> connected (or disconnect) the signal\n\nGood point. If the value comes from the config file, we will never have a\nruntime change. In these cases, I can disconnect the signal.\n\nFor simplicity, I'll continue emitting the signal from the IPA, but presumably\nthat's ok if the handler is disconnected?\n\nRegards,\nNaush\n\n>\n> > + /*\n> > + * Set the dequeue timeout to the larger of 5x the maximum reported\n> > + * frame length advertised by the IPA over a number of frames. Allow\n> > + * a minimum timeout value of 1s.\n> > + */\n> > + timeout = std::max(1s, 5 * maxFrameLengthMs * 1ms);\n> > + } else\n> > + timeout = config_.unicamTimeoutValue * 1ms;\n> >\n> > LOG(RPI, Debug) << \"Setting Unicam timeout to \" << timeout;\n> > unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> > --\n> > 2.25.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 4BC27BF415\n\tfor ;\n\tThu, 2 Mar 2023 08:31:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9030626C0;\n\tThu, 2 Mar 2023 09:31:48 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677745908;\n\tbh=sT+DZvW7N2kBvZwW+3RwYiofoucrhfC2Bm0MzMrrfB8=;\n\th=References:In-Reply-To:Date:To:List-Id:List-Post:From:Cc:\n\tList-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help:\n\tSubject:From;\n\tb=ZGzNel0YL9SHubauxO3FIoI1uQDO3AJg2ICAE5FKZiwc4hFIe1OhIjhZV2DE8CU2z\n\tEKTaqSLxMMYi64DtZgqF/eHi+ctRa+vU1SxyT+0rMGJRIsHhS8bUOwCBHM3g477pxy\n\t4JWSsHvMXPm1YSfw+NkapEuzo9ABJjeex5o7prEVhvTrZE8DJQwFF4AZT+ZpSCTqwV\n\tgQsfVj9YdQY5M6c60pCZV+StPf5+rci7BH/NGVQf51VIG6FfCRLRq3TVrVNdpHo+PO\n\tXMNjcg2V/QQqZEFpVxk34WTmj2mQyXSBqSwJhOxvSkPYyJBFRw7KlECZp6NiH1Wybi\n\tY22/oP8XYdhiA==","References":"<20230223124957.11084-1-naush@raspberrypi.com>\n\t\n\t<20230301144721.bohfg7egujasjbo3@uno.localdomain>","In-Reply-To":"<20230301144721.bohfg7egujasjbo3@uno.localdomain>","Date":"Thu, 2 Mar 2023 08:31:29 +0000","To":"Jacopo Mondi ","MIME-Version":"1.0","Message-ID":"","List-Id":"","List-Post":"","From":"Naushir Patuck via libcamera-devel\n\t","Precedence":"list","Cc":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","X-BeenThere":"libcamera-devel@lists.libcamera.org","List-Subscribe":",\n\t","List-Unsubscribe":",\n\t","List-Archive":"","Reply-To":"Naushir Patuck ","List-Help":"","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] pipeline: raspberrypi: Add a\n\tUnicam timeout override config options","Content-Type":"message/rfc822","Content-Disposition":"inline","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]