[libcamera-devel] [PATCH v2 7/9] android: camera_device: Use CameraMetadata wrapper in processControls

[Jacopo Mondi]

Hi Paul,

On Thu, Jan 21, 2021 at 07:15:47PM +0900, Paul Elder wrote:
> Use the CameraMetadata wrapper to access android request settings
> instead of accessing camera_metadata_t directly.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> New in v2
> ---
>  src/android/camera_device.cpp | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 16d5b472..49af221b 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1639,12 +1639,10 @@ int CameraDevice::processControls(camera3_capture_request_t *camera3Request,
>  	 * \todo As soon as more controls are handled, this part should be
>  	 * broke out to a dedicated function.
>  	 */
> -	const camera_metadata_t *camera3Settings = camera3Request->settings;
> +	CameraMetadata camera3Settings(camera3Request->settings);

I think it is very unfortunate that we clone a full metadata pack just
to inspect it. In [8/9] you clone the metadata at processRequest
beginning, and you should be able to re-use it here.

The settings shall actually be cloned in the Camera3RequestDescriptor
constructor, as it can be made to accept the full
camera3_capture_request_t.

I have an implementation of that which I was waiting to rebase on your
CameraMetadata change. What do you think if I break out your [2/9] and
handle the conversion here ? It should be rather quick and if we
fast-track it you can remove it from your series (I keep thinking you
should have broke out the Camera3RequestDescriptor change in 6/9 from
the handling of JPEG related metadata)

Thanks
  j

>  	camera_metadata_ro_entry_t entry;
> -	int ret = find_camera_metadata_ro_entry(camera3Settings,
> -						ANDROID_SCALER_CROP_REGION,
> -						&entry);
> -	if (!ret) {
> +	int ret = camera3Settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry);
> +	if (ret) {
>  		const int32_t *data = entry.data.i32;
>  		Rectangle cropRegion = { data[0], data[1],
>  					 static_cast<unsigned int>(data[2]),
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel