[{"id":22806,"web_url":"https://patchwork.libcamera.org/comment/22806/","msgid":"<765138ed-2f85-28eb-83de-134fab94c859@ideasonboard.com>","date":"2022-04-27T19:10:51","subject":"Re: [libcamera-devel] [PATCH v3] android: camera_capabilities:\n\tAdjust minimum frame duration to match FPS","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi\n\nOn 4/27/22 20:43, 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\nIt seems(to me at-least) that the placement of the + 0.05f adjustment is \nchanged as per this patch.\n\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\nHence we are not introducing any 'new adjustments' rather changing the \nplacement of the adjustment.\n\nGiven my inference is correct (of course I might be missing the entire \npoint here, please do correct me\nthen), I would re-pharse this commit message  to reflect that this patch \ndoesn't introduce any functional\nchanges.\n\n     android: camera_capabilities: Move minimum frame duration adjustment\n\n     CTS calculates FPS with a rounding formula: See\nCamera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n\n     fps = floor(1e9 / minFrameDuration + 0.05f)\n\n     The android adapter reports it as the AE target FPS. The patch\n     moves this adjustment while populating the stream configuration's\n     minimum frame duration itself.\n\nWhat do you think ?\n\n>\n> Signed-off-by: Han-Lin Chen \n> Reviewed-by: Jacopo Mondi \n> ---\n> src/android/camera_capabilities.cpp | 16 +++++++++++-----\n> 1 file changed, 11 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 55d651f3..ee17cdc7 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -687,6 +687,14 @@ 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 and adjust the minimum\n> +\t\t\t * frame duration accordingly: see\n> +\t\t\t * Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n> +\t\t\t */\n> +\t\t\tminFrameDuration =\n> +\t\t\t\t1e9 / static_cast(floor(1e9 / minFrameDuration + 0.05f));\n> +\n> \t\t\tstreamConfigurations_.push_back({\n> \t\t\t\tres, androidFormat, minFrameDuration, maxFrameDuration,\n> \t\t\t});\n> @@ -1287,12 +1295,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\n> -\t\t\t\t (floor(1e9 / entry.minFrameDurationNsec + 0.05f));\n> +\t\tunsigned int fps =\n> +\t\t\tstatic_cast(floor(1e9 / entry.minFrameDurationNsec));\n> +\n> \t\tif (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)\n> \t\t\tcontinue;\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 23A4FC0F2A\n\tfor ;\n\tWed, 27 Apr 2022 19:11:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8066A65644;\n\tWed, 27 Apr 2022 21:11:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DD7846042F\n\tfor ;\n\tWed, 27 Apr 2022 21:11:01 +0200 (CEST)","from [IPV6:2401:4900:1f3f:280b:bf23:2859:bcb2:4d82] (unknown\n\t[IPv6:2401:4900:1f3f:280b:bf23:2859:bcb2:4d82])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8509A30B;\n\tWed, 27 Apr 2022 21:11:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651086663;\n\tbh=8YDhzIIoohFeS0QY/OjKR+UUtAXjOeDoUyKTFAF0zSU=;\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:\n\tFrom;\n\tb=C5pLNmbcZ2erpuDyX5BtzXsgG4nmIQPBhMcp7UtvLdrH4wkujsw35sEA6ydhneZ8j\n\t4QhsOky4O6vXdvuElTKPirE6+YwtWvb7KGRHMLJ71km3obNvabHhlQ/meFHHoc0fsO\n\tLmMJ1xLZiBsfTvbrSWb40bLGf68SRU8YVONk4xDN/fIrDTMWq5mNlqGREPwjxnUHPb\n\tzP1fcat5wKmgP8JrRdnI5h38b0mVSiWPdKjfFmGmIGv54ySOzUbDqCuBCbwlXfisQB\n\tB6HEgrzqc1tErHKsCG9k1fkLni8Ub+hbI+m+4mhLOKOOxvYDEbmzGexLv0PPDHLOlJ\n\tt6wb/mduQtejg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651086661;\n\tbh=8YDhzIIoohFeS0QY/OjKR+UUtAXjOeDoUyKTFAF0zSU=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=kEjvBHjcRMC4+rveKu8FWewjnmnxyrCVAsInst84TXRYmCwn28mDjAiHXXL0CuKdT\n\t1Lsvu4icDEYeyQ5qAKdQzjGUpnvzKiH07kqeKveLOxITrvf9UblxwSTmfxZDOeks6B\n\tLc6qpJfkbX9N0S7JRUmOwdMVqVZoHbh3jejJC9oE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"kEjvBHjc\"; dkim-atps=neutral","Message-ID":"<765138ed-2f85-28eb-83de-134fab94c859@ideasonboard.com>","Date":"Thu, 28 Apr 2022 00:40:51 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Han-Lin Chen ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220427151334.609152-1-hanlinchen@chromium.org>","In-Reply-To":"<20220427151334.609152-1-hanlinchen@chromium.org>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22807,"web_url":"https://patchwork.libcamera.org/comment/22807/","msgid":"<075ff7d2-25f1-5136-d7fb-181d67653b40@ideasonboard.com>","date":"2022-04-28T07:28:30","subject":"Re: [libcamera-devel] [PATCH v3] android: camera_capabilities:\n\tAdjust minimum frame duration to match FPS","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Han-Lin,\n\nOn 4/28/22 00:40, Umang Jain via libcamera-devel wrote:\n> Hi\n>\n> On 4/27/22 20:43, 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>\n> It seems(to me at-least) that the placement of the + 0.05f adjustment \n> is changed as per this patch.\n>\n>>\n>> The android adapter reports it as the AE target FPS. The patch \n>> adjusts the\n>> reported minimum frame duration to match the reported FPS.\n>\n>\n> Hence we are not introducing any 'new adjustments' rather changing the \n> placement of the adjustment.\n\n\nBeing clarified by Jacopo on IRC, that the patch makes the FPS and \nminFrameDuration in-sync, such that\nthe adjustment is now reflected for both. That makes perfect sense now \nthat I am looking, hence,\n\nReviewed-by: Umang Jain \n\n>\n> Given my inference is correct (of course I might be missing the entire \n> point here, please do correct me\n> then), I would re-pharse this commit message  to reflect that this \n> patch doesn't introduce any functional\n> changes.\n>\n>     android: camera_capabilities: Move minimum frame duration adjustment\n>\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\n>     moves this adjustment while populating the stream configuration's\n>     minimum frame duration itself.\n>\n> What do you think ?\n>\n>>\n>> Signed-off-by: Han-Lin Chen \n>> Reviewed-by: Jacopo Mondi \n>> ---\n>>   src/android/camera_capabilities.cpp | 16 +++++++++++-----\n>>   1 file changed, 11 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/src/android/camera_capabilities.cpp \n>> b/src/android/camera_capabilities.cpp\n>> index 55d651f3..ee17cdc7 100644\n>> --- a/src/android/camera_capabilities.cpp\n>> +++ b/src/android/camera_capabilities.cpp\n>> @@ -687,6 +687,14 @@ int \n>> CameraCapabilities::initializeStreamConfigurations()\n>>                       minFrameDuration = minFrameDurationCap;\n>>               }\n>>   +            /*\n>> +             * Calculate FPS as CTS does and adjust the minimum\n>> +             * frame duration accordingly: see\n>> +             * \n>> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n>> +             */\n>> +            minFrameDuration =\n>> +                1e9 / static_cast(floor(1e9 / \n>> minFrameDuration + 0.05f));\n>> +\n>>               streamConfigurations_.push_back({\n>>                   res, androidFormat, minFrameDuration, \n>> maxFrameDuration,\n>>               });\n>> @@ -1287,12 +1295,10 @@ int \n>> CameraCapabilities::initializeStaticMetadata()\n>>            * recording profile. Inspecting the Intel IPU3 HAL\n>>            * implementation confirms this but no reference has been \n>> found\n>>            * in the metadata documentation.\n>> -         *\n>> -         * Calculate FPS as CTS does: see\n>> -         * \n>> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()\n>>            */\n>> -        unsigned int fps = static_cast\n>> -                   (floor(1e9 / entry.minFrameDurationNsec + 0.05f));\n>> +        unsigned int fps =\n>> +            static_cast(floor(1e9 / \n>> entry.minFrameDurationNsec));\n>> +\n>>           if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)\n>>               continue;","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 5AE82C0F2A\n\tfor ;\n\tThu, 28 Apr 2022 07:28:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C82565644;\n\tThu, 28 Apr 2022 09:28:37 +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 DF0BB6563E\n\tfor ;\n\tThu, 28 Apr 2022 09:28:35 +0200 (CEST)","from [IPV6:2401:4900:1f3f:280b:bf23:2859:bcb2:4d82] (unknown\n\t[IPv6:2401:4900:1f3f:280b:bf23:2859:bcb2:4d82])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A1DC3496;\n\tThu, 28 Apr 2022 09:28:34 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1651130917;\n\tbh=vvx0o07SDPSbaH5cK2L2sjPvAbWHjZ2PtLnzgViES8I=;\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:\n\tFrom;\n\tb=aggu2rR6YiRAqxJRLORFed/WpiIsv87ahrRg55kV1IMciqSaf7OPsvWFLauOghyhS\n\tEL7QIAnRI9jPpf8H7lYuaxvZEzitdKgEGCQHlk8n6jvAJ+NQfRxDhB2gIGKEMcFI1v\n\tucAWV+9Vinlc6HvCTfE/IGPGz2uRy7gefnRS86NYb7QlXhkCTAAkN+QaI79fr1Q41n\n\twnLQ8z+FWWGwstjZCJpwxm7nKDl6IWAXUiC/xJVw2gEOD0JCL+KjxXVQBemXCg3k9Z\n\ty5ccPekj7N3qs9i3RD+Kax0JmGr6nU4KZp141qg7TZFnaVemanWHSv/P5EZpnWRJsn\n\t3LMwZr3gOCwEw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1651130915;\n\tbh=vvx0o07SDPSbaH5cK2L2sjPvAbWHjZ2PtLnzgViES8I=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=UaYTM65r8zCpcOV6p3sY+vl4VigyvDyZq8uy9oPMjchtMEQMus1+Ig+SHBGwAWC63\n\tBcoM4NVhI9xtQf8koqb0jst9LE82N+UTkAhZMb9Qk81+8sKm5RqbCe+isbF7ukn2NJ\n\tPKbxsWSNLb9wIwmb4TYVdp8YKxiDsb87ukd0rbt8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UaYTM65r\"; dkim-atps=neutral","Message-ID":"<075ff7d2-25f1-5136-d7fb-181d67653b40@ideasonboard.com>","Date":"Thu, 28 Apr 2022 12:58:30 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Han-Lin Chen ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220427151334.609152-1-hanlinchen@chromium.org>\n\t<765138ed-2f85-28eb-83de-134fab94c859@ideasonboard.com>","In-Reply-To":"<765138ed-2f85-28eb-83de-134fab94c859@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]