From 566ccd75cafd0bd9d60d350405ca7d337786ae39 Mon Sep 17 00:00:00 2001
From: Paul Elder <paul.elder@ideasonboard.com>
Date: Tue, 16 Jun 2020 16:40:34 +0900
Subject: v4l2: v4l2_camera: Don't use libcamera::Semaphore for available
 buffers

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@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 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(-)

(limited to 'src/v4l2')

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 177b1ea5..f7df9b85 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 ee53c2b0..515e9060 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 7bf9e7f8..a222795d 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -588,10 +588,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_];
-- 
cgit v1.2.1