[{"id":34789,"web_url":"https://patchwork.libcamera.org/comment/34789/","msgid":"<175187690874.2875288.12758652832457503660@ping.linuxembedded.co.uk>","date":"2025-07-07T08:28:28","subject":"Re: [PATCH v2 2/5] libcamera: pipeline_handler: Allow to limit the\n\tnumber of queued requests","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-07-07 08:53:36)\n> Add a maxQueuedRequestsDevice constructor parameter to allow pipeline\n> handler classes to limit the maximum number of requests that get queued\n> to the device in queueRequestDevice().\n> \n> The default value is set to an arbitrary number of 32 which is big\n> enough for all currently known use cases.\n> \n> Signed-off-by: Stefan Klug \n> Signed-off-by: Kieran Bingham \n> Signed-off-by: Stefan Klug \n> \n> ---\n> \n> Changes in v2:\n> - Moved the fix to properly handle the waitingRequests queue when\n> stopping the device into this patch as they belong together\n> - Added documentation for maxQueuedRequestsDevice_\n> - Fixed an issue with doQueueRequests beeing() called recursively\n> \n> Changes in v1:\n> - Used a const member variable to carry the maximum number of requests\n> - Improved commit message\n> - Added docs\n> ---\n> include/libcamera/internal/pipeline_handler.h | 4 +-\n> src/libcamera/pipeline_handler.cpp | 56 +++++++++++++++----\n> 2 files changed, 48 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index be017ad47219..e89d6a33e398 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -33,7 +33,8 @@ class PipelineHandler : public std::enable_shared_from_this,\n> public Object\n> {\n> public:\n> - PipelineHandler(CameraManager *manager);\n> + PipelineHandler(CameraManager *manager,\n> + unsigned int maxQueuedRequestsDevice = 32);\n> virtual ~PipelineHandler();\n> \n> virtual bool match(DeviceEnumerator *enumerator) = 0;\n> @@ -80,6 +81,7 @@ protected:\n> virtual void releaseDevice(Camera *camera);\n> \n> CameraManager *manager_;\n> + const unsigned int maxQueuedRequestsDevice_;\n> \n> private:\n> void unlockMediaDevices();\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index b50eda5e0f86..e8601be108f0 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -62,13 +62,17 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> /**\n> * \\brief Construct a PipelineHandler instance\n> * \\param[in] manager The camera manager\n> + * \\param[in] maxQueuedRequestsDevice The maximum number of requests queued to\n> + * the device\n> *\n> * In order to honour the std::enable_shared_from_this<> contract,\n> * PipelineHandler instances shall never be constructed manually, but always\n> * through the PipelineHandlerFactoryBase::create() function.\n> */\n> -PipelineHandler::PipelineHandler(CameraManager *manager)\n> - : manager_(manager), useCount_(0)\n> +PipelineHandler::PipelineHandler(CameraManager *manager,\n> + unsigned int maxQueuedRequestsDevice)\n> + : manager_(manager), maxQueuedRequestsDevice_(maxQueuedRequestsDevice),\n> + useCount_(0)\n> {\n> }\n> \n> @@ -360,20 +364,28 @@ void PipelineHandler::unlockMediaDevices()\n> */\n> void PipelineHandler::stop(Camera *camera)\n> {\n> + /*\n> + * Take all waiting requests so that they are not requeued in response\n> + * to completeRequest() being called inside stopDevice(). Cancel them\n> + * after the device to keep them in order.\n> + */\n> + Camera::Private *data = camera->_d();\n> + std::queue waitingRequests;\n> + waitingRequests.swap(data->waitingRequests_);\n\nYes, this looks good - solves the issue and maintains the order.\n\nReviewed-by: Kieran Bingham \n\n> +\n> /* Stop the pipeline handler and let the queued requests complete. */\n> stopDevice(camera);\n> \n> - Camera::Private *data = camera->_d();\n> -\n> /* Cancel and signal as complete all waiting requests. */\n> - while (!data->waitingRequests_.empty()) {\n> - Request *request = data->waitingRequests_.front();\n> - data->waitingRequests_.pop();\n> + while (!waitingRequests.empty()) {\n> + Request *request = waitingRequests.front();\n> + waitingRequests.pop();\n> cancelRequest(request);\n> }\n> \n> /* Make sure no requests are pending. */\n> ASSERT(data->queuedRequests_.empty());\n> + ASSERT(data->waitingRequests_.empty());\n> \n> data->requestSequence_ = 0;\n> }\n> @@ -430,9 +442,9 @@ void PipelineHandler::registerRequest(Request *request)\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> + * The queue of waiting requests is iterated and up to \\a\n> + * maxQueuedRequestsDevice_ prepared requests are passed to the pipeline handler\n> + * in the same order they have been queued by 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> @@ -487,12 +499,19 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> {\n> Camera::Private *data = camera->_d();\n> while (!data->waitingRequests_.empty()) {\n> + if (data->queuedRequests_.size() == maxQueuedRequestsDevice_)\n> + break;\n> +\n> Request *request = data->waitingRequests_.front();\n> if (!request->_d()->prepared_)\n> break;\n> \n> - doQueueRequest(request);\n> + /*\n> + * Pop the request first, in case doQueueRequests() is called\n> + * recursively from within doQueueRequest()\n> + */\n> data->waitingRequests_.pop();\n> + doQueueRequest(request);\n> }\n> }\n> \n> @@ -568,6 +587,9 @@ void PipelineHandler::completeRequest(Request *request)\n> data->queuedRequests_.pop_front();\n> camera->requestComplete(req);\n> }\n> +\n> + /* Allow any waiting requests to be queued to the pipeline. */\n> + doQueueRequests(camera);\n> }\n> \n> /**\n> @@ -768,6 +790,18 @@ void PipelineHandler::disconnect()\n> * constant for the whole lifetime of the pipeline handler.\n> */\n> \n> +/**\n> + * \\var PipelineHandler::maxQueuedRequestsDevice_\n> + * \\brief The maximum number of requests the pipeline handler shall queue to the\n> + * device\n> + *\n> + * Some pipeline handlers do not work efficiently with too many requests queued\n> + * into the device. Still it is beneficial for the application to circulate more\n> + * than the minimum number of buffers. This variable limits the number of\n> + * requests that get queued to the device at once. The rest is kept in a\n> + * a waiting queue.\n> + */\n> +\n> /**\n> * \\fn PipelineHandler::name()\n> * \\brief Retrieve the pipeline handler name\n> -- \n> 2.48.1\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 5BE35C0DA4\n\tfor ;\n\tMon, 7 Jul 2025 08:28:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F2BA268E87;\n\tMon, 7 Jul 2025 10:28:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA26B68E66\n\tfor ;\n\tMon, 7 Jul 2025 10:28:31 +0200 (CEST)","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 0E51F1026;\n\tMon, 7 Jul 2025 10:28:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HTB2PuGO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751876885;\n\tbh=NSD4tkC8ZxaNm4p+BZlbzrvXTbYb4uAu0gu1uuNGSIc=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=HTB2PuGObc/PriwU+x4GDuDobfO1M+oTlEOerIv4zIDW8/cdI0b+Cd6HMI4lxbt99\n\twbpgEaZzCa74UExYKEs107xvZP5owJV8wZVnDjQ+ubYI0lGcLuRB68vk6jJIE9zj2G\n\tcxUg8nHr6Vn+1yo6NSQhenWjbiTNKUQbX8ZOqBRA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250707075400.9079-3-stefan.klug@ideasonboard.com>","References":"<20250707075400.9079-1-stefan.klug@ideasonboard.com>\n\t<20250707075400.9079-3-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v2 2/5] libcamera: pipeline_handler: Allow to limit the\n\tnumber of queued requests","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 07 Jul 2025 09:28:28 +0100","Message-ID":"<175187690874.2875288.12758652832457503660@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":34816,"web_url":"https://patchwork.libcamera.org/comment/34816/","msgid":"","date":"2025-07-08T03:59:07","subject":"Re: [PATCH v2 2/5] libcamera: pipeline_handler: Allow to limit the\n\tnumber of queued requests","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Stefan\n\nOn Mon, Jul 07, 2025 at 09:53:36AM +0200, Stefan Klug wrote:\n> Add a maxQueuedRequestsDevice constructor parameter to allow pipeline\n> handler classes to limit the maximum number of requests that get queued\n> to the device in queueRequestDevice().\n> \n> The default value is set to an arbitrary number of 32 which is big\n> enough for all currently known use cases.\n> \n> Signed-off-by: Stefan Klug \n> Signed-off-by: Kieran Bingham \n> Signed-off-by: Stefan Klug \n> \n\nDouble S-o-B\n\n> ---\n> \n> Changes in v2:\n> - Moved the fix to properly handle the waitingRequests queue when\n> stopping the device into this patch as they belong together\n> - Added documentation for maxQueuedRequestsDevice_\n> - Fixed an issue with doQueueRequests beeing() called recursively\n> \n> Changes in v1:\n> - Used a const member variable to carry the maximum number of requests\n> - Improved commit message\n> - Added docs\n> ---\n> include/libcamera/internal/pipeline_handler.h | 4 +-\n> src/libcamera/pipeline_handler.cpp | 56 +++++++++++++++----\n> 2 files changed, 48 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index be017ad47219..e89d6a33e398 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -33,7 +33,8 @@ class PipelineHandler : public std::enable_shared_from_this,\n> \t\t\tpublic Object\n> {\n> public:\n> -\tPipelineHandler(CameraManager *manager);\n> +\tPipelineHandler(CameraManager *manager,\n> +\t\t\tunsigned int maxQueuedRequestsDevice = 32);\n> \tvirtual ~PipelineHandler();\n> \n> \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> @@ -80,6 +81,7 @@ protected:\n> \tvirtual void releaseDevice(Camera *camera);\n> \n> \tCameraManager *manager_;\n> +\tconst unsigned int maxQueuedRequestsDevice_;\n> \n> private:\n> \tvoid unlockMediaDevices();\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index b50eda5e0f86..e8601be108f0 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -62,13 +62,17 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> /**\n> * \\brief Construct a PipelineHandler instance\n> * \\param[in] manager The camera manager\n> + * \\param[in] maxQueuedRequestsDevice The maximum number of requests queued to\n> + * the device\n> *\n> * In order to honour the std::enable_shared_from_this<> contract,\n> * PipelineHandler instances shall never be constructed manually, but always\n> * through the PipelineHandlerFactoryBase::create() function.\n> */\n> -PipelineHandler::PipelineHandler(CameraManager *manager)\n> -\t: manager_(manager), useCount_(0)\n> +PipelineHandler::PipelineHandler(CameraManager *manager,\n> +\t\t\t\t unsigned int maxQueuedRequestsDevice)\n> +\t: manager_(manager), maxQueuedRequestsDevice_(maxQueuedRequestsDevice),\n> +\t useCount_(0)\n> {\n> }\n> \n> @@ -360,20 +364,28 @@ void PipelineHandler::unlockMediaDevices()\n> */\n> void PipelineHandler::stop(Camera *camera)\n> {\n> +\t/*\n> +\t * Take all waiting requests so that they are not requeued in response\n> +\t * to completeRequest() being called inside stopDevice(). Cancel them\n> +\t * after the device to keep them in order.\n> +\t */\n> +\tCamera::Private *data = camera->_d();\n> +\tstd::queue waitingRequests;\n> +\twaitingRequests.swap(data->waitingRequests_);\n> +\n> \t/* Stop the pipeline handler and let the queued requests complete. */\n> \tstopDevice(camera);\n> \n> -\tCamera::Private *data = camera->_d();\n> -\n> \t/* Cancel and signal as complete all waiting requests. */\n> -\twhile (!data->waitingRequests_.empty()) {\n> -\t\tRequest *request = data->waitingRequests_.front();\n> -\t\tdata->waitingRequests_.pop();\n> +\twhile (!waitingRequests.empty()) {\n> +\t\tRequest *request = waitingRequests.front();\n> +\t\twaitingRequests.pop();\n> \t\tcancelRequest(request);\n> \t}\n> \n> \t/* Make sure no requests are pending. */\n> \tASSERT(data->queuedRequests_.empty());\n> +\tASSERT(data->waitingRequests_.empty());\n> \n> \tdata->requestSequence_ = 0;\n> }\n> @@ -430,9 +442,9 @@ void PipelineHandler::registerRequest(Request *request)\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> + * The queue of waiting requests is iterated and up to \\a\n> + * maxQueuedRequestsDevice_ prepared requests are passed to the pipeline handler\n> + * in the same order they have been queued by 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> @@ -487,12 +499,19 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> {\n> \tCamera::Private *data = camera->_d();\n> \twhile (!data->waitingRequests_.empty()) {\n> +\t\tif (data->queuedRequests_.size() == maxQueuedRequestsDevice_)\n> +\t\t\tbreak;\n> +\n> \t\tRequest *request = data->waitingRequests_.front();\n> \t\tif (!request->_d()->prepared_)\n> \t\t\tbreak;\n> \n> -\t\tdoQueueRequest(request);\n> +\t\t/*\n> +\t\t * Pop the request first, in case doQueueRequests() is called\n> +\t\t * recursively from within doQueueRequest()\n> +\t\t */\n\nI think what you meant here is, doQueueRequests is called within\ncompleteBuffer().\n\n> \t\tdata->waitingRequests_.pop();\n> +\t\tdoQueueRequest(request);\n> \t}\n> }\n> \n> @@ -568,6 +587,9 @@ void PipelineHandler::completeRequest(Request *request)\n> \t\tdata->queuedRequests_.pop_front();\n> \t\tcamera->requestComplete(req);\n> \t}\n> +\n> +\t/* Allow any waiting requests to be queued to the pipeline. */\n> +\tdoQueueRequests(camera);\n> }\n> \n> /**\n> @@ -768,6 +790,18 @@ void PipelineHandler::disconnect()\n> * constant for the whole lifetime of the pipeline handler.\n> */\n> \n> +/**\n> + * \\var PipelineHandler::maxQueuedRequestsDevice_\n> + * \\brief The maximum number of requests the pipeline handler shall queue to the\n> + * device\n> + *\n> + * Some pipeline handlers do not work efficiently with too many requests queued\n> + * into the device. Still it is beneficial for the application to circulate more\n> + * than the minimum number of buffers. This variable limits the number of\n> + * requests that get queued to the device at once. The rest is kept in a\n> + * a waiting queue.\n> + */\n> +\n\n/**\n * \\var PipelineHandler::maxQueuedRequestsDevice_\n * \\brief The maximum number of requests the pipeline handler shall queue to the\n * device\n *\n * maxQueuedRequestsDevice_ limits the number of request that the\n * pipeline handler shall queue to the underlying hardware, in order to\n * saturate the pipeline with requests. The application may choose to queue\n * as many requests as it desires, however only maxQueuedRequestsDevice_\n * requests will be queued to the hardware at a given point in time. The\n * remaining requests will be kept waiting in the internal waiting\n * queue, to be queued at a later stage.\n */\n\nReviewed-by: Umang Jain \n\n\n> /**\n> * \\fn PipelineHandler::name()\n> * \\brief Retrieve the pipeline handler name\n> -- \n> 2.48.1\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 27F07C3237\n\tfor ;\n\tTue, 8 Jul 2025 03:59:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 41E1568ECD;\n\tTue, 8 Jul 2025 05:59:10 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 93AC36186C\n\tfor ;\n\tTue, 8 Jul 2025 05:59:06 +0200 (CEST)","from [49.36.71.127] (helo=uajain)\n\tby fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1uYzU1-00Dq1k-6b; Tue, 08 Jul 2025 05:59:05 +0200"],"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=\"pY5Gx6ek\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:\n\tSubject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding: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=ple4WR63UEmnDJmiKXgQ+ahjpDtKO2yIhpMOZCKFdOU=;\n\tb=pY5Gx6ekXHrIyA6EceIBDf+9ys\n\tyGBfOGPHTTC7axRIWgCnFWvXqFAv43itqUXhKcN62Y02D9/TliwmGVFMvWbxENFh1RhmAjq2dQKt6\n\t4LTIA9S97tAva6kxicL2DkKFq2gChqjm93QPyrMoPh/A+0HYEizC+HfJqPWK+ylZQNsYwvm3XWCfe\n\tBMAvQFbGPoe+13viGlq429VWY0FYDGEAiwUWCEC161MAn6GTQob8VFCheCeVvQ+9za1z1S25kmagG\n\tr2DLRV9oFHCc6zz23/jbd3kSxN8uKRMjmUC/BYCv/WQrqznvUW3+jDZlGZOE0Y3ik8Iup/c8FPmno\n\tv7NFnNAA==;","Date":"Tue, 8 Jul 2025 09:29:07 +0530","From":"Umang Jain ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org, \n\tKieran Bingham ","Subject":"Re: [PATCH v2 2/5] libcamera: pipeline_handler: Allow to limit the\n\tnumber of queued requests","Message-ID":"","References":"<20250707075400.9079-1-stefan.klug@ideasonboard.com>\n\t<20250707075400.9079-3-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250707075400.9079-3-stefan.klug@ideasonboard.com>","User-Agent":"NeoMutt/20250510-dirty","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":34915,"web_url":"https://patchwork.libcamera.org/comment/34915/","msgid":"<175274657978.2809694.692482683663363458@localhost>","date":"2025-07-17T10:02:59","subject":"Re: [PATCH v2 2/5] libcamera: pipeline_handler: Allow to limit the\n\tnumber of queued requests","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the review.\n\nQuoting Umang Jain (2025-07-08 05:59:07)\n> Hi Stefan\n> \n> On Mon, Jul 07, 2025 at 09:53:36AM +0200, Stefan Klug wrote:\n> > Add a maxQueuedRequestsDevice constructor parameter to allow pipeline\n> > handler classes to limit the maximum number of requests that get queued\n> > to the device in queueRequestDevice().\n> > \n> > The default value is set to an arbitrary number of 32 which is big\n> > enough for all currently known use cases.\n> > \n> > Signed-off-by: Stefan Klug \n> > Signed-off-by: Kieran Bingham \n> > Signed-off-by: Stefan Klug \n> > \n> \n> Double S-o-B\n> \n> > ---\n> > \n> > Changes in v2:\n> > - Moved the fix to properly handle the waitingRequests queue when\n> > stopping the device into this patch as they belong together\n> > - Added documentation for maxQueuedRequestsDevice_\n> > - Fixed an issue with doQueueRequests beeing() called recursively\n> > \n> > Changes in v1:\n> > - Used a const member variable to carry the maximum number of requests\n> > - Improved commit message\n> > - Added docs\n> > ---\n> > include/libcamera/internal/pipeline_handler.h | 4 +-\n> > src/libcamera/pipeline_handler.cpp | 56 +++++++++++++++----\n> > 2 files changed, 48 insertions(+), 12 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index be017ad47219..e89d6a33e398 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -33,7 +33,8 @@ class PipelineHandler : public std::enable_shared_from_this,\n> > public Object\n> > {\n> > public:\n> > - PipelineHandler(CameraManager *manager);\n> > + PipelineHandler(CameraManager *manager,\n> > + unsigned int maxQueuedRequestsDevice = 32);\n> > virtual ~PipelineHandler();\n> > \n> > virtual bool match(DeviceEnumerator *enumerator) = 0;\n> > @@ -80,6 +81,7 @@ protected:\n> > virtual void releaseDevice(Camera *camera);\n> > \n> > CameraManager *manager_;\n> > + const unsigned int maxQueuedRequestsDevice_;\n> > \n> > private:\n> > void unlockMediaDevices();\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index b50eda5e0f86..e8601be108f0 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -62,13 +62,17 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> > /**\n> > * \\brief Construct a PipelineHandler instance\n> > * \\param[in] manager The camera manager\n> > + * \\param[in] maxQueuedRequestsDevice The maximum number of requests queued to\n> > + * the device\n> > *\n> > * In order to honour the std::enable_shared_from_this<> contract,\n> > * PipelineHandler instances shall never be constructed manually, but always\n> > * through the PipelineHandlerFactoryBase::create() function.\n> > */\n> > -PipelineHandler::PipelineHandler(CameraManager *manager)\n> > - : manager_(manager), useCount_(0)\n> > +PipelineHandler::PipelineHandler(CameraManager *manager,\n> > + unsigned int maxQueuedRequestsDevice)\n> > + : manager_(manager), maxQueuedRequestsDevice_(maxQueuedRequestsDevice),\n> > + useCount_(0)\n> > {\n> > }\n> > \n> > @@ -360,20 +364,28 @@ void PipelineHandler::unlockMediaDevices()\n> > */\n> > void PipelineHandler::stop(Camera *camera)\n> > {\n> > + /*\n> > + * Take all waiting requests so that they are not requeued in response\n> > + * to completeRequest() being called inside stopDevice(). Cancel them\n> > + * after the device to keep them in order.\n> > + */\n> > + Camera::Private *data = camera->_d();\n> > + std::queue waitingRequests;\n> > + waitingRequests.swap(data->waitingRequests_);\n> > +\n> > /* Stop the pipeline handler and let the queued requests complete. */\n> > stopDevice(camera);\n> > \n> > - Camera::Private *data = camera->_d();\n> > -\n> > /* Cancel and signal as complete all waiting requests. */\n> > - while (!data->waitingRequests_.empty()) {\n> > - Request *request = data->waitingRequests_.front();\n> > - data->waitingRequests_.pop();\n> > + while (!waitingRequests.empty()) {\n> > + Request *request = waitingRequests.front();\n> > + waitingRequests.pop();\n> > cancelRequest(request);\n> > }\n> > \n> > /* Make sure no requests are pending. */\n> > ASSERT(data->queuedRequests_.empty());\n> > + ASSERT(data->waitingRequests_.empty());\n> > \n> > data->requestSequence_ = 0;\n> > }\n> > @@ -430,9 +442,9 @@ void PipelineHandler::registerRequest(Request *request)\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> > + * The queue of waiting requests is iterated and up to \\a\n> > + * maxQueuedRequestsDevice_ prepared requests are passed to the pipeline handler\n> > + * in the same order they have been queued by 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> > @@ -487,12 +499,19 @@ void PipelineHandler::doQueueRequests(Camera *camera)\n> > {\n> > Camera::Private *data = camera->_d();\n> > while (!data->waitingRequests_.empty()) {\n> > + if (data->queuedRequests_.size() == maxQueuedRequestsDevice_)\n> > + break;\n> > +\n> > Request *request = data->waitingRequests_.front();\n> > if (!request->_d()->prepared_)\n> > break;\n> > \n> > - doQueueRequest(request);\n> > + /*\n> > + * Pop the request first, in case doQueueRequests() is called\n> > + * recursively from within doQueueRequest()\n> > + */\n> \n> I think what you meant here is, doQueueRequests is called within\n> completeBuffer().\n\nActually I meant it the way I wrote it. You are right, that\ndoQueueRequests() is called within completeBuffer() but it is only\nproblematic if that completeBuffer() is called inside doQueueRequest()\nleading to the recursion.\n\nBest regards,\nStefan\n\n> \n> > data->waitingRequests_.pop();\n> > + doQueueRequest(request);\n> > }\n> > }\n> > \n> > @@ -568,6 +587,9 @@ void PipelineHandler::completeRequest(Request *request)\n> > data->queuedRequests_.pop_front();\n> > camera->requestComplete(req);\n> > }\n> > +\n> > + /* Allow any waiting requests to be queued to the pipeline. */\n> > + doQueueRequests(camera);\n> > }\n> > \n> > /**\n> > @@ -768,6 +790,18 @@ void PipelineHandler::disconnect()\n> > * constant for the whole lifetime of the pipeline handler.\n> > */\n> > \n> > +/**\n> > + * \\var PipelineHandler::maxQueuedRequestsDevice_\n> > + * \\brief The maximum number of requests the pipeline handler shall queue to the\n> > + * device\n> > + *\n> > + * Some pipeline handlers do not work efficiently with too many requests queued\n> > + * into the device. Still it is beneficial for the application to circulate more\n> > + * than the minimum number of buffers. This variable limits the number of\n> > + * requests that get queued to the device at once. The rest is kept in a\n> > + * a waiting queue.\n> > + */\n> > +\n> \n> /**\n> * \\var PipelineHandler::maxQueuedRequestsDevice_\n> * \\brief The maximum number of requests the pipeline handler shall queue to the\n> * device\n> *\n> * maxQueuedRequestsDevice_ limits the number of request that the\n> * pipeline handler shall queue to the underlying hardware, in order to\n> * saturate the pipeline with requests. The application may choose to queue\n> * as many requests as it desires, however only maxQueuedRequestsDevice_\n> * requests will be queued to the hardware at a given point in time. The\n> * remaining requests will be kept waiting in the internal waiting\n> * queue, to be queued at a later stage.\n> */\n> \n> Reviewed-by: Umang Jain \n> \n> \n> > /**\n> > * \\fn PipelineHandler::name()\n> > * \\brief Retrieve the pipeline handler name\n> > -- \n> > 2.48.1\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 C095BBE175\n\tfor ;\n\tThu, 17 Jul 2025 10:03:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ABFFF68F78;\n\tThu, 17 Jul 2025 12:03:04 +0200 (CEST)","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 DE44F61517\n\tfor ;\n\tThu, 17 Jul 2025 12:03:02 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7b93:8acd:d82d:248d])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id E2897E92;\n\tThu, 17 Jul 2025 12:02:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OS5crLeX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752746549;\n\tbh=jJJnxtwF2hD791PCpMVN7ae7XCy5Q0ve//wphk2ZCeQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=OS5crLeX/7Iy6A3CQrf2DeO82r1FvwlLVmxmEn2J2Crq7UhwGhwUGAUIBVCVZML52\n\tZOooAlGdiVMZjLFi6zfa3piIRoju6vV9N+AyxDcKjwq8T5cI7bytir3ko4MiuzBah0\n\tV/QlSbVu4pETZn5u5rhJTqhzLZvAK0t9Q5duk5KU=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250707075400.9079-1-stefan.klug@ideasonboard.com>\n\t<20250707075400.9079-3-stefan.klug@ideasonboard.com>\n\t","Subject":"Re: [PATCH v2 2/5] libcamera: pipeline_handler: Allow to limit the\n\tnumber of queued requests","From":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","To":"Umang Jain ","Date":"Thu, 17 Jul 2025 12:02:59 +0200","Message-ID":"<175274657978.2809694.692482683663363458@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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\" "}}]