[{"id":17433,"web_url":"https://patchwork.libcamera.org/comment/17433/","msgid":"<20210607062432.GG156622@pyrite.rasen.tech>","date":"2021-06-07T06:24:32","subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:\n> Signed-off-by: Umang Jain \n> ---\n> \n> This patch is for reference and discussion purposes only.\n> Please DO NOT MERGE. If anything, there will be follow up\n> separate patches for merge separately.\n\nThank you for the investigation and starting the discussion.\n\n> \n> Problem:\n> On nautilus, 26 tests were failing because of \"Fail to open camera\",\n> but more specifically from adb logcat:\n> \n> ```\n> E Camera2-Parameters: generated preview size list is empty!!\n> E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)\n> E CameraService: connectHelper: Could not initialize client from HAL.\n> I Camera2Client: Camera 0: Closed\n> ```\n> \n> was found to be root of the problem. The checks triggered are here:\n> > Parameters::getFilteredSizes()\n> https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976\n> \n> nautilus reports higher frame-duration to start with: 34100000\n\niirc this was the minimum frame duration. This converts to 29.32 fps.\n\n> (in CameraDevice::getStaticMetadata()).\n> minFrameDurationNsec is meant to be re-adjusted down the line \n> before round-up, *if* the difference is < 500 useconds.\n\nAnd the minimum frame duration gets compared to what android requires as\nthe maximum value of the minimum frame duration, which is 33366700ns =\n29.97 fps.\n\n> \n> On nautilus, since the difference was much larger than 500 useconds,\n\nWhat this adjustment does is if the minimum frame duration (= max fps)\nsupported by the sensor is greater than the maximum allowed value (= the\nsupported max fps is lower than the max requirement), but it's within\n500us, we can adjust it.\n\n> the re-adjustment failed and libcamera reported a much higher\n> frame-duration to upper-layers/libcameraservice. \n\n(Yes, it was rounded down instead to 29 fps and the corresponding\n34482758ns, which is less/greater than 29.97fps / 33366700ns.)\n\n> This led to the problem in Parameters::getFilteredSizes(),\n> where all potential streams for preview are skipped to be added,\n> due to high minFrameDuration.\n> \n> To force this re-adjustment, the scope of difference was increased\n> to 1200 useconds as done in the patch.\n\nSo by allowing 1200us, what we're saying is that we can still reasonably\nsay that we support a minimum frame duration of 33366700ns when the real\nminimum that we support is 34566700ns. It's a whole 1.2ms off per frame.\nIs this a reasonable assertion?\n\n> \n> CTS Results on nautilus after applying the changes:\n> Total Run time: 8m 42s\n> 1/1 modules completed\n> Total Tests : 221\n> PASSED : 221\n> FAILED : 0\n> \n> ---\n> src/android/camera_device.cpp | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index fe332ec3..d0676a7f 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\t * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service\n> \t\t * implementation).\n> \t\t *\n> -\t\t * If we're close enough (+ 500 useconds) to that value, round\n> +\t\t * If we're close enough (+ 1200 useconds) to that value, round\n> \t\t * the minimum frame duration of the camera to an accepted\n> \t\t * value.\n> \t\t */\n> \t\tstatic constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;\n> \t\tif (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&\n> -\t\t minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)\n> +\t\t minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)\n> \t\t\tminFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;\n> \n> \t\t/*\n> @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\tANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,\n> \t\tANDROID_SCALER_AVAILABLE_STALL_DURATIONS,\n> \t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,\n> +\t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,\n\nWhat's this for?\n\n\nThanks,\n\nPaul\n\n> \t\tANDROID_SCALER_CROPPING_TYPE,\n> \t\tANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,\n> \t\tANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n> -- \n> 2.31.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 AB609C3206\n\tfor ;\n\tMon, 7 Jun 2021 06:24:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 01B6868928;\n\tMon, 7 Jun 2021 08:24:43 +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 866946029E\n\tfor ;\n\tMon, 7 Jun 2021 08:24:41 +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 B25EF80F;\n\tMon, 7 Jun 2021 08:24:39 +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=\"qT9lVeRR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623047081;\n\tbh=ayUp/c2ZaNA+9roS1zVCn0MGwMrtH05I2eCuwxeIAbE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qT9lVeRRM6qx9vHuMqw40gu0jPy8wGwDiAuCvsWcBXFytZbBZdGE4XLA6Uc7pb+Nj\n\tn966iO8nW1ByPd1kXHp3oeEKb5Uq9W67I89CB0/CyA4gx7wrqbQh/bOLxdSy+AZB5F\n\tmxsJSwq2KPxW+bHXC6aecf7FRrKCyB/GamSdp6Mg=","Date":"Mon, 7 Jun 2021 15:24:32 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain ","Message-ID":"<20210607062432.GG156622@pyrite.rasen.tech>","References":"<20210604102606.218464-1-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210604102606.218464-1-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","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":17464,"web_url":"https://patchwork.libcamera.org/comment/17464/","msgid":"<027818f5-9fb3-30a8-3ae3-5ea6f53abef4@ideasonboard.com>","date":"2021-06-08T05:38:00","subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nThanks for pitching in.\n\nOn 6/7/21 11:54 AM, paul.elder@ideasonboard.com wrote:\n> Hi Umang,\n>\n> On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:\n>> Signed-off-by: Umang Jain \n>> ---\n>>\n>> This patch is for reference and discussion purposes only.\n>> Please DO NOT MERGE. If anything, there will be follow up\n>> separate patches for merge separately.\n> Thank you for the investigation and starting the discussion.\n>\n>> Problem:\n>> On nautilus, 26 tests were failing because of \"Fail to open camera\",\n>> but more specifically from adb logcat:\n>>\n>> ```\n>> E Camera2-Parameters: generated preview size list is empty!!\n>> E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)\n>> E CameraService: connectHelper: Could not initialize client from HAL.\n>> I Camera2Client: Camera 0: Closed\n>> ```\n>>\n>> was found to be root of the problem. The checks triggered are here:\n>>> Parameters::getFilteredSizes()\n>> https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976\n>>\n>> nautilus reports higher frame-duration to start with: 34100000\n> iirc this was the minimum frame duration. This converts to 29.32 fps.\n>\n>> (in CameraDevice::getStaticMetadata()).\n>> minFrameDurationNsec is meant to be re-adjusted down the line\n>> before round-up, *if* the difference is < 500 useconds.\n> And the minimum frame duration gets compared to what android requires as\n> the maximum value of the minimum frame duration, which is 33366700ns =\n> 29.97 fps.\n>\n>> On nautilus, since the difference was much larger than 500 useconds,\n> What this adjustment does is if the minimum frame duration (= max fps)\n> supported by the sensor is greater than the maximum allowed value (= the\n> supported max fps is lower than the max requirement), but it's within\n> 500us, we can adjust it.\n>\n>> the re-adjustment failed and libcamera reported a much higher\n>> frame-duration to upper-layers/libcameraservice.\n> (Yes, it was rounded down instead to 29 fps and the corresponding\n> 34482758ns, which is less/greater than 29.97fps / 33366700ns.)\n>\n>> This led to the problem in Parameters::getFilteredSizes(),\n>> where all potential streams for preview are skipped to be added,\n>> due to high minFrameDuration.\n>>\n>> To force this re-adjustment, the scope of difference was increased\n>> to 1200 useconds as done in the patch.\n> So by allowing 1200us, what we're saying is that we can still reasonably\n> say that we support a minimum frame duration of 33366700ns when the real\n> minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.\n> Is this a reasonable assertion?\nIndeed, that's what is in question. I plan to discuss this in today's \nweekly.\n>\n>> CTS Results on nautilus after applying the changes:\n>> Total Run time: 8m 42s\n>> 1/1 modules completed\n>> Total Tests : 221\n>> PASSED : 221\n>> FAILED : 0\n>>\n>> ---\n>> src/android/camera_device.cpp | 5 +++--\n>> 1 file changed, 3 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index fe332ec3..d0676a7f 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>> \t\t * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service\n>> \t\t * implementation).\n>> \t\t *\n>> -\t\t * If we're close enough (+ 500 useconds) to that value, round\n>> +\t\t * If we're close enough (+ 1200 useconds) to that value, round\n>> \t\t * the minimum frame duration of the camera to an accepted\n>> \t\t * value.\n>> \t\t */\n>> \t\tstatic constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;\n>> \t\tif (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&\n>> -\t\t minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)\n>> +\t\t minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)\n>> \t\t\tminFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;\n>> \n>> \t\t/*\n>> @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>> \t\tANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,\n>> \t\tANDROID_SCALER_AVAILABLE_STALL_DURATIONS,\n>> \t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,\n>> +\t\tANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,\n> What's this for?\n\n\nThe \nhttps://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976\ncheck , reads in ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT \nkey, and I realized we don't report it hence, I thought it's a similar case\nas 51c09236c4e8 \n \n(\"android: Make FRAME_DURATION result key available in static metadata\")\n\n\nHowever this morning, I ran CTS without\n > +ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT\nand it ran just fine. Maybe I don't understand keys-related magic not \nwell-enough.\n\n\n>\n>\n> Thanks,\n>\n> Paul\n>\n>> \t\tANDROID_SCALER_CROPPING_TYPE,\n>> \t\tANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,\n>> \t\tANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n>> -- \n>> 2.31.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 E7A31BD22E\n\tfor ;\n\tTue, 8 Jun 2021 05:38:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A2206892F;\n\tTue, 8 Jun 2021 07:38:07 +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 3E11D6029D\n\tfor ;\n\tTue, 8 Jun 2021 07:38:06 +0200 (CEST)","from [192.168.0.107] (unknown [103.251.226.98])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9EA293E6;\n\tTue, 8 Jun 2021 07:38:04 +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=\"BSUe2amg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623130685;\n\tbh=HEH8yx6Jeqh+XOvbxB1sg5QDDWhqu84IcZ7NB4v8h8Y=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=BSUe2amg5S/igd/tq0mqzQm4rTasjRm/Rd+N/mg+dNpTGd4T/irAaXGWofH4T+H2W\n\tcgX2ptHEXkir3jxCagYQtB7eG0T9J1rxiTGI1Bqn353SaBsMsEd0gCLaBy3cBJbjOZ\n\tp5Iug/4YuapxrT8HErHEPXi3JP1cgVW2KMjORM+M=","To":"paul.elder@ideasonboard.com","References":"<20210604102606.218464-1-umang.jain@ideasonboard.com>\n\t<20210607062432.GG156622@pyrite.rasen.tech>","From":"Umang Jain ","Message-ID":"<027818f5-9fb3-30a8-3ae3-5ea6f53abef4@ideasonboard.com>","Date":"Tue, 8 Jun 2021 11:08:00 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210607062432.GG156622@pyrite.rasen.tech>","Content-Type":"multipart/alternative;\n\tboundary=\"------------5F959BE8FA2613DCA0EBDF44\"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","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":17467,"web_url":"https://patchwork.libcamera.org/comment/17467/","msgid":"<20210608062458.GJ156622@pyrite.rasen.tech>","date":"2021-06-08T06:24:58","subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Tue, Jun 08, 2021 at 11:08:00AM +0530, Umang Jain wrote:\n> Hi Paul,\n> \n> Thanks for pitching in.\n> \n> On 6/7/21 11:54 AM, paul.elder@ideasonboard.com wrote:\n> \n> Hi Umang,\n> \n> On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:\n> \n> Signed-off-by: Umang Jain \n> ---\n> \n> This patch is for reference and discussion purposes only.\n> Please DO NOT MERGE. If anything, there will be follow up\n> separate patches for merge separately.\n> \n> Thank you for the investigation and starting the discussion.\n> \n> \n> Problem:\n> On nautilus, 26 tests were failing because of \"Fail to open camera\",\n> but more specifically from adb logcat:\n> \n> ```\n> E Camera2-Parameters: generated preview size list is empty!!\n> E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)\n> E CameraService: connectHelper: Could not initialize client from HAL.\n> I Camera2Client: Camera 0: Closed\n> ```\n> \n> was found to be root of the problem. The checks triggered are here:\n> \n> Parameters::getFilteredSizes()\n> \n> https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976\n> \n> nautilus reports higher frame-duration to start with: 34100000\n> \n> iirc this was the minimum frame duration. This converts to 29.32 fps.\n> \n> \n> (in CameraDevice::getStaticMetadata()).\n> minFrameDurationNsec is meant to be re-adjusted down the line\n> before round-up, *if* the difference is < 500 useconds.\n> \n> And the minimum frame duration gets compared to what android requires as\n> the maximum value of the minimum frame duration, which is 33366700ns =\n> 29.97 fps.\n> \n> \n> On nautilus, since the difference was much larger than 500 useconds,\n> \n> What this adjustment does is if the minimum frame duration (= max fps)\n> supported by the sensor is greater than the maximum allowed value (= the\n> supported max fps is lower than the max requirement), but it's within\n> 500us, we can adjust it.\n> \n> \n> the re-adjustment failed and libcamera reported a much higher\n> frame-duration to upper-layers/libcameraservice.\n> \n> (Yes, it was rounded down instead to 29 fps and the corresponding\n> 34482758ns, which is less/greater than 29.97fps / 33366700ns.)\n> \n> \n> This led to the problem in Parameters::getFilteredSizes(),\n> where all potential streams for preview are skipped to be added,\n> due to high minFrameDuration.\n> \n> To force this re-adjustment, the scope of difference was increased\n> to 1200 useconds as done in the patch.\n> \n> So by allowing 1200us, what we're saying is that we can still reasonably\n> say that we support a minimum frame duration of 33366700ns when the real\n> minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.\n> Is this a reasonable assertion?\n> \n> Indeed, that's what is in question. I plan to discuss this in today's weekly.\n> \n> \n> \n> CTS Results on nautilus after applying the changes:\n> Total Run time: 8m 42s\n> 1/1 modules completed\n> Total Tests : 221\n> PASSED : 221\n> FAILED : 0\n> \n> ---\n> src/android/camera_device.cpp | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index fe332ec3..d0676a7f 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service\n> * implementation).\n> *\n> - * If we're close enough (+ 500 useconds) to that value, round\n> + * If we're close enough (+ 1200 useconds) to that value, round\n> * the minimum frame duration of the camera to an accepted\n> * value.\n> */\n> static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;\n> if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&\n> - minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)\n> + minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)\n> minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;\n> \n> /*\n> @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,\n> ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,\n> ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,\n> + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,\n> \n> What's this for?\n> \n> \n> The https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master\n> /services/camera/libcameraservice/api1/client2/Parameters.cpp#2976\n> check , reads in ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT key, and\n> I realized we don't report it hence, I thought it's a similar case\n> as 51c09236c4e8 (\"android: Make FRAME_DURATION result key available in static\n> metadata\")\n\nI'm pretty sure that section corresponds to [1], so it should indeed be\nfine without declaring the key.\n\n[1] https://git.linuxtv.org/libcamera.git/tree/src/android/camera_device.cpp#n1215\n\n\nPaul\n\n> \n> \n> However this morning, I ran CTS without\n> > +ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT\n> and it ran just fine. Maybe I don't understand keys-related magic not\n> well-enough.\n> \n> \n> \n> ANDROID_SCALER_CROPPING_TYPE,\n> ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,\n> ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n> --\n> 2.31.0\n> \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 C1148C3206\n\tfor ;\n\tTue, 8 Jun 2021 06:25:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B6DC68926;\n\tTue, 8 Jun 2021 08:25:08 +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 B688B6029D\n\tfor ;\n\tTue, 8 Jun 2021 08:25:06 +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 D778D3E6;\n\tTue, 8 Jun 2021 08:25:04 +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=\"KFDF3+wO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623133506;\n\tbh=uZQp09Dea6jNmyMpsHDYBGf0OAt9Xjqk4wJGPOlcPUM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KFDF3+wOUVv0mdUrelD3C2+/WhDNFblcaC+rOf5AZt6DxrXyPsGpWiETQ/zRC2VMq\n\tMrMHkcTIAvQdmDF6SlqkLILpOiFEAlSHri45NcdEuwuAFT+rxh+mF8N24G6tSnodLi\n\tNLkWHiIa0N2w96884eP81WY8cldqPsWAHdwsjgZ4=","Date":"Tue, 8 Jun 2021 15:24:58 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain ","Message-ID":"<20210608062458.GJ156622@pyrite.rasen.tech>","References":"<20210604102606.218464-1-umang.jain@ideasonboard.com>\n\t<20210607062432.GG156622@pyrite.rasen.tech>\n\t<027818f5-9fb3-30a8-3ae3-5ea6f53abef4@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<027818f5-9fb3-30a8-3ae3-5ea6f53abef4@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","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":17468,"web_url":"https://patchwork.libcamera.org/comment/17468/","msgid":"<20210608073818.dnerzj2bkmebjl3q@uno.localdomain>","date":"2021-06-08T07:38:18","subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello Umang, Paul\n\nOn Tue, Jun 08, 2021 at 03:24:58PM +0900, paul.elder@ideasonboard.com wrote:\n> Hi Umang,\n>\n> On Tue, Jun 08, 2021 at 11:08:00AM +0530, Umang Jain wrote:\n> > Hi Paul,\n> >\n> > Thanks for pitching in.\n> >\n> > On 6/7/21 11:54 AM, paul.elder@ideasonboard.com wrote:\n> >\n> > Hi Umang,\n> >\n> > On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:\n> >\n> > Signed-off-by: Umang Jain \n> > ---\n> >\n> > This patch is for reference and discussion purposes only.\n> > Please DO NOT MERGE. If anything, there will be follow up\n> > separate patches for merge separately.\n> >\n> > Thank you for the investigation and starting the discussion.\n> >\n> >\n> > Problem:\n> > On nautilus, 26 tests were failing because of \"Fail to open camera\",\n> > but more specifically from adb logcat:\n> >\n> > ```\n> > E Camera2-Parameters: generated preview size list is empty!!\n> > E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)\n> > E CameraService: connectHelper: Could not initialize client from HAL.\n> > I Camera2Client: Camera 0: Closed\n> > ```\n> >\n> > was found to be root of the problem. The checks triggered are here:\n> >\n> > Parameters::getFilteredSizes()\n> >\n> > https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976\n> >\n> > nautilus reports higher frame-duration to start with: 34100000\n> >\n> > iirc this was the minimum frame duration. This converts to 29.32 fps.\n> >\n> >\n> > (in CameraDevice::getStaticMetadata()).\n> > minFrameDurationNsec is meant to be re-adjusted down the line\n> > before round-up, *if* the difference is < 500 useconds.\n> >\n> > And the minimum frame duration gets compared to what android requires as\n> > the maximum value of the minimum frame duration, which is 33366700ns =\n> > 29.97 fps.\n> >\n> >\n> > On nautilus, since the difference was much larger than 500 useconds,\n> >\n> > What this adjustment does is if the minimum frame duration (= max fps)\n> > supported by the sensor is greater than the maximum allowed value (= the\n> > supported max fps is lower than the max requirement), but it's within\n> > 500us, we can adjust it.\n> >\n> >\n> > the re-adjustment failed and libcamera reported a much higher\n> > frame-duration to upper-layers/libcameraservice.\n> >\n> > (Yes, it was rounded down instead to 29 fps and the corresponding\n> > 34482758ns, which is less/greater than 29.97fps / 33366700ns.)\n> >\n> >\n> > This led to the problem in Parameters::getFilteredSizes(),\n> > where all potential streams for preview are skipped to be added,\n> > due to high minFrameDuration.\n> >\n> > To force this re-adjustment, the scope of difference was increased\n> > to 1200 useconds as done in the patch.\n> >\n> > So by allowing 1200us, what we're saying is that we can still reasonably\n> > say that we support a minimum frame duration of 33366700ns when the real\n> > minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.\n> > Is this a reasonable assertion?\n> >\n> > Indeed, that's what is in question. I plan to discuss this in today's weekly.\n> >\n> >\n> >\n> > CTS Results on nautilus after applying the changes:\n> > Total Run time: 8m 42s\n> > 1/1 modules completed\n> > Total Tests : 221\n> > PASSED : 221\n> > FAILED : 0\n> >\n> > ---\n> > src/android/camera_device.cpp | 5 +++--\n> > 1 file changed, 3 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index fe332ec3..d0676a7f 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> > * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service\n> > * implementation).\n> > *\n> > - * If we're close enough (+ 500 useconds) to that value, round\n> > + * If we're close enough (+ 1200 useconds) to that value, round\n> > * the minimum frame duration of the camera to an accepted\n> > * value.\n> > */\n> > static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;\n> > if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&\n> > - minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)\n> > + minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)\n> > minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;\n> >\n> > /*\n> > @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> > ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,\n> > ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,\n> > ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,\n> > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,\n> >\n> > What's this for?\n> >\n> >\n> > The https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master\n> > /services/camera/libcameraservice/api1/client2/Parameters.cpp#2976\n> > check , reads in ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT key, and\n> > I realized we don't report it hence, I thought it's a similar case\n> > as 51c09236c4e8 (\"android: Make FRAME_DURATION result key available in static\n> > metadata\")\n>\n> I'm pretty sure that section corresponds to [1], so it should indeed be\n> fine without declaring the key.\n>\n> [1] https://git.linuxtv.org/libcamera.git/tree/src/android/camera_device.cpp#n1215\n\nMore than that, ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT\n(and the ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_INPUT\ncouterpart) are flags that are expected to be used to populate the 4th\nfield of each stream confiuration reported as available through\nANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, which aims to report\nthe stream direction (output == produced by the camera, input ==\nprovided by application for reprocessing).\n\nIt should indeed not be added to the list of available characteristics\nkeys.\n\nThanks\n j\n\n>\n>\n> Paul\n>\n> >\n> >\n> > However this morning, I ran CTS without\n> > > +ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT\n> > and it ran just fine. Maybe I don't understand keys-related magic not\n> > well-enough.\n> >\n> >\n> >\n> > ANDROID_SCALER_CROPPING_TYPE,\n> > ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,\n> > ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n> > --\n> > 2.31.0\n> >\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 34984BD22E\n\tfor ;\n\tTue, 8 Jun 2021 07:37:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4C8F6892B;\n\tTue, 8 Jun 2021 09:37:31 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 98FD668928\n\tfor ;\n\tTue, 8 Jun 2021 09:37:29 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id DDED2FF807;\n\tTue, 8 Jun 2021 07:37:28 +0000 (UTC)"],"Date":"Tue, 8 Jun 2021 09:38:18 +0200","From":"Jacopo Mondi ","To":"paul.elder@ideasonboard.com","Message-ID":"<20210608073818.dnerzj2bkmebjl3q@uno.localdomain>","References":"<20210604102606.218464-1-umang.jain@ideasonboard.com>\n\t<20210607062432.GG156622@pyrite.rasen.tech>\n\t<027818f5-9fb3-30a8-3ae3-5ea6f53abef4@ideasonboard.com>\n\t<20210608062458.GJ156622@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210608062458.GJ156622@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH] [DNI]: Fixes for CTS on\n\tnautilus(LIMITED)","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\" "}}]