[libcamera-devel] [PATCH v4 7/8] android: camera_device: Cache request metadata

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Mon, Jan 25, 2021 at 04:14:43PM +0900, Paul Elder wrote:
> The settings in an android capture request may be null, in which case
> the settings from the most recently submitted capture request should be
> used. Cache the request settings to achieve this.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> ---
> Changes in v4:
> - use default CameraMetadata constructor for lastSettings_ (so no
>   initialization in CameraDevice constructor)
> - add todo for incremental caching of android request settings
> 
> Changes in v3:
> - rebase on "android: camera device and metatada improvements", so it's
>   a bit cleaner
> 
> New in v2
> ---
>  src/android/camera_device.cpp | 6 +++++-
>  src/android/camera_device.h   | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 3068f89f..9330db39 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1688,9 +1688,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	 * The descriptor and the associated memory reserved here are freed
>  	 * at request complete time.
>  	 */
> -	/* \todo Handle null request settings */
>  	Camera3RequestDescriptor *descriptor =
>  		new Camera3RequestDescriptor(camera_.get(), camera3Request);
> +	/* \todo Set cache incrementally? */

I'm not sure we'll remember what this means in a few months :-)

	/*
	 * \todo The Android request model is incremental, settings passed in
	 * previous requests are to be effective until overridden explicitly in
	 * a new request. Do we need to cache settings incrementally here, or is
	 * it handled by the Android camera service ?
	 */

> +	if (camera3Request->settings)
> +		lastSettings_ = camera3Request->settings;
> +	else
> +		descriptor->settings_ = lastSettings_;
>  
>  	LOG(HAL, Debug) << "Queueing Request to libcamera with "
>  			<< descriptor->numBuffers_ << " HAL streams";
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 058a3f9a..fa4fb544 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -134,6 +134,8 @@ private:
>  	int orientation_;
>  
>  	unsigned int maxJpegBufferSize_;
> +
> +	CameraMetadata lastSettings_;
>  };
>  
>  #endif /* __ANDROID_CAMERA_DEVICE_H__ */

-- 
Regards,

Laurent Pinchart