[{"id":27863,"web_url":"https://patchwork.libcamera.org/comment/27863/","msgid":"<20230924160640.GD13101@pendragon.ideasonboard.com>","date":"2023-09-24T16:06:40","subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Mattijs,\n\nThank you for the patch.\n\nOn Sat, Sep 23, 2023 at 06:23:33PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n> If we want to keep building libcamera on traditional Linux systems with:\n>   -Dandroid=enabled -Dandroid_platform=generic\n> \n> We should stub GraphicBufferAllocator, which is not available.\n> It's only available when building with the VNDK or when building within the\n> AOSP tree.\n> \n> Also remove some deprecated methods and inclusions which are not needed for\n> the stub class.\n> \n> Note: the imported headers from Android generate the -Wextra-semi warning.\n>       To avoid patching the files, a pragma has been added before inclusion.\n> \n> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  .../libs/ui/include/ui/GraphicBufferAllocator.h    | 30 ------------\n>  src/android/mm/graphic_buffer_allocator_stub.cpp   | 54 ++++++++++++++++++++++\n>  src/android/mm/meson.build                         |  7 +++\n>  3 files changed, 61 insertions(+), 30 deletions(-)\n> \n> diff --git a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n> index e4674d746e37..9eac5bbe8324 100644\n> --- a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n> +++ b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n> @@ -29,15 +29,10 @@\n>  #include <ui/PixelFormat.h>\n>  \n>  #include <utils/Errors.h>\n> -#include <utils/KeyedVector.h>\n> -#include <utils/Mutex.h>\n>  #include <utils/Singleton.h>\n>  \n>  namespace android {\n>  \n> -class GrallocAllocator;\n> -class GraphicBufferMapper;\n> -\n>  class GraphicBufferAllocator : public Singleton<GraphicBufferAllocator>\n>  {\n>  public:\n> @@ -52,25 +47,6 @@ public:\n>                        uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>                        std::string requestorName);\n>  \n> -    /**\n> -     * Allocates and does NOT import a gralloc buffer. Buffers cannot be used until they have\n> -     * been imported. This function is for advanced use cases only.\n> -     *\n> -     * The raw native handle must be freed by calling native_handle_close() followed by\n> -     * native_handle_delete().\n> -     */\n> -    status_t allocateRawHandle(uint32_t w, uint32_t h, PixelFormat format, uint32_t layerCount,\n> -                               uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n> -                               std::string requestorName);\n> -\n> -    /**\n> -     * DEPRECATED: GraphicBufferAllocator does not use the graphicBufferId.\n> -     */\n> -    status_t allocate(uint32_t w, uint32_t h, PixelFormat format,\n> -            uint32_t layerCount, uint64_t usage,\n> -            buffer_handle_t* handle, uint32_t* stride, uint64_t graphicBufferId,\n> -            std::string requestorName);\n> -\n>      status_t free(buffer_handle_t handle);\n>  \n>      uint64_t getTotalSize() const;\n> @@ -94,15 +70,9 @@ protected:\n>                              uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>                              std::string requestorName, bool importBuffer);\n>  \n> -    static Mutex sLock;\n> -    static KeyedVector<buffer_handle_t, alloc_rec_t> sAllocList;\n> -\n>      friend class Singleton<GraphicBufferAllocator>;\n>      GraphicBufferAllocator();\n>      ~GraphicBufferAllocator();\n> -\n> -    GraphicBufferMapper& mMapper;\n> -    std::unique_ptr<const GrallocAllocator> mAllocator;\n\nThis makes me very uncomfortable. You're changing the class size. It is\nprobably harmless here as the fields are at the end, but it's still\nfairly bad practice, and could cause hard to debug issues later.\n\nI think we're reaching a point where we may want to require a VNDK to\ncompile the Android HAL.\n\n>  };\n>  \n>  // ---------------------------------------------------------------------------\n> diff --git a/src/android/mm/graphic_buffer_allocator_stub.cpp b/src/android/mm/graphic_buffer_allocator_stub.cpp\n> new file mode 100644\n> index 000000000000..7e412c956887\n> --- /dev/null\n> +++ b/src/android/mm/graphic_buffer_allocator_stub.cpp\n> @@ -0,0 +1,54 @@\n> +/* SPDX-License-Identifier: Apache-2.0 */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board\n> + * Copyright (C) 2023, BayLibre\n> + *\n> + * graphic_buffer_allocator_stub.cpp - Android GraphicBufferAllocator\n> + * stub for compile-testing\n> + */\n> +\n> +#pragma GCC diagnostic push\n> +#pragma GCC diagnostic ignored \"-Wextra-semi\"\n\nCould you handle this in meson.build ? Adding pragmas around header\ninclusion isn't nice, especially given that you need to do the same in\npatch 4/4.\n\n> +#include <ui/GraphicBufferAllocator.h>\n> +#pragma GCC diagnostic pop\n> +\n> +namespace android {\n> +\n> +ANDROID_SINGLETON_STATIC_INSTANCE(GraphicBufferAllocator)\n> +\n> +GraphicBufferAllocator::GraphicBufferAllocator()\n> +{\n> +}\n> +\n> +GraphicBufferAllocator::~GraphicBufferAllocator()\n> +{\n> +}\n> +\n> +uint64_t GraphicBufferAllocator::getTotalSize() const\n> +{\n> +\treturn 0;\n> +}\n> +\n> +#pragma GCC diagnostic push\n> +#pragma GCC diagnostic ignored \"-Wunused-parameter\"\n\nYou can use [[maybe_unused]] instead.\n\n> +status_t GraphicBufferAllocator::allocate(uint32_t width,\n> +\t\t\t\t\t  uint32_t height,\n> +\t\t\t\t\t  PixelFormat format,\n> +\t\t\t\t\t  uint32_t layerCount,\n> +\t\t\t\t\t  uint64_t usage,\n> +\t\t\t\t\t  buffer_handle_t *handle,\n> +\t\t\t\t\t  uint32_t *stride,\n> +\t\t\t\t\t  std::string requestorName)\n> +{\n> +\t*handle = nullptr;\n> +\t*stride = 0;\n> +\treturn INVALID_OPERATION;\n> +}\n> +\n> +status_t GraphicBufferAllocator::free(buffer_handle_t handle)\n> +{\n> +\treturn INVALID_OPERATION;\n> +}\n> +#pragma GCC diagnostic pop\n> +\n> +} // namespace android\n> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> index e3e0484c3720..4d1fb718e94e 100644\n> --- a/src/android/mm/meson.build\n> +++ b/src/android/mm/meson.build\n> @@ -12,6 +12,13 @@ if platform == 'generic'\n>      else\n>          android_hal_sources += files(['libhardware_stub.c'])\n>      endif\n> +\n> +    libui = dependency('libui', required : false)\n> +    if libui.found()\n> +        android_deps += [libui]\n> +     else\n> +        android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])\n> +    endif\n>  elif platform == 'cros'\n>      android_hal_sources += files(['cros_camera_buffer.cpp',\n>                                    'cros_frame_buffer_allocator.cpp'])\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7D1A1BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 24 Sep 2023 16:06:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE1A662944;\n\tSun, 24 Sep 2023 18:06:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D93FB62916\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 24 Sep 2023 18:06:28 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2CBA96BE;\n\tSun, 24 Sep 2023 18:04:49 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695571590;\n\tbh=Cvc2BUy+7SB8XQljjEw4BAAerzEsvYVSG29Z7l6gUFo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=J9pBzWRxUwFKl6OJdo0RNkGcYa4aPrMABX5L11PJm6kbdaQlYGFiLJ8zt1M5gkg8+\n\tFQo904G6sa0cJp4NtxN0O255nIzQpBDjaf220eArzE6Jqrz2qxZRS158m6CmHigdTW\n\t0deJ6VXK2pZ8RZWUE1vmyVq4pM54q9XgKzNIKbN6PA4HzyGLmi4FIWXD3FiMYg6FvF\n\t1he1iOzUEC5sc5vQmXGnbLrFR4hPXxHd5K3sDCxavhF5IEs5WxEVn7ZPiGVWXhm7Si\n\twx9cadGbd2CZdSpPwTg71eLJdXUmi3dNUpoC2e2f5wqUnOTcaxdG11DoAr6vHfvbQt\n\t5PaugdSwu/cpg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695571489;\n\tbh=Cvc2BUy+7SB8XQljjEw4BAAerzEsvYVSG29Z7l6gUFo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pRFG8zwBcv/9VNnB3gUM6BeyNAkvPiDBh02n6k31tp4bZNqu3IkXHsFnbxsLVbCS4\n\tfpgP04pn26wx99O22MoC9RNhkyC0CtkJMWAtXMMqh4x9s6UaBCdDENrtDJchMEZlP3\n\tiD6L8dEweqGQQdiblwx+Edi+Me3P6FOadCUjsSow="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pRFG8zwB\"; dkim-atps=neutral","Date":"Sun, 24 Sep 2023 19:06:40 +0300","To":"Mattijs Korpershoek <mkorpershoek@baylibre.com>","Message-ID":"<20230924160640.GD13101@pendragon.ideasonboard.com>","References":"<20230923-gralloc-api-v4-v3-0-9a9e039284ba@baylibre.com>\n\t<20230923-gralloc-api-v4-v3-3-9a9e039284ba@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230923-gralloc-api-v4-v3-3-9a9e039284ba@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque <glaroque@baylibre.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27865,"web_url":"https://patchwork.libcamera.org/comment/27865/","msgid":"<20230924162032.GF13101@pendragon.ideasonboard.com>","date":"2023-09-24T16:20:32","subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Sep 24, 2023 at 07:06:40PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Hi Mattijs,\n> \n> Thank you for the patch.\n> \n> On Sat, Sep 23, 2023 at 06:23:33PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n> > If we want to keep building libcamera on traditional Linux systems with:\n> >   -Dandroid=enabled -Dandroid_platform=generic\n> > \n> > We should stub GraphicBufferAllocator, which is not available.\n> > It's only available when building with the VNDK or when building within the\n> > AOSP tree.\n> > \n> > Also remove some deprecated methods and inclusions which are not needed for\n> > the stub class.\n> > \n> > Note: the imported headers from Android generate the -Wextra-semi warning.\n> >       To avoid patching the files, a pragma has been added before inclusion.\n> > \n> > Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >  .../libs/ui/include/ui/GraphicBufferAllocator.h    | 30 ------------\n> >  src/android/mm/graphic_buffer_allocator_stub.cpp   | 54 ++++++++++++++++++++++\n> >  src/android/mm/meson.build                         |  7 +++\n> >  3 files changed, 61 insertions(+), 30 deletions(-)\n> > \n> > diff --git a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n> > index e4674d746e37..9eac5bbe8324 100644\n> > --- a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n> > +++ b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n> > @@ -29,15 +29,10 @@\n> >  #include <ui/PixelFormat.h>\n> >  \n> >  #include <utils/Errors.h>\n> > -#include <utils/KeyedVector.h>\n> > -#include <utils/Mutex.h>\n> >  #include <utils/Singleton.h>\n> >  \n> >  namespace android {\n> >  \n> > -class GrallocAllocator;\n> > -class GraphicBufferMapper;\n> > -\n> >  class GraphicBufferAllocator : public Singleton<GraphicBufferAllocator>\n> >  {\n> >  public:\n> > @@ -52,25 +47,6 @@ public:\n> >                        uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n> >                        std::string requestorName);\n> >  \n> > -    /**\n> > -     * Allocates and does NOT import a gralloc buffer. Buffers cannot be used until they have\n> > -     * been imported. This function is for advanced use cases only.\n> > -     *\n> > -     * The raw native handle must be freed by calling native_handle_close() followed by\n> > -     * native_handle_delete().\n> > -     */\n> > -    status_t allocateRawHandle(uint32_t w, uint32_t h, PixelFormat format, uint32_t layerCount,\n> > -                               uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n> > -                               std::string requestorName);\n> > -\n> > -    /**\n> > -     * DEPRECATED: GraphicBufferAllocator does not use the graphicBufferId.\n> > -     */\n> > -    status_t allocate(uint32_t w, uint32_t h, PixelFormat format,\n> > -            uint32_t layerCount, uint64_t usage,\n> > -            buffer_handle_t* handle, uint32_t* stride, uint64_t graphicBufferId,\n> > -            std::string requestorName);\n> > -\n> >      status_t free(buffer_handle_t handle);\n> >  \n> >      uint64_t getTotalSize() const;\n> > @@ -94,15 +70,9 @@ protected:\n> >                              uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n> >                              std::string requestorName, bool importBuffer);\n> >  \n> > -    static Mutex sLock;\n> > -    static KeyedVector<buffer_handle_t, alloc_rec_t> sAllocList;\n> > -\n> >      friend class Singleton<GraphicBufferAllocator>;\n> >      GraphicBufferAllocator();\n> >      ~GraphicBufferAllocator();\n> > -\n> > -    GraphicBufferMapper& mMapper;\n> > -    std::unique_ptr<const GrallocAllocator> mAllocator;\n> \n> This makes me very uncomfortable. You're changing the class size. It is\n> probably harmless here as the fields are at the end, but it's still\n> fairly bad practice, and could cause hard to debug issues later.\n> \n> I think we're reaching a point where we may want to require a VNDK to\n> compile the Android HAL.\n> \n> >  };\n> >  \n> >  // ---------------------------------------------------------------------------\n> > diff --git a/src/android/mm/graphic_buffer_allocator_stub.cpp b/src/android/mm/graphic_buffer_allocator_stub.cpp\n> > new file mode 100644\n> > index 000000000000..7e412c956887\n> > --- /dev/null\n> > +++ b/src/android/mm/graphic_buffer_allocator_stub.cpp\n> > @@ -0,0 +1,54 @@\n> > +/* SPDX-License-Identifier: Apache-2.0 */\n> > +/*\n> > + * Copyright (C) 2023, Ideas on Board\n> > + * Copyright (C) 2023, BayLibre\n> > + *\n> > + * graphic_buffer_allocator_stub.cpp - Android GraphicBufferAllocator\n> > + * stub for compile-testing\n> > + */\n> > +\n> > +#pragma GCC diagnostic push\n> > +#pragma GCC diagnostic ignored \"-Wextra-semi\"\n> \n> Could you handle this in meson.build ? Adding pragmas around header\n> inclusion isn't nice, especially given that you need to do the same in\n> patch 4/4.\n\nAfter reviewing the whole series, I'm a bit in two minds about this :-S\nIt's nice to keep the warning active for the rest of the code, to catch\nissues in the libcamera code base. We shouldn't compile the whole HAL\nwithout this, but maybe it's fine to compile android/mm only ?\n\n> > +#include <ui/GraphicBufferAllocator.h>\n> > +#pragma GCC diagnostic pop\n> > +\n> > +namespace android {\n> > +\n> > +ANDROID_SINGLETON_STATIC_INSTANCE(GraphicBufferAllocator)\n> > +\n> > +GraphicBufferAllocator::GraphicBufferAllocator()\n> > +{\n> > +}\n> > +\n> > +GraphicBufferAllocator::~GraphicBufferAllocator()\n> > +{\n> > +}\n> > +\n> > +uint64_t GraphicBufferAllocator::getTotalSize() const\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > +#pragma GCC diagnostic push\n> > +#pragma GCC diagnostic ignored \"-Wunused-parameter\"\n> \n> You can use [[maybe_unused]] instead.\n> \n> > +status_t GraphicBufferAllocator::allocate(uint32_t width,\n> > +\t\t\t\t\t  uint32_t height,\n> > +\t\t\t\t\t  PixelFormat format,\n> > +\t\t\t\t\t  uint32_t layerCount,\n> > +\t\t\t\t\t  uint64_t usage,\n> > +\t\t\t\t\t  buffer_handle_t *handle,\n> > +\t\t\t\t\t  uint32_t *stride,\n> > +\t\t\t\t\t  std::string requestorName)\n> > +{\n> > +\t*handle = nullptr;\n> > +\t*stride = 0;\n> > +\treturn INVALID_OPERATION;\n> > +}\n> > +\n> > +status_t GraphicBufferAllocator::free(buffer_handle_t handle)\n> > +{\n> > +\treturn INVALID_OPERATION;\n> > +}\n> > +#pragma GCC diagnostic pop\n> > +\n> > +} // namespace android\n> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n> > index e3e0484c3720..4d1fb718e94e 100644\n> > --- a/src/android/mm/meson.build\n> > +++ b/src/android/mm/meson.build\n> > @@ -12,6 +12,13 @@ if platform == 'generic'\n> >      else\n> >          android_hal_sources += files(['libhardware_stub.c'])\n> >      endif\n> > +\n> > +    libui = dependency('libui', required : false)\n> > +    if libui.found()\n> > +        android_deps += [libui]\n> > +     else\n> > +        android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])\n> > +    endif\n> >  elif platform == 'cros'\n> >      android_hal_sources += files(['cros_camera_buffer.cpp',\n> >                                    'cros_frame_buffer_allocator.cpp'])\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 71F3CBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 24 Sep 2023 16:20:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D543962945;\n\tSun, 24 Sep 2023 18:20:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 018DA62931\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 24 Sep 2023 18:20:20 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 28E1B6BE;\n\tSun, 24 Sep 2023 18:18:41 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695572421;\n\tbh=jml6C+z2P3hIhIvuT6QRKU8yzoqn/B+ZHR3/OWHAyI0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=Y4UVLLrazDl5IxVb/uSnsSnq5TwkiFuMgp6cu2Z/fEShkhhJVlKhpfAlBVdGlkbnh\n\t0HrvqBt4M1jH0PD5H/8LoIZb0F17TLw2tT/ZjzolGQ0t0W52/mzK/AKWuWu2Rz16+q\n\tNBCA6FRW54/5uVLPckFHWOZg5LGC68nYkIPqD+0qwTiL+pBLhSglFKR3viL76/cFvC\n\t3iX221GjqwCk/UuSGV4s1TNO7Im8PdkP4w+E4cyHZL3FVG1QqAeAEUnLoF0oyE3wB8\n\tkAPe41nv53tHLZ6JrxBIBO378RtES9JE9JaKxUO7/yXhX6rANcGkbIiL2l4T84Xbj5\n\t8+6y4hKPQntoA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695572321;\n\tbh=jml6C+z2P3hIhIvuT6QRKU8yzoqn/B+ZHR3/OWHAyI0=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=Dw+TFDbc+KH3b8hY3DhJZR1OW89D6WoQHZimJUu95ZQGtd+p9g69cER0ZXlhpWds9\n\t/8xk1gvrIWhqH3eD/VK+IT7eYT0x0YhKBZwk5IdAl8J2/xhsXh9JbPdHTlpdSVfWi5\n\tnMX1yPVi3T0PtNkT23yzLCQZW8d+58SM0euDddtY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Dw+TFDbc\"; dkim-atps=neutral","Date":"Sun, 24 Sep 2023 19:20:32 +0300","To":"Mattijs Korpershoek <mkorpershoek@baylibre.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque <glaroque@baylibre.com>","Message-ID":"<20230924162032.GF13101@pendragon.ideasonboard.com>","References":"<20230923-gralloc-api-v4-v3-0-9a9e039284ba@baylibre.com>\n\t<20230923-gralloc-api-v4-v3-3-9a9e039284ba@baylibre.com>\n\t<20230924160640.GD13101@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230924160640.GD13101@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27906,"web_url":"https://patchwork.libcamera.org/comment/27906/","msgid":"<87o7hk2idq.fsf@baylibre.com>","date":"2023-09-30T08:08:17","subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","submitter":{"id":153,"url":"https://patchwork.libcamera.org/api/people/153/","name":"Mattijs Korpershoek","email":"mkorpershoek@baylibre.com"},"content":"Hi Laurent,\n\nThank you for your review.\n\nOn dim., sept. 24, 2023 at 19:06, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Mattijs,\n>\n> Thank you for the patch.\n>\n> On Sat, Sep 23, 2023 at 06:23:33PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n>> If we want to keep building libcamera on traditional Linux systems with:\n>>   -Dandroid=enabled -Dandroid_platform=generic\n>> \n>> We should stub GraphicBufferAllocator, which is not available.\n>> It's only available when building with the VNDK or when building within the\n>> AOSP tree.\n>> \n>> Also remove some deprecated methods and inclusions which are not needed for\n>> the stub class.\n>> \n>> Note: the imported headers from Android generate the -Wextra-semi warning.\n>>       To avoid patching the files, a pragma has been added before inclusion.\n>> \n>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>\n>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>> ---\n>>  .../libs/ui/include/ui/GraphicBufferAllocator.h    | 30 ------------\n>>  src/android/mm/graphic_buffer_allocator_stub.cpp   | 54 ++++++++++++++++++++++\n>>  src/android/mm/meson.build                         |  7 +++\n>>  3 files changed, 61 insertions(+), 30 deletions(-)\n>> \n>> diff --git a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n>> index e4674d746e37..9eac5bbe8324 100644\n>> --- a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n>> +++ b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n>> @@ -29,15 +29,10 @@\n>>  #include <ui/PixelFormat.h>\n>>  \n>>  #include <utils/Errors.h>\n>> -#include <utils/KeyedVector.h>\n>> -#include <utils/Mutex.h>\n>>  #include <utils/Singleton.h>\n>>  \n>>  namespace android {\n>>  \n>> -class GrallocAllocator;\n>> -class GraphicBufferMapper;\n>> -\n>>  class GraphicBufferAllocator : public Singleton<GraphicBufferAllocator>\n>>  {\n>>  public:\n>> @@ -52,25 +47,6 @@ public:\n>>                        uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>>                        std::string requestorName);\n>>  \n>> -    /**\n>> -     * Allocates and does NOT import a gralloc buffer. Buffers cannot be used until they have\n>> -     * been imported. This function is for advanced use cases only.\n>> -     *\n>> -     * The raw native handle must be freed by calling native_handle_close() followed by\n>> -     * native_handle_delete().\n>> -     */\n>> -    status_t allocateRawHandle(uint32_t w, uint32_t h, PixelFormat format, uint32_t layerCount,\n>> -                               uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>> -                               std::string requestorName);\n>> -\n>> -    /**\n>> -     * DEPRECATED: GraphicBufferAllocator does not use the graphicBufferId.\n>> -     */\n>> -    status_t allocate(uint32_t w, uint32_t h, PixelFormat format,\n>> -            uint32_t layerCount, uint64_t usage,\n>> -            buffer_handle_t* handle, uint32_t* stride, uint64_t graphicBufferId,\n>> -            std::string requestorName);\n>> -\n>>      status_t free(buffer_handle_t handle);\n>>  \n>>      uint64_t getTotalSize() const;\n>> @@ -94,15 +70,9 @@ protected:\n>>                              uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>>                              std::string requestorName, bool importBuffer);\n>>  \n>> -    static Mutex sLock;\n>> -    static KeyedVector<buffer_handle_t, alloc_rec_t> sAllocList;\n>> -\n>>      friend class Singleton<GraphicBufferAllocator>;\n>>      GraphicBufferAllocator();\n>>      ~GraphicBufferAllocator();\n>> -\n>> -    GraphicBufferMapper& mMapper;\n>> -    std::unique_ptr<const GrallocAllocator> mAllocator;\n>\n> This makes me very uncomfortable. You're changing the class size. It is\n> probably harmless here as the fields are at the end, but it's still\n> fairly bad practice, and could cause hard to debug issues later.\n\nI understand the concern. I felt a little \"ugly\" doing this but I could\nnot come up with a better solution to \"compile test\" with meson only.\n\n>\n> I think we're reaching a point where we may want to require a VNDK to\n> compile the Android HAL.\n\nI will try it again to see if it's doable. I found some interesting\nlinks from Igalia who use the cerbero tool from gstreamer to do this:\n\nhttps://github.com/Igalia/wpe-android\n\n>\n>>  };\n>>  \n>>  // ---------------------------------------------------------------------------\n>> diff --git a/src/android/mm/graphic_buffer_allocator_stub.cpp b/src/android/mm/graphic_buffer_allocator_stub.cpp\n>> new file mode 100644\n>> index 000000000000..7e412c956887\n>> --- /dev/null\n>> +++ b/src/android/mm/graphic_buffer_allocator_stub.cpp\n>> @@ -0,0 +1,54 @@\n>> +/* SPDX-License-Identifier: Apache-2.0 */\n>> +/*\n>> + * Copyright (C) 2023, Ideas on Board\n>> + * Copyright (C) 2023, BayLibre\n>> + *\n>> + * graphic_buffer_allocator_stub.cpp - Android GraphicBufferAllocator\n>> + * stub for compile-testing\n>> + */\n>> +\n>> +#pragma GCC diagnostic push\n>> +#pragma GCC diagnostic ignored \"-Wextra-semi\"\n>\n> Could you handle this in meson.build ? Adding pragmas around header\n> inclusion isn't nice, especially given that you need to do the same in\n> patch 4/4.\n\nI could, but that would force a whole module to have this flag. As far\nas I know, it's not possible (by design) to add compiler flags on a\nper-file basis.\n\nWe discussed this on IRC with Kieran and Jacopo on 12th\nseptember and they seemed fine with it. Should I try to do this on the\nwhole android/mm directory?\n\n>\n>> +#include <ui/GraphicBufferAllocator.h>\n>> +#pragma GCC diagnostic pop\n>> +\n>> +namespace android {\n>> +\n>> +ANDROID_SINGLETON_STATIC_INSTANCE(GraphicBufferAllocator)\n>> +\n>> +GraphicBufferAllocator::GraphicBufferAllocator()\n>> +{\n>> +}\n>> +\n>> +GraphicBufferAllocator::~GraphicBufferAllocator()\n>> +{\n>> +}\n>> +\n>> +uint64_t GraphicBufferAllocator::getTotalSize() const\n>> +{\n>> +\treturn 0;\n>> +}\n>> +\n>> +#pragma GCC diagnostic push\n>> +#pragma GCC diagnostic ignored \"-Wunused-parameter\"\n>\n> You can use [[maybe_unused]] instead.\n\nWill do.\n\n>\n>> +status_t GraphicBufferAllocator::allocate(uint32_t width,\n>> +\t\t\t\t\t  uint32_t height,\n>> +\t\t\t\t\t  PixelFormat format,\n>> +\t\t\t\t\t  uint32_t layerCount,\n>> +\t\t\t\t\t  uint64_t usage,\n>> +\t\t\t\t\t  buffer_handle_t *handle,\n>> +\t\t\t\t\t  uint32_t *stride,\n>> +\t\t\t\t\t  std::string requestorName)\n>> +{\n>> +\t*handle = nullptr;\n>> +\t*stride = 0;\n>> +\treturn INVALID_OPERATION;\n>> +}\n>> +\n>> +status_t GraphicBufferAllocator::free(buffer_handle_t handle)\n>> +{\n>> +\treturn INVALID_OPERATION;\n>> +}\n>> +#pragma GCC diagnostic pop\n>> +\n>> +} // namespace android\n>> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n>> index e3e0484c3720..4d1fb718e94e 100644\n>> --- a/src/android/mm/meson.build\n>> +++ b/src/android/mm/meson.build\n>> @@ -12,6 +12,13 @@ if platform == 'generic'\n>>      else\n>>          android_hal_sources += files(['libhardware_stub.c'])\n>>      endif\n>> +\n>> +    libui = dependency('libui', required : false)\n>> +    if libui.found()\n>> +        android_deps += [libui]\n>> +     else\n>> +        android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])\n>> +    endif\n>>  elif platform == 'cros'\n>>      android_hal_sources += files(['cros_camera_buffer.cpp',\n>>                                    'cros_frame_buffer_allocator.cpp'])\n>> \n>\n> -- \n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3F764C0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 30 Sep 2023 08:08:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A6CDF6295F;\n\tSat, 30 Sep 2023 10:08:22 +0200 (CEST)","from mail-wr1-x436.google.com (mail-wr1-x436.google.com\n\t[IPv6:2a00:1450:4864:20::436])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 91B746295E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Sep 2023 10:08:20 +0200 (CEST)","by mail-wr1-x436.google.com with SMTP id\n\tffacd0b85a97d-3231d67aff2so11273713f8f.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Sep 2023 01:08:20 -0700 (PDT)","from localhost\n\t(2a01cb06b0237dd3c01dd287ba46c248.ipv6.abo.wanadoo.fr.\n\t[2a01:cb06:b023:7dd3:c01d:d287:ba46:c248])\n\tby smtp.gmail.com with ESMTPSA id\n\ty10-20020adff14a000000b00327297abe31sm482294wro.68.2023.09.30.01.08.18\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 30 Sep 2023 01:08:19 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1696061302;\n\tbh=2Uns9Wao7aaQSPAYbd8Z9+huOh1Fk9PNYp3xdqZ3wDU=;\n\th=To:In-Reply-To:References:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JfazRRTiqOUqF6GvJf1Na7ppcwFZbYFitaXNF3j8PSrbgRuX+ZhUkF5dGw4mar8eP\n\tEdQMVIs/FJFjKThxwJAdbYUdufrURTMsU0pk3a9mSE2fHK3F5lN6sh8NmA34M75sHX\n\tTBndfjl3X0kMm/yTJSJg9H2t+CVWe7Aur9PR5fcJWNntSxOkhSIy/kdmuJg/XXZzRz\n\tnO9sQP9ggP/rKOcj2e9nY9M1j9KcqJnJVdMhykcwuo5tuz0ECX9yB3rsn3x1vOTGkf\n\tzFxaax6inL+BKmYZkKl23I4pApUZl8l4HeaAO02y2lY7ytqh/zo58TZccjdQc7r6lJ\n\tfNNYzQf+u07ew==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1696061300;\n\tx=1696666100; darn=lists.libcamera.org; \n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:from:to:cc:subject:date:message-id:reply-to;\n\tbh=kWa27mMDUxIC3hpd6Y38DOcKgiBxJ333cBE/FkN7VNQ=;\n\tb=HebovOnL/G/cz9+tSsd3wCQtOraC7goMPrbCCWzhBRrEwc4nk1GxZ3KOIa2fdzwiEr\n\t0C/x/nUSQBb3bCVFEN0fkWSLrTINQt2NTAGmFu0UVCtHDQ6e8TYlqCrh38lWR4P4iwwp\n\t/tUizD4DlYaCQ/N9W42Q6a0ZV2Xi9x2Z/qEFTBJt9+uAwzW8j3iq0WH13W10dry1JLLU\n\tSusGmcjJ26OW4wZegGIXj1wSGq+e5y9EoiSZFVVnCn2Y6qn5/VIrnCv4ks/UsIRKaFKH\n\trzix9jqVNcYlAAKCH3A3RehXHESGeimgFCVfZOg65HccaXZwbtGZQkcf/CEOFaaPqZXK\n\t/9zg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=baylibre-com.20230601.gappssmtp.com\n\theader.i=@baylibre-com.20230601.gappssmtp.com header.b=\"HebovOnL\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1696061300; x=1696666100;\n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; \n\tbh=kWa27mMDUxIC3hpd6Y38DOcKgiBxJ333cBE/FkN7VNQ=;\n\tb=jL+JRNZIJp8N3UL5j65BBUkxXi3leB2axje7FbHZ8xEVKiA0oEkXWG1CYFluaNCGZk\n\tEENiIGCaSfbBEyPDE+ffM2RMXpsX1FdW0y7l5Jx8bKj2C2f75mdNKUJKmcEthQNkbE3K\n\tAhnfTDWvMbuqqFFdVM59H9AeDssxCrrmdnlZKRaQxx8VQHTFN7bGPlVgd4MYdqppIECy\n\tbIUXOqvr1ZnzWr2DoBlZTeAtrioTw4vzS2ldhdwEGC25wRV+FFmIKUSNsbiAyFHyX1IZ\n\tQKjmPdulz9EkWCV6G1MurSGTZACqSkxkOFgqiPvcGSl9ugEjH9a9EwqXZLRSJKnqlvRU\n\tcY2Q==","X-Gm-Message-State":"AOJu0YxzFj3WIXhXIm5prbMK8eSdISxuqsokygGWNy1h0Fm/JwC4nzKL\n\tXQlxMxm2+i0gV58TYh84IfcDkg==","X-Google-Smtp-Source":"AGHT+IExL82aMEgmMulIIDOXpC90uo8eZnDrbnHH/BeTwSpKFHeypoQuss3AxrxHHDcnKqK7RYeq3A==","X-Received":"by 2002:a5d:6101:0:b0:317:3d6c:5b27 with SMTP id\n\tv1-20020a5d6101000000b003173d6c5b27mr4706780wrt.46.1696061299957; \n\tSat, 30 Sep 2023 01:08:19 -0700 (PDT)","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20230924160640.GD13101@pendragon.ideasonboard.com>","References":"<20230923-gralloc-api-v4-v3-0-9a9e039284ba@baylibre.com>\n\t<20230923-gralloc-api-v4-v3-3-9a9e039284ba@baylibre.com>\n\t<20230924160640.GD13101@pendragon.ideasonboard.com>","Date":"Sat, 30 Sep 2023 10:08:17 +0200","Message-ID":"<87o7hk2idq.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Mattijs Korpershoek via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Mattijs Korpershoek <mkorpershoek@baylibre.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque <glaroque@baylibre.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27907,"web_url":"https://patchwork.libcamera.org/comment/27907/","msgid":"<87leco2i8d.fsf@baylibre.com>","date":"2023-09-30T08:11:30","subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","submitter":{"id":153,"url":"https://patchwork.libcamera.org/api/people/153/","name":"Mattijs Korpershoek","email":"mkorpershoek@baylibre.com"},"content":"On dim., sept. 24, 2023 at 19:20, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:\n\n> On Sun, Sep 24, 2023 at 07:06:40PM +0300, Laurent Pinchart via libcamera-devel wrote:\n>> Hi Mattijs,\n>> \n>> Thank you for the patch.\n>> \n>> On Sat, Sep 23, 2023 at 06:23:33PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n>> > If we want to keep building libcamera on traditional Linux systems with:\n>> >   -Dandroid=enabled -Dandroid_platform=generic\n>> > \n>> > We should stub GraphicBufferAllocator, which is not available.\n>> > It's only available when building with the VNDK or when building within the\n>> > AOSP tree.\n>> > \n>> > Also remove some deprecated methods and inclusions which are not needed for\n>> > the stub class.\n>> > \n>> > Note: the imported headers from Android generate the -Wextra-semi warning.\n>> >       To avoid patching the files, a pragma has been added before inclusion.\n>> > \n>> > Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>\n>> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>> > ---\n>> >  .../libs/ui/include/ui/GraphicBufferAllocator.h    | 30 ------------\n>> >  src/android/mm/graphic_buffer_allocator_stub.cpp   | 54 ++++++++++++++++++++++\n>> >  src/android/mm/meson.build                         |  7 +++\n>> >  3 files changed, 61 insertions(+), 30 deletions(-)\n>> > \n>> > diff --git a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n>> > index e4674d746e37..9eac5bbe8324 100644\n>> > --- a/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n>> > +++ b/include/android/frameworks/native/libs/ui/include/ui/GraphicBufferAllocator.h\n>> > @@ -29,15 +29,10 @@\n>> >  #include <ui/PixelFormat.h>\n>> >  \n>> >  #include <utils/Errors.h>\n>> > -#include <utils/KeyedVector.h>\n>> > -#include <utils/Mutex.h>\n>> >  #include <utils/Singleton.h>\n>> >  \n>> >  namespace android {\n>> >  \n>> > -class GrallocAllocator;\n>> > -class GraphicBufferMapper;\n>> > -\n>> >  class GraphicBufferAllocator : public Singleton<GraphicBufferAllocator>\n>> >  {\n>> >  public:\n>> > @@ -52,25 +47,6 @@ public:\n>> >                        uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>> >                        std::string requestorName);\n>> >  \n>> > -    /**\n>> > -     * Allocates and does NOT import a gralloc buffer. Buffers cannot be used until they have\n>> > -     * been imported. This function is for advanced use cases only.\n>> > -     *\n>> > -     * The raw native handle must be freed by calling native_handle_close() followed by\n>> > -     * native_handle_delete().\n>> > -     */\n>> > -    status_t allocateRawHandle(uint32_t w, uint32_t h, PixelFormat format, uint32_t layerCount,\n>> > -                               uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>> > -                               std::string requestorName);\n>> > -\n>> > -    /**\n>> > -     * DEPRECATED: GraphicBufferAllocator does not use the graphicBufferId.\n>> > -     */\n>> > -    status_t allocate(uint32_t w, uint32_t h, PixelFormat format,\n>> > -            uint32_t layerCount, uint64_t usage,\n>> > -            buffer_handle_t* handle, uint32_t* stride, uint64_t graphicBufferId,\n>> > -            std::string requestorName);\n>> > -\n>> >      status_t free(buffer_handle_t handle);\n>> >  \n>> >      uint64_t getTotalSize() const;\n>> > @@ -94,15 +70,9 @@ protected:\n>> >                              uint64_t usage, buffer_handle_t* handle, uint32_t* stride,\n>> >                              std::string requestorName, bool importBuffer);\n>> >  \n>> > -    static Mutex sLock;\n>> > -    static KeyedVector<buffer_handle_t, alloc_rec_t> sAllocList;\n>> > -\n>> >      friend class Singleton<GraphicBufferAllocator>;\n>> >      GraphicBufferAllocator();\n>> >      ~GraphicBufferAllocator();\n>> > -\n>> > -    GraphicBufferMapper& mMapper;\n>> > -    std::unique_ptr<const GrallocAllocator> mAllocator;\n>> \n>> This makes me very uncomfortable. You're changing the class size. It is\n>> probably harmless here as the fields are at the end, but it's still\n>> fairly bad practice, and could cause hard to debug issues later.\n>> \n>> I think we're reaching a point where we may want to require a VNDK to\n>> compile the Android HAL.\n>> \n>> >  };\n>> >  \n>> >  // ---------------------------------------------------------------------------\n>> > diff --git a/src/android/mm/graphic_buffer_allocator_stub.cpp b/src/android/mm/graphic_buffer_allocator_stub.cpp\n>> > new file mode 100644\n>> > index 000000000000..7e412c956887\n>> > --- /dev/null\n>> > +++ b/src/android/mm/graphic_buffer_allocator_stub.cpp\n>> > @@ -0,0 +1,54 @@\n>> > +/* SPDX-License-Identifier: Apache-2.0 */\n>> > +/*\n>> > + * Copyright (C) 2023, Ideas on Board\n>> > + * Copyright (C) 2023, BayLibre\n>> > + *\n>> > + * graphic_buffer_allocator_stub.cpp - Android GraphicBufferAllocator\n>> > + * stub for compile-testing\n>> > + */\n>> > +\n>> > +#pragma GCC diagnostic push\n>> > +#pragma GCC diagnostic ignored \"-Wextra-semi\"\n>> \n>> Could you handle this in meson.build ? Adding pragmas around header\n>> inclusion isn't nice, especially given that you need to do the same in\n>> patch 4/4.\n>\n> After reviewing the whole series, I'm a bit in two minds about this :-S\n> It's nice to keep the warning active for the rest of the code, to catch\n> issues in the libcamera code base. We shouldn't compile the whole HAL\n> without this, but maybe it's fine to compile android/mm only ?\n\nThat would mean that we have to move android/mm to a static library\nbecause meson cannot apply per directory/per file build flags (as far as\nI know)\nSee: https://github.com/mesonbuild/meson/issues/1367\n\nDo we want to move this to a static library just not have the pragmas?\n\nI'm OK with both options, let me know.\n\n>\n>> > +#include <ui/GraphicBufferAllocator.h>\n>> > +#pragma GCC diagnostic pop\n>> > +\n>> > +namespace android {\n>> > +\n>> > +ANDROID_SINGLETON_STATIC_INSTANCE(GraphicBufferAllocator)\n>> > +\n>> > +GraphicBufferAllocator::GraphicBufferAllocator()\n>> > +{\n>> > +}\n>> > +\n>> > +GraphicBufferAllocator::~GraphicBufferAllocator()\n>> > +{\n>> > +}\n>> > +\n>> > +uint64_t GraphicBufferAllocator::getTotalSize() const\n>> > +{\n>> > +\treturn 0;\n>> > +}\n>> > +\n>> > +#pragma GCC diagnostic push\n>> > +#pragma GCC diagnostic ignored \"-Wunused-parameter\"\n>> \n>> You can use [[maybe_unused]] instead.\n>> \n>> > +status_t GraphicBufferAllocator::allocate(uint32_t width,\n>> > +\t\t\t\t\t  uint32_t height,\n>> > +\t\t\t\t\t  PixelFormat format,\n>> > +\t\t\t\t\t  uint32_t layerCount,\n>> > +\t\t\t\t\t  uint64_t usage,\n>> > +\t\t\t\t\t  buffer_handle_t *handle,\n>> > +\t\t\t\t\t  uint32_t *stride,\n>> > +\t\t\t\t\t  std::string requestorName)\n>> > +{\n>> > +\t*handle = nullptr;\n>> > +\t*stride = 0;\n>> > +\treturn INVALID_OPERATION;\n>> > +}\n>> > +\n>> > +status_t GraphicBufferAllocator::free(buffer_handle_t handle)\n>> > +{\n>> > +\treturn INVALID_OPERATION;\n>> > +}\n>> > +#pragma GCC diagnostic pop\n>> > +\n>> > +} // namespace android\n>> > diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build\n>> > index e3e0484c3720..4d1fb718e94e 100644\n>> > --- a/src/android/mm/meson.build\n>> > +++ b/src/android/mm/meson.build\n>> > @@ -12,6 +12,13 @@ if platform == 'generic'\n>> >      else\n>> >          android_hal_sources += files(['libhardware_stub.c'])\n>> >      endif\n>> > +\n>> > +    libui = dependency('libui', required : false)\n>> > +    if libui.found()\n>> > +        android_deps += [libui]\n>> > +     else\n>> > +        android_hal_sources += files(['graphic_buffer_allocator_stub.cpp'])\n>> > +    endif\n>> >  elif platform == 'cros'\n>> >      android_hal_sources += files(['cros_camera_buffer.cpp',\n>> >                                    'cros_frame_buffer_allocator.cpp'])\n>> > \n>\n> -- \n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1D8F6BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 30 Sep 2023 08:11:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 76A7162964;\n\tSat, 30 Sep 2023 10:11:35 +0200 (CEST)","from mail-wr1-x429.google.com (mail-wr1-x429.google.com\n\t[IPv6:2a00:1450:4864:20::429])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 767156295F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Sep 2023 10:11:34 +0200 (CEST)","by mail-wr1-x429.google.com with SMTP id\n\tffacd0b85a97d-32003aae100so867590f8f.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Sep 2023 01:11:34 -0700 (PDT)","from localhost\n\t(2a01cb06b0237dd3c01dd287ba46c248.ipv6.abo.wanadoo.fr.\n\t[2a01:cb06:b023:7dd3:c01d:d287:ba46:c248])\n\tby smtp.gmail.com with ESMTPSA id\n\tv9-20020a5d4b09000000b0032008f99216sm23540651wrq.96.2023.09.30.01.11.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 30 Sep 2023 01:11:33 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1696061495;\n\tbh=e4M1P/4kDshK8Lxr7Rt36jEgSa4HGA6BnjBet6aLItI=;\n\th=To:In-Reply-To:References:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=kTRg6Gbe18GzdU3c5jQaFkn7kf5lKlitBV4Trg8T8r6alD3NqQXxJ+dtPEZfpQIlN\n\tCH2b4e540j5CHukz767LnOjfHbSiKZFZwCbLbcOWr6zt82WAkm+hnVzCW5rQjg6lkR\n\tmORLq27MoWp+eHql3350jUhm8gMahfYiPvLho59xA4nY4mJoSQtjCWFpQxuDIAcak5\n\t3aLjYWhyaZfGgpUrrSmAhBOSpHkmbPwzlG9l1IO5GUv/xmlPhAaoauCMQFEWb4qh4t\n\tsnXcsOzatCMra9rBiSpWTLhnW1kCieoQea8DuonwtAviFZfScs6tNKFoQZcxZtRIUS\n\tlUmh55RDgxN5A==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1696061494;\n\tx=1696666294; darn=lists.libcamera.org; \n\th=mime-version:message-id:date:references:in-reply-to:subject:to:from\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=p2IWIXOnPrZ7O7rQBo2vLDJwrOZ621isKqymDW2szo4=;\n\tb=kLHAAbmPqAcyD3F1cHGz9ZQTkVQLJd1FrSEFMpftbnVM6UuWBajg0W9v11ez9W2Mwq\n\tLPLSHcQygmXixkF6rBASNLZixWugD4AoKZE8U2CnAahmrAYfKoY4A1MafWxNWqCPf5nz\n\t62vahPGu5COk6bMNpN+o4Fm9B2WcJ15thC7Qg2utpdYKNLsrtZtSZGZAXwyr1PAqGRre\n\tB45h3EVTsKi28122E7NqwTT70E/MJ4M4gLk5v49dZMlGfaA4zODD9gDYys7vEILDeG+3\n\t0ai5bb7hKxDbO3wYfLijxuvylAwPMIgDkr+e/SGvQWaoyELfP/YdxsXzdNPzojPQzGrn\n\t7L+g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=baylibre-com.20230601.gappssmtp.com\n\theader.i=@baylibre-com.20230601.gappssmtp.com header.b=\"kLHAAbmP\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1696061494; x=1696666294;\n\th=mime-version:message-id:date:references:in-reply-to:subject:to:from\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=p2IWIXOnPrZ7O7rQBo2vLDJwrOZ621isKqymDW2szo4=;\n\tb=svKe4U9BqtheXnHZiOv48dvxeVKiABxMHFoM+osfnNBMuoU7pM37gTRu/zbhASYUJk\n\t5cEjeONLxCiEKQeFoJolTcU9gay++GJiIXDIg96N2lzYnjrRyvSy8r6Lq6S6cb2HSB8/\n\tlMsBS1yP9sPd0SPO2W2vvtGd/sxTr34qSEePLy17IFtvGVdW2Ou6Vq6C+mRGs3YWdo2b\n\tzEshJx0K7EBAF1t6Y8ZPjVbjDhQxm6jaf8f+vCcQA0LhaWgywgqhhU7PYlm/uGn9Onqp\n\tJTGBsmsGxnTm3+InaEpy4BiS7oQUcc9PhT+eHd62V8kJQRLRtha/mPeMe716poZxaVhg\n\tHu6w==","X-Gm-Message-State":"AOJu0Ywz2/nw0e/nXV3Ldq6CN+aCibytyY61TGnlFIcyuFq7bBLwlpUY\n\tfAngiPVx43voIYxlG4349LP74Q==","X-Google-Smtp-Source":"AGHT+IHEr9aZIxpoOsu9Cwi5+rOy9pXJKX/SgbCRLLkzO1iOs5uQhrb0rNEOq4bRTQy0qO8yFNZGbw==","X-Received":"by 2002:a5d:5265:0:b0:314:314e:fdda with SMTP id\n\tl5-20020a5d5265000000b00314314efddamr5944033wrc.23.1696061493914; \n\tSat, 30 Sep 2023 01:11:33 -0700 (PDT)","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Jacopo Mondi\n\t<jacopo.mondi@ideasonboard.com>, libcamera-devel@lists.libcamera.org, \n\tGuillaume La Roque <glaroque@baylibre.com>","In-Reply-To":"<20230924162032.GF13101@pendragon.ideasonboard.com>","References":"<20230923-gralloc-api-v4-v3-0-9a9e039284ba@baylibre.com>\n\t<20230923-gralloc-api-v4-v3-3-9a9e039284ba@baylibre.com>\n\t<20230924160640.GD13101@pendragon.ideasonboard.com>\n\t<20230924162032.GF13101@pendragon.ideasonboard.com>","Date":"Sat, 30 Sep 2023 10:11:30 +0200","Message-ID":"<87leco2i8d.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] android: Stub\n\tGraphicBufferAllocator for build tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Mattijs Korpershoek via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Mattijs Korpershoek <mkorpershoek@baylibre.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]