{"id":13451,"url":"https://patchwork.libcamera.org/api/1.1/patches/13451/?format=json","web_url":"https://patchwork.libcamera.org/patch/13451/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210823131221.1034059-9-hiroh@chromium.org>","date":"2021-08-23T13:12:19","name":"[libcamera-devel,RFC,v2,08/10] libcamera: v4l2_videodevice: Create color-format planes in createBuffer()","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"0a36f1236f9fb2e01d7e841a1b5645beff9dafdc","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/1.1/people/63/?format=json","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/13451/mbox/","series":[{"id":2386,"url":"https://patchwork.libcamera.org/api/1.1/series/2386/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2386","date":"2021-08-23T13:12:11","name":"Add offset to FrameBuffer::Plane","version":2,"mbox":"https://patchwork.libcamera.org/series/2386/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13451/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13451/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E8748BD87C\n\tfor ;\n\tMon, 23 Aug 2021 13:12:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE0C4688CA;\n\tMon, 23 Aug 2021 15:12:52 +0200 (CEST)","from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com\n\t[IPv6:2607:f8b0:4864:20::102a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE245688A2\n\tfor ;\n\tMon, 23 Aug 2021 15:12:50 +0200 (CEST)","by mail-pj1-x102a.google.com with SMTP id\n\toc2-20020a17090b1c0200b00179e56772d6so8740143pjb.4\n\tfor ;\n\tMon, 23 Aug 2021 06:12:50 -0700 (PDT)","from hiroh2.tok.corp.google.com\n\t([2401:fa00:8f:203:184e:5d20:8fcb:dfcd])\n\tby smtp.gmail.com with ESMTPSA id\n\tj4sm18596891pgi.6.2021.08.23.06.12.45\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 23 Aug 2021 06:12:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"oQtRZi4d\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=VMr5JtxGWOv7zUWIkxNhnnNOPhxrXVqs3DGz83bed2o=;\n\tb=oQtRZi4dzIa+jp/glNpWyFwy8GQ91ucY/Z976CgvNCp8Ms1gqXwk+oJMQFJuPUqB4w\n\tAs8p5pQEYMvnrVhSdkqbdhWIFASUVHQr9cC5pk2X+NTTja5wLp4IGMUVCNw4m4ZRMZGO\n\tZhtrmC5KtZ09oAay3AgYcVxpA/pG7cao2gGGk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=VMr5JtxGWOv7zUWIkxNhnnNOPhxrXVqs3DGz83bed2o=;\n\tb=U+hDxNS4WSjVcLFSlYpaN8iv2Q68sfSeRZ9f8wtc3iC1lQ7b2tZiyMzY1fsCWF7V/T\n\tkIkhI6ElCeUJqP1cEciBUEFESFR8t4BN8w/lcBfjb3BabfZyaFDzI2aCgzJWPOt9Rclx\n\tVdj8WNXhs3ar6V67LBd2hoxMhbdLJrOIOaBBOr4HJhIYtKYlB3B5Y3yXhE9aL2bsjpaQ\n\tvwOkWBj8R32dmRfzCjHabbmfdoK9WA3A6vFtW+sW5y6ls/4X4mG0v0RV7V2oYsV+PbWJ\n\tg1sLnF9FKohFrpqdtRkCrhs7t6B+djMn2sFQjIA9mM+sIdfGFKhAIPSgTMFIEKUj9stP\n\thRLg==","X-Gm-Message-State":"AOAM530+HzaJpvEVC/rEPttWRyPM+/U7QRzmPllAlpR4ePJoIAgVd/xQ\n\tabrAcPL/PkljJyUn4WTcLKBOMG59Re3FCA==","X-Google-Smtp-Source":"ABdhPJyPsSab1RGWFDgFHWDr4ZYputxNeETibeqf6j9Ppl3aA+NwX9rKFdladJPpi5JS6+lXWWa6fw==","X-Received":"by 2002:a17:90a:708c:: with SMTP id\n\tg12mr20365493pjk.13.1629724368327; \n\tMon, 23 Aug 2021 06:12:48 -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","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [RFC PATCH v2 08/10] libcamera: v4l2_videodevice:\n\tCreate color-format planes in createBuffer()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"V4L2VideDevice::createBuffer() creates the same number of\nFrameBuffer::Planes as V4L2 format planes. Therefore, if the v4l2\nformat single is single-planar format, the created number of\nFrameBuffer::Planes is 1.\nIt should rather create the same number of FrameBuffer::Planes as the\ncolor format planes.\n\nSigned-off-by: Hirokazu Honda \n---\n include/libcamera/internal/v4l2_videodevice.h | 9 +-\n src/libcamera/v4l2_videodevice.cpp | 147 ++++++++++++++----\n test/v4l2_videodevice/buffer_cache.cpp | 6 +-\n 3 files changed, 128 insertions(+), 34 deletions(-)","diff":"diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\nindex e767ec84..69bb964a 100644\n--- a/include/libcamera/internal/v4l2_videodevice.h\n+++ b/include/libcamera/internal/v4l2_videodevice.h\n@@ -114,8 +114,9 @@ struct V4L2Capability final : v4l2_capability {\n class V4L2BufferCache\n {\n public:\n-\tV4L2BufferCache(unsigned int numEntries);\n-\tV4L2BufferCache(const std::vector> &buffers);\n+\tV4L2BufferCache(unsigned int numEntries, unsigned int numPlanes);\n+\tV4L2BufferCache(const std::vector> &buffers,\n+\t\t\tunsigned int numPlanes);\n \t~V4L2BufferCache();\n \n \tint get(const FrameBuffer &buffer);\n@@ -126,7 +127,8 @@ private:\n \t{\n \tpublic:\n \t\tEntry();\n-\t\tEntry(bool free, uint64_t lastUsed, const FrameBuffer &buffer);\n+\t\tEntry(bool free, uint64_t lastUsed, unsigned int numPlanes,\n+\t\t const FrameBuffer &buffer);\n \n \t\tbool operator==(const FrameBuffer &buffer) const;\n \n@@ -149,6 +151,7 @@ private:\n \n \tstd::atomic lastUsedCounter_;\n \tstd::vector cache_;\n+\tunsigned int numPlanes_;\n \t/* \\todo Expose the miss counter through an instrumentation API. */\n \tunsigned int missCounter_;\n };\ndiff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\nindex ce60dff6..42b66bd3 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -25,6 +25,7 @@\n \n #include \n \n+#include \"libcamera/internal/formats.h\"\n #include \"libcamera/internal/media_device.h\"\n #include \"libcamera/internal/media_object.h\"\n \n@@ -158,12 +159,13 @@ LOG_DECLARE_CATEGORY(V4L2)\n * \\brief Create an empty cache with \\a numEntries entries\n * \\param[in] numEntries Number of entries to reserve in the cache\n *\n- * Create a cache with \\a numEntries entries all marked as unused. The entries\n- * will be populated as the cache is used. This is typically used to implement\n- * buffer import, with buffers added to the cache as they are queued.\n+ * Create a cache with \\a numEntries entries all marked as unused. The entry is\n+ * for \\a numPlanes planes V4L2 buffer. The entries will be populated as the\n+ * cache is used. This is typically used to implement buffer import, with\n+ * buffers added to the cache as they are queued.\n */\n-V4L2BufferCache::V4L2BufferCache(unsigned int numEntries)\n-\t: lastUsedCounter_(1), missCounter_(0)\n+V4L2BufferCache::V4L2BufferCache(unsigned int numEntries, unsigned int numPlanes)\n+\t: lastUsedCounter_(1), numPlanes_(numPlanes), missCounter_(0)\n {\n \tcache_.resize(numEntries);\n }\n@@ -172,16 +174,18 @@ V4L2BufferCache::V4L2BufferCache(unsigned int numEntries)\n * \\brief Create a pre-populated cache\n * \\param[in] buffers Array of buffers to pre-populated with\n *\n- * Create a cache pre-populated with \\a buffers. This is typically used to\n- * implement buffer export, with all buffers added to the cache when they are\n- * allocated.\n+ * Create a cache pre-populated with \\a buffers. The entry is for \\a numPlanes\n+ * planes V4L2 buffer. This is typically used to implement buffer export, with\n+ * all buffers added to the cache when they are allocated.\n */\n-V4L2BufferCache::V4L2BufferCache(const std::vector> &buffers)\n-\t: lastUsedCounter_(1), missCounter_(0)\n+V4L2BufferCache::V4L2BufferCache(const std::vector> &buffers,\n+\t\t\t\t unsigned int numPlanes)\n+\t: lastUsedCounter_(1), numPlanes_(numPlanes), missCounter_(0)\n {\n \tfor (const std::unique_ptr &buffer : buffers)\n \t\tcache_.emplace_back(true,\n \t\t\t\t lastUsedCounter_.fetch_add(1, std::memory_order_acq_rel),\n+\t\t\t\t numPlanes_,\n \t\t\t\t *buffer.get());\n }\n \n@@ -237,6 +241,7 @@ int V4L2BufferCache::get(const FrameBuffer &buffer)\n \n \tcache_[use] = Entry(false,\n \t\t\t lastUsedCounter_.fetch_add(1, std::memory_order_acq_rel),\n+\t\t\t numPlanes_,\n \t\t\t buffer);\n \n \treturn use;\n@@ -257,24 +262,47 @@ V4L2BufferCache::Entry::Entry()\n {\n }\n \n-V4L2BufferCache::Entry::Entry(bool free, uint64_t lastUsed, const FrameBuffer &buffer)\n+V4L2BufferCache::Entry::Entry(bool free, uint64_t lastUsed,\n+\t\t\t unsigned int numPlanes, const FrameBuffer &buffer)\n \t: free_(free), lastUsed_(lastUsed)\n {\n-\tfor (const FrameBuffer::Plane &plane : buffer.planes())\n-\t\tplanes_.emplace_back(plane);\n+\tASSERT(numPlanes <= buffer.planes().size());\n+\tunsigned int length = 0;\n+\tfor (const FrameBuffer::Plane &plane : buffer.planes()) {\n+\t\tASSERT(plane.offset == length);\n+\t\tlength += plane.length;\n+\n+\t\tif (planes_.size() < numPlanes)\n+\t\t\tplanes_.emplace_back(plane);\n+\t}\n+\n+\tif (numPlanes == 1) {\n+\t\tplanes_[0].length = length;\n+\t}\n }\n \n bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const\n {\n \tconst std::vector &planes = buffer.planes();\n \n-\tif (planes_.size() != planes.size())\n+\tif (planes_.size() != planes.size() || planes_.size() == 1)\n \t\treturn false;\n-\n-\tfor (unsigned int i = 0; i < planes.size(); i++)\n-\t\tif (planes_[i].fd != planes[i].fd.fd() ||\n-\t\t planes_[i].length != planes[i].length)\n+\tif (planes_.size() == 1) {\n+\t\tunsigned int length = 0;\n+\t\tfor (unsigned int i = 0; i < planes.size(); i++) {\n+\t\t\tif (planes_[0].fd != planes[i].fd.fd())\n+\t\t\t\treturn false;\n+\t\t\tlength += planes[i].length;\n+\t\t}\n+\t\tif (length != planes_[0].length)\n \t\t\treturn false;\n+\t} else {\n+\t\tfor (unsigned int i = 0; i < planes.size(); i++)\n+\t\t\tif (planes_[i].fd != planes[i].fd.fd() ||\n+\t\t\t planes_[i].length != planes[i].length)\n+\t\t\t\treturn false;\n+\t}\n+\n \treturn true;\n }\n \n@@ -1152,8 +1180,13 @@ int V4L2VideoDevice::requestBuffers(unsigned int count,\n * successful return the driver's internal buffer management is initialized in\n * MMAP mode, and the video device is ready to accept queueBuffer() calls.\n *\n- * The number of planes and the plane sizes for the allocation are determined\n- * by the currently active format on the device as set by setFormat().\n+ * The number of planes and their offsets and sizes are determined by the\n+ * currently active format on the device as set by setFormat(). They do not map\n+ * to the V4L2 buffer planes, but to colour planes of the pixel format. For\n+ * instance, if the active format is formats::NV12, the allocated FrameBuffer\n+ * instances will have two planes, for the luma and chroma components,\n+ * regardless of whether the device uses V4L2_PIX_FMT_NV12 or\n+ * V4L2_PIX_FMT_NV12M.\n *\n * Buffers allocated with this function shall later be free with\n * releaseBuffers(). If buffers have already been allocated with\n@@ -1167,11 +1200,19 @@ int V4L2VideoDevice::requestBuffers(unsigned int count,\n int V4L2VideoDevice::allocateBuffers(unsigned int count,\n \t\t\t\t std::vector> *buffers)\n {\n-\tint ret = createBuffers(count, buffers);\n+\tV4L2DeviceFormat format{};\n+\tint ret = getFormat(&format);\n+\tif (ret < 0) {\n+\t\tLOG(V4L2, Error)\n+\t\t\t<< \"Failed to get format: \" << strerror(-ret);\n+\t\treturn ret;\n+\t}\n+\n+\tret = createBuffers(count, buffers);\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tcache_ = new V4L2BufferCache(*buffers);\n+\tcache_ = new V4L2BufferCache(*buffers, format.planesCount);\n \tmemoryType_ = V4L2_MEMORY_MMAP;\n \n \treturn ret;\n@@ -1190,8 +1231,13 @@ int V4L2VideoDevice::allocateBuffers(unsigned int count,\n * usable with any V4L2 video device in DMABUF mode, or with other dmabuf\n * importers.\n *\n- * The number of planes and the plane sizes for the allocation are determined\n- * by the currently active format on the device as set by setFormat().\n+ * The number of planes and their offsets and sizes are determined by the\n+ * currently active format on the device as set by setFormat(). They do not map\n+ * to the V4L2 buffer planes, but to colour planes of the pixel format. For\n+ * instance, if the active format is formats::NV12, the allocated FrameBuffer\n+ * instances will have two planes, for the luma and chroma components,\n+ * regardless of whether the device uses V4L2_PIX_FMT_NV12 or\n+ * V4L2_PIX_FMT_NV12M.\n *\n * Multiple independent sets of buffers can be allocated with multiple calls to\n * this function. Device-specific limitations may apply regarding the minimum\n@@ -1283,12 +1329,49 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index)\n \n \t\tFrameBuffer::Plane plane;\n \t\tplane.fd = std::move(fd);\n-\t\tplane.length = multiPlanar ?\n-\t\t\tbuf.m.planes[nplane].length : buf.length;\n+\t\t/*\n+\t\t * V4L2 API doesn't provide dmabuf offset information of plane.\n+\t\t * Set 0 as a placeholder offset.\n+\t\t * \\todo Set the right offset once V4L2 API provides a way.\n+\t\t */\n+\t\tplane.offset = 0;\n+\t\tplane.length = multiPlanar ? buf.m.planes[nplane].length : buf.length;\n \n \t\tplanes.push_back(std::move(plane));\n \t}\n \n+\t/*\n+\t * Get V4L2DeviceFormat of the frame buffer. If it is V4L2 single-planar\n+\t * format, overwrite FrameBuffer::Plane to make it color format planes.\n+\t * */\n+\tV4L2DeviceFormat format{};\n+\tret = getFormat(&format);\n+\tif (ret < 0) {\n+\t\tLOG(V4L2, Error)\n+\t\t\t<< \"Failed to get format: \" << strerror(-ret);\n+\t\treturn nullptr;\n+\t}\n+\n+\tconst auto &info = PixelFormatInfo::info(format.fourcc);\n+\tif (info.isValid() && info.numPlanes() != numPlanes) {\n+\t\tASSERT(numPlanes == 1u);\n+\t\tconst size_t numColorPlanes = info.numPlanes();\n+\t\tplanes.resize(numColorPlanes);\n+\t\tconst FileDescriptor &fd = planes[0].fd;\n+\t\tsize_t offset = 0;\n+\t\tfor (size_t i = 0; i < numColorPlanes; ++i) {\n+\t\t\tplanes[i].fd = fd;\n+\t\t\tplanes[i].offset = offset;\n+\n+\t\t\tconst unsigned int vertSubSample =\n+\t\t\t\tinfo.planes[i].verticalSubSampling;\n+\t\t\tplanes[i].length =\n+\t\t\t\tinfo.stride(format.size.width, i, 1u) *\n+\t\t\t\t((format.size.height + vertSubSample - 1) / vertSubSample);\n+\t\t\toffset += planes[i].length;\n+\t\t}\n+\t}\n+\n \treturn std::make_unique(std::move(planes));\n }\n \n@@ -1342,11 +1425,19 @@ int V4L2VideoDevice::importBuffers(unsigned int count)\n \n \tmemoryType_ = V4L2_MEMORY_DMABUF;\n \n-\tint ret = requestBuffers(count, V4L2_MEMORY_DMABUF);\n+\tV4L2DeviceFormat format{};\n+\tint ret = getFormat(&format);\n+\tif (ret < 0) {\n+\t\tLOG(V4L2, Error)\n+\t\t\t<< \"Failed to get format: \" << strerror(-ret);\n+\t\treturn ret;\n+\t}\n+\n+\tret = requestBuffers(count, V4L2_MEMORY_DMABUF);\n \tif (ret)\n \t\treturn ret;\n \n-\tcache_ = new V4L2BufferCache(count);\n+\tcache_ = new V4L2BufferCache(count, format.planesCount);\n \n \tLOG(V4L2, Debug) << \"Prepared to import \" << count << \" buffers\";\n \ndiff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp\nindex b3f2bec1..48f748bc 100644\n--- a/test/v4l2_videodevice/buffer_cache.cpp\n+++ b/test/v4l2_videodevice/buffer_cache.cpp\n@@ -166,7 +166,7 @@ public:\n \t\t * Test cache of same size as there are buffers, the cache is\n \t\t * created from a list of buffers and will be pre-populated.\n \t\t */\n-\t\tV4L2BufferCache cacheFromBuffers(buffers);\n+\t\tV4L2BufferCache cacheFromBuffers(buffers, 1u);\n \n \t\tif (testSequential(&cacheFromBuffers, buffers) != TestPass)\n \t\t\treturn TestFail;\n@@ -181,7 +181,7 @@ public:\n \t\t * Test cache of same size as there are buffers, the cache is\n \t\t * not pre-populated.\n \t\t */\n-\t\tV4L2BufferCache cacheFromNumbers(numBuffers);\n+\t\tV4L2BufferCache cacheFromNumbers(numBuffers, 1u);\n \n \t\tif (testSequential(&cacheFromNumbers, buffers) != TestPass)\n \t\t\treturn TestFail;\n@@ -196,7 +196,7 @@ public:\n \t\t * Test cache half the size of number of buffers used, the cache\n \t\t * is not pre-populated.\n \t\t */\n-\t\tV4L2BufferCache cacheHalf(numBuffers / 2);\n+\t\tV4L2BufferCache cacheHalf(numBuffers / 2, 1u);\n \n \t\tif (testRandom(&cacheHalf, buffers) != TestPass)\n \t\t\treturn TestFail;\n","prefixes":["libcamera-devel","RFC","v2","08/10"]}