[{"id":27014,"web_url":"https://patchwork.libcamera.org/comment/27014/","msgid":"<16139d89-d95f-29ff-99e2-a5b7667b6b98@ideasonboard.com>","date":"2023-05-02T21:13:45","subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Barnabas,\n\nThank you for the patch\n\nOn 5/2/23 11:28 PM, Barnabás Pőcze via libcamera-devel wrote:\n> Instead of explicitly creating a vector of StreamRole,\n> simply use an initializer list as the argument of\n> `Camera::generateConfiguration()`.\n>\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/gstreamer/gstlibcameraprovider.cpp | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> index 6eb0a0eb..4f325ad4 100644\n> --- a/src/gstreamer/gstlibcameraprovider.cpp\n> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> @@ -128,11 +128,9 @@ gst_libcamera_device_new(const std::shared_ptr &camera)\n> {\n> \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n> \tconst gchar *name = camera->id().c_str();\n> -\tStreamRoles roles;\n>\n> -\troles.push_back(StreamRole::VideoRecording);\n> -\tstd::unique_ptr config = camera->generateConfiguration(roles);\n> -\tif (!config || config->size() != roles.size()) {\n\nI would leave it as it is, as it's easier to add new roles (just a \n.push_back() call) and the adjoining pieces automatically makes it fall \ninto place..\n> +\tstd::unique_ptr config = camera->generateConfiguration({ StreamRole::VideoRecording });\n> +\tif (!config || config->size() != 1) {\n> \t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n> \t\treturn nullptr;\n> \t}\n> --\n> 2.40.1\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 46C78BD16B\n\tfor ;\n\tTue, 2 May 2023 21:13:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B2C1260538;\n\tTue, 2 May 2023 23:13:51 +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 214B360538\n\tfor ;\n\tTue, 2 May 2023 23:13:50 +0200 (CEST)","from [192.168.1.105] (unknown [103.86.18.137])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4D57B105;\n\tTue, 2 May 2023 23:13:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683062031;\n\tbh=fr6UcNQvU1uKN6s7Z2+GDqnWOMlKfcQO5N8mfnPQWhY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=UBi5nlD1yqWUOltb+VeJGLS+Yx34VwMB6eW3NOdyLttFoTLK4DZ0FauGWVyyC1S4h\n\tnR7wARXvV7h+PImaiUlDRTAvDhTnMjKDRU8Vqlsf23ma1GT2QuXRYh7UkGtLUb1+Xd\n\tgRvu+XnzNatVJITXU5cHnx6kJloNpNFKOHvCotsPdf9ARTbem6nXR+Z1gJ9kcQAPgj\n\tC3nf9FUgXStLNAu2sfJgR+zwaS385AfW1EYbD/vY9aeCF7lCuWcQWV/ltr24HFSWYy\n\tf8v7dYemIwak11ZHPMmllD4bAKBnfX7aa315/kEmzYu2IemOI7ezQKbPMiWCSbW4Ny\n\tN7+EFd0Wl094A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683062027;\n\tbh=fr6UcNQvU1uKN6s7Z2+GDqnWOMlKfcQO5N8mfnPQWhY=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=S8CvUoFyiTKAbjgeS8FIjFseWWCrHOND3F0izrTCfquNKh3Yy7O4Y3VGVxZdCXzUb\n\tcpzt5rcFuYljBOmFEkx6NzjqFG8DXVO48ftdbwo/Exki3Wdsy2jUv7zTzbaxgwPRlK\n\t56MAkvmtfcvz4nSDB1WErO4tmhIW4yDd5jR8EUFo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"S8CvUoFy\"; dkim-atps=neutral","Message-ID":"<16139d89-d95f-29ff-99e2-a5b7667b6b98@ideasonboard.com>","Date":"Wed, 3 May 2023 02:43:45 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","Content-Language":"en-US","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230502175842.70629-1-pobrn@protonmail.com>","In-Reply-To":"<20230502175842.70629-1-pobrn@protonmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27015,"web_url":"https://patchwork.libcamera.org/comment/27015/","msgid":"","date":"2023-05-03T00:26:08","subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. május 2., kedd 23:13 keltezéssel, Umang Jain írta:\n\n> [...]\n> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> > index 6eb0a0eb..4f325ad4 100644\n> > --- a/src/gstreamer/gstlibcameraprovider.cpp\n> > +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> > @@ -128,11 +128,9 @@ gst_libcamera_device_new(const std::shared_ptr &camera)\n> > {\n> > \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n> > \tconst gchar *name = camera->id().c_str();\n> > -\tStreamRoles roles;\n> >\n> > -\troles.push_back(StreamRole::VideoRecording);\n> > -\tstd::unique_ptr config = camera->generateConfiguration(roles);\n> > -\tif (!config || config->size() != roles.size()) {\n> \n> I would leave it as it is, as it's easier to add new roles (just a\n> .push_back() call) and the adjoining pieces automatically makes it fall\n> into place..\n\nThat is a fair point, I did not really explain the motivation. I would like to\npost a second version of https://patchwork.libcamera.org/patch/18285/ that would\nalso remove the `StreamRoles` alias altogether.\n\nIf this current version is undesirable, then I can incorporate this change into\nthe second version of the aforementioned patch series as follows:\n\ndiff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\nindex 6eb0a0eb..332837e4 100644\n--- a/src/gstreamer/gstlibcameraprovider.cpp\n+++ b/src/gstreamer/gstlibcameraprovider.cpp\n@@ -126,13 +126,12 @@ gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass)\n static GstDevice *\n gst_libcamera_device_new(const std::shared_ptr &camera)\n {\n+\tstatic const StreamRole roles[] = { StreamRole::VideoRecording };\n \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n \tconst gchar *name = camera->id().c_str();\n-\tStreamRoles roles;\n \n-\troles.push_back(StreamRole::VideoRecording);\n \tstd::unique_ptr config = camera->generateConfiguration(roles);\n-\tif (!config || config->size() != roles.size()) {\n+\tif (!config || config->size() != std::size(roles)) {\n \t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n \t\treturn nullptr;\n \t}\n\nwould this be then acceptable?\n\n\n> > +\tstd::unique_ptr config = camera->generateConfiguration({ StreamRole::VideoRecording });\n> > +\tif (!config || config->size() != 1) {\n> > \t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n> > \t\treturn nullptr;\n> > \t}\n> [...]\n\n\nRegards,\nBarnabás Pőcze","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 E99DDC0DA4\n\tfor ;\n\tWed, 3 May 2023 00:26:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B14B627DC;\n\tWed, 3 May 2023 02:26:18 +0200 (CEST)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 09988627DC\n\tfor ;\n\tWed, 3 May 2023 02:26:16 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683073578;\n\tbh=RlSme3Ig3Zvei/IoedWjjsllUBD6IuLeaB8hbzKLWV4=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ZXaS4w9Ne6poGDaj8tqH2JubaRcD5EhKO3TG8PJsgsKnIVN3PBwxPtUbY/IEa2mgj\n\tNBIRVVU9xvd7oaietDBvcGH4TX8HiuG4kE+FWZEDAnu433M9XZQgC+7sfV6lPiGgeC\n\txVw1zbj8rh0qgvp5obGIlKAMWlKSrVO08OGVnH7VBzaMbstGf1RvD4uUdhiGk3ewjz\n\tcJKnjoO6ESkUgE2UXtXyMaBVQ5VdEoq5FaGm7mF1vHnwUM46JE7jX2Rp0XUoPsoUib\n\tdKgwgAiz4ZW7TXhmCY0xR0XBJgJaK8BYDXJ78E3Ro/SGOtU8yayhoskPHCIVw7NpMY\n\tebCkHKaxe6t7g==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1683073575; x=1683332775;\n\tbh=8duXcOdGxW4/gU1Cic+3RkAjU4io/kkmKa2CxctWckI=;\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=pLqgObHAosdCLiJLxmTigWTLeMPTFE4VsgcIqIfxC7eTVb6sOno5bIBljxXx1CLw2\n\tiT6qehUaRZDXIvKpEbP6rYiqvWJjyOXwKZfjWTr8AOLZOLL7X/gnYGuXX8K4Nh1DJ1\n\t8X9PmqCMhj88tFjlMeg6FrosnI98NXhb/w5vjxzzkgUGpZ9iVm/ZcVlbOQkJAyB7m5\n\t2vvIqVwic2vj65b/ryAQ7CVCdnRe7zAswqL78/YaWlxLN/uFuGWA+z1yeHGqw9RyAR\n\tbp5wpe8pIfZAmJk+2KlK2fJax9bkHGcN2PSrHk24XDStl9MuL0ytZ6SZXUBseeDk+V\n\tgtls0fCnqcD8g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"pLqgObHA\"; dkim-atps=neutral","Date":"Wed, 03 May 2023 00:26:08 +0000","To":"Umang Jain ","Message-ID":"","In-Reply-To":"<16139d89-d95f-29ff-99e2-a5b7667b6b98@ideasonboard.com>","References":"<20230502175842.70629-1-pobrn@protonmail.com>\n\t<16139d89-d95f-29ff-99e2-a5b7667b6b98@ideasonboard.com>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","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","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27016,"web_url":"https://patchwork.libcamera.org/comment/27016/","msgid":"<182809c8-0e2e-6c1f-e1c6-a1ca1537def9@ideasonboard.com>","date":"2023-05-03T04:31:48","subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi\n\nOn 5/3/23 5:56 AM, Barnabás Pőcze wrote:\n> Hi\n>\n>\n> 2023. május 2., kedd 23:13 keltezéssel, Umang Jain írta:\n>\n>> [...]\n>>> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n>>> index 6eb0a0eb..4f325ad4 100644\n>>> --- a/src/gstreamer/gstlibcameraprovider.cpp\n>>> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n>>> @@ -128,11 +128,9 @@ gst_libcamera_device_new(const std::shared_ptr &camera)\n>>> {\n>>> \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n>>> \tconst gchar *name = camera->id().c_str();\n>>> -\tStreamRoles roles;\n>>>\n>>> -\troles.push_back(StreamRole::VideoRecording);\n>>> -\tstd::unique_ptr config = camera->generateConfiguration(roles);\n>>> -\tif (!config || config->size() != roles.size()) {\n>> I would leave it as it is, as it's easier to add new roles (just a\n>> .push_back() call) and the adjoining pieces automatically makes it fall\n>> into place..\n> That is a fair point, I did not really explain the motivation. I would like to\n> post a second version of https://patchwork.libcamera.org/patch/18285/ that would\n> also remove the `StreamRoles` alias altogether.\n\nIf there's are adjoining changes, I suggest clubbing all that in a \nsingle series and posting.\n\nFor e.g. I would remove `StreamRoles` from every place in the codebase \nand then remove the `StreamRoles` definition / implementation from the \nconcerned class. This provides a more holisitic view.\n>\n> If this current version is undesirable, then I can incorporate this change into\n> the second version of the aforementioned patch series as follows:\n>\n> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> index 6eb0a0eb..332837e4 100644\n> --- a/src/gstreamer/gstlibcameraprovider.cpp\n> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> @@ -126,13 +126,12 @@ gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass)\n> static GstDevice *\n> gst_libcamera_device_new(const std::shared_ptr &camera)\n> {\n> +\tstatic const StreamRole roles[] = { StreamRole::VideoRecording };\n> \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n> \tconst gchar *name = camera->id().c_str();\n> -\tStreamRoles roles;\n> \n> -\troles.push_back(StreamRole::VideoRecording);\n> \tstd::unique_ptr config = camera->generateConfiguration(roles);\n> -\tif (!config || config->size() != roles.size()) {\n> +\tif (!config || config->size() != std::size(roles)) {\n> \t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n> \t\treturn nullptr;\n> \t}\n>\n> would this be then acceptable?\n\nThis looks okay to me. But might be connected to the wider StreamRoles \ndiscussion?\n>\n>\n>>> +\tstd::unique_ptr config = camera->generateConfiguration({ StreamRole::VideoRecording });\n>>> +\tif (!config || config->size() != 1) {\n>>> \t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n>>> \t\treturn nullptr;\n>>> \t}\n>> [...]\n>\n> Regards,\n> Barnabás Pőcze","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 42C0EBD16B\n\tfor ;\n\tWed, 3 May 2023 04:31:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 89D1B633B4;\n\tWed, 3 May 2023 06:31:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4545B60544\n\tfor ;\n\tWed, 3 May 2023 06:31:54 +0200 (CEST)","from [192.168.1.105] (unknown [103.86.18.137])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 143344DE;\n\tWed, 3 May 2023 06:31:50 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683088315;\n\tbh=9zpQWoRo31hMiCbtJMCbdyvt587gTPNwajoI259PiOo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tLEobMa4FJH7IE/opJVyXmup6t8pLlryZRrMeDCKHGSO9m2ZpBdoYigMujClbQjMm\n\t0W3C8yn5cWInkHss6RSP4c1hjy7EVI25/3UuFeleQP+o6BBrm8qxLKN6m0Ud6CBAKE\n\tfqd62Gne1UmvmzbbjGFiVDqwQ8hPUL604oGZ9iKdcM+PfZkn6yumNhTKgnlvG5qEYr\n\tG+FG0gnpiNJuRsq+8Kc/C4OqF1REKXAkoKfFEfcjCu0Afj8yCjBkt9ue3JU+WN96Ib\n\tEe7StlIaQz3VR7e6iW7lGdXxsMZNLjYNmQW9KTMFvzXCrAHwRwaVFkZlAJsP6QnFt7\n\tCPixC8i97X+gw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683088311;\n\tbh=9zpQWoRo31hMiCbtJMCbdyvt587gTPNwajoI259PiOo=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=O/e+nvzcHpl0T7nAYyOiec6aDysTybOxQLebsRDfMovKkcWj4ZYgAWmT55/Kcwlyd\n\t3T758GU5vc3aC81OwVGeCbGoeFvLhk1Y01t38+h6ssjWvuV5foV5lCUJR1wsMZKoP6\n\tBkY24ErdMrX86//Bdshgw5ZslRM1zdhTzM8RpgMQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"O/e+nvzc\"; dkim-atps=neutral","Message-ID":"<182809c8-0e2e-6c1f-e1c6-a1ca1537def9@ideasonboard.com>","Date":"Wed, 3 May 2023 10:01:48 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","References":"<20230502175842.70629-1-pobrn@protonmail.com>\n\t<16139d89-d95f-29ff-99e2-a5b7667b6b98@ideasonboard.com>\n\t","Content-Language":"en-US","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27022,"web_url":"https://patchwork.libcamera.org/comment/27022/","msgid":"<00_UMCGp_3t_hKlbgk3IoeN76JoOyW8ovNMYileIrNRBv-FDcFT1t1stgyGGoTiMz_xG22BXuuYZe58T5IMeG_EftG7lQd8S34BAEfnWwyg=@protonmail.com>","date":"2023-05-03T12:31:15","subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. május 3., szerda 6:31 keltezéssel, Umang Jain írta:\n\n> [...]\n> >>> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> >>> index 6eb0a0eb..4f325ad4 100644\n> >>> --- a/src/gstreamer/gstlibcameraprovider.cpp\n> >>> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> >>> @@ -128,11 +128,9 @@ gst_libcamera_device_new(const std::shared_ptr &camera)\n> >>> {\n> >>> \tg_autoptr(GstCaps) caps = gst_caps_new_empty();\n> >>> \tconst gchar *name = camera->id().c_str();\n> >>> -\tStreamRoles roles;\n> >>>\n> >>> -\troles.push_back(StreamRole::VideoRecording);\n> >>> -\tstd::unique_ptr config = camera->generateConfiguration(roles);\n> >>> -\tif (!config || config->size() != roles.size()) {\n> >> I would leave it as it is, as it's easier to add new roles (just a\n> >> .push_back() call) and the adjoining pieces automatically makes it fall\n> >> into place..\n> > That is a fair point, I did not really explain the motivation. I would like to\n> > post a second version of https://patchwork.libcamera.org/patch/18285/ that would\n> > also remove the `StreamRoles` alias altogether.\n> \n> If there's are adjoining changes, I suggest clubbing all that in a\n> single series and posting.\n> \n> For e.g. I would remove `StreamRoles` from every place in the codebase\n> and then remove the `StreamRoles` definition / implementation from the\n> concerned class. This provides a more holisitic view.\n\nGood point. Please disregard this patch.\n\n\n> [...]\n\n\nRegards,\nBarnabás Pőcze","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 1868AC0DA4\n\tfor ;\n\tWed, 3 May 2023 12:31:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D6F6627DE;\n\tWed, 3 May 2023 14:31:35 +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 6AE2161EAF\n\tfor ;\n\tWed, 3 May 2023 14:31:34 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683117095;\n\tbh=07HPkSOEg1JeHh1jp0u38l/wEw6qJCs+tuD2fEkOX2c=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=qnPZSkSziV9y+rcLOOHqUSSWgkA5hzOoU/ffUIbBCcPZ8v+7R0K/vhdH42tkkYwCi\n\tQGmgxoitsYs9DySbBobmWP8Qp//xbgAUU5W1fueUIMc+Rz9A7/V8BbM/X+tPxEj0V1\n\t+2zopnhsHrQmj/hxrvQDUWcyH+5cKqiZR6MTF9g5xKDNMsVu6bkZjxrIOcs+OSOICU\n\t/I4xqib8lzhxRgI+V8C5JYskASNiZ452ZQdurEx/CWfFrKQDS5fq90AY5u59mBe+vD\n\t3YXzLn7Uit8f4K7KNlbGRnD1BiD2Rz8ChIOQHa4zkb3y+LbelSNkJT9/95Sc3ZOot7\n\t3uJUfad08exsg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1683117093; x=1683376293;\n\tbh=TSdfZt0bK+ywt3nbiF0916FkZjWQsyhK5mqLZmzP4To=;\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=Z624hQJ1kjLkBfDaYa//YbuhEY8TausYANVyQmp4iSDRApdIwKedrT2lMg/uq2Odl\n\tefiqw6QDIiOfWoAIDZA5hw0H5fsM0QEWo6ZhfE+i1YPa5LhEvipwVZ3Gk8ta0MGzM0\n\tqocYVsyEEFgozW8cjOfR2S/UffCSsg4/syvyUP9oM8hUkKUXNYLQOWxjPrHBX+ao/O\n\tF7/QBz91HmFjrN0hDbGVk1LEQRJcxZtubk1M4mjRGPOuU3XGcBFiq9VlIpC9KDgpNM\n\tvbXnbJLX+aQjjPuidINu1qua9wa31D2A4tO5qpGEgcHtvfUyaWdICVC8xXSrAVMQJJ\n\tKgj3D+B67+upA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"Z624hQJ1\"; dkim-atps=neutral","Date":"Wed, 03 May 2023 12:31:15 +0000","To":"Umang Jain ","Message-ID":"<00_UMCGp_3t_hKlbgk3IoeN76JoOyW8ovNMYileIrNRBv-FDcFT1t1stgyGGoTiMz_xG22BXuuYZe58T5IMeG_EftG7lQd8S34BAEfnWwyg=@protonmail.com>","In-Reply-To":"<182809c8-0e2e-6c1f-e1c6-a1ca1537def9@ideasonboard.com>","References":"<20230502175842.70629-1-pobrn@protonmail.com>\n\t<16139d89-d95f-29ff-99e2-a5b7667b6b98@ideasonboard.com>\n\t\n\t<182809c8-0e2e-6c1f-e1c6-a1ca1537def9@ideasonboard.com>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v1] gstreamer: Simplify camera config\n\tgeneration call","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","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]