[libcamera-devel] [PATCH v4 16/22] 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---
Changes in v4:
- cosmetic changes

Changes in v3:
- don't notify all with the lock held

Changes in v2:
- remove mutex for isRunning_
- use V4L2Camera::isRunning() instead of V4L2CameraProxy::streaming_ in
  the dqbuf ioctl handler
---
 src/v4l2/v4l2_camera.cpp       | 34 +++++++++++++++++++++++++++++++---
 src/v4l2/v4l2_camera.h         |  9 +++++++--
 src/v4l2/v4l2_camera_proxy.cpp | 11 +++++++++--
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 177b1ea..f7df9b8 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,11 @@ 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,
@@ -192,7 +196,11 @@ int V4L2Camera::streamOff()
 	if (ret < 0)
 		return ret == -EACCES ? -EBUSY : ret;
 
-	isRunning_ = false;
+	{
+		MutexLocker locker(bufferMutex_);
+		isRunning_ = false;
+	}
+	bufferCV_.notify_all();
 
 	return 0;
 }
@@ -228,6 +236,26 @@ 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;
+}
+
 bool V4L2Camera::isRunning()
 {
 	return isRunning_;
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index ee53c2b..515e906 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -57,9 +57,10 @@ public:
 
 	int qbuf(unsigned int index);
 
-	bool isRunning();
+	void waitForBufferAvailable();
+	bool isBufferAvailable();
 
-	Semaphore bufferSema_;
+	bool isRunning();
 
 private:
 	void requestComplete(Request *request);
@@ -76,6 +77,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 9965a53..4c140eb 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -590,10 +590,17 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)
 		return -EINVAL;
 
 	if (!file->nonBlocking())
-		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 waitForBufferAvailable().
+	 */
+	if (!vcam_->isRunning())
+		return -EINVAL;
+
 	updateBuffers();
 
 	struct v4l2_buffer &buf = buffers_[currentBuf_];
-- 
2.27.0