[{"id":29531,"web_url":"https://patchwork.libcamera.org/comment/29531/","msgid":"<20240514191614.GB11473@pendragon.ideasonboard.com>","date":"2024-05-14T19:16:14","subject":"Re: [PATCH v1] gstreamer: Use copied camera name","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Tue, May 14, 2024 at 06:02:07PM +0000, Barnabás Pőcze wrote:\n> It seems the intent is to use the copied camera name to avoid\n> concurrency problems related to `gst_libcamera_src_set_property()`.\n> \n> However, the current code makes the copy, but does not actually\n> use it. So fix that.\n\nIndeed, the local camera_name variable seems pointless otherwise.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nDid you catch the issue by chance while reading the code, or did\nsomething trigger an investigation ?\n\n> Fixes: 58feb69f852289 (\"gst: libcamerasrc: Implement selection and acquisition\")\n> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> ---\n>  src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index a284110b..9680d809 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -377,10 +377,10 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n>  \t}\n>  \n>  \tif (camera_name) {\n> -\t\tcam = cm->get(self->camera_name);\n> +\t\tcam = cm->get(camera_name);\n>  \t\tif (!cam) {\n>  \t\t\tGST_ELEMENT_ERROR(self, RESOURCE, NOT_FOUND,\n> -\t\t\t\t\t  (\"Could not find a camera named '%s'.\", self->camera_name),\n> +\t\t\t\t\t  (\"Could not find a camera named '%s'.\", camera_name),\n>  \t\t\t\t\t  (\"libcamera::CameraMananger::get() returned nullptr\"));\n>  \t\t\treturn false;\n>  \t\t}\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B2025BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 14 May 2024 19:16:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4B8A6347E;\n\tTue, 14 May 2024 21:16:24 +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 B853063469\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 May 2024 21:16:22 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 489B822A;\n\tTue, 14 May 2024 21:16:15 +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=\"O8Fq/Qbn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715714175;\n\tbh=6q8s6okTPQ/p654/gO8eeUkFl4CZwnY4q80t1dCsVaM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O8Fq/QbnLYY27FMEDmFDMpcZjUh2O8xzYmOiaWRwkQkdXhxC7XVagpCXhCyBhjrAm\n\tKX+gZWeNg8T1v3NMLB6/hWRhug3oIOSlrcAYOgObG0twaEZnmQtyPyjFL9+VSRsOG5\n\tEFB4o+a5eULB8cvknH/Gz0CqrwUwZpdsmxlQkVLw=","Date":"Tue, 14 May 2024 22:16:14 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] gstreamer: Use copied camera name","Message-ID":"<20240514191614.GB11473@pendragon.ideasonboard.com>","References":"<20240514180206.198938-1-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240514180206.198938-1-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29535,"web_url":"https://patchwork.libcamera.org/comment/29535/","msgid":"<71ef9c879160c86fcfd9bbb74fa86fb33ad630bb.camel@ndufresne.ca>","date":"2024-05-14T20:44:21","subject":"Re: [PATCH v1] gstreamer: Use copied camera name","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hey,\n\nLe mardi 14 mai 2024 à 18:02 +0000, Barnabás Pőcze a écrit :\n> It seems the intent is to use the copied camera name to avoid\n> concurrency problems related to `gst_libcamera_src_set_property()`.\n> \n> However, the current code makes the copy, but does not actually\n> use it. So fix that.\n> \n> Fixes: 58feb69f852289 (\"gst: libcamerasrc: Implement selection and acquisition\")\n> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> ---\n>  src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index a284110b..9680d809 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -377,10 +377,10 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n>  \t}\n>  \n>  \tif (camera_name) {\n> -\t\tcam = cm->get(self->camera_name);\n> +\t\tcam = cm->get(camera_name);\n>  \t\tif (!cam) {\n>  \t\t\tGST_ELEMENT_ERROR(self, RESOURCE, NOT_FOUND,\n> -\t\t\t\t\t  (\"Could not find a camera named '%s'.\", self->camera_name),\n> +\t\t\t\t\t  (\"Could not find a camera named '%s'.\", camera_name),\n>  \t\t\t\t\t  (\"libcamera::CameraMananger::get() returned nullptr\"));\n\nWhat can I say for my defence here ... oops. The point was that we can't hold\nthe lock while calling GST_ELEMENT_ERROR(), as it sends a message on the bus,\ntaking more mutex and possibly causing issues.\n\nReviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n\n>  \t\t\treturn false;\n>  \t\t}","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5DA8CBD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 14 May 2024 20:44:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 866456347E;\n\tTue, 14 May 2024 22:44:24 +0200 (CEST)","from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com\n\t[IPv6:2607:f8b0:4864:20::82d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4559963469\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 May 2024 22:44:23 +0200 (CEST)","by mail-qt1-x82d.google.com with SMTP id\n\td75a77b69052e-43e12244dc9so19703851cf.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 May 2024 13:44:23 -0700 (PDT)","from nicolas-tpx395.lan ([2606:6d00:17:6448::7a9])\n\tby smtp.gmail.com with ESMTPSA id\n\t6a1803df08f44-6a15f1d9dd2sm56733446d6.129.2024.05.14.13.44.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 14 May 2024 13:44:21 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com\n\theader.b=\"pU7vMKAx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1715719462;\n\tx=1716324262; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=lonD3rbhMLKjFOb+lIPEjaW3/cnuxDb3Dnv+gLdr/tY=;\n\tb=pU7vMKAxtXQ/UoJRnnCbx/cZyTRx3wf6Bj7HR+ckzP4cbtF59yL5u1kw6SYNPXjqb9\n\t3B4qwo59Vp7dhKjYUMxTvfXOiE1ZKYjadICF3oWKXONn093QZsEKPxv6rL7jlOT0+058\n\tLLW5BLRvYfOHT4XXFWnRxST6wwSHGMu4koCDVRUeapw/ouUdPbIR3hlry0TXFOQJk1yV\n\t/tWlcoYGU57aaGhl3+bslylO5gSwlckri3PpBj88H6a4Ker1aAdsmGbUOH6aJl8Qh00m\n\tXDRImBH+fiCXFjjP8xOcoVRZUpl3Gq5QfDQ1JrkVz2/GW9eYhXurbMWmUI0Jv2E5PXbY\n\td0TA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1715719462; x=1716324262;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=lonD3rbhMLKjFOb+lIPEjaW3/cnuxDb3Dnv+gLdr/tY=;\n\tb=PVSgd7oDactJzDixxJm4vfYL7fa5yK/tnhWkdrMmjJZPproxO40zWjrv8qg4XnzJLV\n\tMJpQOgBeAmTLPFMpfvhu6+xs8tNFkDpoF53bJ8ThrWLMWwx7Qsi1U1yfTPDl0J0Kiynw\n\tO0b3bnm/gxTBvKG9J94BuPSrvaEPayRbNIsaJDR/QOwoKNdGnpbpMCAHW7Gqe0H2fBxC\n\tFRCM1/CyO5b56rg0Z6hcRmDWJQaqsbku4S6Hlk434vy8kVV0knjlqrGvnWGSZFSEMoPE\n\tXR6mdjv33xv+9cpIL1TKCDQuh5ZbtIl+DXFTqOlVb6hoY9cBl3rwWNbj//RBL7zitBXh\n\tjPRg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWVUSua7UTjDkkWI2xjjpb61a1A4/88fi/cp+01HfZhjBaZFSEBMp5niYFJnlHYmwR7fhIKWpcAmsHQn6ClvncVMdOzuayIiOYNVYp/dESH/2Xvfg==","X-Gm-Message-State":"AOJu0YwhFmKx1n020AHTD+osHu7zQ8m5hrbkQBsBvBJjAAvpmYMA8O0E\n\txsuBFuAG16CJqkA1yZperAOggiN3cQ9fkdhDzCq9xFuHG9DkQFeD8QGIemnnLyg=","X-Google-Smtp-Source":"AGHT+IHEm/BoHf3urIldszY3HByuRmkxbjyXCylVcFe1ExX5mim3Varnf4dolWmsFHDlSSh0hwmkHQ==","X-Received":"by 2002:a05:6214:534a:b0:6a0:cc6b:14da with SMTP id\n\t6a1803df08f44-6a168224d3dmr180331576d6.47.1715719462130; \n\tTue, 14 May 2024 13:44:22 -0700 (PDT)","Message-ID":"<71ef9c879160c86fcfd9bbb74fa86fb33ad630bb.camel@ndufresne.ca>","Subject":"Re: [PATCH v1] gstreamer: Use copied camera name","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 14 May 2024 16:44:21 -0400","In-Reply-To":"<20240514180206.198938-1-pobrn@protonmail.com>","References":"<20240514180206.198938-1-pobrn@protonmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.52.1 (3.52.1-1.fc40) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29536,"web_url":"https://patchwork.libcamera.org/comment/29536/","msgid":"<20240514205214.GP32013@pendragon.ideasonboard.com>","date":"2024-05-14T20:52:14","subject":"Re: [PATCH v1] gstreamer: Use copied camera name","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, May 14, 2024 at 04:44:21PM -0400, Nicolas Dufresne wrote:\n> Hey,\n> \n> Le mardi 14 mai 2024 à 18:02 +0000, Barnabás Pőcze a écrit :\n> > It seems the intent is to use the copied camera name to avoid\n> > concurrency problems related to `gst_libcamera_src_set_property()`.\n> > \n> > However, the current code makes the copy, but does not actually\n> > use it. So fix that.\n> > \n> > Fixes: 58feb69f852289 (\"gst: libcamerasrc: Implement selection and acquisition\")\n> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > ---\n> >  src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index a284110b..9680d809 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -377,10 +377,10 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> >  \t}\n> >  \n> >  \tif (camera_name) {\n> > -\t\tcam = cm->get(self->camera_name);\n> > +\t\tcam = cm->get(camera_name);\n> >  \t\tif (!cam) {\n> >  \t\t\tGST_ELEMENT_ERROR(self, RESOURCE, NOT_FOUND,\n> > -\t\t\t\t\t  (\"Could not find a camera named '%s'.\", self->camera_name),\n> > +\t\t\t\t\t  (\"Could not find a camera named '%s'.\", camera_name),\n> >  \t\t\t\t\t  (\"libcamera::CameraMananger::get() returned nullptr\"));\n> \n> What can I say for my defence here ... oops.\n\nYou don't need to say anything. Your contributions to libcamerasrc are\nso vast that, if you hadn't made a single mistake, I wouldn't believe\nyou are human :-)\n\n> The point was that we can't hold\n> the lock while calling GST_ELEMENT_ERROR(), as it sends a message on the bus,\n> taking more mutex and possibly causing issues.\n\nI'll record this in the commit message, I think it's useful information.\n\n--------\ngstreamer: Use copied camera name\n\nThe camera name is copied in gst_libcamera_src_open() as we can't hold\nthe lock protecting the name while calling GST_ELEMENT_ERROR(). The\nGST_ELEMENT_ERROR() macro sends a message on the bus, taking more locks\nand possibly causing issues.\n\nHowever, the current code makes the copy, but does not actually use it.\nSo fix that.\n--------\n\n> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> \n> >  \t\t\treturn false;\n> >  \t\t}","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 89464BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 14 May 2024 20:52:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1CD063469;\n\tTue, 14 May 2024 22:52:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFA6A63469\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 May 2024 22:52:22 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 53DF727C;\n\tTue, 14 May 2024 22:52:15 +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=\"RxB9YnOS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715719935;\n\tbh=BLcqceEq3Uc8gYsWSBQNt4mLTG/snSh1qNI7/4lR8lg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RxB9YnOS3WabLy79Qx+Y4IWOjuDQ3u8vEQRXM/1u1kdC8BeOfIIsJ+pojPQg2aPlv\n\tvdn1dLC+T5rGSWYw8ZPV8nlEcZHO/KJNMDr8TIZYaUn72gXhPb/NPmhlMi81xvy5FN\n\t9HPOmpdXi2Say/aZ6EbTrw85DQEfEhexetYm4gx8=","Date":"Tue, 14 May 2024 23:52:14 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] gstreamer: Use copied camera name","Message-ID":"<20240514205214.GP32013@pendragon.ideasonboard.com>","References":"<20240514180206.198938-1-pobrn@protonmail.com>\n\t<71ef9c879160c86fcfd9bbb74fa86fb33ad630bb.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<71ef9c879160c86fcfd9bbb74fa86fb33ad630bb.camel@ndufresne.ca>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29537,"web_url":"https://patchwork.libcamera.org/comment/29537/","msgid":"<HrJyThUXGEoBMK4ZD2vZuZvKcjPtFRbqPbcj_FGiHD4w073fGI_FlHjAMkwLlkdWeQ_IJ8CLRYZVc176yLCcIe9I3XNYVEi1_Qp-R_n1oNE=@protonmail.com>","date":"2024-05-14T21:52:00","subject":"Re: [PATCH v1] gstreamer: Use copied camera name","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. május 14., kedd 21:16 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta:\n\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Tue, May 14, 2024 at 06:02:07PM +0000, Barnabás Pőcze wrote:\n> > It seems the intent is to use the copied camera name to avoid\n> > concurrency problems related to `gst_libcamera_src_set_property()`.\n> >\n> > However, the current code makes the copy, but does not actually\n> > use it. So fix that.\n> \n> Indeed, the local camera_name variable seems pointless otherwise.\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> Did you catch the issue by chance while reading the code, or did\n> something trigger an investigation ?\n\nI was experimenting with some code changes, and I noticed this discrepancy during that.\n\n\n> \n> > Fixes: 58feb69f852289 (\"gst: libcamerasrc: Implement selection and acquisition\")\n> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > ---\n> >  src/gstreamer/gstlibcamerasrc.cpp | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index a284110b..9680d809 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -377,10 +377,10 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> >  \t}\n> >\n> >  \tif (camera_name) {\n> > -\t\tcam = cm->get(self->camera_name);\n> > +\t\tcam = cm->get(camera_name);\n> >  \t\tif (!cam) {\n> >  \t\t\tGST_ELEMENT_ERROR(self, RESOURCE, NOT_FOUND,\n> > -\t\t\t\t\t  (\"Could not find a camera named '%s'.\", self->camera_name),\n> > +\t\t\t\t\t  (\"Could not find a camera named '%s'.\", camera_name),\n> >  \t\t\t\t\t  (\"libcamera::CameraMananger::get() returned nullptr\"));\n> >  \t\t\treturn false;\n> >  \t\t}\n> [...]\n\n\nRegards,\nBarnabás Pőcze","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id ACB20BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 14 May 2024 21:52:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AFD906347E;\n\tTue, 14 May 2024 23:52:08 +0200 (CEST)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4DA9963469\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 May 2024 23:52:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"qZm0tM0R\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1715723526; x=1715982726;\n\tbh=oOs2xiwdphVjww3cP5pvnbgdhQMcItGkf53aU/Rueok=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=qZm0tM0R457z6JPQJNmYEVVMm6oNExsbb/LGT/WApFsLoZ6w+uXWEUdoQCvdSToG3\n\tY84UuBcNgZT3BzvMp/U/RCIGDEB3JuueFBcLwiSOKKuSxzDSTVz6kDcfkTUEWvdtOl\n\tltQcSpF3uwC9T/XNbNdR64nXZp9wt6feYWkr9G7qMm+8nM+YLBoS85vF8u+SCTZmRj\n\taad33j6lK3j/VVaMJ5MOrd/qKQiccErUBJHiN2V+JZmlTD5ZhkZGCNjSx+f1/OVkch\n\tqxjItWumt7Y+ShXSaY35LB2f78DwM5Mo6oMgtBM21yOVjRU0MgmPWXHIMw1L4oEFAU\n\tvofgvXqtUkckA==","Date":"Tue, 14 May 2024 21:52:00 +0000","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] gstreamer: Use copied camera name","Message-ID":"<HrJyThUXGEoBMK4ZD2vZuZvKcjPtFRbqPbcj_FGiHD4w073fGI_FlHjAMkwLlkdWeQ_IJ8CLRYZVc176yLCcIe9I3XNYVEi1_Qp-R_n1oNE=@protonmail.com>","In-Reply-To":"<20240514191614.GB11473@pendragon.ideasonboard.com>","References":"<20240514180206.198938-1-pobrn@protonmail.com>\n\t<20240514191614.GB11473@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"91a015140ea9fc2669ff9fcf8efea79d97c0edfc","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]