[libcamera-devel,10/16] android: Filter preview streams on FPS

13551 diff mbox series

Register as preview streams only streams capable of producing at least
30 FPS.

This requirement comes from inspecting the existing HAL implementation
on Intel IPU3 platform.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_capabilities.cpp | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Hi Jacopo,

On Fri, Aug 27, 2021 at 02:07:51PM +0200, Jacopo Mondi wrote:
> Register as preview streams only streams capable of producing at least
> 30 FPS.
> 
> This requirement comes from inspecting the existing HAL implementation
> on Intel IPU3 platform.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/android/camera_capabilities.cpp | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 388552963c47..c1ce63018aa8 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -1257,6 +1257,21 @@ int CameraCapabilities::initializeStaticMetadata()
>  	std::vector<uint32_t> availableStreamConfigurations;
>  	availableStreamConfigurations.reserve(streamConfigurations_.size() * 4);
>  	for (const auto &entry : streamConfigurations_) {
> +		/*
> +		 * Filter out YUV streams not capable of running at 30 FPS.
> +		 *
> +		 * This requirement comes from inspecting the Intel IPU3
> +		 * HAL implementation but no reference has been found in the
> +		 * metadata documentation.
> +		 *
> +		 * Calculate FPS as CTS does: see
> +		 * Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()
> +		 */
> +		unsigned int fps = static_cast<unsigned int>
> +				   (floor(1e9 / entry.minFrameDurationNsec + 0.05f));
> +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)
> +			continue;
> +
>  		availableStreamConfigurations.push_back(entry.androidFormat);
>  		availableStreamConfigurations.push_back(entry.resolution.width);
>  		availableStreamConfigurations.push_back(entry.resolution.height);
> @@ -1269,6 +1284,11 @@ int CameraCapabilities::initializeStaticMetadata()
>  	std::vector<int64_t> minFrameDurations;
>  	minFrameDurations.reserve(streamConfigurations_.size() * 4);
>  	for (const auto &entry : streamConfigurations_) {
> +		unsigned int fps = static_cast<unsigned int>
> +				   (floor(1e9 / entry.minFrameDurationNsec + 0.05f));
> +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)
> +			continue;
> +
>  		minFrameDurations.push_back(entry.androidFormat);
>  		minFrameDurations.push_back(entry.resolution.width);
>  		minFrameDurations.push_back(entry.resolution.height);
> -- 
> 2.32.0
>

Hi Jacopo

Looks good to me overall

On 8/27/21 5:37 PM, Jacopo Mondi wrote:
> Register as preview streams only streams capable of producing at least
> 30 FPS.
>
> This requirement comes from inspecting the existing HAL implementation
> on Intel IPU3 platform.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>   src/android/camera_capabilities.cpp | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 388552963c47..c1ce63018aa8 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -1257,6 +1257,21 @@ int CameraCapabilities::initializeStaticMetadata()
>   	std::vector<uint32_t> availableStreamConfigurations;
>   	availableStreamConfigurations.reserve(streamConfigurations_.size() * 4);
>   	for (const auto &entry : streamConfigurations_) {
> +		/*
> +		 * Filter out YUV streams not capable of running at 30 FPS.
> +		 *
> +		 * This requirement comes from inspecting the Intel IPU3
> +		 * HAL implementation but no reference has been found in the
> +		 * metadata documentation.
> +		 *
> +		 * Calculate FPS as CTS does: see
> +		 * Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()
> +		 */
> +		unsigned int fps = static_cast<unsigned int>
> +				   (floor(1e9 / entry.minFrameDurationNsec + 0.05f));


However, this is similar on the lines we do for minFrameDurationNsec for 
ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES right?

I am trying to find to find a relation between the two places, if any. 
I'll probably come around to take another look at this later. For now,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)
> +			continue;
> +
>   		availableStreamConfigurations.push_back(entry.androidFormat);
>   		availableStreamConfigurations.push_back(entry.resolution.width);
>   		availableStreamConfigurations.push_back(entry.resolution.height);
> @@ -1269,6 +1284,11 @@ int CameraCapabilities::initializeStaticMetadata()
>   	std::vector<int64_t> minFrameDurations;
>   	minFrameDurations.reserve(streamConfigurations_.size() * 4);
>   	for (const auto &entry : streamConfigurations_) {
> +		unsigned int fps = static_cast<unsigned int>
> +				   (floor(1e9 / entry.minFrameDurationNsec + 0.05f));
> +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)
> +			continue;
> +
>   		minFrameDurations.push_back(entry.androidFormat);
>   		minFrameDurations.push_back(entry.resolution.width);
>   		minFrameDurations.push_back(entry.resolution.height);