[{"id":34753,"web_url":"https://patchwork.libcamera.org/comment/34753/","msgid":"<175135679502.2426344.2083856518536494275@localhost>","date":"2025-07-01T07:59:55","subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","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 patch. \n\nQuoting Umang Jain (2025-07-01 06:58:05)\n> PipelineHandler::queueRequestDevice() returns a negative error code if\n> the request fails to get queued to the underlying hardware. One such\n> case would be if the hardware is saturated with requests beyond its\n> limits, in which case queueRequestDevice() should return -EAGAIN.\n> \n> This way, each pipeline handler can keep their limits internal and\n> decide to start returning -EAGAIN if they foresee their pipelines\n> queues getting over-saturated with requests.\n> \n> Hence, returns error codes from doQueueRequest() so that\n> doQueueRequests() can make an inform decision whether to pop off the\n> requests from the internal waiting queue or not.\n\nIf I got you right, the idea is to replace the MaxQueuedRequestsDevice I\nadded in the other series by this mechanism that return -EAGAIN on\nsaturation. That might actually work equally well...\n\nI think I need to try that out.\n\nBest regards,\nStefan\n\n> \n> Signed-off-by: Umang Jain \n> ---\n> include/libcamera/internal/pipeline_handler.h | 2 +-\n> src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++-----\n> 2 files changed, 16 insertions(+), 6 deletions(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 972a2fa6..35252e3a 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -88,7 +88,7 @@ private:\n> void mediaDeviceDisconnected(MediaDevice *media);\n> virtual void disconnect();\n> \n> - void doQueueRequest(Request *request);\n> + int doQueueRequest(Request *request);\n> void doQueueRequests();\n> \n> std::vector> mediaDevices_;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index d84dff3c..2ce093ff 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -452,8 +452,10 @@ void PipelineHandler::queueRequest(Request *request)\n> /**\n> * \\brief Queue one requests to the device\n> */\n> -void PipelineHandler::doQueueRequest(Request *request)\n> +int PipelineHandler::doQueueRequest(Request *request)\n> {\n> + int ret;\n> +\n> LIBCAMERA_TRACEPOINT(request_device_queue, request);\n> \n> Camera *camera = request->_d()->camera();\n> @@ -464,12 +466,14 @@ void PipelineHandler::doQueueRequest(Request *request)\n> \n> if (request->_d()->cancelled_) {\n> completeRequest(request);\n> - return;\n> + return -ECANCELED;\n> }\n> \n> - int ret = queueRequestDevice(camera, request);\n> - if (ret)\n> + ret = queueRequestDevice(camera, request);\n> + if (ret && ret != -EAGAIN)\n> cancelRequest(request);\n> +\n> + return ret;\n> }\n> \n> /**\n> @@ -485,7 +489,9 @@ void PipelineHandler::doQueueRequests()\n> if (!request->_d()->prepared_)\n> break;\n> \n> - doQueueRequest(request);\n> + if (doQueueRequest(request) == -EAGAIN)\n> + break;\n> +\n> waitingRequests_.pop();\n> }\n> }\n> @@ -502,6 +508,10 @@ void PipelineHandler::doQueueRequests()\n> * parameters will be applied to the frames captured in the buffers provided in\n> * the request.\n> *\n> + * If the underlying hardware pipeline is saturated with requests, this\n> + * function returns -EAGAIN, so that the \\a request stays in the internal\n> + * waiting queue.\n> + *\n> * \\context This function is called from the CameraManager thread.\n> *\n> * \\return 0 on success or a negative error code otherwise\n> -- \n> 2.50.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 058C7C3237\n\tfor ;\n\tTue, 1 Jul 2025 08:00:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2EDA068E12;\n\tTue, 1 Jul 2025 10:00:00 +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 6552361528\n\tfor ;\n\tTue, 1 Jul 2025 09:59:58 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7cdc:2548:b8ee:3b98])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 74ACFC74;\n\tTue, 1 Jul 2025 09:59:35 +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=\"GtGCeC59\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751356775;\n\tbh=7abBWjbrgIQyir8PG00gr+DpSich8d/eRNU/IBk5ci0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GtGCeC59LiAeJOdHW63yGwc/HRM+3WwgS2oulTtPX3bK/RFdMdqkeza3zu+rWSXn1\n\tjUX/iuoVGGwiwoRM8+N0bMCrsVbGt+ogcSoM0Emmb5YBq9gbyDVrlGhNp/yANVp13n\n\tw6YtF6uAxjqs4/PT6+FtNGOfHn0ofS49Vo8+i1TE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250701045805.15201-1-uajain@igalia.com>","References":"<20250701045805.15201-1-uajain@igalia.com>","Subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","From":"Stefan Klug ","Cc":"Umang Jain ","To":"Umang Jain , libcamera-devel@lists.libcamera.org","Date":"Tue, 01 Jul 2025 09:59:55 +0200","Message-ID":"<175135679502.2426344.2083856518536494275@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\" "}},{"id":34755,"web_url":"https://patchwork.libcamera.org/comment/34755/","msgid":"<175135842398.2426344.5758856435692493490@localhost>","date":"2025-07-01T08:27:03","subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Umang,\n\nQuoting Stefan Klug (2025-07-01 09:59:55)\n> Hi Umang,\n> \n> Thank you for the patch. \n> \n> Quoting Umang Jain (2025-07-01 06:58:05)\n> > PipelineHandler::queueRequestDevice() returns a negative error code if\n> > the request fails to get queued to the underlying hardware. One such\n> > case would be if the hardware is saturated with requests beyond its\n> > limits, in which case queueRequestDevice() should return -EAGAIN.\n> > \n> > This way, each pipeline handler can keep their limits internal and\n> > decide to start returning -EAGAIN if they foresee their pipelines\n> > queues getting over-saturated with requests.\n> > \n> > Hence, returns error codes from doQueueRequest() so that\n> > doQueueRequests() can make an inform decision whether to pop off the\n> > requests from the internal waiting queue or not.\n> \n> If I got you right, the idea is to replace the MaxQueuedRequestsDevice I\n> added in the other series by this mechanism that return -EAGAIN on\n> saturation. That might actually work equally well...\n\nAnother note on that matter. This solution moves the complexity of\nbookkeeping into the pipeline handler implementation. Could you explain\nyour rationale to prefer this way over the maxQueuedRequests counter?\n\nBest regards,\nStefan\n\n> I think I need to try that out.\n> \n> Best regards,\n> Stefan\n> \n> > \n> > Signed-off-by: Umang Jain \n> > ---\n> > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++-----\n> > 2 files changed, 16 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index 972a2fa6..35252e3a 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -88,7 +88,7 @@ private:\n> > void mediaDeviceDisconnected(MediaDevice *media);\n> > virtual void disconnect();\n> > \n> > - void doQueueRequest(Request *request);\n> > + int doQueueRequest(Request *request);\n> > void doQueueRequests();\n> > \n> > std::vector> mediaDevices_;\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index d84dff3c..2ce093ff 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -452,8 +452,10 @@ void PipelineHandler::queueRequest(Request *request)\n> > /**\n> > * \\brief Queue one requests to the device\n> > */\n> > -void PipelineHandler::doQueueRequest(Request *request)\n> > +int PipelineHandler::doQueueRequest(Request *request)\n> > {\n> > + int ret;\n> > +\n> > LIBCAMERA_TRACEPOINT(request_device_queue, request);\n> > \n> > Camera *camera = request->_d()->camera();\n> > @@ -464,12 +466,14 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > \n> > if (request->_d()->cancelled_) {\n> > completeRequest(request);\n> > - return;\n> > + return -ECANCELED;\n> > }\n> > \n> > - int ret = queueRequestDevice(camera, request);\n> > - if (ret)\n> > + ret = queueRequestDevice(camera, request);\n> > + if (ret && ret != -EAGAIN)\n> > cancelRequest(request);\n> > +\n> > + return ret;\n> > }\n> > \n> > /**\n> > @@ -485,7 +489,9 @@ void PipelineHandler::doQueueRequests()\n> > if (!request->_d()->prepared_)\n> > break;\n> > \n> > - doQueueRequest(request);\n> > + if (doQueueRequest(request) == -EAGAIN)\n> > + break;\n> > +\n> > waitingRequests_.pop();\n> > }\n> > }\n> > @@ -502,6 +508,10 @@ void PipelineHandler::doQueueRequests()\n> > * parameters will be applied to the frames captured in the buffers provided in\n> > * the request.\n> > *\n> > + * If the underlying hardware pipeline is saturated with requests, this\n> > + * function returns -EAGAIN, so that the \\a request stays in the internal\n> > + * waiting queue.\n> > + *\n> > * \\context This function is called from the CameraManager thread.\n> > *\n> > * \\return 0 on success or a negative error code otherwise\n> > -- \n> > 2.50.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 10B5BC3237\n\tfor ;\n\tTue, 1 Jul 2025 08:27:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E525468E13;\n\tTue, 1 Jul 2025 10:27:08 +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 8BC9161528\n\tfor ;\n\tTue, 1 Jul 2025 10:27:06 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7cdc:2548:b8ee:3b98])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 159F57E0;\n\tTue, 1 Jul 2025 10:26:44 +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=\"G6Uho5HV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751358404;\n\tbh=1h6gOkbiQHs7NJgJutQ0zY7bq1LJu1dvFmsIMwiLLa8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=G6Uho5HVW8s6ibBD01ELEr8kYpNloBlmo4b1qnhvN+nHbPm7MKtlh6bqFQJy9wFTL\n\tSzg7DDpHIY6bZjFqyPwdZP+xtYDUQG9rVybbZXOuOv0gHLbqTknCYKfQIyYjrZpsqb\n\tPLO0eRA2XnGKmIlVmg8UKIsYt/hlG3ZA2F38gs8o=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175135679502.2426344.2083856518536494275@localhost>","References":"<20250701045805.15201-1-uajain@igalia.com>\n\t<175135679502.2426344.2083856518536494275@localhost>","Subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","From":"Stefan Klug ","Cc":"Umang Jain ","To":"Umang Jain , libcamera-devel@lists.libcamera.org","Date":"Tue, 01 Jul 2025 10:27:03 +0200","Message-ID":"<175135842398.2426344.5758856435692493490@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\" "}},{"id":34760,"web_url":"https://patchwork.libcamera.org/comment/34760/","msgid":"","date":"2025-07-01T09:43:18","subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Stefan,\n\nOn Tue, Jul 01, 2025 at 10:27:03AM +0200, Stefan Klug wrote:\n> Hi Umang,\n> \n> Quoting Stefan Klug (2025-07-01 09:59:55)\n> > Hi Umang,\n> > \n> > Thank you for the patch. \n> > \n> > Quoting Umang Jain (2025-07-01 06:58:05)\n> > > PipelineHandler::queueRequestDevice() returns a negative error code if\n> > > the request fails to get queued to the underlying hardware. One such\n> > > case would be if the hardware is saturated with requests beyond its\n> > > limits, in which case queueRequestDevice() should return -EAGAIN.\n> > > \n> > > This way, each pipeline handler can keep their limits internal and\n> > > decide to start returning -EAGAIN if they foresee their pipelines\n> > > queues getting over-saturated with requests.\n> > > \n> > > Hence, returns error codes from doQueueRequest() so that\n> > > doQueueRequests() can make an inform decision whether to pop off the\n> > > requests from the internal waiting queue or not.\n> > \n> > If I got you right, the idea is to replace the MaxQueuedRequestsDevice I\n> > added in the other series by this mechanism that return -EAGAIN on\n> > saturation. That might actually work equally well...\n> \n\nWell, I wrote while reviewing the maxQueuedRequestsDevice patch so,\nI am not sure if this is a replacement (depends on your ultimate goals)\nor an enhancement in bubbling up return values.\n\n> Another note on that matter. This solution moves the complexity of\n> bookkeeping into the pipeline handler implementation. Could you explain\n> your rationale to prefer this way over the maxQueuedRequests counter?\n\nI don't have any particular rationale for this implementation. For\nstarters, you can even bubble up -EAGAIN (from individual pipelines)\nto applications to let them know to rate-limit their requests...\n\nAbout maxQueuedRequestsDevice, I think I am missing over-arching\ndesign perspective/goals on what needs to be achieved, I have left\ncomments on that threads. And about the complexity, I don't think\nit's that complex to detect requests queue saturation, so it doesn't\nmatter where it lives, as long as it serves the design.\n\n> \n> Best regards,\n> Stefan\n> \n> > I think I need to try that out.\n> > \n> > Best regards,\n> > Stefan\n> > \n> > > \n> > > Signed-off-by: Umang Jain \n> > > ---\n> > > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++-----\n> > > 2 files changed, 16 insertions(+), 6 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > > index 972a2fa6..35252e3a 100644\n> > > --- a/include/libcamera/internal/pipeline_handler.h\n> > > +++ b/include/libcamera/internal/pipeline_handler.h\n> > > @@ -88,7 +88,7 @@ private:\n> > > void mediaDeviceDisconnected(MediaDevice *media);\n> > > virtual void disconnect();\n> > > \n> > > - void doQueueRequest(Request *request);\n> > > + int doQueueRequest(Request *request);\n> > > void doQueueRequests();\n> > > \n> > > std::vector> mediaDevices_;\n> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index d84dff3c..2ce093ff 100644\n> > > --- a/src/libcamera/pipeline_handler.cpp\n> > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > @@ -452,8 +452,10 @@ void PipelineHandler::queueRequest(Request *request)\n> > > /**\n> > > * \\brief Queue one requests to the device\n> > > */\n> > > -void PipelineHandler::doQueueRequest(Request *request)\n> > > +int PipelineHandler::doQueueRequest(Request *request)\n> > > {\n> > > + int ret;\n> > > +\n> > > LIBCAMERA_TRACEPOINT(request_device_queue, request);\n> > > \n> > > Camera *camera = request->_d()->camera();\n> > > @@ -464,12 +466,14 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > > \n> > > if (request->_d()->cancelled_) {\n> > > completeRequest(request);\n> > > - return;\n> > > + return -ECANCELED;\n> > > }\n> > > \n> > > - int ret = queueRequestDevice(camera, request);\n> > > - if (ret)\n> > > + ret = queueRequestDevice(camera, request);\n> > > + if (ret && ret != -EAGAIN)\n> > > cancelRequest(request);\n> > > +\n> > > + return ret;\n> > > }\n> > > \n> > > /**\n> > > @@ -485,7 +489,9 @@ void PipelineHandler::doQueueRequests()\n> > > if (!request->_d()->prepared_)\n> > > break;\n> > > \n> > > - doQueueRequest(request);\n> > > + if (doQueueRequest(request) == -EAGAIN)\n> > > + break;\n> > > +\n> > > waitingRequests_.pop();\n> > > }\n> > > }\n> > > @@ -502,6 +508,10 @@ void PipelineHandler::doQueueRequests()\n> > > * parameters will be applied to the frames captured in the buffers provided in\n> > > * the request.\n> > > *\n> > > + * If the underlying hardware pipeline is saturated with requests, this\n> > > + * function returns -EAGAIN, so that the \\a request stays in the internal\n> > > + * waiting queue.\n> > > + *\n> > > * \\context This function is called from the CameraManager thread.\n> > > *\n> > > * \\return 0 on success or a negative error code otherwise\n> > > -- \n> > > 2.50.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 A17A6BDCBF\n\tfor ;\n\tTue, 1 Jul 2025 09:43:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C5D2768E19;\n\tTue, 1 Jul 2025 11:43:17 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F7F861528\n\tfor ;\n\tTue, 1 Jul 2025 11:43:15 +0200 (CEST)","from [49.36.69.141] (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 1uWXWE-00AuAJ-0F; Tue, 01 Jul 2025 11:43:14 +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=\"YYbmKg7l\"; 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=e2U6xbLMvuHBCmVyejuwUcMpy2EWQIidiF3S9lc47FA=;\n\tb=YYbmKg7lYXZlPJcVQTS2y2ci/D\n\t7ebdKc/jrtdxJk26ZUNF8XmHCyeTXSwyysCY2Xlyt+dcjLhln4+Uzg3yPM0eNBUWZuDXa2sl35zD0\n\t/n+gM7cTcruf2B+AmPP3lKGILQTnyOWfQN4Np+y6OUQXU4upWQKi4bGbKRlkLJguzEtUCrep0CKlw\n\t4xBFIVea5zBhdZv4y13ic0cT2GjFmjG5PEvwB5LYURZY+8S2+lVkp1NYXDRJIb/Tf10Qtt0bdJnoC\n\tJakPACji/XEzrOvn9slmr02gd4XhCke37WsH7MtN7544kAmV7iVeabOxpO0GMDCyS2hKUWF7NJKQF\n\tremuzJCw==;","Date":"Tue, 1 Jul 2025 15:13:18 +0530","From":"Umang Jain ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","Message-ID":"","References":"<20250701045805.15201-1-uajain@igalia.com>\n\t<175135679502.2426344.2083856518536494275@localhost>\n\t<175135842398.2426344.5758856435692493490@localhost>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<175135842398.2426344.5758856435692493490@localhost>","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":34764,"web_url":"https://patchwork.libcamera.org/comment/34764/","msgid":"<20250701112221.GA1936@pendragon.ideasonboard.com>","date":"2025-07-01T11:22:21","subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jul 01, 2025 at 03:13:18PM +0530, Umang Jain wrote:\n> On Tue, Jul 01, 2025 at 10:27:03AM +0200, Stefan Klug wrote:\n> > Quoting Stefan Klug (2025-07-01 09:59:55)\n> > > Quoting Umang Jain (2025-07-01 06:58:05)\n> > > > PipelineHandler::queueRequestDevice() returns a negative error code if\n> > > > the request fails to get queued to the underlying hardware. One such\n> > > > case would be if the hardware is saturated with requests beyond its\n> > > > limits, in which case queueRequestDevice() should return -EAGAIN.\n> > > > \n> > > > This way, each pipeline handler can keep their limits internal and\n> > > > decide to start returning -EAGAIN if they foresee their pipelines\n> > > > queues getting over-saturated with requests.\n> > > > \n> > > > Hence, returns error codes from doQueueRequest() so that\n> > > > doQueueRequests() can make an inform decision whether to pop off the\n> > > > requests from the internal waiting queue or not.\n> > > \n> > > If I got you right, the idea is to replace the MaxQueuedRequestsDevice I\n> > > added in the other series by this mechanism that return -EAGAIN on\n> > > saturation. That might actually work equally well...\n> \n> Well, I wrote while reviewing the maxQueuedRequestsDevice patch so,\n> I am not sure if this is a replacement (depends on your ultimate goals)\n> or an enhancement in bubbling up return values.\n> \n> > Another note on that matter. This solution moves the complexity of\n> > bookkeeping into the pipeline handler implementation. Could you explain\n> > your rationale to prefer this way over the maxQueuedRequests counter?\n> \n> I don't have any particular rationale for this implementation. For\n> starters, you can even bubble up -EAGAIN (from individual pipelines)\n> to applications to let them know to rate-limit their requests...\n\nNo we can't do that, the call is asynchronous.\n\n> About maxQueuedRequestsDevice, I think I am missing over-arching\n> design perspective/goals on what needs to be achieved, I have left\n> comments on that threads. And about the complexity, I don't think\n> it's that complex to detect requests queue saturation, so it doesn't\n> matter where it lives, as long as it serves the design.\n\nIf it lives in the pipeline handler the logic will need to be duplicated\nin all pipeline handlers. Unless there are clear use cases for a logic\ndifferent than cheking the number of queued requests, I'd rather handle\nthis in the base class.\n\n> > > I think I need to try that out.\n> > > \n> > > > Signed-off-by: Umang Jain \n> > > > ---\n> > > > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > > > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++-----\n> > > > 2 files changed, 16 insertions(+), 6 deletions(-)\n> > > > \n> > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > > > index 972a2fa6..35252e3a 100644\n> > > > --- a/include/libcamera/internal/pipeline_handler.h\n> > > > +++ b/include/libcamera/internal/pipeline_handler.h\n> > > > @@ -88,7 +88,7 @@ private:\n> > > > void mediaDeviceDisconnected(MediaDevice *media);\n> > > > virtual void disconnect();\n> > > > \n> > > > - void doQueueRequest(Request *request);\n> > > > + int doQueueRequest(Request *request);\n> > > > void doQueueRequests();\n> > > > \n> > > > std::vector> mediaDevices_;\n> > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > > index d84dff3c..2ce093ff 100644\n> > > > --- a/src/libcamera/pipeline_handler.cpp\n> > > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > > @@ -452,8 +452,10 @@ void PipelineHandler::queueRequest(Request *request)\n> > > > /**\n> > > > * \\brief Queue one requests to the device\n> > > > */\n> > > > -void PipelineHandler::doQueueRequest(Request *request)\n> > > > +int PipelineHandler::doQueueRequest(Request *request)\n> > > > {\n> > > > + int ret;\n> > > > +\n> > > > LIBCAMERA_TRACEPOINT(request_device_queue, request);\n> > > > \n> > > > Camera *camera = request->_d()->camera();\n> > > > @@ -464,12 +466,14 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > > > \n> > > > if (request->_d()->cancelled_) {\n> > > > completeRequest(request);\n> > > > - return;\n> > > > + return -ECANCELED;\n> > > > }\n> > > > \n> > > > - int ret = queueRequestDevice(camera, request);\n> > > > - if (ret)\n> > > > + ret = queueRequestDevice(camera, request);\n> > > > + if (ret && ret != -EAGAIN)\n> > > > cancelRequest(request);\n> > > > +\n> > > > + return ret;\n> > > > }\n> > > > \n> > > > /**\n> > > > @@ -485,7 +489,9 @@ void PipelineHandler::doQueueRequests()\n> > > > if (!request->_d()->prepared_)\n> > > > break;\n> > > > \n> > > > - doQueueRequest(request);\n> > > > + if (doQueueRequest(request) == -EAGAIN)\n> > > > + break;\n> > > > +\n> > > > waitingRequests_.pop();\n> > > > }\n> > > > }\n> > > > @@ -502,6 +508,10 @@ void PipelineHandler::doQueueRequests()\n> > > > * parameters will be applied to the frames captured in the buffers provided in\n> > > > * the request.\n> > > > *\n> > > > + * If the underlying hardware pipeline is saturated with requests, this\n> > > > + * function returns -EAGAIN, so that the \\a request stays in the internal\n> > > > + * waiting queue.\n> > > > + *\n> > > > * \\context This function is called from the CameraManager thread.\n> > > > *\n> > > > * \\return 0 on success or a negative error code otherwise","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 B2086BDCBF\n\tfor ;\n\tTue, 1 Jul 2025 11:22:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6F6E68E1E;\n\tTue, 1 Jul 2025 13:22:49 +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 E61A76152A\n\tfor ;\n\tTue, 1 Jul 2025 13:22:47 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 316D978C;\n\tTue, 1 Jul 2025 13:22:25 +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=\"SOeIkyaz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751368945;\n\tbh=wEfmlRp+njPsvfQt/5NEiyl85GPKkiscgi+xYuvMNoY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SOeIkyaz4wfSypA9Ucty1yTGuWfvV5HwbF+kFQYwvIPaXyvtPs+UwTr0YCcouuVz5\n\t1K9RI94J3qL/QiHSAYxMMHoUZdaN+mkXt47tE7sQB9HgFr0i/MZv3zru/EXL/s/z/p\n\tc7vU2LqWniiIu9eJdM//fwXfS27jmgnz9oYLushc=","Date":"Tue, 1 Jul 2025 14:22:21 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","Message-ID":"<20250701112221.GA1936@pendragon.ideasonboard.com>","References":"<20250701045805.15201-1-uajain@igalia.com>\n\t<175135679502.2426344.2083856518536494275@localhost>\n\t<175135842398.2426344.5758856435692493490@localhost>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34767,"web_url":"https://patchwork.libcamera.org/comment/34767/","msgid":"<175137604423.2426344.7329689415295958835@localhost>","date":"2025-07-01T13:20:44","subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Umang,\n\nQuoting Umang Jain (2025-07-01 11:43:18)\n> Hi Stefan,\n> \n> On Tue, Jul 01, 2025 at 10:27:03AM +0200, Stefan Klug wrote:\n> > Hi Umang,\n> > \n> > Quoting Stefan Klug (2025-07-01 09:59:55)\n> > > Hi Umang,\n> > > \n> > > Thank you for the patch. \n> > > \n> > > Quoting Umang Jain (2025-07-01 06:58:05)\n> > > > PipelineHandler::queueRequestDevice() returns a negative error code if\n> > > > the request fails to get queued to the underlying hardware. One such\n> > > > case would be if the hardware is saturated with requests beyond its\n> > > > limits, in which case queueRequestDevice() should return -EAGAIN.\n> > > > \n> > > > This way, each pipeline handler can keep their limits internal and\n> > > > decide to start returning -EAGAIN if they foresee their pipelines\n> > > > queues getting over-saturated with requests.\n> > > > \n> > > > Hence, returns error codes from doQueueRequest() so that\n> > > > doQueueRequests() can make an inform decision whether to pop off the\n> > > > requests from the internal waiting queue or not.\n> > > \n> > > If I got you right, the idea is to replace the MaxQueuedRequestsDevice I\n> > > added in the other series by this mechanism that return -EAGAIN on\n> > > saturation. That might actually work equally well...\n> > \n> \n> Well, I wrote while reviewing the maxQueuedRequestsDevice patch so,\n> I am not sure if this is a replacement (depends on your ultimate goals)\n> or an enhancement in bubbling up return values.\n> \n> > Another note on that matter. This solution moves the complexity of\n> > bookkeeping into the pipeline handler implementation. Could you explain\n> > your rationale to prefer this way over the maxQueuedRequests counter?\n> \n> I don't have any particular rationale for this implementation. For\n> starters, you can even bubble up -EAGAIN (from individual pipelines)\n> to applications to let them know to rate-limit their requests...\n> \n> About maxQueuedRequestsDevice, I think I am missing over-arching\n> design perspective/goals on what needs to be achieved, I have left\n> comments on that threads. And about the complexity, I don't think\n> it's that complex to detect requests queue saturation, so it doesn't\n> matter where it lives, as long as it serves the design.\n\nHm, then I'm not really getting the purpose of this patch. I don't think\nwe should bubble -EAGAIN up to applications as that would be a big break\nin the behavior of libcamera (and I don't see a reason for user land to\nforce them to rate limit their requests). If we implement -EAGAIN to\nlimit the number of buffers queued to the device, we should not\nimplement maxQueuedRequestsDevice as it would be two implementations for\nbasically the same thing. Could you explain your thoughts? I seem to be\nmissing something.\n\nBest regards,\nStefan\n\n> \n> > \n> > Best regards,\n> > Stefan\n> > \n> > > I think I need to try that out.\n> > > \n> > > Best regards,\n> > > Stefan\n> > > \n> > > > \n> > > > Signed-off-by: Umang Jain \n> > > > ---\n> > > > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > > > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++-----\n> > > > 2 files changed, 16 insertions(+), 6 deletions(-)\n> > > > \n> > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > > > index 972a2fa6..35252e3a 100644\n> > > > --- a/include/libcamera/internal/pipeline_handler.h\n> > > > +++ b/include/libcamera/internal/pipeline_handler.h\n> > > > @@ -88,7 +88,7 @@ private:\n> > > > void mediaDeviceDisconnected(MediaDevice *media);\n> > > > virtual void disconnect();\n> > > > \n> > > > - void doQueueRequest(Request *request);\n> > > > + int doQueueRequest(Request *request);\n> > > > void doQueueRequests();\n> > > > \n> > > > std::vector> mediaDevices_;\n> > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > > index d84dff3c..2ce093ff 100644\n> > > > --- a/src/libcamera/pipeline_handler.cpp\n> > > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > > @@ -452,8 +452,10 @@ void PipelineHandler::queueRequest(Request *request)\n> > > > /**\n> > > > * \\brief Queue one requests to the device\n> > > > */\n> > > > -void PipelineHandler::doQueueRequest(Request *request)\n> > > > +int PipelineHandler::doQueueRequest(Request *request)\n> > > > {\n> > > > + int ret;\n> > > > +\n> > > > LIBCAMERA_TRACEPOINT(request_device_queue, request);\n> > > > \n> > > > Camera *camera = request->_d()->camera();\n> > > > @@ -464,12 +466,14 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > > > \n> > > > if (request->_d()->cancelled_) {\n> > > > completeRequest(request);\n> > > > - return;\n> > > > + return -ECANCELED;\n> > > > }\n> > > > \n> > > > - int ret = queueRequestDevice(camera, request);\n> > > > - if (ret)\n> > > > + ret = queueRequestDevice(camera, request);\n> > > > + if (ret && ret != -EAGAIN)\n> > > > cancelRequest(request);\n> > > > +\n> > > > + return ret;\n> > > > }\n> > > > \n> > > > /**\n> > > > @@ -485,7 +489,9 @@ void PipelineHandler::doQueueRequests()\n> > > > if (!request->_d()->prepared_)\n> > > > break;\n> > > > \n> > > > - doQueueRequest(request);\n> > > > + if (doQueueRequest(request) == -EAGAIN)\n> > > > + break;\n> > > > +\n> > > > waitingRequests_.pop();\n> > > > }\n> > > > }\n> > > > @@ -502,6 +508,10 @@ void PipelineHandler::doQueueRequests()\n> > > > * parameters will be applied to the frames captured in the buffers provided in\n> > > > * the request.\n> > > > *\n> > > > + * If the underlying hardware pipeline is saturated with requests, this\n> > > > + * function returns -EAGAIN, so that the \\a request stays in the internal\n> > > > + * waiting queue.\n> > > > + *\n> > > > * \\context This function is called from the CameraManager thread.\n> > > > *\n> > > > * \\return 0 on success or a negative error code otherwise\n> > > > -- \n> > > > 2.50.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 3CA19BDCBF\n\tfor ;\n\tTue, 1 Jul 2025 13:20:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 06D9A68E29;\n\tTue, 1 Jul 2025 15:20:49 +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 DA48568E24\n\tfor ;\n\tTue, 1 Jul 2025 15:20:47 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7cdc:2548:b8ee:3b98])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 3AB246F3;\n\tTue, 1 Jul 2025 15:20:25 +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=\"Z75O8fVJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751376025;\n\tbh=vn6gICkhImhy1azjZfqPCD+4asWCdzWwWiLz2MsNPNQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Z75O8fVJFlX2IxTcxWrTGrGOQqWWAJuKZMsJTDLuXTsofsv8ob2/EHHYyZj0+Y4sd\n\tCQYkjd6m1tBS6GFGU17UmyfNpm7Ohfm+UlFVaztLmfZMcJRD9XdTBI2qguaooR6mIc\n\tNrENb47KmrNIxFZAw7mgscTeJym8m3x+Hy5eR4ts=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250701045805.15201-1-uajain@igalia.com>\n\t<175135679502.2426344.2083856518536494275@localhost>\n\t<175135842398.2426344.5758856435692493490@localhost>\n\t","Subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","From":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","To":"Umang Jain ","Date":"Tue, 01 Jul 2025 15:20:44 +0200","Message-ID":"<175137604423.2426344.7329689415295958835@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\" "}},{"id":34771,"web_url":"https://patchwork.libcamera.org/comment/34771/","msgid":"","date":"2025-07-01T15:44:26","subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On Tue, Jul 01, 2025 at 03:20:44PM +0200, Stefan Klug wrote:\n> Hi Umang,\n> \n> Quoting Umang Jain (2025-07-01 11:43:18)\n> > Hi Stefan,\n> > \n> > On Tue, Jul 01, 2025 at 10:27:03AM +0200, Stefan Klug wrote:\n> > > Hi Umang,\n> > > \n> > > Quoting Stefan Klug (2025-07-01 09:59:55)\n> > > > Hi Umang,\n> > > > \n> > > > Thank you for the patch. \n> > > > \n> > > > Quoting Umang Jain (2025-07-01 06:58:05)\n> > > > > PipelineHandler::queueRequestDevice() returns a negative error code if\n> > > > > the request fails to get queued to the underlying hardware. One such\n> > > > > case would be if the hardware is saturated with requests beyond its\n> > > > > limits, in which case queueRequestDevice() should return -EAGAIN.\n> > > > > \n> > > > > This way, each pipeline handler can keep their limits internal and\n> > > > > decide to start returning -EAGAIN if they foresee their pipelines\n> > > > > queues getting over-saturated with requests.\n> > > > > \n> > > > > Hence, returns error codes from doQueueRequest() so that\n> > > > > doQueueRequests() can make an inform decision whether to pop off the\n> > > > > requests from the internal waiting queue or not.\n> > > > \n> > > > If I got you right, the idea is to replace the MaxQueuedRequestsDevice I\n> > > > added in the other series by this mechanism that return -EAGAIN on\n> > > > saturation. That might actually work equally well...\n> > > \n> > \n> > Well, I wrote while reviewing the maxQueuedRequestsDevice patch so,\n> > I am not sure if this is a replacement (depends on your ultimate goals)\n> > or an enhancement in bubbling up return values.\n> > \n> > > Another note on that matter. This solution moves the complexity of\n> > > bookkeeping into the pipeline handler implementation. Could you explain\n> > > your rationale to prefer this way over the maxQueuedRequests counter?\n> > \n> > I don't have any particular rationale for this implementation. For\n> > starters, you can even bubble up -EAGAIN (from individual pipelines)\n> > to applications to let them know to rate-limit their requests...\n> > \n> > About maxQueuedRequestsDevice, I think I am missing over-arching\n> > design perspective/goals on what needs to be achieved, I have left\n> > comments on that threads. And about the complexity, I don't think\n> > it's that complex to detect requests queue saturation, so it doesn't\n> > matter where it lives, as long as it serves the design.\n> \n> Hm, then I'm not really getting the purpose of this patch. I don't think\n> we should bubble -EAGAIN up to applications as that would be a big break\n> in the behavior of libcamera (and I don't see a reason for user land to\n> force them to rate limit their requests). If we implement -EAGAIN to\n> limit the number of buffers queued to the device, we should not\n> implement maxQueuedRequestsDevice as it would be two implementations for\n> basically the same thing. Could you explain your thoughts? I seem to be\n> missing something.\n\nAs Laurent has pointed out in the other reply, -EAGAIN would end\nup in code duplication *if* the only use case is to keep the\nrequest queue in check - from overflow.\n\nMy idea was to keep the PipelineHandler base simple, and add a facility\nto let individual pipeline handler dictate - whether or not they want\nto consume more waiting requests at the moment, by returning -EAGAIN.\nIt could have been based on a no. of factors - the individual\npipeline-handler would/can decide.\n\nYou can go ahead with maxQueuedRequestsDevice as that's the simplest\nand only use case we have right now. We can always revisit this,\nif there are clear use cases supporting this case. I don't have any\nright now.\n\n> \n> Best regards,\n> Stefan\n> \n> > \n> > > \n> > > Best regards,\n> > > Stefan\n> > > \n> > > > I think I need to try that out.\n> > > > \n> > > > Best regards,\n> > > > Stefan\n> > > > \n> > > > > \n> > > > > Signed-off-by: Umang Jain \n> > > > > ---\n> > > > > include/libcamera/internal/pipeline_handler.h | 2 +-\n> > > > > src/libcamera/pipeline_handler.cpp | 20 ++++++++++++++-----\n> > > > > 2 files changed, 16 insertions(+), 6 deletions(-)\n> > > > > \n> > > > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > > > > index 972a2fa6..35252e3a 100644\n> > > > > --- a/include/libcamera/internal/pipeline_handler.h\n> > > > > +++ b/include/libcamera/internal/pipeline_handler.h\n> > > > > @@ -88,7 +88,7 @@ private:\n> > > > > void mediaDeviceDisconnected(MediaDevice *media);\n> > > > > virtual void disconnect();\n> > > > > \n> > > > > - void doQueueRequest(Request *request);\n> > > > > + int doQueueRequest(Request *request);\n> > > > > void doQueueRequests();\n> > > > > \n> > > > > std::vector> mediaDevices_;\n> > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > > > index d84dff3c..2ce093ff 100644\n> > > > > --- a/src/libcamera/pipeline_handler.cpp\n> > > > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > > > @@ -452,8 +452,10 @@ void PipelineHandler::queueRequest(Request *request)\n> > > > > /**\n> > > > > * \\brief Queue one requests to the device\n> > > > > */\n> > > > > -void PipelineHandler::doQueueRequest(Request *request)\n> > > > > +int PipelineHandler::doQueueRequest(Request *request)\n> > > > > {\n> > > > > + int ret;\n> > > > > +\n> > > > > LIBCAMERA_TRACEPOINT(request_device_queue, request);\n> > > > > \n> > > > > Camera *camera = request->_d()->camera();\n> > > > > @@ -464,12 +466,14 @@ void PipelineHandler::doQueueRequest(Request *request)\n> > > > > \n> > > > > if (request->_d()->cancelled_) {\n> > > > > completeRequest(request);\n> > > > > - return;\n> > > > > + return -ECANCELED;\n> > > > > }\n> > > > > \n> > > > > - int ret = queueRequestDevice(camera, request);\n> > > > > - if (ret)\n> > > > > + ret = queueRequestDevice(camera, request);\n> > > > > + if (ret && ret != -EAGAIN)\n> > > > > cancelRequest(request);\n> > > > > +\n> > > > > + return ret;\n> > > > > }\n> > > > > \n> > > > > /**\n> > > > > @@ -485,7 +489,9 @@ void PipelineHandler::doQueueRequests()\n> > > > > if (!request->_d()->prepared_)\n> > > > > break;\n> > > > > \n> > > > > - doQueueRequest(request);\n> > > > > + if (doQueueRequest(request) == -EAGAIN)\n> > > > > + break;\n> > > > > +\n> > > > > waitingRequests_.pop();\n> > > > > }\n> > > > > }\n> > > > > @@ -502,6 +508,10 @@ void PipelineHandler::doQueueRequests()\n> > > > > * parameters will be applied to the frames captured in the buffers provided in\n> > > > > * the request.\n> > > > > *\n> > > > > + * If the underlying hardware pipeline is saturated with requests, this\n> > > > > + * function returns -EAGAIN, so that the \\a request stays in the internal\n> > > > > + * waiting queue.\n> > > > > + *\n> > > > > * \\context This function is called from the CameraManager thread.\n> > > > > *\n> > > > > * \\return 0 on success or a negative error code otherwise\n> > > > > -- \n> > > > > 2.50.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 2D497BDCBF\n\tfor ;\n\tTue, 1 Jul 2025 15:44:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6186668E2A;\n\tTue, 1 Jul 2025 17:44:27 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56DE468E1E\n\tfor ;\n\tTue, 1 Jul 2025 17:44:26 +0200 (CEST)","from [49.36.69.141] (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 1uWd9k-00B1v1-RZ; Tue, 01 Jul 2025 17:44:25 +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=\"nC2Siax3\"; 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=voHwbfaNrJPP06p1LSo2hh+/ZZ41zVidt7kIJbgYHmg=;\n\tb=nC2Siax3KS/lfG/qSUow57Gyq2\n\tDtIfM6XVfvR8CoM7eMRIUTJf7rC+gPqgn5F1nslIuaC5IaCpNKptNFn2AjUleyzMDo2orTjCPVHjE\n\twwT6ZkwvdofHPhF7w7oh28mluvm17lfiyNf8cnPAaF6tuDvmOyd6YShTHIOgQ1BaXmJHFb7izZykm\n\tDwFtfJ8nPQuuO+rKx0FN2+tPh42n7Xv3sTvYdMArlghSWvT4+DgFrZ+xZxbNwyq3sA45N16RGUajc\n\tioxZV+gfHVOm+f0/iX4uzhWlUJsa1PySwF2Rgx/PwAbZFFF1EETw5DsKmsiQKUsKDAGuIgGXm5E8p\n\tbJbRmGDA==;","Date":"Tue, 1 Jul 2025 21:14:26 +0530","From":"Umang Jain ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: pipeline_hander: Return -EAGAIN if pipeline\n\tis saturated","Message-ID":"","References":"<20250701045805.15201-1-uajain@igalia.com>\n\t<175135679502.2426344.2083856518536494275@localhost>\n\t<175135842398.2426344.5758856435692493490@localhost>\n\t\n\t<175137604423.2426344.7329689415295958835@localhost>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<175137604423.2426344.7329689415295958835@localhost>","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\" "}}]