[libcamera-devel] [PATCH v2 2/2] android: Introduce PlatformFrameBufferAllocator
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Nov 9 00:26:34 CET 2021
Quoting Hirokazu Honda (2021-11-01 07:16:52)
> After we unify identical streams to one stream configuration, the
> capture requested by a HAL client can have been resolved as
> CameraStream::Mapped. That is, a buffer to be written by a camera
> is not provided by a client. We would handle this case by
> dynamically allocating FrameBuffer.
>
> The existing FrameBufferAllocator cannot used because it is not
> allowed to allocate a new buffer while Camera is running.
>
> This introduces PlatformFrameBufferAllocator. It allocates
> FrameBuffer using cros::CameraBufferManager on ChromeOS and
> gralloc on non ChromeOS platform. The allocated FrameBuffer owns
> the underlying buffer but must be destroyed before
> PlatformFrameBufferAllocator is destroyed.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
I'm finding this one harder to grasp. It's late so I think I might have
to continue tomorrow.
I wish we had better infrastructure for unit tests on the Android layer,
as I'm sure some of this would benefit from more tests, but even then
we'd have to have an environment to run the unit tests against the
allocators...
> ---
> src/android/frame_buffer_allocator.h | 54 +++++++
> .../mm/cros_frame_buffer_allocator.cpp | 90 +++++++++++
> .../mm/generic_frame_buffer_allocator.cpp | 144 ++++++++++++++++++
> src/android/mm/meson.build | 6 +-
> 4 files changed, 292 insertions(+), 2 deletions(-)
> create mode 100644 src/android/frame_buffer_allocator.h
> create mode 100644 src/android/mm/cros_frame_buffer_allocator.cpp
> create mode 100644 src/android/mm/generic_frame_buffer_allocator.cpp
>
> diff --git a/src/android/frame_buffer_allocator.h b/src/android/frame_buffer_allocator.h
> new file mode 100644
> index 00000000..41ed4ae1
> --- /dev/null
> +++ b/src/android/frame_buffer_allocator.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * frame_buffer_allocator.h - Interface definition to allocate Frame buffer in
> + * platform dependent way.
> + */
> +#ifndef __ANDROID_FRAME_BUFFER_ALLOCATOR_H__
> +#define __ANDROID_FRAME_BUFFER_ALLOCATOR_H__
> +
> +#include <memory>
> +
> +#include <libcamera/base/class.h>
> +#include <libcamera/camera.h>
> +#include <libcamera/framebuffer.h>
> +#include <libcamera/geometry.h>
> +
> +class CameraDevice;
> +
> +class PlatformFrameBufferAllocator : libcamera::Extensible
Does the 'PlatformFrameBufferAllocater' benefit from using Extensible?
Is it required somehow? Can't it just be a normal class that the CrOS
and Android variants inherit from?
> +{
> + LIBCAMERA_DECLARE_PRIVATE()
> +
> +public:
> + explicit PlatformFrameBufferAllocator(CameraDevice *const cameraDevice);
> + ~PlatformFrameBufferAllocator();
> +
> + /*
> + * FrameBuffer owns the underlying buffer. Returns nullptr on failure.
> + * Note: The returned FrameBuffer needs to be destroyed before
> + * PlatformFrameBufferAllocator is destroyed.
> + */
> + std::unique_ptr<libcamera::FrameBuffer> allocate(
> + int halPixelFormat, const libcamera::Size &size, uint32_t usage);
> +};
> +
> +#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION \
> +PlatformFrameBufferAllocator::PlatformFrameBufferAllocator( \
> + CameraDevice *const cameraDevice) \
> + : Extensible(std::make_unique<Private>(this, cameraDevice)) \
> +{ \
> +} \
> +PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator() \
> +{ \
> +} \
> +std::unique_ptr<libcamera::FrameBuffer> \
> +PlatformFrameBufferAllocator::allocate(int halPixelFormat, \
> + const libcamera::Size& size, \
> + uint32_t usage) \
> +{ \
> + return _d()->allocate(halPixelFormat, size, usage); \
> +}
> +
> +#endif /* __ANDROID_FRAME_BUFFER_ALLOCATOR_H__ */
> diff --git a/src/android/mm/cros_frame_buffer_allocator.cpp b/src/android/mm/cros_frame_buffer_allocator.cpp
> new file mode 100644
> index 00000000..9c7e4ea4
> --- /dev/null
> +++ b/src/android/mm/cros_frame_buffer_allocator.cpp
> @@ -0,0 +1,90 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * cros_frame_buffer.cpp - Allocate FrameBuffer for Chromium OS using
> + * CameraBufferManager
> + */
> +
> +#include <memory>
> +#include <vector>
> +
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/framebuffer.h"
> +
> +#include "../camera_device.h"
> +#include "../frame_buffer_allocator.h"
> +#include "cros-camera/camera_buffer_manager.h"
> +
> +using namespace libcamera;
> +
> +LOG_DECLARE_CATEGORY(HAL)
> +
> +namespace {
> +class CrosFrameBufferData : public FrameBuffer::Private
> +{
> + LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)
> +public:
> + CrosFrameBufferData(cros::ScopedBufferHandle scopedHandle)
> + : FrameBuffer::Private(), scopedHandle_(std::move(scopedHandle))
> + {
> + }
> +
> + ~CrosFrameBufferData() override = default;
> +
> +private:
> + cros::ScopedBufferHandle scopedHandle_;
> +};
> +} /* namespace */
> +
> +class PlatformFrameBufferAllocator::Private : public Extensible::Private
> +{
> + LIBCAMERA_DECLARE_PUBLIC(PlatformFrameBufferAllocator)
> +
> +public:
> + Private([[maybe_unused]] PlatformFrameBufferAllocator *allocator,
> + [[maybe_unused]] CameraDevice *const cameraDevice)
> + {
> + }
> +
> + ~Private() override = default;
> +
> + std::unique_ptr<libcamera::FrameBuffer> allocate(
> + int halPixelFormat, const libcamera::Size &size, uint32_t usage);
> +};
> +
> +std::unique_ptr<libcamera::FrameBuffer>
> +PlatformFrameBufferAllocator::Private::allocate(
> + int halPixelFormat, const libcamera::Size &size, uint32_t usage)
> +{
> + cros::ScopedBufferHandle scopedHandle =
> + cros::CameraBufferManager::AllocateScopedBuffer(
> + size.width, size.height, halPixelFormat, usage);
> + if (!scopedHandle) {
> + LOG(HAL, Error) << "Failed to allocate buffer handle";
> + return nullptr;
> + }
> +
> + buffer_handle_t handle = *scopedHandle;
> + const size_t numPlanes = cros::CameraBufferManager::GetNumPlanes(handle);
> + std::vector<FrameBuffer::Plane> planes(numPlanes);
> + FileDescriptor fd{ handle->data[0] };
> + if (!fd.isValid()) {
> + LOG(HAL, Fatal) << "Invalid fd";
> + return nullptr;
> + }
> +
> + /* This code assumes all the planes are located in the same buffer. */
> + for (auto [i, plane] : utils::enumerate(planes)) {
> + plane.fd = fd;
> + plane.offset = cros::CameraBufferManager::GetPlaneOffset(handle, i);
> + plane.length = cros::CameraBufferManager::GetPlaneSize(handle, i);
> + }
> +
> + return std::make_unique<FrameBuffer>(
> + std::make_unique<CrosFrameBufferData>(std::move(scopedHandle)),
> + planes);
> +}
> +
> +PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION
> diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp
> new file mode 100644
> index 00000000..167cc1a5
> --- /dev/null
> +++ b/src/android/mm/generic_frame_buffer_allocator.cpp
> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * generic_camera_buffer.cpp - Allocate FrameBuffer using gralloc API
> + */
> +
> +#include <memory>
> +#include <vector>
> +
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/formats.h"
> +#include "libcamera/internal/framebuffer.h"
> +
> +#include <hardware/camera3.h>
> +#include <hardware/gralloc.h>
> +#include <hardware/hardware.h>
> +
> +#include "../camera_device.h"
> +#include "../frame_buffer_allocator.h"
> +
> +using namespace libcamera;
> +
> +LOG_DECLARE_CATEGORY(HAL)
> +
> +namespace {
> +class GenericFrameBufferData : public FrameBuffer::Private
> +{
> + LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)
> +
> +public:
> + GenericFrameBufferData(struct alloc_device_t *allocDevice,
> + buffer_handle_t handle)
> + : allocDevice_(allocDevice), handle_(handle)
> + {
> + ASSERT(allocDevice_);
> + ASSERT(handle_);
> + }
> +
> + ~GenericFrameBufferData() override
> + {
> + /*
> + * allocDevice_ is used to destroy handle_. allocDevice_ is
> + * owned by PlatformFrameBufferAllocator::Private.
> + * GenericFrameBufferData must be destroyed before it is
> + * destroyed.
> + *
> + * \todo: Consider managing alloc_device_t with std::shared_ptr
> + * if this is difficult to maintain.
> + *
> + * Q: Thread safety against alloc_device_t is not documented.
> + * Is it no problem to call alloc/free in parallel?
> + */
> + ASSERT(allocDevice_);
> + allocDevice_->free(allocDevice_, handle_);
> + }
> +
> +private:
> + struct alloc_device_t *allocDevice_;
> + const buffer_handle_t handle_;
> +};
> +} /* namespace */
> +
> +class PlatformFrameBufferAllocator::Private : public Extensible::Private
> +{
> + LIBCAMERA_DECLARE_PUBLIC(PlatformFrameBufferAllocator)
> +
> +public:
> + Private([[maybe_unused]] PlatformFrameBufferAllocator *allocator,
> + CameraDevice *const cameraDevice)
> + : cameraDevice_(cameraDevice),
> + hardwareModule_(cameraDevice->camera3Device()->common.module),
> + allocDevice_(nullptr)
> + {
> + ASSERT(hardwareModule_);
> + }
> +
> + ~Private() override;
> +
> + std::unique_ptr<libcamera::FrameBuffer> allocate(
> + int halPixelFormat, const libcamera::Size &size, uint32_t usage);
> +
> +private:
> + const CameraDevice *const cameraDevice_;
> + struct hw_module_t *const hardwareModule_;
> + struct alloc_device_t *allocDevice_;
> +};
> +
> +PlatformFrameBufferAllocator::Private::~Private()
> +{
> + if (allocDevice_)
> + gralloc_close(allocDevice_);
> +}
> +
> +std::unique_ptr<libcamera::FrameBuffer>
> +PlatformFrameBufferAllocator::Private::allocate(
> + int halPixelFormat, const libcamera::Size &size, uint32_t usage)
> +{
> + if (!allocDevice_) {
> + int ret = gralloc_open(hardwareModule_, &allocDevice_);
> + if (ret) {
> + LOG(HAL, Fatal) << "gralloc_open() failed: " << ret;
> + return nullptr;
> + }
> + }
> +
> + int stride = 0;
> + buffer_handle_t handle = nullptr;
> + int ret = allocDevice_->alloc(allocDevice_, size.width, size.height,
> + halPixelFormat, usage, &handle, &stride);
> + if (ret) {
> + LOG(HAL, Error) << "failed buffer allocating: " << ret;
> + return nullptr;
> + }
> + if (!handle) {
> + LOG(HAL, Fatal) << "buffer_handle_t is empty on success";
> + return nullptr;
> + }
> +
> + const libcamera::PixelFormat pixelFormat =
> + cameraDevice_->capabilities()->toPixelFormat(halPixelFormat);
> + const auto &info = PixelFormatInfo::info(pixelFormat);
> + const unsigned int numPlanes = info.numPlanes();
> + std::vector<FrameBuffer::Plane> planes(numPlanes);
> +
> + /* This code assumes the planes are mapped consecutively. */
> + FileDescriptor fd{ handle->data[0] };
> + size_t offset = 0;
> + for (auto [i, plane] : utils::enumerate(planes)) {
> + const size_t planeSize = info.planeSize(size.height, i, stride);
> +
> + planes[i].fd = fd;
> + planes[i].offset = offset;
> + planes[i].length = planeSize;
> + offset += planeSize;
> + }
> +
> + return std::make_unique<FrameBuffer>(
> + std::make_unique<GenericFrameBufferData>(allocDevice_, handle),
> + planes);
> +}
> +
> +PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION
> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
> index eeb5cc2e..d40a3b0b 100644
> --- a/src/android/mm/meson.build
> +++ b/src/android/mm/meson.build
> @@ -2,8 +2,10 @@
>
> platform = get_option('android_platform')
> if platform == 'generic'
> - android_hal_sources += files(['generic_camera_buffer.cpp'])
> + android_hal_sources += files(['generic_camera_buffer.cpp',
> + 'generic_frame_buffer_allocator.cpp'])
> elif platform == 'cros'
> - android_hal_sources += files(['cros_camera_buffer.cpp'])
> + android_hal_sources += files(['cros_camera_buffer.cpp',
> + 'cros_frame_buffer_allocator.cpp'])
> android_deps += [dependency('libcros_camera')]
> endif
> --
> 2.33.1.1089.g2158813163f-goog
>
More information about the libcamera-devel
mailing list