From patchwork Mon Dec 1 12:44:15 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 25280 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1EAF7BD80A for ; Mon, 1 Dec 2025 12:44:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9BBFA60B2F; Mon, 1 Dec 2025 13:44:21 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DOIWPsUF"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C873260A7B for ; Mon, 1 Dec 2025 13:44:19 +0100 (CET) Received: from pb-laptop.local (185.182.214.104.nat.pool.zt.hu [185.182.214.104]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 81EA46DF for ; Mon, 1 Dec 2025 13:42:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1764592926; bh=7bwcqolPjJ5nnSMO+zbTArRNQYteYTPMAbrR1S5DVCc=; h=From:To:Subject:Date:From; b=DOIWPsUFK3pg3OEdE5z17Gl3hOKn0JwsHb6cjAszLUmbGPaYJwUIZ2gVHsT4Lzx5P RVDILGeir9S52fuLnoHDv6Rpvv5ApKBgS/hsJ8tJXcGbdu/H+I2vl4XisgOYhI07aW 4HgRtGF6zcfbwzEalHzNhbYyDSjOvPd9XAjL4ajM= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [RFC PATCH v1] libcamera: request: Split "Pending" state into two Date: Mon, 1 Dec 2025 13:44:15 +0100 Message-ID: <20251201124415.1307419-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.52.0 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- .../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(-) 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(); - 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(); - 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()->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(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);