[libcamera-devel] [PATCH 02/12] android: Introduce CameraBuffer interface

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Feb 26, 2021 at 02:29:22PM +0100, Jacopo Mondi wrote:
> In order to provide support for different memory backends,
> move the MappedCamera3Buffer class definition outside of the
> CameraDevice class to its own file.
> 
> The interface defined in camera_buffer.h will be implemented by
> different backends that will be placed in the src/android/mm
> subdirectory.
> 
> Provide a first implementation for the 'generic android' backend which
> matches the existing one.
> 
> The MappedCamera3Buffer interface will be renamed in CameraBuffer
> in the next patch to match the name of the file and not in this patch
> to ease review.

The next patch is fairly small, I'd squash it with this one. Splitting
the two makes 02/12 a bit weird, as you mention introducing CameraBuffer
in the subject while this patch doesn't create such a class.

If you really want to keep the two separate, I'd rename
MappedCamera3Buffer to CameraBuffer first.

With this handled,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_buffer.h               | 21 ++++++++++
>  src/android/camera_device.cpp             | 30 --------------
>  src/android/camera_device.h               |  7 +---
>  src/android/meson.build                   |  2 +
>  src/android/mm/android_generic_buffer.cpp | 48 +++++++++++++++++++++++
>  src/android/mm/meson.build                |  6 +++
>  6 files changed, 78 insertions(+), 36 deletions(-)
>  create mode 100644 src/android/camera_buffer.h
>  create mode 100644 src/android/mm/android_generic_buffer.cpp
>  create mode 100644 src/android/mm/meson.build
> 
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> new file mode 100644
> index 000000000000..a1fb97a3c6db
> --- /dev/null
> +++ b/src/android/camera_buffer.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * camera_buffer.h - Frame buffer handling interface definition
> + */
> +#ifndef __ANDROID_CAMERA_BUFFER_H__
> +#define __ANDROID_CAMERA_BUFFER_H__
> +
> +#include <hardware/camera3.h>
> +
> +#include <libcamera/internal/buffer.h>
> +
> +class MappedCamera3Buffer : public libcamera::MappedBuffer
> +{
> +public:
> +	MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
> +	~MappedCamera3Buffer();
> +};
> +
> +#endif /* __ANDROID_CAMERA_BUFFER_H__ */
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 16cb8c6d2b84..a7a5b7986aa4 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -257,36 +257,6 @@ void sortCamera3StreamConfigs(std::vector<Camera3StreamConfig> &unsortedConfigs,
>  
>  } /* namespace */
>  
> -MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
> -					 int flags)
> -{
> -	maps_.reserve(camera3buffer->numFds);
> -	error_ = 0;
> -
> -	for (int i = 0; i < camera3buffer->numFds; i++) {
> -		if (camera3buffer->data[i] == -1)
> -			continue;
> -
> -		off_t length = lseek(camera3buffer->data[i], 0, SEEK_END);
> -		if (length < 0) {
> -			error_ = -errno;
> -			LOG(HAL, Error) << "Failed to query plane length";
> -			break;
> -		}
> -
> -		void *address = mmap(nullptr, length, flags, MAP_SHARED,
> -				     camera3buffer->data[i], 0);
> -		if (address == MAP_FAILED) {
> -			error_ = -errno;
> -			LOG(HAL, Error) << "Failed to mmap plane";
> -			break;
> -		}
> -
> -		maps_.emplace_back(static_cast<uint8_t *>(address),
> -				   static_cast<size_t>(length));
> -	}
> -}
> -
>  /*
>   * \struct Camera3RequestDescriptor
>   *
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 9cbfcad38433..e6c192c2100b 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -24,17 +24,12 @@
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/message.h"
>  
> +#include "camera_buffer.h"
>  #include "camera_metadata.h"
>  #include "camera_stream.h"
>  #include "camera_worker.h"
>  #include "jpeg/encoder.h"
>  
> -class MappedCamera3Buffer : public libcamera::MappedBuffer
> -{
> -public:
> -	MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
> -};
> -
>  class CameraDevice : protected libcamera::Loggable
>  {
>  public:
> diff --git a/src/android/meson.build b/src/android/meson.build
> index a13ce63b1d58..fd41c74f78ef 100644
> --- a/src/android/meson.build
> +++ b/src/android/meson.build
> @@ -57,6 +57,8 @@ android_hal_sources = files([
>      'yuv/post_processor_yuv.cpp'
>  ])
>  
> +subdir('mm')
> +
>  android_camera_metadata_sources = files([
>      'metadata/camera_metadata.c',
>  ])
> diff --git a/src/android/mm/android_generic_buffer.cpp b/src/android/mm/android_generic_buffer.cpp
> new file mode 100644
> index 000000000000..2504d9276e9e
> --- /dev/null
> +++ b/src/android/mm/android_generic_buffer.cpp
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * android_generic_buffer.cpp - Generic Android frame buffer backend
> + */
> +
> +#include "../camera_buffer.h"
> +
> +#include "libcamera/internal/log.h"
> +
> +using namespace libcamera;
> +
> +LOG_DECLARE_CATEGORY(HAL)
> +
> +MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
> +					 int flags)
> +{
> +	maps_.reserve(camera3buffer->numFds);
> +	error_ = 0;
> +
> +	for (int i = 0; i < camera3buffer->numFds; i++) {
> +		if (camera3buffer->data[i] == -1)
> +			continue;
> +
> +		off_t length = lseek(camera3buffer->data[i], 0, SEEK_END);
> +		if (length < 0) {
> +			error_ = -errno;
> +			LOG(HAL, Error) << "Failed to query plane length";
> +			break;
> +		}
> +
> +		void *address = mmap(nullptr, length, flags, MAP_SHARED,
> +				     camera3buffer->data[i], 0);
> +		if (address == MAP_FAILED) {
> +			error_ = -errno;
> +			LOG(HAL, Error) << "Failed to mmap plane";
> +			break;
> +		}
> +
> +		maps_.emplace_back(static_cast<uint8_t *>(address),
> +				   static_cast<size_t>(length));
> +	}
> +}
> +
> +MappedCamera3Buffer::~MappedCamera3Buffer()
> +{
> +}
> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
> new file mode 100644
> index 000000000000..39be8fec8567
> --- /dev/null
> +++ b/src/android/mm/meson.build
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +memory_backend = get_option('android_memory_backend')
> +if memory_backend == 'android_generic'
> +    android_hal_sources += files(['android_generic_buffer.cpp'])
> +endif

-- 
Regards,

Laurent Pinchart