[libcamera-devel] [PATCH 1/2] android: camera_device: Fix sensor frame duration

[Jacopo Mondi]

Hi Paul,
   the Fix in subject is probably not required, as the
ANDROID_SENSOR_FRAME_DURATION property was not registered at all.

On Fri, May 21, 2021 at 07:55:33PM +0900, Paul Elder wrote:
> The sensor frame duration should be set by IPA. Get the information for
> the result metadata from libcamera.

how about:

"Libcamera reports the frame duration through the
controls::FrameDuration controls. Populate the
android.sensor.frameDuration result metadata using the libcamera
control value"

>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index b32e8be5..779ce554 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>  		resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);
>  	}
>
> +	if (metadata.contains(controls::FrameDurations)) {
> +		Span<const int64_t> durations =
> +			metadata.get(controls::FrameDurations);
> +		if (durations[0] == durations[1]) {

Yes, we have a bit of an unspecified behaviour here.

What is the use case for reporting two different frame durations ?

Indeed I see this one:
src/ipa/raspberrypi/raspberrypi.cpp:    libcameraMetadata_.set(controls::FrameDurations,
src/ipa/raspberrypi/raspberrypi.cpp:                           { static_cast<int64_t>(minFrameDuration_),
src/ipa/raspberrypi/raspberrypi.cpp:                             static_cast<int64_t>(maxFrameDuration_) });

Where the RPi IPA reports the min/max frame durations received as
controls part of a Request before using them to clamp the AE computed
exposure time and blankings.

+Naush, +David

Wouldn't it be better to report the actual duration by knowing the
actual exposure+blanking values once AGC has run ?

Thanks
   j

> +			int64_t duration = durations[0] * 1000;
> +			resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
> +						 duration);
> +		}
> +	}
> +
>  	if (metadata.contains(controls::ScalerCrop)) {
>  		Rectangle crop = metadata.get(controls::ScalerCrop);
>  		int32_t cropRect[] = {
> --
> 2.27.0
>