From patchwork Wed Aug 25 04:44:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 13480 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 9F99ABD87C for ; Wed, 25 Aug 2021 04:44:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 29936688E4; Wed, 25 Aug 2021 06:44:29 +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="YYpM7Q4C"; dkim-atps=neutral Received: from mail-pg1-x535.google.com (mail-pg1-x535.google.com [IPv6:2607:f8b0:4864:20::535]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3481768892 for ; Wed, 25 Aug 2021 06:44:27 +0200 (CEST) Received: by mail-pg1-x535.google.com with SMTP id x4so21871874pgh.1 for ; Tue, 24 Aug 2021 21:44:27 -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=ndI1g/8W4kp/si65bsqgz/FPXoFB1QRpFBfbcRAu3Do=; b=YYpM7Q4CHaCMd9CKakG9Q3jBi+mndf7jLNBB1++CQWxDjZ0NaoCLBAQ3REIaDpZ6Pj xuvOZuEb6I5t1QB24dqeN14YrXP2jme3xmfa5aXMbFBxWr3F1VEUSNHnlwdkSBipn0gp Rh99PeVrCNt0tiV9vjYiXGH8lurFZaWDzsTNs= 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=ndI1g/8W4kp/si65bsqgz/FPXoFB1QRpFBfbcRAu3Do=; b=NfnHtPUNe7c440yjRZ+i82MzKA1fDro4qzba6gy0uKNcFdPLeRJ2osuyXS3KoL+frY GQxOACwV+kwbkzgEOQJJWHaclvBwgm1LBOhe6vRVYmQ5GP5VKPYWe3fRJhOVf1KsP+JN QZiNyqBrXYE/qDPHJqqJT5oDq5k67hwtXmjMex06qYhoPcoFU9aWcbzYaqhH9sxpC8bg v+ENAbQgkcPevkO6iQGCPWG77R9ygpowhlIMxH7gJeCkJVj2e38jvsgljazgBGwh3Ikg 7sQYtU/aQv7kfpbNDTdSm+SaHIDKrAnhOR+/zNirdvKpNUHpY3pNXJMm4Yn7MHL+0MjS rzZQ== X-Gm-Message-State: AOAM531UDqg2CpC/lHtAdowCssetpD/hgEj4CFRhQVDvH52JA4/sdpt+ xUBjIs2mqD8GK0B0Gz3xb8SuFZ83ssH+PQ== X-Google-Smtp-Source: ABdhPJzTv27s/vLomgconZK4u2TebYmCYKQLZ5SEtrd+E2N/LuDbchO3cSuUC1Yfs3bqSx6eYCVlZA== X-Received: by 2002:aa7:8e56:0:b029:3cd:c2ec:6c1c with SMTP id d22-20020aa78e560000b02903cdc2ec6c1cmr41939886pfr.80.1629866665551; Tue, 24 Aug 2021 21:44:25 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:203:ddfd:dd12:81a2:3205]) by smtp.gmail.com with ESMTPSA id n20sm2797544pfa.91.2021.08.24.21.44.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Aug 2021 21:44:25 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Wed, 25 Aug 2021 13:44:09 +0900 Message-Id: <20210825044410.2787433-3-hiroh@chromium.org> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog In-Reply-To: <20210825044410.2787433-1-hiroh@chromium.org> References: <20210825044410.2787433-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/3] android: camera_buffer: Map buffer in the first plane() call 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" CameraBuffer implementation maps a given buffer_handle_t in constructor. Mapping is redundant to only know the plane info like stride and offset. Mapping should be executed rater in the first plane() call. Signed-off-by: Hirokazu Honda --- src/android/mm/cros_camera_buffer.cpp | 71 ++++++++++++-------- src/android/mm/generic_camera_buffer.cpp | 85 ++++++++++++++++-------- 2 files changed, 101 insertions(+), 55 deletions(-) diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp index 50732637..42546d87 100644 --- a/src/android/mm/cros_camera_buffer.cpp +++ b/src/android/mm/cros_camera_buffer.cpp @@ -25,7 +25,7 @@ public: int flags); ~Private(); - bool isValid() const { return valid_; } + bool isValid() const { return registered_; } unsigned int numPlanes() const; @@ -34,10 +34,12 @@ public: size_t jpegBufferSize(size_t maxJpegBufferSize) const; private: + bool map(); + cros::CameraBufferManager *bufferManager_; buffer_handle_t handle_; unsigned int numPlanes_; - bool valid_; + bool mapped_; bool registered_; union { void *addr; @@ -50,7 +52,7 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, [[maybe_unused]] libcamera::PixelFormat pixelFormat, [[maybe_unused]] const libcamera::Size &size, [[maybe_unused]] int flags) - : handle_(camera3Buffer), numPlanes_(0), valid_(false), + : handle_(camera3Buffer), numPlanes_(0), mapped_(false), registered_(false) { bufferManager_ = cros::CameraBufferManager::GetInstance(); @@ -63,36 +65,11 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, registered_ = true; numPlanes_ = bufferManager_->GetNumPlanes(camera3Buffer); - switch (numPlanes_) { - case 1: { - ret = bufferManager_->Lock(handle_, 0, 0, 0, 0, 0, &mem.addr); - if (ret) { - LOG(HAL, Error) << "Single plane buffer mapping failed"; - return; - } - break; - } - case 2: - case 3: { - ret = bufferManager_->LockYCbCr(handle_, 0, 0, 0, 0, 0, - &mem.ycbcr); - if (ret) { - LOG(HAL, Error) << "YCbCr buffer mapping failed"; - return; - } - break; - } - default: - LOG(HAL, Error) << "Invalid number of planes: " << numPlanes_; - return; - } - - valid_ = true; } CameraBuffer::Private::~Private() { - if (valid_) + if (mapped_) bufferManager_->Unlock(handle_); if (registered_) bufferManager_->Deregister(handle_); @@ -105,6 +82,12 @@ unsigned int CameraBuffer::Private::numPlanes() const Span CameraBuffer::Private::plane(unsigned int plane) { + if (!mapped_) { + mapped_ = map(); + if (!mapped_) + return {}; + } + void *addr; switch (numPlanes()) { @@ -134,4 +117,34 @@ size_t CameraBuffer::Private::jpegBufferSize([[maybe_unused]] size_t maxJpegBuff return bufferManager_->GetPlaneSize(handle_, 0); } +bool CameraBuffer::Private::map() +{ + int ret; + switch (numPlanes_) { + case 1: { + ret = bufferManager_->Lock(handle_, 0, 0, 0, 0, 0, &mem.addr); + if (ret) { + LOG(HAL, Error) << "Single plane buffer mapping failed"; + return false; + } + break; + } + case 2: + case 3: { + ret = bufferManager_->LockYCbCr(handle_, 0, 0, 0, 0, 0, + &mem.ycbcr); + if (ret) { + LOG(HAL, Error) << "YCbCr buffer mapping failed"; + return false; + } + break; + } + default: + LOG(HAL, Error) << "Invalid number of planes: " << numPlanes_; + return false; + } + + return true; +} + PUBLIC_CAMERA_BUFFER_IMPLEMENTATION diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp index 0bfdc2ba..37868d26 100644 --- a/src/android/mm/generic_camera_buffer.cpp +++ b/src/android/mm/generic_camera_buffer.cpp @@ -37,6 +37,17 @@ public: size_t jpegBufferSize(size_t maxJpegBufferSize) const; private: + struct PlaneInfo { + unsigned int offset; + unsigned int size; + }; + + bool map(); + + int fd_; + int flags_; + off_t bufferLength_; + std::vector planeInfo_; /* \todo Remove planes_ when it will be added to MappedBuffer */ std::vector> planes_; }; @@ -45,6 +56,7 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer, libcamera::PixelFormat pixelFormat, const libcamera::Size &size, int flags) + : fd_(-1), flags_(flags) { error_ = 0; /* @@ -52,19 +64,18 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, * 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 || - camera3Buffer->data[i] == fd) { + camera3Buffer->data[i] == fd_) { continue; } - if (fd != -1) + if (fd_ != -1) LOG(HAL, Fatal) << "Discontiguous planes are not supported"; - fd = camera3Buffer->data[i]; + fd_ = camera3Buffer->data[i]; } - if (fd != -1) { + if (fd_ != -1) { error_ = -EINVAL; LOG(HAL, Error) << "No valid file descriptor"; return; @@ -78,23 +89,16 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, return; } - off_t bufferLength = lseek(fd, 0, SEEK_END); + size_t bufferLength = lseek(fd_, 0, SEEK_END); if (bufferLength < 0) { - error_ = -errno; + error_ = -EINVAL; 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); + planeInfo_.resize(numPlanes); + unsigned int offset = 0; for (unsigned int i = 0; i < numPlanes; ++i) { /* @@ -106,17 +110,17 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer, const unsigned int planeSize = stride * ((size.height + vertSubSample - 1) / vertSubSample); - planes_[i] = libcamera::Span( - static_cast(address) + offset, planeSize); + planeInfo_[i].offset = offset; + planeInfo_[i].size = planeSize; if (bufferLength < offset + planeSize) { - error_ = -EINVAL; - LOG(HAL, Error) << "Plane " << i << " is out of buffer" - << ", buffer length=" << bufferLength - << ", offset=" << offset - << ", size=" << planeSize; + LOG(HAL, Error) << "Plane " << i << " is out of buffer:" + << " plane offset=" << offset + << ", plane size=" << planeSize + << ", buffer length=" << bufferLength; return; } + offset += planeSize; } } @@ -127,11 +131,14 @@ CameraBuffer::Private::~Private() unsigned int CameraBuffer::Private::numPlanes() const { - return planes_.size(); + return planeInfo_.size(); } Span CameraBuffer::Private::plane(unsigned int plane) { + if (planes_.empty() && !map()) + return {}; + if (plane >= planes_.size()) return {}; @@ -140,8 +147,34 @@ Span CameraBuffer::Private::plane(unsigned int plane) size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const { - return std::min(maps_[0].size(), - maxJpegBufferSize); + if (maps_.empty()) { + ASSERT(bufferLength_ >= 0); + return std::min(bufferLength_, maxJpegBufferSize); + } + + return std::min(maps_[0].size(), maxJpegBufferSize); +} + +bool CameraBuffer::Private::map() +{ + ASSERT(fd_ != -1); + ASSERT(bufferLength_ >= 0); + + void *address = mmap(nullptr, bufferLength_, flags_, MAP_SHARED, fd_, 0); + if (address == MAP_FAILED) { + error_ = -errno; + LOG(HAL, Error) << "Failed to mmap plane"; + return false; + } + maps_.emplace_back(static_cast(address), bufferLength_); + + planes_.reserve(planeInfo_.size()); + for (const auto &info : planeInfo_) { + planes_.emplace_back( + static_cast(address) + info.offset, info.size); + } + + return true; } PUBLIC_CAMERA_BUFFER_IMPLEMENTATION