[{"id":19649,"web_url":"https://patchwork.libcamera.org/comment/19649/","msgid":"<20210914013302.GB1857410@pyrite.rasen.tech>","date":"2021-09-14T01:33:02","subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Vedant,\n\nOn Fri, Sep 10, 2021 at 11:41:52PM +0530, Vedant Paranjape wrote:\n> This patch adds a test to test if multi stream using libcamera's\n> gstreamer element works.\n> \n> Signed-off-by: Vedant Paranjape \n> ---\n> .../gstreamer/gstreamer_multi_stream_test.cpp | 138 ++++++++++++++++++\n> test/gstreamer/meson.build | 1 +\n> 2 files changed, 139 insertions(+)\n> create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp\n> \n> diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> new file mode 100644\n> index 000000000000..e5c909c85da2\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> @@ -0,0 +1,138 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2021, Vedant Paranjape\n> + *\n> + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/source_paths.h\"\n> +\n> +#include \n> +\n> +#include \"gstreamer_test.h\"\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +\n> +class GstreamerMultiStreamTest : public GstreamerTest, public Test\n> +{\n> +public:\n> +\tGstreamerMultiStreamTest()\n> +\t\t: GstreamerTest()\n> +\t{\n> +\t}\n> +\n> +protected:\n> +\tint init() override\n> +\t{\n> +\t\tif (status_ != TestPass)\n> +\t\t\treturn status_;\n> +\n> +\t\t/* Check if platform support multistream output */\n> +\t\tlibcamera::CameraManager cm;\n> +\t\tcm.start();\n> +\t\tbool cameraFound = false;\n> +\t\tfor (auto &i : cm.cameras()) {\n\nThis isn't an index, it's a Camera object, so I'd s/i/camera/\n\n> +\t\t\tif (i->streams().size() > 1) {\n> +\t\t\t\tcameraName = i->id();\n> +\t\t\t\tcameraFound = true;\n> +\t\t\t\tcm.stop();\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tif (!cameraFound) {\n> +\t\t\tcm.stop();\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tg_autoptr(GstElement) convert0 = gst_element_factory_make(\"videoconvert\", \"convert0\");\n> +\t\tg_autoptr(GstElement) convert1 = gst_element_factory_make(\"videoconvert\", \"convert1\");\n> +\t\tg_autoptr(GstElement) sink0 = gst_element_factory_make(\"fakesink\", \"sink0\");\n> +\t\tg_autoptr(GstElement) sink1 = gst_element_factory_make(\"fakesink\", \"sink1\");\n> +\t\tg_autoptr(GstElement) queue0 = gst_element_factory_make(\"queue\", \"queue0\");\n> +\t\tg_autoptr(GstElement) queue1 = gst_element_factory_make(\"queue\", \"queue1\");\n> +\t\tg_object_ref_sink(convert0);\n> +\t\tg_object_ref_sink(convert1);\n> +\t\tg_object_ref_sink(sink0);\n> +\t\tg_object_ref_sink(sink1);\n> +\t\tg_object_ref_sink(queue0);\n> +\t\tg_object_ref_sink(queue1);\n> +\n> +\t\tif (!convert0 || !convert1 || !sink0 || !sink1 || !queue0 || !queue1) {\n> +\t\t\tg_printerr(\"Not all elements could be created. %p.%p.%p.%p.%p.%p\\n\",\n> +\t\t\t\t convert0, convert1, sink0, sink1, queue0, queue1);\n> +\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tconvert0_ = reinterpret_cast(g_steal_pointer(&convert0));\n> +\t\tconvert1_ = reinterpret_cast(g_steal_pointer(&convert1));\n> +\t\tsink0_ = reinterpret_cast(g_steal_pointer(&sink0));\n> +\t\tsink1_ = reinterpret_cast(g_steal_pointer(&sink1));\n> +\t\tqueue0_ = reinterpret_cast(g_steal_pointer(&queue0));\n> +\t\tqueue1_ = reinterpret_cast(g_steal_pointer(&queue1));\n> +\n> +\t\tif (createPipeline() != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint run() override\n> +\t{\n> +\t\tg_object_set(libcameraSrc_, \"camera-name\", cameraName.c_str(), NULL);\n> +\n> +\t\t/* Build the pipeline */\n> +\t\tgst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, queue0_, queue1_,\n> +\t\t\t\t\t\t\t\t\tconvert0_, convert1_, sink0_, sink1_, NULL);\n\nIndentation.\n\n> +\t\tif (gst_element_link_many(queue0_, convert0_, sink0_, NULL) != TRUE\n> +\t\t\t|| gst_element_link_many(queue1_, convert1_, sink1_, NULL) != TRUE) {\n> +\t\t\tg_printerr(\"Elements could not be linked.\\n\");\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tg_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, \"src\");\n> +\t\tg_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n> +\t\tGstPad *queue0_sink_pad = gst_element_get_static_pad(queue0_, \"sink\");\n> +\t\tGstPad *queue1_sink_pad = gst_element_get_static_pad(queue1_, \"sink\");\n> +\n> +\t\tif (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK\n> +\t\t\t|| gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n\ncheckstyle says to put this on one line but you can do:\n\n+\t\tif (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK ||\n+\t\t gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n\nI think that looks nicer.\n\n> +\t\t\tif (queue0_sink_pad)\n> +\t\t\t\tgst_object_unref(queue0_sink_pad);\n> +\t\t\tif (queue1_sink_pad)\n> +\t\t\t\tgst_object_unref(queue1_sink_pad);\n> +\t\t\tg_printerr(\"Pads could not be linked.\\n\");\n> +\t\t\treturn TestFail;\n> +\t\t}\n> + gst_object_unref(queue0_sink_pad);\n> + gst_object_unref(queue1_sink_pad);\n\ncheckstyle ?\n\n> +\n> +\t\tif (startPipeline() != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (processEvent() != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +private:\n> +\tstd::string cameraName;\n> +\tGstElement *convert0_;\n> +\tGstElement *convert1_;\n> +\tGstElement *sink0_;\n> +\tGstElement *sink1_;\n> +\tGstElement *queue0_;\n> +\tGstElement *queue1_;\n> +};\n> +\n> +TEST_REGISTER(GstreamerMultiStreamTest)\n> diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build\n> index aca53b920365..13652e87d05c 100644\n> --- a/test/gstreamer/meson.build\n> +++ b/test/gstreamer/meson.build\n> @@ -6,6 +6,7 @@ endif\n> \n> gstreamer_tests = [\n> ['single_stream_test', 'gstreamer_single_stream_test.cpp'],\n> + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'],\n\ncheckstyle.\n\n\nWith those fixed, looks good.\n\nReviewed-by: Paul Elder \n\nIt runs fine too (on uvc skips properly, and it passes on raspberrypi).\n\nTested-by: Paul Elder \n\n> ]\n> gstreamer_dep = dependency('gstreamer-1.0', required: true)\n> \n> -- \n> 2.25.1\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 41595BDB1D\n\tfor ;\n\tTue, 14 Sep 2021 01:33:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0694269184;\n\tTue, 14 Sep 2021 03:33:12 +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 AE8806917A\n\tfor ;\n\tTue, 14 Sep 2021 03:33:10 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 33F282A5;\n\tTue, 14 Sep 2021 03:33:08 +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=\"mLmg5FYA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631583190;\n\tbh=1NCRt/rjShINk+o4wd453cDn5mA3M7aF8jcjhbhK4os=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mLmg5FYAkLqcebxyXD5uQ20PCJMQ1tXn8+YzqhDljOM2+x8UCy+DqArGtCqZoZJFn\n\t+IFs0Kv4Yf6W2Ij0BuhryUOxi7tkfqlLAZFHC2EST5unPwvgi6OKVhK2UGqTcO1HBE\n\t6NXtitbyE4AYhuh3eFpf/23lgxOmZzgBcc2wi5Vo=","Date":"Tue, 14 Sep 2021 10:33:02 +0900","From":"paul.elder@ideasonboard.com","To":"Vedant Paranjape ","Message-ID":"<20210914013302.GB1857410@pyrite.rasen.tech>","References":"<20210910181152.878132-1-vedantparanjape160201@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210910181152.878132-1-vedantparanjape160201@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19666,"web_url":"https://patchwork.libcamera.org/comment/19666/","msgid":"","date":"2021-09-14T05:34:12","subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hi Paul,\nThanks for the review.\n\nThe checkstyle was asking to out code in long lines which cross 80 char\nlimit, should I still go for it?\n\nOn Tue, 14 Sep, 2021, 07:03 , wrote:\n\n> Hi Vedant,\n>\n> On Fri, Sep 10, 2021 at 11:41:52PM +0530, Vedant Paranjape wrote:\n> > This patch adds a test to test if multi stream using libcamera's\n> > gstreamer element works.\n> >\n> > Signed-off-by: Vedant Paranjape \n> > ---\n> > .../gstreamer/gstreamer_multi_stream_test.cpp | 138 ++++++++++++++++++\n> > test/gstreamer/meson.build | 1 +\n> > 2 files changed, 139 insertions(+)\n> > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp\n> >\n> > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp\n> b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > new file mode 100644\n> > index 000000000000..e5c909c85da2\n> > --- /dev/null\n> > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > @@ -0,0 +1,138 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Vedant Paranjape\n> > + *\n> > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/source_paths.h\"\n> > +\n> > +#include \n> > +\n> > +#include \"gstreamer_test.h\"\n> > +#include \"test.h\"\n> > +\n> > +using namespace std;\n> > +\n> > +class GstreamerMultiStreamTest : public GstreamerTest, public Test\n> > +{\n> > +public:\n> > + GstreamerMultiStreamTest()\n> > + : GstreamerTest()\n> > + {\n> > + }\n> > +\n> > +protected:\n> > + int init() override\n> > + {\n> > + if (status_ != TestPass)\n> > + return status_;\n> > +\n> > + /* Check if platform support multistream output */\n> > + libcamera::CameraManager cm;\n> > + cm.start();\n> > + bool cameraFound = false;\n> > + for (auto &i : cm.cameras()) {\n>\n> This isn't an index, it's a Camera object, so I'd s/i/camera/\n>\n> > + if (i->streams().size() > 1) {\n> > + cameraName = i->id();\n> > + cameraFound = true;\n> > + cm.stop();\n> > + break;\n> > + }\n> > + }\n> > +\n> > + if (!cameraFound) {\n> > + cm.stop();\n> > + return TestSkip;\n> > + }\n> > +\n> > + g_autoptr(GstElement) convert0 =\n> gst_element_factory_make(\"videoconvert\", \"convert0\");\n> > + g_autoptr(GstElement) convert1 =\n> gst_element_factory_make(\"videoconvert\", \"convert1\");\n> > + g_autoptr(GstElement) sink0 =\n> gst_element_factory_make(\"fakesink\", \"sink0\");\n> > + g_autoptr(GstElement) sink1 =\n> gst_element_factory_make(\"fakesink\", \"sink1\");\n> > + g_autoptr(GstElement) queue0 =\n> gst_element_factory_make(\"queue\", \"queue0\");\n> > + g_autoptr(GstElement) queue1 =\n> gst_element_factory_make(\"queue\", \"queue1\");\n> > + g_object_ref_sink(convert0);\n> > + g_object_ref_sink(convert1);\n> > + g_object_ref_sink(sink0);\n> > + g_object_ref_sink(sink1);\n> > + g_object_ref_sink(queue0);\n> > + g_object_ref_sink(queue1);\n> > +\n> > + if (!convert0 || !convert1 || !sink0 || !sink1 || !queue0\n> || !queue1) {\n> > + g_printerr(\"Not all elements could be created.\n> %p.%p.%p.%p.%p.%p\\n\",\n> > + convert0, convert1, sink0, sink1,\n> queue0, queue1);\n> > +\n> > + return TestFail;\n> > + }\n> > +\n> > + convert0_ = reinterpret_cast *>(g_steal_pointer(&convert0));\n> > + convert1_ = reinterpret_cast *>(g_steal_pointer(&convert1));\n> > + sink0_ = reinterpret_cast *>(g_steal_pointer(&sink0));\n> > + sink1_ = reinterpret_cast *>(g_steal_pointer(&sink1));\n> > + queue0_ = reinterpret_cast *>(g_steal_pointer(&queue0));\n> > + queue1_ = reinterpret_cast *>(g_steal_pointer(&queue1));\n> > +\n> > + if (createPipeline() != TestPass)\n> > + return TestFail;\n> > +\n> > + return TestPass;\n> > + }\n> > +\n> > + int run() override\n> > + {\n> > + g_object_set(libcameraSrc_, \"camera-name\",\n> cameraName.c_str(), NULL);\n> > +\n> > + /* Build the pipeline */\n> > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> queue0_, queue1_,\n> > +\n> convert0_, convert1_, sink0_, sink1_, NULL);\n>\n> Indentation.\n>\n> > + if (gst_element_link_many(queue0_, convert0_, sink0_,\n> NULL) != TRUE\n> > + || gst_element_link_many(queue1_, convert1_,\n> sink1_, NULL) != TRUE) {\n> > + g_printerr(\"Elements could not be linked.\\n\");\n> > + return TestFail;\n> > + }\n> > +\n> > + g_autoptr(GstPad) src_pad =\n> gst_element_get_static_pad(libcameraSrc_, \"src\");\n> > + g_autoptr(GstPad) request_pad =\n> gst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n> > + GstPad *queue0_sink_pad =\n> gst_element_get_static_pad(queue0_, \"sink\");\n> > + GstPad *queue1_sink_pad =\n> gst_element_get_static_pad(queue1_, \"sink\");\n> > +\n> > + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> GST_PAD_LINK_OK\n> > + || gst_pad_link(request_pad, queue1_sink_pad) !=\n> GST_PAD_LINK_OK) {\n>\n> checkstyle says to put this on one line but you can do:\n>\n> + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> GST_PAD_LINK_OK ||\n> + gst_pad_link(request_pad, queue1_sink_pad) !=\n> GST_PAD_LINK_OK) {\n>\n> I think that looks nicer.\n>\n> > + if (queue0_sink_pad)\n> > + gst_object_unref(queue0_sink_pad);\n> > + if (queue1_sink_pad)\n> > + gst_object_unref(queue1_sink_pad);\n> > + g_printerr(\"Pads could not be linked.\\n\");\n> > + return TestFail;\n> > + }\n> > + gst_object_unref(queue0_sink_pad);\n> > + gst_object_unref(queue1_sink_pad);\n>\n> checkstyle ?\n>\n> > +\n> > + if (startPipeline() != TestPass)\n> > + return TestFail;\n> > +\n> > + if (processEvent() != TestPass)\n> > + return TestFail;\n> > +\n> > + return TestPass;\n> > + }\n> > +\n> > +private:\n> > + std::string cameraName;\n> > + GstElement *convert0_;\n> > + GstElement *convert1_;\n> > + GstElement *sink0_;\n> > + GstElement *sink1_;\n> > + GstElement *queue0_;\n> > + GstElement *queue1_;\n> > +};\n> > +\n> > +TEST_REGISTER(GstreamerMultiStreamTest)\n> > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build\n> > index aca53b920365..13652e87d05c 100644\n> > --- a/test/gstreamer/meson.build\n> > +++ b/test/gstreamer/meson.build\n> > @@ -6,6 +6,7 @@ endif\n> >\n> > gstreamer_tests = [\n> > ['single_stream_test', 'gstreamer_single_stream_test.cpp'],\n> > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'],\n>\n> checkstyle.\n>\n>\n> With those fixed, looks good.\n>\n> Reviewed-by: Paul Elder \n>\n> It runs fine too (on uvc skips properly, and it passes on raspberrypi).\n>\n> Tested-by: Paul Elder \n>\n> > ]\n> > gstreamer_dep = dependency('gstreamer-1.0', required: true)\n> >\n> > --\n> > 2.25.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 6488FBDC71\n\tfor ;\n\tTue, 14 Sep 2021 05:34:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3F9D69187;\n\tTue, 14 Sep 2021 07:34:27 +0200 (CEST)","from mail-yb1-xb2c.google.com (mail-yb1-xb2c.google.com\n\t[IPv6:2607:f8b0:4864:20::b2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A03DC6916F\n\tfor ;\n\tTue, 14 Sep 2021 07:34:25 +0200 (CEST)","by mail-yb1-xb2c.google.com with SMTP id z5so25637353ybj.2\n\tfor ;\n\tMon, 13 Sep 2021 22:34:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"qwJTFQyA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=67xl16dfzmNCNa3bLoDPg1LFd5B5H5wC6ByJJjjajYc=;\n\tb=qwJTFQyAaBX20rNhJQOOzReX/PFJsSRVJNDMqb9Bpr8HYar3X84NfCHiDBDEG0t64M\n\tsU+6MBLdcD/lIYa438T6bPlQzRYETgCKlu9POEpLOZzaPzaqM2xfWjaJ1qjzuSkgJNsd\n\tV4iOi+8pXKdZ+XRSLYjX+PCb9UnzIOOBD9a+fckG8QXIiCIxHu+s7j2aN7GatIqb/iV+\n\tqAW+THPyr93SIa0AwNBwCuCmcLiaJgdID8lYXTJQpFGb7ClO5XYHaBLkoELv2E+P/Lu9\n\toPXSgF84vu5H9/GKCotvvafnvt3TD6FOn9G0MVQr5QvrVeIFez+DbtNANaXnz7urkW4N\n\t+Qpw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=67xl16dfzmNCNa3bLoDPg1LFd5B5H5wC6ByJJjjajYc=;\n\tb=xKSnvkte3ip4zhrs7FDfmiaoxBKxZtYEw0ajxbPn/BrhVtkxS2ERKCK1qKRZf+oyCL\n\tqLjpQbbCL8RVIgWOXFkFMVo91/+Ojwk+cpR7pNWHHx0ol1UK/sVksZYJyg6z1gYrNVch\n\t2PlTxL14fJV97IqoPtiWl75/fCXjUuLiGf8j2n4fv03yT5tqeAoq5ae8IANAYaV+Wdfi\n\tk4goExiLfVZX5VgnuLUudRmPGdjxTqA7fUmhNk070Ei9P07O/pOgZSwVqht6VczxcolQ\n\tCew930GWLX/067F41SBdxxvzQbJrhc5z+EAc7CRZ6l/A1jkLRxw0w3NhqsiDTHTaRB1N\n\tp/JA==","X-Gm-Message-State":"AOAM5318E5PSNbWuX2TNZQPVpt+6LzFCHXOt0bvYDW2Vd1J9fdchPLMc\n\tEsYvut6HMEK37SShhItKBVdzLTI6/wUWpncCM989ckVe4/m8jQ==","X-Google-Smtp-Source":"ABdhPJxj+gQbJPb8b83S3zjI5zc9WeeTbulRyakwMMjVnDn7AqbB7WFfGxx90ssDUn5EdQIbWRvUi/RAOmoGc26VV9Y=","X-Received":"by 2002:a25:c504:: with SMTP id\n\tv4mr20738838ybe.308.1631597663974; \n\tMon, 13 Sep 2021 22:34:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20210910181152.878132-1-vedantparanjape160201@gmail.com>\n\t<20210914013302.GB1857410@pyrite.rasen.tech>","In-Reply-To":"<20210914013302.GB1857410@pyrite.rasen.tech>","From":"Vedant Paranjape ","Date":"Tue, 14 Sep 2021 11:04:12 +0530","Message-ID":"","To":"Paul Elder ","Content-Type":"multipart/alternative; boundary=\"0000000000002d783f05cbedefe7\"","Subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","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","Cc":"LibCamera Devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19681,"web_url":"https://patchwork.libcamera.org/comment/19681/","msgid":"<4e47983eb9c63dd3efad0f539dc4f5df0f5b22c6.camel@ndufresne.ca>","date":"2021-09-14T17:38:04","subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le vendredi 10 septembre 2021 à 23:41 +0530, Vedant Paranjape a écrit :\n> This patch adds a test to test if multi stream using libcamera's\n> gstreamer element works.\n> \n> Signed-off-by: Vedant Paranjape \n> ---\n> .../gstreamer/gstreamer_multi_stream_test.cpp | 138 ++++++++++++++++++\n> test/gstreamer/meson.build | 1 +\n> 2 files changed, 139 insertions(+)\n> create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp\n> \n> diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> new file mode 100644\n> index 000000000000..e5c909c85da2\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> @@ -0,0 +1,138 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2021, Vedant Paranjape\n> + *\n> + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/source_paths.h\"\n> +\n> +#include \n> +\n> +#include \"gstreamer_test.h\"\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +\n> +class GstreamerMultiStreamTest : public GstreamerTest, public Test\n> +{\n> +public:\n> +\tGstreamerMultiStreamTest()\n> +\t\t: GstreamerTest()\n> +\t{\n> +\t}\n> +\n> +protected:\n> +\tint init() override\n> +\t{\n> +\t\tif (status_ != TestPass)\n> +\t\t\treturn status_;\n> +\n> +\t\t/* Check if platform support multistream output */\n> +\t\tlibcamera::CameraManager cm;\n> +\t\tcm.start();\n> +\t\tbool cameraFound = false;\n> +\t\tfor (auto &i : cm.cameras()) {\n> +\t\t\tif (i->streams().size() > 1) {\n> +\t\t\t\tcameraName = i->id();\n> +\t\t\t\tcameraFound = true;\n> +\t\t\t\tcm.stop();\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tif (!cameraFound) {\n> +\t\t\tcm.stop();\n> +\t\t\treturn TestSkip;\n> +\t\t}\n> +\n> +\t\tg_autoptr(GstElement) convert0 = gst_element_factory_make(\"videoconvert\", \"convert0\");\n> +\t\tg_autoptr(GstElement) convert1 = gst_element_factory_make(\"videoconvert\", \"convert1\");\n> +\t\tg_autoptr(GstElement) sink0 = gst_element_factory_make(\"fakesink\", \"sink0\");\n> +\t\tg_autoptr(GstElement) sink1 = gst_element_factory_make(\"fakesink\", \"sink1\");\n> +\t\tg_autoptr(GstElement) queue0 = gst_element_factory_make(\"queue\", \"queue0\");\n> +\t\tg_autoptr(GstElement) queue1 = gst_element_factory_make(\"queue\", \"queue1\");\n> +\t\tg_object_ref_sink(convert0);\n> +\t\tg_object_ref_sink(convert1);\n> +\t\tg_object_ref_sink(sink0);\n> +\t\tg_object_ref_sink(sink1);\n> +\t\tg_object_ref_sink(queue0);\n> +\t\tg_object_ref_sink(queue1);\n\nref_sink should be inline or next to their allocation.\n\n> +\n> +\t\tif (!convert0 || !convert1 || !sink0 || !sink1 || !queue0 || !queue1) {\n> +\t\t\tg_printerr(\"Not all elements could be created. %p.%p.%p.%p.%p.%p\\n\",\n> +\t\t\t\t convert0, convert1, sink0, sink1, queue0, queue1);\n> +\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tconvert0_ = reinterpret_cast(g_steal_pointer(&convert0));\n> +\t\tconvert1_ = reinterpret_cast(g_steal_pointer(&convert1));\n> +\t\tsink0_ = reinterpret_cast(g_steal_pointer(&sink0));\n> +\t\tsink1_ = reinterpret_cast(g_steal_pointer(&sink1));\n> +\t\tqueue0_ = reinterpret_cast(g_steal_pointer(&queue0));\n> +\t\tqueue1_ = reinterpret_cast(g_steal_pointer(&queue1));\n\nWhat is the point of using autoptr here, you will clean them up in your\ndestructure, just store them directly in the class. In general, if you feel the\ncode is ugly, that's likely before you are doing something that is not needed.\n\n> +\n> +\t\tif (createPipeline() != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint run() override\n> +\t{\n> +\t\tg_object_set(libcameraSrc_, \"camera-name\", cameraName.c_str(), NULL);\n> +\n> +\t\t/* Build the pipeline */\n> +\t\tgst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, queue0_, queue1_,\n> +\t\t\t\t\t\t\t\t\tconvert0_, convert1_, sink0_, sink1_, NULL);\n> +\t\tif (gst_element_link_many(queue0_, convert0_, sink0_, NULL) != TRUE\n> +\t\t\t|| gst_element_link_many(queue1_, convert1_, sink1_, NULL) != TRUE) {\n> +\t\t\tg_printerr(\"Elements could not be linked.\\n\");\n> +\t\t\treturn TestFail;\n> +\t\t}\n\nI think we'd gain in readability of using gst_parse_launch_full(). You can even\nlink requested pad with that. It's also very ackward here, since you have a\nrun() function which cannot be run twice.\n\n> +\n> +\t\tg_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, \"src\");\n> +\t\tg_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n> +\t\tGstPad *queue0_sink_pad = gst_element_get_static_pad(queue0_, \"sink\");\n> +\t\tGstPad *queue1_sink_pad = gst_element_get_static_pad(queue1_, \"sink\");\n> +\n> +\t\tif (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK\n> +\t\t\t|| gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n> +\t\t\tif (queue0_sink_pad)\n> +\t\t\t\tgst_object_unref(queue0_sink_pad);\n> +\t\t\tif (queue1_sink_pad)\n> +\t\t\t\tgst_object_unref(queue1_sink_pad);\n> +\t\t\tg_printerr(\"Pads could not be linked.\\n\");\n> +\t\t\treturn TestFail;\n> +\t\t}\n> + gst_object_unref(queue0_sink_pad);\n> + gst_object_unref(queue1_sink_pad);\n> +\n> +\t\tif (startPipeline() != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (processEvent() != TestPass)\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +private:\n> +\tstd::string cameraName;\n> +\tGstElement *convert0_;\n> +\tGstElement *convert1_;\n> +\tGstElement *sink0_;\n> +\tGstElement *sink1_;\n> +\tGstElement *queue0_;\n> +\tGstElement *queue1_;\n\nNo destructor ? All leaked ?\n\n> +};\n> +\n> +TEST_REGISTER(GstreamerMultiStreamTest)\n> diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build\n> index aca53b920365..13652e87d05c 100644\n> --- a/test/gstreamer/meson.build\n> +++ b/test/gstreamer/meson.build\n> @@ -6,6 +6,7 @@ endif\n> \n> gstreamer_tests = [\n> ['single_stream_test', 'gstreamer_single_stream_test.cpp'],\n> + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'],\n> ]\n> gstreamer_dep = dependency('gstreamer-1.0', required: true)\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 26BBEBDC71\n\tfor ;\n\tTue, 14 Sep 2021 17:38:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81F4E6024A;\n\tTue, 14 Sep 2021 19:38:09 +0200 (CEST)","from mail-qk1-x72d.google.com (mail-qk1-x72d.google.com\n\t[IPv6:2607:f8b0:4864:20::72d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CA6D60132\n\tfor ;\n\tTue, 14 Sep 2021 19:38:07 +0200 (CEST)","by mail-qk1-x72d.google.com with SMTP id w78so240994qkb.4\n\tfor ;\n\tTue, 14 Sep 2021 10:38:07 -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\tj23sm8099309qkl.65.2021.09.14.10.38.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 14 Sep 2021 10:38:05 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"wLxf8WYP\"; 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:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=LEjTpMYn2L2RaqCIzWMerx19yZqJ30pVPeEpskKeRoA=;\n\tb=wLxf8WYPOzQVLnDgq6VUSwKawvZfab+gBcu6uROvsrnX7rm9ak8VC6RxwdQh/PKJRq\n\tdAIIoNXo2YYvvrr0dcgl6DpB6ixJnLxytnXwDCWUgvvIAXDDx08gsKCcgCi1q7HnUKdQ\n\t1Tav8vViJvxBKwN5h93+7LupDnn5zwdj7KNlqZ01PHzw39r6uns4p+728xoRjdZV0FK5\n\t2Uo7Yh41sYV3yxHK/XpDCQQCBof64pawuXUu5V9jaAs0T72nT8lz/IGsmD830Y0/0Q1S\n\twP9cVn9NyknG1rPAF9A6bGcO/0bbGJC+xf5A1xu09jsxH9qiceA088hgIt0u26NrWxnq\n\twVqA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=LEjTpMYn2L2RaqCIzWMerx19yZqJ30pVPeEpskKeRoA=;\n\tb=pOiySlU3JjjJndMcxMZN9IAOriGAucZV5G1kWXk8eK71PhfvLwIkvB+0uE59l8o+Vd\n\tXyK7VPXhsZKRNRQP+gE9PoWe8UJM6FsMZRAICg3rK6qNV40nqGPw/OL3dKaiBtpvI5pi\n\tijl9Vgy7KVgeOWVkUIReoH2m8B45XZKy8Hl4/FgQSwLEr3zUPKh+qpQOnteEjqyrv4HU\n\tux97p4HCcD4s3mkT5cIYYcZn3GriK2zNlrH1B3ncPjNOGHuhYFIAuMKilZ8U4nnKHQSJ\n\tpwjmbnQVbzdHPHhVTMrYUIHsltY6XLgVsby2VZc3Jymky4a7FmdvrOrNNLNq3ye0XY9o\n\tcp2Q==","X-Gm-Message-State":"AOAM533TDBIDpsZBe7xCNecOTS68nPgNcuu2DK0khmvi9RzkHt7tAAU4\n\tHFWToEhNNNKCaGLbnA6kibW21g==","X-Google-Smtp-Source":"ABdhPJxE0sVYDqThq62DEeQQtp6lP8Fq6XWVz5QKWA4NVyr+7uGFkcIrr1WaD7v9D5F+hZ/WMFCsOw==","X-Received":"by 2002:a05:620a:1388:: with SMTP id\n\tk8mr5999200qki.152.1631641086061; \n\tTue, 14 Sep 2021 10:38:06 -0700 (PDT)","Message-ID":"<4e47983eb9c63dd3efad0f539dc4f5df0f5b22c6.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Vedant Paranjape , \n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 14 Sep 2021 13:38:04 -0400","In-Reply-To":"<20210910181152.878132-1-vedantparanjape160201@gmail.com>","References":"<20210910181152.878132-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","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":19684,"web_url":"https://patchwork.libcamera.org/comment/19684/","msgid":"","date":"2021-09-14T17:46:26","subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hi Nicolas,\n\nOn Tue, 14 Sep, 2021, 23:08 Nicolas Dufresne, wrote:\n\n> Le vendredi 10 septembre 2021 à 23:41 +0530, Vedant Paranjape a écrit :\n> > This patch adds a test to test if multi stream using libcamera's\n> > gstreamer element works.\n> >\n> > Signed-off-by: Vedant Paranjape \n> > ---\n> > .../gstreamer/gstreamer_multi_stream_test.cpp | 138 ++++++++++++++++++\n> > test/gstreamer/meson.build | 1 +\n> > 2 files changed, 139 insertions(+)\n> > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp\n> >\n> > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp\n> b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > new file mode 100644\n> > index 000000000000..e5c909c85da2\n> > --- /dev/null\n> > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > @@ -0,0 +1,138 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Vedant Paranjape\n> > + *\n> > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/source_paths.h\"\n> > +\n> > +#include \n> > +\n> > +#include \"gstreamer_test.h\"\n> > +#include \"test.h\"\n> > +\n> > +using namespace std;\n> > +\n> > +class GstreamerMultiStreamTest : public GstreamerTest, public Test\n> > +{\n> > +public:\n> > + GstreamerMultiStreamTest()\n> > + : GstreamerTest()\n> > + {\n> > + }\n> > +\n> > +protected:\n> > + int init() override\n> > + {\n> > + if (status_ != TestPass)\n> > + return status_;\n> > +\n> > + /* Check if platform support multistream output */\n> > + libcamera::CameraManager cm;\n> > + cm.start();\n> > + bool cameraFound = false;\n> > + for (auto &i : cm.cameras()) {\n> > + if (i->streams().size() > 1) {\n> > + cameraName = i->id();\n> > + cameraFound = true;\n> > + cm.stop();\n> > + break;\n> > + }\n> > + }\n> > +\n> > + if (!cameraFound) {\n> > + cm.stop();\n> > + return TestSkip;\n> > + }\n> > +\n> > + g_autoptr(GstElement) convert0 =\n> gst_element_factory_make(\"videoconvert\", \"convert0\");\n> > + g_autoptr(GstElement) convert1 =\n> gst_element_factory_make(\"videoconvert\", \"convert1\");\n> > + g_autoptr(GstElement) sink0 =\n> gst_element_factory_make(\"fakesink\", \"sink0\");\n> > + g_autoptr(GstElement) sink1 =\n> gst_element_factory_make(\"fakesink\", \"sink1\");\n> > + g_autoptr(GstElement) queue0 =\n> gst_element_factory_make(\"queue\", \"queue0\");\n> > + g_autoptr(GstElement) queue1 =\n> gst_element_factory_make(\"queue\", \"queue1\");\n> > + g_object_ref_sink(convert0);\n> > + g_object_ref_sink(convert1);\n> > + g_object_ref_sink(sink0);\n> > + g_object_ref_sink(sink1);\n> > + g_object_ref_sink(queue0);\n> > + g_object_ref_sink(queue1);\n>\n> ref_sink should be inline or next to their allocation.\n>\n\nOkay, will change.\n\n\n> > +\n> > + if (!convert0 || !convert1 || !sink0 || !sink1 || !queue0\n> || !queue1) {\n> > + g_printerr(\"Not all elements could be created.\n> %p.%p.%p.%p.%p.%p\\n\",\n> > + convert0, convert1, sink0, sink1,\n> queue0, queue1);\n> > +\n> > + return TestFail;\n> > + }\n> > +\n> > + convert0_ = reinterpret_cast *>(g_steal_pointer(&convert0));\n> > + convert1_ = reinterpret_cast *>(g_steal_pointer(&convert1));\n> > + sink0_ = reinterpret_cast *>(g_steal_pointer(&sink0));\n> > + sink1_ = reinterpret_cast *>(g_steal_pointer(&sink1));\n> > + queue0_ = reinterpret_cast *>(g_steal_pointer(&queue0));\n> > + queue1_ = reinterpret_cast *>(g_steal_pointer(&queue1));\n>\n> What is the point of using autoptr here, you will clean them up in your\n> destructure, just store them directly in the class. In general, if you\n> feel the\n> code is ugly, that's likely before you are doing something that is not\n> needed.\n>\n\nOkay, makes sense.\n\n> +\n> > + if (createPipeline() != TestPass)\n> > + return TestFail;\n> > +\n> > + return TestPass;\n> > + }\n> > +\n> > + int run() override\n> > + {\n> > + g_object_set(libcameraSrc_, \"camera-name\",\n> cameraName.c_str(), NULL);\n> > +\n> > + /* Build the pipeline */\n> > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> queue0_, queue1_,\n> > +\n> convert0_, convert1_, sink0_, sink1_, NULL);\n> > + if (gst_element_link_many(queue0_, convert0_, sink0_,\n> NULL) != TRUE\n> > + || gst_element_link_many(queue1_, convert1_,\n> sink1_, NULL) != TRUE) {\n> > + g_printerr(\"Elements could not be linked.\\n\");\n> > + return TestFail;\n> > + }\n>\n> I think we'd gain in readability of using gst_parse_launch_full(). You can\n> even\n> link requested pad with that. It's also very ackward here, since you have a\n> run() function which cannot be run twice.\n>\n\nI agree with this, I'll take a look at the function.\n\n> +\n> > + g_autoptr(GstPad) src_pad =\n> gst_element_get_static_pad(libcameraSrc_, \"src\");\n> > + g_autoptr(GstPad) request_pad =\n> gst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n> > + GstPad *queue0_sink_pad =\n> gst_element_get_static_pad(queue0_, \"sink\");\n> > + GstPad *queue1_sink_pad =\n> gst_element_get_static_pad(queue1_, \"sink\");\n> > +\n> > + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> GST_PAD_LINK_OK\n> > + || gst_pad_link(request_pad, queue1_sink_pad) !=\n> GST_PAD_LINK_OK) {\n> > + if (queue0_sink_pad)\n> > + gst_object_unref(queue0_sink_pad);\n> > + if (queue1_sink_pad)\n> > + gst_object_unref(queue1_sink_pad);\n> > + g_printerr(\"Pads could not be linked.\\n\");\n> > + return TestFail;\n> > + }\n> > + gst_object_unref(queue0_sink_pad);\n> > + gst_object_unref(queue1_sink_pad);\n> > +\n> > + if (startPipeline() != TestPass)\n> > + return TestFail;\n> > +\n> > + if (processEvent() != TestPass)\n> > + return TestFail;\n> > +\n> > + return TestPass;\n> > + }\n> > +\n> > +private:\n> > + std::string cameraName;\n> > + GstElement *convert0_;\n> > + GstElement *convert1_;\n> > + GstElement *sink0_;\n> > + GstElement *sink1_;\n> > + GstElement *queue0_;\n> > + GstElement *queue1_;\n>\n> No destructor ? All leaked ?\n>\n\nHow ? I have added all to pipeline, it will take care of their lifetime.\n\n> +};\n> > +\n> > +TEST_REGISTER(GstreamerMultiStreamTest)\n> > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build\n> > index aca53b920365..13652e87d05c 100644\n> > --- a/test/gstreamer/meson.build\n> > +++ b/test/gstreamer/meson.build\n> > @@ -6,6 +6,7 @@ endif\n> >\n> > gstreamer_tests = [\n> > ['single_stream_test', 'gstreamer_single_stream_test.cpp'],\n> > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'],\n> > ]\n> > gstreamer_dep = dependency('gstreamer-1.0', required: true)\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 BE061BDC71\n\tfor ;\n\tTue, 14 Sep 2021 17:46:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BE1860248;\n\tTue, 14 Sep 2021 19:46:40 +0200 (CEST)","from mail-yb1-xb2e.google.com (mail-yb1-xb2e.google.com\n\t[IPv6:2607:f8b0:4864:20::b2e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AB36C60132\n\tfor ;\n\tTue, 14 Sep 2021 19:46:39 +0200 (CEST)","by mail-yb1-xb2e.google.com with SMTP id y16so30120439ybm.3\n\tfor ;\n\tTue, 14 Sep 2021 10:46:39 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"awUzgA2Q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=YIOjJbe2ZUqsdRbautHO64g/BVGzF/dNvw6R1epSYog=;\n\tb=awUzgA2QbUCJAsJcxA3tZuQIUNqizyByVk/JC7i263cNoJEZcyrol5vj0hrQdhOWKf\n\tVyAMiJDbJm9mRf90whhBO04y4W2yM7asMmlCaFCKgy4AajC4qI/gXdRGQO7UOTiYlSt2\n\ttyiBejJgPEZfVdVYABCejW0wFFKptLN5Oo6ASRAqF+unlT6TnflwmVEqIu+czItDBpl3\n\ti6aw4anaI2os2xrMBH5e5DOw9lCcZyaEr4BM+xxO6nKNgG/0J6VlBCZiRqzfmHBqPqc0\n\t2Xp/TRRUPWVBZ2aGBdZpSNOHIt6wg6rm+gg5s73/e0BTWDv+osrHwvom2Yqe2DVAdAR6\n\tS6tA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=YIOjJbe2ZUqsdRbautHO64g/BVGzF/dNvw6R1epSYog=;\n\tb=FPmp5k3gS15f2ig+vhzOHoYvikaQPQJCrvtpqdZgnB4PVkYcveixxQ+HMrXdpK0GQg\n\tV0uYPzyRy3taH/zLplUIKtyJEXtNc3PbgvAjkmIdXZmbaNmsE8w8DskTA4sLGj2Onv0e\n\tOJWFB+cjmzoWMNX21/tLPkMJN31GS/LnHy7LNYz8BxQYOLJMG0Au2/d0wcUtpSaoiHSo\n\tKDeD+MG7jxhLFQCPoPKWw+3MVt8FhKD10s2GLvIL+CqdDdtb0EjZUuTN8aGmSvaa5GhI\n\tSz37+Tu2DlJfvDFB3nSd2dVjRDSJjQps0bR7RthG39jCA+4iQt8vGO3fu0LNLvqRd/go\n\tgjFg==","X-Gm-Message-State":"AOAM531hmyzFuydBRJlZ9Wasl8BM5U6h8811QVmwuY87GOavobeMwsQ4\n\tFBJh3F+Yfi6mKe/Gnk9TBVQh5/a4rDXVci6CTp0=","X-Google-Smtp-Source":"ABdhPJzufc42rNu86vrohkVkfpOPEa+2VnLRPW+iwpBTpWDsc7oKRPJUXE6Jb/JMxbgBU8qpvGRWxY3DJP20zdZy2fU=","X-Received":"by 2002:a25:c504:: with SMTP id v4mr527444ybe.308.1631641598369; \n\tTue, 14 Sep 2021 10:46:38 -0700 (PDT)","MIME-Version":"1.0","References":"<20210910181152.878132-1-vedantparanjape160201@gmail.com>\n\t<4e47983eb9c63dd3efad0f539dc4f5df0f5b22c6.camel@ndufresne.ca>","In-Reply-To":"<4e47983eb9c63dd3efad0f539dc4f5df0f5b22c6.camel@ndufresne.ca>","From":"Vedant Paranjape ","Date":"Tue, 14 Sep 2021 23:16:26 +0530","Message-ID":"","To":"Nicolas Dufresne ","Content-Type":"multipart/alternative; boundary=\"000000000000df23b305cbf82934\"","Subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","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","Cc":"LibCamera Devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19685,"web_url":"https://patchwork.libcamera.org/comment/19685/","msgid":"","date":"2021-09-14T17:49:00","subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le mardi 14 septembre 2021 à 23:16 +0530, Vedant Paranjape a écrit :\n> Hi Nicolas,\n> \n> On Tue, 14 Sep, 2021, 23:08 Nicolas Dufresne, wrote:\n> > Le vendredi 10 septembre 2021 à 23:41 +0530, Vedant Paranjape a écrit :\n> > > This patch adds a test to test if multi stream using libcamera's\n> > > gstreamer element works.\n> > > \n> > > Signed-off-by: Vedant Paranjape \n> > > ---\n> > >  .../gstreamer/gstreamer_multi_stream_test.cpp | 138 ++++++++++++++++++\n> > >  test/gstreamer/meson.build                    |   1 +\n> > >  2 files changed, 139 insertions(+)\n> > >  create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp\n> > > \n> > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > > new file mode 100644\n> > > index 000000000000..e5c909c85da2\n> > > --- /dev/null\n> > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > > @@ -0,0 +1,138 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2021, Vedant Paranjape\n> > > + *\n> > > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test\n> > > + */\n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +#include \"libcamera/internal/source_paths.h\"\n> > > +\n> > > +#include \n> > > +\n> > > +#include \"gstreamer_test.h\"\n> > > +#include \"test.h\"\n> > > +\n> > > +using namespace std;\n> > > +\n> > > +class GstreamerMultiStreamTest : public GstreamerTest, public Test\n> > > +{\n> > > +public:\n> > > +     GstreamerMultiStreamTest()\n> > > +             : GstreamerTest()\n> > > +     {\n> > > +     }\n> > > +\n> > > +protected:\n> > > +     int init() override\n> > > +     {\n> > > +             if (status_ != TestPass)\n> > > +                     return status_;\n> > > +\n> > > +             /* Check if platform support multistream output */\n> > > +             libcamera::CameraManager cm;\n> > > +             cm.start();\n> > > +             bool cameraFound = false;\n> > > +             for (auto &i : cm.cameras()) {\n> > > +                     if (i->streams().size() > 1) {\n> > > +                             cameraName = i->id();\n> > > +                             cameraFound = true;\n> > > +                             cm.stop();\n> > > +                             break;\n> > > +                     }\n> > > +             }\n> > > +\n> > > +             if (!cameraFound) {\n> > > +                     cm.stop();\n> > > +                     return TestSkip;\n> > > +             }\n> > > +\n> > > +             g_autoptr(GstElement) convert0 =\n> > gst_element_factory_make(\"videoconvert\", \"convert0\");\n> > > +             g_autoptr(GstElement) convert1 =\n> > gst_element_factory_make(\"videoconvert\", \"convert1\");\n> > > +             g_autoptr(GstElement) sink0 =\n> > gst_element_factory_make(\"fakesink\", \"sink0\");\n> > > +             g_autoptr(GstElement) sink1 =\n> > gst_element_factory_make(\"fakesink\", \"sink1\");\n> > > +             g_autoptr(GstElement) queue0 =\n> > gst_element_factory_make(\"queue\", \"queue0\");\n> > > +             g_autoptr(GstElement) queue1 =\n> > gst_element_factory_make(\"queue\", \"queue1\");\n> > > +             g_object_ref_sink(convert0);\n> > > +             g_object_ref_sink(convert1);\n> > > +             g_object_ref_sink(sink0);\n> > > +             g_object_ref_sink(sink1);\n> > > +             g_object_ref_sink(queue0);\n> > > +             g_object_ref_sink(queue1);\n> > \n> > ref_sink should be inline or next to their allocation.\n> \n> Okay, will change.\n> \n> > \n> > > +\n> > > +             if (!convert0 || !convert1 || !sink0 || !sink1 || !queue0 ||\n> > !queue1) {\n> > > +                     g_printerr(\"Not all elements could be created.\n> > %p.%p.%p.%p.%p.%p\\n\",\n> > > +                                convert0, convert1, sink0, sink1, queue0,\n> > queue1);\n> > > +\n> > > +                     return TestFail;\n> > > +             }\n> > > +\n> > > +             convert0_ = reinterpret_cast > *>(g_steal_pointer(&convert0));\n> > > +             convert1_ = reinterpret_cast > *>(g_steal_pointer(&convert1));\n> > > +             sink0_ = reinterpret_cast > *>(g_steal_pointer(&sink0));\n> > > +             sink1_ = reinterpret_cast > *>(g_steal_pointer(&sink1));\n> > > +             queue0_ = reinterpret_cast > *>(g_steal_pointer(&queue0));\n> > > +             queue1_ = reinterpret_cast > *>(g_steal_pointer(&queue1));\n> > \n> > What is the point of using autoptr here, you will clean them up in your\n> > destructure, just store them directly in the class. In general, if you feel\n> > the\n> > code is ugly, that's likely before you are doing something that is not\n> > needed.\n> \n> Okay, makes sense.\n> \n> > > +\n> > > +             if (createPipeline() != TestPass)\n> > > +                     return TestFail;\n> > > +\n> > > +             return TestPass;\n> > > +     }\n> > > +\n> > > +     int run() override\n> > > +     {\n> > > +             g_object_set(libcameraSrc_, \"camera-name\",\n> > cameraName.c_str(), NULL);\n> > > +\n> > > +             /* Build the pipeline */\n> > > +             gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, queue0_,\n> > queue1_,\n> > > +                                                                   \n> >  convert0_, convert1_, sink0_, sink1_, NULL);\n> > > +             if (gst_element_link_many(queue0_, convert0_, sink0_, NULL)\n> > != TRUE\n> > > +                     || gst_element_link_many(queue1_, convert1_, sink1_,\n> > NULL) != TRUE) {\n> > > +                     g_printerr(\"Elements could not be linked.\\n\");\n> > > +                     return TestFail;\n> > > +             }\n> > \n> > I think we'd gain in readability of using gst_parse_launch_full(). You can\n> > even\n> > link requested pad with that. It's also very ackward here, since you have a\n> > run() function which cannot be run twice.\n> \n> I agree with this, I'll take a look at the function.\n> \n> > > +\n> > > +             g_autoptr(GstPad) src_pad =\n> > gst_element_get_static_pad(libcameraSrc_, \"src\");\n> > > +             g_autoptr(GstPad) request_pad =\n> > gst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n> > > +             GstPad *queue0_sink_pad =\n> > gst_element_get_static_pad(queue0_, \"sink\");\n> > > +             GstPad *queue1_sink_pad =\n> > gst_element_get_static_pad(queue1_, \"sink\");\n> > > +\n> > > +             if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> > GST_PAD_LINK_OK\n> > > +                     || gst_pad_link(request_pad, queue1_sink_pad) !=\n> > GST_PAD_LINK_OK) {\n> > > +                     if (queue0_sink_pad)\n> > > +                             gst_object_unref(queue0_sink_pad);\n> > > +                     if (queue1_sink_pad)\n> > > +                             gst_object_unref(queue1_sink_pad);\n> > > +                     g_printerr(\"Pads could not be linked.\\n\");\n> > > +                     return TestFail;\n> > > +             }\n> > > +        gst_object_unref(queue0_sink_pad);\n> > > +        gst_object_unref(queue1_sink_pad);\n> > > +\n> > > +             if (startPipeline() != TestPass)\n> > > +                     return TestFail;\n> > > +\n> > > +             if (processEvent() != TestPass)\n> > > +                     return TestFail;\n> > > +\n> > > +             return TestPass;\n> > > +     }\n> > > +\n> > > +private:\n> > > +     std::string cameraName;\n> > > +     GstElement *convert0_;\n> > > +     GstElement *convert1_;\n> > > +     GstElement *sink0_;\n> > > +     GstElement *sink1_;\n> > > +     GstElement *queue0_;\n> > > +     GstElement *queue1_;\n> > \n> > No destructor ? All leaked ?\n> \n> How ? I have added all to pipeline, it will take care of their lifetime.\n\nWell, if you remove the floating ref, ownership is not transferred to the\npipeline and you have to unref when you are done. That is the core principal of\nfloating references.\n\n> \n> > > +};\n> > > +\n> > > +TEST_REGISTER(GstreamerMultiStreamTest)\n> > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build\n> > > index aca53b920365..13652e87d05c 100644\n> > > --- a/test/gstreamer/meson.build\n> > > +++ b/test/gstreamer/meson.build\n> > > @@ -6,6 +6,7 @@ endif\n> > >  \n> > >  gstreamer_tests = [\n> > >      ['single_stream_test',   'gstreamer_single_stream_test.cpp'],\n> > > +    ['multi_stream_test',   'gstreamer_multi_stream_test.cpp'],\n> > >  ]\n> > >  gstreamer_dep = dependency('gstreamer-1.0', required: true)\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 40ABDBDC71\n\tfor ;\n\tTue, 14 Sep 2021 17:49:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7FDB369183;\n\tTue, 14 Sep 2021 19:49:04 +0200 (CEST)","from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com\n\t[IPv6:2607:f8b0:4864:20::f32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B7D4B60248\n\tfor ;\n\tTue, 14 Sep 2021 19:49:02 +0200 (CEST)","by mail-qv1-xf32.google.com with SMTP id di6so177121qvb.1\n\tfor ;\n\tTue, 14 Sep 2021 10:49:02 -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\th9sm8023050qkl.4.2021.09.14.10.49.01\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 14 Sep 2021 10:49:01 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"MYc5Okql\"; 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;\n\tbh=fBQMDSRkmMY/6WDppdj6cVFhNxIifwkjtwWy8F6q6Vg=;\n\tb=MYc5Okql5I8gTpoXbQHpQPlXrG/SU43LIhkW8tX6YysTRibqZj7xmFIJGO0+a2oUNg\n\t9z/3uAXSGwpM+9UewXsOSJvbnoxA+dasHNxtWfUgYBIPUDtKJRqmuwnKTV/crb5A0wpi\n\txf76QJsXR6TXNVGgosBg9HnUYZKnRSU2i1KKmlfAcijFrgf9W7fe4HzwbopuoM04AD6w\n\tv9upF8IvwfH/tDuSN8A3moSXMtrdQjx4wsVP8FtltJBeWUHz4bj1/GJ/ElpZFJLpO3HN\n\tQuwaQWvo5Jl/kif9HmQbVbWWTjB0LQTJnDO18KATIph1PWlTAqb8JkGZkh6Ignxh9VM5\n\tHKqw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:user-agent:mime-version;\n\tbh=fBQMDSRkmMY/6WDppdj6cVFhNxIifwkjtwWy8F6q6Vg=;\n\tb=uwRJcaU8e6mXkmTMDP3rBNRn4zQXCw33VbaNJYBHs5N/p4CttLUrP0OIJwKaWO/3Hx\n\tP3x7tfRz6GnQ8/3U1IDVQNA+vurE6qLLz3rgjhJC/Ct4EDFO8b0o0omqqtaly0B6AVlZ\n\t0vmQdFLlfHQI8M3mKg+3YsJ8gEKl0s7OZWid+xEcJIWhfssluVO8Uzbwr+QvcZSNhJ8s\n\tGt5hyYp8lo2M1elpRbtNQIOEe2y3k2iTJNzXHP/Blxbh+NR2xG6DS5oRUZyr0QwtF6F3\n\tH2n5rehGREYHFSRPAxjQboJn9nHObLp/RL90vxHDZNkCWVgOUg6qYGusrdt8OdmDLkj1\n\td3pw==","X-Gm-Message-State":"AOAM533fM+E6pZCRQATdIb6KOIETbpMq+JAhd5eS3zep0Y9EaL3t6yvu\n\tpPnfN2E7+chkHy31JY1DvGqioV4AV9FTRA==","X-Google-Smtp-Source":"ABdhPJzyJeGFf3M1KorrgMFSn0vsqffFzaZ1t5Kf8anOfWiYCF4ejO7mwFl78tHQxhUmQIam2K0cCg==","X-Received":"by 2002:a0c:e2d3:: with SMTP id t19mr6589557qvl.23.1631641741753;\n\tTue, 14 Sep 2021 10:49:01 -0700 (PDT)","Message-ID":"","From":"Nicolas Dufresne ","To":"Vedant Paranjape ","Date":"Tue, 14 Sep 2021 13:49:00 -0400","In-Reply-To":"","References":"<20210910181152.878132-1-vedantparanjape160201@gmail.com>\n\t<4e47983eb9c63dd3efad0f539dc4f5df0f5b22c6.camel@ndufresne.ca>\n\t","Content-Type":"multipart/alternative; boundary=\"=-Uf9QkCFKg7KSfGtRI9fh\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH v2] test: gstreamer: Add a test for\n\tgstreamer multi stream","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","Cc":"LibCamera Devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]