[{"id":15991,"web_url":"https://patchwork.libcamera.org/comment/15991/","msgid":"<20210329052650.2p5bph3dcrfvzxuu@basti-TUXEDO-Book-XA1510>","date":"2021-03-29T05:26:50","subject":"Re: [libcamera-devel] [RFC PATCH 2/2] libcamera: PipelineHandler:\n\tRetry queuing a capture request","submitter":{"id":78,"url":"https://patchwork.libcamera.org/api/people/78/","name":"Sebastian Fricke","email":"sebastian.fricke@posteo.net"},"content":"Hey Hirokazu,\n\nThanks for the patch.\n\nThis patch currently doesn't apply on the master branch of libcamera:\n```\nerror: sha1 information is lacking or useless (include/libcamera/internal/pipeline_handler.h).\nerror: could not build fake ancestor\nPatch failed at 0002 libcamera: PipelineHandler: Retry queuing a capture request\n```\n\n\nOn 29.03.2021 11:26, Hirokazu Honda wrote:\n>PipelineHandler::queueRequestDevice() fails due to a buffer\n>shortage. We should retry queuing a capture request later in the\n>case.\n\ns/in the case/in that case/\n\n>\n>Signed-off-by: Hirokazu Honda \n>---\n> include/libcamera/internal/pipeline_handler.h | 2 +\n> src/libcamera/pipeline_handler.cpp | 57 +++++++++++++++++--\n> 2 files changed, 54 insertions(+), 5 deletions(-)\n>\n>diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n>index 763da63e..e6f771a6 100644\n>--- a/include/libcamera/internal/pipeline_handler.h\n>+++ b/include/libcamera/internal/pipeline_handler.h\n>@@ -44,6 +44,7 @@ public:\n> \tvirtual ~CameraData() = default;\n>\n> \tPipelineHandler *pipe_;\n>+\tstd::queue waitedRequests_;\n\ns/waitedRequests_/waitingRequests_/ ?\n\nWaited is the past tense of wait and therefore this variable sounds like\nthose are requests, that we waited for in the past, but we still wait\nfor them. Therefore we should use the present progressive, which in the\ncase of wait is waiting.\nAs an alternative, we might also call the variable: failedRequests_,\nas it describes requests, that failed to be inserted into the queue\nand need to be queued again.\n\nThe correction applies to all other cases below, where that variable is\nused.\n\n> \tstd::deque queuedRequests_;\n> \tControlInfoMap controlInfo_;\n> \tControlList properties_;\n>@@ -99,6 +100,7 @@ protected:\n> \tCameraManager *manager_;\n>\n> private:\n>+\tvoid tryQueueRequests(CameraData *data);\n> \tvoid mediaDeviceDisconnected(MediaDevice *media);\n> \tvirtual void disconnect();\n>\n>diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>index 4cb58084..18952a1b 100644\n>--- a/src/libcamera/pipeline_handler.cpp\n>+++ b/src/libcamera/pipeline_handler.cpp\n>@@ -70,6 +70,16 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> * and remains valid until the instance is destroyed.\n> */\n>\n>+/**\n>+ * \\var CameraData::waitedRequests_\n>+ * \\brief The queue of not yet queued request\n>+ *\n>+ * The queue of not yet queued request is used to track requests that are not\n>+ * queued in order to retry queueing them when a pipeline is ready to accept.\n\nI find this sentence confusing to read, there is too much use of the\nword queue. How about:\nThis queue of failed requests is used to retry queuing as soon as the pipeline\nis ready to accept them.\n\n\n>+ *\n>+ * \\sa PipelineHandler::queueRequest(), PipelineHandler::tryQueueRequests().\n>+ */\n>+\n> /**\n> * \\var CameraData::queuedRequests_\n> * \\brief The queue of queued and not yet completed request\n>@@ -378,12 +388,44 @@ void PipelineHandler::queueRequest(Request *request)\n>\n> \tCamera *camera = request->camera_;\n> \tCameraData *data = cameraData(camera);\n>-\tdata->queuedRequests_.push_back(request);\n>+\tdata->waitedRequests_.push(request);\n>+\n>+\ttryQueueRequests(data);\n>+}\n>+\n>+/**\n>+ * \\fn PipelineHandler::tryQueueRequests()\n>+ * \\brief Queue requests that are not yet queued.\n>+ * \\param[in] data The camera data whose waited requests are tried to queue.\n\ns/The camera data whose waited requests are tried to queue./\n CameraData containing a queue of failed requests to retry queuing./\n>+ *\n>+ * This tries to queue as many requests as possible in order of the\n>+ * waitedRequests_ in data. If queuing fails due to a buffer shortage, this\n>+ * method stops and the next call of this method starts from the request that\n>+ * fails in the previous call. On any other failures, the request is marked as\n\ns/fails/failed/\ns/any other failures/any other failure/\n\n>+ * complete and proceed the successive requests.\n\ns/proceed the/proceed with/\n\n>+ *\n>+ * \\context This function shall be called from the CameraManager thread.\n>+ */\n>+void PipelineHandler::tryQueueRequests(CameraData *data)\n>+{\n>+\twhile (!data->waitedRequests_.empty()) {\n>+\t\tRequest *request = data->waitedRequests_.front();\n>+\t\tCamera *camera = request->camera_;\n>+\t\tASSERT(data == cameraData(camera));\n>+\n>+\t\tdata->queuedRequests_.push_back(request);\n>+\t\tint ret = queueRequestDevice(camera, request);\n>+\t\tif (ret == -ENOBUFS || ret == -ENOMEM) {\n>+\t\t\tdata->queuedRequests_.pop_back();\n>+\t\t\tbreak;\n>+\t\t}\n>\n>-\tint ret = queueRequestDevice(camera, request);\n>-\tif (ret) {\n>-\t\trequest->result_ = ret;\n>-\t\tcompleteRequest(request);\n>+\t\tdata->waitedRequests_.pop();\n>+\t\tif (ret) {\n>+\t\t\trequest->result_ = ret;\n>+\t\t\tcompleteRequest(request);\n>+\t\t\tbreak;\n>+\t\t}\n> \t}\n> }\n>\n>@@ -440,6 +482,9 @@ bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)\n> * submission order, the pipeline handler may call it on any complete request\n> * without any ordering constraint.\n> *\n>+ * There might be requests that are waiting to be queued, this triggers\n>+ * tryQueueRequests().\n>+ *\n> * \\context This function shall be called from the CameraManager thread.\n> */\n> void PipelineHandler::completeRequest(Request *request)\n>@@ -459,6 +504,8 @@ void PipelineHandler::completeRequest(Request *request)\n> \t\tdata->queuedRequests_.pop_front();\n> \t\tcamera->requestComplete(req);\n> \t}\n>+\n>+\ttryQueueRequests(data);\n> }\n\nIt is difficult for me to follow the code here and the patch currently\ndoesn't apply could you rebase it on master and repost?\n\nGreetings,\nSebastian\n>\n> /**\n>-- \n>2.31.0.291.g576ba9dcdaf-goog\n>\n>_______________________________________________\n>libcamera-devel mailing list\n>libcamera-devel@lists.libcamera.org\n>https://lists.libcamera.org/listinfo/libcamera-devel","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 2DE41C32EA\n\tfor ;\n\tMon, 29 Mar 2021 05:26:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6BB6F6877D;\n\tMon, 29 Mar 2021 07:26:53 +0200 (CEST)","from mout02.posteo.de (mout02.posteo.de [185.67.36.66])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60CE6602D2\n\tfor ;\n\tMon, 29 Mar 2021 07:26:52 +0200 (CEST)","from submission (posteo.de [89.146.220.130]) \n\tby mout02.posteo.de (Postfix) with ESMTPS id E25A52400FB\n\tfor ;\n\tMon, 29 Mar 2021 07:26:51 +0200 (CEST)","from customer (localhost [127.0.0.1])\n\tby submission (posteo.de) with ESMTPSA id 4F81KR36mkz6tmB;\n\tMon, 29 Mar 2021 07:26:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=posteo.net header.i=@posteo.net\n\theader.b=\"BBl6E5eD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017;\n\tt=1616995611; bh=rErxzxjJ011b5NmrgWXi68aw8ZyOL9iSNOQVITY1KL0=;\n\th=Date:From:To:Cc:Subject:From;\n\tb=BBl6E5eDejJOO/MB4cOjwu35iQmD+zCiQuJLVD1Hl3ZQKchxaTTc/bwRRvX1FQkhV\n\tjlQ27+bEPka9E3L+lC0pq97Lmzx3ldcYHl8Qu4TdHWQDJNgNO3puJLHGiQNxmr8a8M\n\tMlzPPatsX5JWdQMyrZfPauc+tbxKciEcQzaPdoRSMCeU3fDEDHiGtvkRg254ofpwTn\n\tQayPifsmv0CnVodziKSeliNVaG+0iFQhHwxAgAXBL0eT5v/nmt+U19H/WSvQvHHvXK\n\t6NcbBlk5vICT1BQyhhO9c5guyaHuJmC57f4eHfHLD9JsezNk8nKQ2o5CMEQb0ZdEJ4\n\tomibPRrHbISLQ==","Date":"Mon, 29 Mar 2021 07:26:50 +0200","From":"Sebastian Fricke ","To":"Hirokazu Honda ","Message-ID":"<20210329052650.2p5bph3dcrfvzxuu@basti-TUXEDO-Book-XA1510>","References":"<20210329022604.110201-1-hiroh@chromium.org>\n\t<20210329022604.110201-3-hiroh@chromium.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210329022604.110201-3-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 2/2] libcamera: PipelineHandler:\n\tRetry queuing a capture request","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15992,"web_url":"https://patchwork.libcamera.org/comment/15992/","msgid":"","date":"2021-03-29T05:27:26","subject":"Re: [libcamera-devel] [RFC PATCH 2/2] libcamera: PipelineHandler:\n\tRetry queuing a capture request","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Mon, Mar 29, 2021 at 11:26:04AM +0900, Hirokazu Honda wrote:\n> PipelineHandler::queueRequestDevice() fails due to a buffer\n> shortage. We should retry queuing a capture request later in the\n> case.\n\nI don't think this should be handled by the base PipelineHandler class,\nbut by individual pipeline handlers. There may be needs for pipeline\nhandlers to peek ahead in the queue of requests to get metadata needed\nby the algorithms, with a queue depth larger than the number of buffers.\nYou queue those requests on waitedRequests_ so the pipeline handler can\nhave access to them, but the semantics isn't well defined, and I think\nthat will be quite error prone. Furthermore, the call to\ntryQueueRequests() in PipelineHandler::completeRequest() will result in\nthe pipeline handler's queueRequestDevice() function being called from\nthe request completion handler, making the pipeline handler code\nreentrant, and pipeline handlers are not prepared for that. This may\ncreate race conditions, I'd rather not go that route.\n\nThis being said, duplicating request queues in all pipeline handlers\nisn't a great idea either. We should share code, but that should be\nimplemented by helper classes that pipeline handlers can opt-in to use,\nnot in a mandatory middle layer. Now that we have multiple pipeline\nhandler implementations, it's time to start cleaning things up, and the\nRaspberry Pi and IPU3 pipeline handlers are good candidates for code\nsharing as they share a common hardware architecture.\n\nIf you want to fix this issue for the IPU3 pipeline handler without\ndepending on the creation of those helper classes, then the fix should\nbe implemented in the IPU3 pipeline handler itself in the meantime.\n\n> Signed-off-by: Hirokazu Honda \n> ---\n> include/libcamera/internal/pipeline_handler.h | 2 +\n> src/libcamera/pipeline_handler.cpp | 57 +++++++++++++++++--\n> 2 files changed, 54 insertions(+), 5 deletions(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 763da63e..e6f771a6 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -44,6 +44,7 @@ public:\n> \tvirtual ~CameraData() = default;\n> \n> \tPipelineHandler *pipe_;\n> +\tstd::queue waitedRequests_;\n> \tstd::deque queuedRequests_;\n> \tControlInfoMap controlInfo_;\n> \tControlList properties_;\n> @@ -99,6 +100,7 @@ protected:\n> \tCameraManager *manager_;\n> \n> private:\n> +\tvoid tryQueueRequests(CameraData *data);\n> \tvoid mediaDeviceDisconnected(MediaDevice *media);\n> \tvirtual void disconnect();\n> \n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 4cb58084..18952a1b 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -70,6 +70,16 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> * and remains valid until the instance is destroyed.\n> */\n> \n> +/**\n> + * \\var CameraData::waitedRequests_\n> + * \\brief The queue of not yet queued request\n> + *\n> + * The queue of not yet queued request is used to track requests that are not\n> + * queued in order to retry queueing them when a pipeline is ready to accept.\n> + *\n> + * \\sa PipelineHandler::queueRequest(), PipelineHandler::tryQueueRequests().\n> + */\n> +\n> /**\n> * \\var CameraData::queuedRequests_\n> * \\brief The queue of queued and not yet completed request\n> @@ -378,12 +388,44 @@ void PipelineHandler::queueRequest(Request *request)\n> \n> \tCamera *camera = request->camera_;\n> \tCameraData *data = cameraData(camera);\n> -\tdata->queuedRequests_.push_back(request);\n> +\tdata->waitedRequests_.push(request);\n> +\n> +\ttryQueueRequests(data);\n> +}\n> +\n> +/**\n> + * \\fn PipelineHandler::tryQueueRequests()\n> + * \\brief Queue requests that are not yet queued.\n> + * \\param[in] data The camera data whose waited requests are tried to queue.\n> + *\n> + * This tries to queue as many requests as possible in order of the\n> + * waitedRequests_ in data. If queuing fails due to a buffer shortage, this\n> + * method stops and the next call of this method starts from the request that\n> + * fails in the previous call. On any other failures, the request is marked as\n> + * complete and proceed the successive requests.\n> + *\n> + * \\context This function shall be called from the CameraManager thread.\n> + */\n> +void PipelineHandler::tryQueueRequests(CameraData *data)\n> +{\n> +\twhile (!data->waitedRequests_.empty()) {\n> +\t\tRequest *request = data->waitedRequests_.front();\n> +\t\tCamera *camera = request->camera_;\n> +\t\tASSERT(data == cameraData(camera));\n> +\n> +\t\tdata->queuedRequests_.push_back(request);\n> +\t\tint ret = queueRequestDevice(camera, request);\n> +\t\tif (ret == -ENOBUFS || ret == -ENOMEM) {\n> +\t\t\tdata->queuedRequests_.pop_back();\n> +\t\t\tbreak;\n> +\t\t}\n> \n> -\tint ret = queueRequestDevice(camera, request);\n> -\tif (ret) {\n> -\t\trequest->result_ = ret;\n> -\t\tcompleteRequest(request);\n> +\t\tdata->waitedRequests_.pop();\n> +\t\tif (ret) {\n> +\t\t\trequest->result_ = ret;\n> +\t\t\tcompleteRequest(request);\n> +\t\t\tbreak;\n> +\t\t}\n> \t}\n> }\n> \n> @@ -440,6 +482,9 @@ bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)\n> * submission order, the pipeline handler may call it on any complete request\n> * without any ordering constraint.\n> *\n> + * There might be requests that are waiting to be queued, this triggers\n> + * tryQueueRequests().\n> + *\n> * \\context This function shall be called from the CameraManager thread.\n> */\n> void PipelineHandler::completeRequest(Request *request)\n> @@ -459,6 +504,8 @@ void PipelineHandler::completeRequest(Request *request)\n> \t\tdata->queuedRequests_.pop_front();\n> \t\tcamera->requestComplete(req);\n> \t}\n> +\n> +\ttryQueueRequests(data);\n> }\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 4EE20C32EA\n\tfor ;\n\tMon, 29 Mar 2021 05:28:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B28F6877D;\n\tMon, 29 Mar 2021 07:28:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 79F90602D2\n\tfor ;\n\tMon, 29 Mar 2021 07:28:10 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 16C8231A;\n\tMon, 29 Mar 2021 07:28:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pV9XKidX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616995690;\n\tbh=vHVlaW/pL2fmKRFmehO7ZS1TxZPNAVJyyxlYa+fZuLs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pV9XKidXoBI832npVLfUsoXlRg33qfy2kG9ydfKFySRIbI/vtlMTxBs7nC30pTmxf\n\tMYflE2RLYS6Xs9EVOlGmY4TZF1DEJw9C85e1ChRSRfflevpSUmUf7+ibtuGGfqXs9U\n\tGiBSZxv5eQg3PbhR1tiN9W5IL8nxbcRJnfNm000w=","Date":"Mon, 29 Mar 2021 08:27:26 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210329022604.110201-1-hiroh@chromium.org>\n\t<20210329022604.110201-3-hiroh@chromium.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210329022604.110201-3-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 2/2] libcamera: PipelineHandler:\n\tRetry queuing a capture request","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16070,"web_url":"https://patchwork.libcamera.org/comment/16070/","msgid":"","date":"2021-03-31T08:46:57","subject":"Re: [libcamera-devel] [RFC PATCH 2/2] libcamera: PipelineHandler:\n\tRetry queuing a capture request","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Sebastian and Laurent, thanks for reviewing.\n\nOn Mon, Mar 29, 2021 at 2:28 PM Laurent Pinchart\n wrote:\n>\n> Hi Hiro,\n>\n> Thank you for the patch.\n>\n> On Mon, Mar 29, 2021 at 11:26:04AM +0900, Hirokazu Honda wrote:\n> > PipelineHandler::queueRequestDevice() fails due to a buffer\n> > shortage. We should retry queuing a capture request later in the\n> > case.\n>\n> I don't think this should be handled by the base PipelineHandler class,\n> but by individual pipeline handlers. There may be needs for pipeline\n> handlers to peek ahead in the queue of requests to get metadata needed\n> by the algorithms, with a queue depth larger than the number of buffers.\n> You queue those requests on waitedRequests_ so the pipeline handler can\n> have access to them, but the semantics isn't well defined, and I think\n> that will be quite error prone. Furthermore, the call to\n> tryQueueRequests() in PipelineHandler::completeRequest() will result in\n> the pipeline handler's queueRequestDevice() function being called from\n> the request completion handler, making the pipeline handler code\n> reentrant, and pipeline handlers are not prepared for that. This may\n> create race conditions, I'd rather not go that route.\n>\n> This being said, duplicating request queues in all pipeline handlers\n> isn't a great idea either. We should share code, but that should be\n> implemented by helper classes that pipeline handlers can opt-in to use,\n> not in a mandatory middle layer. Now that we have multiple pipeline\n> handler implementations, it's time to start cleaning things up, and the\n> Raspberry Pi and IPU3 pipeline handlers are good candidates for code\n> sharing as they share a common hardware architecture.\n>\n> If you want to fix this issue for the IPU3 pipeline handler without\n> depending on the creation of those helper classes, then the fix should\n> be implemented in the IPU3 pipeline handler itself in the meantime.\n>\n\nThanks for comments.\nMy first idea is to avoid code duplication caused by handling in each\npipeline handler.\nBut your comment makes sense.\nI upload the change for PipelineHandlerIPU3.\nhttps://patchwork.libcamera.org/patch/11804/\n\n-Hiro\n\n\n\n\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > include/libcamera/internal/pipeline_handler.h | 2 +\n> > src/libcamera/pipeline_handler.cpp | 57 +++++++++++++++++--\n> > 2 files changed, 54 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index 763da63e..e6f771a6 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -44,6 +44,7 @@ public:\n> > virtual ~CameraData() = default;\n> >\n> > PipelineHandler *pipe_;\n> > + std::queue waitedRequests_;\n> > std::deque queuedRequests_;\n> > ControlInfoMap controlInfo_;\n> > ControlList properties_;\n> > @@ -99,6 +100,7 @@ protected:\n> > CameraManager *manager_;\n> >\n> > private:\n> > + void tryQueueRequests(CameraData *data);\n> > void mediaDeviceDisconnected(MediaDevice *media);\n> > virtual void disconnect();\n> >\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 4cb58084..18952a1b 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -70,6 +70,16 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> > * and remains valid until the instance is destroyed.\n> > */\n> >\n> > +/**\n> > + * \\var CameraData::waitedRequests_\n> > + * \\brief The queue of not yet queued request\n> > + *\n> > + * The queue of not yet queued request is used to track requests that are not\n> > + * queued in order to retry queueing them when a pipeline is ready to accept.\n> > + *\n> > + * \\sa PipelineHandler::queueRequest(), PipelineHandler::tryQueueRequests().\n> > + */\n> > +\n> > /**\n> > * \\var CameraData::queuedRequests_\n> > * \\brief The queue of queued and not yet completed request\n> > @@ -378,12 +388,44 @@ void PipelineHandler::queueRequest(Request *request)\n> >\n> > Camera *camera = request->camera_;\n> > CameraData *data = cameraData(camera);\n> > - data->queuedRequests_.push_back(request);\n> > + data->waitedRequests_.push(request);\n> > +\n> > + tryQueueRequests(data);\n> > +}\n> > +\n> > +/**\n> > + * \\fn PipelineHandler::tryQueueRequests()\n> > + * \\brief Queue requests that are not yet queued.\n> > + * \\param[in] data The camera data whose waited requests are tried to queue.\n> > + *\n> > + * This tries to queue as many requests as possible in order of the\n> > + * waitedRequests_ in data. If queuing fails due to a buffer shortage, this\n> > + * method stops and the next call of this method starts from the request that\n> > + * fails in the previous call. On any other failures, the request is marked as\n> > + * complete and proceed the successive requests.\n> > + *\n> > + * \\context This function shall be called from the CameraManager thread.\n> > + */\n> > +void PipelineHandler::tryQueueRequests(CameraData *data)\n> > +{\n> > + while (!data->waitedRequests_.empty()) {\n> > + Request *request = data->waitedRequests_.front();\n> > + Camera *camera = request->camera_;\n> > + ASSERT(data == cameraData(camera));\n> > +\n> > + data->queuedRequests_.push_back(request);\n> > + int ret = queueRequestDevice(camera, request);\n> > + if (ret == -ENOBUFS || ret == -ENOMEM) {\n> > + data->queuedRequests_.pop_back();\n> > + break;\n> > + }\n> >\n> > - int ret = queueRequestDevice(camera, request);\n> > - if (ret) {\n> > - request->result_ = ret;\n> > - completeRequest(request);\n> > + data->waitedRequests_.pop();\n> > + if (ret) {\n> > + request->result_ = ret;\n> > + completeRequest(request);\n> > + break;\n> > + }\n> > }\n> > }\n> >\n> > @@ -440,6 +482,9 @@ bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)\n> > * submission order, the pipeline handler may call it on any complete request\n> > * without any ordering constraint.\n> > *\n> > + * There might be requests that are waiting to be queued, this triggers\n> > + * tryQueueRequests().\n> > + *\n> > * \\context This function shall be called from the CameraManager thread.\n> > */\n> > void PipelineHandler::completeRequest(Request *request)\n> > @@ -459,6 +504,8 @@ void PipelineHandler::completeRequest(Request *request)\n> > data->queuedRequests_.pop_front();\n> > camera->requestComplete(req);\n> > }\n> > +\n> > + tryQueueRequests(data);\n> > }\n> >\n> > /**\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 5F056C0DA3\n\tfor ;\n\tWed, 31 Mar 2021 08:47:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F78F68781;\n\tWed, 31 Mar 2021 10:47:09 +0200 (CEST)","from mail-ej1-x635.google.com (mail-ej1-x635.google.com\n\t[IPv6:2a00:1450:4864:20::635])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ECFCC602D7\n\tfor ;\n\tWed, 31 Mar 2021 10:47:07 +0200 (CEST)","by mail-ej1-x635.google.com with SMTP id ap14so15712805ejc.0\n\tfor ;\n\tWed, 31 Mar 2021 01:47:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"Zoh45FW9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=t21S4HI1kpuThpjXmSXZ912eZGgP3ca2ILmZXP8d7fQ=;\n\tb=Zoh45FW9DZS4naJ4ey3gLK+qpSc/5B4EVS7AWbxRXFX0LR85nmey6lp5BOqRmDgkLp\n\typxbG7BDhylRtUHqxz1w0aFqqjQ8aj1l5ZeN5bEDh2Nm5pWGA8Jf9/OZbXLowU40H57V\n\t89OIUdWTEzZ6fitaRZVq4a7r73LJqclz9CW1M=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=t21S4HI1kpuThpjXmSXZ912eZGgP3ca2ILmZXP8d7fQ=;\n\tb=D0foUONRda6u2ibKAW21fs8X4i4ZAk9szydti4KP4Dc/lhRJBZ8KjFuwnLWo2NFxGZ\n\tkAbjqbKkKpp84cyzejoYYwCmnvFvuYteBM7yq0d9+TNytATzbmQQAok539TsLYLLF2Ao\n\to4zZQixQnPfTbk3+48EULo+8seV3A8wFi+t8Le8XE9dwrhWTXtQGkryeLoYjYy78epwF\n\tkAHwo33tPPYst/AhP4r1URufFbUDMqgPT5lz2Y1rvMulQT9yOklHT4YojxdkmsqZYw2h\n\t0zAcPsXwSkj1MRYhA9NOZj6wX4YluwPYDp18zF+cQGr3HDDZJGNQ1xUhci7CajMmnSVY\n\tY06Q==","X-Gm-Message-State":"AOAM530KaVlqduSvrz1qAPwsx2wsZWTSpIe7SwpdOy6L7GcXmWh04HqO\n\tjCEfaqpqr89NW93k+0D9lYKBFGxRu3Savz+M/9OorEQlqij/ZA==","X-Google-Smtp-Source":"ABdhPJwohEvdGolluXxMwgOKPW0BItgwmtTFzGn2k9ZzT8cwQ5n4gXlqkotEgTU4ZHwM4sHKHlHQ3oSoXFInrwsIVTw=","X-Received":"by 2002:a17:906:6a06:: with SMTP id\n\to6mr2325063ejr.306.1617180427557; \n\tWed, 31 Mar 2021 01:47:07 -0700 (PDT)","MIME-Version":"1.0","References":"<20210329022604.110201-1-hiroh@chromium.org>\n\t<20210329022604.110201-3-hiroh@chromium.org>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Wed, 31 Mar 2021 17:46:57 +0900","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [RFC PATCH 2/2] libcamera: PipelineHandler:\n\tRetry queuing a capture request","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]