[{"id":35752,"web_url":"https://patchwork.libcamera.org/comment/35752/","msgid":"","date":"2025-09-09T14:34:49","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Hi,\n\nLe mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n> The moment execution enters `Camera::queueRequest()`, an application must be\n> prepared to received the corresponding `requestCompleted` signal. However,\n> the gstreamer element is currently not prepared: it queues the request,\n> takes a lock, then inserts into a list.\n> \n> If the request completion handler acquires the lock right after the queueing,\n> then it will not find the object in the list that it expects. Even potentially\n> encountering an empty list, running into undefined behaviour when trying\n> to pop from it.\n> \n> Fix that by queueing the request after the lock protected insertion.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n> Signed-off-by: Barnabás Pőcze \n> ---\n> \n> Doing it while holding the lock is not ideal, but the alternative is\n> getting a raw pointer to the `Request` object beforehand, but that does\n> not seem too desirable either.\n\nIf we are worried calling into libcamera with uur own lock held, have you\nconsidered pushing it into the list before calling cam_->queueRequest() ? The\ncamera object in libcamera is supposed to be threadsafe, and we should have\nstrong ownership on the state and request wrap (if not some reffing is missing I\nguess).\n\nNicolas\n\np.s. the lock is dangerous if libcamera decides to callback from the caller\nthread instead of a separate thread. So calling with lock is implementation\ndependant.\n\n> \n> ---\n>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> b/src/gstreamer/gstlibcamerasrc.cpp\n> index 79a025a57..103dfbca2 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>  \t}\n> \n>  \tGST_TRACE_OBJECT(src_, \"Requesting buffers\");\n> -\tcam_->queueRequest(wrap->request_.get());\n> \n>  \t{\n>  \t\tGLibLocker locker(&lock_);\n> +\t\tcam_->queueRequest(wrap->request_.get());\n>  \t\tqueuedRequests_.push(std::move(wrap));\n>  \t}\n> \n> --\n> 2.51.0","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 AEC85C324E\n\tfor ;\n\tTue, 9 Sep 2025 14:34:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70CD46936D;\n\tTue, 9 Sep 2025 16:34:55 +0200 (CEST)","from bali.collaboradmins.com (bali.collaboradmins.com\n\t[148.251.105.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7C566934B\n\tfor ;\n\tTue, 9 Sep 2025 16:34:52 +0200 (CEST)","from [IPv6:2606:6d00:15:d961::5ac] (unknown\n\t[IPv6:2606:6d00:15:d961::5ac])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bali.collaboradmins.com (Postfix) with ESMTPSA id C837D17E00A3;\n\tTue, 9 Sep 2025 16:34:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"GnLAeEJb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1757428492;\n\tbh=KRwy5rxuCLHqLBoth7ok4oWI7ijGqiN+WKAK7CXP6bE=;\n\th=Subject:From:To:Date:In-Reply-To:References:From;\n\tb=GnLAeEJbmPVLN5AAKQj7vdkzjYHwoHEGPvGoQCrpJEXihcS7Wrpc29cUIU58fMIyr\n\tHs3hTdyqp0aYe31ZdUV15YUHxNL2zqVadzA6bxkhyNBPkevZnnQs4Yanx7nwqqdiv4\n\tfnOuR+5uuWGRjS5PPbAGDvydL7pOEdM8DnX38DEuEhbbe51C2MTySs/KYaOaOTXpH3\n\trw+t8zD1rpJErJOlK9Fus3FQykBg4ouyHfSUtoYsXhoxWdtpkdRmL9oXjHcv6kAOF0\n\tHVrNGp0nvyaIpEVLY0nz7N9PXmlvN6e3j8eNeY+tLMHWOsaYN0BvM1usejMpHm6+R6\n\tlw2UuMP/7DDmQ==","Message-ID":"","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"Nicolas Dufresne ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, Umang Jain ","Date":"Tue, 09 Sep 2025 10:34:49 -0400","In-Reply-To":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>","Autocrypt":"addr=nicolas.dufresne@collabora.com; prefer-encrypt=mutual;\n\tkeydata=mDMEaCN2ixYJKwYBBAHaRw8BAQdAM0EHepTful3JOIzcPv6ekHOenE1u0vDG1gdHFrChD\n\t/e0J05pY29sYXMgRHVmcmVzbmUgPG5pY29sYXNAbmR1ZnJlc25lLmNhPoicBBMWCgBEAhsDBQsJCA\n\tcCAiICBhUKCQgLAgQWAgMBAh4HAheABQkJZfd1FiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrjo\n\tCGQEACgkQ2UGUUSlgcvQlQwD/RjpU1SZYcKG6pnfnQ8ivgtTkGDRUJ8gP3fK7+XUjRNIA/iXfhXMN\n\tabIWxO2oCXKf3TdD7aQ4070KO6zSxIcxgNQFtDFOaWNvbGFzIER1ZnJlc25lIDxuaWNvbGFzLmR1Z\n\tnJlc25lQGNvbGxhYm9yYS5jb20+iJkEExYKAEECGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4\n\tAWIQTvDVKBFcTDwhoEbxLZQZRRKWBy9AUCaCyyxgUJCWX3dQAKCRDZQZRRKWBy9ARJAP96pFmLffZ\n\tsmBUpkyVBfFAf+zq6BJt769R0al3kHvUKdgD9G7KAHuioxD2v6SX7idpIazjzx8b8rfzwTWyOQWHC\n\tAAS0LU5pY29sYXMgRHVmcmVzbmUgPG5pY29sYXMuZHVmcmVzbmVAZ21haWwuY29tPoiZBBMWCgBBF\n\tiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrGYCGwMFCQll93UFCwkIBwICIgIGFQoJCAsCBBYCAw\n\tECHgcCF4AACgkQ2UGUUSlgcvRObgD/YnQjfi4+L8f4fI7p1pPMTwRTcaRdy6aqkKEmKsCArzQBAK8\n\tbRLv9QjuqsE6oQZra/RB4widZPvphs78H0P6NmpIJ","Organization":"Collabora Canada","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-ldjPPhwotem12rrIOHI6\"","User-Agent":"Evolution 3.56.2 (3.56.2-1.fc42) ","MIME-Version":"1.0","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":35765,"web_url":"https://patchwork.libcamera.org/comment/35765/","msgid":"","date":"2025-09-10T14:01:15","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","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. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n> Hi,\n> \n> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n>> The moment execution enters `Camera::queueRequest()`, an application must be\n>> prepared to received the corresponding `requestCompleted` signal. However,\n>> the gstreamer element is currently not prepared: it queues the request,\n>> takes a lock, then inserts into a list.\n>>\n>> If the request completion handler acquires the lock right after the queueing,\n>> then it will not find the object in the list that it expects. Even potentially\n>> encountering an empty list, running into undefined behaviour when trying\n>> to pop from it.\n>>\n>> Fix that by queueing the request after the lock protected insertion.\n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>>\n>> Doing it while holding the lock is not ideal, but the alternative is\n>> getting a raw pointer to the `Request` object beforehand, but that does\n>> not seem too desirable either.\n> \n> If we are worried calling into libcamera with uur own lock held, have you\n> considered pushing it into the list before calling cam_->queueRequest() ? The\n> camera object in libcamera is supposed to be threadsafe, and we should have\n> strong ownership on the state and request wrap (if not some reffing is missing I\n> guess).\n\nThe `RequestWrap` pointer is moved into the container, so one would need to take\na raw pointer to the `Request`. The fact that the request completions handler\nunconditionally pops the first element worries me a bit. Although if things have\nalready gone out of sync there, it might not matter.\n\n\n> \n> Nicolas\n> \n> p.s. the lock is dangerous if libcamera decides to callback from the caller\n> thread instead of a separate thread. So calling with lock is implementation\n> dependant.\n\nThat's true, but currently it is guaranteed. The application developer guide\nmentions it explicitly.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>>\n>> ---\n>>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>> b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 79a025a57..103dfbca2 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>>  \t}\n>>\n>>  \tGST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>> -\tcam_->queueRequest(wrap->request_.get());\n>>\n>>  \t{\n>>  \t\tGLibLocker locker(&lock_);\n>> +\t\tcam_->queueRequest(wrap->request_.get());\n>>  \t\tqueuedRequests_.push(std::move(wrap));\n>>  \t}\n>>\n>> --\n>> 2.51.0","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 05E33C324E\n\tfor ;\n\tWed, 10 Sep 2025 14:01:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C02A69367;\n\tWed, 10 Sep 2025 16:01: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 7201869357\n\tfor ;\n\tWed, 10 Sep 2025 16:01:21 +0200 (CEST)","from [192.168.33.8] (185.221.142.115.nat.pool.zt.hu\n\t[185.221.142.115])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BD16E1198;\n\tWed, 10 Sep 2025 16:00:06 +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=\"VWJVhSN4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757512807;\n\tbh=ncjVNlGDgcOR7Nly3jczbWVrxOjr4Qx6+hVkVWcYxA4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=VWJVhSN4BRipNahQsnF72sJ8MtBokUNcAI5ITp23EoDTgMWkibyDFhQCmau7ue7g5\n\tcQzzkhwsJLst5dVZzKTZwUB2es+D1esOHGP8SHAjAYyAe/EA1LzVFbljZMKuO16tKO\n\tRA2Td9Q8UB6aFGYj9xLKmnubmWJlixpIi7AU+6rY=","Message-ID":"","Date":"Wed, 10 Sep 2025 16:01:15 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","To":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org, Umang Jain ","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":"","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":36221,"web_url":"https://patchwork.libcamera.org/comment/36221/","msgid":"<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>","date":"2025-10-13T11:40:57","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-09-10 15:01:15)\n> Hi\n> \n> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n> > Hi,\n> > \n> > Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n> >> The moment execution enters `Camera::queueRequest()`, an application must be\n> >> prepared to received the corresponding `requestCompleted` signal. However,\n> >> the gstreamer element is currently not prepared: it queues the request,\n> >> takes a lock, then inserts into a list.\n> >>\n> >> If the request completion handler acquires the lock right after the queueing,\n> >> then it will not find the object in the list that it expects. Even potentially\n> >> encountering an empty list, running into undefined behaviour when trying\n> >> to pop from it.\n> >>\n> >> Fix that by queueing the request after the lock protected insertion.\n> >>\n> >> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n> >> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n\nBased on https://bugs.libcamera.org/show_bug.cgi?id=238#c18 I suspect we\nshould try to get this into 0.6.\n\n\n> >> Signed-off-by: Barnabás Pőcze \n> >> ---\n> >>\n> >> Doing it while holding the lock is not ideal, but the alternative is\n> >> getting a raw pointer to the `Request` object beforehand, but that does\n> >> not seem too desirable either.\n> > \n> > If we are worried calling into libcamera with uur own lock held, have you\n> > considered pushing it into the list before calling cam_->queueRequest() ? The\n> > camera object in libcamera is supposed to be threadsafe, and we should have\n> > strong ownership on the state and request wrap (if not some reffing is missing I\n> > guess).\n> \n> The `RequestWrap` pointer is moved into the container, so one would need to take\n> a raw pointer to the `Request`. The fact that the request completions handler\n> unconditionally pops the first element worries me a bit. Although if things have\n> already gone out of sync there, it might not matter.\n> \n> \n> > \n> > Nicolas\n> > \n> > p.s. the lock is dangerous if libcamera decides to callback from the caller\n> > thread instead of a separate thread. So calling with lock is implementation\n> > dependant.\n> \n> That's true, but currently it is guaranteed. The application developer guide\n> mentions it explicitly.\n> \n\nIs there anything else required to progress this patch? any more\nspecific concerns?\n\n--\nKieran\n\n\n\n\n> \n> Regards,\n> Barnabás Pőcze\n> \n> > \n> >>\n> >> ---\n> >>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n> >>  1 file changed, 1 insertion(+), 1 deletion(-)\n> >>\n> >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> >> b/src/gstreamer/gstlibcamerasrc.cpp\n> >> index 79a025a57..103dfbca2 100644\n> >> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n> >>   }\n> >>\n> >>   GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n> >> - cam_->queueRequest(wrap->request_.get());\n> >>\n> >>   {\n> >>   GLibLocker locker(&lock_);\n> >> + cam_->queueRequest(wrap->request_.get());\n> >>   queuedRequests_.push(std::move(wrap));\n> >>   }\n> >>\n> >> --\n> >> 2.51.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 D5D88BF415\n\tfor ;\n\tMon, 13 Oct 2025 11:41:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C011C6046D;\n\tMon, 13 Oct 2025 13:41:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5CCA36031A\n\tfor ;\n\tMon, 13 Oct 2025 13:41:00 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AA106183;\n\tMon, 13 Oct 2025 13:39:22 +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=\"cOJkjTNJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760355562;\n\tbh=yBV/2xbyyJanaXC6TC77ZB+j5n2IxfsyVRr296RLTuY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=cOJkjTNJS8dvH3icExOEuJfVkSyeeHpmnwTXX0PIw8NmXlzO+bWmV4aWaw3IJR5DA\n\t/n9Zl1mWxk2yCAgo6b5hNewE7bbvNQkSW/pI+CRCPClv2bhDJq6rllIczNyY0WQ/W4\n\t/rPl0YLDs98mGnlcVcTLzJiX+z+Ry+r28MWv176c=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tNicolas Dufresne , \n\tUmang Jain , libcamera-devel@lists.libcamera.org","Date":"Mon, 13 Oct 2025 12:40:57 +0100","Message-ID":"<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36222,"web_url":"https://patchwork.libcamera.org/comment/36222/","msgid":"<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>","date":"2025-10-13T12:19:32","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 13. 13:40 keltezéssel, Kieran Bingham írta:\n> Quoting Barnabás Pőcze (2025-09-10 15:01:15)\n>> Hi\n>>\n>> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n>>> Hi,\n>>>\n>>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n>>>> The moment execution enters `Camera::queueRequest()`, an application must be\n>>>> prepared to received the corresponding `requestCompleted` signal. However,\n>>>> the gstreamer element is currently not prepared: it queues the request,\n>>>> takes a lock, then inserts into a list.\n>>>>\n>>>> If the request completion handler acquires the lock right after the queueing,\n>>>> then it will not find the object in the list that it expects. Even potentially\n>>>> encountering an empty list, running into undefined behaviour when trying\n>>>> to pop from it.\n>>>>\n>>>> Fix that by queueing the request after the lock protected insertion.\n>>>>\n>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n>>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n> \n> Based on https://bugs.libcamera.org/show_bug.cgi?id=238#c18 I suspect we\n> should try to get this into 0.6.\n> \n> \n>>>> Signed-off-by: Barnabás Pőcze \n>>>> ---\n>>>>\n>>>> Doing it while holding the lock is not ideal, but the alternative is\n>>>> getting a raw pointer to the `Request` object beforehand, but that does\n>>>> not seem too desirable either.\n>>>\n>>> If we are worried calling into libcamera with uur own lock held, have you\n>>> considered pushing it into the list before calling cam_->queueRequest() ? The\n>>> camera object in libcamera is supposed to be threadsafe, and we should have\n>>> strong ownership on the state and request wrap (if not some reffing is missing I\n>>> guess).\n>>\n>> The `RequestWrap` pointer is moved into the container, so one would need to take\n>> a raw pointer to the `Request`. The fact that the request completions handler\n>> unconditionally pops the first element worries me a bit. Although if things have\n>> already gone out of sync there, it might not matter.\n>>\n>>\n>>>\n>>> Nicolas\n>>>\n>>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n>>> thread instead of a separate thread. So calling with lock is implementation\n>>> dependant.\n>>\n>> That's true, but currently it is guaranteed. The application developer guide\n>> mentions it explicitly.\n>>\n> \n> Is there anything else required to progress this patch? any more\n> specific concerns?\n\n From my point of view, there are no concerns, it can be merged as is.\nBefore the mentioned commit (06bd05beced313) the queueRequest() and push()\ncalls were done under the same lock, and the commit message also does not\nmention any correctness issues with that setup.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> --\n> Kieran\n> \n> \n> \n> \n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>>\n>>>>\n>>>> ---\n>>>>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>>>> b/src/gstreamer/gstlibcamerasrc.cpp\n>>>> index 79a025a57..103dfbca2 100644\n>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>>>>   }\n>>>>\n>>>>   GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>>>> - cam_->queueRequest(wrap->request_.get());\n>>>>\n>>>>   {\n>>>>   GLibLocker locker(&lock_);\n>>>> + cam_->queueRequest(wrap->request_.get());\n>>>>   queuedRequests_.push(std::move(wrap));\n>>>>   }\n>>>>\n>>>> --\n>>>> 2.51.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 6AA41BF415\n\tfor ;\n\tMon, 13 Oct 2025 12:19:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 847B66046D;\n\tMon, 13 Oct 2025 14:19:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E7D96031A\n\tfor ;\n\tMon, 13 Oct 2025 14:19:36 +0200 (CEST)","from [192.168.33.36] (185.182.214.105.nat.pool.zt.hu\n\t[185.182.214.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2DB11D0;\n\tMon, 13 Oct 2025 14:17:58 +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=\"h+8s0JZ4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760357878;\n\tbh=fdmWSGV6Mg4H51cwgoU+KTTQcXE9Hv9UYUuHwXJYLKk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=h+8s0JZ4z9E3RBLHK/qrwO1wKNTx7bDWBDo3QUF9gWxo8EDD4a9IgnR+6tWcnt0u6\n\tK/4q78r646l106ZvMaoVv/29NjDMFMJ6v2TSW0xbS+3o/pOSQQZtAwC1CQUFjIqaIY\n\tIvYd7sfHCR5Swnht7S/ty9N1wKoHc6DmZ1wtIMrk=","Message-ID":"<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>","Date":"Mon, 13 Oct 2025 14:19:32 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","To":"Kieran Bingham ,\n\tNicolas Dufresne ,\n\tUmang Jain , libcamera-devel@lists.libcamera.org","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t\n\t<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>","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":"","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":36223,"web_url":"https://patchwork.libcamera.org/comment/36223/","msgid":"","date":"2025-10-13T12:30:01","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 13. 14:19 keltezéssel, Barnabás Pőcze írta:\n> 2025. 10. 13. 13:40 keltezéssel, Kieran Bingham írta:\n>> Quoting Barnabás Pőcze (2025-09-10 15:01:15)\n>>> Hi\n>>>\n>>> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n>>>> Hi,\n>>>>\n>>>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n>>>>> The moment execution enters `Camera::queueRequest()`, an application must be\n>>>>> prepared to received the corresponding `requestCompleted` signal. However,\n>>>>> the gstreamer element is currently not prepared: it queues the request,\n>>>>> takes a lock, then inserts into a list.\n>>>>>\n>>>>> If the request completion handler acquires the lock right after the queueing,\n>>>>> then it will not find the object in the list that it expects. Even potentially\n>>>>> encountering an empty list, running into undefined behaviour when trying\n>>>>> to pop from it.\n>>>>>\n>>>>> Fix that by queueing the request after the lock protected insertion.\n>>>>>\n>>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n>>>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n>>\n>> Based on https://bugs.libcamera.org/show_bug.cgi?id=238#c18 I suspect we\n>> should try to get this into 0.6.\n>>\n>>\n>>>>> Signed-off-by: Barnabás Pőcze \n>>>>> ---\n>>>>>\n>>>>> Doing it while holding the lock is not ideal, but the alternative is\n>>>>> getting a raw pointer to the `Request` object beforehand, but that does\n>>>>> not seem too desirable either.\n>>>>\n>>>> If we are worried calling into libcamera with uur own lock held, have you\n>>>> considered pushing it into the list before calling cam_->queueRequest() ? The\n>>>> camera object in libcamera is supposed to be threadsafe, and we should have\n>>>> strong ownership on the state and request wrap (if not some reffing is missing I\n>>>> guess).\n>>>\n>>> The `RequestWrap` pointer is moved into the container, so one would need to take\n>>> a raw pointer to the `Request`. The fact that the request completions handler\n>>> unconditionally pops the first element worries me a bit. Although if things have\n>>> already gone out of sync there, it might not matter.\n>>>\n>>>\n>>>>\n>>>> Nicolas\n>>>>\n>>>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n>>>> thread instead of a separate thread. So calling with lock is implementation\n>>>> dependant.\n>>>\n>>> That's true, but currently it is guaranteed. The application developer guide\n>>> mentions it explicitly.\n>>>\n>>\n>> Is there anything else required to progress this patch? any more\n>> specific concerns?\n> \n> From my point of view, there are no concerns, it can be merged as is.\n> Before the mentioned commit (06bd05beced313) the queueRequest() and push()\n> calls were done under the same lock, and the commit message also does not\n> mention any correctness issues with that setup.\n\nCorrection: I would like to slightly tweak the commit message. For example,\nthe last sentence is not in sync with the code.\n\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n>>\n>> --\n>> Kieran\n>>\n>>\n>>\n>>\n>>>\n>>> Regards,\n>>> Barnabás Pőcze\n>>>\n>>>>\n>>>>>\n>>>>> ---\n>>>>>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> index 79a025a57..103dfbca2 100644\n>>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>>>>>   }\n>>>>>\n>>>>>   GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>>>>> - cam_->queueRequest(wrap->request_.get());\n>>>>>\n>>>>>   {\n>>>>>   GLibLocker locker(&lock_);\n>>>>> + cam_->queueRequest(wrap->request_.get());\n>>>>>   queuedRequests_.push(std::move(wrap));\n>>>>>   }\n>>>>>\n>>>>> --\n>>>>> 2.51.0\n>>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EEA07BE080\n\tfor ;\n\tMon, 13 Oct 2025 12:30:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D91D06046E;\n\tMon, 13 Oct 2025 14:30:07 +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 906A96031A\n\tfor ;\n\tMon, 13 Oct 2025 14:30:06 +0200 (CEST)","from [192.168.33.36] (185.182.214.105.nat.pool.zt.hu\n\t[185.182.214.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 158F86F9;\n\tMon, 13 Oct 2025 14:28:27 +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=\"fW5Hu4Yw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760358508;\n\tbh=KhxRCX7jPx311ciMv0vDF4qaFJV7pwiDjgmt1ljq/B8=;\n\th=Date:Subject:From:To:Reply-To:References:In-Reply-To:From;\n\tb=fW5Hu4YwTrmCS5aLU86rHIkL5vsz4sz480MUdx+DtP4b7M4HCwtg7UvEmnuskovi7\n\tDYsOJTAyaR2bd59+yevK32kk7/UDrE7cuQEg2pvQk2a8ZQ+15qmcXYsfV55DmD7M9t\n\tV1fqLL18aeqiY/Twds+AtK8U5bKvDqU4BgyhwHFA=","Message-ID":"","Date":"Mon, 13 Oct 2025 14:30:01 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","To":"Kieran Bingham ,\n\tNicolas Dufresne ,\n\tUmang Jain , libcamera-devel@lists.libcamera.org","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t\n\t<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>\n\t\n\t<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<26b4b54e-6c18-4253-88ed-3f1e85f8c894@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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36226,"web_url":"https://patchwork.libcamera.org/comment/36226/","msgid":"<860bc27c46ad1fe29f8f30b2cc4775d5@igalia.com>","date":"2025-10-13T12:55:41","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2025-10-13 17:49, Barnabás Pőcze wrote:\n> 2025. 10. 13. 13:40 keltezéssel, Kieran Bingham írta:\n>> Quoting Barnabás Pőcze (2025-09-10 15:01:15)\n>>> Hi\n>>>\n>>> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n>>>> Hi,\n>>>>\n>>>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n>>>>> The moment execution enters `Camera::queueRequest()`, an application must be\n>>>>> prepared to received the corresponding `requestCompleted` signal. However,\n>>>>> the gstreamer element is currently not prepared: it queues the request,\n>>>>> takes a lock, then inserts into a list.\n>>>>>\n>>>>> If the request completion handler acquires the lock right after the queueing,\n>>>>> then it will not find the object in the list that it expects. Even potentially\n>>>>> encountering an empty list, running into undefined behaviour when trying\n>>>>> to pop from it.\n>>>>>\n>>>>> Fix that by queueing the request after the lock protected insertion.\n>>>>>\n>>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n>>>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n>> \n>> Based on https://bugs.libcamera.org/show_bug.cgi?id=238#c18 I suspect we\n>> should try to get this into 0.6.\n>> \n>> \n>>>>> Signed-off-by: Barnabás Pőcze \n>>>>> ---\n>>>>>\n>>>>> Doing it while holding the lock is not ideal, but the alternative is\n>>>>> getting a raw pointer to the `Request` object beforehand, but that does\n>>>>> not seem too desirable either.\n>>>>\n>>>> If we are worried calling into libcamera with uur own lock held, have you\n>>>> considered pushing it into the list before calling cam_->queueRequest() ? The\n>>>> camera object in libcamera is supposed to be threadsafe, and we should have\n>>>> strong ownership on the state and request wrap (if not some reffing is missing I\n>>>> guess).\n>>>\n>>> The `RequestWrap` pointer is moved into the container, so one would need to take\n>>> a raw pointer to the `Request`. The fact that the request completions handler\n>>> unconditionally pops the first element worries me a bit. Although if things have\n>>> already gone out of sync there, it might not matter.\n>>>\n>>>\n>>>>\n>>>> Nicolas\n>>>>\n>>>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n>>>> thread instead of a separate thread. So calling with lock is implementation\n>>>> dependant.\n>>>\n>>> That's true, but currently it is guaranteed. The application developer guide\n>>> mentions it explicitly.\n>>>\n>> \n>> Is there anything else required to progress this patch? any more\n>> specific concerns?\n> \n> From my point of view, there are no concerns, it can be merged as is.\n> Before the mentioned commit (06bd05beced313) the queueRequest() and push()\n> calls were done under the same lock, and the commit message also does not\n> mention any correctness issues with that setup.\n\nI don't see any major objections here and ack the merge given it helps\nwith the current bug.\n\nThe existence of g_return_if_fail() in the request complete handler\nsuggests that going out-of-sync\nis a programming error and should not occur. This patch aligns with that\nas well hence,\n\nAcked-by: Umang Jain \n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n>> \n>> --\n>> Kieran\n>> \n>> \n>> \n>> \n>>>\n>>> Regards,\n>>> Barnabás Pőcze\n>>>\n>>>>\n>>>>>\n>>>>> ---\n>>>>>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> index 79a025a57..103dfbca2 100644\n>>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>>>>>   }\n>>>>>\n>>>>>   GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>>>>> - cam_->queueRequest(wrap->request_.get());\n>>>>>\n>>>>>   {\n>>>>>   GLibLocker locker(&lock_);\n>>>>> + cam_->queueRequest(wrap->request_.get());\n>>>>>   queuedRequests_.push(std::move(wrap));\n>>>>>   }\n>>>>>\n>>>>> --\n>>>>> 2.51.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 292C8BE080\n\tfor ;\n\tMon, 13 Oct 2025 12:55:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0500C604E7;\n\tMon, 13 Oct 2025 14:55:50 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A4356031A\n\tfor ;\n\tMon, 13 Oct 2025 14:55:48 +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 1v8I5Y-008w3W-Fe; Mon, 13 Oct 2025 14:55:44 +0200","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1v8I5V-00Giql-VY; Mon, 13 Oct 2025 14:55:44 +0200","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from )\n\tid 1v8I5V-00FZyu-1J; Mon, 13 Oct 2025 14:55:41 +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=\"ehoodoPJ\"; 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=DxdiPh0AQKCZSpNS3xqhvMXJpX4NpOpbJvjTUnaIWmY=;\n\tb=ehoodoPJeTpZ0h+vFCZFtGZrJw\n\tqsIXakk7M+Hz6UWPe5GBZIMR15Y+4hEp3OrKmVLsfdKgAvEwmEHIsRacsgWwpDgMU7b7qlPt1UwbR\n\tady1Ss5sj6nKbQAM1LulugV8tnu6R4dmoqxTRkZkw5tlgfsYk46Jome9NwkAFVra4RxjLNcZzaSPT\n\tJ+0T5xbShuJR5UcKRmNHi9H3OvMg2sG6P+qWmfsY4rp2X19ZDXZF1IJeZJR5cMf2P6qzizcMt9maw\n\tSkQcld/5lGLFRAbvDsWDRUUs4RTFIcvrSNlLdJy7cVqlp8Yg1UeKmiELh1sxrAP4m7/3iMxdhjofL\n\tCxgpxhjA==;","MIME-Version":"1.0","Date":"Mon, 13 Oct 2025 18:25:41 +0530","From":"uajain ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Kieran Bingham , Nicolas Dufresne\n\t, libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","In-Reply-To":"<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t\n\t<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>\n\t<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>","Message-ID":"<860bc27c46ad1fe29f8f30b2cc4775d5@igalia.com>","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-Spam-Report":"NO, Score=-3.1, Tests=ALL_TRUSTED=-3, AWL=-0.856, BAYES_50=0.8,\n\tURIBL_BLOCKED=0.001","X-Spam-Score":"-30","X-Spam-Bar":"---","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":36404,"web_url":"https://patchwork.libcamera.org/comment/36404/","msgid":"<33014e98-a72b-432c-9332-4ab9fd546666@ideasonboard.com>","date":"2025-10-23T14:04:49","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi Nicolas\n\n2025. 09. 10. 16:01 keltezéssel, Barnabás Pőcze írta:\n> Hi\n> \n> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n>> Hi,\n>>\n>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n>>> The moment execution enters `Camera::queueRequest()`, an application must be\n>>> prepared to received the corresponding `requestCompleted` signal. However,\n>>> the gstreamer element is currently not prepared: it queues the request,\n>>> takes a lock, then inserts into a list.\n>>>\n>>> If the request completion handler acquires the lock right after the queueing,\n>>> then it will not find the object in the list that it expects. Even potentially\n>>> encountering an empty list, running into undefined behaviour when trying\n>>> to pop from it.\n>>>\n>>> Fix that by queueing the request after the lock protected insertion.\n>>>\n>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n>>> Signed-off-by: Barnabás Pőcze \n>>> ---\n>>>\n>>> Doing it while holding the lock is not ideal, but the alternative is\n>>> getting a raw pointer to the `Request` object beforehand, but that does\n>>> not seem too desirable either.\n>>\n>> If we are worried calling into libcamera with uur own lock held, have you\n>> considered pushing it into the list before calling cam_->queueRequest() ? The\n>> camera object in libcamera is supposed to be threadsafe, and we should have\n>> strong ownership on the state and request wrap (if not some reffing is missing I\n>> guess).\n> \n> The `RequestWrap` pointer is moved into the container, so one would need to take\n> a raw pointer to the `Request`. The fact that the request completions handler\n> unconditionally pops the first element worries me a bit. Although if things have\n> already gone out of sync there, it might not matter.\n> \n> \n>>\n>> Nicolas\n>>\n>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n>> thread instead of a separate thread. So calling with lock is implementation\n>> dependant.\n> \n> That's true, but currently it is guaranteed. The application developer guide\n> mentions it explicitly.\n> \n> \n\nWe would like to move forward with this change, and wondering if it looks acceptable\nto you and/or if you have any other concerns?\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>>\n>>>\n>>> ---\n>>>   src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>>>   1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>>> b/src/gstreamer/gstlibcamerasrc.cpp\n>>> index 79a025a57..103dfbca2 100644\n>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>>>       }\n>>>\n>>>       GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>>> -    cam_->queueRequest(wrap->request_.get());\n>>>\n>>>       {\n>>>           GLibLocker locker(&lock_);\n>>> +        cam_->queueRequest(wrap->request_.get());\n>>>           queuedRequests_.push(std::move(wrap));\n>>>       }\n>>>\n>>> -- \n>>> 2.51.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 DE18FC3259\n\tfor ;\n\tThu, 23 Oct 2025 14:04:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B806C607CF;\n\tThu, 23 Oct 2025 16:04:54 +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 0B7E4606DE\n\tfor ;\n\tThu, 23 Oct 2025 16:04:53 +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 B90EF4C7;\n\tThu, 23 Oct 2025 16:03:07 +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=\"hzlksK+H\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761228187;\n\tbh=sa76Q4+Yi6K+tVvbh+MJymvJUiH/cixBPQ6060fACXw=;\n\th=Date:Subject:From:To:References:Cc:In-Reply-To:From;\n\tb=hzlksK+HwDBt8zL/mh+7Rf4NQnK9GqQV442P0KNc84swCI5iLlP+m2Z1jI/R4r46c\n\tnAw/zyzGd7mktFCRyRfOwTbOcZ3y5RXK0B7IkJ4Pzjcg9utSc/Iiei8hqkaJpRHBK/\n\t9Pbv+LkCcwUkRjntqVL7iMlhRnDJTXLJuEitxvS8=","Message-ID":"<33014e98-a72b-432c-9332-4ab9fd546666@ideasonboard.com>","Date":"Thu, 23 Oct 2025 16:04:49 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","To":"Nicolas Dufresne ","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t","Content-Language":"en-US, hu-HU","Cc":"libcamera-devel@lists.libcamera.org, Umang Jain ","In-Reply-To":"","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":"","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":36688,"web_url":"https://patchwork.libcamera.org/comment/36688/","msgid":"","date":"2025-11-04T18:02:36","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Hi,\n\nLe jeudi 23 octobre 2025 à 16:04 +0200, Barnabás Pőcze a écrit :\n> Hi Nicolas\n> \n> 2025. 09. 10. 16:01 keltezéssel, Barnabás Pőcze írta:\n> > Hi\n> > \n> > 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n> > > Hi,\n> > > \n> > > Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n> > > > The moment execution enters `Camera::queueRequest()`, an application must be\n> > > > prepared to received the corresponding `requestCompleted` signal. However,\n> > > > the gstreamer element is currently not prepared: it queues the request,\n> > > > takes a lock, then inserts into a list.\n> > > > \n> > > > If the request completion handler acquires the lock right after the queueing,\n> > > > then it will not find the object in the list that it expects. Even potentially\n> > > > encountering an empty list, running into undefined behaviour when trying\n> > > > to pop from it.\n> > > > \n> > > > Fix that by queueing the request after the lock protected insertion.\n> > > > \n> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n> > > > Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n> > > > Signed-off-by: Barnabás Pőcze \n> > > > ---\n> > > > \n> > > > Doing it while holding the lock is not ideal, but the alternative is\n> > > > getting a raw pointer to the `Request` object beforehand, but that does\n> > > > not seem too desirable either.\n> > > \n> > > If we are worried calling into libcamera with uur own lock held, have you\n> > > considered pushing it into the list before calling cam_->queueRequest() ? The\n> > > camera object in libcamera is supposed to be threadsafe, and we should have\n> > > strong ownership on the state and request wrap (if not some reffing is missing I\n> > > guess).\n> > \n> > The `RequestWrap` pointer is moved into the container, so one would need to take\n> > a raw pointer to the `Request`. The fact that the request completions handler\n> > unconditionally pops the first element worries me a bit. Although if things have\n> > already gone out of sync there, it might not matter.\n> > \n> > \n> > > \n> > > Nicolas\n> > > \n> > > p.s. the lock is dangerous if libcamera decides to callback from the caller\n> > > thread instead of a separate thread. So calling with lock is implementation\n> > > dependant.\n> > \n> > That's true, but currently it is guaranteed. The application developer guide\n> > mentions it explicitly.\n> > \n> > \n> \n> We would like to move forward with this change, and wondering if it looks acceptable\n> to you and/or if you have any other concerns?\n\nI was mostly making suggestion to improve memory ownership, with some effort,\nthe use of C++ implicit ref count is possible with shared pointer, but really\nits up to you. It seems no one is worried of calling into libcamera with lock\nheld.\n\nNicolas\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> > \n> > > \n> > > > \n> > > > ---\n> > > >   src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n> > > >   1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > \n> > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> > > > b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > index 79a025a57..103dfbca2 100644\n> > > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > > @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n> > > >       }\n> > > > \n> > > >       GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n> > > > -    cam_->queueRequest(wrap->request_.get());\n> > > > \n> > > >       {\n> > > >           GLibLocker locker(&lock_);\n> > > > +        cam_->queueRequest(wrap->request_.get());\n> > > >           queuedRequests_.push(std::move(wrap));\n> > > >       }\n> > > > \n> > > > -- \n> > > > 2.51.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 E7CC0BDE4C\n\tfor ;\n\tTue, 4 Nov 2025 18:02:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC3DF60A80;\n\tTue, 4 Nov 2025 19:02:40 +0100 (CET)","from bali.collaboradmins.com (bali.collaboradmins.com\n\t[148.251.105.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 221B56069A\n\tfor ;\n\tTue, 4 Nov 2025 19:02:39 +0100 (CET)","from [IPv6:2606:6d00:17:ebd3::5ac] (unknown\n\t[IPv6:2606:6d00:17:ebd3::5ac])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bali.collaboradmins.com (Postfix) with ESMTPSA id EAD8117E10F4;\n\tTue, 4 Nov 2025 19:02:37 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"PD1kqeYd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1762279358;\n\tbh=htd158OCBQZi4M1sDFl0vmt6PlqOBHPlpRdanQ7vmzI=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=PD1kqeYdMYKYa7udDfW8ES4ZZtSGcseAi4xVHNhv4oQZcTkKPH4MGT+HZerI4jEWo\n\tATJk+q7B99Df2ODzB9MsadFHTe427ULTNs1aXAL9hoc1KSzZ8bk3B9nw/07ekYRtlp\n\t/55Z7GHOn5rF3QZh5s4enSzT9L/tUNEf/3YvdFtu6GjkKsOdTvCrlXwX16qG+/wMF1\n\t/fwlXpCkG9sphUrhSMQTXUgj2E4akxny3q3uuQ9NMfm5OR24M846chjNB0HPUO0PW6\n\tq7H24OfFbIh2PtQad5+w2+6bp5BsEvw2IbdIgTRFWdcpdRUkKTPYa55MVdZkPygyXV\n\t2A/m2sTl0lo0w==","Message-ID":"","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"Nicolas Dufresne ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Umang Jain ","Date":"Tue, 04 Nov 2025 13:02:36 -0500","In-Reply-To":"<33014e98-a72b-432c-9332-4ab9fd546666@ideasonboard.com>","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t<33014e98-a72b-432c-9332-4ab9fd546666@ideasonboard.com>","Autocrypt":"addr=nicolas.dufresne@collabora.com; prefer-encrypt=mutual;\n\tkeydata=mDMEaCN2ixYJKwYBBAHaRw8BAQdAM0EHepTful3JOIzcPv6ekHOenE1u0vDG1gdHFrChD\n\t/e0J05pY29sYXMgRHVmcmVzbmUgPG5pY29sYXNAbmR1ZnJlc25lLmNhPoicBBMWCgBEAhsDBQsJCA\n\tcCAiICBhUKCQgLAgQWAgMBAh4HAheABQkJZfd1FiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrjo\n\tCGQEACgkQ2UGUUSlgcvQlQwD/RjpU1SZYcKG6pnfnQ8ivgtTkGDRUJ8gP3fK7+XUjRNIA/iXfhXMN\n\tabIWxO2oCXKf3TdD7aQ4070KO6zSxIcxgNQFtDFOaWNvbGFzIER1ZnJlc25lIDxuaWNvbGFzLmR1Z\n\tnJlc25lQGNvbGxhYm9yYS5jb20+iJkEExYKAEECGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4\n\tAWIQTvDVKBFcTDwhoEbxLZQZRRKWBy9AUCaCyyxgUJCWX3dQAKCRDZQZRRKWBy9ARJAP96pFmLffZ\n\tsmBUpkyVBfFAf+zq6BJt769R0al3kHvUKdgD9G7KAHuioxD2v6SX7idpIazjzx8b8rfzwTWyOQWHC\n\tAAS0LU5pY29sYXMgRHVmcmVzbmUgPG5pY29sYXMuZHVmcmVzbmVAZ21haWwuY29tPoiZBBMWCgBBF\n\tiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrGYCGwMFCQll93UFCwkIBwICIgIGFQoJCAsCBBYCAw\n\tECHgcCF4AACgkQ2UGUUSlgcvRObgD/YnQjfi4+L8f4fI7p1pPMTwRTcaRdy6aqkKEmKsCArzQBAK8\n\tbRLv9QjuqsE6oQZra/RB4widZPvphs78H0P6NmpIJ","Organization":"Collabora Canada","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-vRmrBM5SkCJf8dbZz5bp\"","User-Agent":"Evolution 3.56.2 (3.56.2-2.fc42) ","MIME-Version":"1.0","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":36974,"web_url":"https://patchwork.libcamera.org/comment/36974/","msgid":"","date":"2025-11-21T11:24:45","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 11. 04. 19:02 keltezéssel, Nicolas Dufresne írta:\n> Hi,\n> \n> Le jeudi 23 octobre 2025 à 16:04 +0200, Barnabás Pőcze a écrit :\n>> Hi Nicolas\n>>\n>> 2025. 09. 10. 16:01 keltezéssel, Barnabás Pőcze írta:\n>>> Hi\n>>>\n>>> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n>>>> Hi,\n>>>>\n>>>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n>>>>> The moment execution enters `Camera::queueRequest()`, an application must be\n>>>>> prepared to received the corresponding `requestCompleted` signal. However,\n>>>>> the gstreamer element is currently not prepared: it queues the request,\n>>>>> takes a lock, then inserts into a list.\n>>>>>\n>>>>> If the request completion handler acquires the lock right after the queueing,\n>>>>> then it will not find the object in the list that it expects. Even potentially\n>>>>> encountering an empty list, running into undefined behaviour when trying\n>>>>> to pop from it.\n>>>>>\n>>>>> Fix that by queueing the request after the lock protected insertion.\n>>>>>\n>>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n>>>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n>>>>> Signed-off-by: Barnabás Pőcze \n>>>>> ---\n>>>>>\n>>>>> Doing it while holding the lock is not ideal, but the alternative is\n>>>>> getting a raw pointer to the `Request` object beforehand, but that does\n>>>>> not seem too desirable either.\n>>>>\n>>>> If we are worried calling into libcamera with uur own lock held, have you\n>>>> considered pushing it into the list before calling cam_->queueRequest() ? The\n>>>> camera object in libcamera is supposed to be threadsafe, and we should have\n>>>> strong ownership on the state and request wrap (if not some reffing is missing I\n>>>> guess).\n>>>\n>>> The `RequestWrap` pointer is moved into the container, so one would need to take\n>>> a raw pointer to the `Request`. The fact that the request completions handler\n>>> unconditionally pops the first element worries me a bit. Although if things have\n>>> already gone out of sync there, it might not matter.\n>>>\n>>>\n>>>>\n>>>> Nicolas\n>>>>\n>>>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n>>>> thread instead of a separate thread. So calling with lock is implementation\n>>>> dependant.\n>>>\n>>> That's true, but currently it is guaranteed. The application developer guide\n>>> mentions it explicitly.\n>>>\n>>>\n>>\n>> We would like to move forward with this change, and wondering if it looks acceptable\n>> to you and/or if you have any other concerns?\n> \n> I was mostly making suggestion to improve memory ownership, with some effort,\n> the use of C++ implicit ref count is possible with shared pointer, but really\n> its up to you. It seems no one is worried of calling into libcamera with lock\n> held.\n\nThe `queueRequest()` call can be placed in 3 different position wrt. the insertion\ninto the `queuedRequests_` list.\n\n(a) before\n * currently implemented\n * not entirely correct: the request could complete before the insertion\n\n(b) at the same time (wrt. `lock_`):\n * this is proposed\n * addresses the \"early\" completion issue of (a) by doing the `queueRequest() under the lock,\n preventing the request completion handler from progressing too soon\n * not ideal because it calls into libcamera with the lock held\n\n(c) after\n * addresses the \"early\" completion issue of (a) by doing the `queueRequest()`\n after the mutex has been unlocked\n * not ideal because the list contains std::unique_ptr, so the ownership of the request\n is transferred to the container; a raw pointer to the request would be needed\n\nAs far as I understand `gst_libcamera_src_task_run()` is the only caller, so\n`GstLibcameraSrcState::queueRequest()` only ever runs in a single thread, thus\nboth (b) and (c) should be correct.\n\nAs far as I gather you would prefer something that does not call into libcamera with\n`lock_` held, which leaves only (c).\n\nMy concern with (c) is what happens if things go wrong. That is, if we get a raw pointer\nto the `Request`, can that be invalidated before `queueRequest()` time? I suppose things\nare already in a bad state at that point, so maybe it does not matter much.\n\nAs a variant of (c), if I understand you correctly, std::shared_ptr could be used in\nthe container. Is that the suggestion or am I misunderstanding?\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Nicolas\n> \n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>>\n>>>>\n>>>>>\n>>>>> ---\n>>>>>   src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> index 79a025a57..103dfbca2 100644\n>>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>>>>>       }\n>>>>>\n>>>>>       GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>>>>> -    cam_->queueRequest(wrap->request_.get());\n>>>>>\n>>>>>       {\n>>>>>           GLibLocker locker(&lock_);\n>>>>> +        cam_->queueRequest(wrap->request_.get());\n>>>>>           queuedRequests_.push(std::move(wrap));\n>>>>>       }\n>>>>>\n>>>>> -- \n>>>>> 2.51.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 95A3ABD80A\n\tfor ;\n\tFri, 21 Nov 2025 11:24:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D68D960AA0;\n\tFri, 21 Nov 2025 12:24:50 +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 009B6609D8\n\tfor ;\n\tFri, 21 Nov 2025 12:24:48 +0100 (CET)","from [192.168.33.39] (185.221.143.100.nat.pool.zt.hu\n\t[185.221.143.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9EA4666B;\n\tFri, 21 Nov 2025 12:22:42 +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=\"kIqx/kT7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763724162;\n\tbh=YSFH1rKrhLsKLdzt2AJxZ3J094n3CdwsBRUBhvmDp68=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=kIqx/kT7ejp3o0KnaNuJF5LItsFT1lnG3RlBrR0KCzqXxn6VGLRHIT9nXJAEtkViC\n\tpQNsZ2DEuZBWJC2YdafeYC6ryGRQrdtnoEeNIWCwJQD9Wm1dQN6unuBo4D2dV+bm/0\n\t+leORBp9tumO3UwS80ceU+biLH2+CN2nHQ2deeXc=","Message-ID":"","Date":"Fri, 21 Nov 2025 12:24:45 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org, Umang Jain ","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t<33014e98-a72b-432c-9332-4ab9fd546666@ideasonboard.com>\n\t<6LWvyeBR2t0XlVlid0OXmVnBm88pVmQ6m6gRZTPXyw8z3yR2bVaWeANn1-ZtXV_WHm3qhAvXpgnzGtlFf93rpQ==@protonmail.internalid>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":"","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":36987,"web_url":"https://patchwork.libcamera.org/comment/36987/","msgid":"","date":"2025-11-21T13:36:00","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Hi,\n\nLe vendredi 21 novembre 2025 à 12:24 +0100, Barnabás Pőcze a écrit :\n> (c) after\n>      * addresses the \"early\" completion issue of (a) by doing the `queueRequest()`\n>        after the mutex has been unlocked\n>      * not ideal because the list contains std::unique_ptr, so the ownership of the request\n>        is transferred to the container; a raw pointer to the request would be needed\n\n[...]\n\n> \n> As a variant of (c), if I understand you correctly, std::shared_ptr could be used in\n> the container. Is that the suggestion or am I misunderstanding?\n\nThis is what I initially proposed couple of weeks ago (you simply replied you\ncan't its unique). This is all internal, it was made the most restrictive\nbecause when you don't know this is what you should do, but since it no longer\napplies, it can be changed.\n\nPerhaps my take for future libcamera C API, make sure asynchronous operation\nhave a user_data and a destroy function for that user data. This way, we don't\nhave to use lists, and don't endup in this ownership loop.\n\nNicolas","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 09178BD80A\n\tfor ;\n\tFri, 21 Nov 2025 13:36:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C7AA060A8B;\n\tFri, 21 Nov 2025 14:36:05 +0100 (CET)","from bali.collaboradmins.com (bali.collaboradmins.com\n\t[148.251.105.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AC3A260805\n\tfor ;\n\tFri, 21 Nov 2025 14:36:03 +0100 (CET)","from [IPv6:2606:6d00:17:7b4b::5ac] (unknown\n\t[IPv6:2606:6d00:17:7b4b::5ac])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\t(Authenticated sender: nicolas)\n\tby bali.collaboradmins.com (Postfix) with ESMTPSA id 9014E17E0360;\n\tFri, 21 Nov 2025 14:36:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"htUA8zW2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1763732163;\n\tbh=jk1G0CmwmeLxyrl30OFq5S9S5/wHTgotUSOjD0KiXPg=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=htUA8zW2Ff/ZCpqsxZvlSM7ur6IjbpBiKx0DXTDLEH1bfeKvvjOz1UCZK6xJsTfQd\n\tvy37cFLzdXX9VBjLVg9CyxFKGxNKXxQwVICI8UmVdujFWPbBefWxH8iK0tsIfBAX2W\n\t7jQyG+zZiWFDX20c1wMxIZ4iZnPSLWvtkTeKmEWnrJielY1zm6K35lfmw0QuXB4hSN\n\tLRKNf6lyuBOPM7DRbkffAfS0dScqLFSyfKJQDtAcuZ0yQjfcqrwjtYw5nLKAi71jvD\n\tTNP/RVA4SkFNaKJZIHxx+fSozvuFjMZh9ub5j77ZT4zXfcchnLyilN+RHh05KUyy1Z\n\tENCQaQVZ78gbA==","Message-ID":"","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"Nicolas Dufresne ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Umang Jain ","Date":"Fri, 21 Nov 2025 08:36:00 -0500","In-Reply-To":"","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t<33014e98-a72b-432c-9332-4ab9fd546666@ideasonboard.com>\n\t<6LWvyeBR2t0XlVlid0OXmVnBm88pVmQ6m6gRZTPXyw8z3yR2bVaWeANn1-ZtXV_WHm3qhAvXpgnzGtlFf93rpQ==@protonmail.internalid>\n\t\n\t","Autocrypt":"addr=nicolas.dufresne@collabora.com; prefer-encrypt=mutual;\n\tkeydata=mDMEaCN2ixYJKwYBBAHaRw8BAQdAM0EHepTful3JOIzcPv6ekHOenE1u0vDG1gdHFrChD\n\t/e0J05pY29sYXMgRHVmcmVzbmUgPG5pY29sYXNAbmR1ZnJlc25lLmNhPoicBBMWCgBEAhsDBQsJCA\n\tcCAiICBhUKCQgLAgQWAgMBAh4HAheABQkJZfd1FiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrjo\n\tCGQEACgkQ2UGUUSlgcvQlQwD/RjpU1SZYcKG6pnfnQ8ivgtTkGDRUJ8gP3fK7+XUjRNIA/iXfhXMN\n\tabIWxO2oCXKf3TdD7aQ4070KO6zSxIcxgNQFtDFOaWNvbGFzIER1ZnJlc25lIDxuaWNvbGFzLmR1Z\n\tnJlc25lQGNvbGxhYm9yYS5jb20+iJkEExYKAEECGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4\n\tAWIQTvDVKBFcTDwhoEbxLZQZRRKWBy9AUCaCyyxgUJCWX3dQAKCRDZQZRRKWBy9ARJAP96pFmLffZ\n\tsmBUpkyVBfFAf+zq6BJt769R0al3kHvUKdgD9G7KAHuioxD2v6SX7idpIazjzx8b8rfzwTWyOQWHC\n\tAAS0LU5pY29sYXMgRHVmcmVzbmUgPG5pY29sYXMuZHVmcmVzbmVAZ21haWwuY29tPoiZBBMWCgBBF\n\tiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrGYCGwMFCQll93UFCwkIBwICIgIGFQoJCAsCBBYCAw\n\tECHgcCF4AACgkQ2UGUUSlgcvRObgD/YnQjfi4+L8f4fI7p1pPMTwRTcaRdy6aqkKEmKsCArzQBAK8\n\tbRLv9QjuqsE6oQZra/RB4widZPvphs78H0P6NmpIJ","Organization":"Collabora Canada","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-OVhkQ+uMtIPSHucTAdYF\"","User-Agent":"Evolution 3.58.1 (3.58.1-1.fc43) ","MIME-Version":"1.0","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":37629,"web_url":"https://patchwork.libcamera.org/comment/37629/","msgid":"<176836941443.3888464.12622909927488267252@ping.linuxembedded.co.uk>","date":"2026-01-14T05:43:34","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-10-13 13:30:01)\n> 2025. 10. 13. 14:19 keltezéssel, Barnabás Pőcze írta:\n> > 2025. 10. 13. 13:40 keltezéssel, Kieran Bingham írta:\n> >> Quoting Barnabás Pőcze (2025-09-10 15:01:15)\n> >>> Hi\n> >>>\n> >>> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n> >>>> Hi,\n> >>>>\n> >>>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n> >>>>> The moment execution enters `Camera::queueRequest()`, an application must be\n> >>>>> prepared to received the corresponding `requestCompleted` signal. However,\n> >>>>> the gstreamer element is currently not prepared: it queues the request,\n> >>>>> takes a lock, then inserts into a list.\n> >>>>>\n> >>>>> If the request completion handler acquires the lock right after the queueing,\n> >>>>> then it will not find the object in the list that it expects. Even potentially\n> >>>>> encountering an empty list, running into undefined behaviour when trying\n> >>>>> to pop from it.\n> >>>>>\n> >>>>> Fix that by queueing the request after the lock protected insertion.\n> >>>>>\n> >>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n> >>>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n> >>\n> >> Based on https://bugs.libcamera.org/show_bug.cgi?id=238#c18 I suspect we\n> >> should try to get this into 0.6.\n> >>\n> >>\n> >>>>> Signed-off-by: Barnabás Pőcze \n> >>>>> ---\n> >>>>>\n> >>>>> Doing it while holding the lock is not ideal, but the alternative is\n> >>>>> getting a raw pointer to the `Request` object beforehand, but that does\n> >>>>> not seem too desirable either.\n> >>>>\n> >>>> If we are worried calling into libcamera with uur own lock held, have you\n> >>>> considered pushing it into the list before calling cam_->queueRequest() ? The\n> >>>> camera object in libcamera is supposed to be threadsafe, and we should have\n> >>>> strong ownership on the state and request wrap (if not some reffing is missing I\n> >>>> guess).\n> >>>\n> >>> The `RequestWrap` pointer is moved into the container, so one would need to take\n> >>> a raw pointer to the `Request`. The fact that the request completions handler\n> >>> unconditionally pops the first element worries me a bit. Although if things have\n> >>> already gone out of sync there, it might not matter.\n> >>>\n> >>>\n> >>>>\n> >>>> Nicolas\n> >>>>\n> >>>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n> >>>> thread instead of a separate thread. So calling with lock is implementation\n> >>>> dependant.\n> >>>\n> >>> That's true, but currently it is guaranteed. The application developer guide\n> >>> mentions it explicitly.\n> >>>\n> >>\n> >> Is there anything else required to progress this patch? any more\n> >> specific concerns?\n> > \n> > From my point of view, there are no concerns, it can be merged as is.\n> > Before the mentioned commit (06bd05beced313) the queueRequest() and push()\n> > calls were done under the same lock, and the commit message also does not\n> > mention any correctness issues with that setup.\n> \n> Correction: I would like to slightly tweak the commit message. For example,\n> the last sentence is not in sync with the code.\n> \n\nAs the locking is expected to be safe, and this fixes now multiple user\nbug reports - I think we should try to land this.\n\nWe can always look at ways to change how request are allocated and\nfree-d by the application and passed around on top.\n\nWith the commit message updated if you wish:\n\nReviewed-by: Kieran Bingham \n\n> \n> > \n> > \n> > Regards,\n> > Barnabás Pőcze\n> > \n> >>\n> >> --\n> >> Kieran\n> >>\n> >>\n> >>\n> >>\n> >>>\n> >>> Regards,\n> >>> Barnabás Pőcze\n> >>>\n> >>>>\n> >>>>>\n> >>>>> ---\n> >>>>>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n> >>>>>\n> >>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>> b/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>> index 79a025a57..103dfbca2 100644\n> >>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n> >>>>>   }\n> >>>>>\n> >>>>>   GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n> >>>>> - cam_->queueRequest(wrap->request_.get());\n> >>>>>\n> >>>>>   {\n> >>>>>   GLibLocker locker(&lock_);\n> >>>>> + cam_->queueRequest(wrap->request_.get());\n\nBut actually - now I look again - shouldn't this be checking for errors?\n\ncam_->queueRequest() can fail to queue it if the request is deemed\ninvalid before making the asyncrhonous request. So it should not be\nadded to this queuedRequests_ unless the above call succeeds.\n\n> >>>>>   queuedRequests_.push(std::move(wrap));\n> >>>>>   }\n> >>>>>\n> >>>>> --\n> >>>>> 2.51.0\n> >>>\n> > \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F3117BDCBF\n\tfor ;\n\tWed, 14 Jan 2026 05:43:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2351D61FBC;\n\tWed, 14 Jan 2026 06:43:38 +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 208E76142F\n\tfor ;\n\tWed, 14 Jan 2026 06:43:37 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 557D04BB;\n\tWed, 14 Jan 2026 06:43:10 +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=\"uuWSWtT8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768369390;\n\tbh=uj/wblDSAj7itEkkqr3ovU40QQKjTVIjNFkGC5Z+FFc=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=uuWSWtT8qVIcQYRaECXqPTaU2w9dUX7LXtElpyJ8PStcvuwdyjAeJq4zP/F/1pvhB\n\t3tqQfwE9a1Pxg5jTnVZAiJv5GH3PLK+vx/oGnLLzYY86JyScHXCOZ/dP4frU0TjDSl\n\tuCTrIhLsJgcODk49U2cJhIXUBkpH7/V7UR7MKyP4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t\n\t<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>\n\t\n\t<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>\n\t","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tNicolas Dufresne , \n\tUmang Jain , libcamera-devel@lists.libcamera.org","Date":"Wed, 14 Jan 2026 05:43:34 +0000","Message-ID":"<176836941443.3888464.12622909927488267252@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37631,"web_url":"https://patchwork.libcamera.org/comment/37631/","msgid":"","date":"2026-01-14T08:49:45","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 14. 6:43 keltezéssel, Kieran Bingham írta:\n> Quoting Barnabás Pőcze (2025-10-13 13:30:01)\n>> 2025. 10. 13. 14:19 keltezéssel, Barnabás Pőcze írta:\n>>> 2025. 10. 13. 13:40 keltezéssel, Kieran Bingham írta:\n>>>> Quoting Barnabás Pőcze (2025-09-10 15:01:15)\n>>>>> Hi\n>>>>>\n>>>>> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n>>>>>> Hi,\n>>>>>>\n>>>>>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n>>>>>>> The moment execution enters `Camera::queueRequest()`, an application must be\n>>>>>>> prepared to received the corresponding `requestCompleted` signal. However,\n>>>>>>> the gstreamer element is currently not prepared: it queues the request,\n>>>>>>> takes a lock, then inserts into a list.\n>>>>>>>\n>>>>>>> If the request completion handler acquires the lock right after the queueing,\n>>>>>>> then it will not find the object in the list that it expects. Even potentially\n>>>>>>> encountering an empty list, running into undefined behaviour when trying\n>>>>>>> to pop from it.\n>>>>>>>\n>>>>>>> Fix that by queueing the request after the lock protected insertion.\n>>>>>>>\n>>>>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n>>>>>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n>>>>\n>>>> Based on https://bugs.libcamera.org/show_bug.cgi?id=238#c18 I suspect we\n>>>> should try to get this into 0.6.\n>>>>\n>>>>\n>>>>>>> Signed-off-by: Barnabás Pőcze \n>>>>>>> ---\n>>>>>>>\n>>>>>>> Doing it while holding the lock is not ideal, but the alternative is\n>>>>>>> getting a raw pointer to the `Request` object beforehand, but that does\n>>>>>>> not seem too desirable either.\n>>>>>>\n>>>>>> If we are worried calling into libcamera with uur own lock held, have you\n>>>>>> considered pushing it into the list before calling cam_->queueRequest() ? The\n>>>>>> camera object in libcamera is supposed to be threadsafe, and we should have\n>>>>>> strong ownership on the state and request wrap (if not some reffing is missing I\n>>>>>> guess).\n>>>>>\n>>>>> The `RequestWrap` pointer is moved into the container, so one would need to take\n>>>>> a raw pointer to the `Request`. The fact that the request completions handler\n>>>>> unconditionally pops the first element worries me a bit. Although if things have\n>>>>> already gone out of sync there, it might not matter.\n>>>>>\n>>>>>\n>>>>>>\n>>>>>> Nicolas\n>>>>>>\n>>>>>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n>>>>>> thread instead of a separate thread. So calling with lock is implementation\n>>>>>> dependant.\n>>>>>\n>>>>> That's true, but currently it is guaranteed. The application developer guide\n>>>>> mentions it explicitly.\n>>>>>\n>>>>\n>>>> Is there anything else required to progress this patch? any more\n>>>> specific concerns?\n>>>\n>>> From my point of view, there are no concerns, it can be merged as is.\n>>> Before the mentioned commit (06bd05beced313) the queueRequest() and push()\n>>> calls were done under the same lock, and the commit message also does not\n>>> mention any correctness issues with that setup.\n>>\n>> Correction: I would like to slightly tweak the commit message. For example,\n>> the last sentence is not in sync with the code.\n>>\n> \n> As the locking is expected to be safe, and this fixes now multiple user\n> bug reports - I think we should try to land this.\n> \n> We can always look at ways to change how request are allocated and\n> free-d by the application and passed around on top.\n> \n> With the commit message updated if you wish:\n> \n> Reviewed-by: Kieran Bingham \n> \n>>\n>>>\n>>>\n>>> Regards,\n>>> Barnabás Pőcze\n>>>\n>>>>\n>>>> --\n>>>> Kieran\n>>>>\n>>>>\n>>>>\n>>>>\n>>>>>\n>>>>> Regards,\n>>>>> Barnabás Pőcze\n>>>>>\n>>>>>>\n>>>>>>>\n>>>>>>> ---\n>>>>>>>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>>>\n>>>>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>>>> b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>>>> index 79a025a57..103dfbca2 100644\n>>>>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>>>>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>>>>>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>>>>>>>   }\n>>>>>>>\n>>>>>>>   GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>>>>>>> - cam_->queueRequest(wrap->request_.get());\n>>>>>>>\n>>>>>>>   {\n>>>>>>>   GLibLocker locker(&lock_);\n>>>>>>> + cam_->queueRequest(wrap->request_.get());\n> \n> But actually - now I look again - shouldn't this be checking for errors?\n> \n> cam_->queueRequest() can fail to queue it if the request is deemed\n> invalid before making the asyncrhonous request. So it should not be\n> added to this queuedRequests_ unless the above call succeeds.\n\nYes, certainly, but this has never been done to my knowledge. So I think\nthis can be a separate change.\n\n\n> \n>>>>>>>   queuedRequests_.push(std::move(wrap));\n>>>>>>>   }\n>>>>>>>\n>>>>>>> --\n>>>>>>> 2.51.0\n>>>>>\n>>>\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A6B72BDCBF\n\tfor ;\n\tWed, 14 Jan 2026 08:49:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAB7661FA3;\n\tWed, 14 Jan 2026 09:49:50 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2124E615B2\n\tfor ;\n\tWed, 14 Jan 2026 09:49:49 +0100 (CET)","from [192.168.33.18] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 293D8316;\n\tWed, 14 Jan 2026 09:49:22 +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=\"vOlIMI06\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768380562;\n\tbh=FH4z+bZ5mMzOR7mBUzaZWz1qfoxNMvxKyswV0P9rCW4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=vOlIMI06tC0DFTTRgeIOB7+SmuiIAf1LW0F1a0UDMuTHdTxW6aMUfldWduanjCsHN\n\tWGGV26GlZZ6LGh1oUegy2sYJjCoI6h5cxIobtny4zp0GlGebEPUnp1tzBFN/NykTEE\n\tlq4n9x26ZwrDp5e4YWvow8E0dJKIUpJoIlysavB0=","Message-ID":"","Date":"Wed, 14 Jan 2026 09:49:45 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","To":"Kieran Bingham ,\n\tNicolas Dufresne ,\n\tUmang Jain , libcamera-devel@lists.libcamera.org","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<8g69Fy9jB420Hh3QxNIrWCoMFFJ_4Ek6pwSMduhSLhiFhzG_UZj8ptZaLYqX5kvlDrmutbMin9H1htD2IkbLAw==@protonmail.internalid>\n\t\n\t\n\t\n\t<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>\n\t\n\t<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>\n\t\n\t<176836941443.3888464.12622909927488267252@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176836941443.3888464.12622909927488267252@ping.linuxembedded.co.uk>","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":"","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":37634,"web_url":"https://patchwork.libcamera.org/comment/37634/","msgid":"<176838239009.3486172.146792398759284862@ping.linuxembedded.co.uk>","date":"2026-01-14T09:19:50","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2026-01-14 08:49:45)\n> 2026. 01. 14. 6:43 keltezéssel, Kieran Bingham írta:\n> > Quoting Barnabás Pőcze (2025-10-13 13:30:01)\n> >> 2025. 10. 13. 14:19 keltezéssel, Barnabás Pőcze írta:\n> >>> 2025. 10. 13. 13:40 keltezéssel, Kieran Bingham írta:\n> >>>> Quoting Barnabás Pőcze (2025-09-10 15:01:15)\n> >>>>> Hi\n> >>>>>\n> >>>>> 2025. 09. 09. 16:34 keltezéssel, Nicolas Dufresne írta:\n> >>>>>> Hi,\n> >>>>>>\n> >>>>>> Le mardi 09 septembre 2025 à 15:12 +0200, Barnabás Pőcze a écrit :\n> >>>>>>> The moment execution enters `Camera::queueRequest()`, an application must be\n> >>>>>>> prepared to received the corresponding `requestCompleted` signal. However,\n> >>>>>>> the gstreamer element is currently not prepared: it queues the request,\n> >>>>>>> takes a lock, then inserts into a list.\n> >>>>>>>\n> >>>>>>> If the request completion handler acquires the lock right after the queueing,\n> >>>>>>> then it will not find the object in the list that it expects. Even potentially\n> >>>>>>> encountering an empty list, running into undefined behaviour when trying\n> >>>>>>> to pop from it.\n> >>>>>>>\n> >>>>>>> Fix that by queueing the request after the lock protected insertion.\n> >>>>>>>\n> >>>>>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n> >>>>>>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n> >>>>\n> >>>> Based on https://bugs.libcamera.org/show_bug.cgi?id=238#c18 I suspect we\n> >>>> should try to get this into 0.6.\n> >>>>\n> >>>>\n> >>>>>>> Signed-off-by: Barnabás Pőcze \n> >>>>>>> ---\n> >>>>>>>\n> >>>>>>> Doing it while holding the lock is not ideal, but the alternative is\n> >>>>>>> getting a raw pointer to the `Request` object beforehand, but that does\n> >>>>>>> not seem too desirable either.\n> >>>>>>\n> >>>>>> If we are worried calling into libcamera with uur own lock held, have you\n> >>>>>> considered pushing it into the list before calling cam_->queueRequest() ? The\n> >>>>>> camera object in libcamera is supposed to be threadsafe, and we should have\n> >>>>>> strong ownership on the state and request wrap (if not some reffing is missing I\n> >>>>>> guess).\n> >>>>>\n> >>>>> The `RequestWrap` pointer is moved into the container, so one would need to take\n> >>>>> a raw pointer to the `Request`. The fact that the request completions handler\n> >>>>> unconditionally pops the first element worries me a bit. Although if things have\n> >>>>> already gone out of sync there, it might not matter.\n> >>>>>\n> >>>>>\n> >>>>>>\n> >>>>>> Nicolas\n> >>>>>>\n> >>>>>> p.s. the lock is dangerous if libcamera decides to callback from the caller\n> >>>>>> thread instead of a separate thread. So calling with lock is implementation\n> >>>>>> dependant.\n> >>>>>\n> >>>>> That's true, but currently it is guaranteed. The application developer guide\n> >>>>> mentions it explicitly.\n> >>>>>\n> >>>>\n> >>>> Is there anything else required to progress this patch? any more\n> >>>> specific concerns?\n> >>>\n> >>> From my point of view, there are no concerns, it can be merged as is.\n> >>> Before the mentioned commit (06bd05beced313) the queueRequest() and push()\n> >>> calls were done under the same lock, and the commit message also does not\n> >>> mention any correctness issues with that setup.\n> >>\n> >> Correction: I would like to slightly tweak the commit message. For example,\n> >> the last sentence is not in sync with the code.\n> >>\n> > \n> > As the locking is expected to be safe, and this fixes now multiple user\n> > bug reports - I think we should try to land this.\n> > \n> > We can always look at ways to change how request are allocated and\n> > free-d by the application and passed around on top.\n> > \n> > With the commit message updated if you wish:\n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> >>\n> >>>\n> >>>\n> >>> Regards,\n> >>> Barnabás Pőcze\n> >>>\n> >>>>\n> >>>> --\n> >>>> Kieran\n> >>>>\n> >>>>\n> >>>>\n> >>>>\n> >>>>>\n> >>>>> Regards,\n> >>>>> Barnabás Pőcze\n> >>>>>\n> >>>>>>\n> >>>>>>>\n> >>>>>>> ---\n> >>>>>>>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n> >>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n> >>>>>>>\n> >>>>>>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>>>> b/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>>>> index 79a025a57..103dfbca2 100644\n> >>>>>>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>>>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >>>>>>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n> >>>>>>>   }\n> >>>>>>>\n> >>>>>>>   GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n> >>>>>>> - cam_->queueRequest(wrap->request_.get());\n> >>>>>>>\n> >>>>>>>   {\n> >>>>>>>   GLibLocker locker(&lock_);\n> >>>>>>> + cam_->queueRequest(wrap->request_.get());\n> > \n> > But actually - now I look again - shouldn't this be checking for errors?\n> > \n> > cam_->queueRequest() can fail to queue it if the request is deemed\n> > invalid before making the asyncrhonous request. So it should not be\n> > added to this queuedRequests_ unless the above call succeeds.\n> \n> Yes, certainly, but this has never been done to my knowledge. So I think\n> this can be a separate change.\n\nSure, that can be on top.\n\n--\nKieran\n\n\n> \n> \n> > \n> >>>>>>>   queuedRequests_.push(std::move(wrap));\n> >>>>>>>   }\n> >>>>>>>\n> >>>>>>> --\n> >>>>>>> 2.51.0\n> >>>>>\n> >>>\n> >>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4983EBDCBF\n\tfor ;\n\tWed, 14 Jan 2026 09:19:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BE8F61FC3;\n\tWed, 14 Jan 2026 10:19:54 +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 2127C615B2\n\tfor ;\n\tWed, 14 Jan 2026 10:19:53 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 35A39316;\n\tWed, 14 Jan 2026 10:19:26 +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=\"MhZpxOpT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768382366;\n\tbh=VpOUHKu5tsDCB29PRvW+hXnEhC94Msukj+kD68Q+SWc=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=MhZpxOpTFIvFUGK7R688lsQtm3QakOzjBx3qnCLETCTRmgoM9GCIQG+Ld5LzDxLU+\n\t/9JKqRGAaWrWyrmHJ3gIJe0mJWqqIRyN/to1ZOkEwE21EdN9b21gP8dlDbJguO/uei\n\thisWnXxL0qUIdyaInGcMlKu+OutlkcXXelCyw6MI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t\n\t\n\t\n\t<176035565743.935713.12395014970103924661@ping.linuxembedded.co.uk>\n\t\n\t<26b4b54e-6c18-4253-88ed-3f1e85f8c894@ideasonboard.com>\n\t\n\t<176836941443.3888464.12622909927488267252@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tNicolas Dufresne , \n\tUmang Jain , libcamera-devel@lists.libcamera.org","Date":"Wed, 14 Jan 2026 09:19:50 +0000","Message-ID":"<176838239009.3486172.146792398759284862@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37665,"web_url":"https://patchwork.libcamera.org/comment/37665/","msgid":"<176846795375.1693075.2679826820622943338@ping.linuxembedded.co.uk>","date":"2026-01-15T09:05:53","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-09-09 14:12:37)\n> The moment execution enters `Camera::queueRequest()`, an application must be\n> prepared to received the corresponding `requestCompleted` signal. However,\n> the gstreamer element is currently not prepared: it queues the request,\n> takes a lock, then inserts into a list.\n> \n> If the request completion handler acquires the lock right after the queueing,\n> then it will not find the object in the list that it expects. Even potentially\n> encountering an empty list, running into undefined behaviour when trying\n> to pop from it.\n> \n> Fix that by queueing the request after the lock protected insertion.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n\nFor patchworking - even though I don't think patchwork collects these?\nBut I'll try:\n\nCloses: https://gitlab.freedesktop.org/camera/libcamera/-/issues/238\nCloses: https://gitlab.freedesktop.org/camera/libcamera/-/issues/306\n\n> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n> Signed-off-by: Barnabás Pőcze \n\n> ---\n> \n> Doing it while holding the lock is not ideal, but the alternative is\n> getting a raw pointer to the `Request` object beforehand, but that does\n> not seem too desirable either.\n> \n> ---\n> src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 79a025a57..103dfbca2 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n> }\n> \n> GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n> - cam_->queueRequest(wrap->request_.get());\n> \n> {\n> GLibLocker locker(&lock_);\n> + cam_->queueRequest(wrap->request_.get());\n> queuedRequests_.push(std::move(wrap));\n> }\n> \n> --\n> 2.51.0","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 6EA59BDCBF\n\tfor ;\n\tThu, 15 Jan 2026 09:05:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 90AF361FBC;\n\tThu, 15 Jan 2026 10:05:57 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEE2C61F61\n\tfor ;\n\tThu, 15 Jan 2026 10:05:56 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C2B84D3;\n\tThu, 15 Jan 2026 10:05:29 +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=\"Jsg2cuvP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768467929;\n\tbh=JDTEk57ZmYX3eO700EVxRYyBbrjMt1iSTaHxot7UltA=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Jsg2cuvPR/XGBBkHb5EhVBci3pAVyqPn4jthRaHfF3x4jl0kEQ+kaw4VJCF+cI3x/\n\tRqIvuYEER/9ZX/hhPkVFVJEob4HY9cO4kEE7wg27QvN7LRDBYfFKxOKsQzx9NrHnXe\n\tTgsmNe1O4x1GvhLx1Zi6xXaZ5YLnoWq99N5QetDM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tNicolas Dufresne , \n\tUmang Jain , libcamera-devel@lists.libcamera.org","Date":"Thu, 15 Jan 2026 09:05:53 +0000","Message-ID":"<176846795375.1693075.2679826820622943338@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37666,"web_url":"https://patchwork.libcamera.org/comment/37666/","msgid":"<050edc95-a1ba-4c24-9d01-21254f445834@ideasonboard.com>","date":"2026-01-15T09:08:08","subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 15. 10:05 keltezéssel, Kieran Bingham írta:\n> Quoting Barnabás Pőcze (2025-09-09 14:12:37)\n>> The moment execution enters `Camera::queueRequest()`, an application must be\n>> prepared to received the corresponding `requestCompleted` signal. However,\n>> the gstreamer element is currently not prepared: it queues the request,\n>> takes a lock, then inserts into a list.\n>>\n>> If the request completion handler acquires the lock right after the queueing,\n>> then it will not find the object in the list that it expects. Even potentially\n>> encountering an empty list, running into undefined behaviour when trying\n>> to pop from it.\n>>\n>> Fix that by queueing the request after the lock protected insertion.\n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=238\n> \n> For patchworking - even though I don't think patchwork collects these?\n> But I'll try:\n> \n> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/238\n> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/306\n\nFWIW I have the final commit in https://gitlab.freedesktop.org/camera/libcamera/-/commits/patchwork/5426\n(https://gitlab.freedesktop.org/camera/libcamera/-/commit/466b8485f620f96ca97c070a2a857fa99df9ca9e)\nwith these tags and with the modified description. I was just waiting\nfor more input in issue#306 and possibly more comments here.\n\n\n> \n>> Fixes: 06bd05beced313 (\"gstreamer: Use dedicated lock for request queues\")\n>> Signed-off-by: Barnabás Pőcze \n> \n>> ---\n>>\n>> Doing it while holding the lock is not ideal, but the alternative is\n>> getting a raw pointer to the `Request` object beforehand, but that does\n>> not seem too desirable either.\n>>\n>> ---\n>> src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 79a025a57..103dfbca2 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -214,10 +214,10 @@ int GstLibcameraSrcState::queueRequest()\n>> }\n>>\n>> GST_TRACE_OBJECT(src_, \"Requesting buffers\");\n>> - cam_->queueRequest(wrap->request_.get());\n>>\n>> {\n>> GLibLocker locker(&lock_);\n>> + cam_->queueRequest(wrap->request_.get());\n>> queuedRequests_.push(std::move(wrap));\n>> }\n>>\n>> --\n>> 2.51.0","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 357C9C3226\n\tfor ;\n\tThu, 15 Jan 2026 09:08:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B51E61FA3;\n\tThu, 15 Jan 2026 10:08:14 +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 8B8F261F61\n\tfor ;\n\tThu, 15 Jan 2026 10:08:12 +0100 (CET)","from [192.168.33.20] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9907D4D3;\n\tThu, 15 Jan 2026 10:07:44 +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=\"VnJTGL1G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768468064;\n\tbh=j4iSeYqMFQE6A0Lrjlc5XyguceJ3upz+hfhzsanIuTg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=VnJTGL1GMbqtcO7+HeSrEbpz3qB8CJpS2GIZ1jkM3gaG2FGActpoGw0SnwReSKebN\n\tppNA+/uySZbTJ8why+pUMufKNElpPfEj83zHTSOgJs13s31Roz3+g69slkNzQYWXP8\n\takZ/2y+ic/fABXcD6iGHoMNvIFn7E0syQYO4YsHw=","Message-ID":"<050edc95-a1ba-4c24-9d01-21254f445834@ideasonboard.com>","Date":"Thu, 15 Jan 2026 10:08:08 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1] gstreamer: Be prepared when queueing request","To":"Kieran Bingham ,\n\tNicolas Dufresne ,\n\tUmang Jain , libcamera-devel@lists.libcamera.org","References":"<20250909131237.1344483-1-barnabas.pocze@ideasonboard.com>\n\t<176846795375.1693075.2679826820622943338@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176846795375.1693075.2679826820622943338@ping.linuxembedded.co.uk>","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":"","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\" "}}]