[{"id":17632,"web_url":"https://patchwork.libcamera.org/comment/17632/","msgid":"<4048411c1bdfba572981623fb2a0aac8b0dee22a.camel@ndufresne.ca>","date":"2021-06-18T17:40:15","subject":"Re: [libcamera-devel] [RFC PATCH v5] gstreamer: Added virtual\n\tfunctions needed to support request pads","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le samedi 12 juin 2021 à 13:52 +0530, Vedant Paranjape a écrit :\n> Using request pads needs virtual functions to create new request pads and\n> release the allocated pads. Added way to generate unique stream_id in\n> gst_libcamera_src_task_enter(). It failed to execute when more than one\n> pad was added.\n> \n> This patch defines the functions gst_libcamera_src_request_new_pad() and\n> gst_libcamera_src_release_pad() which handle, creating and releasing\n> request pads. Also assigns these functions to their callback\n> handlers in GstLibcameraSrcClass.\n> \n> gst_pad_create_stream_id() failed as this gstreamer element has more than 2\n> source pads, and a nullptr was passed to it's stream_id argument.\n> This function fails to auto generate a stream id for scenario of more\n> than one source pad present.\n> \n> Used a gint which increments everytime new stream_id is requested and\n> group_id to generate an unique stream_id by appending them togther as\n> %i%i.\n> \n> This doesn't enable request pad support in gstreamer element. This is\n> one of the series of changes needed to make it work.\n\nI would suggest to rework the commit message, and focus on what feature this\nadds and how to use it.\n\n> \n> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n\nThe code looks fine, I'll let you send the final one before adding by R-b. Note\nthat when importing this patch:\n\n.git/rebase-apply/patch:58: trailing whitespace.\n\t\treturn NULL;\t\t\n.git/rebase-apply/patch:60: trailing whitespace.\n\t\n.git/rebase-apply/patch:68: trailing whitespace.\n\t\n.git/rebase-apply/patch:85: trailing whitespace.\n}\t\n\nAnd also, running checkstyle.py shows some more issues:\n\n\n----------------------------------------------------------------------------------------------------------\n0e4fbbad9ffaed512593848212facffed0f4620f gstreamer: Added virtual functions needed to support request pads\n----------------------------------------------------------------------------------------------------------\n--- src/gstreamer/gstlibcamerasrc.cpp\n+++ src/gstreamer/gstlibcamerasrc.cpp\n@@ -642,9 +641,9 @@\n \tself->state = state;\n }\n \n-static GstPad*\n+static GstPad *\n gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,\n-\t\t\t\t\tconst gchar *name, [[maybe_unused]] const GstCaps *caps)\n+\t\t\t\t  const gchar *name, [[maybe_unused]] const GstCaps *caps)\n {\n \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n \tg_autoptr(GstPad) pad = NULL;\n@@ -654,44 +653,40 @@\n \tpad = gst_pad_new_from_template(templ, name);\n \tg_object_ref_sink(pad);\n \n-\tif (gst_element_add_pad(element, pad))\n+\tif (gst_element_add_pad(element, pad)) {\n+\t\tGLibLocker lock(GST_OBJECT(self));\n+\t\tself->state->srcpads_.push_back(reinterpret_cast<GstPad *>(g_object_ref(pad)));\n+\t} else {\n+\t\tGST_ELEMENT_ERROR(element, STREAM, FAILED,\n+\t\t\t\t  (\"Internal data stream error.\"),\n+\t\t\t\t  (\"Could not add pad to element\"));\n+\t\treturn NULL;\n+\t}\n+\n+\treturn reinterpret_cast<GstPad *>(g_steal_pointer(&pad));\n+}\n+\n+static void\n+gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n+{\n+\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n+\n+\tGST_DEBUG_OBJECT(self, \"Pad %\" GST_PTR_FORMAT \" being released\", pad);\n+\n \t{\n \t\tGLibLocker lock(GST_OBJECT(self));\n-\t\tself->state->srcpads_.push_back(reinterpret_cast<GstPad*>(g_object_ref(pad)));\n-\t}\n-\telse\n-\t{\n-\t\tGST_ELEMENT_ERROR (element, STREAM, FAILED,\n-\t\t\t\t\t(\"Internal data stream error.\"),\n-\t\t\t\t\t(\"Could not add pad to element\"));\n-\t\treturn NULL;\t\t\n-\t}\n-\t\n-\treturn reinterpret_cast<GstPad*>(g_steal_pointer(&pad));\n-}\n-\n-static void\n-gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n-{\n-\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n-\t\n-\tGST_DEBUG_OBJECT (self, \"Pad %\" GST_PTR_FORMAT \" being released\", pad);\n-\n-\t{\n-\t\tGLibLocker lock(GST_OBJECT(self));\n-\t\tstd::vector<GstPad*> &pads = self->state->srcpads_;\n+\t\tstd::vector<GstPad *> &pads = self->state->srcpads_;\n \t\tauto begin_iterator = pads.begin();\n \t\tauto end_iterator = pads.end();\n \t\tauto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n \n-\t\tif (pad_iterator != end_iterator)\n-\t\t{\n+\t\tif (pad_iterator != end_iterator) {\n \t\t\tg_object_unref(*pad_iterator);\n \t\t\tpads.erase(pad_iterator);\n \t\t}\n \t}\n \tgst_element_remove_pad(element, pad);\n-}\t\n+}\n \n static void\n gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n---\n2 potential issues detected, please review\n\n\n> ---\n> In _request_new_pad and _release_pad, a GLibLock is taken on\n> self->state. But since self->state is a C++ object,\n> GST_OBJECT(self->state) fails and returns null, so GLibLocker throws a\n> warning \"invalid cast from '(null)' to 'GstObject'. This means that it\n> actually fails to get lock the resource that needs to be protected,\n> i.e., the srcpads_ vector.\n> \n> The following changes were tested using the following test code: https://pastebin.com/gNDF1cyG\n> \n>  src/gstreamer/gstlibcamerasrc.cpp | 57 ++++++++++++++++++++++++++++++-\n>  1 file changed, 56 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index ccc61590..1614971d 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -361,10 +361,12 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>  \tGST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n>  \n>  \tguint group_id = gst_util_group_id_next();\n> +\tgint stream_id_num = 0;\n>  \tStreamRoles roles;\n>  \tfor (GstPad *srcpad : state->srcpads_) {\n>  \t\t/* Create stream-id and push stream-start. */\n> -\t\tg_autofree gchar *stream_id = gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), nullptr);\n> +\t\tg_autofree gchar *stream_id_intermediate = g_strdup_printf(\"%i%i\", group_id, stream_id_num++);\n> +\t\tg_autofree gchar *stream_id = gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), stream_id_intermediate);\n>  \t\tGstEvent *event = gst_event_new_stream_start(stream_id);\n>  \t\tgst_event_set_group_id(event, group_id);\n>  \t\tgst_pad_push_event(srcpad, event);\n> @@ -640,6 +642,57 @@ gst_libcamera_src_init(GstLibcameraSrc *self)\n>  \tself->state = state;\n>  }\n>  \n> +static GstPad*\n> +gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,\n> +\t\t\t\t\tconst gchar *name, [[maybe_unused]] const GstCaps *caps)\n> +{\n> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> +\tg_autoptr(GstPad) pad = NULL;\n> +\n> +\tGST_DEBUG_OBJECT(self, \"new request pad created\");\n> +\n> +\tpad = gst_pad_new_from_template(templ, name);\n> +\tg_object_ref_sink(pad);\n> +\n> +\tif (gst_element_add_pad(element, pad))\n> +\t{\n> +\t\tGLibLocker lock(GST_OBJECT(self));\n> +\t\tself->state->srcpads_.push_back(reinterpret_cast<GstPad*>(g_object_ref(pad)));\n> +\t}\n> +\telse\n> +\t{\n> +\t\tGST_ELEMENT_ERROR (element, STREAM, FAILED,\n> +\t\t\t\t\t(\"Internal data stream error.\"),\n> +\t\t\t\t\t(\"Could not add pad to element\"));\n> +\t\treturn NULL;\t\t\n> +\t}\n> +\t\n> +\treturn reinterpret_cast<GstPad*>(g_steal_pointer(&pad));\n> +}\n> +\n> +static void\n> +gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n> +{\n> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> +\t\n> +\tGST_DEBUG_OBJECT (self, \"Pad %\" GST_PTR_FORMAT \" being released\", pad);\n> +\n> +\t{\n> +\t\tGLibLocker lock(GST_OBJECT(self));\n> +\t\tstd::vector<GstPad*> &pads = self->state->srcpads_;\n> +\t\tauto begin_iterator = pads.begin();\n> +\t\tauto end_iterator = pads.end();\n> +\t\tauto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n> +\n> +\t\tif (pad_iterator != end_iterator)\n> +\t\t{\n> +\t\t\tg_object_unref(*pad_iterator);\n> +\t\t\tpads.erase(pad_iterator);\n> +\t\t}\n> +\t}\n> +\tgst_element_remove_pad(element, pad);\n> +}\t\n> +\n>  static void\n>  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>  {\n> @@ -650,6 +703,8 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>  \tobject_class->get_property = gst_libcamera_src_get_property;\n>  \tobject_class->finalize = gst_libcamera_src_finalize;\n>  \n> +\telement_class->request_new_pad = gst_libcamera_src_request_new_pad;\n> +\telement_class->release_pad = gst_libcamera_src_release_pad;\n>  \telement_class->change_state = gst_libcamera_src_change_state;\n>  \n>  \tgst_element_class_set_metadata(element_class,","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 357F8BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 18 Jun 2021 17:40:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 746D068945;\n\tFri, 18 Jun 2021 19:40:20 +0200 (CEST)","from mail-qv1-xf35.google.com (mail-qv1-xf35.google.com\n\t[IPv6:2607:f8b0:4864:20::f35])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C7FFB60298\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jun 2021 19:40:18 +0200 (CEST)","by mail-qv1-xf35.google.com with SMTP id r19so3959231qvw.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jun 2021 10:40:18 -0700 (PDT)","from nicolas-tpx395.localdomain (173-246-12-168.qc.cable.ebox.net.\n\t[173.246.12.168]) by smtp.gmail.com with ESMTPSA id\n\t16sm5538403qty.15.2021.06.18.10.40.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 18 Jun 2021 10:40:16 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"l428MpqQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:user-agent:mime-version:content-transfer-encoding;\n\tbh=VZ5HXJHmxKE7kqS50MQepBcXxiiPQHBZ2hL+r22fDl8=;\n\tb=l428MpqQ18ESJ6XQM8PY7c6MCWqmt3zjy1vIn6Tsk6SV0hDwR5qPFCv8TTcOgzc+v0\n\tPb8NfmwuSAG11pOsU+qV/tONTr9W19hW8iRkNQXf5QWSLuKLi9aXVhT+opXI1j9IEsXc\n\txo0VRU8tD9Y69sglpzcnJpsv6NTx1LYNxq4uCJVtKxsbQDbiYqOuXydmPwz40LsMklec\n\tSt5aTwR2pfIQm14psKUuPu8wzrTFf6QgtOWg6fz2sTQ2RzB6Ft5jlTfU9x/uX+6VqbLh\n\tTKhTJUdJmlLpNjDSX0n8LOp5dwLyrd+WIZALpACeN7o3cnhBRLaSaVpDvwKlhUy6G165\n\tgGUw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=VZ5HXJHmxKE7kqS50MQepBcXxiiPQHBZ2hL+r22fDl8=;\n\tb=DIMRpGNFpPeZ8lqneyYp4r5r2xLMf3bFEGrFvmiLBuL5CSV52jeAIZemieO2Xf3/aS\n\tAtM7Cjtwlwjuy9JuztXHB0YV3eefQfRLctOweFj6N73CPZVFlPei0w5oUxHAuUMc5PMc\n\tuQGwlmex7cBf7SsWS1b8ebW5vQvQbXllQwaUriFtNVXx713IZV4RAnt7RtrdhjjDPUtM\n\tYfqnXS80cFFj7HnjNiKsuQgUtzDx/Ll+Q75tTyj4jpdwlwsVd8DciWh8yLdyyj5SnNMY\n\tyCkllm6nVLp6wizIE92nZzqUwDFnnqWoONyfCjuKR6XVYPJWXDMJyWcuKKVSVuOlR0Wq\n\t4EJw==","X-Gm-Message-State":"AOAM533gwx/wp1AJeCfV5Gml62ptZ5hyhDiv3RpJoRFqF/dpdPhP3moP\n\tnbjqfs0wJCqsgG6kfNfUS2mAPg==","X-Google-Smtp-Source":"ABdhPJyKPki5URFbPAyozo1WcyDLJJcgtVVUuZx6bi/n6i2mW8yWxn6Z76zr5k7ZYl1e9TnHnrD/8g==","X-Received":"by 2002:a0c:e644:: with SMTP id c4mr6481906qvn.33.1624038017381; \n\tFri, 18 Jun 2021 10:40:17 -0700 (PDT)","Message-ID":"<4048411c1bdfba572981623fb2a0aac8b0dee22a.camel@ndufresne.ca>","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>, \n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 18 Jun 2021 13:40:15 -0400","In-Reply-To":"<20210612082231.111138-1-vedantparanjape160201@gmail.com>","References":"<20210612082231.111138-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.1 (3.40.1-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC PATCH v5] gstreamer: Added virtual\n\tfunctions needed to support request pads","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":17633,"web_url":"https://patchwork.libcamera.org/comment/17633/","msgid":"<YMzd2crC9n1YM7rV@pendragon.ideasonboard.com>","date":"2021-06-18T17:54:33","subject":"Re: [libcamera-devel] [RFC PATCH v5] gstreamer: Added virtual\n\tfunctions needed to support request pads","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jun 18, 2021 at 01:40:15PM -0400, Nicolas Dufresne wrote:\n> Le samedi 12 juin 2021 à 13:52 +0530, Vedant Paranjape a écrit :\n> > Using request pads needs virtual functions to create new request pads and\n> > release the allocated pads. Added way to generate unique stream_id in\n> > gst_libcamera_src_task_enter(). It failed to execute when more than one\n> > pad was added.\n> > \n> > This patch defines the functions gst_libcamera_src_request_new_pad() and\n> > gst_libcamera_src_release_pad() which handle, creating and releasing\n> > request pads. Also assigns these functions to their callback\n> > handlers in GstLibcameraSrcClass.\n> > \n> > gst_pad_create_stream_id() failed as this gstreamer element has more than 2\n> > source pads, and a nullptr was passed to it's stream_id argument.\n> > This function fails to auto generate a stream id for scenario of more\n> > than one source pad present.\n> > \n> > Used a gint which increments everytime new stream_id is requested and\n> > group_id to generate an unique stream_id by appending them togther as\n> > %i%i.\n> > \n> > This doesn't enable request pad support in gstreamer element. This is\n> > one of the series of changes needed to make it work.\n> \n> I would suggest to rework the commit message, and focus on what feature this\n> adds and how to use it.\n> \n> > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> \n> The code looks fine, I'll let you send the final one before adding by R-b. Note\n> that when importing this patch:\n> \n> .git/rebase-apply/patch:58: trailing whitespace.\n> \t\treturn NULL;\t\t\n> .git/rebase-apply/patch:60: trailing whitespace.\n> \t\n> .git/rebase-apply/patch:68: trailing whitespace.\n> \t\n> .git/rebase-apply/patch:85: trailing whitespace.\n> }\t\n> \n> And also, running checkstyle.py shows some more issues:\n\nThis can be automated by using the pre- or post-commit hooks:\n\ncp utils/hooks/post-commit .git/hooks/\n\nor\n\ncp utils/hooks/pre-commit .git/hooks/\n\nNote that checkstyle.py can only provide a best effort, it's fine to\nignore some of its suggestions if there's a valid reason to do so.\n\n> ----------------------------------------------------------------------------------------------------------\n> 0e4fbbad9ffaed512593848212facffed0f4620f gstreamer: Added virtual functions needed to support request pads\n> ----------------------------------------------------------------------------------------------------------\n> --- src/gstreamer/gstlibcamerasrc.cpp\n> +++ src/gstreamer/gstlibcamerasrc.cpp\n> @@ -642,9 +641,9 @@\n>  \tself->state = state;\n>  }\n>  \n> -static GstPad*\n> +static GstPad *\n>  gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,\n> -\t\t\t\t\tconst gchar *name, [[maybe_unused]] const GstCaps *caps)\n> +\t\t\t\t  const gchar *name, [[maybe_unused]] const GstCaps *caps)\n>  {\n>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n>  \tg_autoptr(GstPad) pad = NULL;\n> @@ -654,44 +653,40 @@\n>  \tpad = gst_pad_new_from_template(templ, name);\n>  \tg_object_ref_sink(pad);\n>  \n> -\tif (gst_element_add_pad(element, pad))\n> +\tif (gst_element_add_pad(element, pad)) {\n> +\t\tGLibLocker lock(GST_OBJECT(self));\n> +\t\tself->state->srcpads_.push_back(reinterpret_cast<GstPad *>(g_object_ref(pad)));\n> +\t} else {\n> +\t\tGST_ELEMENT_ERROR(element, STREAM, FAILED,\n> +\t\t\t\t  (\"Internal data stream error.\"),\n> +\t\t\t\t  (\"Could not add pad to element\"));\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\treturn reinterpret_cast<GstPad *>(g_steal_pointer(&pad));\n> +}\n> +\n> +static void\n> +gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n> +{\n> +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> +\n> +\tGST_DEBUG_OBJECT(self, \"Pad %\" GST_PTR_FORMAT \" being released\", pad);\n> +\n>  \t{\n>  \t\tGLibLocker lock(GST_OBJECT(self));\n> -\t\tself->state->srcpads_.push_back(reinterpret_cast<GstPad*>(g_object_ref(pad)));\n> -\t}\n> -\telse\n> -\t{\n> -\t\tGST_ELEMENT_ERROR (element, STREAM, FAILED,\n> -\t\t\t\t\t(\"Internal data stream error.\"),\n> -\t\t\t\t\t(\"Could not add pad to element\"));\n> -\t\treturn NULL;\t\t\n> -\t}\n> -\t\n> -\treturn reinterpret_cast<GstPad*>(g_steal_pointer(&pad));\n> -}\n> -\n> -static void\n> -gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n> -{\n> -\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> -\t\n> -\tGST_DEBUG_OBJECT (self, \"Pad %\" GST_PTR_FORMAT \" being released\", pad);\n> -\n> -\t{\n> -\t\tGLibLocker lock(GST_OBJECT(self));\n> -\t\tstd::vector<GstPad*> &pads = self->state->srcpads_;\n> +\t\tstd::vector<GstPad *> &pads = self->state->srcpads_;\n>  \t\tauto begin_iterator = pads.begin();\n>  \t\tauto end_iterator = pads.end();\n>  \t\tauto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n>  \n> -\t\tif (pad_iterator != end_iterator)\n> -\t\t{\n> +\t\tif (pad_iterator != end_iterator) {\n>  \t\t\tg_object_unref(*pad_iterator);\n>  \t\t\tpads.erase(pad_iterator);\n>  \t\t}\n>  \t}\n>  \tgst_element_remove_pad(element, pad);\n> -}\t\n> +}\n>  \n>  static void\n>  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> ---\n> 2 potential issues detected, please review\n> \n> \n> > ---\n> > In _request_new_pad and _release_pad, a GLibLock is taken on\n> > self->state. But since self->state is a C++ object,\n> > GST_OBJECT(self->state) fails and returns null, so GLibLocker throws a\n> > warning \"invalid cast from '(null)' to 'GstObject'. This means that it\n> > actually fails to get lock the resource that needs to be protected,\n> > i.e., the srcpads_ vector.\n> > \n> > The following changes were tested using the following test code: https://pastebin.com/gNDF1cyG\n> > \n> >  src/gstreamer/gstlibcamerasrc.cpp | 57 ++++++++++++++++++++++++++++++-\n> >  1 file changed, 56 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index ccc61590..1614971d 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -361,10 +361,12 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >  \tGST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n> >  \n> >  \tguint group_id = gst_util_group_id_next();\n> > +\tgint stream_id_num = 0;\n> >  \tStreamRoles roles;\n> >  \tfor (GstPad *srcpad : state->srcpads_) {\n> >  \t\t/* Create stream-id and push stream-start. */\n> > -\t\tg_autofree gchar *stream_id = gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), nullptr);\n> > +\t\tg_autofree gchar *stream_id_intermediate = g_strdup_printf(\"%i%i\", group_id, stream_id_num++);\n> > +\t\tg_autofree gchar *stream_id = gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), stream_id_intermediate);\n> >  \t\tGstEvent *event = gst_event_new_stream_start(stream_id);\n> >  \t\tgst_event_set_group_id(event, group_id);\n> >  \t\tgst_pad_push_event(srcpad, event);\n> > @@ -640,6 +642,57 @@ gst_libcamera_src_init(GstLibcameraSrc *self)\n> >  \tself->state = state;\n> >  }\n> >  \n> > +static GstPad*\n> > +gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,\n> > +\t\t\t\t\tconst gchar *name, [[maybe_unused]] const GstCaps *caps)\n> > +{\n> > +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> > +\tg_autoptr(GstPad) pad = NULL;\n> > +\n> > +\tGST_DEBUG_OBJECT(self, \"new request pad created\");\n> > +\n> > +\tpad = gst_pad_new_from_template(templ, name);\n> > +\tg_object_ref_sink(pad);\n> > +\n> > +\tif (gst_element_add_pad(element, pad))\n> > +\t{\n> > +\t\tGLibLocker lock(GST_OBJECT(self));\n> > +\t\tself->state->srcpads_.push_back(reinterpret_cast<GstPad*>(g_object_ref(pad)));\n> > +\t}\n> > +\telse\n> > +\t{\n> > +\t\tGST_ELEMENT_ERROR (element, STREAM, FAILED,\n> > +\t\t\t\t\t(\"Internal data stream error.\"),\n> > +\t\t\t\t\t(\"Could not add pad to element\"));\n> > +\t\treturn NULL;\t\t\n> > +\t}\n> > +\t\n> > +\treturn reinterpret_cast<GstPad*>(g_steal_pointer(&pad));\n> > +}\n> > +\n> > +static void\n> > +gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n> > +{\n> > +\tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);\n> > +\t\n> > +\tGST_DEBUG_OBJECT (self, \"Pad %\" GST_PTR_FORMAT \" being released\", pad);\n> > +\n> > +\t{\n> > +\t\tGLibLocker lock(GST_OBJECT(self));\n> > +\t\tstd::vector<GstPad*> &pads = self->state->srcpads_;\n> > +\t\tauto begin_iterator = pads.begin();\n> > +\t\tauto end_iterator = pads.end();\n> > +\t\tauto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n> > +\n> > +\t\tif (pad_iterator != end_iterator)\n> > +\t\t{\n> > +\t\t\tg_object_unref(*pad_iterator);\n> > +\t\t\tpads.erase(pad_iterator);\n> > +\t\t}\n> > +\t}\n> > +\tgst_element_remove_pad(element, pad);\n> > +}\t\n> > +\n> >  static void\n> >  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >  {\n> > @@ -650,6 +703,8 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >  \tobject_class->get_property = gst_libcamera_src_get_property;\n> >  \tobject_class->finalize = gst_libcamera_src_finalize;\n> >  \n> > +\telement_class->request_new_pad = gst_libcamera_src_request_new_pad;\n> > +\telement_class->release_pad = gst_libcamera_src_release_pad;\n> >  \telement_class->change_state = gst_libcamera_src_change_state;\n> >  \n> >  \tgst_element_class_set_metadata(element_class,","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 B6278C3218\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 18 Jun 2021 17:54:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2226C6050D;\n\tFri, 18 Jun 2021 19:54:59 +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 BE18660298\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jun 2021 19:54:57 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 379739C7;\n\tFri, 18 Jun 2021 19:54:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"wTCIuvpB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624038897;\n\tbh=xMPB+tVDR4AXUExWOhd3RCTObhEDcs21APc/EtefDqc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wTCIuvpBZMl9O/Xaj+LLTZal1LALJSEE7uADki4T9ZLvBcknZ3msw709gOkjH7euf\n\tiTcKmwSrd/kbrIAi0bZSQHo+huLjqUiw/MtWSqNiC5lNgNGJxhnuCPefeG1d4sIjMt\n\toy3+SiYIPbEPMRrhtsVCyCJ7sybcDmVfgKKMmgBo=","Date":"Fri, 18 Jun 2021 20:54:33 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Message-ID":"<YMzd2crC9n1YM7rV@pendragon.ideasonboard.com>","References":"<20210612082231.111138-1-vedantparanjape160201@gmail.com>\n\t<4048411c1bdfba572981623fb2a0aac8b0dee22a.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<4048411c1bdfba572981623fb2a0aac8b0dee22a.camel@ndufresne.ca>","Subject":"Re: [libcamera-devel] [RFC PATCH v5] gstreamer: Added virtual\n\tfunctions needed to support request pads","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>","Cc":"libcamera-devel@lists.libcamera.org,\n\tVedant Paranjape <vedantparanjape160201@gmail.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]