From patchwork Thu Aug 26 11:25:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 13507 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 85DADC3243 for ; Thu, 26 Aug 2021 11:26:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3F4876891B; Thu, 26 Aug 2021 13:26:02 +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="NAomP8cE"; dkim-atps=neutral Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EF6FD6891B for ; Thu, 26 Aug 2021 13:25:59 +0200 (CEST) Received: by mail-pl1-x636.google.com with SMTP id m4so1633017pll.0 for ; Thu, 26 Aug 2021 04:25:59 -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=RKGsxDXeIpUrZGD7PC/qMrhFninvZF/n+M+4dHhZyu8=; b=NAomP8cEGvvsXLbkdDFJlrWyryxfoGYJ9IKGalcxCkkrJPNU/9YhUOWki5/rwZyvoM PltQ7p8lC03qrlacnSVu/LTPpFBAZYUb+RMAiIJp8p06tejOICu8QFLg2hkCAkevF6HF PZU1Lpji1Gvdjf7S7RARihGYgR5EOHu5NuoQc= 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=RKGsxDXeIpUrZGD7PC/qMrhFninvZF/n+M+4dHhZyu8=; b=Pai30Chpt2/JJ//ZGHkbRyQm29b0WlcIcD5NEAXLmOStQpkryrsDt8xsJXR5dNPWD5 Lz0f3CJSSdSsEhnJ1rHmuO/e3clAVMZ7oyz1BRDwkb3MInia6YXAvq2HHMxVliB69/3E DFd9xJfORvZathHhjiXYHhgZ1F7eAGWHOprBO9U13AXV1hhjYYFle9q8fhwQgiiwI346 H8x8A5ztLkRi//rbnp9U1oj0K5O237ElCHj1C+SC/4uXcYqm4+1R4BPjK1CS5rhM6WXH IWchPG52bvcSjC8J+QZ6oJPA+pMm2zO1rp7pdCmw0744nFXnkwxpTBtNloQo5bW2XyKm SbYw== X-Gm-Message-State: AOAM531jGJaRfk7JoSG6mJyb0hOXpQpT+5f2NtBw+6s8RNM9aVeVT6ug ghTY/pbK2OUrZU3apiBsmXAgT/4QfuRXMg== X-Google-Smtp-Source: ABdhPJwXU7p2z3HT9a3DLxWOlPUWX3VUZCDAaB7HDdWzQOWN03BODRn2cs3sbuAuratVetVfhx1+lg== X-Received: by 2002:a17:90a:680a:: with SMTP id p10mr3760726pjj.179.1629977158131; Thu, 26 Aug 2021 04:25:58 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:203:a5bc:b3dd:7208:bec1]) by smtp.gmail.com with ESMTPSA id s29sm3472057pgl.38.2021.08.26.04.25.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Aug 2021 04:25:57 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Thu, 26 Aug 2021 20:25:37 +0900 Message-Id: <20210826112539.170694-8-hiroh@chromium.org> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog In-Reply-To: <20210826112539.170694-1-hiroh@chromium.org> References: <20210826112539.170694-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 7/9] 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. 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" Signed-off-by: Hirokazu Honda --- include/libcamera/internal/v4l2_videodevice.h | 10 +- src/libcamera/v4l2_videodevice.cpp | 141 ++++++++++++++---- test/v4l2_videodevice/buffer_cache.cpp | 6 +- 3 files changed, 123 insertions(+), 34 deletions(-) diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index e767ec84..bfda6726 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_; }; @@ -242,6 +245,7 @@ private: FrameBuffer *dequeueBuffer(); V4L2Capability caps_; + V4L2DeviceFormat format_; enum v4l2_buf_type bufferType_; enum v4l2_memory memoryType_; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 2ff25af2..f5f8741a 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" @@ -157,13 +158,15 @@ LOG_DECLARE_CATEGORY(V4L2) /** * \brief Create an empty cache with \a numEntries entries * \param[in] numEntries Number of entries to reserve in the cache + * \param[in] numPlanes Number of V4L2 buffer planes * - * 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); } @@ -171,17 +174,20 @@ V4L2BufferCache::V4L2BufferCache(unsigned int numEntries) /** * \brief Create a pre-populated cache * \param[in] buffers Array of buffers to pre-populated with + * \param[in] numPlanes Number of V4L2 buffer planes * - * 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 +243,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 +264,53 @@ 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) { + /* + * planes_ is V4L2 single-planar format buffer. fds of + * FrameBuffer::Plane must be identical and the sum of plane + * size is the V4L2 buffer length. + */ + 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 { + /* planes_ is V4L2 multi-planar format buffer. */ + 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; } @@ -579,6 +615,12 @@ int V4L2VideoDevice::open() << "Opened device " << caps_.bus_info() << ": " << caps_.driver() << ": " << caps_.card(); + ret = getFormat(&format_); + if (ret) { + LOG(V4L2, Error) << "Failed to get format"; + return ret; + } + return 0; } @@ -668,6 +710,12 @@ int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) << "Opened device " << caps_.bus_info() << ": " << caps_.driver() << ": " << caps_.card(); + ret = getFormat(&format_); + if (ret) { + LOG(V4L2, Error) << "Failed to get format"; + return ret; + } + return 0; } @@ -761,12 +809,19 @@ int V4L2VideoDevice::tryFormat(V4L2DeviceFormat *format) */ int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format) { + int ret = 0; if (caps_.isMeta()) - return trySetFormatMeta(format, true); + ret = trySetFormatMeta(format, true); else if (caps_.isMultiplanar()) - return trySetFormatMultiplane(format, true); + ret = trySetFormatMultiplane(format, true); else - return trySetFormatSingleplane(format, true); + ret = trySetFormatSingleplane(format, true); + + /* Cache the set format on success. */ + if (ret == 0) + format_ = *format; + + return ret; } int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format) @@ -1152,8 +1207,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 @@ -1171,7 +1231,7 @@ int V4L2VideoDevice::allocateBuffers(unsigned int count, if (ret < 0) return ret; - cache_ = new V4L2BufferCache(*buffers); + cache_ = new V4L2BufferCache(*buffers, format_.planesCount); memoryType_ = V4L2_MEMORY_MMAP; return ret; @@ -1190,8 +1250,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 @@ -1289,12 +1354,32 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) * \todo Set the right offset once V4L2 API provides a way. */ plane.offset = 0; - plane.length = multiPlanar ? - buf.m.planes[nplane].length : buf.length; + plane.length = multiPlanar ? buf.m.planes[nplane].length : buf.length; planes.push_back(std::move(plane)); } + 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; + + /* \todo Take the V4L2 stride into account */ + 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)); } @@ -1352,7 +1437,7 @@ int V4L2VideoDevice::importBuffers(unsigned int count) 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;