[libcamera-devel] android: camera_device: Always add mandatory metadata entries

11744 diff mbox series

The following static metadata entries are mandatory. Without them,
the Android camera service cannot work properly and application will
fail to start (error: "This device doesn't support camera2 API"):

- ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES
- ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS

Fixes: edd4b1dab26b ("android: camera_device: Compute frame durations")

Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
---
 src/android/camera_device.cpp | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

Hi Phi-Bang,

Thank you for the patch.

On Sun, Mar 28, 2021 at 10:17:15PM +0200, Phi-Bang Nguyen wrote:
> The following static metadata entries are mandatory. Without them,
> the Android camera service cannot work properly and application will
> fail to start (error: "This device doesn't support camera2 API"):
> 
> - ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES
> - ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS
> 
> Fixes: edd4b1dab26b ("android: camera_device: Compute frame durations")
> 
> Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
> ---
>  src/android/camera_device.cpp | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index a8108e3a..4b5d8f97 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -769,6 +769,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  
>  	int64_t minFrameDurationNsec = -1;
>  	int64_t maxFrameDurationNsec = -1;
> +	std::vector<int32_t> availableAeFpsTarget = {
> +		15, 30, 30, 30,
> +	};

I agree that these tags need to be provided, as they're mandatory.
However, I don't think providing a default value is the right way to go.
We should instead add support for the FrameDurations controls in the
simple pipeline handler. The HAL should also likely fail to initialize
(or open the camera device) if the pipeline handler doesn't support the
required controls, with a big warning message to make it clear what
needs to be fixed.

>  	const auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurations);
>  	if (frameDurationsInfo != controlsInfo.end()) {
>  		minFrameDurationNsec = frameDurationsInfo->second.min().get<int64_t>() * 1000;
> @@ -803,12 +806,12 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		 * Register to the camera service {min, max} and {max, max}
>  		 * intervals as requested by the metadata documentation.
>  		 */
> -		int32_t availableAeFpsTarget[] = {
> +		availableAeFpsTarget = {
>  			minFps, maxFps, maxFps, maxFps
>  		};
> -		staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
> -					  availableAeFpsTarget, 4);
>  	}
> +	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
> +				  availableAeFpsTarget.data(), availableAeFpsTarget.size());
>  
>  	std::vector<int32_t> aeCompensationRange = {
>  		0, 0,
> @@ -1140,19 +1143,20 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  availableStallDurations.size());
>  
>  	/* Use the minimum frame duration for all the YUV/RGB formats. */
> -	if (minFrameDurationNsec > 0) {
> -		std::vector<int64_t> minFrameDurations;
> -		minFrameDurations.reserve(streamConfigurations_.size() * 4);
> -		for (const auto &entry : streamConfigurations_) {
> -			minFrameDurations.push_back(entry.androidFormat);
> -			minFrameDurations.push_back(entry.resolution.width);
> -			minFrameDurations.push_back(entry.resolution.height);
> +	std::vector<int64_t> minFrameDurations;
> +	minFrameDurations.reserve(streamConfigurations_.size() * 4);
> +	for (const auto &entry : streamConfigurations_) {
> +		minFrameDurations.push_back(entry.androidFormat);
> +		minFrameDurations.push_back(entry.resolution.width);
> +		minFrameDurations.push_back(entry.resolution.height);
> +		if (minFrameDurationNsec > 0)
>  			minFrameDurations.push_back(minFrameDurationNsec);
> -		}
> -		staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
> -					  minFrameDurations.data(),
> -					  minFrameDurations.size());
> +		else
> +			minFrameDurations.push_back(33333333);
>  	}
> +	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
> +				  minFrameDurations.data(),
> +				  minFrameDurations.size());
>  
>  	uint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY;
>  	staticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, &croppingType, 1);