[libcamera-devel] [PATCH v2 4/6] fixup! android: camera_device: Compute frame durations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Jan 26, 2021 at 06:30:06PM +0100, Jacopo Mondi wrote:
> ---
>  src/android/camera_device.cpp | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index e3d43bea4700..5a8072a8a007 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -768,6 +768,26 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		minFrameDurationUsec = frameDurationsInfo->second.min().get<int64_t>();
>  		maxFrameDurationUsec = frameDurationsInfo->second.max().get<int64_t>();
>  
> +		/*
> +		 * Adjust the minimum frame duration to comply with Android
> +		 * requirements. The camera service mandates all preview/record
> +		 * streams to have a minimum frame duration < 33,366 milliseconds
> +		 * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
> +		 * implementation).
> +		 *
> +		 * If we're close enough (+- 500 useconds) to that value round
> +		 * the minimum frame duration of the camera to an accepted
> +		 * value.
> +		 */
> +		static constexpr double MIN_PREVIEW_RECORD_FPS = 29.97;
> +		static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_US = 1e6 / MIN_PREVIEW_RECORD_FPS;

You could write

		static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_US = 1e6 / 29.97;

as MIN_PREVIEW_RECORD_FPS is only used here.

> +		if (minFrameDurationUsec > MAX_PREVIEW_RECORD_DURATION_US) {
> +			double frameDurationDelta = minFrameDurationUsec -
> +						    MAX_PREVIEW_RECORD_DURATION_US;
> +			if (frameDurationDelta < 500)
> +				minFrameDurationUsec = MAX_PREVIEW_RECORD_DURATION_US - 1;
> +		}

Is the following more readable ?

		if (minFrameDurationUsec > MAX_PREVIEW_RECORD_DURATION_US &&
		    minFrameDurationUsec < MAX_PREVIEW_RECORD_DURATION_US + 500) {
			minFrameDurationUsec = MAX_PREVIEW_RECORD_DURATION_US - 1;

This is of course a horrible hack, but if we have no choice...

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

What's the maximum FPS we otherwise report ? Is it really that the
sensor can't reach 30fps, or is there an issue somewhere else ?

>  		/*
>  		 * The AE routine frame rate limits are computed using the frame
>  		 * duration limits, as libcamera clips the AE routine to the

-- 
Regards,

Laurent Pinchart