[libcamera-devel] [PATCH v11 1/5] libcamera: camera: Make stop() idempotent

[Nícolas F. R. A. Prado]

Make Camera::stop() idempotent so that it can be called in any state and
consecutive times. When called in any state other than CameraRunning, it
is a no-op. This simplifies the cleanup path for applications.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
---
Changes in v11:
- Thanks to Paul:
  - Updated statemachine test so that it always expects Camera::stop() to pass

No changes in v10

No changes in v9

No changes in v8

No changes in v7

No changes in v6

Changes in v5:
- Thanks to Laurent:
  - Simplified isRunning()

Changes in v4:
- Thanks to Jacopo:
  - Added \todo in Camera::stop()

No changes in v3

Changes in v2:
- Suggested by Kieran:
  - Add new isRunning() function instead of relying on isAccessAllowed()
- Suggested by Laurent:
  - Make stop()'s description clearer

 src/libcamera/camera.cpp     | 20 +++++++++++++++++---
 test/camera/statemachine.cpp | 15 ++++++++-------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index de0123aeafca..29f2d91d05d3 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -348,6 +348,7 @@ public:
 		const std::set<Stream *> &streams);
 	~Private();
 
+	bool isRunning() const;
 	int isAccessAllowed(State state, bool allowDisconnected = false,
 			    const char *from = __builtin_FUNCTION()) const;
 	int isAccessAllowed(State low, State high,
@@ -389,6 +390,11 @@ static const char *const camera_state_names[] = {
 	"Running",
 };
 
+bool Camera::Private::isRunning() const
+{
+	return state_.load(std::memory_order_acquire) == CameraRunning;
+}
+
 int Camera::Private::isAccessAllowed(State state, bool allowDisconnected,
 				     const char *from) const
 {
@@ -1062,9 +1068,10 @@ int Camera::start(const ControlList *controls)
  * This method stops capturing and processing requests immediately. All pending
  * requests are cancelled and complete synchronously in an error state.
  *
- * \context This function may only be called when the camera is in the Running
- * state as defined in \ref camera_operation, and shall be synchronized by the
- * caller with other functions that affect the camera state.
+ * \context This function may be called in any camera state as defined in \ref
+ * camera_operation, and shall be synchronized by the caller with other
+ * functions that affect the camera state. If called when the camera isn't
+ * running, it is a no-op.
  *
  * \return 0 on success or a negative error code otherwise
  * \retval -ENODEV The camera has been disconnected from the system
@@ -1074,6 +1081,13 @@ int Camera::stop()
 {
 	Private *const d = LIBCAMERA_D_PTR();
 
+	/*
+	 * \todo Make calling stop() when not in 'Running' part of the state
+	 * machine rather than take this shortcut
+	 */
+	if (!d->isRunning())
+		return 0;
+
 	int ret = d->isAccessAllowed(Private::CameraRunning);
 	if (ret < 0)
 		return ret;
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index f0c3d7764027..26fb5ca17139 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -41,13 +41,13 @@ protected:
 		if (camera_->queueRequest(&request) != -EACCES)
 			return TestFail;
 
-		if (camera_->stop() != -EACCES)
-			return TestFail;
-
 		/* Test operations which should pass. */
 		if (camera_->release())
 			return TestFail;
 
+		if (camera_->stop())
+			return TestFail;
+
 		/* Test valid state transitions, end in Acquired state. */
 		if (camera_->acquire())
 			return TestFail;
@@ -71,7 +71,8 @@ protected:
 		if (camera_->queueRequest(&request) != -EACCES)
 			return TestFail;
 
-		if (camera_->stop() != -EACCES)
+		/* Test operations which should pass. */
+		if (camera_->stop())
 			return TestFail;
 
 		/* Test valid state transitions, end in Configured state. */
@@ -97,14 +98,14 @@ protected:
 		if (camera_->queueRequest(&request1) != -EACCES)
 			return TestFail;
 
-		if (camera_->stop() != -EACCES)
-			return TestFail;
-
 		/* Test operations which should pass. */
 		std::unique_ptr<Request> request2 = camera_->createRequest();
 		if (!request2)
 			return TestFail;
 
+		if (camera_->stop())
+			return TestFail;
+
 		/* Test valid state transitions, end in Running state. */
 		if (camera_->release())
 			return TestFail;
-- 
2.32.0