[libcamera-devel,10/10] android: Introduce Chromium OS buffer manager

11428 diff mbox series

Introduce the CameraBuffer backend for the Chromium OS operating system
and the associated meson option.

The Chromium OS CameraBuffer implementation uses the
cros::CameraBufferManager class to perform mapping of 1 plane and multiplane
buffers and to retrieve size information.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 meson_options.txt                     |   2 +-
 src/android/mm/cros_camera_buffer.cpp | 141 ++++++++++++++++++++++++++
 src/android/mm/meson.build            |   3 +
 3 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 src/android/mm/cros_camera_buffer.cpp

Hi Jacopo,

Thank you for the patch.

On Mon, Mar 01, 2021 at 04:01:11PM +0100, Jacopo Mondi wrote:
> Introduce the CameraBuffer backend for the Chromium OS operating system
> and the associated meson option.
> 
> The Chromium OS CameraBuffer implementation uses the
> cros::CameraBufferManager class to perform mapping of 1 plane and multiplane
> buffers and to retrieve size information.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  meson_options.txt                     |   2 +-
>  src/android/mm/cros_camera_buffer.cpp | 141 ++++++++++++++++++++++++++
>  src/android/mm/meson.build            |   3 +
>  3 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 src/android/mm/cros_camera_buffer.cpp
> 
> diff --git a/meson_options.txt b/meson_options.txt
> index d840543b01f5..870914662f3e 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -7,7 +7,7 @@ option('android',
>  
>  option('android_platform',
>          type : 'combo',
> -        choices : ['generic'],
> +        choices : ['generic', 'cros'],

Alphabetical order ?

>          value : 'generic',
>          description : 'Select the Android platform to compile for')
>  
> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
> new file mode 100644
> index 000000000000..1103f4423c2f
> --- /dev/null
> +++ b/src/android/mm/cros_camera_buffer.cpp
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * cros_camera_buffer.cpp - Chromium OS buffer backend using CameraBufferManager
> + */
> +
> +#include "../camera_buffer.h"
> +
> +#include "libcamera/internal/log.h"
> +#include <libcamera/span.h>

I think you can drop span.h, it's provided by camera_buffer.h as part of
the CameraBuffer interface..

> +
> +#include "cros-camera/camera_buffer_manager.h"
> +
> +using namespace libcamera;
> +
> +LOG_DECLARE_CATEGORY(HAL)
> +
> +class CameraBuffer::Private : public Extensible::Private
> +{
> +	LIBCAMERA_DECLARE_PUBLIC(CameraBuffer)
> +
> +public:
> +	Private(CameraBuffer *cameraBuffer,
> +		buffer_handle_t camera3Buffer, int flags);
> +	~Private();
> +
> +	bool isValid() const { return valid_; }
> +
> +	unsigned int numPlanes() const;
> +
> +	Span<const uint8_t> plane(unsigned int plane) const;
> +	Span<uint8_t> plane(unsigned int plane);
> +
> +private:
> +	cros::CameraBufferManager *bufferManager_;
> +	buffer_handle_t handle_;
> +	unsigned int numPlanes_;
> +	bool valid_;
> +	union {
> +		void *addr;
> +		android_ycbcr ycbcr;
> +	} mem;
> +
> +	const uint8_t *planeAddr(unsigned int plane) const;
> +	uint8_t *planeAddr(unsigned int plane);
> +};
> +
> +CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,
> +			       buffer_handle_t camera3Buffer, int flags)
> +	: Extensible::Private(cameraBuffer), handle_(camera3Buffer),
> +	  numPlanes_(0), valid_(false)
> +{
> +	bufferManager_ = cros::CameraBufferManager::GetInstance();
> +
> +	bufferManager_->Register(camera3Buffer);
> +
> +	numPlanes_ = bufferManager_->GetNumPlanes(camera3Buffer);
> +	switch (numPlanes_) {
> +	case 1: {
> +		int ret = bufferManager_->Lock(handle_, 0, 0, 0, 0, 0, &mem.addr);
> +		if (ret) {
> +			LOG(HAL, Error) << "Single plane buffer mapping failed";
> +			return;
> +		}
> +		break;
> +	}
> +	case 2:
> +	case 3: {
> +		int ret = bufferManager_->LockYCbCr(handle_, 0, 0, 0, 0, 0,
> +						    &mem.ycbcr);
> +		if (ret) {
> +			LOG(HAL, Error) << "YCbCr buffer mapping failed";
> +			return;
> +		}
> +		break;
> +	}
> +	default:
> +		LOG(HAL, Error) << "Invalid number of planes: " << numPlanes_;
> +		return;
> +	}
> +
> +	valid_ = true;
> +}
> +
> +CameraBuffer::Private::~Private()
> +{
> +	bufferManager_->Unlock(handle_);
> +	bufferManager_->Deregister(handle_);
> +}
> +
> +unsigned int CameraBuffer::Private::numPlanes() const
> +{
> +	return bufferManager_->GetNumPlanes(handle_);
> +}
> +
> +Span<const uint8_t> CameraBuffer::Private::plane(unsigned int plane) const
> +{
> +	const uint8_t *addr = planeAddr(plane);
> +	return { addr, bufferManager_->GetPlaneSize(handle_, plane) };
> +}
> +
> +Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane)
> +{
> +	uint8_t *addr = planeAddr(plane);
> +	return { addr, bufferManager_->GetPlaneSize(handle_, plane) };
> +}
> +
> +const uint8_t *CameraBuffer::Private::planeAddr(unsigned int plane) const
> +{
> +	const void *addr;
> +
> +	switch (numPlanes()) {
> +	case 1:
> +		addr = mem.addr;
> +		break;
> +	default:
> +		switch (plane) {
> +		case 1:
> +			addr = mem.ycbcr.y;
> +			break;
> +		case 2:
> +			addr = mem.ycbcr.cb;
> +			break;
> +		case 3:
> +			addr = mem.ycbcr.cr;
> +			break;
> +		}
> +	}
> +
> +	return static_cast<const uint8_t *>(addr);
> +}
> +
> +uint8_t *CameraBuffer::Private::planeAddr(unsigned int plane)
> +{
> +	const CameraBuffer::Private *self =
> +		const_cast<const CameraBuffer::Private *>(this);
> +	return const_cast<uint8_t *>(self->planeAddr(plane));
> +}

These functions are only called in plane(), I'd inline them there (and
following the comments on previous patches, you can get rid of the
non-const version).

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

> +
> +PUBLIC_CAMERA_BUFFER
> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
> index 97f83f2a7380..eeb5cc2e6a31 100644
> --- a/src/android/mm/meson.build
> +++ b/src/android/mm/meson.build
> @@ -3,4 +3,7 @@
>  platform = get_option('android_platform')
>  if platform == 'generic'
>      android_hal_sources += files(['generic_camera_buffer.cpp'])
> +elif platform == 'cros'
> +    android_hal_sources += files(['cros_camera_buffer.cpp'])
> +    android_deps += [dependency('libcros_camera')]
>  endif