[libcamera-devel,0/4] android: switch over to modern gralloc API via libui
mbox series

Message ID 20230912-gralloc-api-v4-v1-0-0f80402d8e7a@baylibre.com
Headers show
Series
  • android: switch over to modern gralloc API via libui
Related show

Message

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

Devices are encouraged to switch over to HIDL interface named
android.hardware.graphics.allocator@<VERSION>, where <VERSION> can be
2.0 ,3.0 or 4.0.

This is mandatory since Android Q (10) [1]

Fortunately, Android provides an abstraction on top of
android.hardware.graphics.allocator which is compatible with each
version.
This abstraction is implemented in libui, which is available in the
VNDK.

Import all necessary headers from AOSP and switch over the
generic_frame_buffer_allocator to use GraphicBufferAllocator.

This series has been build-tested on a linux host and functionally
tested on an AM62x SK EVM with Android 13. (preview and capture).

[1] https://android.googlesource.com/platform/hardware/interfaces/+/refs/heads/main/compatibility_matrices/compatibility_matrix.4.xml#195

Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
Mattijs Korpershoek (4):
      android: Import libutils/libui headers from vndk v33
      android: Import GraphicBufferAllocator header from vndk v33
      android: Stub GraphicBufferAllocator for build tests
      android: mm: generic: Use GraphicBufferAllocator instead of gralloc.h

 .../libs/ui/include/ui/GraphicBufferAllocator.h    |  81 ++++++++
 .../native/libs/ui/include/ui/PixelFormat.h        |  75 +++++++
 include/android/meson.build                        |   2 +
 .../system/core/libutils/include/utils/Compat.h    |  94 +++++++++
 .../system/core/libutils/include/utils/Errors.h    |  78 ++++++++
 .../system/core/libutils/include/utils/Mutex.h     | 219 +++++++++++++++++++++
 .../system/core/libutils/include/utils/Singleton.h | 102 ++++++++++
 .../system/core/libutils/include/utils/Timers.h    | 103 ++++++++++
 src/android/mm/generic_frame_buffer_allocator.cpp  |  58 ++----
 src/android/mm/graphic_buffer_allocator_stub.cpp   |  50 +++++
 src/android/mm/libhardware_stub.c                  |  17 --
 src/android/mm/meson.build                         |   9 +-
 12 files changed, 825 insertions(+), 63 deletions(-)
---
base-commit: 58e501c71c47e57f02afde1bd296a037038cd6d5
change-id: 20230824-gralloc-api-v4-e3388fd364c6

Best regards,

Comments

Kieran Bingham Sept. 12, 2023, 12:24 p.m. UTC | #1 
Hi Mattijs,

This looks like a very interesting development.
I've just tried to run it through my integration tests and get the
following:

clang++-11 -Isrc/android/libcamera-hal.so.p -Isrc/android -I../../../src/libcamera/src/android -I../../../src/libcamera/include/android/frameworks/native/libs/ui/include -I../../../src/libcamera/include/android/hardware/libhardware/include -I../../../src/libcamera/include/android/metadata -I../../../src/libcamera/include/android/system/core/include -I../../../src/libcamera/include/android/system/core/libutils/include -Iinclude -I../../../src/libcamera/include -I../../../src/libcamera/subprojects/libyuv/include -Isubprojects/libyuv/__CMake_build -I../../../src/libcamera/subprojects/libyuv/__CMake_build -Isubprojects/libyuv -I../../../src/libcamera/subprojects/libyuv -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/android/libcamera-hal.so.p/mm_graphic_buffer_allocator_stub.cpp.o -MF src/android/libcamera-hal.so.p/mm_graphic_buffer_allocator_stub.cpp.o.d -o src/android/libcamera-hal.so.p/mm_graphic_buffer_allocator_stub.cpp.o -c ../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp
In file included from ../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp:10:
In file included from ../../../src/libcamera/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h:29:
../../../src/libcamera/include/android/frameworks/native/libs/ui/include/ui/PixelFormat.h:73:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
}; // namespace android
 ^
In file included from ../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp:10:
../../../src/libcamera/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h:79:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
}; // namespace android
 ^
../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp:50:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
}; // namespace android
 ^


I guess you haven't hit these warnings ?


This is building with clang-11.

For files we import we should probably disable the extra warning level.
I don't think we should modify external headers.

But for files in libcamera
(src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp) we
should probably keep consistent with the rest of the coding style.

Can you investigate this please?

--
Kieran



Quoting Mattijs Korpershoek via libcamera-devel (2023-09-12 10:36:09)
> gralloc.h is a very old API that has been deprecated at least since
> Android P (9).
> 
> Devices are encouraged to switch over to HIDL interface named
> android.hardware.graphics.allocator@<VERSION>, where <VERSION> can be
> 2.0 ,3.0 or 4.0.
> 
> This is mandatory since Android Q (10) [1]
> 
> Fortunately, Android provides an abstraction on top of
> android.hardware.graphics.allocator which is compatible with each
> version.
> This abstraction is implemented in libui, which is available in the
> VNDK.
> 
> Import all necessary headers from AOSP and switch over the
> generic_frame_buffer_allocator to use GraphicBufferAllocator.
> 
> This series has been build-tested on a linux host and functionally
> tested on an AM62x SK EVM with Android 13. (preview and capture).
> 
> [1] https://android.googlesource.com/platform/hardware/interfaces/+/refs/heads/main/compatibility_matrices/compatibility_matrix.4.xml#195
> 
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> Mattijs Korpershoek (4):
>       android: Import libutils/libui headers from vndk v33
>       android: Import GraphicBufferAllocator header from vndk v33
>       android: Stub GraphicBufferAllocator for build tests
>       android: mm: generic: Use GraphicBufferAllocator instead of gralloc.h
> 
>  .../libs/ui/include/ui/GraphicBufferAllocator.h    |  81 ++++++++
>  .../native/libs/ui/include/ui/PixelFormat.h        |  75 +++++++
>  include/android/meson.build                        |   2 +
>  .../system/core/libutils/include/utils/Compat.h    |  94 +++++++++
>  .../system/core/libutils/include/utils/Errors.h    |  78 ++++++++
>  .../system/core/libutils/include/utils/Mutex.h     | 219 +++++++++++++++++++++
>  .../system/core/libutils/include/utils/Singleton.h | 102 ++++++++++
>  .../system/core/libutils/include/utils/Timers.h    | 103 ++++++++++
>  src/android/mm/generic_frame_buffer_allocator.cpp  |  58 ++----
>  src/android/mm/graphic_buffer_allocator_stub.cpp   |  50 +++++
>  src/android/mm/libhardware_stub.c                  |  17 --
>  src/android/mm/meson.build                         |   9 +-
>  12 files changed, 825 insertions(+), 63 deletions(-)
> ---
> base-commit: 58e501c71c47e57f02afde1bd296a037038cd6d5
> change-id: 20230824-gralloc-api-v4-e3388fd364c6
> 
> Best regards,
> -- 
> Mattijs Korpershoek <mkorpershoek@baylibre.com>
>
Mattijs Korpershoek Sept. 12, 2023, 12:30 p.m. UTC | #2 
Hi Kieran,

On mar., sept. 12, 2023 at 13:24, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote:

> Hi Mattijs,
>
> This looks like a very interesting development.
> I've just tried to run it through my integration tests and get the
> following:
>
> clang++-11 -Isrc/android/libcamera-hal.so.p -Isrc/android -I../../../src/libcamera/src/android -I../../../src/libcamera/include/android/frameworks/native/libs/ui/include -I../../../src/libcamera/include/android/hardware/libhardware/include -I../../../src/libcamera/include/android/metadata -I../../../src/libcamera/include/android/system/core/include -I../../../src/libcamera/include/android/system/core/libutils/include -Iinclude -I../../../src/libcamera/include -I../../../src/libcamera/subprojects/libyuv/include -Isubprojects/libyuv/__CMake_build -I../../../src/libcamera/subprojects/libyuv/__CMake_build -Isubprojects/libyuv -I../../../src/libcamera/subprojects/libyuv -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/android/libcamera-hal.so.p/mm_graphic_buffer_allocator_stub.cpp.o -MF src/android/libcamera-hal.so.p/mm_graphic_buffer_allocator_stub.cpp.o.d -o src/android/libcamera-hal.so.p/mm_graphic_buffer_allocator_stub.cpp.o -c ../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp
> In file included from ../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp:10:
> In file included from ../../../src/libcamera/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h:29:
> ../../../src/libcamera/include/android/frameworks/native/libs/ui/include/ui/PixelFormat.h:73:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
> }; // namespace android
>  ^
> In file included from ../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp:10:
> ../../../src/libcamera/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h:79:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
> }; // namespace android
>  ^
> ../../../src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp:50:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
> }; // namespace android
>  ^

Thank you for the quick feedback and sorry I missed this.

I was aware that including libui.h requires us to ignore extra-semi
columns because GraphicBufferAllocator.h has some.

>
>
> I guess you haven't hit these warnings ?

I did in the Android build system, but not when building with meson.

>
>
> This is building with clang-11.

Ack.

>
> For files we import we should probably disable the extra warning level.
> I don't think we should modify external headers.
>
> But for files in libcamera
> (src/libcamera/src/android/mm/graphic_buffer_allocator_stub.cpp) we
> should probably keep consistent with the rest of the coding style.
>
> Can you investigate this please?

Yes, will do and send a v2.

>
> --
> Kieran
>
>
>
> Quoting Mattijs Korpershoek via libcamera-devel (2023-09-12 10:36:09)
>> gralloc.h is a very old API that has been deprecated at least since
>> Android P (9).
>> 
>> Devices are encouraged to switch over to HIDL interface named
>> android.hardware.graphics.allocator@<VERSION>, where <VERSION> can be
>> 2.0 ,3.0 or 4.0.
>> 
>> This is mandatory since Android Q (10) [1]
>> 
>> Fortunately, Android provides an abstraction on top of
>> android.hardware.graphics.allocator which is compatible with each
>> version.
>> This abstraction is implemented in libui, which is available in the
>> VNDK.
>> 
>> Import all necessary headers from AOSP and switch over the
>> generic_frame_buffer_allocator to use GraphicBufferAllocator.
>> 
>> This series has been build-tested on a linux host and functionally
>> tested on an AM62x SK EVM with Android 13. (preview and capture).
>> 
>> [1] https://android.googlesource.com/platform/hardware/interfaces/+/refs/heads/main/compatibility_matrices/compatibility_matrix.4.xml#195
>> 
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> ---
>> Mattijs Korpershoek (4):
>>       android: Import libutils/libui headers from vndk v33
>>       android: Import GraphicBufferAllocator header from vndk v33
>>       android: Stub GraphicBufferAllocator for build tests
>>       android: mm: generic: Use GraphicBufferAllocator instead of gralloc.h
>> 
>>  .../libs/ui/include/ui/GraphicBufferAllocator.h    |  81 ++++++++
>>  .../native/libs/ui/include/ui/PixelFormat.h        |  75 +++++++
>>  include/android/meson.build                        |   2 +
>>  .../system/core/libutils/include/utils/Compat.h    |  94 +++++++++
>>  .../system/core/libutils/include/utils/Errors.h    |  78 ++++++++
>>  .../system/core/libutils/include/utils/Mutex.h     | 219 +++++++++++++++++++++
>>  .../system/core/libutils/include/utils/Singleton.h | 102 ++++++++++
>>  .../system/core/libutils/include/utils/Timers.h    | 103 ++++++++++
>>  src/android/mm/generic_frame_buffer_allocator.cpp  |  58 ++----
>>  src/android/mm/graphic_buffer_allocator_stub.cpp   |  50 +++++
>>  src/android/mm/libhardware_stub.c                  |  17 --
>>  src/android/mm/meson.build                         |   9 +-
>>  12 files changed, 825 insertions(+), 63 deletions(-)
>> ---
>> base-commit: 58e501c71c47e57f02afde1bd296a037038cd6d5
>> change-id: 20230824-gralloc-api-v4-e3388fd364c6
>> 
>> Best regards,
>> -- 
>> Mattijs Korpershoek <mkorpershoek@baylibre.com>
>>