[libcamera-devel,v2,4/4] android: mm: generic: Use GraphicBufferAllocator instead of gralloc.h
diff mbox series

Message ID 20230912-gralloc-api-v4-v2-4-e859da63f98c@baylibre.com
State Superseded
Headers show
Series
  • android: switch over to modern gralloc API via libui
Related show

Commit Message

Mattijs Korpershoek Sept. 12, 2023, 2:15 p.m. UTC 
gralloc.h is a very old API that has been deprecated at least since
Android P (9).

Switch over to a higher level abstraction of gralloc from libui, which
is compatible with Android 11 and up.
Libui:
* is provided in the VNDK (so it's available to vendors).
* is also used in the camera vts test named VtsAidlHalCameraProvider_TargetTest.

Drop the libhardware stub since we no longer need it.

Notes:
* GraphicsBufferAllocator being a Singleton, buffer lifecycle
  management is easier.
* 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@baylibre.com>
---
 src/android/mm/generic_frame_buffer_allocator.cpp | 61 ++++++++---------------
 src/android/mm/libhardware_stub.c                 | 17 -------
 src/android/mm/meson.build                        |  8 ++-
 3 files changed, 23 insertions(+), 63 deletions(-)

Comments

Jacopo Mondi Sept. 22, 2023, 9:27 a.m. UTC | #1 
HI Mattijs

 nice work!

On Tue, Sep 12, 2023 at 04:15:23PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> gralloc.h is a very old API that has been deprecated at least since
> Android P (9).
>
> Switch over to a higher level abstraction of gralloc from libui, which
> is compatible with Android 11 and up.
> Libui:
> * is provided in the VNDK (so it's available to vendors).
> * is also used in the camera vts test named VtsAidlHalCameraProvider_TargetTest.
>
> Drop the libhardware stub since we no longer need it.
>
> Notes:
> * GraphicsBufferAllocator being a Singleton, buffer lifecycle
>   management is easier.
> * 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@baylibre.com>
> ---
>  src/android/mm/generic_frame_buffer_allocator.cpp | 61 ++++++++---------------
>  src/android/mm/libhardware_stub.c                 | 17 -------
>  src/android/mm/meson.build                        |  8 ++-
>  3 files changed, 23 insertions(+), 63 deletions(-)
>
> diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp
> index 7ecef2c669df..468579068c32 100644
> --- a/src/android/mm/generic_frame_buffer_allocator.cpp
> +++ b/src/android/mm/generic_frame_buffer_allocator.cpp
> @@ -16,8 +16,11 @@
>  #include "libcamera/internal/framebuffer.h"
>
>  #include <hardware/camera3.h>
> -#include <hardware/gralloc.h>

our version of include/android/system/core/include/system/camera.h
still includes gralloc.h. We should update all headers most probably

> -#include <hardware/hardware.h>
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wextra-semi"
> +#include <ui/GraphicBufferAllocator.h>
> +#pragma GCC diagnostic pop
> +#include <utils/Errors.h>
>
>  #include "../camera_device.h"
>  #include "../frame_buffer_allocator.h"
> @@ -33,35 +36,28 @@ class GenericFrameBufferData : public FrameBuffer::Private
>  	LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)
>
>  public:
> -	GenericFrameBufferData(struct alloc_device_t *allocDevice,
> +	GenericFrameBufferData(android::GraphicBufferAllocator &allocDevice,

You could

        GenericFrameBufferData(buffer_handle_t handle,
                               const std::vector<FrameBuffer::Plane> &planes)
                : FrameBuffer::Private(planes),
                  allocDevice_(android::GraphicBufferAllocator::get()),
                  handle_(handle)

Instead of passing it to the constructor of ::Private.
It does't make much different though, up to you!

>  			       buffer_handle_t handle,
>  			       const std::vector<FrameBuffer::Plane> &planes)
>  		: FrameBuffer::Private(planes), 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.
> -		 *

nice!

>  		 * \todo Thread safety against alloc_device_t is not documented.
>  		 * Is it no problem to call alloc/free in parallel?
>  		 */
> -		allocDevice_->free(allocDevice_, handle_);
> +		android::status_t status = allocDevice_.free(handle_);

Or you could even get the singleton instance here!

> +		if (status != android::NO_ERROR)
> +			LOG(HAL, Error) << "Error freeing framebuffer: " << status;
>  	}
>
>  private:
> -	struct alloc_device_t *allocDevice_;
> +	android::GraphicBufferAllocator &allocDevice_;

Usually reference class members are worrying to me, but this really
just points to a system-wide component accessed via a singleton if i'm
not mistaken, so this should be fine!


>  	const buffer_handle_t handle_;
>  };
>  } /* namespace */
> @@ -73,50 +69,33 @@ class PlatformFrameBufferAllocator::Private : public Extensible::Private
>  public:
>  	Private(CameraDevice *const cameraDevice)
>  		: cameraDevice_(cameraDevice),
> -		  hardwareModule_(nullptr),
> -		  allocDevice_(nullptr)
> +		  allocDevice_(android::GraphicBufferAllocator::get())
>  	{
> -		hw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hardwareModule_);
> -		ASSERT(hardwareModule_);
>  	}
>
> -	~Private() override;
> +	~Private() = default;
>
>  	std::unique_ptr<HALFrameBuffer>
>  	allocate(int halPixelFormat, const libcamera::Size &size, uint32_t usage);
>
>  private:
>  	const CameraDevice *const cameraDevice_;
> -	const struct hw_module_t *hardwareModule_;
> -	struct alloc_device_t *allocDevice_;
> +	android::GraphicBufferAllocator &allocDevice_;
>  };
>
> -PlatformFrameBufferAllocator::Private::~Private()
> -{
> -	if (allocDevice_)
> -		gralloc_close(allocDevice_);
> -	dlclose(hardwareModule_->dso);
> -}
> -
>  std::unique_ptr<HALFrameBuffer>
>  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;
> +	uint32_t 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 allocation: " << ret;
> +
> +	android::status_t status = allocDevice_.allocate(size.width, size.height, halPixelFormat,
> +							 1 /*layerCount*/, usage, &handle, &stride,
> +							 "libcameraHAL");
> +	if (status != android::NO_ERROR) {
> +		LOG(HAL, Error) << "failed buffer allocation: " << status;
>  		return nullptr;
>  	}
>  	if (!handle) {
> diff --git a/src/android/mm/libhardware_stub.c b/src/android/mm/libhardware_stub.c
> deleted file mode 100644
> index 00f15cd90cac..000000000000
> --- a/src/android/mm/libhardware_stub.c
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/* SPDX-License-Identifier: Apache-2.0 */
> -/*
> - * Copyright (C) 2023, Ideas on Board
> - *
> - * libhardware_stub.c - Android libhardware stub for test compilation
> - */
> -
> -#include <errno.h>
> -
> -#include <hardware/hardware.h>
> -
> -int hw_get_module(const char *id __attribute__((__unused__)),
> -		  const struct hw_module_t **module)
> -{
> -	*module = NULL;
> -	return -ENOTSUP;
> -}
> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
> index e9ceb3afba67..203b8c3e5804 100644
> --- a/src/android/mm/meson.build
> +++ b/src/android/mm/meson.build
> @@ -4,13 +4,11 @@ platform = get_option('android_platform')
>  if platform == 'generic'
>      android_hal_sources += files(['generic_camera_buffer.cpp',
>                                    'generic_frame_buffer_allocator.cpp'])
> -    android_deps += [libdl]

libdl was used for ?

>
> -    libhardware = dependency('libhardware', required : false)
> -    if libhardware.found()
> -        android_deps += [libhardware]
> +    libui = dependency('libui', required : false)
> +    if libui.found()
> +        android_deps += [libui]
>      else
> -        android_hal_sources += files(['libhardware_stub.c'])
>          android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])
>      endif
>  elif platform == 'cros'

All nits
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>
> --
> 2.41.0
>
Mattijs Korpershoek Sept. 23, 2023, 10:31 a.m. UTC | #2 
Hi Jacopo,

Thank you for your review.

On ven., sept. 22, 2023 at 11:27, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:

> HI Mattijs
>
>  nice work!
>
> On Tue, Sep 12, 2023 at 04:15:23PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
>> gralloc.h is a very old API that has been deprecated at least since
>> Android P (9).
>>
>> Switch over to a higher level abstraction of gralloc from libui, which
>> is compatible with Android 11 and up.
>> Libui
:
>> * is provided in the VNDK (so it's available to vendors).
>> * is also used in the camera vts test named VtsAidlHalCameraProvider_TargetTest.
>>
>> Drop the libhardware stub since we no longer need it.
>>
>> Notes:
>> * GraphicsBufferAllocator being a Singleton, buffer lifecycle
>>   management is easier.
>> * 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@baylibre.com>
>> ---
>>  src/android/mm/generic_frame_buffer_allocator.cpp | 61 ++++++++---------------
>>  src/android/mm/libhardware_stub.c                 | 17 -------
>>  src/android/mm/meson.build                        |  8 ++-
>>  3 files changed, 23 insertions(+), 63 deletions(-)
>>
>> diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp
>> index 7ecef2c669df..468579068c32 100644
>> --- a/src/android/mm/generic_frame_buffer_allocator.cpp
>> +++ b/src/android/mm/generic_frame_buffer_allocator.cpp
>> @@ -16,8 +16,11 @@
>>  #include "libcamera/internal/framebuffer.h"
>>
>>  #include <hardware/camera3.h>
>> -#include <hardware/gralloc.h>
>
> our version of include/android/system/core/include/system/camera.h
> still includes gralloc.h. We should update all headers most probably

That's also the case for the one in aosp main:

https://cs.android.com/android/platform/superproject/main/+/main:system/core/libsystem/include/system/camera.h?q=camera.h%20&ss=android%2Fplatform%2Fsuperproject%2Fmain

How about, instead, we get rid of the unused headers (which were copied
from AOSP) in the libcamera include/android tree instead?

If that seems fine, I will do some cleaning up and get rid of the
"no-longer needed" headers in a new patch for v2.


>
>> -#include <hardware/hardware.h>
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wextra-semi"
>> +#include <ui/GraphicBufferAllocator.h>
>> +#pragma GCC diagnostic pop
>> +#include <utils/Errors.h>
>>
>>  #include "../camera_device.h"
>>  #include "../frame_buffer_allocator.h"
>> @@ -33,35 +36,28 @@ class GenericFrameBufferData : public FrameBuffer::Private
>>  	LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)
>>
>>  public:
>> -	GenericFrameBufferData(struct alloc_device_t *allocDevice,
>> +	GenericFrameBufferData(android::GraphicBufferAllocator &allocDevice,
>
> You could
>
>         GenericFrameBufferData(buffer_handle_t handle,
>                                const std::vector<FrameBuffer::Plane> &planes)
>                 : FrameBuffer::Private(planes),
>                   allocDevice_(android::GraphicBufferAllocator::get()),
>                   handle_(handle)
>
> Instead of passing it to the constructor of ::Private.
> It does't make much different though, up to you!

I will give it some thought and either change it for v2 or put a note in
the cover to explain why I kept it the same.

>
>>  			       buffer_handle_t handle,
>>  			       const std::vector<FrameBuffer::Plane> &planes)
>>  		: FrameBuffer::Private(planes), 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.
>> -		 *
>
> nice!
>
>>  		 * \todo Thread safety against alloc_device_t is not documented.
>>  		 * Is it no problem to call alloc/free in parallel?
>>  		 */
>> -		allocDevice_->free(allocDevice_, handle_);
>> +		android::status_t status = allocDevice_.free(handle_);
>
> Or you could even get the singleton instance here!

You mean dropping the member and just getting the singleton instance with
android::GraphicBufferAllocator::get() ?

That might be even simpler and avoid adding an additional member indeed.

>
>> +		if (status != android::NO_ERROR)
>> +			LOG(HAL, Error) << "Error freeing framebuffer: " << status;
>>  	}
>>
>>  private:
>> -	struct alloc_device_t *allocDevice_;
>> +	android::GraphicBufferAllocator &allocDevice_;
>
> Usually reference class members are worrying to me, but this really
> just points to a system-wide component accessed via a singleton if i'm
> not mistaken, so this should be fine!

Yes, that is correct. I will study to remove the member for v2.

>
>
>>  	const buffer_handle_t handle_;
>>  };
>>  } /* namespace */
>> @@ -73,50 +69,33 @@ class PlatformFrameBufferAllocator::Private : public Extensible::Private
>>  public:
>>  	Private(CameraDevice *const cameraDevice)
>>  		: cameraDevice_(cameraDevice),
>> -		  hardwareModule_(nullptr),
>> -		  allocDevice_(nullptr)
>> +		  allocDevice_(android::GraphicBufferAllocator::get())
>>  	{
>> -		hw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hardwareModule_);
>> -		ASSERT(hardwareModule_);
>>  	}
>>
>> -	~Private() override;
>> +	~Private() = default;
>>
>>  	std::unique_ptr<HALFrameBuffer>
>>  	allocate(int halPixelFormat, const libcamera::Size &size, uint32_t usage);
>>
>>  private:
>>  	const CameraDevice *const cameraDevice_;
>> -	const struct hw_module_t *hardwareModule_;
>> -	struct alloc_device_t *allocDevice_;
>> +	android::GraphicBufferAllocator &allocDevice_;
>>  };
>>
>> -PlatformFrameBufferAllocator::Private::~Private()
>> -{
>> -	if (allocDevice_)
>> -		gralloc_close(allocDevice_);
>> -	dlclose(hardwareModule_->dso);
>> -}
>> -
>>  std::unique_ptr<HALFrameBuffer>
>>  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;
>> +	uint32_t 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 allocation: " << ret;
>> +
>> +	android::status_t status = allocDevice_.allocate(size.width, size.height, halPixelFormat,
>> +							 1 /*layerCount*/, usage, &handle, &stride,
>> +							 "libcameraHAL");
>> +	if (status != android::NO_ERROR) {
>> +		LOG(HAL, Error) << "failed buffer allocation: " << status;
>>  		return nullptr;
>>  	}
>>  	if (!handle) {
>> diff --git a/src/android/mm/libhardware_stub.c b/src/android/mm/libhardware_stub.c
>> deleted file mode 100644
>> index 00f15cd90cac..000000000000
>> --- a/src/android/mm/libhardware_stub.c
>> +++ /dev/null
>> @@ -1,17 +0,0 @@
>> -/* SPDX-License-Identifier: Apache-2.0 */
>> -/*
>> - * Copyright (C) 2023, Ideas on Board
>> - *
>> - * libhardware_stub.c - Android libhardware stub for test compilation
>> - */
>> -
>> -#include <errno.h>
>> -
>> -#include <hardware/hardware.h>
>> -
>> -int hw_get_module(const char *id __attribute__((__unused__)),
>> -		  const struct hw_module_t **module)
>> -{
>> -	*module = NULL;
>> -	return -ENOTSUP;
>> -}
>> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
>> index e9ceb3afba67..203b8c3e5804 100644
>> --- a/src/android/mm/meson.build
>> +++ b/src/android/mm/meson.build
>> @@ -4,13 +4,11 @@ platform = get_option('android_platform')
>>  if platform == 'generic'
>>      android_hal_sources += files(['generic_camera_buffer.cpp',
>>                                    'generic_frame_buffer_allocator.cpp'])
>> -    android_deps += [libdl]
>
> libdl was used for ?

for dlclose() call done in PlatformFrameBufferAllocator::Private::~Private().

For some background of why this was needed, refer to:
* 1450e09a0839 ("android: mm: generic: use GRALLOC_HARDWARE_MODULE_ID")
* https://patchwork.libcamera.org/patch/18309/#26496


>
>>
>> -    libhardware = dependency('libhardware', required : false)
>> -    if libhardware.found()
>> -        android_deps += [libhardware]
>> +    libui = dependency('libui', required : false)
>> +    if libui.found()
>> +        android_deps += [libui]
>>      else
>> -        android_hal_sources += files(['libhardware_stub.c'])
>>          android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])
>>      endif
>>  elif platform == 'cros'
>
> All nits
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>   j
>
>>
>> --
>> 2.41.0
>>

Patch
diff mbox series 

diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp
index 7ecef2c669df..468579068c32 100644
--- a/src/android/mm/generic_frame_buffer_allocator.cpp
+++ b/src/android/mm/generic_frame_buffer_allocator.cpp
@@ -16,8 +16,11 @@ 
 #include "libcamera/internal/framebuffer.h"
 
 #include <hardware/camera3.h>
-#include <hardware/gralloc.h>
-#include <hardware/hardware.h>
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wextra-semi"
+#include <ui/GraphicBufferAllocator.h>
+#pragma GCC diagnostic pop
+#include <utils/Errors.h>
 
 #include "../camera_device.h"
 #include "../frame_buffer_allocator.h"
@@ -33,35 +36,28 @@  class GenericFrameBufferData : public FrameBuffer::Private
 	LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)
 
 public:
-	GenericFrameBufferData(struct alloc_device_t *allocDevice,
+	GenericFrameBufferData(android::GraphicBufferAllocator &allocDevice,
 			       buffer_handle_t handle,
 			       const std::vector<FrameBuffer::Plane> &planes)
 		: FrameBuffer::Private(planes), 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.
-		 *
 		 * \todo Thread safety against alloc_device_t is not documented.
 		 * Is it no problem to call alloc/free in parallel?
 		 */
-		allocDevice_->free(allocDevice_, handle_);
+		android::status_t status = allocDevice_.free(handle_);
+		if (status != android::NO_ERROR)
+			LOG(HAL, Error) << "Error freeing framebuffer: " << status;
 	}
 
 private:
-	struct alloc_device_t *allocDevice_;
+	android::GraphicBufferAllocator &allocDevice_;
 	const buffer_handle_t handle_;
 };
 } /* namespace */
@@ -73,50 +69,33 @@  class PlatformFrameBufferAllocator::Private : public Extensible::Private
 public:
 	Private(CameraDevice *const cameraDevice)
 		: cameraDevice_(cameraDevice),
-		  hardwareModule_(nullptr),
-		  allocDevice_(nullptr)
+		  allocDevice_(android::GraphicBufferAllocator::get())
 	{
-		hw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hardwareModule_);
-		ASSERT(hardwareModule_);
 	}
 
-	~Private() override;
+	~Private() = default;
 
 	std::unique_ptr<HALFrameBuffer>
 	allocate(int halPixelFormat, const libcamera::Size &size, uint32_t usage);
 
 private:
 	const CameraDevice *const cameraDevice_;
-	const struct hw_module_t *hardwareModule_;
-	struct alloc_device_t *allocDevice_;
+	android::GraphicBufferAllocator &allocDevice_;
 };
 
-PlatformFrameBufferAllocator::Private::~Private()
-{
-	if (allocDevice_)
-		gralloc_close(allocDevice_);
-	dlclose(hardwareModule_->dso);
-}
-
 std::unique_ptr<HALFrameBuffer>
 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;
+	uint32_t 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 allocation: " << ret;
+
+	android::status_t status = allocDevice_.allocate(size.width, size.height, halPixelFormat,
+							 1 /*layerCount*/, usage, &handle, &stride,
+							 "libcameraHAL");
+	if (status != android::NO_ERROR) {
+		LOG(HAL, Error) << "failed buffer allocation: " << status;
 		return nullptr;
 	}
 	if (!handle) {
diff --git a/src/android/mm/libhardware_stub.c b/src/android/mm/libhardware_stub.c
deleted file mode 100644
index 00f15cd90cac..000000000000
--- a/src/android/mm/libhardware_stub.c
+++ /dev/null
@@ -1,17 +0,0 @@ 
-/* SPDX-License-Identifier: Apache-2.0 */
-/*
- * Copyright (C) 2023, Ideas on Board
- *
- * libhardware_stub.c - Android libhardware stub for test compilation
- */
-
-#include <errno.h>
-
-#include <hardware/hardware.h>
-
-int hw_get_module(const char *id __attribute__((__unused__)),
-		  const struct hw_module_t **module)
-{
-	*module = NULL;
-	return -ENOTSUP;
-}
diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
index e9ceb3afba67..203b8c3e5804 100644
--- a/src/android/mm/meson.build
+++ b/src/android/mm/meson.build
@@ -4,13 +4,11 @@  platform = get_option('android_platform')
 if platform == 'generic'
     android_hal_sources += files(['generic_camera_buffer.cpp',
                                   'generic_frame_buffer_allocator.cpp'])
-    android_deps += [libdl]
 
-    libhardware = dependency('libhardware', required : false)
-    if libhardware.found()
-        android_deps += [libhardware]
+    libui = dependency('libui', required : false)
+    if libui.found()
+        android_deps += [libui]
     else
-        android_hal_sources += files(['libhardware_stub.c'])
         android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])
     endif
 elif platform == 'cros'