[libcamera-devel,v2,05/17] android: capabilities: Collect per-stream frame durations
diff mbox series

Message ID 20210907194107.803730-6-jacopo@jmondi.org
State Superseded
Headers show
Series
  • IPU3 control info update and HAL frame durations
Related show

Commit Message

Jacopo Mondi Sept. 7, 2021, 7:40 p.m. UTC 
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.

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@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/android/camera_capabilities.cpp | 36 ++++++++++++++++++++++++++---
 src/android/camera_capabilities.h   |  2 ++
 2 files changed, 35 insertions(+), 3 deletions(-)

Comments

Paul Elder Sept. 8, 2021, 12:16 a.m. UTC | #1 
Hi Jacopo,

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.
> 
> The per-configuration durations will be used to populate the Android
> ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS static metadata.
> 

Where did your Signed-off-by go? :D


Paul

> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain@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.
> +			 */
> +			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;
> +			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();
> -- 
> 2.32.0
>
Laurent Pinchart Oct. 6, 2021, 1:30 a.m. UTC | #2 
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@ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain@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@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();

Patch
diff mbox series 

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.
+			 */
+			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;
+			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();