libcamera: Allow concurrent use of cameras from same pipeline handler

libcamera implements a pipeline handler locking mechanism based on
advisory locks on media devices, to prevent concurrent access to cameras
from the same pipeline handler from different processes (this only works
between multiple libcamera instances, as other processes won't use
advisory locks on media devices).

A side effect of the implementation prevents multiple cameras created by
the same pipeline handler from being used concurrently. Fix this by
turning the PipelineHandler lock() and unlock() functions into acquire()
and release(), with a use count to replace the boolean lock flag. The
Camera class is updated accordingly.

As a consequence of this change, the IPU3 pipeline handler will fail to
operate properly when the cameras it exposes are operated concurrently.
The android.hardware.camera2.cts.MultiViewTest#testDualCameraPreview
test fails as a result. This should be fixed in the IPU3 pipeline
handler to implement mutual exclusion between cameras.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>

4 files changed, 55 insertions, 29 deletions

diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
index 597426a6..38dd94ff 100644
--- a/include/libcamera/internal/camera.h
+++ b/include/libcamera/internal/camera.h
@@ -50,6 +50,7 @@ private:
 		CameraRunning,
 	};
 
+	bool isAcquired() const;
 	bool isRunning() const;
 	int isAccessAllowed(State state, bool allowDisconnected = false,
 			    const char *from = __builtin_FUNCTION()) const;
diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index c3e4c258..20f1cbb0 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -45,8 +45,8 @@ public:
 	MediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator,
 					const DeviceMatch &dm);
 
-	bool lock();
-	void unlock();
+	bool acquire();
+	void release();
 
 	virtual CameraConfiguration *generateConfiguration(Camera *camera,
 		const StreamRoles &roles) = 0;
@@ -77,6 +77,8 @@ protected:
 	CameraManager *manager_;
 
 private:
+	void unlockMediaDevices();
+
 	void mediaDeviceDisconnected(MediaDevice *media);
 	virtual void disconnect();
 
@@ -91,7 +93,7 @@ private:
 	const char *name_;
 
 	Mutex lock_;
-	bool lockOwner_ LIBCAMERA_TSA_GUARDED_BY(lock_); /* *Not* ownership of lock_ */
+	unsigned int useCount_ LIBCAMERA_TSA_GUARDED_BY(lock_);
 
 	friend class PipelineHandlerFactory;
 };
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 3910915c..a5c3aabe 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -508,6 +508,11 @@ static const char *const camera_state_names[] = {
 	"Running",
 };
 
+bool Camera::Private::isAcquired() const
+{
+	return state_.load(std::memory_order_acquire) == CameraRunning;
+}
+
 bool Camera::Private::isRunning() const
 {
 	return state_.load(std::memory_order_acquire) == CameraRunning;
@@ -811,7 +816,7 @@ int Camera::exportFrameBuffers(Stream *stream,
  * not blocking, if the device has already been acquired (by the same or another
  * process) the -EBUSY error code is returned.
  *
- * Acquiring a camera will limit usage of any other camera(s) provided by the
+ * Acquiring a camera may limit usage of any other camera(s) provided by the
  * same pipeline handler to the same instance of libcamera. The limit is in
  * effect until all cameras from the pipeline handler are released. Other
  * instances of libcamera can still list and examine the cameras but will fail
@@ -839,7 +844,7 @@ int Camera::acquire()
 	if (ret < 0)
 		return ret == -EACCES ? -EBUSY : ret;
 
-	if (!d->pipe_->lock()) {
+	if (!d->pipe_->acquire()) {
 		LOG(Camera, Info)
 			<< "Pipeline handler in use by another process";
 		return -EBUSY;
@@ -873,7 +878,8 @@ int Camera::release()
 	if (ret < 0)
 		return ret == -EACCES ? -EBUSY : ret;
 
-	d->pipe_->unlock();
+	if (d->isAcquired())
+		d->pipe_->release();
 
 	d->setState(Private::CameraAvailable);
 
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 67540533..7d2d00ef 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -68,7 +68,7 @@ LOG_DEFINE_CATEGORY(Pipeline)
  * respective factories.
  */
 PipelineHandler::PipelineHandler(CameraManager *manager)
-	: manager_(manager), lockOwner_(false)
+	: manager_(manager), useCount_(0)
 {
 }
 
@@ -143,58 +143,75 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,
 }
 
 /**
- * \brief Lock all media devices acquired by the pipeline
+ * \brief Acquire exclusive access to the pipeline handler for the process
  *
- * This function shall not be called from pipeline handler implementation, as
- * the Camera class handles locking directly.
+ * This function locks all the media devices used by the pipeline to ensure
+ * that no other process can access them concurrently.
+ *
+ * Access to a pipeline handler may be acquired recursively from within the
+ * same process. Every successful acquire() call shall be matched with a
+ * release() call. This allows concurrent access to the same pipeline handler
+ * from different cameras within the same process.
+ *
+ * Pipeline handlers shall not call this function directly as the Camera class
+ * handles access internally.
  *
  * \context This function is \threadsafe.
  *
- * \return True if the devices could be locked, false otherwise
- * \sa unlock()
- * \sa MediaDevice::lock()
+ * \return True if the pipeline handler was acquired, false if another process
+ * has already acquired it
+ * \sa release()
  */
-bool PipelineHandler::lock()
+bool PipelineHandler::acquire()
 {
 	MutexLocker locker(lock_);
 
-	/* Do not allow nested locking in the same libcamera instance. */
-	if (lockOwner_)
-		return false;
+	if (useCount_) {
+		++useCount_;
+		return true;
+	}
 
 	for (std::shared_ptr<MediaDevice> &media : mediaDevices_) {
 		if (!media->lock()) {
-			unlock();
+			unlockMediaDevices();
 			return false;
 		}
 	}
 
-	lockOwner_ = true;
-
+	++useCount_;
 	return true;
 }
 
 /**
- * \brief Unlock all media devices acquired by the pipeline
+ * \brief Release exclusive access to the pipeline handler
+ *
+ * This function releases access to the pipeline handler previously acquired by
+ * a call to acquire(). Every release() call shall match a previous successful
+ * acquire() call. Calling this function on a pipeline handler that hasn't been
+ * acquired results in undefined behaviour.
  *
- * This function shall not be called from pipeline handler implementation, as
- * the Camera class handles locking directly.
+ * Pipeline handlers shall not call this function directly as the Camera class
+ * handles access internally.
  *
  * \context This function is \threadsafe.
  *
- * \sa lock()
+ * \sa acquire()
  */
-void PipelineHandler::unlock()
+void PipelineHandler::release()
 {
 	MutexLocker locker(lock_);
 
-	if (!lockOwner_)
-		return;
+	ASSERT(useCount_);
 
+	unlockMediaDevices();
+
+	--useCount_;
+}
+
+void PipelineHandler::unlockMediaDevices()
+{
 	for (std::shared_ptr<MediaDevice> &media : mediaDevices_)
 		media->unlock();
-
-	lockOwner_ = false;
 }
 
 /**