[libcamera-devel] [PATCH v2 9/9] android: camera_device: Cache request metadata

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Thu, Jan 21, 2021 at 07:15:49PM +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>
> 
> ---
> New in v2
> ---
>  src/android/camera_device.cpp | 10 ++++++----
>  src/android/camera_device.h   |  2 ++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 1b803c92..4f92cecc 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -337,7 +337,9 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor()
>  
>  CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera)
>  	: id_(id), running_(false), camera_(camera), staticMetadata_(nullptr),
> -	  facing_(CAMERA_FACING_FRONT), orientation_(0)
> +	  facing_(CAMERA_FACING_FRONT), orientation_(0),
> +	  /* \todo Keep this in sync with the actual number of entries. */
> +	  lastSettings_(CameraMetadata(40, 163))

I think you can construct an empty CameraMetadata to start with, as the
first request is guaranteed to have non-null settings.

>  {
>  	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>  
> @@ -1692,12 +1694,12 @@ 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 */
> -	CameraMetadata settings(camera3Request->settings);
> +	if (camera3Request->settings)
> +		lastSettings_ = CameraMetadata(camera3Request->settings);
>  	Camera3RequestDescriptor *descriptor =
>  		new Camera3RequestDescriptor(camera_.get(), camera3Request->frame_number,
>  					     camera3Request->num_output_buffers,
> -					     settings);
> +					     lastSettings_);

I wonder if this should be moved to the beginning of this series. If it
causes too painful rebase conflicts we could leave it here.

>  
>  	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 111a7d8f..d3a9f777 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -139,6 +139,8 @@ private:
>  	int orientation_;
>  
>  	unsigned int maxJpegBufferSize_;
> +
> +	CameraMetadata lastSettings_;
>  };
>  
>  #endif /* __ANDROID_CAMERA_DEVICE_H__ */

-- 
Regards,

Laurent Pinchart