From patchwork Thu Aug 26 08:00:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 13498 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 59ABBBD87C for ; Thu, 26 Aug 2021 08:00:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 21D7E68911; Thu, 26 Aug 2021 10:00:35 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="i5T+Mon+"; dkim-atps=neutral Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AC045688C4 for ; Thu, 26 Aug 2021 10:00:31 +0200 (CEST) Received: by mail-pg1-x52e.google.com with SMTP id n18so2376011pgm.12 for ; Thu, 26 Aug 2021 01:00:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=u+h2oOWzEIeI2WME3h5R0OSUyW5I4nshv74ofZZm5G4=; b=i5T+Mon+bPE97Cm1VEIJB0lSEExQJ2bHL8VTSPzDEVmKd0fitZ1Y+5Q0IegrCOHCNb 409NRQQ1u0/jYIssoubpyzVY9kk6H3DSXU03aJg9LwFQD5rJSNuRO3Fblp/0Qb41wAkl dopNmDviWG9BcLQ00EeeP+T2v1jFauBuXXNtA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=u+h2oOWzEIeI2WME3h5R0OSUyW5I4nshv74ofZZm5G4=; b=eBBEx/ooXeHxpLK63FB8X+ilvBqqi0XyChMgMMLOK496I5kguvp+NDESNfKB2ujTk7 FXn1Vgbm33DazpEdftJlv97VjxXcDsP+X+5XehPwUG4y1svUpr0UWC16SyWHaNEn/K+w HWtHjiXRqIcfxZDcy497AR1Sr0+6b/uEIVffY+r3Y0j72g1YPwmPS5IcJdKlYmxKKQhX il9JWTJ8syysP6UJv4PqNKDQykLxYmueBnETlE4aZCKwj4kxOgkz1fpo5/dltM7NBBXD GMUkmhxxrCG/zHoturQndAV6H5kfCNgSCscn+twkRxdh4i95trNn1QuTG3kkxljgNVPJ 1vbQ== X-Gm-Message-State: AOAM532b0gQWOKpzjK0ORDUjOqGEqxj7Nb8sJvvgrjcg25sBD6kmf0S5 zT59nz89uk/ku7vqOEmrsUklXItPjLkftw== X-Google-Smtp-Source: ABdhPJw5hDuMUARR1Gx4nSzt9kpMUXwWR80pU1+bF+CSDHUSElveT8MzHrTB9KOAZSYWe+3JqVDHuw== X-Received: by 2002:a63:1247:: with SMTP id 7mr2261483pgs.366.1629964829457; Thu, 26 Aug 2021 01:00:29 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:203:a5bc:b3dd:7208:bec1]) by smtp.gmail.com with ESMTPSA id g3sm2570805pgj.66.2021.08.26.01.00.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Aug 2021 01:00:29 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Thu, 26 Aug 2021 17:00:19 +0900 Message-Id: <20210826080021.3454520-2-hiroh@chromium.org> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog In-Reply-To: <20210826080021.3454520-1-hiroh@chromium.org> References: <20210826080021.3454520-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 1/3] android: generic_camera_buffer: Correct buffer mapping 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" buffer_handle_t doesn't provide sufficient info to map a buffer properly. cros::CameraBufferManager enables handling the buffer on ChromeOS, but no way is provided for other platforms. Therefore, we put the assumption that planes are in the same buffer and they are consecutive. This modifies the way of mapping in generic_camera_buffer with the assumption. Signed-off-by: Hirokazu Honda Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/android/camera_buffer.h | 14 ++- src/android/camera_stream.cpp | 4 +- src/android/mm/cros_camera_buffer.cpp | 7 +- src/android/mm/generic_camera_buffer.cpp | 103 ++++++++++++++++++----- 4 files changed, 100 insertions(+), 28 deletions(-) diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h index c4e3a9e7..87df2570 100644 --- a/src/android/camera_buffer.h +++ b/src/android/camera_buffer.h @@ -11,13 +11,17 @@ #include #include +#include +#include class CameraBuffer final : public libcamera::Extensible { LIBCAMERA_DECLARE_PRIVATE() public: - CameraBuffer(buffer_handle_t camera3Buffer, int flags); + CameraBuffer(buffer_handle_t camera3Buffer, + libcamera::PixelFormat pixelFormat, + const libcamera::Size &size, int flags); ~CameraBuffer(); bool isValid() const; @@ -31,8 +35,12 @@ public: }; #define PUBLIC_CAMERA_BUFFER_IMPLEMENTATION \ -CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) \ - : Extensible(std::make_unique(this, camera3Buffer, flags)) \ +CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, \ + libcamera::PixelFormat pixelFormat, \ + const libcamera::Size &size, int flags) \ + : Extensible(std::make_unique(this, camera3Buffer, \ + pixelFormat, size, \ + flags)) \ { \ } \ CameraBuffer::~CameraBuffer() \ diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 61b44183..01909ec7 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -110,7 +110,9 @@ int CameraStream::process(const libcamera::FrameBuffer &source, * \todo Buffer mapping and processing should be moved to a * separate thread. */ - CameraBuffer dest(camera3Dest, PROT_READ | PROT_WRITE); + const StreamConfiguration &output = configuration(); + CameraBuffer dest(camera3Dest, formats::MJPEG, output.size, + PROT_READ | PROT_WRITE); if (!dest.isValid()) { LOG(HAL, Error) << "Failed to map android blob buffer"; return -EINVAL; diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp index e8783ff8..50732637 100644 --- a/src/android/mm/cros_camera_buffer.cpp +++ b/src/android/mm/cros_camera_buffer.cpp @@ -20,8 +20,9 @@ class CameraBuffer::Private : public Extensible::Private LIBCAMERA_DECLARE_PUBLIC(CameraBuffer) public: - Private(CameraBuffer *cameraBuffer, - buffer_handle_t camera3Buffer, int flags); + Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer, + libcamera::PixelFormat pixelFormat, const libcamera::Size &size, + int flags); ~Private(); bool isValid() const { return valid_; } @@ -46,6 +47,8 @@ private: CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer, + [[maybe_unused]] libcamera::PixelFormat pixelFormat, + [[maybe_unused]] const libcamera::Size &size, [[maybe_unused]] int flags) : handle_(camera3Buffer), numPlanes_(0), valid_(false), registered_(false) diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp index b3af194c..22753490 100644 --- a/src/android/mm/generic_camera_buffer.cpp +++ b/src/android/mm/generic_camera_buffer.cpp @@ -12,6 +12,7 @@ #include +#include "libcamera/internal/formats.h" #include "libcamera/internal/mapped_framebuffer.h" using namespace libcamera; @@ -24,8 +25,9 @@ class CameraBuffer::Private : public Extensible::Private, LIBCAMERA_DECLARE_PUBLIC(CameraBuffer) public: - Private(CameraBuffer *cameraBuffer, - buffer_handle_t camera3Buffer, int flags); + Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer, + libcamera::PixelFormat pixelFormat, const libcamera::Size &size, + int flags); ~Private(); unsigned int numPlanes() const; @@ -33,35 +35,92 @@ public: Span plane(unsigned int plane); size_t jpegBufferSize(size_t maxJpegBufferSize) const; + +private: + /* \todo Remove planes_ when it will be added to MappedBuffer */ + std::vector> planes_; }; CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, - buffer_handle_t camera3Buffer, int flags) + buffer_handle_t camera3Buffer, + libcamera::PixelFormat pixelFormat, + const libcamera::Size &size, int flags) { - maps_.reserve(camera3Buffer->numFds); error_ = 0; + const auto &info = libcamera::PixelFormatInfo::info(pixelFormat); + if (!info.isValid()) { + error_ = -EINVAL; + LOG(HAL, Error) << "Invalid pixel format: " + << pixelFormat.toString(); + return; + } + + /* + * As Android doesn't offer an API to query buffer layouts, assume for + * now that the buffer is backed by a single dmabuf, with planes being + * stored contiguously. + */ + int fd = -1; for (int i = 0; i < camera3Buffer->numFds; i++) { - if (camera3Buffer->data[i] == -1) + if (camera3Buffer->data[i] == -1 || camera3Buffer->data[i] == fd) continue; - off_t length = lseek(camera3Buffer->data[i], 0, SEEK_END); - if (length < 0) { - error_ = -errno; - LOG(HAL, Error) << "Failed to query plane length"; - break; + if (fd != -1) { + error_ = -EINVAL; + LOG(HAL, Error) << "Discontiguous planes are not supported"; + return; } - void *address = mmap(nullptr, length, flags, MAP_SHARED, - camera3Buffer->data[i], 0); - if (address == MAP_FAILED) { - error_ = -errno; - LOG(HAL, Error) << "Failed to mmap plane"; - break; - } + fd = camera3Buffer->data[i]; + } - maps_.emplace_back(static_cast(address), - static_cast(length)); + if (fd == -1) { + error_ = -EINVAL; + LOG(HAL, Error) << "No valid file descriptor"; + return; + } + + off_t bufferLength = lseek(fd, 0, SEEK_END); + if (bufferLength < 0) { + error_ = -errno; + LOG(HAL, Error) << "Failed to get buffer length"; + return; + } + + void *address = mmap(nullptr, bufferLength, flags, MAP_SHARED, fd, 0); + if (address == MAP_FAILED) { + error_ = -errno; + LOG(HAL, Error) << "Failed to mmap plane"; + return; + } + maps_.emplace_back(static_cast(address), bufferLength); + + const unsigned int numPlanes = info.numPlanes(); + planes_.resize(numPlanes); + unsigned int offset = 0; + for (unsigned int i = 0; i < numPlanes; ++i) { + /* + * \todo Remove if this plane size computation function is + * added to PixelFormatInfo. + */ + const unsigned int vertSubSample = info.planes[i].verticalSubSampling; + const unsigned int stride = info.stride(size.width, i, 1u); + const unsigned int planeSize = + stride * ((size.height + vertSubSample - 1) / vertSubSample); + + planes_[i] = libcamera::Span( + static_cast(address) + offset, planeSize); + + if (bufferLength < offset + planeSize) { + error_ = -EINVAL; + LOG(HAL, Error) << "Plane " << i << " is out of buffer" + << ", buffer length=" << bufferLength + << ", offset=" << offset + << ", size=" << planeSize; + return; + } + offset += planeSize; } } @@ -71,15 +130,15 @@ CameraBuffer::Private::~Private() unsigned int CameraBuffer::Private::numPlanes() const { - return maps_.size(); + return planes_.size(); } Span CameraBuffer::Private::plane(unsigned int plane) { - if (plane >= maps_.size()) + if (plane >= planes_.size()) return {}; - return maps_[plane]; + return planes_[plane]; } size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const