From patchwork Mon Aug 23 13:12: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: 13451 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 E8748BD87C for ; Mon, 23 Aug 2021 13:12:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AE0C4688CA; Mon, 23 Aug 2021 15:12:52 +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="oQtRZi4d"; dkim-atps=neutral Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CE245688A2 for ; Mon, 23 Aug 2021 15:12:50 +0200 (CEST) Received: by mail-pj1-x102a.google.com with SMTP id oc2-20020a17090b1c0200b00179e56772d6so8740143pjb.4 for ; Mon, 23 Aug 2021 06:12:50 -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=VMr5JtxGWOv7zUWIkxNhnnNOPhxrXVqs3DGz83bed2o=; b=oQtRZi4dzIa+jp/glNpWyFwy8GQ91ucY/Z976CgvNCp8Ms1gqXwk+oJMQFJuPUqB4w As8p5pQEYMvnrVhSdkqbdhWIFASUVHQr9cC5pk2X+NTTja5wLp4IGMUVCNw4m4ZRMZGO ZhtrmC5KtZ09oAay3AgYcVxpA/pG7cao2gGGk= 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=VMr5JtxGWOv7zUWIkxNhnnNOPhxrXVqs3DGz83bed2o=; b=U+hDxNS4WSjVcLFSlYpaN8iv2Q68sfSeRZ9f8wtc3iC1lQ7b2tZiyMzY1fsCWF7V/T kIkhI6ElCeUJqP1cEciBUEFESFR8t4BN8w/lcBfjb3BabfZyaFDzI2aCgzJWPOt9Rclx Vdj8WNXhs3ar6V67LBd2hoxMhbdLJrOIOaBBOr4HJhIYtKYlB3B5Y3yXhE9aL2bsjpaQ vwOkWBj8R32dmRfzCjHabbmfdoK9WA3A6vFtW+sW5y6ls/4X4mG0v0RV7V2oYsV+PbWJ g1sLnF9FKohFrpqdtRkCrhs7t6B+djMn2sFQjIA9mM+sIdfGFKhAIPSgTMFIEKUj9stP hRLg== X-Gm-Message-State: AOAM530+HzaJpvEVC/rEPttWRyPM+/U7QRzmPllAlpR4ePJoIAgVd/xQ abrAcPL/PkljJyUn4WTcLKBOMG59Re3FCA== X-Google-Smtp-Source: ABdhPJyPsSab1RGWFDgFHWDr4ZYputxNeETibeqf6j9Ppl3aA+NwX9rKFdladJPpi5JS6+lXWWa6fw== X-Received: by 2002:a17:90a:708c:: with SMTP id g12mr20365493pjk.13.1629724368327; Mon, 23 Aug 2021 06:12:48 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:203:184e:5d20:8fcb:dfcd]) by smtp.gmail.com with ESMTPSA id j4sm18596891pgi.6.2021.08.23.06.12.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Aug 2021 06:12:47 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Mon, 23 Aug 2021 22:12:19 +0900 Message-Id: <20210823131221.1034059-9-hiroh@chromium.org> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog In-Reply-To: <20210823131221.1034059-1-hiroh@chromium.org> References: <20210823131221.1034059-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH v2 08/10] libcamera: v4l2_videodevice: Create color-format planes in createBuffer() 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" V4L2VideDevice::createBuffer() creates the same number of FrameBuffer::Planes as V4L2 format planes. Therefore, if the v4l2 format single is single-planar format, the created number of FrameBuffer::Planes is 1. It should rather create the same number of FrameBuffer::Planes as the color format planes. Signed-off-by: Hirokazu Honda --- include/libcamera/internal/v4l2_videodevice.h | 9 +- src/libcamera/v4l2_videodevice.cpp | 147 ++++++++++++++---- test/v4l2_videodevice/buffer_cache.cpp | 6 +- 3 files changed, 128 insertions(+), 34 deletions(-) diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index e767ec84..69bb964a 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -114,8 +114,9 @@ struct V4L2Capability final : v4l2_capability { class V4L2BufferCache { public: - V4L2BufferCache(unsigned int numEntries); - V4L2BufferCache(const std::vector> &buffers); + V4L2BufferCache(unsigned int numEntries, unsigned int numPlanes); + V4L2BufferCache(const std::vector> &buffers, + unsigned int numPlanes); ~V4L2BufferCache(); int get(const FrameBuffer &buffer); @@ -126,7 +127,8 @@ private: { public: Entry(); - Entry(bool free, uint64_t lastUsed, const FrameBuffer &buffer); + Entry(bool free, uint64_t lastUsed, unsigned int numPlanes, + const FrameBuffer &buffer); bool operator==(const FrameBuffer &buffer) const; @@ -149,6 +151,7 @@ private: std::atomic lastUsedCounter_; std::vector cache_; + unsigned int numPlanes_; /* \todo Expose the miss counter through an instrumentation API. */ unsigned int missCounter_; }; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index ce60dff6..42b66bd3 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -25,6 +25,7 @@ #include +#include "libcamera/internal/formats.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/media_object.h" @@ -158,12 +159,13 @@ LOG_DECLARE_CATEGORY(V4L2) * \brief Create an empty cache with \a numEntries entries * \param[in] numEntries Number of entries to reserve in the cache * - * Create a cache with \a numEntries entries all marked as unused. The entries - * will be populated as the cache is used. This is typically used to implement - * buffer import, with buffers added to the cache as they are queued. + * Create a cache with \a numEntries entries all marked as unused. The entry is + * for \a numPlanes planes V4L2 buffer. The entries will be populated as the + * cache is used. This is typically used to implement buffer import, with + * buffers added to the cache as they are queued. */ -V4L2BufferCache::V4L2BufferCache(unsigned int numEntries) - : lastUsedCounter_(1), missCounter_(0) +V4L2BufferCache::V4L2BufferCache(unsigned int numEntries, unsigned int numPlanes) + : lastUsedCounter_(1), numPlanes_(numPlanes), missCounter_(0) { cache_.resize(numEntries); } @@ -172,16 +174,18 @@ V4L2BufferCache::V4L2BufferCache(unsigned int numEntries) * \brief Create a pre-populated cache * \param[in] buffers Array of buffers to pre-populated with * - * Create a cache pre-populated with \a buffers. This is typically used to - * implement buffer export, with all buffers added to the cache when they are - * allocated. + * Create a cache pre-populated with \a buffers. The entry is for \a numPlanes + * planes V4L2 buffer. This is typically used to implement buffer export, with + * all buffers added to the cache when they are allocated. */ -V4L2BufferCache::V4L2BufferCache(const std::vector> &buffers) - : lastUsedCounter_(1), missCounter_(0) +V4L2BufferCache::V4L2BufferCache(const std::vector> &buffers, + unsigned int numPlanes) + : lastUsedCounter_(1), numPlanes_(numPlanes), missCounter_(0) { for (const std::unique_ptr &buffer : buffers) cache_.emplace_back(true, lastUsedCounter_.fetch_add(1, std::memory_order_acq_rel), + numPlanes_, *buffer.get()); } @@ -237,6 +241,7 @@ int V4L2BufferCache::get(const FrameBuffer &buffer) cache_[use] = Entry(false, lastUsedCounter_.fetch_add(1, std::memory_order_acq_rel), + numPlanes_, buffer); return use; @@ -257,24 +262,47 @@ V4L2BufferCache::Entry::Entry() { } -V4L2BufferCache::Entry::Entry(bool free, uint64_t lastUsed, const FrameBuffer &buffer) +V4L2BufferCache::Entry::Entry(bool free, uint64_t lastUsed, + unsigned int numPlanes, const FrameBuffer &buffer) : free_(free), lastUsed_(lastUsed) { - for (const FrameBuffer::Plane &plane : buffer.planes()) - planes_.emplace_back(plane); + ASSERT(numPlanes <= buffer.planes().size()); + unsigned int length = 0; + for (const FrameBuffer::Plane &plane : buffer.planes()) { + ASSERT(plane.offset == length); + length += plane.length; + + if (planes_.size() < numPlanes) + planes_.emplace_back(plane); + } + + if (numPlanes == 1) { + planes_[0].length = length; + } } bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const { const std::vector &planes = buffer.planes(); - if (planes_.size() != planes.size()) + if (planes_.size() != planes.size() || planes_.size() == 1) return false; - - for (unsigned int i = 0; i < planes.size(); i++) - if (planes_[i].fd != planes[i].fd.fd() || - planes_[i].length != planes[i].length) + if (planes_.size() == 1) { + unsigned int length = 0; + for (unsigned int i = 0; i < planes.size(); i++) { + if (planes_[0].fd != planes[i].fd.fd()) + return false; + length += planes[i].length; + } + if (length != planes_[0].length) return false; + } else { + for (unsigned int i = 0; i < planes.size(); i++) + if (planes_[i].fd != planes[i].fd.fd() || + planes_[i].length != planes[i].length) + return false; + } + return true; } @@ -1152,8 +1180,13 @@ int V4L2VideoDevice::requestBuffers(unsigned int count, * successful return the driver's internal buffer management is initialized in * MMAP mode, and the video device is ready to accept queueBuffer() calls. * - * The number of planes and the plane sizes for the allocation are determined - * by the currently active format on the device as set by setFormat(). + * The number of planes and their offsets and sizes are determined by the + * currently active format on the device as set by setFormat(). They do not map + * to the V4L2 buffer planes, but to colour planes of the pixel format. For + * instance, if the active format is formats::NV12, the allocated FrameBuffer + * instances will have two planes, for the luma and chroma components, + * regardless of whether the device uses V4L2_PIX_FMT_NV12 or + * V4L2_PIX_FMT_NV12M. * * Buffers allocated with this function shall later be free with * releaseBuffers(). If buffers have already been allocated with @@ -1167,11 +1200,19 @@ int V4L2VideoDevice::requestBuffers(unsigned int count, int V4L2VideoDevice::allocateBuffers(unsigned int count, std::vector> *buffers) { - int ret = createBuffers(count, buffers); + V4L2DeviceFormat format{}; + int ret = getFormat(&format); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to get format: " << strerror(-ret); + return ret; + } + + ret = createBuffers(count, buffers); if (ret < 0) return ret; - cache_ = new V4L2BufferCache(*buffers); + cache_ = new V4L2BufferCache(*buffers, format.planesCount); memoryType_ = V4L2_MEMORY_MMAP; return ret; @@ -1190,8 +1231,13 @@ int V4L2VideoDevice::allocateBuffers(unsigned int count, * usable with any V4L2 video device in DMABUF mode, or with other dmabuf * importers. * - * The number of planes and the plane sizes for the allocation are determined - * by the currently active format on the device as set by setFormat(). + * The number of planes and their offsets and sizes are determined by the + * currently active format on the device as set by setFormat(). They do not map + * to the V4L2 buffer planes, but to colour planes of the pixel format. For + * instance, if the active format is formats::NV12, the allocated FrameBuffer + * instances will have two planes, for the luma and chroma components, + * regardless of whether the device uses V4L2_PIX_FMT_NV12 or + * V4L2_PIX_FMT_NV12M. * * Multiple independent sets of buffers can be allocated with multiple calls to * this function. Device-specific limitations may apply regarding the minimum @@ -1283,12 +1329,49 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) FrameBuffer::Plane plane; plane.fd = std::move(fd); - plane.length = multiPlanar ? - buf.m.planes[nplane].length : buf.length; + /* + * V4L2 API doesn't provide dmabuf offset information of plane. + * Set 0 as a placeholder offset. + * \todo Set the right offset once V4L2 API provides a way. + */ + plane.offset = 0; + plane.length = multiPlanar ? buf.m.planes[nplane].length : buf.length; planes.push_back(std::move(plane)); } + /* + * Get V4L2DeviceFormat of the frame buffer. If it is V4L2 single-planar + * format, overwrite FrameBuffer::Plane to make it color format planes. + * */ + V4L2DeviceFormat format{}; + ret = getFormat(&format); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to get format: " << strerror(-ret); + return nullptr; + } + + const auto &info = PixelFormatInfo::info(format.fourcc); + if (info.isValid() && info.numPlanes() != numPlanes) { + ASSERT(numPlanes == 1u); + const size_t numColorPlanes = info.numPlanes(); + planes.resize(numColorPlanes); + const FileDescriptor &fd = planes[0].fd; + size_t offset = 0; + for (size_t i = 0; i < numColorPlanes; ++i) { + planes[i].fd = fd; + planes[i].offset = offset; + + const unsigned int vertSubSample = + info.planes[i].verticalSubSampling; + planes[i].length = + info.stride(format.size.width, i, 1u) * + ((format.size.height + vertSubSample - 1) / vertSubSample); + offset += planes[i].length; + } + } + return std::make_unique(std::move(planes)); } @@ -1342,11 +1425,19 @@ int V4L2VideoDevice::importBuffers(unsigned int count) memoryType_ = V4L2_MEMORY_DMABUF; - int ret = requestBuffers(count, V4L2_MEMORY_DMABUF); + V4L2DeviceFormat format{}; + int ret = getFormat(&format); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to get format: " << strerror(-ret); + return ret; + } + + ret = requestBuffers(count, V4L2_MEMORY_DMABUF); if (ret) return ret; - cache_ = new V4L2BufferCache(count); + cache_ = new V4L2BufferCache(count, format.planesCount); LOG(V4L2, Debug) << "Prepared to import " << count << " buffers"; diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp index b3f2bec1..48f748bc 100644 --- a/test/v4l2_videodevice/buffer_cache.cpp +++ b/test/v4l2_videodevice/buffer_cache.cpp @@ -166,7 +166,7 @@ public: * Test cache of same size as there are buffers, the cache is * created from a list of buffers and will be pre-populated. */ - V4L2BufferCache cacheFromBuffers(buffers); + V4L2BufferCache cacheFromBuffers(buffers, 1u); if (testSequential(&cacheFromBuffers, buffers) != TestPass) return TestFail; @@ -181,7 +181,7 @@ public: * Test cache of same size as there are buffers, the cache is * not pre-populated. */ - V4L2BufferCache cacheFromNumbers(numBuffers); + V4L2BufferCache cacheFromNumbers(numBuffers, 1u); if (testSequential(&cacheFromNumbers, buffers) != TestPass) return TestFail; @@ -196,7 +196,7 @@ public: * Test cache half the size of number of buffers used, the cache * is not pre-populated. */ - V4L2BufferCache cacheHalf(numBuffers / 2); + V4L2BufferCache cacheHalf(numBuffers / 2, 1u); if (testRandom(&cacheHalf, buffers) != TestPass) return TestFail;