[libcamera-devel] [RFC PATCH v2 03/12] android: CameraDevice: Report proper min and max frame durations

[Jacopo Mondi]

Hi Paul,

On Thu, Apr 22, 2021 at 06:40:53PM +0900, Paul Elder wrote:
> The HAL layer was getting the min and max frame durations from from the

from from

> camera, then rounding it to fps to report as available fps ranges. The
> same min and max frame durations were then being reported as min and max
> frame durations. Since the fps are integer values while the frame
> durations are in ns, this caused a rounding error making it seem like we
> were reporting an available max fps that was higher than was was allowed

s/was was/what was/

> by the minimum frame duration.
>
> An example is if the minimum frame duration is reported as 33366700ns.
> The HAL layer would then convert it to fps, which is 29.97, but it would
> be rounded and reported as 30 fps. When 30 fps is converted to a frame
> duration it is 33333333ns, which is less than the minimum frame duration
> that we report. Thus the minimum frame duration that we report
> contradicts the fps rage that we report.

Not easy to explain :)

>
> Fix this by recalculating the frame durations based on the rounded fps
> values.
>
> This allows the following CTS test to pass:
> - android.hardware.camera2.cts.SurfaceViewPreviewTest#testPreviewFpsRange
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/android/camera_device.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 76863877..a11ad848 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -900,6 +900,10 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		int32_t minFps = std::round(1e9 / maxFrameDurationNsec);
>  		minFps = std::max(1, minFps);
>
> +		/* Avoid rounding errors when we reuse these variables later */
> +		minFrameDurationNsec = 1e9 / maxFps;
> +		maxFrameDurationNsec = 1e9 / minFps;
> +

Agreed
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j
>  		/*
>  		 * Register to the camera service {min, max} and {max, max}
>  		 * intervals as requested by the metadata documentation.
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel