[{"id":19251,"web_url":"https://patchwork.libcamera.org/comment/19251/","msgid":"<20210901082315.GV968527@pyrite.rasen.tech>","date":"2021-09-01T08:23:15","subject":"Re: [libcamera-devel] [PATCH 14/16] 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 Fri, Aug 27, 2021 at 02:07:55PM +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> Signed-off-by: Jacopo Mondi \n\nIt looks like this patch is reverted in the next patch...?\n\n\nPaul\n\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 01cdbcf1395d..6890bfecab29 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 CA8FEBDC71\n\tfor ;\n\tWed, 1 Sep 2021 08:23:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3765160255;\n\tWed, 1 Sep 2021 10:23:25 +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 D553860253\n\tfor ;\n\tWed, 1 Sep 2021 10:23:22 +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 58FAC3D7;\n\tWed, 1 Sep 2021 10:23:21 +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=\"eGWkf9Sv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630484602;\n\tbh=jWl4p7fMcbsZ9uPZ1ky9TDHxvXROShJIi2FkMi1AGG4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eGWkf9SvStkfHkNAc4cAegkdgnAY9xQ+5aEEex0jwMKtG4+zjlnpnfP1HG6jBpmwZ\n\tjEkygNcyT8K57xFP++mxiJFjwj8gMweSAPjawH4z7uyikIdgoifigy/xWvpGIlChnu\n\t3+UnppsyrCmjboW6F2yXDa3vZ0Q2p8RoULoBNMgU=","Date":"Wed, 1 Sep 2021 17:23:15 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20210901082315.GV968527@pyrite.rasen.tech>","References":"<20210827120757.110615-1-jacopo@jmondi.org>\n\t<20210827120757.110615-15-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210827120757.110615-15-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 14/16] 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":19252,"web_url":"https://patchwork.libcamera.org/comment/19252/","msgid":"<20210901082931.fs55v3mj5qyvq2xq@uno.localdomain>","date":"2021-09-01T08:29:31","subject":"Re: [libcamera-devel] [PATCH 14/16] 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, Sep 01, 2021 at 05:23:15PM +0900, paul.elder@ideasonboard.com wrote:\n> Hi Jacopo,\n>\n> On Fri, Aug 27, 2021 at 02:07:55PM +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> > Signed-off-by: Jacopo Mondi \n>\n> It looks like this patch is reverted in the next patch...?\n>\n\nYes, the reason is reported in the cover letter\n\n>\n> Paul\n>\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 01cdbcf1395d..6890bfecab29 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 70FD9BD87D\n\tfor ;\n\tWed, 1 Sep 2021 08:28:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6D2B6916A;\n\tWed, 1 Sep 2021 10:28:42 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1EAE960253\n\tfor ;\n\tWed, 1 Sep 2021 10:28:42 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 920F01BF20B;\n\tWed, 1 Sep 2021 08:28:41 +0000 (UTC)"],"Date":"Wed, 1 Sep 2021 10:29:31 +0200","From":"Jacopo Mondi ","To":"paul.elder@ideasonboard.com","Message-ID":"<20210901082931.fs55v3mj5qyvq2xq@uno.localdomain>","References":"<20210827120757.110615-1-jacopo@jmondi.org>\n\t<20210827120757.110615-15-jacopo@jmondi.org>\n\t<20210901082315.GV968527@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210901082315.GV968527@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH 14/16] 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":19254,"web_url":"https://patchwork.libcamera.org/comment/19254/","msgid":"<20210901084036.GX968527@pyrite.rasen.tech>","date":"2021-09-01T08:40:36","subject":"Re: [libcamera-devel] [PATCH 14/16] 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 Wed, Sep 01, 2021 at 10:29:31AM +0200, Jacopo Mondi wrote:\n> On Wed, Sep 01, 2021 at 05:23:15PM +0900, paul.elder@ideasonboard.com wrote:\n> > Hi Jacopo,\n> >\n> > On Fri, Aug 27, 2021 at 02:07:55PM +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> > > Signed-off-by: Jacopo Mondi \n> >\n> > It looks like this patch is reverted in the next patch...?\n> >\n> \n> Yes, the reason is reported in the cover letter\n\nOh oops, I read that yesterday and forgot about it :p\n\nI vote for the IPA solution.\n\n\nPaul\n\n> >\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 01cdbcf1395d..6890bfecab29 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 1A0CFBD87D\n\tfor ;\n\tWed, 1 Sep 2021 08:40:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B94F6916A;\n\tWed, 1 Sep 2021 10:40:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA64A60253\n\tfor ;\n\tWed, 1 Sep 2021 10:40:43 +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 5DDEE3D7;\n\tWed, 1 Sep 2021 10:40:42 +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=\"mVlOkCH+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630485643;\n\tbh=l8Tmic4sIhbrQO+7MfQw0o90/lMFlzyHM9C7rWXIo9E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mVlOkCH+/2EXxzq+0J/IfL8fko/8M1wg5RoOxaZZRTT2qvMc6pgNZ5jJjF8AgxHDC\n\tI+sdtvrcz+TZ8SUpK8qHXQ3yBuM/xiSXgcOrF4GZow+xbNGP3YO3ACNV5O7MDawh+D\n\tm3bC59edcQCA7BES9H8gHXnFtaryioDL6aBLcGSA=","Date":"Wed, 1 Sep 2021 17:40:36 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20210901084036.GX968527@pyrite.rasen.tech>","References":"<20210827120757.110615-1-jacopo@jmondi.org>\n\t<20210827120757.110615-15-jacopo@jmondi.org>\n\t<20210901082315.GV968527@pyrite.rasen.tech>\n\t<20210901082931.fs55v3mj5qyvq2xq@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210901082931.fs55v3mj5qyvq2xq@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 14/16] 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\" "}}]