[libcamera-devel] [PATCH v2 09/17] android: capabilties: Correctly populate STALL durations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Sep 07, 2021 at 09:40:59PM +0200, Jacopo Mondi wrote:
> We currently hardcode 2560x1920 at 30FPS as the only stalling frame duration.
> 
> This is of course not correct, and all the required information to
> properly populate the ANDROID_SCALER_AVAILABLE_STALL_DURATIONS static
> metadata are available from initializeStaticMetadata().
> 
> Use the collected stalling durations and sizes to properly popoulate the
> static property.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/camera_capabilities.cpp | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 5d8293843f57..526127176678 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -1265,12 +1265,6 @@ int CameraCapabilities::initializeStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
>  				  availableStreamConfigurations);
>  
> -	std::vector<int64_t> availableStallDurations = {
> -		ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333,
> -	};
> -	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
> -				  availableStallDurations);
> -
>  	std::vector<int64_t> minFrameDurations;
>  	minFrameDurations.reserve(streamConfigurations_.size() * 4);
>  	for (const auto &entry : streamConfigurations_) {
> @@ -1282,6 +1276,19 @@ int CameraCapabilities::initializeStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
>  				  minFrameDurations);
>  
> +	std::vector<int64_t> availableStallDurations;
> +	for (const auto &entry : streamConfigurations_) {
> +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB)
> +			continue;
> +
> +		availableStallDurations.push_back(entry.androidFormat);
> +		availableStallDurations.push_back(entry.resolution.width);
> +		availableStallDurations.push_back(entry.resolution.height);
> +		availableStallDurations.push_back(entry.minFrameDurationNsec);

Should we add a break here, as there should be a single JPEG stream
configuration ? Or have I misunderstood this ?

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

> +	}
> +	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
> +				  availableStallDurations);
> +
>  	uint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY;
>  	staticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, croppingType);
>  

-- 
Regards,

Laurent Pinchart