[libcamera-devel] [PATCH 11/15] v4l2: v4l2_camera: Don't use libcamera::Semaphore for available buffers

[Paul Elder]

In V4L2, a blocked dqbuf should not not also block a streamoff. This
means that on streamoff, the blocked dqbuf must return (with error). We
cannot do this with the libcamera semaphore, so pull out the necessary
components of a semaphore, and put them into V4L2Camera, so that dqbuf
from V4L2CameraProxy can wait on a disjunct condition of the
availability of the semaphore or the stopping of the stream.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
---
 src/v4l2/v4l2_camera.cpp       | 32 ++++++++++++++++++++++++++++++--
 src/v4l2/v4l2_camera.h         |  8 +++++++-
 src/v4l2/v4l2_camera_proxy.cpp | 11 +++++++++--
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index fdbf461..f0ec54b 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -18,7 +18,7 @@ LOG_DECLARE_CATEGORY(V4L2Compat);
 
 V4L2Camera::V4L2Camera(std::shared_ptr<Camera> camera)
 	: camera_(camera), isRunning_(false), bufferAllocator_(nullptr),
-	  efd_(-1)
+	  efd_(-1), bufferAvailableCount_(0)
 {
 	camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);
 }
@@ -100,7 +100,9 @@ void V4L2Camera::requestComplete(Request *request)
 	if (ret != sizeof(data))
 		LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
 
-	bufferSema_.release();
+	MutexLocker locker(bufferMutex_);
+	bufferAvailableCount_++;
+	bufferCV_.notify_all();
 }
 
 int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
@@ -151,6 +153,7 @@ int V4L2Camera::allocBuffers(unsigned int count)
 void V4L2Camera::freeBuffers()
 {
 	Stream *stream = *camera_->streams().begin();
+
 	bufferAllocator_->free(stream);
 }
 
@@ -168,6 +171,7 @@ FileDescriptor V4L2Camera::getBufferFd(unsigned int index)
 
 int V4L2Camera::streamOn()
 {
+	MutexLocker locker(isRunningMutex_);
 	if (isRunning_)
 		return 0;
 
@@ -191,6 +195,7 @@ int V4L2Camera::streamOn()
 
 int V4L2Camera::streamOff()
 {
+	MutexLocker locker(isRunningMutex_);
 	/* \todo Restore buffers to reqbufs state? */
 	if (!isRunning_)
 		return 0;
@@ -200,12 +205,15 @@ int V4L2Camera::streamOff()
 		return ret == -EACCES ? -EBUSY : ret;
 
 	isRunning_ = false;
+	bufferCV_.notify_all();
 
 	return 0;
 }
 
 int V4L2Camera::qbuf(unsigned int index)
 {
+	MutexLocker locker(isRunningMutex_);
+
 	std::unique_ptr<Request> request =
 		std::unique_ptr<Request>(camera_->createRequest(index));
 	if (!request) {
@@ -234,3 +242,23 @@ int V4L2Camera::qbuf(unsigned int index)
 
 	return 0;
 }
+
+void V4L2Camera::waitForBufferAvailable()
+{
+	MutexLocker locker(bufferMutex_);
+	bufferCV_.wait(locker, [&] {
+			return bufferAvailableCount_ >= 1 || !isRunning_;
+			});
+	if (isRunning_)
+		bufferAvailableCount_--;
+}
+
+bool V4L2Camera::isBufferAvailable()
+{
+	MutexLocker locker(bufferMutex_);
+	if (bufferAvailableCount_ < 1)
+		return false;
+
+	bufferAvailableCount_--;
+	return true;
+}
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index 30114ed..81dcbe2 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -57,7 +57,8 @@ public:
 
 	int qbuf(unsigned int index);
 
-	Semaphore bufferSema_;
+	void waitForBufferAvailable();
+	bool isBufferAvailable();
 
 private:
 	void requestComplete(Request *request);
@@ -65,6 +66,7 @@ private:
 	std::shared_ptr<Camera> camera_;
 	std::unique_ptr<CameraConfiguration> config_;
 
+	Mutex isRunningMutex_;
 	bool isRunning_;
 
 	std::mutex bufferLock_;
@@ -74,6 +76,10 @@ private:
 	std::deque<std::unique_ptr<Buffer>> completedBuffers_;
 
 	int efd_;
+
+	Mutex bufferMutex_;
+	std::condition_variable bufferCV_;
+	unsigned int bufferAvailableCount_;
 };
 
 #endif /* __V4L2_CAMERA_H__ */
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 45e4656..079961a 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -648,10 +648,17 @@ int V4L2CameraProxy::vidioc_dqbuf(int fd, struct v4l2_buffer *arg)
 		return -EINVAL;
 
 	if (!nonBlockingFds_[fd])
-		vcam_->bufferSema_.acquire();
-	else if (!vcam_->bufferSema_.tryAcquire())
+		vcam_->waitForBufferAvailable();
+	else if (!vcam_->isBufferAvailable())
 		return -EAGAIN;
 
+	/*
+	 * We need to check here again in case stream was turned off while we
+	 * were blocked on dqbuf.
+	 */
+	if (!streaming_)
+		return -EINVAL;
+
 	updateBuffers();
 
 	struct v4l2_buffer &buf = buffers_[currentBuf_];
-- 
2.27.0