[{"id":22795,"web_url":"https://patchwork.libcamera.org/comment/22795/","msgid":"<20220426143525.6exm5bzuav5ottvt@uno.localdomain>","date":"2022-04-26T14:35:25","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_capabilities:\n\tAdjust minimum frame duration to match FPS","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Han-Lin\n\nOn Tue, Apr 26, 2022 at 07:43:30PM +0800, Han-Lin Chen via libcamera-devel wrote:\n> CTS calculates FPS with a rounding formula: See\n> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n>\n> fps = floor(1e9 / minFrameDuration + 0.05f)\n>\n> The android adapter reports it as the AE target FPS. The patch adjusts the\n> reported minimum frame duration to match the reported FPS.\n>\n> The requirement comes from ChromeOS which only allows the stream configuration\n> with the minimum frame duration achieves the target FPS.\n>\n> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> ---\n>  src/android/camera_capabilities.cpp | 23 ++++++++++++++++++-----\n>  1 file changed, 18 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 55d651f3..5242055c 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -687,6 +687,21 @@ int CameraCapabilities::initializeStreamConfigurations()\n>  \t\t\t\t\tminFrameDuration = minFrameDurationCap;\n>  \t\t\t}\n>\n> +\t\t\t/*\n> +\t\t\t * Calculate FPS as CTS does: see\n> +\t\t\t * Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n> +\t\t\t */\n> +\t\t\tunsigned int fps =\n> +\t\t\t\tstatic_cast<unsigned int>(floor(1e9 / minFrameDuration + 0.05f));\n> +\n> +\t\t\t/*\n> +\t\t\t * Adjust the minimum frame duration to match the\n> +\t\t\t * calculated FPS.The requirement comes from ChromeOS\n                                         ^ space\n\n> +\t\t\t * which only allows the stream configuration with the\n> +\t\t\t * minimum frame duration achieves the target FPS.\n\nTo be honest I would drop the last phrase. We adjust the min frame\nduration to match the just calculated fps, regardless of the platform\nthat requires it.\n\nAnyway, can be adjusted when applying eventually\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n> +\t\t\t */\n> +\t\t\tminFrameDuration = 1e9 / fps;\n> +\n>  \t\t\tstreamConfigurations_.push_back({\n>  \t\t\t\tres, androidFormat, minFrameDuration, maxFrameDuration,\n>  \t\t\t});\n> @@ -1287,12 +1302,10 @@ int CameraCapabilities::initializeStaticMetadata()\n>  \t\t * recording profile. Inspecting the Intel IPU3 HAL\n>  \t\t * implementation confirms this but no reference has been found\n>  \t\t * in the metadata documentation.\n> -\t\t *\n> -\t\t * Calculate FPS as CTS does: see\n> -\t\t * Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n>  \t\t */\n> -\t\tunsigned int fps = static_cast<unsigned int>\n> -\t\t\t\t   (floor(1e9 / entry.minFrameDurationNsec + 0.05f));\n> +\t\tunsigned int fps =\n> +\t\t\tstatic_cast<unsigned int>(floor(1e9 / entry.minFrameDurationNsec));\n> +\n>  \t\tif (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)\n>  \t\t\tcontinue;\n>\n> --\n> 2.36.0.rc2.479.g8af0fa9b8e-goog\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 9B3EFC0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Apr 2022 14:35:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D4B0D65642;\n\tTue, 26 Apr 2022 16:35:29 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E78060431\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Apr 2022 16:35:28 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 9DD66100006;\n\tTue, 26 Apr 2022 14:35:27 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1650983729;\n\tbh=Vaot36hUTbeN7exTBgLYiO9Mx+3oWfp/vIuCEedubes=;\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=jlrS6FWTn3aveuNnBHVdbEgxbrYGR7HB93IUzUKYANoH8Jhg9UqtCgDtN+CYy8YgE\n\t81kQntZcPX3VCSsEoGXy3Se5sSk5XicwE2TAhNH7jflCnSy4TjlO1jn1O5nEZw75yA\n\tBTkTEgrKUsXnLFefl6TMueX0E5j+U1TrQNNzodPeEkiCLsXR6CrIJzZqwiAkJh5aCG\n\tBMdd/0qDg1sjA7dOiwEAQuoVhW+cpQmGAIOVc16HobO4t6qa+gzrjnFRK8M1F6fjU8\n\tJMpsxev9ytnQ7pRJxWPZWUIPriDdChCaqKh9i0kd3UgUoqPn1IYF0XGD9WrGmsocOm\n\tozbPr8bepfinQ==","Date":"Tue, 26 Apr 2022 16:35:25 +0200","To":"Han-Lin Chen <hanlinchen@chromium.org>","Message-ID":"<20220426143525.6exm5bzuav5ottvt@uno.localdomain>","References":"<20220426114330.675768-1-hanlinchen@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220426114330.675768-1-hanlinchen@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_capabilities:\n\tAdjust minimum frame duration to match FPS","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22805,"web_url":"https://patchwork.libcamera.org/comment/22805/","msgid":"<165106956682.4076486.9648439362357038769@Monstersaurus>","date":"2022-04-27T14:26:06","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_capabilities:\n\tAdjust minimum frame duration to match FPS","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Han-Lin Chen via libcamera-devel (2022-04-26 12:43:30)\n> CTS calculates FPS with a rounding formula: See\n> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n> \n> fps = floor(1e9 / minFrameDuration + 0.05f)\n> \n> The android adapter reports it as the AE target FPS. The patch adjusts the\n> reported minimum frame duration to match the reported FPS.\n> \n> The requirement comes from ChromeOS which only allows the stream configuration\n> with the minimum frame duration achieves the target FPS.\n> \n> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> ---\n>  src/android/camera_capabilities.cpp | 23 ++++++++++++++++++-----\n>  1 file changed, 18 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 55d651f3..5242055c 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -687,6 +687,21 @@ int CameraCapabilities::initializeStreamConfigurations()\n>                                         minFrameDuration = minFrameDurationCap;\n>                         }\n>  \n> +                       /*\n> +                        * Calculate FPS as CTS does: see\n> +                        * Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n> +                        */\n> +                       unsigned int fps =\n> +                               static_cast<unsigned int>(floor(1e9 / minFrameDuration + 0.05f));\n> +\n> +                       /*\n> +                        * Adjust the minimum frame duration to match the\n> +                        * calculated FPS.The requirement comes from ChromeOS\n> +                        * which only allows the stream configuration with the\n> +                        * minimum frame duration achieves the target FPS.\n\nThis doesn't quite make sense, (perhaps it was supposed to be 'with the\nminimum frame duration which achieves the target FPS') but I think Jacopo\nsaid it may be dropped in his review so I won't worry too much about it.\n\n\n> +                        */\n> +                       minFrameDuration = 1e9 / fps;\n\n\nCan this calculation be simplified so that we operate only on the\nminFrameDuration, and not calculate the FPS from the minFrameDuration,\nonly to then convert the FPS back to a minFrameDuration?\n\n\n> +\n>                         streamConfigurations_.push_back({\n>                                 res, androidFormat, minFrameDuration, maxFrameDuration,\n>                         });\n> @@ -1287,12 +1302,10 @@ int CameraCapabilities::initializeStaticMetadata()\n>                  * recording profile. Inspecting the Intel IPU3 HAL\n>                  * implementation confirms this but no reference has been found\n>                  * in the metadata documentation.\n> -                *\n> -                * Calculate FPS as CTS does: see\n> -                * Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n>                  */\n> -               unsigned int fps = static_cast<unsigned int>\n> -                                  (floor(1e9 / entry.minFrameDurationNsec + 0.05f));\n> +               unsigned int fps =\n> +                       static_cast<unsigned int>(floor(1e9 / entry.minFrameDurationNsec));\n> +\n>                 if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)\n>                         continue;\n>  \n> -- \n> 2.36.0.rc2.479.g8af0fa9b8e-goog\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 0937DC0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Apr 2022 14:26:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B3A66563F;\n\tWed, 27 Apr 2022 16:26:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 50B7F6042F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Apr 2022 16:26:09 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BED3130B;\n\tWed, 27 Apr 2022 16:26:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651069571;\n\tbh=G5IRRhUPb7LhnCN4XBuY9InilwCOhXuzzX9PYKuY6zo=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=rjGO3ZOhbKCFg4/KSZ5QXblOd6Iu/BMZotTF9sQAx6nYKwpEwBCzdiVT7A08gh5Zn\n\tx98wnrBp4bY+EhD/AShEWE+W2lfWqdPgGHKw3i3zlKPFaijkwRcwFe3pfexUHrpeCD\n\tCGPrp1G2dK4c+LutaBw3EMaibGGpKkufBJPibx8I7mZORcyl3fRvbtegOVXKDXQW2t\n\tJ4SVUUM/LAroa7Xc7gSEjuMPZVRcZdIaG12NbvT7JGbk7XbBFaHw9J68v0ktqZerne\n\tEymby9t4loZIX8GbKnKI3sewYBxsjdnRITcGLD+VdAQq3FFMk9lPOKLsU6XIBMzIpD\n\tjSx1NWygcjnnw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651069568;\n\tbh=G5IRRhUPb7LhnCN4XBuY9InilwCOhXuzzX9PYKuY6zo=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=UK8og5KLsPWCaPMlbeC4pojD03JUQuYd9CzFAu6zgNTIwLlZF/3pg6xHYjBzfXFzk\n\t3jiTF55jgn+ZCn2BGYaw6ODUlDHqcnyS4sl2D5HChcBslzsxOlBH8lov7yWLPr+Rja\n\tLYbHQHBQlaxBFOjFpeLSTt8DAYeAV7EEpmNm7QP0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UK8og5KL\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220426114330.675768-1-hanlinchen@chromium.org>","References":"<20220426114330.675768-1-hanlinchen@chromium.org>","To":"Han-Lin Chen <hanlinchen@chromium.org>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 27 Apr 2022 15:26:06 +0100","Message-ID":"<165106956682.4076486.9648439362357038769@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_capabilities:\n\tAdjust minimum frame duration to match FPS","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]