[{"id":37144,"web_url":"https://patchwork.libcamera.org/comment/37144/","msgid":"","date":"2025-12-01T14:42:18","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2025-12-01 18:14, Barnabás Pőcze wrote:\n> The \"Pending\" state at the moment is used for two very distinct scenarios:\n> when the application alone is managing the request, and when the request\n> is being processed. This duality makes it harder to track the true state of\n> a request and consequently it is harder to diagnose misuses.\n> \n> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n> make it explicit when a camera is managing the request. This improves\n> the detection of request double queueing, but since the status flag is\n> not updated atomically, it cannot detect all cases.\n> \n> The name \"Pending\" is not kept because any application depending on it\n> has to be adjusted, so it is better to break the API as well. This is also\n> an ABI break.\n> \n> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n> Signed-off-by: Barnabás Pőcze \n> ---\n> .../internal/tracepoints/request_enums.tp | 7 ++--\n> include/libcamera/request.h | 3 +-\n> src/libcamera/camera.cpp | 2 +-\n> src/libcamera/pipeline_handler.cpp | 7 +++-\n> src/libcamera/request.cpp | 34 ++++++++++++-------\n> src/py/libcamera/py_main.cpp | 3 +-\n> 6 files changed, 36 insertions(+), 20 deletions(-)\n> \n> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n> index bcbd1aaa2c..e9a5a78fac 100644\n> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n> \tlibcamera,\n> \trequest_status,\n> \tTP_ENUM_VALUES(\n> -\t\tctf_enum_value(\"RequestPending\", 0)\n> -\t\tctf_enum_value(\"RequestComplete\", 1)\n> -\t\tctf_enum_value(\"RequestCancelled\", 2)\n> +\t\tctf_enum_value(\"RequestIdle\", 0)\n> +\t\tctf_enum_value(\"RequestInProgress\", 1)\n> +\t\tctf_enum_value(\"RequestComplete\", 2)\n> +\t\tctf_enum_value(\"RequestCancelled\", 3)\n> \t)\n> )\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 0c5939f7b3..1e7a5ca807 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -32,7 +32,8 @@ class Request : public Extensible\n> \n> public:\n> \tenum Status {\n> -\t\tRequestPending,\n> +\t\tRequestIdle,\n> +\t\tRequestInProgress,\n> \t\tRequestComplete,\n> \t\tRequestCancelled,\n> \t};\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 2e1e146a25..b6273f6c29 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n> \t\treturn -EXDEV;\n> \t}\n> \n> -\tif (request->status() != Request::RequestPending) {\n> +\tif (request->status() != Request::RequestIdle) {\n> \t\tLOG(Camera, Error) << request->toString() << \" is not valid\";\n> \t\treturn -EINVAL;\n> \t}\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 5c469e5bad..417b90245a 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n> {\n> \tLIBCAMERA_TRACEPOINT(request_queue, request);\n> \n> +\tASSERT(request->status() == Request::RequestIdle);\n> +\n> \tCamera *camera = request->_d()->camera();\n> \tCamera::Private *data = camera->_d();\n> \tdata->waitingRequests_.push(request);\n> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> \t\t\tbreak;\n> \n> \t\tRequest *request = data->waitingRequests_.front();\n> +\t\tASSERT(request->status() == Request::RequestInProgress);\n> \t\tif (!request->_d()->prepared_)\n> \t\t\tbreak;\n> \n> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n> \n> \twhile (!data->queuedRequests_.empty()) {\n> \t\tRequest *req = data->queuedRequests_.front();\n> -\t\tif (req->status() == Request::RequestPending)\n> +\t\tif (req->status() == Request::RequestInProgress)\n> \t\t\tbreak;\n> \n> +\t\tASSERT(req->status() == Request::RequestCancelled ||\n> +\t\t req->status() == Request::RequestComplete);\n> \t\tASSERT(!req->hasPendingBuffers());\n> \t\tdata->queuedRequests_.pop_front();\n> \t\tcamera->requestComplete(req);\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 60565f5984..89ad0d7c8d 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -121,7 +121,7 @@ void Request::Private::complete()\n> {\n> \tRequest *request = _o();\n> \n> -\tASSERT(request->status() == RequestPending);\n> +\tASSERT(request->status() == RequestInProgress);\n> \tASSERT(!hasPendingBuffers());\n> \n> \trequest->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n> \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n> \n> \tRequest *request = _o();\n> -\tASSERT(request->status() == RequestPending);\n> +\tASSERT(request->status() == RequestInProgress);\n> \n> \tdoCancelRequest();\n> }\n> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n> */\n> void Request::Private::prepare(std::chrono::milliseconds timeout)\n> {\n> +\t_o()->status_ = RequestInProgress;\n> +\n\nMy understanding is that RequestInProgress should be the status as soon\nas the queueRequest() return successfully. Otherwise, it is again hard\nto differentiate between:\n1. RequestIdle (application managing the request)\n2. RequestIdle (request queued by application, and is in internal\nlibcamera waiting queue), what happens if application requeues the same\nrequest here?\n\nDo we need additional status to differentiate between the two?\n\nRequestIdle\nRequestQueued\nRequestInProgress ?\n\n\n> \t/* Create and connect one notifier for each synchronization fence. */\n> \tfor (FrameBuffer *buffer : pending_) {\n> \t\tconst Fence *fence = buffer->_d()->fence();\n> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n> /**\n> * \\enum Request::Status\n> * Request completion status\n> - * \\var Request::RequestPending\n> - * The request hasn't completed yet\n> + * \\var Request::RequestIdle\n> + * The request is idle\n> + * \\var Request::RequestInProgress\n> + * The request is being processed\n> * \\var Request::RequestComplete\n> * The request has completed\n> * \\var Request::RequestCancelled\n> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n> */\n> Request::Request(Camera *camera, uint64_t cookie)\n> \t: Extensible(std::make_unique(camera)),\n> -\t cookie_(cookie), status_(RequestPending)\n> +\t cookie_(cookie), status_(RequestIdle)\n> {\n> \tcontrols_ = new ControlList(controls::controls,\n> \t\t\t\t camera->_d()->validator());\n> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n> {\n> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> \n> +\tASSERT(status_ == RequestIdle ||\n> +\t status_ == RequestComplete ||\n> +\t status_ == RequestCancelled);\n> +\n> \t_d()->reset();\n> \n> \tif (flags & ReuseBuffers) {\n> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n> \t\tbufferMap_.clear();\n> \t}\n> \n> -\tstatus_ = RequestPending;\n> +\tstatus_ = RequestIdle;\n> \n> \tcontrols_->clear();\n> \tmetadata_->clear();\n> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n> * \\fn Request::status()\n> * \\brief Retrieve the request completion status\n> *\n> - * The request status indicates whether the request has completed successfully\n> - * or with an error. When requests are created and before they complete the\n> - * request status is set to RequestPending, and is updated at completion time\n> - * to RequestComplete. If a request is cancelled at capture stop before it has\n> - * completed, its status is set to RequestCancelled.\n> + * The request status indicates whether the request has completed successfully or with\n> + * an error. When requests are created the request status is set to RequestIdle; when\n> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n> + * it has completed, its status is set to RequestCancelled.\n> *\n> * \\return The request completion status\n> */\n> @@ -607,8 +615,8 @@ std::string Request::toString() const\n> */\n> std::ostream &operator<<(std::ostream &out, const Request &r)\n> {\n> -\t/* Pending, Completed, Cancelled(X). */\n> -\tstatic const char *statuses = \"PCX\";\n> +\t/* Idle, InProgress(P), Completed, Cancelled(X). */\n> +\tstatic const char statuses[] = \"IPCX\";\n> \n> \t/* Example Output: Request(55:P:1/2:6523524) */\n> \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index a983ea75c3..d12f00a6f5 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t.def(\"__str__\", &Request::toString);\n> \n> \tpyRequestStatus\n> -\t\t.value(\"Pending\", Request::RequestPending)\n> +\t\t.value(\"Idle\", Request::RequestIdle)\n> +\t\t.value(\"InProgress\", Request::RequestInProgress)\n> \t\t.value(\"Complete\", Request::RequestComplete)\n> \t\t.value(\"Cancelled\", Request::RequestCancelled);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D0D86C0F1B\n\tfor ;\n\tMon, 1 Dec 2025 14:42:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C8D8160BE0;\n\tMon, 1 Dec 2025 15:42:23 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3043F60B36\n\tfor ;\n\tMon, 1 Dec 2025 15:42:22 +0100 (CET)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vQ56b-007Q1O-Fg; Mon, 01 Dec 2025 15:42:21 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vQ56Y-002MIp-Se; Mon, 01 Dec 2025 15:42:21 +0100","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from )\n\tid 1vQ56Y-003g7b-1K; Mon, 01 Dec 2025 15:42:18 +0100"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"Y8Tbq2AO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=o3kghrDDq8RojMpnkFVhvRzjgcykGk8FidTRzl6s2iA=;\n\tb=Y8Tbq2AOWsWhXYZpBD8stQBbBP\n\tq2bvv8V5MJuu2y7IaGVZUq+5xzbYsffskZHkBo8oKccKDbr5gKvT7lQ1bsORpw5C/4MO3p3fcIb/c\n\tRSCH8yJCl3HRoX1JisNX33eDfioTS4kdmYWCJuhBZvkqBAI+6szUh6ZvZQ/Wc2jxKjuAs06G8NkhS\n\tzuEVX9KU0D6PC5p+TAO6Js6y93Yj0qo/NAIfyfci8LmxD5n9Vafx9l68Re1shgMw6btODIgsmE97N\n\tpmfOnlBC4o0XH8VfS0SZszxZh8Sc+daEheX6xa88HC92ZJSD88s1rx6ftJg1Bex/GWgszeBL+j/C4\n\tmRejd6PA==;","MIME-Version":"1.0","Date":"Mon, 01 Dec 2025 20:12:18 +0530","From":"Umang Jain ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","In-Reply-To":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>","Message-ID":"","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-Spam-Report":"NO, Score=-3.3, Tests=ALL_TRUSTED=-3, AWL=-1.108, BAYES_50=0.8,\n\tURIBL_BLOCKED=0.001","X-Spam-Score":"-32","X-Spam-Bar":"---","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37145,"web_url":"https://patchwork.libcamera.org/comment/37145/","msgid":"<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>","date":"2025-12-01T14:56:39","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 01. 15:42 keltezéssel, Umang Jain írta:\n> On 2025-12-01 18:14, Barnabás Pőcze wrote:\n>> The \"Pending\" state at the moment is used for two very distinct scenarios:\n>> when the application alone is managing the request, and when the request\n>> is being processed. This duality makes it harder to track the true state of\n>> a request and consequently it is harder to diagnose misuses.\n>>\n>> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n>> make it explicit when a camera is managing the request. This improves\n>> the detection of request double queueing, but since the status flag is\n>> not updated atomically, it cannot detect all cases.\n>>\n>> The name \"Pending\" is not kept because any application depending on it\n>> has to be adjusted, so it is better to break the API as well. This is also\n>> an ABI break.\n>>\n>> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> .../internal/tracepoints/request_enums.tp | 7 ++--\n>> include/libcamera/request.h | 3 +-\n>> src/libcamera/camera.cpp | 2 +-\n>> src/libcamera/pipeline_handler.cpp | 7 +++-\n>> src/libcamera/request.cpp | 34 ++++++++++++-------\n>> src/py/libcamera/py_main.cpp | 3 +-\n>> 6 files changed, 36 insertions(+), 20 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n>> index bcbd1aaa2c..e9a5a78fac 100644\n>> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n>> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n>> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n>> \tlibcamera,\n>> \trequest_status,\n>> \tTP_ENUM_VALUES(\n>> -\t\tctf_enum_value(\"RequestPending\", 0)\n>> -\t\tctf_enum_value(\"RequestComplete\", 1)\n>> -\t\tctf_enum_value(\"RequestCancelled\", 2)\n>> +\t\tctf_enum_value(\"RequestIdle\", 0)\n>> +\t\tctf_enum_value(\"RequestInProgress\", 1)\n>> +\t\tctf_enum_value(\"RequestComplete\", 2)\n>> +\t\tctf_enum_value(\"RequestCancelled\", 3)\n>> \t)\n>> )\n>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>> index 0c5939f7b3..1e7a5ca807 100644\n>> --- a/include/libcamera/request.h\n>> +++ b/include/libcamera/request.h\n>> @@ -32,7 +32,8 @@ class Request : public Extensible\n>> \n>> public:\n>> \tenum Status {\n>> -\t\tRequestPending,\n>> +\t\tRequestIdle,\n>> +\t\tRequestInProgress,\n>> \t\tRequestComplete,\n>> \t\tRequestCancelled,\n>> \t};\n>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>> index 2e1e146a25..b6273f6c29 100644\n>> --- a/src/libcamera/camera.cpp\n>> +++ b/src/libcamera/camera.cpp\n>> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n>> \t\treturn -EXDEV;\n>> \t}\n>> \n>> -\tif (request->status() != Request::RequestPending) {\n>> +\tif (request->status() != Request::RequestIdle) {\n>> \t\tLOG(Camera, Error) << request->toString() << \" is not valid\";\n>> \t\treturn -EINVAL;\n>> \t}\n>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>> index 5c469e5bad..417b90245a 100644\n>> --- a/src/libcamera/pipeline_handler.cpp\n>> +++ b/src/libcamera/pipeline_handler.cpp\n>> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n>> {\n>> \tLIBCAMERA_TRACEPOINT(request_queue, request);\n>> \n>> +\tASSERT(request->status() == Request::RequestIdle);\n>> +\n>> \tCamera *camera = request->_d()->camera();\n>> \tCamera::Private *data = camera->_d();\n>> \tdata->waitingRequests_.push(request);\n>> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n>> \t\t\tbreak;\n>> \n>> \t\tRequest *request = data->waitingRequests_.front();\n>> +\t\tASSERT(request->status() == Request::RequestInProgress);\n>> \t\tif (!request->_d()->prepared_)\n>> \t\t\tbreak;\n>> \n>> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n>> \n>> \twhile (!data->queuedRequests_.empty()) {\n>> \t\tRequest *req = data->queuedRequests_.front();\n>> -\t\tif (req->status() == Request::RequestPending)\n>> +\t\tif (req->status() == Request::RequestInProgress)\n>> \t\t\tbreak;\n>> \n>> +\t\tASSERT(req->status() == Request::RequestCancelled ||\n>> +\t\t req->status() == Request::RequestComplete);\n>> \t\tASSERT(!req->hasPendingBuffers());\n>> \t\tdata->queuedRequests_.pop_front();\n>> \t\tcamera->requestComplete(req);\n>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n>> index 60565f5984..89ad0d7c8d 100644\n>> --- a/src/libcamera/request.cpp\n>> +++ b/src/libcamera/request.cpp\n>> @@ -121,7 +121,7 @@ void Request::Private::complete()\n>> {\n>> \tRequest *request = _o();\n>> \n>> -\tASSERT(request->status() == RequestPending);\n>> +\tASSERT(request->status() == RequestInProgress);\n>> \tASSERT(!hasPendingBuffers());\n>> \n>> \trequest->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n>> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n>> \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n>> \n>> \tRequest *request = _o();\n>> -\tASSERT(request->status() == RequestPending);\n>> +\tASSERT(request->status() == RequestInProgress);\n>> \n>> \tdoCancelRequest();\n>> }\n>> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n>> */\n>> void Request::Private::prepare(std::chrono::milliseconds timeout)\n>> {\n>> +\t_o()->status_ = RequestInProgress;\n>> +\n> \n> My understanding is that RequestInProgress should be the status as soon\n> as the queueRequest() return successfully. Otherwise, it is again hard\n> to differentiate between:\n> 1. RequestIdle (application managing the request)\n> 2. RequestIdle (request queued by application, and is in internal\n> libcamera waiting queue), what happens if application requeues the same\n> request here?\n> \n> Do we need additional status to differentiate between the two?\n> \n> RequestIdle\n> RequestQueued\n> RequestInProgress ?\n> \n\nPipelineHandler::queueRequest() \"immediately\" calls \"prepare()\" on the request,\nso the state transition will happen \"quickly\", so this does not seem that problematic\nto me at the moment. And we should probably limit the number of \"internal\" states exposed\nin the public interface.\n\nBut something more atomic would be better because there is indeed a small time window\nbetween `Camera::queueRequest()` returning and `Request::Private::prepare()` running.\n(And this say nothing about two parallel `Camera::queueRequest()` calls.)\n`Request::status_` is also not accessed atomically, which is also an issue.\n\nSo there are problems, I mostly just wanted to start some kind of discussion about this.\n\nSome options:\n\n * keep `RequestPending` as is, add an internal `std::atomic_bool queued` to `Request::Private`\n to distinguish the two \"pending\" states;\n * add \"Idle\" and \"InProgress\" states as proposed here, but additionally make `Request::status_` atomic.\n\nIn both cases `Camera::queueRequest()` would need to be changed to do an atomic CAS\nto check and transition into the new state.\n\n\nRegards,\nBarnabás Pőcze\n\n\n\n> \n>> \t/* Create and connect one notifier for each synchronization fence. */\n>> \tfor (FrameBuffer *buffer : pending_) {\n>> \t\tconst Fence *fence = buffer->_d()->fence();\n>> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n>> /**\n>> * \\enum Request::Status\n>> * Request completion status\n>> - * \\var Request::RequestPending\n>> - * The request hasn't completed yet\n>> + * \\var Request::RequestIdle\n>> + * The request is idle\n>> + * \\var Request::RequestInProgress\n>> + * The request is being processed\n>> * \\var Request::RequestComplete\n>> * The request has completed\n>> * \\var Request::RequestCancelled\n>> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n>> */\n>> Request::Request(Camera *camera, uint64_t cookie)\n>> \t: Extensible(std::make_unique(camera)),\n>> -\t cookie_(cookie), status_(RequestPending)\n>> +\t cookie_(cookie), status_(RequestIdle)\n>> {\n>> \tcontrols_ = new ControlList(controls::controls,\n>> \t\t\t\t camera->_d()->validator());\n>> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n>> {\n>> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n>> \n>> +\tASSERT(status_ == RequestIdle ||\n>> +\t status_ == RequestComplete ||\n>> +\t status_ == RequestCancelled);\n>> +\n>> \t_d()->reset();\n>> \n>> \tif (flags & ReuseBuffers) {\n>> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n>> \t\tbufferMap_.clear();\n>> \t}\n>> \n>> -\tstatus_ = RequestPending;\n>> +\tstatus_ = RequestIdle;\n>> \n>> \tcontrols_->clear();\n>> \tmetadata_->clear();\n>> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n>> * \\fn Request::status()\n>> * \\brief Retrieve the request completion status\n>> *\n>> - * The request status indicates whether the request has completed successfully\n>> - * or with an error. When requests are created and before they complete the\n>> - * request status is set to RequestPending, and is updated at completion time\n>> - * to RequestComplete. If a request is cancelled at capture stop before it has\n>> - * completed, its status is set to RequestCancelled.\n>> + * The request status indicates whether the request has completed successfully or with\n>> + * an error. When requests are created the request status is set to RequestIdle; when\n>> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n>> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n>> + * it has completed, its status is set to RequestCancelled.\n>> *\n>> * \\return The request completion status\n>> */\n>> @@ -607,8 +615,8 @@ std::string Request::toString() const\n>> */\n>> std::ostream &operator<<(std::ostream &out, const Request &r)\n>> {\n>> -\t/* Pending, Completed, Cancelled(X). */\n>> -\tstatic const char *statuses = \"PCX\";\n>> +\t/* Idle, InProgress(P), Completed, Cancelled(X). */\n>> +\tstatic const char statuses[] = \"IPCX\";\n>> \n>> \t/* Example Output: Request(55:P:1/2:6523524) */\n>> \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n>> index a983ea75c3..d12f00a6f5 100644\n>> --- a/src/py/libcamera/py_main.cpp\n>> +++ b/src/py/libcamera/py_main.cpp\n>> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n>> \t\t.def(\"__str__\", &Request::toString);\n>> \n>> \tpyRequestStatus\n>> -\t\t.value(\"Pending\", Request::RequestPending)\n>> +\t\t.value(\"Idle\", Request::RequestIdle)\n>> +\t\t.value(\"InProgress\", Request::RequestInProgress)\n>> \t\t.value(\"Complete\", Request::RequestComplete)\n>> \t\t.value(\"Cancelled\", Request::RequestCancelled);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7D704BD80A\n\tfor ;\n\tMon, 1 Dec 2025 14:56:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 950BC60BE0;\n\tMon, 1 Dec 2025 15:56:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D0CC60A7B\n\tfor ;\n\tMon, 1 Dec 2025 15:56:43 +0100 (CET)","from [192.168.33.24] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CDCD14F1;\n\tMon, 1 Dec 2025 15:54:29 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lMUObOJ0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764600869;\n\tbh=LsHX4bjxrdrizd/Pnr7a3UO/gWJNfrYiXGF3c8lUpL4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=lMUObOJ0dm7f13SSiEO4K3qEThR85lnwrgEa/JoOTBuJKBIPLSuKdqjn6AkQUnvYM\n\tjr4YGm20PMyND1cEPwvfe8hlMVQ/vJzwAr3eo4Bh6CIIXikz96Iu23QodqpVVlzPFj\n\trkzbjgHLShla0c8+Hx8sULYOb5AqSCA7/4XafmWM=","Message-ID":"<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>","Date":"Mon, 1 Dec 2025 15:56:39 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37148,"web_url":"https://patchwork.libcamera.org/comment/37148/","msgid":"<20251201153409.GA32430@pendragon.ideasonboard.com>","date":"2025-12-01T15:34:09","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Dec 01, 2025 at 03:56:39PM +0100, Barnabás Pőcze wrote:\n> 2025. 12. 01. 15:42 keltezéssel, Umang Jain írta:\n> > On 2025-12-01 18:14, Barnabás Pőcze wrote:\n> >> The \"Pending\" state at the moment is used for two very distinct scenarios:\n> >> when the application alone is managing the request, and when the request\n> >> is being processed. This duality makes it harder to track the true state of\n> >> a request and consequently it is harder to diagnose misuses.\n> >>\n> >> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n> >> make it explicit when a camera is managing the request. This improves\n> >> the detection of request double queueing, but since the status flag is\n> >> not updated atomically, it cannot detect all cases.\n> >>\n> >> The name \"Pending\" is not kept because any application depending on it\n> >> has to be adjusted, so it is better to break the API as well. This is also\n> >> an ABI break.\n> >>\n> >> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n> >> Signed-off-by: Barnabás Pőcze \n> >> ---\n> >> .../internal/tracepoints/request_enums.tp | 7 ++--\n> >> include/libcamera/request.h | 3 +-\n> >> src/libcamera/camera.cpp | 2 +-\n> >> src/libcamera/pipeline_handler.cpp | 7 +++-\n> >> src/libcamera/request.cpp | 34 ++++++++++++-------\n> >> src/py/libcamera/py_main.cpp | 3 +-\n> >> 6 files changed, 36 insertions(+), 20 deletions(-)\n> >>\n> >> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n> >> index bcbd1aaa2c..e9a5a78fac 100644\n> >> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n> >> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n> >> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n> >> \tlibcamera,\n> >> \trequest_status,\n> >> \tTP_ENUM_VALUES(\n> >> -\t\tctf_enum_value(\"RequestPending\", 0)\n> >> -\t\tctf_enum_value(\"RequestComplete\", 1)\n> >> -\t\tctf_enum_value(\"RequestCancelled\", 2)\n> >> +\t\tctf_enum_value(\"RequestIdle\", 0)\n> >> +\t\tctf_enum_value(\"RequestInProgress\", 1)\n> >> +\t\tctf_enum_value(\"RequestComplete\", 2)\n> >> +\t\tctf_enum_value(\"RequestCancelled\", 3)\n> >> \t)\n> >> )\n> >> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> >> index 0c5939f7b3..1e7a5ca807 100644\n> >> --- a/include/libcamera/request.h\n> >> +++ b/include/libcamera/request.h\n> >> @@ -32,7 +32,8 @@ class Request : public Extensible\n> >> \n> >> public:\n> >> \tenum Status {\n> >> -\t\tRequestPending,\n> >> +\t\tRequestIdle,\n> >> +\t\tRequestInProgress,\n> >> \t\tRequestComplete,\n> >> \t\tRequestCancelled,\n> >> \t};\n> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> >> index 2e1e146a25..b6273f6c29 100644\n> >> --- a/src/libcamera/camera.cpp\n> >> +++ b/src/libcamera/camera.cpp\n> >> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n> >> \t\treturn -EXDEV;\n> >> \t}\n> >> \n> >> -\tif (request->status() != Request::RequestPending) {\n> >> +\tif (request->status() != Request::RequestIdle) {\n> >> \t\tLOG(Camera, Error) << request->toString() << \" is not valid\";\n> >> \t\treturn -EINVAL;\n> >> \t}\n> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >> index 5c469e5bad..417b90245a 100644\n> >> --- a/src/libcamera/pipeline_handler.cpp\n> >> +++ b/src/libcamera/pipeline_handler.cpp\n> >> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n> >> {\n> >> \tLIBCAMERA_TRACEPOINT(request_queue, request);\n> >> \n> >> +\tASSERT(request->status() == Request::RequestIdle);\n> >> +\n> >> \tCamera *camera = request->_d()->camera();\n> >> \tCamera::Private *data = camera->_d();\n> >> \tdata->waitingRequests_.push(request);\n> >> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> >> \t\t\tbreak;\n> >> \n> >> \t\tRequest *request = data->waitingRequests_.front();\n> >> +\t\tASSERT(request->status() == Request::RequestInProgress);\n> >> \t\tif (!request->_d()->prepared_)\n> >> \t\t\tbreak;\n> >> \n> >> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n> >> \n> >> \twhile (!data->queuedRequests_.empty()) {\n> >> \t\tRequest *req = data->queuedRequests_.front();\n> >> -\t\tif (req->status() == Request::RequestPending)\n> >> +\t\tif (req->status() == Request::RequestInProgress)\n> >> \t\t\tbreak;\n> >> \n> >> +\t\tASSERT(req->status() == Request::RequestCancelled ||\n> >> +\t\t req->status() == Request::RequestComplete);\n> >> \t\tASSERT(!req->hasPendingBuffers());\n> >> \t\tdata->queuedRequests_.pop_front();\n> >> \t\tcamera->requestComplete(req);\n> >> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> >> index 60565f5984..89ad0d7c8d 100644\n> >> --- a/src/libcamera/request.cpp\n> >> +++ b/src/libcamera/request.cpp\n> >> @@ -121,7 +121,7 @@ void Request::Private::complete()\n> >> {\n> >> \tRequest *request = _o();\n> >> \n> >> -\tASSERT(request->status() == RequestPending);\n> >> +\tASSERT(request->status() == RequestInProgress);\n> >> \tASSERT(!hasPendingBuffers());\n> >> \n> >> \trequest->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n> >> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n> >> \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n> >> \n> >> \tRequest *request = _o();\n> >> -\tASSERT(request->status() == RequestPending);\n> >> +\tASSERT(request->status() == RequestInProgress);\n> >> \n> >> \tdoCancelRequest();\n> >> }\n> >> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n> >> */\n> >> void Request::Private::prepare(std::chrono::milliseconds timeout)\n> >> {\n> >> +\t_o()->status_ = RequestInProgress;\n> >> +\n> > \n> > My understanding is that RequestInProgress should be the status as soon\n> > as the queueRequest() return successfully. Otherwise, it is again hard\n> > to differentiate between:\n> > 1. RequestIdle (application managing the request)\n> > 2. RequestIdle (request queued by application, and is in internal\n> > libcamera waiting queue), what happens if application requeues the same\n> > request here?\n> > \n> > Do we need additional status to differentiate between the two?\n> > \n> > RequestIdle\n> > RequestQueued\n> > RequestInProgress ?\n> \n> PipelineHandler::queueRequest() \"immediately\" calls \"prepare()\" on the request,\n> so the state transition will happen \"quickly\", so this does not seem that problematic\n> to me at the moment. And we should probably limit the number of \"internal\" states exposed\n> in the public interface.\n> \n> But something more atomic would be better because there is indeed a small time window\n> between `Camera::queueRequest()` returning and `Request::Private::prepare()` running.\n> (And this say nothing about two parallel `Camera::queueRequest()` calls.)\n> `Request::status_` is also not accessed atomically, which is also an issue.\n> \n> So there are problems, I mostly just wanted to start some kind of discussion about this.\n> \n> Some options:\n> \n> * keep `RequestPending` as is, add an internal `std::atomic_bool queued` to `Request::Private`\n> to distinguish the two \"pending\" states;\n> * add \"Idle\" and \"InProgress\" states as proposed here, but additionally make `Request::status_` atomic.\n\nFeel free to explore splitting the state to only expose to applications\nthe information they need, and keeping the rest internal. The current\nstates already feel a bit dirty, with RequestComplete and\nRequestCancelled both indicating completed requests, so there could be\nroom for improvement (I'm not entirely certain alternatives would be\nclearly better though).\n\n> In both cases `Camera::queueRequest()` would need to be changed to do an atomic CAS\n> to check and transition into the new state.\n\nI don't think you need to spend too much effort trying to prevent two\nconcurrent queueRequest() calls using the same request. If it's really\neasy to do I won't object, but it's a clear application bug that I would\npersonally consider as deserving as much empathy as\nqueueRequest(reinterpret_cast(0xdeadbeef));\n\n> >> \t/* Create and connect one notifier for each synchronization fence. */\n> >> \tfor (FrameBuffer *buffer : pending_) {\n> >> \t\tconst Fence *fence = buffer->_d()->fence();\n> >> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n> >> /**\n> >> * \\enum Request::Status\n> >> * Request completion status\n> >> - * \\var Request::RequestPending\n> >> - * The request hasn't completed yet\n> >> + * \\var Request::RequestIdle\n> >> + * The request is idle\n> >> + * \\var Request::RequestInProgress\n> >> + * The request is being processed\n> >> * \\var Request::RequestComplete\n> >> * The request has completed\n> >> * \\var Request::RequestCancelled\n> >> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n> >> */\n> >> Request::Request(Camera *camera, uint64_t cookie)\n> >> \t: Extensible(std::make_unique(camera)),\n> >> -\t cookie_(cookie), status_(RequestPending)\n> >> +\t cookie_(cookie), status_(RequestIdle)\n> >> {\n> >> \tcontrols_ = new ControlList(controls::controls,\n> >> \t\t\t\t camera->_d()->validator());\n> >> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n> >> {\n> >> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> >> \n> >> +\tASSERT(status_ == RequestIdle ||\n> >> +\t status_ == RequestComplete ||\n> >> +\t status_ == RequestCancelled);\n> >> +\n> >> \t_d()->reset();\n> >> \n> >> \tif (flags & ReuseBuffers) {\n> >> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n> >> \t\tbufferMap_.clear();\n> >> \t}\n> >> \n> >> -\tstatus_ = RequestPending;\n> >> +\tstatus_ = RequestIdle;\n> >> \n> >> \tcontrols_->clear();\n> >> \tmetadata_->clear();\n> >> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n> >> * \\fn Request::status()\n> >> * \\brief Retrieve the request completion status\n> >> *\n> >> - * The request status indicates whether the request has completed successfully\n> >> - * or with an error. When requests are created and before they complete the\n> >> - * request status is set to RequestPending, and is updated at completion time\n> >> - * to RequestComplete. If a request is cancelled at capture stop before it has\n> >> - * completed, its status is set to RequestCancelled.\n> >> + * The request status indicates whether the request has completed successfully or with\n> >> + * an error. When requests are created the request status is set to RequestIdle; when\n> >> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n> >> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n> >> + * it has completed, its status is set to RequestCancelled.\n> >> *\n> >> * \\return The request completion status\n> >> */\n> >> @@ -607,8 +615,8 @@ std::string Request::toString() const\n> >> */\n> >> std::ostream &operator<<(std::ostream &out, const Request &r)\n> >> {\n> >> -\t/* Pending, Completed, Cancelled(X). */\n> >> -\tstatic const char *statuses = \"PCX\";\n> >> +\t/* Idle, InProgress(P), Completed, Cancelled(X). */\n> >> +\tstatic const char statuses[] = \"IPCX\";\n> >> \n> >> \t/* Example Output: Request(55:P:1/2:6523524) */\n> >> \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> >> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> >> index a983ea75c3..d12f00a6f5 100644\n> >> --- a/src/py/libcamera/py_main.cpp\n> >> +++ b/src/py/libcamera/py_main.cpp\n> >> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n> >> \t\t.def(\"__str__\", &Request::toString);\n> >> \n> >> \tpyRequestStatus\n> >> -\t\t.value(\"Pending\", Request::RequestPending)\n> >> +\t\t.value(\"Idle\", Request::RequestIdle)\n> >> +\t\t.value(\"InProgress\", Request::RequestInProgress)\n> >> \t\t.value(\"Complete\", Request::RequestComplete)\n> >> \t\t.value(\"Cancelled\", Request::RequestCancelled);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 08E73C0F1B\n\tfor ;\n\tMon, 1 Dec 2025 15:34:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0559060C6D;\n\tMon, 1 Dec 2025 16:34:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 05155606D5\n\tfor ;\n\tMon, 1 Dec 2025 16:34:29 +0100 (CET)","from pendragon.ideasonboard.com\n\t(p9411226-ipngn12302marunouchi.tokyo.ocn.ne.jp [153.160.235.226])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 9ABBE110;\n\tMon, 1 Dec 2025 16:32:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TcrWhmHw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764603136;\n\tbh=9UdYI37BphueXgb+ow94eoRpT/85sGYLNnMYvQ5dm0s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TcrWhmHwhvtbmNUos+LLWyRLVhkYoKi0Y4tVo35A4g7qce6qw1+WGXtYm/0O+9rPb\n\tNFX9qNoMeT7AstQhoIeYkSw7vkz8IAhHKe19sKL9vq88b/PlfFMVvcbvL5b/g7iGAB\n\tY4p+9hNzJfzvSbt98I+TAFpbGSZDPN5und93nIAY=","Date":"Tue, 2 Dec 2025 00:34:09 +0900","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Umang Jain , libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","Message-ID":"<20251201153409.GA32430@pendragon.ideasonboard.com>","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>\n\t\n\t<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37158,"web_url":"https://patchwork.libcamera.org/comment/37158/","msgid":"","date":"2025-12-02T12:26:51","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 01. 16:34 keltezéssel, Laurent Pinchart írta:\n> On Mon, Dec 01, 2025 at 03:56:39PM +0100, Barnabás Pőcze wrote:\n>> 2025. 12. 01. 15:42 keltezéssel, Umang Jain írta:\n>>> On 2025-12-01 18:14, Barnabás Pőcze wrote:\n>>>> The \"Pending\" state at the moment is used for two very distinct scenarios:\n>>>> when the application alone is managing the request, and when the request\n>>>> is being processed. This duality makes it harder to track the true state of\n>>>> a request and consequently it is harder to diagnose misuses.\n>>>>\n>>>> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n>>>> make it explicit when a camera is managing the request. This improves\n>>>> the detection of request double queueing, but since the status flag is\n>>>> not updated atomically, it cannot detect all cases.\n>>>>\n>>>> The name \"Pending\" is not kept because any application depending on it\n>>>> has to be adjusted, so it is better to break the API as well. This is also\n>>>> an ABI break.\n>>>>\n>>>> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n>>>> Signed-off-by: Barnabás Pőcze \n>>>> ---\n>>>> .../internal/tracepoints/request_enums.tp | 7 ++--\n>>>> include/libcamera/request.h | 3 +-\n>>>> src/libcamera/camera.cpp | 2 +-\n>>>> src/libcamera/pipeline_handler.cpp | 7 +++-\n>>>> src/libcamera/request.cpp | 34 ++++++++++++-------\n>>>> src/py/libcamera/py_main.cpp | 3 +-\n>>>> 6 files changed, 36 insertions(+), 20 deletions(-)\n>>>>\n>>>> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n>>>> index bcbd1aaa2c..e9a5a78fac 100644\n>>>> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n>>>> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n>>>> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n>>>> \tlibcamera,\n>>>> \trequest_status,\n>>>> \tTP_ENUM_VALUES(\n>>>> -\t\tctf_enum_value(\"RequestPending\", 0)\n>>>> -\t\tctf_enum_value(\"RequestComplete\", 1)\n>>>> -\t\tctf_enum_value(\"RequestCancelled\", 2)\n>>>> +\t\tctf_enum_value(\"RequestIdle\", 0)\n>>>> +\t\tctf_enum_value(\"RequestInProgress\", 1)\n>>>> +\t\tctf_enum_value(\"RequestComplete\", 2)\n>>>> +\t\tctf_enum_value(\"RequestCancelled\", 3)\n>>>> \t)\n>>>> )\n>>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>>>> index 0c5939f7b3..1e7a5ca807 100644\n>>>> --- a/include/libcamera/request.h\n>>>> +++ b/include/libcamera/request.h\n>>>> @@ -32,7 +32,8 @@ class Request : public Extensible\n>>>> \n>>>> public:\n>>>> \tenum Status {\n>>>> -\t\tRequestPending,\n>>>> +\t\tRequestIdle,\n>>>> +\t\tRequestInProgress,\n>>>> \t\tRequestComplete,\n>>>> \t\tRequestCancelled,\n>>>> \t};\n>>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>>> index 2e1e146a25..b6273f6c29 100644\n>>>> --- a/src/libcamera/camera.cpp\n>>>> +++ b/src/libcamera/camera.cpp\n>>>> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n>>>> \t\treturn -EXDEV;\n>>>> \t}\n>>>> \n>>>> -\tif (request->status() != Request::RequestPending) {\n>>>> +\tif (request->status() != Request::RequestIdle) {\n>>>> \t\tLOG(Camera, Error) << request->toString() << \" is not valid\";\n>>>> \t\treturn -EINVAL;\n>>>> \t}\n>>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>>> index 5c469e5bad..417b90245a 100644\n>>>> --- a/src/libcamera/pipeline_handler.cpp\n>>>> +++ b/src/libcamera/pipeline_handler.cpp\n>>>> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n>>>> {\n>>>> \tLIBCAMERA_TRACEPOINT(request_queue, request);\n>>>> \n>>>> +\tASSERT(request->status() == Request::RequestIdle);\n>>>> +\n>>>> \tCamera *camera = request->_d()->camera();\n>>>> \tCamera::Private *data = camera->_d();\n>>>> \tdata->waitingRequests_.push(request);\n>>>> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n>>>> \t\t\tbreak;\n>>>> \n>>>> \t\tRequest *request = data->waitingRequests_.front();\n>>>> +\t\tASSERT(request->status() == Request::RequestInProgress);\n>>>> \t\tif (!request->_d()->prepared_)\n>>>> \t\t\tbreak;\n>>>> \n>>>> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n>>>> \n>>>> \twhile (!data->queuedRequests_.empty()) {\n>>>> \t\tRequest *req = data->queuedRequests_.front();\n>>>> -\t\tif (req->status() == Request::RequestPending)\n>>>> +\t\tif (req->status() == Request::RequestInProgress)\n>>>> \t\t\tbreak;\n>>>> \n>>>> +\t\tASSERT(req->status() == Request::RequestCancelled ||\n>>>> +\t\t req->status() == Request::RequestComplete);\n>>>> \t\tASSERT(!req->hasPendingBuffers());\n>>>> \t\tdata->queuedRequests_.pop_front();\n>>>> \t\tcamera->requestComplete(req);\n>>>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n>>>> index 60565f5984..89ad0d7c8d 100644\n>>>> --- a/src/libcamera/request.cpp\n>>>> +++ b/src/libcamera/request.cpp\n>>>> @@ -121,7 +121,7 @@ void Request::Private::complete()\n>>>> {\n>>>> \tRequest *request = _o();\n>>>> \n>>>> -\tASSERT(request->status() == RequestPending);\n>>>> +\tASSERT(request->status() == RequestInProgress);\n>>>> \tASSERT(!hasPendingBuffers());\n>>>> \n>>>> \trequest->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n>>>> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n>>>> \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n>>>> \n>>>> \tRequest *request = _o();\n>>>> -\tASSERT(request->status() == RequestPending);\n>>>> +\tASSERT(request->status() == RequestInProgress);\n>>>> \n>>>> \tdoCancelRequest();\n>>>> }\n>>>> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n>>>> */\n>>>> void Request::Private::prepare(std::chrono::milliseconds timeout)\n>>>> {\n>>>> +\t_o()->status_ = RequestInProgress;\n>>>> +\n>>>\n>>> My understanding is that RequestInProgress should be the status as soon\n>>> as the queueRequest() return successfully. Otherwise, it is again hard\n>>> to differentiate between:\n>>> 1. RequestIdle (application managing the request)\n>>> 2. RequestIdle (request queued by application, and is in internal\n>>> libcamera waiting queue), what happens if application requeues the same\n>>> request here?\n>>>\n>>> Do we need additional status to differentiate between the two?\n>>>\n>>> RequestIdle\n>>> RequestQueued\n>>> RequestInProgress ?\n>>\n>> PipelineHandler::queueRequest() \"immediately\" calls \"prepare()\" on the request,\n>> so the state transition will happen \"quickly\", so this does not seem that problematic\n>> to me at the moment. And we should probably limit the number of \"internal\" states exposed\n>> in the public interface.\n>>\n>> But something more atomic would be better because there is indeed a small time window\n>> between `Camera::queueRequest()` returning and `Request::Private::prepare()` running.\n>> (And this say nothing about two parallel `Camera::queueRequest()` calls.)\n>> `Request::status_` is also not accessed atomically, which is also an issue.\n>>\n>> So there are problems, I mostly just wanted to start some kind of discussion about this.\n>>\n>> Some options:\n>>\n>> * keep `RequestPending` as is, add an internal `std::atomic_bool queued` to `Request::Private`\n>> to distinguish the two \"pending\" states;\n>> * add \"Idle\" and \"InProgress\" states as proposed here, but additionally make `Request::status_` atomic.\n> \n> Feel free to explore splitting the state to only expose to applications\n> the information they need, and keeping the rest internal. The current\n> states already feel a bit dirty, with RequestComplete and\n> RequestCancelled both indicating completed requests, so there could be\n> room for improvement (I'm not entirely certain alternatives would be\n> clearly better though).\n> \n>> In both cases `Camera::queueRequest()` would need to be changed to do an atomic CAS\n>> to check and transition into the new state.\n> \n> I don't think you need to spend too much effort trying to prevent two\n> concurrent queueRequest() calls using the same request. If it's really\n> easy to do I won't object, but it's a clear application bug that I would\n> personally consider as deserving as much empathy as\n> queueRequest(reinterpret_cast(0xdeadbeef));\n\nHmm, maybe it's because I myself have run into this issue, but I\nfeel quite differently about this. I think this should be robustly\ndiagnosed.\n\n\n> \n>>>> \t/* Create and connect one notifier for each synchronization fence. */\n>>>> \tfor (FrameBuffer *buffer : pending_) {\n>>>> \t\tconst Fence *fence = buffer->_d()->fence();\n>>>> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n>>>> /**\n>>>> * \\enum Request::Status\n>>>> * Request completion status\n>>>> - * \\var Request::RequestPending\n>>>> - * The request hasn't completed yet\n>>>> + * \\var Request::RequestIdle\n>>>> + * The request is idle\n>>>> + * \\var Request::RequestInProgress\n>>>> + * The request is being processed\n>>>> * \\var Request::RequestComplete\n>>>> * The request has completed\n>>>> * \\var Request::RequestCancelled\n>>>> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n>>>> */\n>>>> Request::Request(Camera *camera, uint64_t cookie)\n>>>> \t: Extensible(std::make_unique(camera)),\n>>>> -\t cookie_(cookie), status_(RequestPending)\n>>>> +\t cookie_(cookie), status_(RequestIdle)\n>>>> {\n>>>> \tcontrols_ = new ControlList(controls::controls,\n>>>> \t\t\t\t camera->_d()->validator());\n>>>> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n>>>> {\n>>>> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n>>>> \n>>>> +\tASSERT(status_ == RequestIdle ||\n>>>> +\t status_ == RequestComplete ||\n>>>> +\t status_ == RequestCancelled);\n>>>> +\n>>>> \t_d()->reset();\n>>>> \n>>>> \tif (flags & ReuseBuffers) {\n>>>> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n>>>> \t\tbufferMap_.clear();\n>>>> \t}\n>>>> \n>>>> -\tstatus_ = RequestPending;\n>>>> +\tstatus_ = RequestIdle;\n>>>> \n>>>> \tcontrols_->clear();\n>>>> \tmetadata_->clear();\n>>>> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n>>>> * \\fn Request::status()\n>>>> * \\brief Retrieve the request completion status\n>>>> *\n>>>> - * The request status indicates whether the request has completed successfully\n>>>> - * or with an error. When requests are created and before they complete the\n>>>> - * request status is set to RequestPending, and is updated at completion time\n>>>> - * to RequestComplete. If a request is cancelled at capture stop before it has\n>>>> - * completed, its status is set to RequestCancelled.\n>>>> + * The request status indicates whether the request has completed successfully or with\n>>>> + * an error. When requests are created the request status is set to RequestIdle; when\n>>>> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n>>>> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n>>>> + * it has completed, its status is set to RequestCancelled.\n>>>> *\n>>>> * \\return The request completion status\n>>>> */\n>>>> @@ -607,8 +615,8 @@ std::string Request::toString() const\n>>>> */\n>>>> std::ostream &operator<<(std::ostream &out, const Request &r)\n>>>> {\n>>>> -\t/* Pending, Completed, Cancelled(X). */\n>>>> -\tstatic const char *statuses = \"PCX\";\n>>>> +\t/* Idle, InProgress(P), Completed, Cancelled(X). */\n>>>> +\tstatic const char statuses[] = \"IPCX\";\n>>>> \n>>>> \t/* Example Output: Request(55:P:1/2:6523524) */\n>>>> \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n>>>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n>>>> index a983ea75c3..d12f00a6f5 100644\n>>>> --- a/src/py/libcamera/py_main.cpp\n>>>> +++ b/src/py/libcamera/py_main.cpp\n>>>> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n>>>> \t\t.def(\"__str__\", &Request::toString);\n>>>> \n>>>> \tpyRequestStatus\n>>>> -\t\t.value(\"Pending\", Request::RequestPending)\n>>>> +\t\t.value(\"Idle\", Request::RequestIdle)\n>>>> +\t\t.value(\"InProgress\", Request::RequestInProgress)\n>>>> \t\t.value(\"Complete\", Request::RequestComplete)\n>>>> \t\t.value(\"Cancelled\", Request::RequestCancelled);\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5AFF7C3257\n\tfor ;\n\tTue, 2 Dec 2025 12:26:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3245A60C77;\n\tTue, 2 Dec 2025 13:26:57 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89E8D6096B\n\tfor ;\n\tTue, 2 Dec 2025 13:26:55 +0100 (CET)","from [192.168.33.23] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0ED889FC;\n\tTue, 2 Dec 2025 13:24:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tLIaT34G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764678281;\n\tbh=6/UE8DuhghLC9zFEkr29dMuWyrts4QMzwCVNgHd/AhE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=tLIaT34G7gg+a3bHx8ytmAWvMkLu9t3d6wcuai5MFcJwSKPCC4R0agSby3/q2COLv\n\tQdlv/WQH+wz/P74KtW9Xb7YS8wW1JRnD1Z7QA/EJQKWc3j+X0Ag0qaUDObMzjnC2Oi\n\tYLOPuwTh8Pb0CpP0MU1iSBkGCNQU/X+kdXmwFfZI=","Message-ID":"","Date":"Tue, 2 Dec 2025 13:26:51 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","To":"Laurent Pinchart ","Cc":"Umang Jain , libcamera-devel@lists.libcamera.org","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>\n\t\n\t<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>\n\t<20251201153409.GA32430@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251201153409.GA32430@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37159,"web_url":"https://patchwork.libcamera.org/comment/37159/","msgid":"<176468010554.2003224.5961048517407596630@ping.linuxembedded.co.uk>","date":"2025-12-02T12:55:05","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-12-02 12:26:51)\n> 2025. 12. 01. 16:34 keltezéssel, Laurent Pinchart írta:\n> > On Mon, Dec 01, 2025 at 03:56:39PM +0100, Barnabás Pőcze wrote:\n> >> 2025. 12. 01. 15:42 keltezéssel, Umang Jain írta:\n> >>> On 2025-12-01 18:14, Barnabás Pőcze wrote:\n> >>>> The \"Pending\" state at the moment is used for two very distinct scenarios:\n> >>>> when the application alone is managing the request, and when the request\n> >>>> is being processed. This duality makes it harder to track the true state of\n> >>>> a request and consequently it is harder to diagnose misuses.\n> >>>>\n> >>>> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n> >>>> make it explicit when a camera is managing the request. This improves\n> >>>> the detection of request double queueing, but since the status flag is\n> >>>> not updated atomically, it cannot detect all cases.\n> >>>>\n> >>>> The name \"Pending\" is not kept because any application depending on it\n> >>>> has to be adjusted, so it is better to break the API as well. This is also\n> >>>> an ABI break.\n> >>>>\n> >>>> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n> >>>> Signed-off-by: Barnabás Pőcze \n> >>>> ---\n> >>>> .../internal/tracepoints/request_enums.tp | 7 ++--\n> >>>> include/libcamera/request.h | 3 +-\n> >>>> src/libcamera/camera.cpp | 2 +-\n> >>>> src/libcamera/pipeline_handler.cpp | 7 +++-\n> >>>> src/libcamera/request.cpp | 34 ++++++++++++-------\n> >>>> src/py/libcamera/py_main.cpp | 3 +-\n> >>>> 6 files changed, 36 insertions(+), 20 deletions(-)\n> >>>>\n> >>>> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n> >>>> index bcbd1aaa2c..e9a5a78fac 100644\n> >>>> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n> >>>> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n> >>>> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n> >>>> libcamera,\n> >>>> request_status,\n> >>>> TP_ENUM_VALUES(\n> >>>> - ctf_enum_value(\"RequestPending\", 0)\n> >>>> - ctf_enum_value(\"RequestComplete\", 1)\n> >>>> - ctf_enum_value(\"RequestCancelled\", 2)\n> >>>> + ctf_enum_value(\"RequestIdle\", 0)\n> >>>> + ctf_enum_value(\"RequestInProgress\", 1)\n> >>>> + ctf_enum_value(\"RequestComplete\", 2)\n> >>>> + ctf_enum_value(\"RequestCancelled\", 3)\n> >>>> )\n> >>>> )\n> >>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> >>>> index 0c5939f7b3..1e7a5ca807 100644\n> >>>> --- a/include/libcamera/request.h\n> >>>> +++ b/include/libcamera/request.h\n> >>>> @@ -32,7 +32,8 @@ class Request : public Extensible\n> >>>> \n> >>>> public:\n> >>>> enum Status {\n> >>>> - RequestPending,\n> >>>> + RequestIdle,\n> >>>> + RequestInProgress,\n> >>>> RequestComplete,\n> >>>> RequestCancelled,\n> >>>> };\n> >>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> >>>> index 2e1e146a25..b6273f6c29 100644\n> >>>> --- a/src/libcamera/camera.cpp\n> >>>> +++ b/src/libcamera/camera.cpp\n> >>>> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n> >>>> return -EXDEV;\n> >>>> }\n> >>>> \n> >>>> - if (request->status() != Request::RequestPending) {\n> >>>> + if (request->status() != Request::RequestIdle) {\n> >>>> LOG(Camera, Error) << request->toString() << \" is not valid\";\n> >>>> return -EINVAL;\n> >>>> }\n> >>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >>>> index 5c469e5bad..417b90245a 100644\n> >>>> --- a/src/libcamera/pipeline_handler.cpp\n> >>>> +++ b/src/libcamera/pipeline_handler.cpp\n> >>>> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n> >>>> {\n> >>>> LIBCAMERA_TRACEPOINT(request_queue, request);\n> >>>> \n> >>>> + ASSERT(request->status() == Request::RequestIdle);\n> >>>> +\n> >>>> Camera *camera = request->_d()->camera();\n> >>>> Camera::Private *data = camera->_d();\n> >>>> data->waitingRequests_.push(request);\n> >>>> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> >>>> break;\n> >>>> \n> >>>> Request *request = data->waitingRequests_.front();\n> >>>> + ASSERT(request->status() == Request::RequestInProgress);\n> >>>> if (!request->_d()->prepared_)\n> >>>> break;\n> >>>> \n> >>>> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n> >>>> \n> >>>> while (!data->queuedRequests_.empty()) {\n> >>>> Request *req = data->queuedRequests_.front();\n> >>>> - if (req->status() == Request::RequestPending)\n> >>>> + if (req->status() == Request::RequestInProgress)\n> >>>> break;\n> >>>> \n> >>>> + ASSERT(req->status() == Request::RequestCancelled ||\n> >>>> + req->status() == Request::RequestComplete);\n> >>>> ASSERT(!req->hasPendingBuffers());\n> >>>> data->queuedRequests_.pop_front();\n> >>>> camera->requestComplete(req);\n> >>>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> >>>> index 60565f5984..89ad0d7c8d 100644\n> >>>> --- a/src/libcamera/request.cpp\n> >>>> +++ b/src/libcamera/request.cpp\n> >>>> @@ -121,7 +121,7 @@ void Request::Private::complete()\n> >>>> {\n> >>>> Request *request = _o();\n> >>>> \n> >>>> - ASSERT(request->status() == RequestPending);\n> >>>> + ASSERT(request->status() == RequestInProgress);\n> >>>> ASSERT(!hasPendingBuffers());\n> >>>> \n> >>>> request->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n> >>>> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n> >>>> LIBCAMERA_TRACEPOINT(request_cancel, this);\n> >>>> \n> >>>> Request *request = _o();\n> >>>> - ASSERT(request->status() == RequestPending);\n> >>>> + ASSERT(request->status() == RequestInProgress);\n> >>>> \n> >>>> doCancelRequest();\n> >>>> }\n> >>>> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n> >>>> */\n> >>>> void Request::Private::prepare(std::chrono::milliseconds timeout)\n> >>>> {\n> >>>> + _o()->status_ = RequestInProgress;\n> >>>> +\n> >>>\n> >>> My understanding is that RequestInProgress should be the status as soon\n> >>> as the queueRequest() return successfully. Otherwise, it is again hard\n> >>> to differentiate between:\n> >>> 1. RequestIdle (application managing the request)\n> >>> 2. RequestIdle (request queued by application, and is in internal\n> >>> libcamera waiting queue), what happens if application requeues the same\n> >>> request here?\n> >>>\n> >>> Do we need additional status to differentiate between the two?\n> >>>\n> >>> RequestIdle\n> >>> RequestQueued\n> >>> RequestInProgress ?\n> >>\n> >> PipelineHandler::queueRequest() \"immediately\" calls \"prepare()\" on the request,\n> >> so the state transition will happen \"quickly\", so this does not seem that problematic\n> >> to me at the moment. And we should probably limit the number of \"internal\" states exposed\n> >> in the public interface.\n> >>\n> >> But something more atomic would be better because there is indeed a small time window\n> >> between `Camera::queueRequest()` returning and `Request::Private::prepare()` running.\n> >> (And this say nothing about two parallel `Camera::queueRequest()` calls.)\n> >> `Request::status_` is also not accessed atomically, which is also an issue.\n> >>\n> >> So there are problems, I mostly just wanted to start some kind of discussion about this.\n> >>\n> >> Some options:\n> >>\n> >> * keep `RequestPending` as is, add an internal `std::atomic_bool queued` to `Request::Private`\n> >> to distinguish the two \"pending\" states;\n> >> * add \"Idle\" and \"InProgress\" states as proposed here, but additionally make `Request::status_` atomic.\n> > \n> > Feel free to explore splitting the state to only expose to applications\n> > the information they need, and keeping the rest internal. The current\n> > states already feel a bit dirty, with RequestComplete and\n> > RequestCancelled both indicating completed requests, so there could be\n> > room for improvement (I'm not entirely certain alternatives would be\n> > clearly better though).\n> > \n> >> In both cases `Camera::queueRequest()` would need to be changed to do an atomic CAS\n> >> to check and transition into the new state.\n> > \n> > I don't think you need to spend too much effort trying to prevent two\n> > concurrent queueRequest() calls using the same request. If it's really\n> > easy to do I won't object, but it's a clear application bug that I would\n> > personally consider as deserving as much empathy as\n> > queueRequest(reinterpret_cast(0xdeadbeef));\n> \n> Hmm, maybe it's because I myself have run into this issue, but I\n> feel quite differently about this. I think this should be robustly\n> diagnosed.\n\nI'm also going to be vocally on the side of *We should do better to\nsupport applications here*.\n\nThis reminds me of the Request Canary I proposed\nhttps://patchwork.libcamera.org/patch/18226/ - Which I /still/ put on\ncustomer branches and it *has* caught and prevented bugs in those\ndevelopments.\n\n\nYes, applications can do bad things, and they can double free or\nintroduce use after free errors. And in C scope - perhaps those can\neasily be detected with asan or such.\n\nHere /we/ are the framework - and /we/ provide the interfaces. \n\nIf we put the shotgun in the hands of the developer, when they point it\nat their feet, I think we have a responsibilty to tell them what will\nhappen when they pull the trigger.\n\n--\nKieran\n\n\n\n> >>>> /* Create and connect one notifier for each synchronization fence. */\n> >>>> for (FrameBuffer *buffer : pending_) {\n> >>>> const Fence *fence = buffer->_d()->fence();\n> >>>> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n> >>>> /**\n> >>>> * \\enum Request::Status\n> >>>> * Request completion status\n> >>>> - * \\var Request::RequestPending\n> >>>> - * The request hasn't completed yet\n> >>>> + * \\var Request::RequestIdle\n> >>>> + * The request is idle\n> >>>> + * \\var Request::RequestInProgress\n> >>>> + * The request is being processed\n> >>>> * \\var Request::RequestComplete\n> >>>> * The request has completed\n> >>>> * \\var Request::RequestCancelled\n> >>>> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n> >>>> */\n> >>>> Request::Request(Camera *camera, uint64_t cookie)\n> >>>> : Extensible(std::make_unique(camera)),\n> >>>> - cookie_(cookie), status_(RequestPending)\n> >>>> + cookie_(cookie), status_(RequestIdle)\n> >>>> {\n> >>>> controls_ = new ControlList(controls::controls,\n> >>>> camera->_d()->validator());\n> >>>> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n> >>>> {\n> >>>> LIBCAMERA_TRACEPOINT(request_reuse, this);\n> >>>> \n> >>>> + ASSERT(status_ == RequestIdle ||\n> >>>> + status_ == RequestComplete ||\n> >>>> + status_ == RequestCancelled);\n> >>>> +\n> >>>> _d()->reset();\n> >>>> \n> >>>> if (flags & ReuseBuffers) {\n> >>>> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n> >>>> bufferMap_.clear();\n> >>>> }\n> >>>> \n> >>>> - status_ = RequestPending;\n> >>>> + status_ = RequestIdle;\n> >>>> \n> >>>> controls_->clear();\n> >>>> metadata_->clear();\n> >>>> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n> >>>> * \\fn Request::status()\n> >>>> * \\brief Retrieve the request completion status\n> >>>> *\n> >>>> - * The request status indicates whether the request has completed successfully\n> >>>> - * or with an error. When requests are created and before they complete the\n> >>>> - * request status is set to RequestPending, and is updated at completion time\n> >>>> - * to RequestComplete. If a request is cancelled at capture stop before it has\n> >>>> - * completed, its status is set to RequestCancelled.\n> >>>> + * The request status indicates whether the request has completed successfully or with\n> >>>> + * an error. When requests are created the request status is set to RequestIdle; when\n> >>>> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n> >>>> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n> >>>> + * it has completed, its status is set to RequestCancelled.\n> >>>> *\n> >>>> * \\return The request completion status\n> >>>> */\n> >>>> @@ -607,8 +615,8 @@ std::string Request::toString() const\n> >>>> */\n> >>>> std::ostream &operator<<(std::ostream &out, const Request &r)\n> >>>> {\n> >>>> - /* Pending, Completed, Cancelled(X). */\n> >>>> - static const char *statuses = \"PCX\";\n> >>>> + /* Idle, InProgress(P), Completed, Cancelled(X). */\n> >>>> + static const char statuses[] = \"IPCX\";\n> >>>> \n> >>>> /* Example Output: Request(55:P:1/2:6523524) */\n> >>>> out << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> >>>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> >>>> index a983ea75c3..d12f00a6f5 100644\n> >>>> --- a/src/py/libcamera/py_main.cpp\n> >>>> +++ b/src/py/libcamera/py_main.cpp\n> >>>> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n> >>>> .def(\"__str__\", &Request::toString);\n> >>>> \n> >>>> pyRequestStatus\n> >>>> - .value(\"Pending\", Request::RequestPending)\n> >>>> + .value(\"Idle\", Request::RequestIdle)\n> >>>> + .value(\"InProgress\", Request::RequestInProgress)\n> >>>> .value(\"Complete\", Request::RequestComplete)\n> >>>> .value(\"Cancelled\", Request::RequestCancelled);\n> > \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E14FCC3257\n\tfor ;\n\tTue, 2 Dec 2025 12:55:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0522C60C77;\n\tTue, 2 Dec 2025 13:55:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FAC36096B\n\tfor ;\n\tTue, 2 Dec 2025 13:55:09 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 381E5FE;\n\tTue, 2 Dec 2025 13:52:54 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lQlwisx5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764679974;\n\tbh=sifTg8mnvUu1Qujq78YxjELUfrgszqib07K4i7u34FM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=lQlwisx5Yx9CbNzKel7pjb70Yf++6EFDq5s5GWofheu85Wp1W9cWvbck9UXNmGhHV\n\ttRHR3vgjL5sirRkj4prAcbRuvKv7DmhEd0FwWO6ZuVl5i4awDgb3OPDHvSw43+vnpT\n\tAtH28rkAtflJBbS0OfVv8A4NFfYSTEye1GvWY9Zg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>\n\t\n\t<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>\n\t<20251201153409.GA32430@pendragon.ideasonboard.com>\n\t","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","From":"Kieran Bingham ","Cc":"Umang Jain , libcamera-devel@lists.libcamera.org","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tLaurent Pinchart ","Date":"Tue, 02 Dec 2025 12:55:05 +0000","Message-ID":"<176468010554.2003224.5961048517407596630@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37164,"web_url":"https://patchwork.libcamera.org/comment/37164/","msgid":"<20251202145953.GA8219@pendragon.ideasonboard.com>","date":"2025-12-02T14:59:53","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Dec 02, 2025 at 12:55:05PM +0000, Kieran Bingham wrote:\n> Quoting Barnabás Pőcze (2025-12-02 12:26:51)\n> > 2025. 12. 01. 16:34 keltezéssel, Laurent Pinchart írta:\n> > > On Mon, Dec 01, 2025 at 03:56:39PM +0100, Barnabás Pőcze wrote:\n> > >> 2025. 12. 01. 15:42 keltezéssel, Umang Jain írta:\n> > >>> On 2025-12-01 18:14, Barnabás Pőcze wrote:\n> > >>>> The \"Pending\" state at the moment is used for two very distinct scenarios:\n> > >>>> when the application alone is managing the request, and when the request\n> > >>>> is being processed. This duality makes it harder to track the true state of\n> > >>>> a request and consequently it is harder to diagnose misuses.\n> > >>>>\n> > >>>> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n> > >>>> make it explicit when a camera is managing the request. This improves\n> > >>>> the detection of request double queueing, but since the status flag is\n> > >>>> not updated atomically, it cannot detect all cases.\n> > >>>>\n> > >>>> The name \"Pending\" is not kept because any application depending on it\n> > >>>> has to be adjusted, so it is better to break the API as well. This is also\n> > >>>> an ABI break.\n> > >>>>\n> > >>>> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n> > >>>> Signed-off-by: Barnabás Pőcze \n> > >>>> ---\n> > >>>> .../internal/tracepoints/request_enums.tp | 7 ++--\n> > >>>> include/libcamera/request.h | 3 +-\n> > >>>> src/libcamera/camera.cpp | 2 +-\n> > >>>> src/libcamera/pipeline_handler.cpp | 7 +++-\n> > >>>> src/libcamera/request.cpp | 34 ++++++++++++-------\n> > >>>> src/py/libcamera/py_main.cpp | 3 +-\n> > >>>> 6 files changed, 36 insertions(+), 20 deletions(-)\n> > >>>>\n> > >>>> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n> > >>>> index bcbd1aaa2c..e9a5a78fac 100644\n> > >>>> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n> > >>>> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n> > >>>> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n> > >>>> libcamera,\n> > >>>> request_status,\n> > >>>> TP_ENUM_VALUES(\n> > >>>> - ctf_enum_value(\"RequestPending\", 0)\n> > >>>> - ctf_enum_value(\"RequestComplete\", 1)\n> > >>>> - ctf_enum_value(\"RequestCancelled\", 2)\n> > >>>> + ctf_enum_value(\"RequestIdle\", 0)\n> > >>>> + ctf_enum_value(\"RequestInProgress\", 1)\n> > >>>> + ctf_enum_value(\"RequestComplete\", 2)\n> > >>>> + ctf_enum_value(\"RequestCancelled\", 3)\n> > >>>> )\n> > >>>> )\n> > >>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > >>>> index 0c5939f7b3..1e7a5ca807 100644\n> > >>>> --- a/include/libcamera/request.h\n> > >>>> +++ b/include/libcamera/request.h\n> > >>>> @@ -32,7 +32,8 @@ class Request : public Extensible\n> > >>>> \n> > >>>> public:\n> > >>>> enum Status {\n> > >>>> - RequestPending,\n> > >>>> + RequestIdle,\n> > >>>> + RequestInProgress,\n> > >>>> RequestComplete,\n> > >>>> RequestCancelled,\n> > >>>> };\n> > >>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > >>>> index 2e1e146a25..b6273f6c29 100644\n> > >>>> --- a/src/libcamera/camera.cpp\n> > >>>> +++ b/src/libcamera/camera.cpp\n> > >>>> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n> > >>>> return -EXDEV;\n> > >>>> }\n> > >>>> \n> > >>>> - if (request->status() != Request::RequestPending) {\n> > >>>> + if (request->status() != Request::RequestIdle) {\n> > >>>> LOG(Camera, Error) << request->toString() << \" is not valid\";\n> > >>>> return -EINVAL;\n> > >>>> }\n> > >>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > >>>> index 5c469e5bad..417b90245a 100644\n> > >>>> --- a/src/libcamera/pipeline_handler.cpp\n> > >>>> +++ b/src/libcamera/pipeline_handler.cpp\n> > >>>> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n> > >>>> {\n> > >>>> LIBCAMERA_TRACEPOINT(request_queue, request);\n> > >>>> \n> > >>>> + ASSERT(request->status() == Request::RequestIdle);\n> > >>>> +\n> > >>>> Camera *camera = request->_d()->camera();\n> > >>>> Camera::Private *data = camera->_d();\n> > >>>> data->waitingRequests_.push(request);\n> > >>>> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> > >>>> break;\n> > >>>> \n> > >>>> Request *request = data->waitingRequests_.front();\n> > >>>> + ASSERT(request->status() == Request::RequestInProgress);\n> > >>>> if (!request->_d()->prepared_)\n> > >>>> break;\n> > >>>> \n> > >>>> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n> > >>>> \n> > >>>> while (!data->queuedRequests_.empty()) {\n> > >>>> Request *req = data->queuedRequests_.front();\n> > >>>> - if (req->status() == Request::RequestPending)\n> > >>>> + if (req->status() == Request::RequestInProgress)\n> > >>>> break;\n> > >>>> \n> > >>>> + ASSERT(req->status() == Request::RequestCancelled ||\n> > >>>> + req->status() == Request::RequestComplete);\n> > >>>> ASSERT(!req->hasPendingBuffers());\n> > >>>> data->queuedRequests_.pop_front();\n> > >>>> camera->requestComplete(req);\n> > >>>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > >>>> index 60565f5984..89ad0d7c8d 100644\n> > >>>> --- a/src/libcamera/request.cpp\n> > >>>> +++ b/src/libcamera/request.cpp\n> > >>>> @@ -121,7 +121,7 @@ void Request::Private::complete()\n> > >>>> {\n> > >>>> Request *request = _o();\n> > >>>> \n> > >>>> - ASSERT(request->status() == RequestPending);\n> > >>>> + ASSERT(request->status() == RequestInProgress);\n> > >>>> ASSERT(!hasPendingBuffers());\n> > >>>> \n> > >>>> request->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n> > >>>> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n> > >>>> LIBCAMERA_TRACEPOINT(request_cancel, this);\n> > >>>> \n> > >>>> Request *request = _o();\n> > >>>> - ASSERT(request->status() == RequestPending);\n> > >>>> + ASSERT(request->status() == RequestInProgress);\n> > >>>> \n> > >>>> doCancelRequest();\n> > >>>> }\n> > >>>> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n> > >>>> */\n> > >>>> void Request::Private::prepare(std::chrono::milliseconds timeout)\n> > >>>> {\n> > >>>> + _o()->status_ = RequestInProgress;\n> > >>>> +\n> > >>>\n> > >>> My understanding is that RequestInProgress should be the status as soon\n> > >>> as the queueRequest() return successfully. Otherwise, it is again hard\n> > >>> to differentiate between:\n> > >>> 1. RequestIdle (application managing the request)\n> > >>> 2. RequestIdle (request queued by application, and is in internal\n> > >>> libcamera waiting queue), what happens if application requeues the same\n> > >>> request here?\n> > >>>\n> > >>> Do we need additional status to differentiate between the two?\n> > >>>\n> > >>> RequestIdle\n> > >>> RequestQueued\n> > >>> RequestInProgress ?\n> > >>\n> > >> PipelineHandler::queueRequest() \"immediately\" calls \"prepare()\" on the request,\n> > >> so the state transition will happen \"quickly\", so this does not seem that problematic\n> > >> to me at the moment. And we should probably limit the number of \"internal\" states exposed\n> > >> in the public interface.\n> > >>\n> > >> But something more atomic would be better because there is indeed a small time window\n> > >> between `Camera::queueRequest()` returning and `Request::Private::prepare()` running.\n> > >> (And this say nothing about two parallel `Camera::queueRequest()` calls.)\n> > >> `Request::status_` is also not accessed atomically, which is also an issue.\n> > >>\n> > >> So there are problems, I mostly just wanted to start some kind of discussion about this.\n> > >>\n> > >> Some options:\n> > >>\n> > >> * keep `RequestPending` as is, add an internal `std::atomic_bool queued` to `Request::Private`\n> > >> to distinguish the two \"pending\" states;\n> > >> * add \"Idle\" and \"InProgress\" states as proposed here, but additionally make `Request::status_` atomic.\n> > > \n> > > Feel free to explore splitting the state to only expose to applications\n> > > the information they need, and keeping the rest internal. The current\n> > > states already feel a bit dirty, with RequestComplete and\n> > > RequestCancelled both indicating completed requests, so there could be\n> > > room for improvement (I'm not entirely certain alternatives would be\n> > > clearly better though).\n> > > \n> > >> In both cases `Camera::queueRequest()` would need to be changed to do an atomic CAS\n> > >> to check and transition into the new state.\n> > > \n> > > I don't think you need to spend too much effort trying to prevent two\n> > > concurrent queueRequest() calls using the same request. If it's really\n> > > easy to do I won't object, but it's a clear application bug that I would\n> > > personally consider as deserving as much empathy as\n> > > queueRequest(reinterpret_cast(0xdeadbeef));\n> > \n> > Hmm, maybe it's because I myself have run into this issue, but I\n> > feel quite differently about this. I think this should be robustly\n> > diagnosed.\n> \n> I'm also going to be vocally on the side of *We should do better to\n> support applications here*.\n> \n> This reminds me of the Request Canary I proposed\n> https://patchwork.libcamera.org/patch/18226/ - Which I /still/ put on\n> customer branches and it *has* caught and prevented bugs in those\n> developments.\n> \n> \n> Yes, applications can do bad things, and they can double free or\n> introduce use after free errors. And in C scope - perhaps those can\n> easily be detected with asan or such.\n> \n> Here /we/ are the framework - and /we/ provide the interfaces. \n> \n> If we put the shotgun in the hands of the developer, when they point it\n> at their feet, I think we have a responsibilty to tell them what will\n> happen when they pull the trigger.\n\nAs I said in my previous e-mail, adding safeguards to report when an\nalready queued request is queued is a good thing. Safeguards against\n*concurrent* queueRequest() calls from different threads *with the same\nrequest* is what I said seems overkill. Barnabás, which one of the two\ndid you mean ?\n\n> > >>>> /* Create and connect one notifier for each synchronization fence. */\n> > >>>> for (FrameBuffer *buffer : pending_) {\n> > >>>> const Fence *fence = buffer->_d()->fence();\n> > >>>> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n> > >>>> /**\n> > >>>> * \\enum Request::Status\n> > >>>> * Request completion status\n> > >>>> - * \\var Request::RequestPending\n> > >>>> - * The request hasn't completed yet\n> > >>>> + * \\var Request::RequestIdle\n> > >>>> + * The request is idle\n> > >>>> + * \\var Request::RequestInProgress\n> > >>>> + * The request is being processed\n> > >>>> * \\var Request::RequestComplete\n> > >>>> * The request has completed\n> > >>>> * \\var Request::RequestCancelled\n> > >>>> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n> > >>>> */\n> > >>>> Request::Request(Camera *camera, uint64_t cookie)\n> > >>>> : Extensible(std::make_unique(camera)),\n> > >>>> - cookie_(cookie), status_(RequestPending)\n> > >>>> + cookie_(cookie), status_(RequestIdle)\n> > >>>> {\n> > >>>> controls_ = new ControlList(controls::controls,\n> > >>>> camera->_d()->validator());\n> > >>>> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n> > >>>> {\n> > >>>> LIBCAMERA_TRACEPOINT(request_reuse, this);\n> > >>>> \n> > >>>> + ASSERT(status_ == RequestIdle ||\n> > >>>> + status_ == RequestComplete ||\n> > >>>> + status_ == RequestCancelled);\n> > >>>> +\n> > >>>> _d()->reset();\n> > >>>> \n> > >>>> if (flags & ReuseBuffers) {\n> > >>>> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n> > >>>> bufferMap_.clear();\n> > >>>> }\n> > >>>> \n> > >>>> - status_ = RequestPending;\n> > >>>> + status_ = RequestIdle;\n> > >>>> \n> > >>>> controls_->clear();\n> > >>>> metadata_->clear();\n> > >>>> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n> > >>>> * \\fn Request::status()\n> > >>>> * \\brief Retrieve the request completion status\n> > >>>> *\n> > >>>> - * The request status indicates whether the request has completed successfully\n> > >>>> - * or with an error. When requests are created and before they complete the\n> > >>>> - * request status is set to RequestPending, and is updated at completion time\n> > >>>> - * to RequestComplete. If a request is cancelled at capture stop before it has\n> > >>>> - * completed, its status is set to RequestCancelled.\n> > >>>> + * The request status indicates whether the request has completed successfully or with\n> > >>>> + * an error. When requests are created the request status is set to RequestIdle; when\n> > >>>> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n> > >>>> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n> > >>>> + * it has completed, its status is set to RequestCancelled.\n> > >>>> *\n> > >>>> * \\return The request completion status\n> > >>>> */\n> > >>>> @@ -607,8 +615,8 @@ std::string Request::toString() const\n> > >>>> */\n> > >>>> std::ostream &operator<<(std::ostream &out, const Request &r)\n> > >>>> {\n> > >>>> - /* Pending, Completed, Cancelled(X). */\n> > >>>> - static const char *statuses = \"PCX\";\n> > >>>> + /* Idle, InProgress(P), Completed, Cancelled(X). */\n> > >>>> + static const char statuses[] = \"IPCX\";\n> > >>>> \n> > >>>> /* Example Output: Request(55:P:1/2:6523524) */\n> > >>>> out << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> > >>>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> > >>>> index a983ea75c3..d12f00a6f5 100644\n> > >>>> --- a/src/py/libcamera/py_main.cpp\n> > >>>> +++ b/src/py/libcamera/py_main.cpp\n> > >>>> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n> > >>>> .def(\"__str__\", &Request::toString);\n> > >>>> \n> > >>>> pyRequestStatus\n> > >>>> - .value(\"Pending\", Request::RequestPending)\n> > >>>> + .value(\"Idle\", Request::RequestIdle)\n> > >>>> + .value(\"InProgress\", Request::RequestInProgress)\n> > >>>> .value(\"Complete\", Request::RequestComplete)\n> > >>>> .value(\"Cancelled\", Request::RequestCancelled);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D3812C3257\n\tfor ;\n\tTue, 2 Dec 2025 15:00:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8FD860D0E;\n\tTue, 2 Dec 2025 16:00:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E09360C8A\n\tfor ;\n\tTue, 2 Dec 2025 16:00:15 +0100 (CET)","from pendragon.ideasonboard.com\n\t(p9411226-ipngn12302marunouchi.tokyo.ocn.ne.jp [153.160.235.226])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id CCCD1D7;\n\tTue, 2 Dec 2025 15:57:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gZUev+GD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764687480;\n\tbh=JsJU6gf5fVT8m3Ud6VABu9RDG5smtcG+VGtksczmqCc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gZUev+GDqQi5/pBUOmhSxPa90oV1Y000ROr3u1sKe3F8zuq4sUJYWGEbQnrQCpYDV\n\thMCqFDpFK/zOvwUt5pviYTv7sPIoFHkrrrXp+TaC+A8ndZt8RQMnqwS43SmZbzSvD3\n\tEly3CJ3OENHoHDufWNWJHTbz0/MAuISOyEnJytt4=","Date":"Tue, 2 Dec 2025 23:59:53 +0900","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tUmang Jain , libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","Message-ID":"<20251202145953.GA8219@pendragon.ideasonboard.com>","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>\n\t\n\t<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>\n\t<20251201153409.GA32430@pendragon.ideasonboard.com>\n\t\n\t<176468010554.2003224.5961048517407596630@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<176468010554.2003224.5961048517407596630@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37170,"web_url":"https://patchwork.libcamera.org/comment/37170/","msgid":"","date":"2025-12-02T18:08:12","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 02. 15:59 keltezéssel, Laurent Pinchart írta:\n> On Tue, Dec 02, 2025 at 12:55:05PM +0000, Kieran Bingham wrote:\n>> Quoting Barnabás Pőcze (2025-12-02 12:26:51)\n>>> 2025. 12. 01. 16:34 keltezéssel, Laurent Pinchart írta:\n>>>> On Mon, Dec 01, 2025 at 03:56:39PM +0100, Barnabás Pőcze wrote:\n>>>>> 2025. 12. 01. 15:42 keltezéssel, Umang Jain írta:\n>>>>>> On 2025-12-01 18:14, Barnabás Pőcze wrote:\n>>>>>>> The \"Pending\" state at the moment is used for two very distinct scenarios:\n>>>>>>> when the application alone is managing the request, and when the request\n>>>>>>> is being processed. This duality makes it harder to track the true state of\n>>>>>>> a request and consequently it is harder to diagnose misuses.\n>>>>>>>\n>>>>>>> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n>>>>>>> make it explicit when a camera is managing the request. This improves\n>>>>>>> the detection of request double queueing, but since the status flag is\n>>>>>>> not updated atomically, it cannot detect all cases.\n>>>>>>>\n>>>>>>> The name \"Pending\" is not kept because any application depending on it\n>>>>>>> has to be adjusted, so it is better to break the API as well. This is also\n>>>>>>> an ABI break.\n>>>>>>>\n>>>>>>> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n>>>>>>> Signed-off-by: Barnabás Pőcze \n>>>>>>> ---\n>>>>>>> .../internal/tracepoints/request_enums.tp | 7 ++--\n>>>>>>> include/libcamera/request.h | 3 +-\n>>>>>>> src/libcamera/camera.cpp | 2 +-\n>>>>>>> src/libcamera/pipeline_handler.cpp | 7 +++-\n>>>>>>> src/libcamera/request.cpp | 34 ++++++++++++-------\n>>>>>>> src/py/libcamera/py_main.cpp | 3 +-\n>>>>>>> 6 files changed, 36 insertions(+), 20 deletions(-)\n>>>>>>>\n>>>>>>> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n>>>>>>> index bcbd1aaa2c..e9a5a78fac 100644\n>>>>>>> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n>>>>>>> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n>>>>>>> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n>>>>>>> libcamera,\n>>>>>>> request_status,\n>>>>>>> TP_ENUM_VALUES(\n>>>>>>> - ctf_enum_value(\"RequestPending\", 0)\n>>>>>>> - ctf_enum_value(\"RequestComplete\", 1)\n>>>>>>> - ctf_enum_value(\"RequestCancelled\", 2)\n>>>>>>> + ctf_enum_value(\"RequestIdle\", 0)\n>>>>>>> + ctf_enum_value(\"RequestInProgress\", 1)\n>>>>>>> + ctf_enum_value(\"RequestComplete\", 2)\n>>>>>>> + ctf_enum_value(\"RequestCancelled\", 3)\n>>>>>>> )\n>>>>>>> )\n>>>>>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>>>>>>> index 0c5939f7b3..1e7a5ca807 100644\n>>>>>>> --- a/include/libcamera/request.h\n>>>>>>> +++ b/include/libcamera/request.h\n>>>>>>> @@ -32,7 +32,8 @@ class Request : public Extensible\n>>>>>>> \n>>>>>>> public:\n>>>>>>> enum Status {\n>>>>>>> - RequestPending,\n>>>>>>> + RequestIdle,\n>>>>>>> + RequestInProgress,\n>>>>>>> RequestComplete,\n>>>>>>> RequestCancelled,\n>>>>>>> };\n>>>>>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>>>>>> index 2e1e146a25..b6273f6c29 100644\n>>>>>>> --- a/src/libcamera/camera.cpp\n>>>>>>> +++ b/src/libcamera/camera.cpp\n>>>>>>> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n>>>>>>> return -EXDEV;\n>>>>>>> }\n>>>>>>> \n>>>>>>> - if (request->status() != Request::RequestPending) {\n>>>>>>> + if (request->status() != Request::RequestIdle) {\n>>>>>>> LOG(Camera, Error) << request->toString() << \" is not valid\";\n>>>>>>> return -EINVAL;\n>>>>>>> }\n>>>>>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>>>>>> index 5c469e5bad..417b90245a 100644\n>>>>>>> --- a/src/libcamera/pipeline_handler.cpp\n>>>>>>> +++ b/src/libcamera/pipeline_handler.cpp\n>>>>>>> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n>>>>>>> {\n>>>>>>> LIBCAMERA_TRACEPOINT(request_queue, request);\n>>>>>>> \n>>>>>>> + ASSERT(request->status() == Request::RequestIdle);\n>>>>>>> +\n>>>>>>> Camera *camera = request->_d()->camera();\n>>>>>>> Camera::Private *data = camera->_d();\n>>>>>>> data->waitingRequests_.push(request);\n>>>>>>> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n>>>>>>> break;\n>>>>>>> \n>>>>>>> Request *request = data->waitingRequests_.front();\n>>>>>>> + ASSERT(request->status() == Request::RequestInProgress);\n>>>>>>> if (!request->_d()->prepared_)\n>>>>>>> break;\n>>>>>>> \n>>>>>>> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n>>>>>>> \n>>>>>>> while (!data->queuedRequests_.empty()) {\n>>>>>>> Request *req = data->queuedRequests_.front();\n>>>>>>> - if (req->status() == Request::RequestPending)\n>>>>>>> + if (req->status() == Request::RequestInProgress)\n>>>>>>> break;\n>>>>>>> \n>>>>>>> + ASSERT(req->status() == Request::RequestCancelled ||\n>>>>>>> + req->status() == Request::RequestComplete);\n>>>>>>> ASSERT(!req->hasPendingBuffers());\n>>>>>>> data->queuedRequests_.pop_front();\n>>>>>>> camera->requestComplete(req);\n>>>>>>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n>>>>>>> index 60565f5984..89ad0d7c8d 100644\n>>>>>>> --- a/src/libcamera/request.cpp\n>>>>>>> +++ b/src/libcamera/request.cpp\n>>>>>>> @@ -121,7 +121,7 @@ void Request::Private::complete()\n>>>>>>> {\n>>>>>>> Request *request = _o();\n>>>>>>> \n>>>>>>> - ASSERT(request->status() == RequestPending);\n>>>>>>> + ASSERT(request->status() == RequestInProgress);\n>>>>>>> ASSERT(!hasPendingBuffers());\n>>>>>>> \n>>>>>>> request->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n>>>>>>> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n>>>>>>> LIBCAMERA_TRACEPOINT(request_cancel, this);\n>>>>>>> \n>>>>>>> Request *request = _o();\n>>>>>>> - ASSERT(request->status() == RequestPending);\n>>>>>>> + ASSERT(request->status() == RequestInProgress);\n>>>>>>> \n>>>>>>> doCancelRequest();\n>>>>>>> }\n>>>>>>> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n>>>>>>> */\n>>>>>>> void Request::Private::prepare(std::chrono::milliseconds timeout)\n>>>>>>> {\n>>>>>>> + _o()->status_ = RequestInProgress;\n>>>>>>> +\n>>>>>>\n>>>>>> My understanding is that RequestInProgress should be the status as soon\n>>>>>> as the queueRequest() return successfully. Otherwise, it is again hard\n>>>>>> to differentiate between:\n>>>>>> 1. RequestIdle (application managing the request)\n>>>>>> 2. RequestIdle (request queued by application, and is in internal\n>>>>>> libcamera waiting queue), what happens if application requeues the same\n>>>>>> request here?\n>>>>>>\n>>>>>> Do we need additional status to differentiate between the two?\n>>>>>>\n>>>>>> RequestIdle\n>>>>>> RequestQueued\n>>>>>> RequestInProgress ?\n>>>>>\n>>>>> PipelineHandler::queueRequest() \"immediately\" calls \"prepare()\" on the request,\n>>>>> so the state transition will happen \"quickly\", so this does not seem that problematic\n>>>>> to me at the moment. And we should probably limit the number of \"internal\" states exposed\n>>>>> in the public interface.\n>>>>>\n>>>>> But something more atomic would be better because there is indeed a small time window\n>>>>> between `Camera::queueRequest()` returning and `Request::Private::prepare()` running.\n>>>>> (And this say nothing about two parallel `Camera::queueRequest()` calls.)\n>>>>> `Request::status_` is also not accessed atomically, which is also an issue.\n>>>>>\n>>>>> So there are problems, I mostly just wanted to start some kind of discussion about this.\n>>>>>\n>>>>> Some options:\n>>>>>\n>>>>> * keep `RequestPending` as is, add an internal `std::atomic_bool queued` to `Request::Private`\n>>>>> to distinguish the two \"pending\" states;\n>>>>> * add \"Idle\" and \"InProgress\" states as proposed here, but additionally make `Request::status_` atomic.\n>>>>\n>>>> Feel free to explore splitting the state to only expose to applications\n>>>> the information they need, and keeping the rest internal. The current\n>>>> states already feel a bit dirty, with RequestComplete and\n>>>> RequestCancelled both indicating completed requests, so there could be\n>>>> room for improvement (I'm not entirely certain alternatives would be\n>>>> clearly better though).\n>>>>\n>>>>> In both cases `Camera::queueRequest()` would need to be changed to do an atomic CAS\n>>>>> to check and transition into the new state.\n>>>>\n>>>> I don't think you need to spend too much effort trying to prevent two\n>>>> concurrent queueRequest() calls using the same request. If it's really\n>>>> easy to do I won't object, but it's a clear application bug that I would\n>>>> personally consider as deserving as much empathy as\n>>>> queueRequest(reinterpret_cast(0xdeadbeef));\n>>>\n>>> Hmm, maybe it's because I myself have run into this issue, but I\n>>> feel quite differently about this. I think this should be robustly\n>>> diagnosed.\n>>\n>> I'm also going to be vocally on the side of *We should do better to\n>> support applications here*.\n>>\n>> This reminds me of the Request Canary I proposed\n>> https://patchwork.libcamera.org/patch/18226/ - Which I /still/ put on\n>> customer branches and it *has* caught and prevented bugs in those\n>> developments.\n>>\n>>\n>> Yes, applications can do bad things, and they can double free or\n>> introduce use after free errors. And in C scope - perhaps those can\n>> easily be detected with asan or such.\n>>\n>> Here /we/ are the framework - and /we/ provide the interfaces.\n>>\n>> If we put the shotgun in the hands of the developer, when they point it\n>> at their feet, I think we have a responsibilty to tell them what will\n>> happen when they pull the trigger.\n> \n> As I said in my previous e-mail, adding safeguards to report when an\n> already queued request is queued is a good thing. Safeguards against\n> *concurrent* queueRequest() calls from different threads *with the same\n> request* is what I said seems overkill. Barnabás, which one of the two\n> did you mean ?\n\nI must have misunderstood then. I wasn't really making a distinction between the two\nbecause it seems to me that the only real difference is whether an atomic cas is used\nor not; at least in both approaches I tried. In any case I believe it can be expected\nthat most applications will use just the same thread to manage a camera, so the\nconcurrent case is probably a lot less likely. Nonetheless it seems to me that it\ncan be addressed relatively easily.\n\n\n> \n>>>>>>> /* Create and connect one notifier for each synchronization fence. */\n>>>>>>> for (FrameBuffer *buffer : pending_) {\n>>>>>>> const Fence *fence = buffer->_d()->fence();\n>>>>>>> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n>>>>>>> /**\n>>>>>>> * \\enum Request::Status\n>>>>>>> * Request completion status\n>>>>>>> - * \\var Request::RequestPending\n>>>>>>> - * The request hasn't completed yet\n>>>>>>> + * \\var Request::RequestIdle\n>>>>>>> + * The request is idle\n>>>>>>> + * \\var Request::RequestInProgress\n>>>>>>> + * The request is being processed\n>>>>>>> * \\var Request::RequestComplete\n>>>>>>> * The request has completed\n>>>>>>> * \\var Request::RequestCancelled\n>>>>>>> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n>>>>>>> */\n>>>>>>> Request::Request(Camera *camera, uint64_t cookie)\n>>>>>>> : Extensible(std::make_unique(camera)),\n>>>>>>> - cookie_(cookie), status_(RequestPending)\n>>>>>>> + cookie_(cookie), status_(RequestIdle)\n>>>>>>> {\n>>>>>>> controls_ = new ControlList(controls::controls,\n>>>>>>> camera->_d()->validator());\n>>>>>>> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n>>>>>>> {\n>>>>>>> LIBCAMERA_TRACEPOINT(request_reuse, this);\n>>>>>>> \n>>>>>>> + ASSERT(status_ == RequestIdle ||\n>>>>>>> + status_ == RequestComplete ||\n>>>>>>> + status_ == RequestCancelled);\n>>>>>>> +\n>>>>>>> _d()->reset();\n>>>>>>> \n>>>>>>> if (flags & ReuseBuffers) {\n>>>>>>> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n>>>>>>> bufferMap_.clear();\n>>>>>>> }\n>>>>>>> \n>>>>>>> - status_ = RequestPending;\n>>>>>>> + status_ = RequestIdle;\n>>>>>>> \n>>>>>>> controls_->clear();\n>>>>>>> metadata_->clear();\n>>>>>>> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n>>>>>>> * \\fn Request::status()\n>>>>>>> * \\brief Retrieve the request completion status\n>>>>>>> *\n>>>>>>> - * The request status indicates whether the request has completed successfully\n>>>>>>> - * or with an error. When requests are created and before they complete the\n>>>>>>> - * request status is set to RequestPending, and is updated at completion time\n>>>>>>> - * to RequestComplete. If a request is cancelled at capture stop before it has\n>>>>>>> - * completed, its status is set to RequestCancelled.\n>>>>>>> + * The request status indicates whether the request has completed successfully or with\n>>>>>>> + * an error. When requests are created the request status is set to RequestIdle; when\n>>>>>>> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n>>>>>>> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n>>>>>>> + * it has completed, its status is set to RequestCancelled.\n>>>>>>> *\n>>>>>>> * \\return The request completion status\n>>>>>>> */\n>>>>>>> @@ -607,8 +615,8 @@ std::string Request::toString() const\n>>>>>>> */\n>>>>>>> std::ostream &operator<<(std::ostream &out, const Request &r)\n>>>>>>> {\n>>>>>>> - /* Pending, Completed, Cancelled(X). */\n>>>>>>> - static const char *statuses = \"PCX\";\n>>>>>>> + /* Idle, InProgress(P), Completed, Cancelled(X). */\n>>>>>>> + static const char statuses[] = \"IPCX\";\n>>>>>>> \n>>>>>>> /* Example Output: Request(55:P:1/2:6523524) */\n>>>>>>> out << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n>>>>>>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n>>>>>>> index a983ea75c3..d12f00a6f5 100644\n>>>>>>> --- a/src/py/libcamera/py_main.cpp\n>>>>>>> +++ b/src/py/libcamera/py_main.cpp\n>>>>>>> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n>>>>>>> .def(\"__str__\", &Request::toString);\n>>>>>>> \n>>>>>>> pyRequestStatus\n>>>>>>> - .value(\"Pending\", Request::RequestPending)\n>>>>>>> + .value(\"Idle\", Request::RequestIdle)\n>>>>>>> + .value(\"InProgress\", Request::RequestInProgress)\n>>>>>>> .value(\"Complete\", Request::RequestComplete)\n>>>>>>> .value(\"Cancelled\", Request::RequestCancelled);\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 64C42BD80A\n\tfor ;\n\tTue, 2 Dec 2025 18:08:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC1FD60D28;\n\tTue, 2 Dec 2025 19:08:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8955E60C8A\n\tfor ;\n\tTue, 2 Dec 2025 19:08:17 +0100 (CET)","from [192.168.33.23] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A9699FE;\n\tTue, 2 Dec 2025 19:06:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uOoMSut2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764698762;\n\tbh=qodJzlfTz47k/gJu8LtafTydg8ZaqQbRmwOE954Aid8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=uOoMSut2t1dj5JPj2RiEX1CTR7qR5+yWB0tpDhg3UahjcbEdUgdQWSklR4UHV2EGQ\n\tkuJzHmn0gnqIStTE56uOvrfko26JzfYYOX3icx9NJa5N0alCPIE+oUOVnf1AYCH7aX\n\t82fzvaEPYzeSTJD5jlrNFcag9ePA7AwEyhDLVGfE=","Message-ID":"","Date":"Tue, 2 Dec 2025 19:08:12 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","To":"Laurent Pinchart ,\n\tKieran Bingham ","Cc":"Umang Jain , libcamera-devel@lists.libcamera.org","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>\n\t\n\t<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>\n\t<20251201153409.GA32430@pendragon.ideasonboard.com>\n\t\n\t<176468010554.2003224.5961048517407596630@ping.linuxembedded.co.uk>\n\t<20251202145953.GA8219@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251202145953.GA8219@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37176,"web_url":"https://patchwork.libcamera.org/comment/37176/","msgid":"<20251203022605.GL8219@pendragon.ideasonboard.com>","date":"2025-12-03T02:26:05","subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Dec 02, 2025 at 07:08:12PM +0100, Barnabás Pőcze wrote:\n> 2025. 12. 02. 15:59 keltezéssel, Laurent Pinchart írta:\n> > On Tue, Dec 02, 2025 at 12:55:05PM +0000, Kieran Bingham wrote:\n> >> Quoting Barnabás Pőcze (2025-12-02 12:26:51)\n> >>> 2025. 12. 01. 16:34 keltezéssel, Laurent Pinchart írta:\n> >>>> On Mon, Dec 01, 2025 at 03:56:39PM +0100, Barnabás Pőcze wrote:\n> >>>>> 2025. 12. 01. 15:42 keltezéssel, Umang Jain írta:\n> >>>>>> On 2025-12-01 18:14, Barnabás Pőcze wrote:\n> >>>>>>> The \"Pending\" state at the moment is used for two very distinct scenarios:\n> >>>>>>> when the application alone is managing the request, and when the request\n> >>>>>>> is being processed. This duality makes it harder to track the true state of\n> >>>>>>> a request and consequently it is harder to diagnose misuses.\n> >>>>>>>\n> >>>>>>> So split the \"Pending\" state into the \"Idle\" and \"InProgress\" states to\n> >>>>>>> make it explicit when a camera is managing the request. This improves\n> >>>>>>> the detection of request double queueing, but since the status flag is\n> >>>>>>> not updated atomically, it cannot detect all cases.\n> >>>>>>>\n> >>>>>>> The name \"Pending\" is not kept because any application depending on it\n> >>>>>>> has to be adjusted, so it is better to break the API as well. This is also\n> >>>>>>> an ABI break.\n> >>>>>>>\n> >>>>>>> Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197\n> >>>>>>> Signed-off-by: Barnabás Pőcze \n> >>>>>>> ---\n> >>>>>>> .../internal/tracepoints/request_enums.tp | 7 ++--\n> >>>>>>> include/libcamera/request.h | 3 +-\n> >>>>>>> src/libcamera/camera.cpp | 2 +-\n> >>>>>>> src/libcamera/pipeline_handler.cpp | 7 +++-\n> >>>>>>> src/libcamera/request.cpp | 34 ++++++++++++-------\n> >>>>>>> src/py/libcamera/py_main.cpp | 3 +-\n> >>>>>>> 6 files changed, 36 insertions(+), 20 deletions(-)\n> >>>>>>>\n> >>>>>>> diff --git a/include/libcamera/internal/tracepoints/request_enums.tp b/include/libcamera/internal/tracepoints/request_enums.tp\n> >>>>>>> index bcbd1aaa2c..e9a5a78fac 100644\n> >>>>>>> --- a/include/libcamera/internal/tracepoints/request_enums.tp\n> >>>>>>> +++ b/include/libcamera/internal/tracepoints/request_enums.tp\n> >>>>>>> @@ -9,8 +9,9 @@ TRACEPOINT_ENUM(\n> >>>>>>> libcamera,\n> >>>>>>> request_status,\n> >>>>>>> TP_ENUM_VALUES(\n> >>>>>>> - ctf_enum_value(\"RequestPending\", 0)\n> >>>>>>> - ctf_enum_value(\"RequestComplete\", 1)\n> >>>>>>> - ctf_enum_value(\"RequestCancelled\", 2)\n> >>>>>>> + ctf_enum_value(\"RequestIdle\", 0)\n> >>>>>>> + ctf_enum_value(\"RequestInProgress\", 1)\n> >>>>>>> + ctf_enum_value(\"RequestComplete\", 2)\n> >>>>>>> + ctf_enum_value(\"RequestCancelled\", 3)\n> >>>>>>> )\n> >>>>>>> )\n> >>>>>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> >>>>>>> index 0c5939f7b3..1e7a5ca807 100644\n> >>>>>>> --- a/include/libcamera/request.h\n> >>>>>>> +++ b/include/libcamera/request.h\n> >>>>>>> @@ -32,7 +32,8 @@ class Request : public Extensible\n> >>>>>>> \n> >>>>>>> public:\n> >>>>>>> enum Status {\n> >>>>>>> - RequestPending,\n> >>>>>>> + RequestIdle,\n> >>>>>>> + RequestInProgress,\n> >>>>>>> RequestComplete,\n> >>>>>>> RequestCancelled,\n> >>>>>>> };\n> >>>>>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> >>>>>>> index 2e1e146a25..b6273f6c29 100644\n> >>>>>>> --- a/src/libcamera/camera.cpp\n> >>>>>>> +++ b/src/libcamera/camera.cpp\n> >>>>>>> @@ -1340,7 +1340,7 @@ int Camera::queueRequest(Request *request)\n> >>>>>>> return -EXDEV;\n> >>>>>>> }\n> >>>>>>> \n> >>>>>>> - if (request->status() != Request::RequestPending) {\n> >>>>>>> + if (request->status() != Request::RequestIdle) {\n> >>>>>>> LOG(Camera, Error) << request->toString() << \" is not valid\";\n> >>>>>>> return -EINVAL;\n> >>>>>>> }\n> >>>>>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >>>>>>> index 5c469e5bad..417b90245a 100644\n> >>>>>>> --- a/src/libcamera/pipeline_handler.cpp\n> >>>>>>> +++ b/src/libcamera/pipeline_handler.cpp\n> >>>>>>> @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request)\n> >>>>>>> {\n> >>>>>>> LIBCAMERA_TRACEPOINT(request_queue, request);\n> >>>>>>> \n> >>>>>>> + ASSERT(request->status() == Request::RequestIdle);\n> >>>>>>> +\n> >>>>>>> Camera *camera = request->_d()->camera();\n> >>>>>>> Camera::Private *data = camera->_d();\n> >>>>>>> data->waitingRequests_.push(request);\n> >>>>>>> @@ -514,6 +516,7 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> >>>>>>> break;\n> >>>>>>> \n> >>>>>>> Request *request = data->waitingRequests_.front();\n> >>>>>>> + ASSERT(request->status() == Request::RequestInProgress);\n> >>>>>>> if (!request->_d()->prepared_)\n> >>>>>>> break;\n> >>>>>>> \n> >>>>>>> @@ -591,9 +594,11 @@ void PipelineHandler::completeRequest(Request *request)\n> >>>>>>> \n> >>>>>>> while (!data->queuedRequests_.empty()) {\n> >>>>>>> Request *req = data->queuedRequests_.front();\n> >>>>>>> - if (req->status() == Request::RequestPending)\n> >>>>>>> + if (req->status() == Request::RequestInProgress)\n> >>>>>>> break;\n> >>>>>>> \n> >>>>>>> + ASSERT(req->status() == Request::RequestCancelled ||\n> >>>>>>> + req->status() == Request::RequestComplete);\n> >>>>>>> ASSERT(!req->hasPendingBuffers());\n> >>>>>>> data->queuedRequests_.pop_front();\n> >>>>>>> camera->requestComplete(req);\n> >>>>>>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> >>>>>>> index 60565f5984..89ad0d7c8d 100644\n> >>>>>>> --- a/src/libcamera/request.cpp\n> >>>>>>> +++ b/src/libcamera/request.cpp\n> >>>>>>> @@ -121,7 +121,7 @@ void Request::Private::complete()\n> >>>>>>> {\n> >>>>>>> Request *request = _o();\n> >>>>>>> \n> >>>>>>> - ASSERT(request->status() == RequestPending);\n> >>>>>>> + ASSERT(request->status() == RequestInProgress);\n> >>>>>>> ASSERT(!hasPendingBuffers());\n> >>>>>>> \n> >>>>>>> request->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n> >>>>>>> @@ -159,7 +159,7 @@ void Request::Private::cancel()\n> >>>>>>> LIBCAMERA_TRACEPOINT(request_cancel, this);\n> >>>>>>> \n> >>>>>>> Request *request = _o();\n> >>>>>>> - ASSERT(request->status() == RequestPending);\n> >>>>>>> + ASSERT(request->status() == RequestInProgress);\n> >>>>>>> \n> >>>>>>> doCancelRequest();\n> >>>>>>> }\n> >>>>>>> @@ -222,6 +222,8 @@ void Request::Private::emitPrepareCompleted()\n> >>>>>>> */\n> >>>>>>> void Request::Private::prepare(std::chrono::milliseconds timeout)\n> >>>>>>> {\n> >>>>>>> + _o()->status_ = RequestInProgress;\n> >>>>>>> +\n> >>>>>>\n> >>>>>> My understanding is that RequestInProgress should be the status as soon\n> >>>>>> as the queueRequest() return successfully. Otherwise, it is again hard\n> >>>>>> to differentiate between:\n> >>>>>> 1. RequestIdle (application managing the request)\n> >>>>>> 2. RequestIdle (request queued by application, and is in internal\n> >>>>>> libcamera waiting queue), what happens if application requeues the same\n> >>>>>> request here?\n> >>>>>>\n> >>>>>> Do we need additional status to differentiate between the two?\n> >>>>>>\n> >>>>>> RequestIdle\n> >>>>>> RequestQueued\n> >>>>>> RequestInProgress ?\n> >>>>>\n> >>>>> PipelineHandler::queueRequest() \"immediately\" calls \"prepare()\" on the request,\n> >>>>> so the state transition will happen \"quickly\", so this does not seem that problematic\n> >>>>> to me at the moment. And we should probably limit the number of \"internal\" states exposed\n> >>>>> in the public interface.\n> >>>>>\n> >>>>> But something more atomic would be better because there is indeed a small time window\n> >>>>> between `Camera::queueRequest()` returning and `Request::Private::prepare()` running.\n> >>>>> (And this say nothing about two parallel `Camera::queueRequest()` calls.)\n> >>>>> `Request::status_` is also not accessed atomically, which is also an issue.\n> >>>>>\n> >>>>> So there are problems, I mostly just wanted to start some kind of discussion about this.\n> >>>>>\n> >>>>> Some options:\n> >>>>>\n> >>>>> * keep `RequestPending` as is, add an internal `std::atomic_bool queued` to `Request::Private`\n> >>>>> to distinguish the two \"pending\" states;\n> >>>>> * add \"Idle\" and \"InProgress\" states as proposed here, but additionally make `Request::status_` atomic.\n> >>>>\n> >>>> Feel free to explore splitting the state to only expose to applications\n> >>>> the information they need, and keeping the rest internal. The current\n> >>>> states already feel a bit dirty, with RequestComplete and\n> >>>> RequestCancelled both indicating completed requests, so there could be\n> >>>> room for improvement (I'm not entirely certain alternatives would be\n> >>>> clearly better though).\n> >>>>\n> >>>>> In both cases `Camera::queueRequest()` would need to be changed to do an atomic CAS\n> >>>>> to check and transition into the new state.\n> >>>>\n> >>>> I don't think you need to spend too much effort trying to prevent two\n> >>>> concurrent queueRequest() calls using the same request. If it's really\n> >>>> easy to do I won't object, but it's a clear application bug that I would\n> >>>> personally consider as deserving as much empathy as\n> >>>> queueRequest(reinterpret_cast(0xdeadbeef));\n> >>>\n> >>> Hmm, maybe it's because I myself have run into this issue, but I\n> >>> feel quite differently about this. I think this should be robustly\n> >>> diagnosed.\n> >>\n> >> I'm also going to be vocally on the side of *We should do better to\n> >> support applications here*.\n> >>\n> >> This reminds me of the Request Canary I proposed\n> >> https://patchwork.libcamera.org/patch/18226/ - Which I /still/ put on\n> >> customer branches and it *has* caught and prevented bugs in those\n> >> developments.\n> >>\n> >>\n> >> Yes, applications can do bad things, and they can double free or\n> >> introduce use after free errors. And in C scope - perhaps those can\n> >> easily be detected with asan or such.\n> >>\n> >> Here /we/ are the framework - and /we/ provide the interfaces.\n> >>\n> >> If we put the shotgun in the hands of the developer, when they point it\n> >> at their feet, I think we have a responsibilty to tell them what will\n> >> happen when they pull the trigger.\n> > \n> > As I said in my previous e-mail, adding safeguards to report when an\n> > already queued request is queued is a good thing. Safeguards against\n> > *concurrent* queueRequest() calls from different threads *with the same\n> > request* is what I said seems overkill. Barnabás, which one of the two\n> > did you mean ?\n> \n> I must have misunderstood then. I wasn't really making a distinction between the two\n> because it seems to me that the only real difference is whether an atomic cas is used\n> or not; at least in both approaches I tried. In any case I believe it can be expected\n> that most applications will use just the same thread to manage a camera, so the\n> concurrent case is probably a lot less likely. Nonetheless it seems to me that it\n> can be addressed relatively easily.\n\nIf it's easy, sure, it's good to have that check.\n\n> >>>>>>> /* Create and connect one notifier for each synchronization fence. */\n> >>>>>>> for (FrameBuffer *buffer : pending_) {\n> >>>>>>> const Fence *fence = buffer->_d()->fence();\n> >>>>>>> @@ -309,8 +311,10 @@ void Request::Private::timeout()\n> >>>>>>> /**\n> >>>>>>> * \\enum Request::Status\n> >>>>>>> * Request completion status\n> >>>>>>> - * \\var Request::RequestPending\n> >>>>>>> - * The request hasn't completed yet\n> >>>>>>> + * \\var Request::RequestIdle\n> >>>>>>> + * The request is idle\n> >>>>>>> + * \\var Request::RequestInProgress\n> >>>>>>> + * The request is being processed\n> >>>>>>> * \\var Request::RequestComplete\n> >>>>>>> * The request has completed\n> >>>>>>> * \\var Request::RequestCancelled\n> >>>>>>> @@ -354,7 +358,7 @@ void Request::Private::timeout()\n> >>>>>>> */\n> >>>>>>> Request::Request(Camera *camera, uint64_t cookie)\n> >>>>>>> : Extensible(std::make_unique(camera)),\n> >>>>>>> - cookie_(cookie), status_(RequestPending)\n> >>>>>>> + cookie_(cookie), status_(RequestIdle)\n> >>>>>>> {\n> >>>>>>> controls_ = new ControlList(controls::controls,\n> >>>>>>> camera->_d()->validator());\n> >>>>>>> @@ -391,6 +395,10 @@ void Request::reuse(ReuseFlag flags)\n> >>>>>>> {\n> >>>>>>> LIBCAMERA_TRACEPOINT(request_reuse, this);\n> >>>>>>> \n> >>>>>>> + ASSERT(status_ == RequestIdle ||\n> >>>>>>> + status_ == RequestComplete ||\n> >>>>>>> + status_ == RequestCancelled);\n> >>>>>>> +\n> >>>>>>> _d()->reset();\n> >>>>>>> \n> >>>>>>> if (flags & ReuseBuffers) {\n> >>>>>>> @@ -403,7 +411,7 @@ void Request::reuse(ReuseFlag flags)\n> >>>>>>> bufferMap_.clear();\n> >>>>>>> }\n> >>>>>>> \n> >>>>>>> - status_ = RequestPending;\n> >>>>>>> + status_ = RequestIdle;\n> >>>>>>> \n> >>>>>>> controls_->clear();\n> >>>>>>> metadata_->clear();\n> >>>>>>> @@ -563,11 +571,11 @@ uint32_t Request::sequence() const\n> >>>>>>> * \\fn Request::status()\n> >>>>>>> * \\brief Retrieve the request completion status\n> >>>>>>> *\n> >>>>>>> - * The request status indicates whether the request has completed successfully\n> >>>>>>> - * or with an error. When requests are created and before they complete the\n> >>>>>>> - * request status is set to RequestPending, and is updated at completion time\n> >>>>>>> - * to RequestComplete. If a request is cancelled at capture stop before it has\n> >>>>>>> - * completed, its status is set to RequestCancelled.\n> >>>>>>> + * The request status indicates whether the request has completed successfully or with\n> >>>>>>> + * an error. When requests are created the request status is set to RequestIdle; when\n> >>>>>>> + * they are queued, the status is set to RequestInProgress. Then it is updated at\n> >>>>>>> + * completion time to RequestComplete. If a request is cancelled at capture stop before\n> >>>>>>> + * it has completed, its status is set to RequestCancelled.\n> >>>>>>> *\n> >>>>>>> * \\return The request completion status\n> >>>>>>> */\n> >>>>>>> @@ -607,8 +615,8 @@ std::string Request::toString() const\n> >>>>>>> */\n> >>>>>>> std::ostream &operator<<(std::ostream &out, const Request &r)\n> >>>>>>> {\n> >>>>>>> - /* Pending, Completed, Cancelled(X). */\n> >>>>>>> - static const char *statuses = \"PCX\";\n> >>>>>>> + /* Idle, InProgress(P), Completed, Cancelled(X). */\n> >>>>>>> + static const char statuses[] = \"IPCX\";\n> >>>>>>> \n> >>>>>>> /* Example Output: Request(55:P:1/2:6523524) */\n> >>>>>>> out << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> >>>>>>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> >>>>>>> index a983ea75c3..d12f00a6f5 100644\n> >>>>>>> --- a/src/py/libcamera/py_main.cpp\n> >>>>>>> +++ b/src/py/libcamera/py_main.cpp\n> >>>>>>> @@ -486,7 +486,8 @@ PYBIND11_MODULE(_libcamera, m)\n> >>>>>>> .def(\"__str__\", &Request::toString);\n> >>>>>>> \n> >>>>>>> pyRequestStatus\n> >>>>>>> - .value(\"Pending\", Request::RequestPending)\n> >>>>>>> + .value(\"Idle\", Request::RequestIdle)\n> >>>>>>> + .value(\"InProgress\", Request::RequestInProgress)\n> >>>>>>> .value(\"Complete\", Request::RequestComplete)\n> >>>>>>> .value(\"Cancelled\", Request::RequestCancelled);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 25931C3257\n\tfor ;\n\tWed, 3 Dec 2025 02:26:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 614F860D39;\n\tWed, 3 Dec 2025 03:26:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 07E6B606D5\n\tfor ;\n\tWed, 3 Dec 2025 03:26:25 +0100 (CET)","from pendragon.ideasonboard.com (p100198.f.east.v6connect.net\n\t[221.113.100.198])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id BB5E7B5;\n\tWed, 3 Dec 2025 03:24:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Pjyd98D6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764728650;\n\tbh=ptU5YTizmit9tZ+t2lyeFYxxi/8+v9sSYj2LOCefJ00=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Pjyd98D6NNvQS0enWcDt/BLWR0Fk08lmTjF40AtEglwMbwXEsv9ZzmBNaDAz1amYx\n\twhFbl+xE8lWxRxShdFyZ/iTFEa811kOd4ga9Cnz6FzlvVUSe1lfew/MMfQh9MmT1dl\n\tThJ/kVUg7yjT8tjx3UuWGf4pxSNKC2ogFmTd4leU=","Date":"Wed, 3 Dec 2025 11:26:05 +0900","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Kieran Bingham ,\n\tUmang Jain , libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1] libcamera: request: Split \"Pending\" state into\n\ttwo","Message-ID":"<20251203022605.GL8219@pendragon.ideasonboard.com>","References":"<20251201124415.1307419-1-barnabas.pocze@ideasonboard.com>\n\t\n\t<2c4053da-00a2-47c1-aafd-22dab83b1c32@ideasonboard.com>\n\t<20251201153409.GA32430@pendragon.ideasonboard.com>\n\t\n\t<176468010554.2003224.5961048517407596630@ping.linuxembedded.co.uk>\n\t<20251202145953.GA8219@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]