[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.

Hirokazu Honda hiroh at chromium.org
Thu Aug 26 13:25:37 CEST 2021


Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
---
 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<std::unique_ptr<FrameBuffer>> &buffers);
+	V4L2BufferCache(unsigned int numEntries, unsigned int numPlanes);
+	V4L2BufferCache(const std::vector<std::unique_ptr<FrameBuffer>> &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<uint64_t> lastUsedCounter_;
 	std::vector<Entry> 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 <libcamera/file_descriptor.h>
 
+#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<std::unique_ptr<FrameBuffer>> &buffers)
-	: lastUsedCounter_(1), missCounter_(0)
+V4L2BufferCache::V4L2BufferCache(const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
+				 unsigned int numPlanes)
+	: lastUsedCounter_(1), numPlanes_(numPlanes), missCounter_(0)
 {
 	for (const std::unique_ptr<FrameBuffer> &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<FrameBuffer::Plane> &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<FrameBuffer> 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<FrameBuffer>(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;
-- 
2.33.0.rc2.250.ged5fa647cd-goog



More information about the libcamera-devel mailing list