[libcamera-devel] [PATCH v2 3/4] android: Stub GraphicBufferAllocator for build tests

[Jacopo Mondi]

Hi Mattijs

On Tue, Sep 12, 2023 at 04:15:22PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> If we want to keep building libcamera on traditional Linux systems with:
>   -Dandroid=enabled -Dandroid_platform=generic
>
> We should stub GraphicBufferAllocator, which is not available.
> It's only available when building with the VNDK or when building within the
> AOSP tree.
>
> Also remove some deprecated methods and inclusions which are not needed for
> the stub class.
>
> Note: the imported headers from Android generate the -Wextra-semi warning.
>       To avoid patching the files, a pragma has been added before inclusion.
>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> ---
>  .../libs/ui/include/ui/GraphicBufferAllocator.h    | 30 ------------
>  src/android/mm/graphic_buffer_allocator_stub.cpp   | 53 ++++++++++++++++++++++
>  src/android/mm/meson.build                         |  1 +
>  3 files changed, 54 insertions(+), 30 deletions(-)
>
> diff --git a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h
> index e4674d746e37..9eac5bbe8324 100644
> --- a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h
> +++ b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h
> @@ -29,15 +29,10 @@
>  #include <ui/PixelFormat.h>
>
>  #include <utils/Errors.h>
> -#include <utils/KeyedVector.h>
> -#include <utils/Mutex.h>

ack, I tried moving Mutex.h as imported by 1/4 but then Singleton.h
fails, so it seems all headers imported in 1/4 are needed! good


>  #include <utils/Singleton.h>
>
>  namespace android {
>
> -class GrallocAllocator;
> -class GraphicBufferMapper;
> -
>  class GraphicBufferAllocator : public Singleton<GraphicBufferAllocator>
>  {
>  public:
> @@ -52,25 +47,6 @@ public:
>                        uint64_t usage, buffer_handle_t* handle, uint32_t* stride,
>                        std::string requestorName);
>
> -    /**
> -     * Allocates and does NOT import a gralloc buffer. Buffers cannot be used until they have
> -     * been imported. This function is for advanced use cases only.
> -     *
> -     * The raw native handle must be freed by calling native_handle_close() followed by
> -     * native_handle_delete().
> -     */
> -    status_t allocateRawHandle(uint32_t w, uint32_t h, PixelFormat format, uint32_t layerCount,
> -                               uint64_t usage, buffer_handle_t* handle, uint32_t* stride,
> -                               std::string requestorName);
> -
> -    /**
> -     * DEPRECATED: GraphicBufferAllocator does not use the graphicBufferId.
> -     */
> -    status_t allocate(uint32_t w, uint32_t h, PixelFormat format,
> -            uint32_t layerCount, uint64_t usage,
> -            buffer_handle_t* handle, uint32_t* stride, uint64_t graphicBufferId,
> -            std::string requestorName);
> -
>      status_t free(buffer_handle_t handle);
>
>      uint64_t getTotalSize() const;
> @@ -94,15 +70,9 @@ protected:
>                              uint64_t usage, buffer_handle_t* handle, uint32_t* stride,
>                              std::string requestorName, bool importBuffer);
>
> -    static Mutex sLock;
> -    static KeyedVector<buffer_handle_t, alloc_rec_t> sAllocList;
> -
>      friend class Singleton<GraphicBufferAllocator>;
>      GraphicBufferAllocator();
>      ~GraphicBufferAllocator();
> -
> -    GraphicBufferMapper& mMapper;
> -    std::unique_ptr<const GrallocAllocator> mAllocator;

I might be confused here. This is fine when building on Linux with the
stub you have provided, but what happens when building on Android ? I
presume the system-wide header takes precendece ?

>  };
>
>  // ---------------------------------------------------------------------------
> diff --git a/src/android/mm/graphic_buffer_allocator_stub.cpp b/src/android/mm/graphic_buffer_allocator_stub.cpp
> new file mode 100644
> index 000000000000..814b3d0e38bd
> --- /dev/null
> +++ b/src/android/mm/graphic_buffer_allocator_stub.cpp
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: Apache-2.0 */
> +/*
> + * Copyright (C) 2023, Ideas on Board
> + * Copyright (C) 2023, BayLibre
> + *
> + * graphic_buffer_allocator_stub.cpp - Android GraphicBufferAllocator
> + * stub for compile-testing
> + */
> +
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wextra-semi"
> +#include <ui/GraphicBufferAllocator.h>
> +#pragma GCC diagnostic pop
> +
> +namespace android {
> +
> +ANDROID_SINGLETON_STATIC_INSTANCE(GraphicBufferAllocator)
> +
> +GraphicBufferAllocator::GraphicBufferAllocator()
> +{
> +}

nit: empty line maybe ?

> +GraphicBufferAllocator::~GraphicBufferAllocator()
> +{
> +}
> +
> +uint64_t GraphicBufferAllocator::getTotalSize() const
> +{
> +	return 0;
> +}
> +
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wunused-parameter"
> +status_t GraphicBufferAllocator::allocate(uint32_t width,
> +					  uint32_t height,
> +					  PixelFormat format,
> +					  uint32_t layerCount,
> +					  uint64_t usage,
> +					  buffer_handle_t *handle,
> +					  uint32_t *stride,
> +					  std::string requestorName)
> +{
> +	*handle = nullptr;
> +	*stride = 0;
> +	return INVALID_OPERATION;
> +}
> +
> +status_t GraphicBufferAllocator::free(buffer_handle_t handle)
> +{
> +	return INVALID_OPERATION;
> +}
> +#pragma GCC diagnostic pop
> +
> +} // namespace android
> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
> index e3e0484c3720..e9ceb3afba67 100644
> --- a/src/android/mm/meson.build
> +++ b/src/android/mm/meson.build
> @@ -11,6 +11,7 @@ if platform == 'generic'
>          android_deps += [libhardware]
>      else
>          android_hal_sources += files(['libhardware_stub.c'])
> +        android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])

Shouldn't this be done later ?

All minors:
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>      endif
>  elif platform == 'cros'
>      android_hal_sources += files(['cros_camera_buffer.cpp',
>
> --
> 2.41.0
>