[RFC,v1] libcamera: request: Split "Pending" state into two
diff mbox series

Message ID 20251201124415.1307419-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [RFC,v1] libcamera: request: Split "Pending" state into two
Related show

Commit Message

Barnabás Pőcze Dec. 1, 2025, 12:44 p.m. UTC
The "Pending" state at the moment is used for two very distinct scenarios:
when the application alone is managing the request, and when the request
is being processed. This duality makes it harder to track the true state of
a request and consequently it is harder to diagnose misuses.

So split the "Pending" state into the "Idle" and "InProgress" states to
make it explicit when a camera is managing the request. This improves
the detection of request double queueing, but since the status flag is
not updated atomically, it cannot detect all cases.

The name "Pending" is not kept because any application depending on it
has to be adjusted, so it is better to break the API as well. This is also
an ABI break.

Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 .../internal/tracepoints/request_enums.tp     |  7 ++--
 include/libcamera/request.h                   |  3 +-
 src/libcamera/camera.cpp                      |  2 +-
 src/libcamera/pipeline_handler.cpp            |  7 +++-
 src/libcamera/request.cpp                     | 34 ++++++++++++-------
 src/py/libcamera/py_main.cpp                  |  3 +-
 6 files changed, 36 insertions(+), 20 deletions(-)

Patch
diff mbox series

diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp
index bcbd1aaa2c..e9a5a78fac 100644
--- a/include/libcamera/internal/tracepoints/request_enums.tp
+++ b/include/libcamera/internal/tracepoints/request_enums.tp
@@ -9,8 +9,9 @@  TRACEPOINT_ENUM(
 	libcamera,
 	request_status,
 	TP_ENUM_VALUES(
-		ctf_enum_value("RequestPending", 0)
-		ctf_enum_value("RequestComplete", 1)
-		ctf_enum_value("RequestCancelled", 2)
+		ctf_enum_value("RequestIdle", 0)
+		ctf_enum_value("RequestInProgress", 1)
+		ctf_enum_value("RequestComplete", 2)
+		ctf_enum_value("RequestCancelled", 3)
 	)
 )
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 0c5939f7b3..1e7a5ca807 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -32,7 +32,8 @@  class Request : public Extensible
 
 public:
 	enum Status {
-		RequestPending,
+		RequestIdle,
+		RequestInProgress,
 		RequestComplete,
 		RequestCancelled,
 	};
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 2e1e146a25..b6273f6c29 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -1340,7 +1340,7 @@  int Camera::queueRequest(Request *request)
 		return -EXDEV;
 	}
 
-	if (request->status() != Request::RequestPending) {
+	if (request->status() != Request::RequestIdle) {
 		LOG(Camera, Error) << request->toString() << " is not valid";
 		return -EINVAL;
 	}
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 5c469e5bad..417b90245a 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -470,6 +470,8 @@  void PipelineHandler::queueRequest(Request *request)
 {
 	LIBCAMERA_TRACEPOINT(request_queue, request);
 
+	ASSERT(request->status() == Request::RequestIdle);
+
 	Camera *camera = request->_d()->camera();
 	Camera::Private *data = camera->_d();
 	data->waitingRequests_.push(request);
@@ -514,6 +516,7 @@  void PipelineHandler::doQueueRequests(Camera *camera)
 			break;
 
 		Request *request = data->waitingRequests_.front();
+		ASSERT(request->status() == Request::RequestInProgress);
 		if (!request->_d()->prepared_)
 			break;
 
@@ -591,9 +594,11 @@  void PipelineHandler::completeRequest(Request *request)
 
 	while (!data->queuedRequests_.empty()) {
 		Request *req = data->queuedRequests_.front();
-		if (req->status() == Request::RequestPending)
+		if (req->status() == Request::RequestInProgress)
 			break;
 
+		ASSERT(req->status() == Request::RequestCancelled ||
+		       req->status() == Request::RequestComplete);
 		ASSERT(!req->hasPendingBuffers());
 		data->queuedRequests_.pop_front();
 		camera->requestComplete(req);
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 60565f5984..89ad0d7c8d 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -121,7 +121,7 @@  void Request::Private::complete()
 {
 	Request *request = _o<Request>();
 
-	ASSERT(request->status() == RequestPending);
+	ASSERT(request->status() == RequestInProgress);
 	ASSERT(!hasPendingBuffers());
 
 	request->status_ = cancelled_ ? RequestCancelled : RequestComplete;
@@ -159,7 +159,7 @@  void Request::Private::cancel()
 	LIBCAMERA_TRACEPOINT(request_cancel, this);
 
 	Request *request = _o<Request>();
-	ASSERT(request->status() == RequestPending);
+	ASSERT(request->status() == RequestInProgress);
 
 	doCancelRequest();
 }
@@ -222,6 +222,8 @@  void Request::Private::emitPrepareCompleted()
  */
 void Request::Private::prepare(std::chrono::milliseconds timeout)
 {
+	_o<Request>()->status_ = RequestInProgress;
+
 	/* Create and connect one notifier for each synchronization fence. */
 	for (FrameBuffer *buffer : pending_) {
 		const Fence *fence = buffer->_d()->fence();
@@ -309,8 +311,10 @@  void Request::Private::timeout()
 /**
  * \enum Request::Status
  * Request completion status
- * \var Request::RequestPending
- * The request hasn't completed yet
+ * \var Request::RequestIdle
+ * The request is idle
+ * \var Request::RequestInProgress
+ * The request is being processed
  * \var Request::RequestComplete
  * The request has completed
  * \var Request::RequestCancelled
@@ -354,7 +358,7 @@  void Request::Private::timeout()
  */
 Request::Request(Camera *camera, uint64_t cookie)
 	: Extensible(std::make_unique<Private>(camera)),
-	  cookie_(cookie), status_(RequestPending)
+	  cookie_(cookie), status_(RequestIdle)
 {
 	controls_ = new ControlList(controls::controls,
 				    camera->_d()->validator());
@@ -391,6 +395,10 @@  void Request::reuse(ReuseFlag flags)
 {
 	LIBCAMERA_TRACEPOINT(request_reuse, this);
 
+	ASSERT(status_ == RequestIdle ||
+	       status_ == RequestComplete ||
+	       status_ == RequestCancelled);
+
 	_d()->reset();
 
 	if (flags & ReuseBuffers) {
@@ -403,7 +411,7 @@  void Request::reuse(ReuseFlag flags)
 		bufferMap_.clear();
 	}
 
-	status_ = RequestPending;
+	status_ = RequestIdle;
 
 	controls_->clear();
 	metadata_->clear();
@@ -563,11 +571,11 @@  uint32_t Request::sequence() const
  * \fn Request::status()
  * \brief Retrieve the request completion status
  *
- * The request status indicates whether the request has completed successfully
- * or with an error. When requests are created and before they complete the
- * request status is set to RequestPending, and is updated at completion time
- * to RequestComplete. If a request is cancelled at capture stop before it has
- * completed, its status is set to RequestCancelled.
+ * The request status indicates whether the request has completed successfully or with
+ * an error. When requests are created the request status is set to RequestIdle; when
+ * they are queued, the status is set to RequestInProgress. Then it is updated at
+ * completion time to RequestComplete. If a request is cancelled at capture stop before
+ * it has completed, its status is set to RequestCancelled.
  *
  * \return The request completion status
  */
@@ -607,8 +615,8 @@  std::string Request::toString() const
  */
 std::ostream &operator<<(std::ostream &out, const Request &r)
 {
-	/* Pending, Completed, Cancelled(X). */
-	static const char *statuses = "PCX";
+	/* Idle, InProgress(P), Completed, Cancelled(X). */
+	static const char statuses[] = "IPCX";
 
 	/* Example Output: Request(55:P:1/2:6523524) */
 	out << "Request(" << r.sequence() << ":" << statuses[r.status()] << ":"
diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
index a983ea75c3..d12f00a6f5 100644
--- a/src/py/libcamera/py_main.cpp
+++ b/src/py/libcamera/py_main.cpp
@@ -486,7 +486,8 @@  PYBIND11_MODULE(_libcamera, m)
 		.def("__str__", &Request::toString);
 
 	pyRequestStatus
-		.value("Pending", Request::RequestPending)
+		.value("Idle", Request::RequestIdle)
+		.value("InProgress", Request::RequestInProgress)
 		.value("Complete", Request::RequestComplete)
 		.value("Cancelled", Request::RequestCancelled);