[{"id":36393,"web_url":"https://patchwork.libcamera.org/comment/36393/","msgid":"<aad5178f9761d3c49266753d74003f96@igalia.com>","date":"2025-10-22T14:46:07","subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Stefan,\n\nOn 2025-10-22 13:29, Stefan Klug wrote:\n> The requestComplete signal is not emitted when the camera is stopped and\n> the request is still in the waitingRequests_ queue. This was reported in\n> [1]. Fix that by calling doQueueRequest() on the waiting requests after\n> marking them as cancelled. This ensures that the request is not queued\n> to the device but gets added to the queuedRequests_ list. This list is\n> then iterated in completeRequest() and leads to the requestComplete\n> signal.\n> \n> [1] https://gitlab.freedesktop.org/camera/libcamera/-/issues/281\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline_handler.cpp | 10 +++++++++-\n>  1 file changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index e5f9e55c9783..3aa814069382 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -380,7 +380,15 @@ void PipelineHandler::stop(Camera *camera)\n>  \twhile (!waitingRequests.empty()) {\n>  \t\tRequest *request = waitingRequests.front();\n>  \t\twaitingRequests.pop();\n> -\t\tcancelRequest(request);\n> +\n> +\t\t/*\n> +\t\t * Cancel all requests by marking them as cancelled and calling\n> +\t\t * doQueueRequest() instead of cancelRequest() to ensure the\n> +\t\t * request is temporarily added to queuedRequests_ so it can\n> +\t\t * then be properly completed in completeRequest().\n> +\t\t */\n> +\t\trequest->_d()->cancel();\n> +\t\tdoQueueRequest(request);\n\nCan't you simple call Camera::requestComplete() here? I don't like the\nidea of putting cancelled requests (from waiting queue) in\nqueuedRequests_.\n\nWith that you will still be able to complete in-order, as at this point,\nqueuedRequests_ is definitely empty, if I am reading this right.\n\n\n>  \t}\n>  \n>  \t/* Make sure no requests are pending. */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 543E0BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 22 Oct 2025 14:46:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3DB7607AF;\n\tWed, 22 Oct 2025 16:46:13 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A8E860763\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Oct 2025 16:46:10 +0200 (CEST)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vBa6L-00D93m-TZ; Wed, 22 Oct 2025 16:46:09 +0200","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vBa6J-00AGZy-He; Wed, 22 Oct 2025 16:46:09 +0200","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from <uajain@igalia.com>)\n\tid 1vBa6I-0001pR-38; Wed, 22 Oct 2025 16:46:07 +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=\"nAt+YXW7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version:Sender:Reply-To: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=bREONgroHp1TcEeASfLbDUNUv0etLqfodnH5zzna5pM=;\n\tb=nAt+YXW72vvgexZYBjrJ6S0PVo\n\tboBJPmH653RyIgEQkR3O49MtGEuDRUdG0EsQQa13loPfSEC+SzseBNBZuQS8++E3lhxd/AkzKipIy\n\t2jsD7FjP3njSQGBow2HBGoMDwyd4w1fwFf2qRfmfBycgWbSb3MxM+Hg1eqVb1gKuQuALFPHA8Y0OJ\n\tHHSNOYkY3qGGAfZdFeVdZiz5YHF2GEW5e/+HWe+gAC3qxmWgBrfylxNl+eyNxy7xH/QOPU7tltDpO\n\t/bwkxImJovwHAT2JPuLWZ8xa7WY4nXBRJNtqk60EI1ByKEpQDTGZH7/WhobBVX24iU1yojDoHtdm+\n\tGkFB4gcw==;","MIME-Version":"1.0","Date":"Wed, 22 Oct 2025 20:16:07 +0530","From":"uajain <uajain@igalia.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","In-Reply-To":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>","References":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>","Message-ID":"<aad5178f9761d3c49266753d74003f96@igalia.com>","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Spam-Report":"NO, Score=-2.2, Tests=ALL_TRUSTED=-3, BAYES_50=0.8,\n\tURIBL_ZEN_BLOCKED_OPENDNS=0.001","X-Spam-Score":"-21","X-Spam-Bar":"--","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36394,"web_url":"https://patchwork.libcamera.org/comment/36394/","msgid":"<176115169123.6155.8734094858620295443@localhost>","date":"2025-10-22T16:48:11","subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","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 uajain (2025-10-22 16:46:07)\n> Hi Stefan,\n> \n> On 2025-10-22 13:29, Stefan Klug wrote:\n> > The requestComplete signal is not emitted when the camera is stopped and\n> > the request is still in the waitingRequests_ queue. This was reported in\n> > [1]. Fix that by calling doQueueRequest() on the waiting requests after\n> > marking them as cancelled. This ensures that the request is not queued\n> > to the device but gets added to the queuedRequests_ list. This list is\n> > then iterated in completeRequest() and leads to the requestComplete\n> > signal.\n> > \n> > [1] https://gitlab.freedesktop.org/camera/libcamera/-/issues/281\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > ---\n> >  src/libcamera/pipeline_handler.cpp | 10 +++++++++-\n> >  1 file changed, 9 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index e5f9e55c9783..3aa814069382 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -380,7 +380,15 @@ void PipelineHandler::stop(Camera *camera)\n> >       while (!waitingRequests.empty()) {\n> >               Request *request = waitingRequests.front();\n> >               waitingRequests.pop();\n> > -             cancelRequest(request);\n> > +\n> > +             /*\n> > +              * Cancel all requests by marking them as cancelled and calling\n> > +              * doQueueRequest() instead of cancelRequest() to ensure the\n> > +              * request is temporarily added to queuedRequests_ so it can\n> > +              * then be properly completed in completeRequest().\n> > +              */\n> > +             request->_d()->cancel();\n> > +             doQueueRequest(request);\n> \n> Can't you simple call Camera::requestComplete() here? I don't like the\n> idea of putting cancelled requests (from waiting queue) in\n> queuedRequests_.\n> \n> With that you will still be able to complete in-order, as at this point,\n> queuedRequests_ is definitely empty, if I am reading this right.\n\nYes, I was considering that, too. I think it would work. I thought it\nwould be nice if the request sequence would be filled and I didn't\nreally check if I need to do additional checks. I'm fine with either\ncase. Maybe there are other opinions?\n\nBest regards,\nStefan\n\n> \n> \n> >       }\n> >  \n> >       /* Make sure no requests are pending. */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D083DC3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 22 Oct 2025 16:48:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1A81607AE;\n\tWed, 22 Oct 2025 18:48:14 +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 A3603606B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Oct 2025 18:48:13 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:743e:771f:a1ba:58db])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 551E08FA;\n\tWed, 22 Oct 2025 18:46:29 +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=\"oWkCWgGu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761151589;\n\tbh=rP6cn6dIdk0P9CeCA5vHRZirbKJRKwq+QO1zYI8Aze8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=oWkCWgGuDKQ8XQEReiHjXwwim0EtdqLPsJwzfdPZA06EMDBsmx+DkTAGxQUuB9eyD\n\tj0XPDhtrldSDzN4b+BrZ3ffFeJcuhOF9lrJh5ZoqrmaEeIzGL3YUdytUEUjrmzyM/R\n\t1JZtZnhFC3wjvng2GFS8JFNJqVOE1xgFvs7NhlcE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<aad5178f9761d3c49266753d74003f96@igalia.com>","References":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>\n\t<aad5178f9761d3c49266753d74003f96@igalia.com>","Subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","From":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","To":"uajain <uajain@igalia.com>","Date":"Wed, 22 Oct 2025 18:48:11 +0200","Message-ID":"<176115169123.6155.8734094858620295443@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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36400,"web_url":"https://patchwork.libcamera.org/comment/36400/","msgid":"<096e4214-cdb8-4d00-9695-7e22106467d5@ideasonboard.com>","date":"2025-10-23T08:51:18","subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 10. 22. 18:48 keltezéssel, Stefan Klug írta:\n> Hi Umang,\n> \n> Thank you for the review.\n> \n> Quoting uajain (2025-10-22 16:46:07)\n>> Hi Stefan,\n>>\n>> On 2025-10-22 13:29, Stefan Klug wrote:\n>>> The requestComplete signal is not emitted when the camera is stopped and\n>>> the request is still in the waitingRequests_ queue. This was reported in\n>>> [1]. Fix that by calling doQueueRequest() on the waiting requests after\n>>> marking them as cancelled. This ensures that the request is not queued\n>>> to the device but gets added to the queuedRequests_ list. This list is\n>>> then iterated in completeRequest() and leads to the requestComplete\n>>> signal.\n>>>\n>>> [1] https://gitlab.freedesktop.org/camera/libcamera/-/issues/281\n\nI think we should select one of https://docs.gitlab.com/user/project/issues/managing_issues/#default-closing-pattern\nand use that so the issues are automatically closed.\n\n\n>>>\n>>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n>>> ---\n>>>   src/libcamera/pipeline_handler.cpp | 10 +++++++++-\n>>>   1 file changed, 9 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>> index e5f9e55c9783..3aa814069382 100644\n>>> --- a/src/libcamera/pipeline_handler.cpp\n>>> +++ b/src/libcamera/pipeline_handler.cpp\n>>> @@ -380,7 +380,15 @@ void PipelineHandler::stop(Camera *camera)\n>>>        while (!waitingRequests.empty()) {\n>>>                Request *request = waitingRequests.front();\n>>>                waitingRequests.pop();\n>>> -             cancelRequest(request);\n>>> +\n>>> +             /*\n>>> +              * Cancel all requests by marking them as cancelled and calling\n>>> +              * doQueueRequest() instead of cancelRequest() to ensure the\n>>> +              * request is temporarily added to queuedRequests_ so it can\n>>> +              * then be properly completed in completeRequest().\n>>> +              */\n>>> +             request->_d()->cancel();\n>>> +             doQueueRequest(request);\n>>\n>> Can't you simple call Camera::requestComplete() here? I don't like the\n>> idea of putting cancelled requests (from waiting queue) in\n>> queuedRequests_.\n>>\n>> With that you will still be able to complete in-order, as at this point,\n>> queuedRequests_ is definitely empty, if I am reading this right.\n> \n> Yes, I was considering that, too. I think it would work. I thought it\n> would be nice if the request sequence would be filled and I didn't\n> really check if I need to do additional checks. I'm fine with either\n> case. Maybe there are other opinions?\n\nI suppose you could have a `completeRequests()` function that does something like:\n\n   completeRequests(auto &c) {\n     while (!c.empty()) {\n       req = c.front();\n       if (req->status() == Pending) break;\n       c.pop_front();\n       camera->requestComplete(req);\n     }\n   }\n\nso mainly what `PipelineHandler::completeRequest()` does but on an arbitrary\ncontainer of requests. Then `PipelineHandler::stop()` could cancel every waiting\nrequest, and complete all of them with the above function.\n\nAlthough this won't set a sequence number on the request. Reading the documentation\nof `Requests::sequence()` seems to suggest that libcamera should guarantee a proper\nrequest sequence number for every request that was successfully queued, which includes\nthese waiting requests.\n\nInitially I also didn't particularly like the fact the these requests are added\nto `queuedRequests_`, but the more I look at it, the more I like the fact that\nthese requests also go through the same \"pipeline\" as the other ones. And the\nproposed change is also quite short and simple. Nonetheless, I think it could\nstill be argued that a future change could (easily?) break it by accident.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Best regards,\n> Stefan\n> \n>>\n>>\n>>>        }\n>>>   \n>>>        /* Make sure no requests are pending. */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 85DCCBE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 23 Oct 2025 08:51:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A1E5607B3;\n\tThu, 23 Oct 2025 10:51:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2589607AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 Oct 2025 10:51:21 +0200 (CEST)","from [192.168.33.33] (185.221.141.231.nat.pool.zt.hu\n\t[185.221.141.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98531177F;\n\tThu, 23 Oct 2025 10:49:36 +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=\"NeU5RcNq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761209376;\n\tbh=66USQhMgM98ziszVYjJMV3IVxBg5d5OSzXKkW/QOIyo=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=NeU5RcNqPVgJNJeEkCB8d9f54wirXngm2l2LwUPB+imeZUobP95TyrKMzVNTkxG/L\n\tGe/91QrM6HmcL/CEGqOajNBK7Eh3xQI9PXzieNWIWUnrQfU+kToYIXRxeQ7xPv6vvT\n\tkj36R6xggyRSOeTSEo8eLEi40IuZNQ0pDtuhw9l0=","Message-ID":"<096e4214-cdb8-4d00-9695-7e22106467d5@ideasonboard.com>","Date":"Thu, 23 Oct 2025 10:51:18 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","To":"Stefan Klug <stefan.klug@ideasonboard.com>, uajain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>\n\t<aad5178f9761d3c49266753d74003f96@igalia.com>\n\t<176115169123.6155.8734094858620295443@localhost>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<176115169123.6155.8734094858620295443@localhost>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36403,"web_url":"https://patchwork.libcamera.org/comment/36403/","msgid":"<20251023105624.GI19043@pendragon.ideasonboard.com>","date":"2025-10-23T10:56:24","subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Oct 23, 2025 at 10:51:18AM +0200, Barnabás Pőcze wrote:\n> Hi\n> \n> 2025. 10. 22. 18:48 keltezéssel, Stefan Klug írta:\n> > Hi Umang,\n> > \n> > Thank you for the review.\n> > \n> > Quoting uajain (2025-10-22 16:46:07)\n> >> Hi Stefan,\n> >>\n> >> On 2025-10-22 13:29, Stefan Klug wrote:\n> >>> The requestComplete signal is not emitted when the camera is stopped and\n> >>> the request is still in the waitingRequests_ queue. This was reported in\n> >>> [1]. Fix that by calling doQueueRequest() on the waiting requests after\n> >>> marking them as cancelled. This ensures that the request is not queued\n> >>> to the device but gets added to the queuedRequests_ list. This list is\n> >>> then iterated in completeRequest() and leads to the requestComplete\n> >>> signal.\n> >>>\n> >>> [1] https://gitlab.freedesktop.org/camera/libcamera/-/issues/281\n> \n> I think we should select one of https://docs.gitlab.com/user/project/issues/managing_issues/#default-closing-pattern\n> and use that so the issues are automatically closed.\n\nI'd like that to go in commit message trailers, with a full URL. That\ndoesn't match the default closing pattern. Apparently it can be\ncustomized, but not per repository.\n\n> >>>\n> >>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> >>> ---\n> >>>   src/libcamera/pipeline_handler.cpp | 10 +++++++++-\n> >>>   1 file changed, 9 insertions(+), 1 deletion(-)\n> >>>\n> >>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >>> index e5f9e55c9783..3aa814069382 100644\n> >>> --- a/src/libcamera/pipeline_handler.cpp\n> >>> +++ b/src/libcamera/pipeline_handler.cpp\n> >>> @@ -380,7 +380,15 @@ void PipelineHandler::stop(Camera *camera)\n> >>>        while (!waitingRequests.empty()) {\n> >>>                Request *request = waitingRequests.front();\n> >>>                waitingRequests.pop();\n> >>> -             cancelRequest(request);\n> >>> +\n> >>> +             /*\n> >>> +              * Cancel all requests by marking them as cancelled and calling\n> >>> +              * doQueueRequest() instead of cancelRequest() to ensure the\n> >>> +              * request is temporarily added to queuedRequests_ so it can\n> >>> +              * then be properly completed in completeRequest().\n> >>> +              */\n> >>> +             request->_d()->cancel();\n> >>> +             doQueueRequest(request);\n> >>\n> >> Can't you simple call Camera::requestComplete() here? I don't like the\n> >> idea of putting cancelled requests (from waiting queue) in\n> >> queuedRequests_.\n> >>\n> >> With that you will still be able to complete in-order, as at this point,\n> >> queuedRequests_ is definitely empty, if I am reading this right.\n> > \n> > Yes, I was considering that, too. I think it would work. I thought it\n> > would be nice if the request sequence would be filled and I didn't\n> > really check if I need to do additional checks. I'm fine with either\n> > case. Maybe there are other opinions?\n> \n> I suppose you could have a `completeRequests()` function that does something like:\n> \n>    completeRequests(auto &c) {\n>      while (!c.empty()) {\n>        req = c.front();\n>        if (req->status() == Pending) break;\n>        c.pop_front();\n>        camera->requestComplete(req);\n>      }\n>    }\n> \n> so mainly what `PipelineHandler::completeRequest()` does but on an arbitrary\n> container of requests. Then `PipelineHandler::stop()` could cancel every waiting\n> request, and complete all of them with the above function.\n> \n> Although this won't set a sequence number on the request. Reading the documentation\n> of `Requests::sequence()` seems to suggest that libcamera should guarantee a proper\n> request sequence number for every request that was successfully queued, which includes\n> these waiting requests.\n\nYes, we should always have a proper sequence number in requests.\n\n> Initially I also didn't particularly like the fact the these requests are added\n> to `queuedRequests_`, but the more I look at it, the more I like the fact that\n> these requests also go through the same \"pipeline\" as the other ones. And the\n> proposed change is also quite short and simple. Nonetheless, I think it could\n> still be argued that a future change could (easily?) break it by accident.\n\nGoing through the same code path for completion of all requests would be\nmy preferred option.\n\n> >>>        }\n> >>>   \n> >>>        /* Make sure no requests are pending. */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E35C5C3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 23 Oct 2025 10:56:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB558607BC;\n\tThu, 23 Oct 2025 12:56:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D6E2607B3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 Oct 2025 12:56:37 +0200 (CEST)","from pendragon.ideasonboard.com (82-203-161-16.bb.dnainternet.fi\n\t[82.203.161.16])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 52015591;\n\tThu, 23 Oct 2025 12:54:52 +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=\"mlMhUV9b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761216892;\n\tbh=l7rSXyUjQLCDMtSf+99qqKztxWmdTnWq4ImraMA7iVE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mlMhUV9b4KZJY7EN0D1iBZDVevBdKuN3Mzssy9xKajd4Xz6prARZDjUXslKfFm1Gq\n\tnutbUQYqfvwHNr0MpRUapYEduhtZ4fDQpWkkgTuIcVpPwWRAH2bJkKEJ75hu4goo/q\n\tJz/M2hkQBrh9OTwiKTrr+Y15aGUdnlCoHJbYJbuE=","Date":"Thu, 23 Oct 2025 13:56:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>, uajain <uajain@igalia.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","Message-ID":"<20251023105624.GI19043@pendragon.ideasonboard.com>","References":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>\n\t<aad5178f9761d3c49266753d74003f96@igalia.com>\n\t<176115169123.6155.8734094858620295443@localhost>\n\t<096e4214-cdb8-4d00-9695-7e22106467d5@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<096e4214-cdb8-4d00-9695-7e22106467d5@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36475,"web_url":"https://patchwork.libcamera.org/comment/36475/","msgid":"<a76fadc0-31ba-460d-9656-e30ba84a13d7@ideasonboard.com>","date":"2025-10-27T08:34:18","subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 10. 23. 12:56 keltezéssel, Laurent Pinchart írta:\n> On Thu, Oct 23, 2025 at 10:51:18AM +0200, Barnabás Pőcze wrote:\n>> Hi\n>>\n>> 2025. 10. 22. 18:48 keltezéssel, Stefan Klug írta:\n>>> Hi Umang,\n>>>\n>>> Thank you for the review.\n>>>\n>>> Quoting uajain (2025-10-22 16:46:07)\n>>>> Hi Stefan,\n>>>>\n>>>> On 2025-10-22 13:29, Stefan Klug wrote:\n>>>>> The requestComplete signal is not emitted when the camera is stopped and\n>>>>> the request is still in the waitingRequests_ queue. This was reported in\n>>>>> [1]. Fix that by calling doQueueRequest() on the waiting requests after\n>>>>> marking them as cancelled. This ensures that the request is not queued\n>>>>> to the device but gets added to the queuedRequests_ list. This list is\n>>>>> then iterated in completeRequest() and leads to the requestComplete\n>>>>> signal.\n>>>>>\n>>>>> [1] https://gitlab.freedesktop.org/camera/libcamera/-/issues/281\n>>\n>> I think we should select one of https://docs.gitlab.com/user/project/issues/managing_issues/#default-closing-pattern\n>> and use that so the issues are automatically closed.\n> \n> I'd like that to go in commit message trailers, with a full URL. That\n> doesn't match the default closing pattern. Apparently it can be\n> customized, but not per repository.\n\nBoth `Fixes: $url` and `Closes: $url` works, so it seems the `:` is not an issue.\nOr the issue is that you would like to use a different word?\n\n\n> \n>>>>>\n>>>>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n>>>>> ---\n>>>>>    src/libcamera/pipeline_handler.cpp | 10 +++++++++-\n>>>>>    1 file changed, 9 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>>>> index e5f9e55c9783..3aa814069382 100644\n>>>>> --- a/src/libcamera/pipeline_handler.cpp\n>>>>> +++ b/src/libcamera/pipeline_handler.cpp\n>>>>> @@ -380,7 +380,15 @@ void PipelineHandler::stop(Camera *camera)\n>>>>>         while (!waitingRequests.empty()) {\n>>>>>                 Request *request = waitingRequests.front();\n>>>>>                 waitingRequests.pop();\n>>>>> -             cancelRequest(request);\n>>>>> +\n>>>>> +             /*\n>>>>> +              * Cancel all requests by marking them as cancelled and calling\n>>>>> +              * doQueueRequest() instead of cancelRequest() to ensure the\n>>>>> +              * request is temporarily added to queuedRequests_ so it can\n>>>>> +              * then be properly completed in completeRequest().\n>>>>> +              */\n>>>>> +             request->_d()->cancel();\n>>>>> +             doQueueRequest(request);\n>>>>\n>>>> Can't you simple call Camera::requestComplete() here? I don't like the\n>>>> idea of putting cancelled requests (from waiting queue) in\n>>>> queuedRequests_.\n>>>>\n>>>> With that you will still be able to complete in-order, as at this point,\n>>>> queuedRequests_ is definitely empty, if I am reading this right.\n>>>\n>>> Yes, I was considering that, too. I think it would work. I thought it\n>>> would be nice if the request sequence would be filled and I didn't\n>>> really check if I need to do additional checks. I'm fine with either\n>>> case. Maybe there are other opinions?\n>>\n>> I suppose you could have a `completeRequests()` function that does something like:\n>>\n>>     completeRequests(auto &c) {\n>>       while (!c.empty()) {\n>>         req = c.front();\n>>         if (req->status() == Pending) break;\n>>         c.pop_front();\n>>         camera->requestComplete(req);\n>>       }\n>>     }\n>>\n>> so mainly what `PipelineHandler::completeRequest()` does but on an arbitrary\n>> container of requests. Then `PipelineHandler::stop()` could cancel every waiting\n>> request, and complete all of them with the above function.\n>>\n>> Although this won't set a sequence number on the request. Reading the documentation\n>> of `Requests::sequence()` seems to suggest that libcamera should guarantee a proper\n>> request sequence number for every request that was successfully queued, which includes\n>> these waiting requests.\n> \n> Yes, we should always have a proper sequence number in requests.\n> \n>> Initially I also didn't particularly like the fact the these requests are added\n>> to `queuedRequests_`, but the more I look at it, the more I like the fact that\n>> these requests also go through the same \"pipeline\" as the other ones. And the\n>> proposed change is also quite short and simple. Nonetheless, I think it could\n>> still be argued that a future change could (easily?) break it by accident.\n> \n> Going through the same code path for completion of all requests would be\n> my preferred option.\n> \n>>>>>         }\n>>>>>\n>>>>>         /* Make sure no requests are pending. */\n> \n> --\n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1A535C3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Oct 2025 08:34:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F094C6071E;\n\tMon, 27 Oct 2025 09:34:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1632760453\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Oct 2025 09:34:22 +0100 (CET)","from [192.168.33.25] (185.182.215.162.nat.pool.zt.hu\n\t[185.182.215.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A0FD1E9B;\n\tMon, 27 Oct 2025 09:32:33 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fk7azK/E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761553953;\n\tbh=kNjdaVw1sybXeumI6cLV8Z6cJ9UKK4eVWe5ZSwNehvk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=fk7azK/EJGru8bHfJ+QXAnQ3FFLqbx/xrUDON0zecpcFarAB/JViMOeZLYOuNcvCH\n\tePrj+8eRRXB5Zv92VkHSYt6TysOaf8ruknxoL9lopjcw9RFdRaO/RPxffRrmzNRpOu\n\tRYY3f4Cpg5qK9p9TxsW+i7FvRAG77SS7t4qNRcgE=","Message-ID":"<a76fadc0-31ba-460d-9656-e30ba84a13d7@ideasonboard.com>","Date":"Mon, 27 Oct 2025 09:34:18 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>, uajain <uajain@igalia.com>, \n\tlibcamera-devel@lists.libcamera.org","References":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>\n\t<aad5178f9761d3c49266753d74003f96@igalia.com>\n\t<176115169123.6155.8734094858620295443@localhost>\n\t<096e4214-cdb8-4d00-9695-7e22106467d5@ideasonboard.com>\n\t<spvSMig4gblyZ4Pw7kduHIjnwXe_x-QWih41XVxvo7h9gI_3Ul-dq6CD2RQjsFXIkfjDaLpNz7APVK5TR1XVyw==@protonmail.internalid>\n\t<20251023105624.GI19043@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251023105624.GI19043@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36476,"web_url":"https://patchwork.libcamera.org/comment/36476/","msgid":"<20251027085015.GA1544@pendragon.ideasonboard.com>","date":"2025-10-27T08:50:15","subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Oct 27, 2025 at 09:34:18AM +0100, Barnabás Pőcze wrote:\n> 2025. 10. 23. 12:56 keltezéssel, Laurent Pinchart írta:\n> > On Thu, Oct 23, 2025 at 10:51:18AM +0200, Barnabás Pőcze wrote:\n> >> 2025. 10. 22. 18:48 keltezéssel, Stefan Klug írta:\n> >>> Quoting uajain (2025-10-22 16:46:07)\n> >>>> On 2025-10-22 13:29, Stefan Klug wrote:\n> >>>>> The requestComplete signal is not emitted when the camera is stopped and\n> >>>>> the request is still in the waitingRequests_ queue. This was reported in\n> >>>>> [1]. Fix that by calling doQueueRequest() on the waiting requests after\n> >>>>> marking them as cancelled. This ensures that the request is not queued\n> >>>>> to the device but gets added to the queuedRequests_ list. This list is\n> >>>>> then iterated in completeRequest() and leads to the requestComplete\n> >>>>> signal.\n> >>>>>\n> >>>>> [1] https://gitlab.freedesktop.org/camera/libcamera/-/issues/281\n> >>\n> >> I think we should select one of https://docs.gitlab.com/user/project/issues/managing_issues/#default-closing-pattern\n> >> and use that so the issues are automatically closed.\n> > \n> > I'd like that to go in commit message trailers, with a full URL. That\n> > doesn't match the default closing pattern. Apparently it can be\n> > customized, but not per repository.\n> \n> Both `Fixes: $url` and `Closes: $url` works, so it seems the `:` is not an issue.\n> Or the issue is that you would like to use a different word?\n\nYou're right, I had missed the (:?) in the regex. It's all fine then.\nLet's standardize on using a trailer and a full url. If we want to\nfollow kernel conventions, it would be 'Closes:'. We can keep the Fixes:\ntag to reference a commit, as we do today.\n\n> >>>>>\n> >>>>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> >>>>> ---\n> >>>>>    src/libcamera/pipeline_handler.cpp | 10 +++++++++-\n> >>>>>    1 file changed, 9 insertions(+), 1 deletion(-)\n> >>>>>\n> >>>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >>>>> index e5f9e55c9783..3aa814069382 100644\n> >>>>> --- a/src/libcamera/pipeline_handler.cpp\n> >>>>> +++ b/src/libcamera/pipeline_handler.cpp\n> >>>>> @@ -380,7 +380,15 @@ void PipelineHandler::stop(Camera *camera)\n> >>>>>         while (!waitingRequests.empty()) {\n> >>>>>                 Request *request = waitingRequests.front();\n> >>>>>                 waitingRequests.pop();\n> >>>>> -             cancelRequest(request);\n> >>>>> +\n> >>>>> +             /*\n> >>>>> +              * Cancel all requests by marking them as cancelled and calling\n> >>>>> +              * doQueueRequest() instead of cancelRequest() to ensure the\n> >>>>> +              * request is temporarily added to queuedRequests_ so it can\n> >>>>> +              * then be properly completed in completeRequest().\n> >>>>> +              */\n> >>>>> +             request->_d()->cancel();\n> >>>>> +             doQueueRequest(request);\n> >>>>\n> >>>> Can't you simple call Camera::requestComplete() here? I don't like the\n> >>>> idea of putting cancelled requests (from waiting queue) in\n> >>>> queuedRequests_.\n> >>>>\n> >>>> With that you will still be able to complete in-order, as at this point,\n> >>>> queuedRequests_ is definitely empty, if I am reading this right.\n> >>>\n> >>> Yes, I was considering that, too. I think it would work. I thought it\n> >>> would be nice if the request sequence would be filled and I didn't\n> >>> really check if I need to do additional checks. I'm fine with either\n> >>> case. Maybe there are other opinions?\n> >>\n> >> I suppose you could have a `completeRequests()` function that does something like:\n> >>\n> >>     completeRequests(auto &c) {\n> >>       while (!c.empty()) {\n> >>         req = c.front();\n> >>         if (req->status() == Pending) break;\n> >>         c.pop_front();\n> >>         camera->requestComplete(req);\n> >>       }\n> >>     }\n> >>\n> >> so mainly what `PipelineHandler::completeRequest()` does but on an arbitrary\n> >> container of requests. Then `PipelineHandler::stop()` could cancel every waiting\n> >> request, and complete all of them with the above function.\n> >>\n> >> Although this won't set a sequence number on the request. Reading the documentation\n> >> of `Requests::sequence()` seems to suggest that libcamera should guarantee a proper\n> >> request sequence number for every request that was successfully queued, which includes\n> >> these waiting requests.\n> > \n> > Yes, we should always have a proper sequence number in requests.\n> > \n> >> Initially I also didn't particularly like the fact the these requests are added\n> >> to `queuedRequests_`, but the more I look at it, the more I like the fact that\n> >> these requests also go through the same \"pipeline\" as the other ones. And the\n> >> proposed change is also quite short and simple. Nonetheless, I think it could\n> >> still be argued that a future change could (easily?) break it by accident.\n> > \n> > Going through the same code path for completion of all requests would be\n> > my preferred option.\n> > \n> >>>>>         }\n> >>>>>\n> >>>>>         /* Make sure no requests are pending. */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 9B566BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Oct 2025 08:50:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9FCA460706;\n\tMon, 27 Oct 2025 09:50:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 34B6260453\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Oct 2025 09:50:29 +0100 (CET)","from pendragon.ideasonboard.com (unknown [193.209.96.36])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1EDC31661; \n\tMon, 27 Oct 2025 09:48:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"amJQZOHY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761554921;\n\tbh=5X35HQkq/ibZwsqm0AJZGhjmPAI4hzsezp2pKq93oLc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=amJQZOHYuUAbmicFnrfeEYuo/VIz4YXBIa+Ss0lCSnzZH+CBKpp6Y7krfIXb+LBUq\n\tCOZ3A8uOMHrOEO/b5IEwktemHs8r8CxbwfB5HGiQwplvVIK/4nnqRdKD1Va9MvNA/p\n\tKk52y7X6gbqLx6QO34xg4FtJC4+fBz47Z7zXAaLs=","Date":"Mon, 27 Oct 2025 10:50:15 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>, uajain <uajain@igalia.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","Message-ID":"<20251027085015.GA1544@pendragon.ideasonboard.com>","References":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>\n\t<aad5178f9761d3c49266753d74003f96@igalia.com>\n\t<176115169123.6155.8734094858620295443@localhost>\n\t<096e4214-cdb8-4d00-9695-7e22106467d5@ideasonboard.com>\n\t<spvSMig4gblyZ4Pw7kduHIjnwXe_x-QWih41XVxvo7h9gI_3Ul-dq6CD2RQjsFXIkfjDaLpNz7APVK5TR1XVyw==@protonmail.internalid>\n\t<20251023105624.GI19043@pendragon.ideasonboard.com>\n\t<a76fadc0-31ba-460d-9656-e30ba84a13d7@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<a76fadc0-31ba-460d-9656-e30ba84a13d7@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36677,"web_url":"https://patchwork.libcamera.org/comment/36677/","msgid":"<176226587576.2116251.1781647617092058508@neptunite.rasen.tech>","date":"2025-11-04T14:17:55","subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-10-22 16:59:11)\n> The requestComplete signal is not emitted when the camera is stopped and\n> the request is still in the waitingRequests_ queue. This was reported in\n> [1]. Fix that by calling doQueueRequest() on the waiting requests after\n> marking them as cancelled. This ensures that the request is not queued\n> to the device but gets added to the queuedRequests_ list. This list is\n> then iterated in completeRequest() and leads to the requestComplete\n> signal.\n> \n> [1] https://gitlab.freedesktop.org/camera/libcamera/-/issues/281\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline_handler.cpp | 10 +++++++++-\n>  1 file changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index e5f9e55c9783..3aa814069382 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -380,7 +380,15 @@ void PipelineHandler::stop(Camera *camera)\n>         while (!waitingRequests.empty()) {\n>                 Request *request = waitingRequests.front();\n>                 waitingRequests.pop();\n> -               cancelRequest(request);\n> +\n> +               /*\n> +                * Cancel all requests by marking them as cancelled and calling\n> +                * doQueueRequest() instead of cancelRequest() to ensure the\n> +                * request is temporarily added to queuedRequests_ so it can\n> +                * then be properly completed in completeRequest().\n> +                */\n> +               request->_d()->cancel();\n> +               doQueueRequest(request);\n\nOn first sight it looked hacky, but the more I looked at it it actually looks\npretty good; it forces the canceled request to go through the same proper paths\na regular canceled request would've taken were it not for the stop() call. I\nthink the doQueueRequest() call itself is what makes this look hacky, but the\ncomment covers it.\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n>         }\n>  \n>         /* Make sure no requests are pending. */\n> -- \n> 2.48.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 CB0AFC3241\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  4 Nov 2025 14:18:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE3B160A80;\n\tTue,  4 Nov 2025 15:18:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EF3716069A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  4 Nov 2025 15:18:03 +0100 (CET)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:617:a3ac:af02:feab])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id C8ABBEB7;\n\tTue,  4 Nov 2025 15:16:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"GfInwHFn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762265770;\n\tbh=PIORUVT1h+FjdBGHTuNQqTxvEceTNXlsafUdLWjmRiE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GfInwHFneyzLHT1+le5L4AOzZ9tPVY6ubKEcxQLBSictSU4CL4YgOTx6S5384tDRq\n\tzCLsuhujRGpVA8QCg4+Z5Q2dEJVyoXLTEp3lZFW9IIt4nQ/Qx3Y/3Ll07ZzMg0BWZA\n\tBP95NYPkO4Yy0vqdpcEd794tAky0BbyMfLOHGgxg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>","References":"<20251022080011.97665-1-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH] libcamera: pipeline_handler: Fix requestComplete on\n\twaiting requests on stop","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 04 Nov 2025 23:17:55 +0900","Message-ID":"<176226587576.2116251.1781647617092058508@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]