[RFC,v1,27/27] v4l2: v4l2_camera: Provide buffers one by one
diff mbox series

Message ID 20260618123844.656396-28-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • Misc. changes before request-buffer split
Related show

Commit Message

Barnabás Pőcze June 18, 2026, 12:38 p.m. UTC
Instead of processing a set of buffers at a time and cycling through
the buffers using `V4L2CameraProxy::currentBuf_`, add a new function
to query just the least recently completed buffer and use that to
service `VIDIOC_DQBUF`. This ensures that the reported index will denote
the buffer that was actually completed after the request-buffer split.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/v4l2/v4l2_camera.cpp       | 62 ++++++++++---------------
 src/v4l2/v4l2_camera.h         | 13 ++----
 src/v4l2/v4l2_camera_proxy.cpp | 84 +++++++++++++++++-----------------
 src/v4l2/v4l2_camera_proxy.h   |  2 -
 4 files changed, 69 insertions(+), 92 deletions(-)

Patch
diff mbox series

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 8c7de8e92b..90773e00bf 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -20,7 +20,7 @@  LOG_DECLARE_CATEGORY(V4L2Compat)
 
 V4L2Camera::V4L2Camera(std::shared_ptr<Camera> camera)
 	: camera_(camera), controls_(controls::controls), isRunning_(false),
-	  efd_(-1), bufferAvailableCount_(0)
+	  efd_(-1)
 {
 	camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);
 }
@@ -68,38 +68,25 @@  void V4L2Camera::unbind()
 	efd_ = -1;
 }
 
-std::vector<V4L2Camera::CompletedBuffer> V4L2Camera::completedBuffers()
-{
-	MutexLocker lock(bufferLock_);
-	std::vector v(std::move_iterator(completedBuffers_.begin()),
-		      std::move_iterator(completedBuffers_.end()));
-
-	completedBuffers_.clear();
-
-	return v;
-}
-
 void V4L2Camera::requestComplete(Request *request)
 {
 	if (request->status() == Request::RequestCancelled)
 		return;
 
 	/* We only have one stream at the moment. */
-	bufferLock_.lock();
-	FrameBuffer *buffer = request->buffers().begin()->second;
-	completedBuffers_.emplace_back(buffer->cookie(), buffer->metadata());
-	bufferLock_.unlock();
-
-	uint64_t data = 1;
-	int ret = ::write(efd_, &data, sizeof(data));
-	if (ret != sizeof(data))
-		LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
-
-	request->reuse();
 	{
 		MutexLocker locker(bufferMutex_);
-		bufferAvailableCount_++;
+		FrameBuffer *buffer = request->buffers().begin()->second;
+		completedBuffers_.emplace_back(buffer->cookie(), buffer->metadata());
+
+		uint64_t data = 1;
+		int ret = ::write(efd_, &data, sizeof(data));
+		if (ret != sizeof(data))
+			LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
+
+		request->reuse();
 	}
+
 	bufferCV_.notify_all();
 }
 
@@ -280,24 +267,23 @@  int V4L2Camera::qbuf(unsigned int index)
 	return 0;
 }
 
-void V4L2Camera::waitForBufferAvailable()
+std::optional<V4L2Camera::CompletedBuffer> V4L2Camera::nextBuffer(bool wait)
 {
 	MutexLocker locker(bufferMutex_);
-	bufferCV_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(bufferMutex_) {
-			       return bufferAvailableCount_ >= 1 || !isRunning_;
-		       });
-	if (isRunning_)
-		bufferAvailableCount_--;
-}
 
-bool V4L2Camera::isBufferAvailable()
-{
-	MutexLocker locker(bufferMutex_);
-	if (bufferAvailableCount_ < 1)
-		return false;
+	if (wait) {
+		bufferCV_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(bufferMutex_) {
+		       return !completedBuffers_.empty() || !isRunning_;
+		});
+	}
+
+	if (!isRunning_)
+		return {};
+
+	auto buffer = std::move(completedBuffers_.front());
+	completedBuffers_.pop_front();
 
-	bufferAvailableCount_--;
-	return true;
+	return buffer;
 }
 
 bool V4L2Camera::isRunning()
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index 1d90eb43e4..06e37077b2 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -9,6 +9,7 @@ 
 
 #include <deque>
 #include <memory>
+#include <optional>
 #include <vector>
 
 #include <libcamera/base/mutex.h>
@@ -41,8 +42,6 @@  public:
 	void bind(int efd);
 	void unbind();
 
-	std::vector<CompletedBuffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_);
-
 	int configure(libcamera::StreamConfiguration *streamConfigOut,
 		      const libcamera::Size &size,
 		      const libcamera::PixelFormat &pixelformat,
@@ -63,14 +62,12 @@  public:
 
 	int qbuf(unsigned int index);
 
-	void waitForBufferAvailable() LIBCAMERA_TSA_EXCLUDES(bufferMutex_);
-	bool isBufferAvailable() LIBCAMERA_TSA_EXCLUDES(bufferMutex_);
+	std::optional<CompletedBuffer> nextBuffer(bool wait) LIBCAMERA_TSA_EXCLUDES(bufferMutex_);
 
 	bool isRunning();
 
 private:
-	void requestComplete(libcamera::Request *request)
-		LIBCAMERA_TSA_EXCLUDES(bufferLock_);
+	void requestComplete(libcamera::Request *request);
 
 	std::shared_ptr<libcamera::Camera> camera_;
 	std::unique_ptr<libcamera::CameraConfiguration> config_;
@@ -79,18 +76,16 @@  private:
 
 	bool isRunning_;
 
-	libcamera::Mutex bufferLock_;
 	std::unique_ptr<libcamera::FrameBufferAllocator> bufferAllocator_;
 
 	std::vector<std::unique_ptr<libcamera::Request>> requestPool_;
 
 	std::deque<libcamera::Request *> pendingRequests_;
 	std::deque<CompletedBuffer> completedBuffers_
-		LIBCAMERA_TSA_GUARDED_BY(bufferLock_);
+		LIBCAMERA_TSA_GUARDED_BY(bufferMutex_);
 
 	int efd_;
 
 	libcamera::Mutex bufferMutex_;
 	libcamera::ConditionVariable bufferCV_;
-	unsigned int bufferAvailableCount_ LIBCAMERA_TSA_GUARDED_BY(bufferMutex_);
 };
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 820a417a9c..3a34e84071 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -39,9 +39,37 @@  using namespace std::literals::chrono_literals;
 
 LOG_DECLARE_CATEGORY(V4L2Compat)
 
+namespace {
+
+void updateBuffer(v4l2_buffer &buf, const FrameMetadata &fmd)
+{
+	switch (fmd.status) {
+	case FrameMetadata::FrameSuccess:
+		buf.bytesused = std::accumulate(fmd.planes().begin(),
+						fmd.planes().end(), 0,
+						[](unsigned int total, const auto &plane) {
+							return total + plane.bytesused;
+						});
+		buf.field = V4L2_FIELD_NONE;
+		buf.timestamp.tv_sec = fmd.timestamp / 1000000000;
+		buf.timestamp.tv_usec = (fmd.timestamp / 1000) % 1000000;
+		buf.sequence = fmd.sequence;
+
+		buf.flags |= V4L2_BUF_FLAG_DONE;
+		break;
+	case FrameMetadata::FrameError:
+		buf.flags |= V4L2_BUF_FLAG_ERROR;
+		break;
+	default:
+		break;
+	}
+}
+
+} /* namespace */
+
 V4L2CameraProxy::V4L2CameraProxy(unsigned int index,
 				 std::shared_ptr<Camera> camera)
-	: refcount_(0), index_(index), bufferCount_(0), currentBuf_(0),
+	: refcount_(0), index_(index), bufferCount_(0),
 	  vcam_(std::make_unique<V4L2Camera>(camera)), owner_(nullptr)
 {
 	querycap(camera);
@@ -241,36 +269,6 @@  void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
 	memset(capabilities_.reserved, 0, sizeof(capabilities_.reserved));
 }
 
-void V4L2CameraProxy::updateBuffers()
-{
-	std::vector<V4L2Camera::CompletedBuffer> completedBuffers = vcam_->completedBuffers();
-	for (const V4L2Camera::CompletedBuffer &buffer : completedBuffers) {
-		const FrameMetadata &fmd = buffer.data_;
-		struct v4l2_buffer &buf = buffers_[buffer.index_];
-
-		switch (fmd.status) {
-		case FrameMetadata::FrameSuccess:
-			buf.bytesused = std::accumulate(fmd.planes().begin(),
-							fmd.planes().end(), 0,
-							[](unsigned int total, const auto &plane) {
-								return total + plane.bytesused;
-							});
-			buf.field = V4L2_FIELD_NONE;
-			buf.timestamp.tv_sec = fmd.timestamp / 1000000000;
-			buf.timestamp.tv_usec = (fmd.timestamp / 1000) % 1000000;
-			buf.sequence = fmd.sequence;
-
-			buf.flags |= V4L2_BUF_FLAG_DONE;
-			break;
-		case FrameMetadata::FrameError:
-			buf.flags |= V4L2_BUF_FLAG_ERROR;
-			break;
-		default:
-			break;
-		}
-	}
-}
-
 int V4L2CameraProxy::vidioc_querycap(V4L2CameraFile *file, struct v4l2_capability *arg)
 {
 	LOG(V4L2Compat, Debug)
@@ -585,8 +583,6 @@  int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a
 	    arg->index >= bufferCount_)
 		return -EINVAL;
 
-	updateBuffers();
-
 	*arg = buffers_[arg->index];
 
 	return 0;
@@ -674,30 +670,34 @@  int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,
 	    !validateMemoryType(arg->memory))
 		return -EINVAL;
 
+	std::optional<V4L2Camera::CompletedBuffer> b;
+
 	if (!file->nonBlocking()) {
 		lock->unlock();
-		vcam_->waitForBufferAvailable();
+		b = vcam_->nextBuffer(true);
 		lock->lock();
-	} else if (!vcam_->isBufferAvailable())
-		return -EAGAIN;
+	} else {
+		b = vcam_->nextBuffer(false);
+	}
 
 	/*
 	 * We need to check here again in case stream was turned off while we
-	 * were blocked on waitForBufferAvailable().
+	 * were blocked in nextBuffer().
 	 */
 	if (!vcam_->isRunning())
 		return -EINVAL;
 
-	updateBuffers();
+	if (!b)
+		return -EAGAIN;
+
+	struct v4l2_buffer &buf = buffers_[b->index_];
 
-	struct v4l2_buffer &buf = buffers_[currentBuf_];
+	updateBuffer(buf, b->data_);
 
 	buf.flags &= ~(V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_PREPARED);
 	buf.length = sizeimage_;
 	*arg = buf;
 
-	currentBuf_ = (currentBuf_ + 1) % bufferCount_;
-
 	uint64_t data;
 	int ret = ::read(file->efd(), &data, sizeof(data));
 	if (ret != sizeof(data))
@@ -753,8 +753,6 @@  int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)
 	if (vcam_->isRunning())
 		return 0;
 
-	currentBuf_ = 0;
-
 	return vcam_->streamOn();
 }
 
diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
index 5aa352c36f..e8f3583aa6 100644
--- a/src/v4l2/v4l2_camera_proxy.h
+++ b/src/v4l2/v4l2_camera_proxy.h
@@ -44,7 +44,6 @@  private:
 	void querycap(std::shared_ptr<libcamera::Camera> camera);
 	int tryFormat(struct v4l2_format *arg);
 	enum v4l2_priority maxPriority();
-	void updateBuffers();
 	void freeBuffers();
 
 	int vidioc_querycap(V4L2CameraFile *file, struct v4l2_capability *arg);
@@ -81,7 +80,6 @@  private:
 
 	libcamera::StreamConfiguration streamConfig_;
 	unsigned int bufferCount_;
-	unsigned int currentBuf_;
 	unsigned int sizeimage_;
 
 	struct v4l2_capability capabilities_;