[{"id":26473,"web_url":"https://patchwork.libcamera.org/comment/26473/","msgid":"","date":"2023-02-24T11:04:10","subject":"Re: [libcamera-devel] [PATCH v1 1/3] pipeline: ipa: raspberrypi:\n\tChange Unicam timeout handling","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 an explicit helper function setCameraTimeout() in the pipeline\n> handler to set the Unicam timeout value. This function is signalled from\n> the IPA to set up an appropriate timeout. This replaces the\n> maxSensorFrameLengthMs value parameter returned back from\n> IPARPi::start().\n>\n> Adjust the timeout to be 5x the maximum frame duration reported by the\n> IPA.\n\n(I guess you could have added \"or 1s (whichever is longer)\", but I\nreally wouldn't bother resubmitting just for that!!)\n\n>\n> Signed-off-by: Naushir Patuck \n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 2 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-\n> .../pipeline/raspberrypi/raspberrypi.cpp | 24 ++++++++++++-------\n> 3 files changed, 18 insertions(+), 10 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index 8e78f167f179..80e0126618c8 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -49,7 +49,6 @@ struct IPAConfigResult {\n> struct StartConfig {\n> libcamera.ControlList controls;\n> int32 dropFrameCount;\n> - uint32 maxSensorFrameLengthMs;\n> };\n>\n> interface IPARPiInterface {\n> @@ -132,4 +131,5 @@ interface IPARPiEventInterface {\n> setIspControls(libcamera.ControlList controls);\n> setDelayedControls(libcamera.ControlList controls, uint32 delayContext);\n> setLensControls(libcamera.ControlList controls);\n> + setCameraTimeout(uint32 maxFrameLengthMs);\n> };\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 9b08ae4ca622..f6826bf27fe1 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -341,7 +341,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig)\n>\n> startConfig->dropFrameCount = dropFrameCount_;\n> const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;\n> - startConfig->maxSensorFrameLengthMs = maxSensorFrameDuration.get();\n> + setCameraTimeout.emit(maxSensorFrameDuration.get());\n>\n> firstStart_ = false;\n> lastRunTimestamp_ = 0;\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 841209548350..3d04842a2440 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -212,6 +212,7 @@ public:\n> void setIspControls(const ControlList &controls);\n> void setDelayedControls(const ControlList &controls, uint32_t delayContext);\n> void setLensControls(const ControlList &controls);\n> + void setCameraTimeout(uint32_t maxExposureTimeMs);\n> void setSensorControls(ControlList &controls);\n> void unicamTimeout();\n>\n> @@ -1166,14 +1167,6 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> }\n> }\n>\n> - /*\n> - * Set the dequeue timeout to the larger of 2x the maximum possible\n> - * frame duration or 1 second.\n> - */\n> - utils::Duration timeout =\n> - std::max(1s, 2 * startConfig.maxSensorFrameLengthMs * 1ms);\n> - data->unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> -\n> return 0;\n> }\n>\n> @@ -1645,6 +1638,7 @@ int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)\n> ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n> ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n> ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls);\n> + ipa_->setCameraTimeout.connect(this, &RPiCameraData::setCameraTimeout);\n>\n> /*\n> * The configuration (tuning file) is made from the sensor name unless\n> @@ -1957,6 +1951,20 @@ void RPiCameraData::setLensControls(const ControlList &controls)\n> }\n> }\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> +\n> + LOG(RPI, Debug) << \"Setting Unicam timeout to \" << timeout;\n> + unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> +}\n> +\n> void RPiCameraData::setSensorControls(ControlList &controls)\n> {\n> /*\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 962B4BE080\n\tfor ;\n\tFri, 24 Feb 2023 11:04:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04E9262646;\n\tFri, 24 Feb 2023 12:04:25 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677236665;\n\tbh=FaW2FtFNMyFPA8Aotp3lq1HeIhr6NpKLhHNurleiLZw=;\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=2w59vuWtz+Byq1rUPd/5v/8aHSuYENt5DheK+oDmn9qShmm7NeU/xDshaePvA8U8x\n\t64MSK9tpwCSXSnT6vgsCK42e1N2+HSS4QPo8EMnDeidDnR4uwtkxr05T2o8iwj2E67\n\tcmRbgHY/WWQc6TgNOU3EupEABF9NBUGs8j3B0jzJLr6kkMZFWuoMc7wNrxpAsd9x2V\n\tIQSEKFcwzf8CPROM7LywQqdmAM3tp9Ob/lw9U+5c0cdRrcoxEABQdF+sFSqiW8qWJ8\n\t64pfnYxpJ2uQVDgga8D180VhwJ7hSqnTKPLldwmCPhT4cyEZVT6K6Ey5LkGHTTfNj/\n\tbJxMW+TFyZ4lw==","References":"<20230223124957.11084-1-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Fri, 24 Feb 2023 11:04:10 +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 1/3] pipeline: ipa: raspberrypi:\n\tChange Unicam timeout handling","Content-Type":"message/rfc822","Content-Disposition":"inline","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26515,"web_url":"https://patchwork.libcamera.org/comment/26515/","msgid":"<20230301151322.u75d7htda36buijy@uno.localdomain>","date":"2023-03-01T15:16:50","subject":"Re: [libcamera-devel] [PATCH v1 1/3] pipeline: ipa: raspberrypi:\n\tChange Unicam timeout handling","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:55PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Date: Thu, 23 Feb 2023 12:49:55 +0000\n> From: Naushir Patuck \n> To: libcamera-devel@lists.libcamera.org\n> Subject: [PATCH v1 1/3] pipeline: ipa: raspberrypi: Change Unicam timeout\n> handling\n> X-Mailer: git-send-email 2.25.1\n>\n> Add an explicit helper function setCameraTimeout() in the pipeline\n> handler to set the Unicam timeout value. This function is signalled from\n> the IPA to set up an appropriate timeout. This replaces the\n> maxSensorFrameLengthMs value parameter returned back from\n> IPARPi::start().\n>\n> Adjust the timeout to be 5x the maximum frame duration reported by the\n> IPA.\n>\n> Signed-off-by: Naushir Patuck \n\nReviewed-by: Jacopo Mondi \n\nThanks\n\n j\n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 2 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-\n> .../pipeline/raspberrypi/raspberrypi.cpp | 24 ++++++++++++-------\n> 3 files changed, 18 insertions(+), 10 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index 8e78f167f179..80e0126618c8 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -49,7 +49,6 @@ struct IPAConfigResult {\n> struct StartConfig {\n> \tlibcamera.ControlList controls;\n> \tint32 dropFrameCount;\n> -\tuint32 maxSensorFrameLengthMs;\n> };\n>\n> interface IPARPiInterface {\n> @@ -132,4 +131,5 @@ interface IPARPiEventInterface {\n> \tsetIspControls(libcamera.ControlList controls);\n> \tsetDelayedControls(libcamera.ControlList controls, uint32 delayContext);\n> \tsetLensControls(libcamera.ControlList controls);\n> +\tsetCameraTimeout(uint32 maxFrameLengthMs);\n> };\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 9b08ae4ca622..f6826bf27fe1 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -341,7 +341,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig)\n>\n> \tstartConfig->dropFrameCount = dropFrameCount_;\n> \tconst Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;\n> -\tstartConfig->maxSensorFrameLengthMs = maxSensorFrameDuration.get();\n> +\tsetCameraTimeout.emit(maxSensorFrameDuration.get());\n>\n> \tfirstStart_ = false;\n> \tlastRunTimestamp_ = 0;\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 841209548350..3d04842a2440 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -212,6 +212,7 @@ public:\n> \tvoid setIspControls(const ControlList &controls);\n> \tvoid setDelayedControls(const ControlList &controls, uint32_t delayContext);\n> \tvoid setLensControls(const ControlList &controls);\n> +\tvoid setCameraTimeout(uint32_t maxExposureTimeMs);\n> \tvoid setSensorControls(ControlList &controls);\n> \tvoid unicamTimeout();\n>\n> @@ -1166,14 +1167,6 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> \t\t}\n> \t}\n>\n> -\t/*\n> -\t * Set the dequeue timeout to the larger of 2x the maximum possible\n> -\t * frame duration or 1 second.\n> -\t */\n> -\tutils::Duration timeout =\n> -\t\tstd::max(1s, 2 * startConfig.maxSensorFrameLengthMs * 1ms);\n> -\tdata->unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> -\n> \treturn 0;\n> }\n>\n> @@ -1645,6 +1638,7 @@ int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)\n> \tipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n> \tipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n> \tipa_->setLensControls.connect(this, &RPiCameraData::setLensControls);\n> +\tipa_->setCameraTimeout.connect(this, &RPiCameraData::setCameraTimeout);\n>\n> \t/*\n> \t * The configuration (tuning file) is made from the sensor name unless\n> @@ -1957,6 +1951,20 @@ void RPiCameraData::setLensControls(const ControlList &controls)\n> \t}\n> }\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> +\n> +\tLOG(RPI, Debug) << \"Setting Unicam timeout to \" << timeout;\n> +\tunicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);\n> +}\n> +\n> void RPiCameraData::setSensorControls(ControlList &controls)\n> {\n> \t/*\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 AB9EABF415\n\tfor ;\n\tWed, 1 Mar 2023 15:16:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F7D362666;\n\tWed, 1 Mar 2023 16:16: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 3198562665\n\tfor ;\n\tWed, 1 Mar 2023 16:16:54 +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 A8B9C890;\n\tWed, 1 Mar 2023 16:16:53 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677683816;\n\tbh=FWIYF5AYbdGFQ/iWO/c2MC1Nn0rj75MGP3SKWk5eJOg=;\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=gYQP2o+zqB4xEpacRkzIsO9/FP8rGcEWWDlSN+cyoszDPysTUopm86wQVowzp31tu\n\tCvMAPyAUWalINSp3n2PiMWzF1Ts9/RT0BDq0zeQKNjiKWoX8jlDiWBhLlym0ZmnbkD\n\tQFtM/WURzQJrhdzq65VBPMB8kdTUz914fpHssYI+k+t993JpzUMUA4hcKWA8KhJyFF\n\tCSd07wMPbS2sybmgYYaBL0eQs+ww0YT8q2GZ6u80QAM1cogb6inK0GP+bmfChKM1lE\n\tG90FuMmdP28rsfxu1BFtQuc8lj7SaP5w1mhCOdZ6jaAtALAeJZxXcnl088ht6fOJ+D\n\tCDvM8PULkGnQQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677683813;\n\tbh=FWIYF5AYbdGFQ/iWO/c2MC1Nn0rj75MGP3SKWk5eJOg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i2nhVDcK/EC+aF/GZoRk1lLe+Xwn9sAcXB01jRMFPNqqt7qEP9F4jUaRF91m+qR6L\n\t8ZbMQ+DucyHxo3N0xfTT3PVJHJgSq0in2b7EGeKETmXu+m6UF9l1bV1b6XHlI7mwR/\n\tq4gJA8Nrfr5/cBk5D9qB0olclelgQX1cdShp9xSg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"i2nhVDcK\"; dkim-atps=neutral","Date":"Wed, 1 Mar 2023 16:16:50 +0100","To":"Naushir Patuck ","Message-ID":"<20230301151322.u75d7htda36buijy@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 1/3] pipeline: ipa: raspberrypi:\n\tChange Unicam timeout handling","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\" "}}]