[{"id":21486,"web_url":"https://patchwork.libcamera.org/comment/21486/","msgid":"","date":"2021-12-01T02:20:35","subject":"Re: [libcamera-devel] [PATCH v3 10/11] libcamera: pipeline_handler:\n\tPrepare Request","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Dec 01, 2021 at 12:36:33AM +0100, Jacopo Mondi wrote:\n> Before queueing a request to the device, any synchronization fence from\n> the Request' framebuffers has to be waited on.\n> \n> Connect the Request::Private::prepared signal to the function that\n> queues requests to the hardware and call Request::Private::prepare().\n> \n> When the waiting request queue is inspected, verify if has completed its\n\ns/if has/if it has/\n\n> preparation phase and queue it to the device.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline_handler.cpp | 48 ++++++++++++++++++++++--------\n> 1 file changed, 36 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 8f1f20e896b0..0a6a2e6ee983 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \"libcamera/internal/pipeline_handler.h\"\n> \n> +#include \n> #include \n> \n> #include \n> @@ -17,6 +18,7 @@\n> #include \n> \n> #include \"libcamera/internal/camera.h\"\n> +#include \"libcamera/internal/framebuffer.h\"\n\nStill no warning about the alphabetical order ?\n\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/request.h\"\n> @@ -36,6 +38,8 @@\n> * the REGISTER_PIPELINE_HANDLER() macro.\n> */\n> \n> +using namespace std::chrono_literals;\n> +\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(Pipeline)\n> @@ -323,10 +327,16 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const\n> * \\param[in] request The request to queue\n> *\n> * This function queues a capture request to the pipeline handler for\n> - * processing. The request is first added to the internal list of queued\n> - * requests, and then passed to the pipeline handler with a call to\n> - * queueRequestDevice(). If the pipeline handler fails in queuing the request\n> - * to the hardware the request is cancelled.\n> + * processing. The request is first added to the internal list of waiting\n> + * requests which have to be prepared to make sure they are ready for being\n> + * queued to the pipeline handler.\n> + *\n> + * The queue of waiting requests is iterated and all prepared requests are\n> + * passed to the pipeline handler in the same order they have been queued by\n> + * calling this function.\n> + *\n> + * If a Request fails during the preparation phase or if the pipeline handler\n> + * fails in queuing the request to the hardware the request is cancelled.\n> *\n> * Keeping track of queued requests ensures automatic completion of all requests\n> * when the pipeline handler is stopped with stop(). Request completion shall be\n> @@ -339,11 +349,20 @@ void PipelineHandler::queueRequest(Request *request)\n> \tLIBCAMERA_TRACEPOINT(request_queue, request);\n> \n> \twaitingRequests_.push(request);\n> -\tdoQueueRequests();\n> +\n> +\trequest->_d()->prepared.connect(this, [this]() {\n> +\t\t\t\t\t\tdoQueueRequests();\n> +\t\t\t\t\t});\n> +\trequest->_d()->prepare(300ms);\n> }\n> \n> /**\n> * \\brief Queue one requests to the device\n> + *\n> + * If a Request has failed during the preparation phase it is cancelled.\n> + *\n> + * If queuing a Request to the pipeline handler fails, the Request is cancelled\n> + * as well.\n> */\n> void PipelineHandler::doQueueRequest(Request *request)\n> {\n> @@ -355,6 +374,12 @@ void PipelineHandler::doQueueRequest(Request *request)\n> \n> \trequest->_d()->sequence_ = data->requestSequence_++;\n> \n> +\tif (request->_d()->cancelled_) {\n> +\t\trequest->_d()->cancel();\n\nNot that it's incorrect, but this looks really weird and calls for some\nrefactoring on top of this series. Could you add a \\todo comment ?\n\n> +\t\tcompleteRequest(request);\n> +\t\treturn;\n> +\t}\n> +\n> \tint ret = queueRequestDevice(camera, request);\n> \tif (ret) {\n> \t\trequest->_d()->cancel();\n> @@ -365,19 +390,18 @@ void PipelineHandler::doQueueRequest(Request *request)\n> /**\n> * \\brief Queue requests to the device\n\nMaybe\n\n * \\brief Queue prepared requests to the device\n\n?\n\nReviewed-by: Laurent Pinchart \n\n> *\n> - * Iterate the lst of waiting requests and queue them to the hardware one\n> - * by one.\n> + * Iterate the queue of waiting requests and queue them in order the hardware if\n> + * they have completed their preparation phase.\n> */\n> void PipelineHandler::doQueueRequests()\n> {\n> -\twhile (true) {\n> -\t\tif (waitingRequests_.empty())\n> -\t\t\treturn;\n> -\n> +\twhile (!waitingRequests_.empty()) {\n> \t\tRequest *request = waitingRequests_.front();\n> -\t\twaitingRequests_.pop();\n> +\t\tif (!request->_d()->prepared_)\n> +\t\t\tbreak;\n> \n> \t\tdoQueueRequest(request);\n> +\t\twaitingRequests_.pop();\n> \t}\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 74CB1BF415\n\tfor ;\n\tWed, 1 Dec 2021 02:21:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BE14660720;\n\tWed, 1 Dec 2021 03:21:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 511E060592\n\tfor ;\n\tWed, 1 Dec 2021 03:21:00 +0100 (CET)","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 BC6BE8AE;\n\tWed, 1 Dec 2021 03:20: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=\"blw3rMPG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638325260;\n\tbh=/iDSpjl1/lvEkfOwaLna3grosfGTU0mmJ2H7ChNrei4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=blw3rMPGk4EenKgC877P1i91MIVYXTaEA57Vf6RYYAmUjOqoxf4GqtnWyAwQbZfe+\n\tR3Jl1WYk8SjTFatumju3p+y+b53fBOWpGWY5qMcmHyaFECoUQ7X4BbxvSuS5Y5jRll\n\t2TUg6jKMVD+m58wHyDEPJcVjiMMCAZkW9ZlsiLDU=","Date":"Wed, 1 Dec 2021 04:20:35 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20211130233634.34173-1-jacopo@jmondi.org>\n\t<20211130233634.34173-11-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211130233634.34173-11-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 10/11] libcamera: pipeline_handler:\n\tPrepare 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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21505,"web_url":"https://patchwork.libcamera.org/comment/21505/","msgid":"<20211201103524.to4faqvedvpfslmg@uno.localdomain>","date":"2021-12-01T10:35:24","subject":"Re: [libcamera-devel] [PATCH v3 10/11] libcamera: pipeline_handler:\n\tPrepare Request","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Dec 01, 2021 at 04:20:35AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Dec 01, 2021 at 12:36:33AM +0100, Jacopo Mondi wrote:\n> > Before queueing a request to the device, any synchronization fence from\n> > the Request' framebuffers has to be waited on.\n> >\n> > Connect the Request::Private::prepared signal to the function that\n> > queues requests to the hardware and call Request::Private::prepare().\n> >\n> > When the waiting request queue is inspected, verify if has completed its\n>\n> s/if has/if it has/\n>\n> > preparation phase and queue it to the device.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline_handler.cpp | 48 ++++++++++++++++++++++--------\n> > 1 file changed, 36 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 8f1f20e896b0..0a6a2e6ee983 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -7,6 +7,7 @@\n> >\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> >\n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -17,6 +18,7 @@\n> > #include \n> >\n> > #include \"libcamera/internal/camera.h\"\n> > +#include \"libcamera/internal/framebuffer.h\"\n>\n> Still no warning about the alphabetical order ?\n>\n\nouch no.\n\nIt's also weird that I get this on a previous patch\n\n--------------------------------------------------------------------------------------------\n3d2e23d0730b257931bc101aafac08c15f0789e6 libcamera: pipeline_handler: Split request queueing\n--------------------------------------------------------------------------------------------\n--- include/libcamera/internal/pipeline_handler.h\n+++ include/libcamera/internal/pipeline_handler.h\n@@ -8,7 +8,6 @@\n #pragma once\n\n #include \n-#include \n #include \n #include \n #include \n---\n1 potential issue detected, please review\n\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > #include \"libcamera/internal/media_device.h\"\n> > #include \"libcamera/internal/request.h\"\n> > @@ -36,6 +38,8 @@\n> > * the REGISTER_PIPELINE_HANDLER() macro.\n> > */\n> >\n> > +using namespace std::chrono_literals;\n> > +\n> > namespace libcamera {\n> >\n> > LOG_DEFINE_CATEGORY(Pipeline)\n> > @@ -323,10 +327,16 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const\n> > * \\param[in] request The request to queue\n> > *\n> > * This function queues a capture request to the pipeline handler for\n> > - * processing. The request is first added to the internal list of queued\n> > - * requests, and then passed to the pipeline handler with a call to\n> > - * queueRequestDevice(). If the pipeline handler fails in queuing the request\n> > - * to the hardware the request is cancelled.\n> > + * processing. The request is first added to the internal list of waiting\n> > + * requests which have to be prepared to make sure they are ready for being\n> > + * queued to the pipeline handler.\n> > + *\n> > + * The queue of waiting requests is iterated and all prepared requests are\n> > + * passed to the pipeline handler in the same order they have been queued by\n> > + * calling this function.\n> > + *\n> > + * If a Request fails during the preparation phase or if the pipeline handler\n> > + * fails in queuing the request to the hardware the request is cancelled.\n> > *\n> > * Keeping track of queued requests ensures automatic completion of all requests\n> > * when the pipeline handler is stopped with stop(). Request completion shall be\n> > @@ -339,11 +349,20 @@ void PipelineHandler::queueRequest(Request *request)\n> > \tLIBCAMERA_TRACEPOINT(request_queue, request);\n> >\n> > \twaitingRequests_.push(request);\n> > -\tdoQueueRequests();\n> > +\n> > +\trequest->_d()->prepared.connect(this, [this]() {\n> > +\t\t\t\t\t\tdoQueueRequests();\n> > +\t\t\t\t\t});\n> > +\trequest->_d()->prepare(300ms);\n> > }\n> >\n> > /**\n> > * \\brief Queue one requests to the device\n> > + *\n> > + * If a Request has failed during the preparation phase it is cancelled.\n> > + *\n> > + * If queuing a Request to the pipeline handler fails, the Request is cancelled\n> > + * as well.\n> > */\n> > void PipelineHandler::doQueueRequest(Request *request)\n> > {\n> > @@ -355,6 +374,12 @@ void PipelineHandler::doQueueRequest(Request *request)\n> >\n> > \trequest->_d()->sequence_ = data->requestSequence_++;\n> >\n> > +\tif (request->_d()->cancelled_) {\n> > +\t\trequest->_d()->cancel();\n>\n> Not that it's incorrect, but this looks really weird and calls for some\n> refactoring on top of this series. Could you add a \\todo comment ?\n\nYes, it feels weird. But either I add new flags specific to the\nprepare phase result or I do this.\n\nI thought I can't call Request::Private::cancel() directly from the timeout\nhandler as it would complete the request out of order, but it actually\nonly notifies buffers completion not the request. I can try looking\ninto that.\n\n>\n> > +\t\tcompleteRequest(request);\n> > +\t\treturn;\n> > +\t}\n> > +\n> > \tint ret = queueRequestDevice(camera, request);\n> > \tif (ret) {\n> > \t\trequest->_d()->cancel();\n> > @@ -365,19 +390,18 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > /**\n> > * \\brief Queue requests to the device\n>\n> Maybe\n>\n> * \\brief Queue prepared requests to the device\n>\n> ?\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > *\n> > - * Iterate the lst of waiting requests and queue them to the hardware one\n> > - * by one.\n> > + * Iterate the queue of waiting requests and queue them in order the hardware if\n> > + * they have completed their preparation phase.\n> > */\n> > void PipelineHandler::doQueueRequests()\n> > {\n> > -\twhile (true) {\n> > -\t\tif (waitingRequests_.empty())\n> > -\t\t\treturn;\n> > -\n> > +\twhile (!waitingRequests_.empty()) {\n> > \t\tRequest *request = waitingRequests_.front();\n> > -\t\twaitingRequests_.pop();\n> > +\t\tif (!request->_d()->prepared_)\n> > +\t\t\tbreak;\n> >\n> > \t\tdoQueueRequest(request);\n> > +\t\twaitingRequests_.pop();\n> > \t}\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 C0509BDB13\n\tfor ;\n\tWed, 1 Dec 2021 10:34:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 218F360718;\n\tWed, 1 Dec 2021 11:34:34 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AEF0A6011D\n\tfor ;\n\tWed, 1 Dec 2021 11:34:32 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id C47D7C0016;\n\tWed, 1 Dec 2021 10:34:31 +0000 (UTC)"],"Date":"Wed, 1 Dec 2021 11:35:24 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20211201103524.to4faqvedvpfslmg@uno.localdomain>","References":"<20211130233634.34173-1-jacopo@jmondi.org>\n\t<20211130233634.34173-11-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 10/11] libcamera: pipeline_handler:\n\tPrepare 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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21517,"web_url":"https://patchwork.libcamera.org/comment/21517/","msgid":"","date":"2021-12-01T11:58:03","subject":"Re: [libcamera-devel] [PATCH v3 10/11] libcamera: pipeline_handler:\n\tPrepare Request","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Dec 01, 2021 at 11:35:24AM +0100, Jacopo Mondi wrote:\n> On Wed, Dec 01, 2021 at 04:20:35AM +0200, Laurent Pinchart wrote:\n> > On Wed, Dec 01, 2021 at 12:36:33AM +0100, Jacopo Mondi wrote:\n> > > Before queueing a request to the device, any synchronization fence from\n> > > the Request' framebuffers has to be waited on.\n> > >\n> > > Connect the Request::Private::prepared signal to the function that\n> > > queues requests to the hardware and call Request::Private::prepare().\n> > >\n> > > When the waiting request queue is inspected, verify if has completed its\n> >\n> > s/if has/if it has/\n> >\n> > > preparation phase and queue it to the device.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/pipeline_handler.cpp | 48 ++++++++++++++++++++++--------\n> > > 1 file changed, 36 insertions(+), 12 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index 8f1f20e896b0..0a6a2e6ee983 100644\n> > > --- a/src/libcamera/pipeline_handler.cpp\n> > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > @@ -7,6 +7,7 @@\n> > >\n> > > #include \"libcamera/internal/pipeline_handler.h\"\n> > >\n> > > +#include \n> > > #include \n> > >\n> > > #include \n> > > @@ -17,6 +18,7 @@\n> > > #include \n> > >\n> > > #include \"libcamera/internal/camera.h\"\n> > > +#include \"libcamera/internal/framebuffer.h\"\n> >\n> > Still no warning about the alphabetical order ?\n> \n> ouch no.\n> \n> It's also weird that I get this on a previous patch\n> \n> --------------------------------------------------------------------------------------------\n> 3d2e23d0730b257931bc101aafac08c15f0789e6 libcamera: pipeline_handler: Split request queueing\n> --------------------------------------------------------------------------------------------\n> --- include/libcamera/internal/pipeline_handler.h\n> +++ include/libcamera/internal/pipeline_handler.h\n> @@ -8,7 +8,6 @@\n> #pragma once\n> \n> #include \n> -#include \n> #include \n> #include \n> #include \n> ---\n> 1 potential issue detected, please review\n\nThere's probably a few things to fix in checkstyle.py :-S\n\n> > > #include \"libcamera/internal/device_enumerator.h\"\n> > > #include \"libcamera/internal/media_device.h\"\n> > > #include \"libcamera/internal/request.h\"\n> > > @@ -36,6 +38,8 @@\n> > > * the REGISTER_PIPELINE_HANDLER() macro.\n> > > */\n> > >\n> > > +using namespace std::chrono_literals;\n> > > +\n> > > namespace libcamera {\n> > >\n> > > LOG_DEFINE_CATEGORY(Pipeline)\n> > > @@ -323,10 +327,16 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const\n> > > * \\param[in] request The request to queue\n> > > *\n> > > * This function queues a capture request to the pipeline handler for\n> > > - * processing. The request is first added to the internal list of queued\n> > > - * requests, and then passed to the pipeline handler with a call to\n> > > - * queueRequestDevice(). If the pipeline handler fails in queuing the request\n> > > - * to the hardware the request is cancelled.\n> > > + * processing. The request is first added to the internal list of waiting\n> > > + * requests which have to be prepared to make sure they are ready for being\n> > > + * queued to the pipeline handler.\n> > > + *\n> > > + * The queue of waiting requests is iterated and all prepared requests are\n> > > + * passed to the pipeline handler in the same order they have been queued by\n> > > + * calling this function.\n> > > + *\n> > > + * If a Request fails during the preparation phase or if the pipeline handler\n> > > + * fails in queuing the request to the hardware the request is cancelled.\n> > > *\n> > > * Keeping track of queued requests ensures automatic completion of all requests\n> > > * when the pipeline handler is stopped with stop(). Request completion shall be\n> > > @@ -339,11 +349,20 @@ void PipelineHandler::queueRequest(Request *request)\n> > > \tLIBCAMERA_TRACEPOINT(request_queue, request);\n> > >\n> > > \twaitingRequests_.push(request);\n> > > -\tdoQueueRequests();\n> > > +\n> > > +\trequest->_d()->prepared.connect(this, [this]() {\n> > > +\t\t\t\t\t\tdoQueueRequests();\n> > > +\t\t\t\t\t});\n> > > +\trequest->_d()->prepare(300ms);\n> > > }\n> > >\n> > > /**\n> > > * \\brief Queue one requests to the device\n> > > + *\n> > > + * If a Request has failed during the preparation phase it is cancelled.\n> > > + *\n> > > + * If queuing a Request to the pipeline handler fails, the Request is cancelled\n> > > + * as well.\n> > > */\n> > > void PipelineHandler::doQueueRequest(Request *request)\n> > > {\n> > > @@ -355,6 +374,12 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > >\n> > > \trequest->_d()->sequence_ = data->requestSequence_++;\n> > >\n> > > +\tif (request->_d()->cancelled_) {\n> > > +\t\trequest->_d()->cancel();\n> >\n> > Not that it's incorrect, but this looks really weird and calls for some\n> > refactoring on top of this series. Could you add a \\todo comment ?\n> \n> Yes, it feels weird. But either I add new flags specific to the\n> prepare phase result or I do this.\n> \n> I thought I can't call Request::Private::cancel() directly from the timeout\n> handler as it would complete the request out of order, but it actually\n> only notifies buffers completion not the request. I can try looking\n> into that.\n\nThe whole cancellation API is a bit dirty in my opinion. We can address\nit on top of this.\n\n> > > +\t\tcompleteRequest(request);\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > \tint ret = queueRequestDevice(camera, request);\n> > > \tif (ret) {\n> > > \t\trequest->_d()->cancel();\n> > > @@ -365,19 +390,18 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > > /**\n> > > * \\brief Queue requests to the device\n> >\n> > Maybe\n> >\n> > * \\brief Queue prepared requests to the device\n> >\n> > ?\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > *\n> > > - * Iterate the lst of waiting requests and queue them to the hardware one\n> > > - * by one.\n> > > + * Iterate the queue of waiting requests and queue them in order the hardware if\n> > > + * they have completed their preparation phase.\n> > > */\n> > > void PipelineHandler::doQueueRequests()\n> > > {\n> > > -\twhile (true) {\n> > > -\t\tif (waitingRequests_.empty())\n> > > -\t\t\treturn;\n> > > -\n> > > +\twhile (!waitingRequests_.empty()) {\n> > > \t\tRequest *request = waitingRequests_.front();\n> > > -\t\twaitingRequests_.pop();\n> > > +\t\tif (!request->_d()->prepared_)\n> > > +\t\t\tbreak;\n> > >\n> > > \t\tdoQueueRequest(request);\n> > > +\t\twaitingRequests_.pop();\n> > > \t}\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 AA311BDB13\n\tfor ;\n\tWed, 1 Dec 2021 11:58:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 668CE604FC;\n\tWed, 1 Dec 2021 12:58:30 +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 A86AF6011A\n\tfor ;\n\tWed, 1 Dec 2021 12:58:28 +0100 (CET)","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 3D0D5A15;\n\tWed, 1 Dec 2021 12:58:28 +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=\"n3eaU/Ad\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638359908;\n\tbh=3d72o6UnI5+zCeB9INmHzzLFDIGSwJ3461nGfECz4gs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n3eaU/AdavLKr3/Vvn0t91QBEjxUAXLfp1qN1s6VqY9ZqMKK80yrcuvCeqjtF3iKT\n\totmeezmiahTL76I+6h1oxTZ+qhDGMSza1f9bU37yYjcB6ymzFuFwfyYVzC4WWhr5cR\n\tOtmOBWXQ9b7DhE47YejPZm7t7Wv1e164kq/n2DI0=","Date":"Wed, 1 Dec 2021 13:58:03 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20211130233634.34173-1-jacopo@jmondi.org>\n\t<20211130233634.34173-11-jacopo@jmondi.org>\n\t\n\t<20211201103524.to4faqvedvpfslmg@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211201103524.to4faqvedvpfslmg@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 10/11] libcamera: pipeline_handler:\n\tPrepare 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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]