[libcamera-devel] [PATCH 10/10] android: Introduce Chromium OS buffer manager
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Mar 2 01:15:11 CET 2021
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 at 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 at 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
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list