[libcamera-devel] [PATCH] [DNI]: Fixes for CTS on nautilus(LIMITED)

[paul.elder at ideasonboard.com]

Hi Umang,

On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> 
> This patch is for reference and discussion purposes only.
> Please DO NOT MERGE. If anything, there will be follow up
> separate patches for merge separately.

Thank you for the investigation and starting the discussion.

> 
> Problem:
> On nautilus, 26 tests were failing because of "Fail to open camera",
> but more specifically from adb logcat:
> 
> ```
> E Camera2-Parameters: generated preview size list is empty!!
> E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)
> E CameraService: connectHelper: Could not initialize client from HAL.
> I Camera2Client: Camera 0: Closed
> ```
> 
> was found to be root of the problem. The checks triggered are here:
> > Parameters::getFilteredSizes()
>   https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
> 
> nautilus reports higher frame-duration to start with: 34100000

iirc this was the minimum frame duration. This converts to 29.32 fps.

> (in CameraDevice::getStaticMetadata()).
> minFrameDurationNsec is meant to be re-adjusted down the line 
> before round-up, *if* the difference is < 500 useconds.

And the minimum frame duration gets compared to what android requires as
the maximum value of the minimum frame duration, which is 33366700ns =
29.97 fps.

> 
> On nautilus, since the difference was much larger than 500 useconds,

What this adjustment does is if the minimum frame duration (= max fps)
supported by the sensor is greater than the maximum allowed value (= the
supported max fps is lower than the max requirement), but it's within
500us, we can adjust it.

> the re-adjustment failed and libcamera reported a much higher
> frame-duration to upper-layers/libcameraservice. 

(Yes, it was rounded down instead to 29 fps and the corresponding
34482758ns, which is less/greater than 29.97fps / 33366700ns.)

> This led to the problem in Parameters::getFilteredSizes(),
> where all potential streams for preview are skipped to be added,
> due to high minFrameDuration.
> 
> To force this re-adjustment, the scope of difference was increased
> to 1200 useconds as done in the patch.

So by allowing 1200us, what we're saying is that we can still reasonably
say that we support a minimum frame duration of 33366700ns when the real
minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.
Is this a reasonable assertion?

> 
> CTS Results on nautilus after applying the changes:
> Total Run time: 8m 42s
> 1/1 modules completed
> Total Tests       : 221
> PASSED            : 221
> FAILED            : 0
> 
> ---
>  src/android/camera_device.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index fe332ec3..d0676a7f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		 * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
>  		 * implementation).
>  		 *
> -		 * If we're close enough (+ 500 useconds) to that value, round
> +		 * If we're close enough (+ 1200 useconds) to that value, round
>  		 * the minimum frame duration of the camera to an accepted
>  		 * value.
>  		 */
>  		static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;
>  		if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&
> -		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)
> +		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)
>  			minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;
>  
>  		/*
> @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
>  		ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
>  		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
> +		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,

What's this for?


Thanks,

Paul

>  		ANDROID_SCALER_CROPPING_TYPE,
>  		ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
>  		ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
> -- 
> 2.31.0
>