[{"id":34359,"web_url":"https://patchwork.libcamera.org/comment/34359/","msgid":"<174835940602.433416.915557287318778416@ping.linuxembedded.co.uk>","date":"2025-05-27T15:23:26","subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof 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-05-26 22:42:16)\n> Allow pipeline handler classes to limit the maximum number of requests that\n> get queued using queueRequestDevice().\n> \n> Signed-off-by: Stefan Klug \n> ---\n> include/libcamera/internal/pipeline_handler.h | 5 +++++\n> src/libcamera/pipeline_handler.cpp | 3 +++\n> 2 files changed, 8 insertions(+)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index dedc29e815fb..86b86d5971dc 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -79,6 +79,11 @@ protected:\n> virtual bool acquireDevice(Camera *camera);\n> virtual void releaseDevice(Camera *camera);\n> \n> + virtual unsigned int maxQueuedRequestsDevice() const\n> + {\n> + return std::numeric_limits::max();\n\nI wonder if something more small yet big enough would be sane like '16'.\n\nBut there's not much difference between 'a number big enough' and the\n'biggest number' if it should always be overridden.\n\nSomething like this should come with a corresponding update to the\npipeline handler writers guide to clarify what should be considered\nhere.\n\n\n> + }\n> +\n> CameraManager *manager_;\n> \n> private:\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 14d8602e67d8..1853bca71371 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests()\n> \n> Camera::Private *data = camera->_d();\n> while (!data->waitingRequests_.empty()) {\n\nPerhaps some verbose explanation here would be helpful to a reader:\n\n\t\t\t/*\n\t\t\t * Limit the number of requests being processed\n\t\t\t * by the device at one time to keep pipeline\n\t\t\t * internal depths controlled. The Camera holds\n\t\t\t * on to requests until the pipeline is ready to\n\t\t\t * process them.\n\t\t\t */\n\n> + if (data->queuedRequests_.size() == maxQueuedRequestsDevice())\n\nWould \n\tif (data->queuedRequests_.size() >= maxQueuedRequestsDevice())\n\t\tbreak;\n\nbe over cautious given the request can only be added below?\n\n> + break;\n> +\n> Request *request = data->waitingRequests_.front();\n> if (!request->_d()->prepared_)\n> break;\n> -- \n> 2.43.0\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 B51C1C31E9\n\tfor ;\n\tTue, 27 May 2025 15:23:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2FE7E68DA0;\n\tTue, 27 May 2025 17:23:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 99F6F68D94\n\tfor ;\n\tTue, 27 May 2025 17:23:29 +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 BD089EFF;\n\tTue, 27 May 2025 17:23:03 +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=\"esF4NB6Q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1748359383;\n\tbh=t5i+JB39GpGBV3a7Aj9SgVTPTQ4vrwuwhHcbjP1Rg3k=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=esF4NB6Q3ACLw+n+kwf+NatBvyv+zHUJLZT6uZ9/tsbVIx2xWdQ9UcWkHT8mVQVOP\n\tgIV1eauudne3keNmu8LA3dCnRZt2NYjAcXS2cmBAK3EZyDnoq1jJNhxjhhAE9sWCEq\n\tSjRqEZGMLCRtf/e+8bjYGvFYNrZU3A2BXzmebnqk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250526214224.13631-3-stefan.klug@ideasonboard.com>","References":"<20250526214224.13631-1-stefan.klug@ideasonboard.com>\n\t<20250526214224.13631-3-stefan.klug@ideasonboard.com>","Subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 27 May 2025 16:23:26 +0100","Message-ID":"<174835940602.433416.915557287318778416@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":34363,"web_url":"https://patchwork.libcamera.org/comment/34363/","msgid":"<3874ce02-64ff-431e-808b-69bb3c48c81e@pengutronix.de>","date":"2025-05-27T15:46:48","subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","submitter":{"id":225,"url":"https://patchwork.libcamera.org/api/people/225/","name":"Sven Püschel","email":"s.pueschel@pengutronix.de"},"content":"Hi Stefan,\n\nOn 5/26/25 23:42, Stefan Klug wrote:\n> Allow pipeline handler classes to limit the maximum number of requests that\n> get queued using queueRequestDevice().\n>\n> Signed-off-by: Stefan Klug \n> ---\n> include/libcamera/internal/pipeline_handler.h | 5 +++++\n> src/libcamera/pipeline_handler.cpp | 3 +++\n> 2 files changed, 8 insertions(+)\n>\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index dedc29e815fb..86b86d5971dc 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -79,6 +79,11 @@ protected:\n> \tvirtual bool acquireDevice(Camera *camera);\n> \tvirtual void releaseDevice(Camera *camera);\n> \n> +\tvirtual unsigned int maxQueuedRequestsDevice() const\n> +\t{\n> +\t\treturn std::numeric_limits::max();\n> +\t}\n> +\nis there a benefit of having a virtual function instead of a variable \n(initialized to max by default and potentially changed in the subclass \nconstructors)?\n> \tCameraManager *manager_;\n> \n> private:\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 14d8602e67d8..1853bca71371 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests()\n> \n> \t\tCamera::Private *data = camera->_d();\n> \t\twhile (!data->waitingRequests_.empty()) {\n> +\t\t\tif (data->queuedRequests_.size() == maxQueuedRequestsDevice())\n> +\t\t\t\tbreak;\n> +\n> \t\t\tRequest *request = data->waitingRequests_.front();\n> \t\t\tif (!request->_d()->prepared_)\n> \t\t\t\tbreak;","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 1B34DC31E9\n\tfor ;\n\tTue, 27 May 2025 15:46:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF4BB68DA0;\n\tTue, 27 May 2025 17:46:50 +0200 (CEST)","from metis.whiteo.stw.pengutronix.de\n\t(metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9263568D94\n\tfor ;\n\tTue, 27 May 2025 17:46:49 +0200 (CEST)","from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77]\n\thelo=[127.0.0.1])\n\tby metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from ) id 1uJwVt-0001O3-3Y\n\tfor libcamera-devel@lists.libcamera.org;\n\tTue, 27 May 2025 17:46:49 +0200"],"Message-ID":"<3874ce02-64ff-431e-808b-69bb3c48c81e@pengutronix.de>","Date":"Tue, 27 May 2025 17:46:48 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","To":"libcamera-devel@lists.libcamera.org","References":"<20250526214224.13631-1-stefan.klug@ideasonboard.com>\n\t<20250526214224.13631-3-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"=?utf-8?q?Sven_P=C3=BCschel?= ","In-Reply-To":"<20250526214224.13631-3-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-SA-Exim-Connect-IP":"2a0a:edc0:0:900:1d::77","X-SA-Exim-Mail-From":"s.pueschel@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.whiteo.stw.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","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":34388,"web_url":"https://patchwork.libcamera.org/comment/34388/","msgid":"","date":"2025-05-30T09:02:55","subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Sven, Stefan\n\nOn Tue, May 27, 2025 at 05:46:48PM +0200, Sven Püschel wrote:\n> Hi Stefan,\n>\n> On 5/26/25 23:42, Stefan Klug wrote:\n> > Allow pipeline handler classes to limit the maximum number of requests that\n> > get queued using queueRequestDevice().\n> >\n> > Signed-off-by: Stefan Klug \n> > ---\n> > include/libcamera/internal/pipeline_handler.h | 5 +++++\n> > src/libcamera/pipeline_handler.cpp | 3 +++\n> > 2 files changed, 8 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index dedc29e815fb..86b86d5971dc 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -79,6 +79,11 @@ protected:\n> > \tvirtual bool acquireDevice(Camera *camera);\n> > \tvirtual void releaseDevice(Camera *camera);\n> > +\tvirtual unsigned int maxQueuedRequestsDevice() const\n> > +\t{\n> > +\t\treturn std::numeric_limits::max();\n> > +\t}\n> > +\n> is there a benefit of having a virtual function instead of a variable\n> (initialized to max by default and potentially changed in the subclass\n> constructors)?\n\nJust for sake of discussion, should we go in the other direction and\nmake this a pure virtual function in the base class to make sure every\npipeline handler provides an implementation ?\n\n> > \tCameraManager *manager_;\n> > private:\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 14d8602e67d8..1853bca71371 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests()\n> > \t\tCamera::Private *data = camera->_d();\n> > \t\twhile (!data->waitingRequests_.empty()) {\n> > +\t\t\tif (data->queuedRequests_.size() == maxQueuedRequestsDevice())\n> > +\t\t\t\tbreak;\n> > +\n> > \t\t\tRequest *request = data->waitingRequests_.front();\n> > \t\t\tif (!request->_d()->prepared_)\n> > \t\t\t\tbreak;","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 13E15C3237\n\tfor ;\n\tFri, 30 May 2025 09:03:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC8D768DA9;\n\tFri, 30 May 2025 11:02:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B661B614D5\n\tfor ;\n\tFri, 30 May 2025 11:02:58 +0200 (CEST)","from ideasonboard.com (mob-5-90-143-247.net.vodafone.it\n\t[5.90.143.247])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DBFCB82E;\n\tFri, 30 May 2025 11:02:30 +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=\"GxPqjuEE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1748595751;\n\tbh=xi/b7nnX+HefHzZapgHo/5tyKM7wWlgomAWRzEgV93c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GxPqjuEE/y2AP5mOzgleYlyC9uqwKJeFIcnI4SLpOKpPlWlmkxKUhBRt+SQWQ6ZoR\n\t3Y1ZkTUf6X9I6TF6BTkwJoJDix1GLzzlLsfiXCpSKRwVGwxp5QT4Uveao9VsPzNCF1\n\tT4N4TzePUBOY8JO4hsbzblFc66rcW7RyfPEe8/K8=","Date":"Fri, 30 May 2025 11:02:55 +0200","From":"Jacopo Mondi ","To":"Sven =?utf-8?q?P=C3=BCschel?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","Message-ID":"","References":"<20250526214224.13631-1-stefan.klug@ideasonboard.com>\n\t<20250526214224.13631-3-stefan.klug@ideasonboard.com>\n\t<3874ce02-64ff-431e-808b-69bb3c48c81e@pengutronix.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<3874ce02-64ff-431e-808b-69bb3c48c81e@pengutronix.de>","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":34392,"web_url":"https://patchwork.libcamera.org/comment/34392/","msgid":"","date":"2025-05-30T10:12:04","subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","submitter":{"id":225,"url":"https://patchwork.libcamera.org/api/people/225/","name":"Sven Püschel","email":"s.pueschel@pengutronix.de"},"content":"Hi Jacopo,\n\nOn 5/30/25 11:02, Jacopo Mondi wrote:\n> Hi Sven, Stefan\n>\n> On Tue, May 27, 2025 at 05:46:48PM +0200, Sven Püschel wrote:\n>> Hi Stefan,\n>>\n>> On 5/26/25 23:42, Stefan Klug wrote:\n>>> Allow pipeline handler classes to limit the maximum number of requests that\n>>> get queued using queueRequestDevice().\n>>>\n>>> Signed-off-by: Stefan Klug \n>>> ---\n>>> include/libcamera/internal/pipeline_handler.h | 5 +++++\n>>> src/libcamera/pipeline_handler.cpp | 3 +++\n>>> 2 files changed, 8 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n>>> index dedc29e815fb..86b86d5971dc 100644\n>>> --- a/include/libcamera/internal/pipeline_handler.h\n>>> +++ b/include/libcamera/internal/pipeline_handler.h\n>>> @@ -79,6 +79,11 @@ protected:\n>>> \tvirtual bool acquireDevice(Camera *camera);\n>>> \tvirtual void releaseDevice(Camera *camera);\n>>> +\tvirtual unsigned int maxQueuedRequestsDevice() const\n>>> +\t{\n>>> +\t\treturn std::numeric_limits::max();\n>>> +\t}\n>>> +\n>> is there a benefit of having a virtual function instead of a variable\n>> (initialized to max by default and potentially changed in the subclass\n>> constructors)?\n> Just for sake of discussion, should we go in the other direction and\n> make this a pure virtual function in the base class to make sure every\n> pipeline handler provides an implementation ?\n\nOption 4: Additional constructor parameter which initializes a constant \nmember variable ^-^\n\nMy reasons why I prefer a variable:\n\n- A function returning a constant just looks weird to me (reminds me of \nhttps://xkcd.com/221/ )\n- (for a constant) code can assume that it won't change during it's \nlifetime (thinking about cases like == maxQueueRequestsDevice() pointed \nout in another comment)\n- If there is an actual use case requiring a variable number, it's not \nhard to change it later to a function\n\n\n>\n>>> \tCameraManager *manager_;\n>>> private:\n>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>> index 14d8602e67d8..1853bca71371 100644\n>>> --- a/src/libcamera/pipeline_handler.cpp\n>>> +++ b/src/libcamera/pipeline_handler.cpp\n>>> @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests()\n>>> \t\tCamera::Private *data = camera->_d();\n>>> \t\twhile (!data->waitingRequests_.empty()) {\n>>> +\t\t\tif (data->queuedRequests_.size() == maxQueuedRequestsDevice())\n>>> +\t\t\t\tbreak;\n>>> +\n>>> \t\t\tRequest *request = data->waitingRequests_.front();\n>>> \t\t\tif (!request->_d()->prepared_)\n>>> \t\t\t\tbreak;","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 2DF4DC31E9\n\tfor ;\n\tFri, 30 May 2025 10:12:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2828568DA9;\n\tFri, 30 May 2025 12:12:07 +0200 (CEST)","from metis.whiteo.stw.pengutronix.de\n\t(metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A0F568D9D\n\tfor ;\n\tFri, 30 May 2025 12:12:05 +0200 (CEST)","from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77]\n\thelo=[127.0.0.1])\n\tby metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from )\n\tid 1uKwia-0000m7-OK; Fri, 30 May 2025 12:12:04 +0200"],"Message-ID":"","Date":"Fri, 30 May 2025 12:12:04 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250526214224.13631-1-stefan.klug@ideasonboard.com>\n\t<20250526214224.13631-3-stefan.klug@ideasonboard.com>\n\t<3874ce02-64ff-431e-808b-69bb3c48c81e@pengutronix.de>\n\t","Content-Language":"en-US","From":"=?utf-8?q?Sven_P=C3=BCschel?= ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-SA-Exim-Connect-IP":"2a0a:edc0:0:900:1d::77","X-SA-Exim-Mail-From":"s.pueschel@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.whiteo.stw.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","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":34705,"web_url":"https://patchwork.libcamera.org/comment/34705/","msgid":"<175104036191.2261782.17246608600470836594@localhost>","date":"2025-06-27T16:06:01","subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Sven,\n\nQuoting Sven Püschel (2025-05-30 12:12:04)\n> Hi Jacopo,\n> \n> On 5/30/25 11:02, Jacopo Mondi wrote:\n> > Hi Sven, Stefan\n> >\n> > On Tue, May 27, 2025 at 05:46:48PM +0200, Sven Püschel wrote:\n> >> Hi Stefan,\n> >>\n> >> On 5/26/25 23:42, Stefan Klug wrote:\n> >>> Allow pipeline handler classes to limit the maximum number of requests that\n> >>> get queued using queueRequestDevice().\n> >>>\n> >>> Signed-off-by: Stefan Klug \n> >>> ---\n> >>> include/libcamera/internal/pipeline_handler.h | 5 +++++\n> >>> src/libcamera/pipeline_handler.cpp | 3 +++\n> >>> 2 files changed, 8 insertions(+)\n> >>>\n> >>> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> >>> index dedc29e815fb..86b86d5971dc 100644\n> >>> --- a/include/libcamera/internal/pipeline_handler.h\n> >>> +++ b/include/libcamera/internal/pipeline_handler.h\n> >>> @@ -79,6 +79,11 @@ protected:\n> >>> virtual bool acquireDevice(Camera *camera);\n> >>> virtual void releaseDevice(Camera *camera);\n> >>> + virtual unsigned int maxQueuedRequestsDevice() const\n> >>> + {\n> >>> + return std::numeric_limits::max();\n> >>> + }\n> >>> +\n> >> is there a benefit of having a virtual function instead of a variable\n> >> (initialized to max by default and potentially changed in the subclass\n> >> constructors)?\n> > Just for sake of discussion, should we go in the other direction and\n> > make this a pure virtual function in the base class to make sure every\n> > pipeline handler provides an implementation ?\n> \n> Option 4: Additional constructor parameter which initializes a constant \n> member variable ^-^\n\nI'm just perparing the promised v1. I tried you suggestion and it turned\nout pretty nice. I'll propose that one :-)\n\nRegards,\nStefan\n\n> \n> My reasons why I prefer a variable:\n> \n> - A function returning a constant just looks weird to me (reminds me of \n> https://xkcd.com/221/ )\n> - (for a constant) code can assume that it won't change during it's \n> lifetime (thinking about cases like == maxQueueRequestsDevice() pointed \n> out in another comment)\n> - If there is an actual use case requiring a variable number, it's not \n> hard to change it later to a function\n> \n> \n> >\n> >>> CameraManager *manager_;\n> >>> private:\n> >>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >>> index 14d8602e67d8..1853bca71371 100644\n> >>> --- a/src/libcamera/pipeline_handler.cpp\n> >>> +++ b/src/libcamera/pipeline_handler.cpp\n> >>> @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests()\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;","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 13131C3237\n\tfor ;\n\tFri, 27 Jun 2025 16:06:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DC57F68E04;\n\tFri, 27 Jun 2025 18:06:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 37C3F68DE5\n\tfor ;\n\tFri, 27 Jun 2025 18:06:05 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:b16c:76dc:2ea2:3cca])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 57B42819;\n\tFri, 27 Jun 2025 18:05:45 +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=\"BOfMihcH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751040345;\n\tbh=eMSOrpsCdsLogCz4p0CN/zJ7OtqZh2FOwplTqihNK1I=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=BOfMihcHZcpjWs5v+eCctI1EMPKqWzDJgL1coh97llPE1IyLJYaqXz0dbO35ujzSj\n\twVXglvc6DwIVNefEsp4hi+F/PftQV6aUpGiU9EPTw9wp7HsGJW+eCZPx4sMZu8HAuN\n\tjZLZniijh9tP0dwT6py8e4Rl3TE8ZkCcBz5c3dkI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250526214224.13631-1-stefan.klug@ideasonboard.com>\n\t<20250526214224.13631-3-stefan.klug@ideasonboard.com>\n\t<3874ce02-64ff-431e-808b-69bb3c48c81e@pengutronix.de>\n\t\n\t","Subject":"Re: [RFC PATCH 2/4] libcamera: internal: Allow to limit the number\n\tof queued requests","From":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","To":"Jacopo Mondi , Sven =?utf-8?b?UMO8c2No?=\n\t=?utf-8?q?el?= ","Date":"Fri, 27 Jun 2025 18:06:01 +0200","Message-ID":"<175104036191.2261782.17246608600470836594@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\" "}}]