From patchwork Sat Sep 23 16:23:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mattijs Korpershoek X-Patchwork-Id: 19083 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2F6F4C326C for ; Sat, 23 Sep 2023 16:23:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BF3776294B; Sat, 23 Sep 2023 18:23:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1695486226; bh=ucAA6j7B+oiN09RDQkUor/fxhuupMT6VWM5gL1TMQf8=; h=Date:References:In-Reply-To:To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=1jxP5KnKZr92YDwt8VqLZSRXZ1NAP61JSonq+574OuVGdhSLyY4iryXtXAayUDAgZ KriyDbZpNGHIAej8hlaQG85WEC0iZt7Agp+St/bUcHwvVQY8y0iv2LBWMrJI/OR94I 1jQzXhQSFC/fgU/mNW2GbpqSoeWMIW1L6408gb+9bHmNOrDdJq1/+Zbb5BzTCpg/TX zAc853jjXhhdRgyQemgN8QMA6TcQV/k8jD3F7PoVOnvxXLiSB4qQhfaAfbCNnxxW5v k26DQ8LEQVKYl83SjEfu7r897M6h6Zk8fUUP+5enTgXmB+1evVMKzoGNMyRMmSwE65 r4PLVdjKV7djQ== Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D6C1162946 for ; Sat, 23 Sep 2023 18:23:42 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="b+mPU0vS"; dkim-atps=neutral Received: by mail-wm1-x330.google.com with SMTP id 5b1f17b1804b1-4053cf48670so25297985e9.0 for ; Sat, 23 Sep 2023 09:23:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1695486222; x=1696091022; darn=lists.libcamera.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=1uGenlFyLv/JhRh2NyVmUvgesNJ0eIJ6c6DG1nsxTNA=; b=b+mPU0vS7egdwwUNYtwexn0mqU+kn+mWeR2WgXfLF4oyT+e/Mz9RmQXo+Jw08psP3Z yj5VnEeJTmtyt/HNJ3wO2qOlOCWCbmgJIwdMTzodjiunvR9UOszJ7FSZ55xG50t0jY5p DnpZmaDcSxKtWOGaMuAuQvkxkVilEPmGei8UMlx4Y0NFf0Xd1zzfvHHEZfbMkuqojW/O dwnKMGQ9dmTJx7LHXr0o9jDPfFECQv0/GLZ3AnSKMNlXwJ4/l/2oyli16jSUAGvv5RIF 9zdg/uzKjWVbGpagrwPintYKpRy5R4d5P+JqU4lOs7m5gtMoGEW57GQJzDAVt2XFnSvp xfdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695486222; x=1696091022; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=1uGenlFyLv/JhRh2NyVmUvgesNJ0eIJ6c6DG1nsxTNA=; b=gKzrqAnmAkpdQroWbKzpDIwLj+S0Kx2C7rJShfXWRdkCKLkAMP/QPWq/QSZmft1v0A sakez9n6hnM67+hEx4zhse5Il3DmqD2WJt411iNlv5NuUuTOeqmbKMIqRI9CYVaXB9Ke aWr7r14Y7UMwHx82ttlOUTTP1mbI9ouu5fA5ltu7TiHNPJj8Pm5Yb+FqTafOJ9a+0N6s 0vm1tgvHjvQoC+DhRmlt072PV7tfDW0rBqPCTCjazPnZxDNBirONxKHj0iZ9s1yyCEte vs0Gb52bVRsfeMErob+rE0n7qHENGqWzIrEkAgX4BAJqmMoig9LHxJE3Pdc1YUEp7zzD uAXw== X-Gm-Message-State: AOJu0YwbEnGg0TAomgpkpRnr7F9qW0UG7RamKFVRoVMxy47KAllGFnw7 KivNMv3LpkAXZqNH1Q1To+Ck4g== X-Google-Smtp-Source: AGHT+IHIH3O9gg1eL/Cw9eTyJe7B17uPW8KKwbhOCiR3zF5xJBFhuvwCh9LWjDWQ8gi38UzTUZJIig== X-Received: by 2002:a1c:720c:0:b0:401:38dc:891c with SMTP id n12-20020a1c720c000000b0040138dc891cmr2094342wmc.5.1695486221711; Sat, 23 Sep 2023 09:23:41 -0700 (PDT) Received: from [192.168.0.39] ([2a01:e0a:324:38a0:25d4:3d10:65f9:654e]) by smtp.gmail.com with ESMTPSA id y7-20020adfd087000000b00317f70240afsm7200595wrh.27.2023.09.23.09.23.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 23 Sep 2023 09:23:41 -0700 (PDT) Date: Sat, 23 Sep 2023 18:23:34 +0200 MIME-Version: 1.0 Message-Id: <20230923-gralloc-api-v4-v3-4-9a9e039284ba@baylibre.com> References: <20230923-gralloc-api-v4-v3-0-9a9e039284ba@baylibre.com> In-Reply-To: <20230923-gralloc-api-v4-v3-0-9a9e039284ba@baylibre.com> To: libcamera-devel@lists.libcamera.org X-Mailer: b4 0.12.4-dev-6aa5d Subject: [libcamera-devel] [PATCH v3 4/4] android: mm: generic: Use GraphicBufferAllocator instead of gralloc.h X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mattijs Korpershoek via libcamera-devel From: Mattijs Korpershoek Reply-To: Mattijs Korpershoek Cc: Jacopo Mondi , Guillaume La Roque Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Reviewed-by: Jacopo Mondi --- src/android/mm/generic_frame_buffer_allocator.cpp | 68 ++++++++--------------- src/android/mm/libhardware_stub.c | 17 ------ src/android/mm/meson.build | 8 --- 3 files changed, 22 insertions(+), 71 deletions(-) diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp index 7ecef2c669df..1f2fbe334f2c 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 -#include -#include +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wextra-semi" +#include +#pragma GCC diagnostic pop +#include #include "../camera_device.h" #include "../frame_buffer_allocator.h" @@ -33,35 +36,26 @@ class GenericFrameBufferData : public FrameBuffer::Private LIBCAMERA_DECLARE_PUBLIC(FrameBuffer) public: - GenericFrameBufferData(struct alloc_device_t *allocDevice, - buffer_handle_t handle, + GenericFrameBufferData(buffer_handle_t handle, const std::vector &planes) - : FrameBuffer::Private(planes), allocDevice_(allocDevice), - handle_(handle) + : FrameBuffer::Private(planes), 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_); + auto &allocator = android::GraphicBufferAllocator::get(); + android::status_t status = allocator.free(handle_); + if (status != android::NO_ERROR) + LOG(HAL, Error) << "Error freeing framebuffer: " << status; } private: - struct alloc_device_t *allocDevice_; const buffer_handle_t handle_; }; } /* namespace */ @@ -72,51 +66,33 @@ class PlatformFrameBufferAllocator::Private : public Extensible::Private public: Private(CameraDevice *const cameraDevice) - : cameraDevice_(cameraDevice), - hardwareModule_(nullptr), - allocDevice_(nullptr) + : cameraDevice_(cameraDevice) { - hw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hardwareModule_); - ASSERT(hardwareModule_); } - ~Private() override; + ~Private() = default; std::unique_ptr 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_; }; -PlatformFrameBufferAllocator::Private::~Private() -{ - if (allocDevice_) - gralloc_close(allocDevice_); - dlclose(hardwareModule_->dso); -} - std::unique_ptr 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; + + auto &allocator = android::GraphicBufferAllocator::get(); + android::status_t status = allocator.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) { @@ -143,7 +119,7 @@ PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat, return std::make_unique( std::make_unique( - allocDevice_, handle, planes), + handle, planes), 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 - -#include - -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 4d1fb718e94e..301a2622008b 100644 --- a/src/android/mm/meson.build +++ b/src/android/mm/meson.build @@ -4,14 +4,6 @@ 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] - else - android_hal_sources += files(['libhardware_stub.c']) - endif libui = dependency('libui', required : false) if libui.found()