[libcamera-devel] [PATCH] 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>
---
This will be used to simplify the cleanup path in lc-compliance without having
error messages [1].

Also, I'm not sure if I should add the silent parameter to the other
isAccessAllowed variant as well. It would make sense for consistency's sake, but
it's not needed currently.

[1] https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019703.html

 src/libcamera/camera.cpp | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 763f3b9926fd..baffdafc8146 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -350,7 +350,7 @@ public:
 	int isAccessAllowed(State state, bool allowDisconnected = false,
 			    const char *from = __builtin_FUNCTION()) const;
 	int isAccessAllowed(State low, State high,
-			    bool allowDisconnected = false,
+			    bool allowDisconnected = false, bool silent = false,
 			    const char *from = __builtin_FUNCTION()) const;
 
 	void disconnect();
@@ -408,7 +408,7 @@ int Camera::Private::isAccessAllowed(State state, bool allowDisconnected,
 }
 
 int Camera::Private::isAccessAllowed(State low, State high,
-				     bool allowDisconnected,
+				     bool allowDisconnected, bool silent,
 				     const char *from) const
 {
 	if (!allowDisconnected && disconnected_)
@@ -421,11 +421,12 @@ int Camera::Private::isAccessAllowed(State low, State high,
 	ASSERT(static_cast<unsigned int>(low) < std::size(camera_state_names) &&
 	       static_cast<unsigned int>(high) < std::size(camera_state_names));
 
-	LOG(Camera, Error) << "Camera in " << camera_state_names[currentState]
-			   << " state trying " << from
-			   << "() requiring state between "
-			   << camera_state_names[low] << " and "
-			   << camera_state_names[high];
+	if (!silent)
+		LOG(Camera, Error) << "Camera in " << camera_state_names[currentState]
+				   << " state trying " << from
+				   << "() requiring state between "
+				   << camera_state_names[low] << " and "
+				   << camera_state_names[high];
 
 	return -EACCES;
 }
@@ -1061,9 +1062,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 even if the camera isn't in the Running
+ * state as defined in \ref camera_operation, in which cases it is a no-op. It
+ * shall be synchronized by the caller with other functions that affect the
+ * camera state.
  *
  * \return 0 on success or a negative error code otherwise
  * \retval -ENODEV The camera has been disconnected from the system
@@ -1073,7 +1075,12 @@ int Camera::stop()
 {
 	Private *const d = LIBCAMERA_D_PTR();
 
-	int ret = d->isAccessAllowed(Private::CameraRunning);
+	int ret = d->isAccessAllowed(Private::CameraAvailable,
+			Private::CameraStopping, false, true);
+	if (!ret)
+		return 0;
+
+	ret = d->isAccessAllowed(Private::CameraRunning);
 	if (ret < 0)
 		return ret;
 
-- 
2.31.1