[libcamera-devel] [PATCH 4/5] libcamera: v4l2_device: Simplify exportBuffers()

Kieran Bingham kieran.bingham at ideasonboard.com
Thu Feb 7 22:21:18 CET 2019


exportBuffers() can only operate on an existing BufferPool allocation. The
pool identifies its size through its .count() method.

Passing a count in to the exportBuffers() call is redundant and can be
incorrect if the value is not the same as the BufferPool size.

Simplify the function and remove the unnecessary argument, correcting all uses
throughout the code base.

While we're here, remove the createBuffers() helper from the V4L2DeviceTest
which only served to obfuscate which pool the buffers were being allocated for.

Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
---
 src/libcamera/include/v4l2_device.h  |  2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp |  3 +--
 src/libcamera/pipeline/uvcvideo.cpp  |  2 +-
 src/libcamera/pipeline/vimc.cpp      |  2 +-
 src/libcamera/v4l2_device.cpp        | 15 ++++++---------
 test/v4l2_device/buffer_sharing.cpp  |  2 +-
 test/v4l2_device/capture_async.cpp   |  4 ++--
 test/v4l2_device/request_buffers.cpp |  4 ++--
 test/v4l2_device/stream_on_off.cpp   |  4 ++--
 test/v4l2_device/v4l2_device_test.h  |  2 --
 10 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
index 3acb5e466d64..98fa2110efb5 100644
--- a/src/libcamera/include/v4l2_device.h
+++ b/src/libcamera/include/v4l2_device.h
@@ -97,7 +97,7 @@ public:
 	int getFormat(V4L2DeviceFormat *format);
 	int setFormat(V4L2DeviceFormat *format);
 
-	int exportBuffers(unsigned int count, BufferPool *pool);
+	int exportBuffers(BufferPool *pool);
 	int importBuffers(BufferPool *pool);
 	int releaseBuffers();
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 34b03995ae31..677e127dd738 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -191,8 +191,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
 	if (!cfg.bufferCount)
 		return -EINVAL;
 
-	int ret = data->cio2_->exportBuffers(cfg.bufferCount,
-					     &stream->bufferPool());
+	int ret = data->cio2_->exportBuffers(&stream->bufferPool());
 	if (ret) {
 		LOG(IPU3, Error) << "Failed to request memory";
 		return ret;
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index ed8228bb2fc6..1a0dcb582670 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -102,7 +102,7 @@ int PipelineHandlerUVC::allocateBuffers(Camera *camera, Stream *stream)
 
 	LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
 
-	return video_->exportBuffers(cfg.bufferCount, &stream->bufferPool());
+	return video_->exportBuffers(&stream->bufferPool());
 }
 
 int PipelineHandlerUVC::freeBuffers(Camera *camera, Stream *stream)
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 0e9ad7b59ee5..06627b1cb847 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -101,7 +101,7 @@ int PipeHandlerVimc::allocateBuffers(Camera *camera, Stream *stream)
 
 	LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
 
-	return video_->exportBuffers(cfg.bufferCount, &stream->bufferPool());
+	return video_->exportBuffers(&stream->bufferPool());
 }
 
 int PipeHandlerVimc::freeBuffers(Camera *camera, Stream *stream)
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index c654e6dd7b8d..c2e4d0ea8db2 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -526,13 +526,12 @@ int V4L2Device::requestBuffers(unsigned int count)
 }
 
 /**
- * \brief Request \a count buffers to be allocated from the device and stored in
- * the buffer pool provided.
- * \param[in] count Number of buffers to allocate
+ * \brief Request buffers to be allocated from the device and stored in the
+ *  buffer pool provided.
  * \param[out] pool BufferPool to populate with buffers
  * \return 0 on success or a negative error code otherwise
  */
-int V4L2Device::exportBuffers(unsigned int count, BufferPool *pool)
+int V4L2Device::exportBuffers(BufferPool *pool)
 {
 	unsigned int allocatedBuffers;
 	unsigned int i;
@@ -540,21 +539,19 @@ int V4L2Device::exportBuffers(unsigned int count, BufferPool *pool)
 
 	memoryType_ = V4L2_MEMORY_MMAP;
 
-	ret = requestBuffers(count);
+	ret = requestBuffers(pool->count());
 	if (ret < 0)
 		return ret;
 
 	allocatedBuffers = ret;
-	if (allocatedBuffers < count) {
+	if (allocatedBuffers < pool->count()) {
 		LOG(V4L2, Error) << "Not enough buffers provided by V4L2Device";
 		requestBuffers(0);
 		return -ENOMEM;
 	}
 
-	count = ret;
-
 	/* Map the buffers. */
-	for (i = 0; i < count; ++i) {
+	for (i = 0; i < pool->count(); ++i) {
 		struct v4l2_plane planes[VIDEO_MAX_PLANES] = {};
 		struct v4l2_buffer buf = {};
 		struct Buffer &buffer = pool->buffers()[i];
diff --git a/test/v4l2_device/buffer_sharing.cpp b/test/v4l2_device/buffer_sharing.cpp
index 0e96f7b894bd..ca086f2728f1 100644
--- a/test/v4l2_device/buffer_sharing.cpp
+++ b/test/v4l2_device/buffer_sharing.cpp
@@ -80,7 +80,7 @@ protected:
 
 		pool_.createBuffers(bufferCount);
 
-		ret = dev_->exportBuffers(bufferCount, &pool_);
+		ret = dev_->exportBuffers(&pool_);
 		if (ret)
 			return TestFail;
 
diff --git a/test/v4l2_device/capture_async.cpp b/test/v4l2_device/capture_async.cpp
index 7a0735f65535..d3e20ed86b98 100644
--- a/test/v4l2_device/capture_async.cpp
+++ b/test/v4l2_device/capture_async.cpp
@@ -38,9 +38,9 @@ protected:
 		Timer timeout;
 		int ret;
 
-		createBuffers(bufferCount);
+		pool_.createBuffers(bufferCount);
 
-		ret = dev_->exportBuffers(bufferCount, &pool_);
+		ret = dev_->exportBuffers(&pool_);
 		if (ret)
 			return TestFail;
 
diff --git a/test/v4l2_device/request_buffers.cpp b/test/v4l2_device/request_buffers.cpp
index bc6ff2c18a57..26d7d9528982 100644
--- a/test/v4l2_device/request_buffers.cpp
+++ b/test/v4l2_device/request_buffers.cpp
@@ -19,9 +19,9 @@ protected:
 		 */
 		const unsigned int bufferCount = 8;
 
-		createBuffers(bufferCount);
+		pool_.createBuffers(bufferCount);
 
-		int ret = dev_->exportBuffers(bufferCount, &pool_);
+		int ret = dev_->exportBuffers(&pool_);
 		if (ret)
 			return TestFail;
 
diff --git a/test/v4l2_device/stream_on_off.cpp b/test/v4l2_device/stream_on_off.cpp
index b564d2a2ab67..861d8d695813 100644
--- a/test/v4l2_device/stream_on_off.cpp
+++ b/test/v4l2_device/stream_on_off.cpp
@@ -14,9 +14,9 @@ protected:
 	{
 		const unsigned int bufferCount = 8;
 
-		createBuffers(bufferCount);
+		pool_.createBuffers(bufferCount);
 
-		int ret = dev_->exportBuffers(bufferCount, &pool_);
+		int ret = dev_->exportBuffers(&pool_);
 		if (ret)
 			return TestFail;
 
diff --git a/test/v4l2_device/v4l2_device_test.h b/test/v4l2_device/v4l2_device_test.h
index f22f0bb555d8..43539655f85b 100644
--- a/test/v4l2_device/v4l2_device_test.h
+++ b/test/v4l2_device/v4l2_device_test.h
@@ -24,8 +24,6 @@ class V4L2DeviceTest : public Test
 public:
 	V4L2DeviceTest() : dev_(nullptr){};
 
-	void createBuffers(unsigned int qty) { pool_.createBuffers(qty); }
-
 protected:
 	int init();
 	void cleanup();
-- 
2.19.1



More information about the libcamera-devel mailing list