[{"id":19525,"web_url":"https://patchwork.libcamera.org/comment/19525/","msgid":"<20210908003620.GA968527@pyrite.rasen.tech>","date":"2021-09-08T00:36:20","subject":"Re: [libcamera-devel] [PATCH v2 15/17] android: capabilities: Cap\n\tframe rate to 30 FPS","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Sep 07, 2021 at 09:41:05PM +0200, Jacopo Mondi wrote:\n> Limit the reported minumum frame duration to 30 FPS.\n> \n> The reason to do is to bring the libcamra HAL in par with the Intel\n> HAL implementation on IPU3 platform, where 30FPS is the frame rate used\n> to perform quality tuning in the closed-source IPA module and has been\n> validated as the most efficient rate for the power/performace budget.\n> \n> This change bring into the HAL a platform specific constraints, which\n> might be opportune for most platforms but should rather be configurable\n> by system integrators. Record that with a \\todo entry.\n> \n> Also record that, even if we report a lower frame rate, we currently\n> do not limit what the camera actually produce.\n\nHm, now I'm starting to wonder if we should cap the reported min frame\nduration in the HAL, but allow libcamera to go as fast as any user\nwants. But then we also need a way to actually set the frame duration in\nlibcamera...\n\nIf the HAL reports max 30fps, then the HAL will only request up to\n30fps, and then we can just set that on libcamera's side... that way we\nsatisfy the HAL requirements while keeping the flexibility on\nlibcamera's side.\n\nOr would it be better to limit libcamera's IPU3 as well, since 30fps has\nbeen confirmed to be the best? \"We know what the user wants more than\nthe user knows what they want\"\n\n\nPaul\n\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_capabilities.cpp | 23 +++++++++++++++++++++++\n> 1 file changed, 23 insertions(+)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index deaec3282fd7..cff1e30a9567 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -632,6 +632,29 @@ int CameraCapabilities::initializeStreamConfigurations()\n> \n> \t\t\tint64_t minFrameDuration = frameDurations->second.min().get() * 1000;\n> \t\t\tint64_t maxFrameDuration = frameDurations->second.max().get() * 1000;\n> +\n> +\t\t\t/*\n> +\t\t\t * Cap min frame duration to 30 FPS.\n> +\t\t\t *\n> +\t\t\t * 30 frames per second has been validated as the most\n> +\t\t\t * opportune frame rate for quality tuning, and power\n> +\t\t\t * vs performances budget on Intel IPU3.\n> +\t\t\t *\n> +\t\t\t * \\todo This is a platform-specific decision that needs\n> +\t\t\t * to be abstracted and delegated to the configuration\n> +\t\t\t * file.\n> +\t\t\t *\n> +\t\t\t * \\todo libcamera only allows to control frame duration\n> +\t\t\t * through the per-request controls::FrameDuration\n> +\t\t\t * control. If we cap the durations here, we should be\n> +\t\t\t * capable of configuring the camera to operate at such\n> +\t\t\t * duration without requiring to have the FrameDuration\n> +\t\t\t * control to be specified for each Request. Defer this\n> +\t\t\t * to the in-development configuration API rework.\n> +\t\t\t */\n> +\t\t\tif (minFrameDuration < 1e9 / 30.0)\n> +\t\t\t\tminFrameDuration = 1e9 / 30.0;\n> +\n> \t\t\tstreamConfigurations_.push_back({\n> \t\t\t\tres, androidFormat, minFrameDuration, maxFrameDuration,\n> \t\t\t});\n> -- \n> 2.32.0\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 C91ACBDC71\n\tfor ;\n\tWed, 8 Sep 2021 00:36:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B8A26916C;\n\tWed, 8 Sep 2021 02:36:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8411660137\n\tfor ;\n\tWed, 8 Sep 2021 02:36:28 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0586B317;\n\tWed, 8 Sep 2021 02:36:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JE8d8H3b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631061388;\n\tbh=O+/oQxFhlcgdphfOt+cRXIB2P8xrfbHz1AUDakxaYPk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JE8d8H3bm9n7/JzP3xhI0X5iEXNPeeubO3qMgtQv4dRYD9BfR7mQsnlqfG9UmGoHP\n\tMyJidSwFepDkTFlFKyDFcRs6QBkc92jzi7RduSzNYtCTsbHsMf0aS0FuR5xjgKu2qQ\n\tec6oUiSpJemzdcuYCUwkZyafELkaous1nnW2G3oA=","Date":"Wed, 8 Sep 2021 09:36:20 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20210908003620.GA968527@pyrite.rasen.tech>","References":"<20210907194107.803730-1-jacopo@jmondi.org>\n\t<20210907194107.803730-16-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210907194107.803730-16-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 15/17] android: capabilities: Cap\n\tframe rate to 30 FPS","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20086,"web_url":"https://patchwork.libcamera.org/comment/20086/","msgid":"","date":"2021-10-06T01:53:57","subject":"Re: [libcamera-devel] [PATCH v2 15/17] android: capabilities: Cap\n\tframe rate to 30 FPS","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Wed, Sep 08, 2021 at 09:36:20AM +0900, paul.elder@ideasonboard.com wrote:\n> On Tue, Sep 07, 2021 at 09:41:05PM +0200, Jacopo Mondi wrote:\n> > Limit the reported minumum frame duration to 30 FPS.\n> > \n> > The reason to do is to bring the libcamra HAL in par with the Intel\n> > HAL implementation on IPU3 platform, where 30FPS is the frame rate used\n> > to perform quality tuning in the closed-source IPA module and has been\n> > validated as the most efficient rate for the power/performace budget.\n> > \n> > This change bring into the HAL a platform specific constraints, which\n> > might be opportune for most platforms but should rather be configurable\n> > by system integrators. Record that with a \\todo entry.\n> > \n> > Also record that, even if we report a lower frame rate, we currently\n> > do not limit what the camera actually produce.\n> \n> Hm, now I'm starting to wonder if we should cap the reported min frame\n> duration in the HAL, but allow libcamera to go as fast as any user\n> wants. But then we also need a way to actually set the frame duration in\n> libcamera...\n> \n> If the HAL reports max 30fps, then the HAL will only request up to\n> 30fps, and then we can just set that on libcamera's side... that way we\n> satisfy the HAL requirements while keeping the flexibility on\n> libcamera's side.\n> \n> Or would it be better to limit libcamera's IPU3 as well, since 30fps has\n> been confirmed to be the best? \"We know what the user wants more than\n> the user knows what they want\"\n\nI'd rather go with this patch, I don't think libcamera should limit the\nframe rate artifically on IPU3.\n\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_capabilities.cpp | 23 +++++++++++++++++++++++\n> > 1 file changed, 23 insertions(+)\n> > \n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index deaec3282fd7..cff1e30a9567 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -632,6 +632,29 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > \n> > \t\t\tint64_t minFrameDuration = frameDurations->second.min().get() * 1000;\n> > \t\t\tint64_t maxFrameDuration = frameDurations->second.max().get() * 1000;\n> > +\n> > +\t\t\t/*\n> > +\t\t\t * Cap min frame duration to 30 FPS.\n> > +\t\t\t *\n> > +\t\t\t * 30 frames per second has been validated as the most\n> > +\t\t\t * opportune frame rate for quality tuning, and power\n> > +\t\t\t * vs performances budget on Intel IPU3.\n\nI'd say \"on Intel IPU3-based Chromebooks\", as this budget decision is\ndevice-specific.\n\nReviewed-by: Laurent Pinchart \n\n> > +\t\t\t *\n> > +\t\t\t * \\todo This is a platform-specific decision that needs\n> > +\t\t\t * to be abstracted and delegated to the configuration\n> > +\t\t\t * file.\n> > +\t\t\t *\n> > +\t\t\t * \\todo libcamera only allows to control frame duration\n> > +\t\t\t * through the per-request controls::FrameDuration\n> > +\t\t\t * control. If we cap the durations here, we should be\n> > +\t\t\t * capable of configuring the camera to operate at such\n> > +\t\t\t * duration without requiring to have the FrameDuration\n> > +\t\t\t * control to be specified for each Request. Defer this\n> > +\t\t\t * to the in-development configuration API rework.\n> > +\t\t\t */\n> > +\t\t\tif (minFrameDuration < 1e9 / 30.0)\n> > +\t\t\t\tminFrameDuration = 1e9 / 30.0;\n> > +\n> > \t\t\tstreamConfigurations_.push_back({\n> > \t\t\t\tres, androidFormat, minFrameDuration, maxFrameDuration,\n> > \t\t\t});","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 379E4C3243\n\tfor ;\n\tWed, 6 Oct 2021 01:54:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3856691B7;\n\tWed, 6 Oct 2021 03:54:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B6E046012B\n\tfor ;\n\tWed, 6 Oct 2021 03:54:05 +0200 (CEST)","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 2E8E3581;\n\tWed, 6 Oct 2021 03:54:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qxcraYu8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1633485245;\n\tbh=iRw8mxuFvRZ8XJwKG3Zcl+bOtxvCDsI/T8pY9yq2ZTk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qxcraYu8ZfsKjQjq8OSJnD+gQJ7rsfGtpBR7Ao6DzJsUTxcVpKeMafvIdRcffmG/7\n\tiRT01uMe2yFwIMo+uIVpINDgB+lBgX8hzqGBOUbftMqCoKoEGFpJciNJQj0KkRfq3K\n\tDJBIo7bshovRjXfuitR7KHlrGx3j4pSfSjNHY0R0=","Date":"Wed, 6 Oct 2021 04:53:57 +0300","From":"Laurent Pinchart ","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20210907194107.803730-1-jacopo@jmondi.org>\n\t<20210907194107.803730-16-jacopo@jmondi.org>\n\t<20210908003620.GA968527@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210908003620.GA968527@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v2 15/17] android: capabilities: Cap\n\tframe rate to 30 FPS","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20092,"web_url":"https://patchwork.libcamera.org/comment/20092/","msgid":"<20211006082619.z75zysk5quwvuf4k@uno.localdomain>","date":"2021-10-06T08:26:19","subject":"Re: [libcamera-devel] [PATCH v2 15/17] android: capabilities: Cap\n\tframe rate to 30 FPS","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Wed, Oct 06, 2021 at 04:53:57AM +0300, Laurent Pinchart wrote:\n> Hello,\n>\n> On Wed, Sep 08, 2021 at 09:36:20AM +0900, paul.elder@ideasonboard.com wrote:\n> > On Tue, Sep 07, 2021 at 09:41:05PM +0200, Jacopo Mondi wrote:\n> > > Limit the reported minumum frame duration to 30 FPS.\n> > >\n> > > The reason to do is to bring the libcamra HAL in par with the Intel\n> > > HAL implementation on IPU3 platform, where 30FPS is the frame rate used\n> > > to perform quality tuning in the closed-source IPA module and has been\n> > > validated as the most efficient rate for the power/performace budget.\n> > >\n> > > This change bring into the HAL a platform specific constraints, which\n> > > might be opportune for most platforms but should rather be configurable\n> > > by system integrators. Record that with a \\todo entry.\n> > >\n> > > Also record that, even if we report a lower frame rate, we currently\n> > > do not limit what the camera actually produce.\n> >\n> > Hm, now I'm starting to wonder if we should cap the reported min frame\n> > duration in the HAL, but allow libcamera to go as fast as any user\n> > wants. But then we also need a way to actually set the frame duration in\n> > libcamera...\n> >\n> > If the HAL reports max 30fps, then the HAL will only request up to\n> > 30fps, and then we can just set that on libcamera's side... that way we\n> > satisfy the HAL requirements while keeping the flexibility on\n> > libcamera's side.\n> >\n\nThe problem here is that we have currently no way to 'limit' the frame\nrate to 30FPS if not issuing a FrameDuration in each request.\n\nIdeally we should be able to set the desired duration as part of the\nStreamConfiguration as we do for the stream size and format.\n\n> > Or would it be better to limit libcamera's IPU3 as well, since 30fps has\n> > been confirmed to be the best? \"We know what the user wants more than\n> > the user knows what they want\"\n>\n> I'd rather go with this patch, I don't think libcamera should limit the\n> frame rate artifically on IPU3.\n\nBe aware that wihtout limiting the actual duration to what we report,\nwe'll have tests failing due to a mismatch in the reported and\ncalculated durations.\n\nThanks\n j\n\n\n>\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/android/camera_capabilities.cpp | 23 +++++++++++++++++++++++\n> > > 1 file changed, 23 insertions(+)\n> > >\n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index deaec3282fd7..cff1e30a9567 100644\n> > > --- a/src/android/camera_capabilities.cpp\n> > > +++ b/src/android/camera_capabilities.cpp\n> > > @@ -632,6 +632,29 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > >\n> > > \t\t\tint64_t minFrameDuration = frameDurations->second.min().get() * 1000;\n> > > \t\t\tint64_t maxFrameDuration = frameDurations->second.max().get() * 1000;\n> > > +\n> > > +\t\t\t/*\n> > > +\t\t\t * Cap min frame duration to 30 FPS.\n> > > +\t\t\t *\n> > > +\t\t\t * 30 frames per second has been validated as the most\n> > > +\t\t\t * opportune frame rate for quality tuning, and power\n> > > +\t\t\t * vs performances budget on Intel IPU3.\n>\n> I'd say \"on Intel IPU3-based Chromebooks\", as this budget decision is\n> device-specific.\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > > +\t\t\t *\n> > > +\t\t\t * \\todo This is a platform-specific decision that needs\n> > > +\t\t\t * to be abstracted and delegated to the configuration\n> > > +\t\t\t * file.\n> > > +\t\t\t *\n> > > +\t\t\t * \\todo libcamera only allows to control frame duration\n> > > +\t\t\t * through the per-request controls::FrameDuration\n> > > +\t\t\t * control. If we cap the durations here, we should be\n> > > +\t\t\t * capable of configuring the camera to operate at such\n> > > +\t\t\t * duration without requiring to have the FrameDuration\n> > > +\t\t\t * control to be specified for each Request. Defer this\n> > > +\t\t\t * to the in-development configuration API rework.\n> > > +\t\t\t */\n> > > +\t\t\tif (minFrameDuration < 1e9 / 30.0)\n> > > +\t\t\t\tminFrameDuration = 1e9 / 30.0;\n> > > +\n> > > \t\t\tstreamConfigurations_.push_back({\n> > > \t\t\t\tres, androidFormat, minFrameDuration, maxFrameDuration,\n> > > \t\t\t});\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 4F016BDC71\n\tfor ;\n\tWed, 6 Oct 2021 08:25:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D62D7691B6;\n\tWed, 6 Oct 2021 10:25:33 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 85035691AB\n\tfor ;\n\tWed, 6 Oct 2021 10:25:32 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id C196C100007;\n\tWed, 6 Oct 2021 08:25:31 +0000 (UTC)"],"Date":"Wed, 6 Oct 2021 10:26:19 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20211006082619.z75zysk5quwvuf4k@uno.localdomain>","References":"<20210907194107.803730-1-jacopo@jmondi.org>\n\t<20210907194107.803730-16-jacopo@jmondi.org>\n\t<20210908003620.GA968527@pyrite.rasen.tech>\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 15/17] android: capabilities: Cap\n\tframe rate to 30 FPS","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]