[libcamera-devel] [PATCH v2 05/17] android: capabilities: Collect per-stream frame durations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Sep 07, 2021 at 09:40:55PM +0200, Jacopo Mondi wrote:
> Collect the per-stream frame durations while building the list
> of supported stream formats and resolutions.
> 
> In order to get an updated list of controls it is necessary to apply
> to the Camera the configuration we're testing, which was so far only
> validated.

Not ideal, but that's unavoidable for now.

> The per-configuration durations will be used to populate the Android
> ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS static metadata.
> 
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/camera_capabilities.cpp | 36 ++++++++++++++++++++++++++---
>  src/android/camera_capabilities.h   |  2 ++
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index e92bca42779a..4e82f12a904e 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -608,7 +608,29 @@ int CameraCapabilities::initializeStreamConfigurations()
>  		}
>  
>  		for (const Size &res : resolutions) {
> -			streamConfigurations_.push_back({ res, androidFormat });
> +			/*
> +			 * Configure the Camera with the collected format and
> +			 * resolution to get an updated list of controls.

Could you add

			 * \todo Avoid the need to configure the camera when
			 * redesigning the configuration API

here ?

> +			 */
> +			cfg.size = res;
> +			int ret = camera_->configure(cameraConfig.get());
> +			if (ret)
> +				return ret;
> +
> +			const ControlInfoMap &controls = camera_->controls();
> +			const auto frameDurations = controls.find(
> +				&controls::FrameDurationLimits);
> +			if (frameDurations == controls.end()) {
> +				LOG(HAL, Error)
> +					<< "Camera does not report frame durations";
> +				return -EINVAL;
> +			}
> +
> +			int64_t minFrameDuration = frameDurations->second.min().get<int64_t>() * 1000;
> +			int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000;

One day we should really standardize everything on the same time unit.

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

> +			streamConfigurations_.push_back({
> +				res, androidFormat, minFrameDuration, maxFrameDuration,
> +			});
>  
>  			/*
>  			 * If the format is HAL_PIXEL_FORMAT_YCbCr_420_888
> @@ -620,10 +642,18 @@ int CameraCapabilities::initializeStreamConfigurations()
>  			 *
>  			 * \todo Support JPEG streams produced by the camera
>  			 * natively.
> +			 *
> +			 * \todo HAL_PIXEL_FORMAT_BLOB is a 'stalling' format,
> +			 * its duration should take into account the time
> +			 * required for the YUV to JPEG encoding. For now
> +			 * use the same frame durations as collected for
> +			 * the YUV/RGB streams.
>  			 */
>  			if (androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888) {
> -				streamConfigurations_.push_back(
> -					{ res, HAL_PIXEL_FORMAT_BLOB });
> +				streamConfigurations_.push_back({
> +					res, HAL_PIXEL_FORMAT_BLOB,
> +					minFrameDuration, maxFrameDuration,
> +				});
>  				maxJpegSize = std::max(maxJpegSize, res);
>  			}
>  		}
> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h
> index a12596993ee5..6e55ddab445e 100644
> --- a/src/android/camera_capabilities.h
> +++ b/src/android/camera_capabilities.h
> @@ -43,6 +43,8 @@ private:
>  	struct Camera3StreamConfiguration {
>  		libcamera::Size resolution;
>  		int androidFormat;
> +		int64_t minFrameDurationNsec;
> +		int64_t maxFrameDurationNsec;
>  	};
>  
>  	bool validateManualSensorCapability();

-- 
Regards,

Laurent Pinchart