[{"id":19820,"web_url":"https://patchwork.libcamera.org/comment/19820/","msgid":"<00cabdeb04f843c1838cf225332eecbb81a257da.camel@collabora.com>","date":"2021-09-23T14:58:08","subject":"Re: [libcamera-devel] [PATCH v3] test: gstreamer: Add a test for\n\tgstreamer multi stream","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le jeudi 23 septembre 2021 à 20:26 +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> Test will run only on devices that support multistream output, i.e.,\n> devices that use IPU3 and raspberrypi pipeline. This was tested on\n> a Raspberry Pi 4B+.\n> \n> Signed-off-by: Vedant Paranjape \n> Reviewed-by: Nicolas Dufresne \n\nAck.\n\n> ---\n> .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> test/gstreamer/meson.build | 1 +\n> 2 files changed, 129 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..6f63f1e748e2\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> @@ -0,0 +1,128 @@\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 &camera : cm.cameras()) {\n> +\t\t\tif (camera->streams().size() > 1) {\n> +\t\t\t\tcameraName = camera->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\tconst gchar *streamDescription = \"queue ! fakesink\";\n> +\t\tg_autoptr(GError) error = NULL;\n> +\n> +\t\tstream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> +\t\t\t\t\t\tNULL,\n> +\t\t\t\t\t\tGST_PARSE_FLAG_FATAL_ERRORS,\n> +\t\t\t\t\t\t&error);\n> +\t\tif (!stream0_) {\n> +\t\t\tg_printerr(\"Stream0 could not be created (%s)\\n\", error->message);\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tg_object_ref_sink(stream0_);\n> +\n> +\t\tstream1_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> +\t\t\t\t\t\tNULL,\n> +\t\t\t\t\t\tGST_PARSE_FLAG_FATAL_ERRORS,\n> +\t\t\t\t\t\t&error);\n> +\t\tif (!stream1_) {\n> +\t\t\tg_printerr(\"Stream1 could not be created (%s)\\n\", error->message);\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tg_object_ref_sink(stream1_);\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_,\n> +\t\t\t\t\t\t\tstream0_, stream1_, NULL);\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> +\n> +\t\t{\n> +\t\t\tg_autoptr(GstPad) queue0_sink_pad = gst_element_get_static_pad(stream0_, \"sink\");\n> +\t\t\tg_autoptr(GstPad) queue1_sink_pad = gst_element_get_static_pad(stream1_, \"sink\");\n> +\n> +\t\t\tif (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK\n> +\t\t\t\t|| gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n> +\t\t\t\tg_printerr(\"Pads could not be linked.\\n\");\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\t\t}\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> +\tvoid cleanup() override\n> +\t{\n> +\t\tg_clear_object(&stream0_);\n> +\t\tg_clear_object(&stream1_);\n> +\t}\n> +\n> +private:\n> +\tstd::string cameraName;\n> +\tGstElement *stream0_;\n> +\tGstElement *stream1_;\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 C04B6BF01C\n\tfor ;\n\tThu, 23 Sep 2021 14:58:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C66F6918C;\n\tThu, 23 Sep 2021 16:58:26 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F2A8169189\n\tfor ;\n\tThu, 23 Sep 2021 16:58:24 +0200 (CEST)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id F42171F44498"],"Message-ID":"<00cabdeb04f843c1838cf225332eecbb81a257da.camel@collabora.com>","From":"Nicolas Dufresne ","To":"Vedant Paranjape , \n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 23 Sep 2021 10:58:08 -0400","In-Reply-To":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>","References":"<20210923145637.1685366-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 v3] 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":19826,"web_url":"https://patchwork.libcamera.org/comment/19826/","msgid":"<20210924060851.GU4382@pyrite.rasen.tech>","date":"2021-09-24T06:08:51","subject":"Re: [libcamera-devel] [PATCH v3] 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 Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:\n> This patch adds a test to test if multi stream using libcamera's\n> gstreamer element works.\n> \n> Test will run only on devices that support multistream output, i.e.,\n\nI think s/output/capture/ might be better.\n\nAlso s/i.e./e.g/\n\n> devices that use IPU3 and raspberrypi pipeline. This was tested on\n> a Raspberry Pi 4B+.\n> \n> Signed-off-by: Vedant Paranjape \n> Reviewed-by: Nicolas Dufresne \n> ---\n> .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> test/gstreamer/meson.build | 1 +\n> 2 files changed, 129 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..6f63f1e748e2\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> @@ -0,0 +1,128 @@\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\nYou don't use anything from this.\n\n> +\n> +#include \n> +\n> +#include \"libcamera/internal/source_paths.h\"\n\nNor this.\n\n(The single stream tests have these too)\n\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 &camera : cm.cameras()) {\n> +\t\t\tif (camera->streams().size() > 1) {\n> +\t\t\t\tcameraName = camera->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\tconst gchar *streamDescription = \"queue ! fakesink\";\n> +\t\tg_autoptr(GError) error = NULL;\n> +\n> +\t\tstream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> +\t\t\t\t\t\tNULL,\n> +\t\t\t\t\t\tGST_PARSE_FLAG_FATAL_ERRORS,\n> +\t\t\t\t\t\t&error);\n> +\t\tif (!stream0_) {\n> +\t\t\tg_printerr(\"Stream0 could not be created (%s)\\n\", error->message);\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tg_object_ref_sink(stream0_);\n> +\n> +\t\tstream1_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> +\t\t\t\t\t\tNULL,\n> +\t\t\t\t\t\tGST_PARSE_FLAG_FATAL_ERRORS,\n> +\t\t\t\t\t\t&error);\n> +\t\tif (!stream1_) {\n> +\t\t\tg_printerr(\"Stream1 could not be created (%s)\\n\", error->message);\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tg_object_ref_sink(stream1_);\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_,\n> +\t\t\t\t\t\t\tstream0_, stream1_, NULL);\n\nIndentation.\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> +\n> +\t\t{\n> +\t\t\tg_autoptr(GstPad) queue0_sink_pad = gst_element_get_static_pad(stream0_, \"sink\");\n> +\t\t\tg_autoptr(GstPad) queue1_sink_pad = gst_element_get_static_pad(stream1_, \"sink\");\n> +\n> +\t\t\tif (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK\n> +\t\t\t\t|| gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n\nIndentation.\n\n> +\t\t\t\tg_printerr(\"Pads could not be linked.\\n\");\n> +\t\t\t\treturn TestFail;\n> +\t\t\t}\n> +\t\t}\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> +\tvoid cleanup() override\n> +\t{\n> +\t\tg_clear_object(&stream0_);\n> +\t\tg_clear_object(&stream1_);\n> +\t}\n> +\n> +private:\n> +\tstd::string cameraName;\n\ns/cameraName/cameraName_/\n\n> +\tGstElement *stream0_;\n> +\tGstElement *stream1_;\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\nIt would be nice if the second elements lined up.\n\n\nOtherwise, looks good.\n\n\nPaul\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 7A3C5BF01C\n\tfor ;\n\tFri, 24 Sep 2021 06:09:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BBDD76918C;\n\tFri, 24 Sep 2021 08:09:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5EA6960242\n\tfor ;\n\tFri, 24 Sep 2021 08:09:00 +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 878F245E;\n\tFri, 24 Sep 2021 08:08:58 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vseJUOsR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632463739;\n\tbh=rII5A4FAqPhbZn+TBT+QcaK8P4z5JpZPzDiltpExZ5M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vseJUOsRGwsnw0MSwMFib8G0RwmEVpFxWaxjFUce5Pcrp0ucOY9NK0a4YdtNR/RPO\n\tYmP2ng01BF7FWg+yquygiGd1PKvl0FBLDpTAJpT3yoz2woMRqCbqoGyv8Li3F4FrUL\n\tkGpN3/G0nWcvC2rW054mC5a0JTpRk/ukYVtBpCYI=","Date":"Fri, 24 Sep 2021 15:08:51 +0900","From":"paul.elder@ideasonboard.com","To":"Vedant Paranjape ","Message-ID":"<20210924060851.GU4382@pyrite.rasen.tech>","References":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v3] 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,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19827,"web_url":"https://patchwork.libcamera.org/comment/19827/","msgid":"","date":"2021-09-24T06:12:18","subject":"Re: [libcamera-devel] [PATCH v3] 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,\nCan you also tell me the expected indentation ?\n\ncheckstyle suggestions didn't adhere with the 80 char width thing\n\n\n\n\nOn Fri, 24 Sep, 2021, 11:39 , wrote:\n\n> Hi Vedant,\n>\n> On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:\n> > This patch adds a test to test if multi stream using libcamera's\n> > gstreamer element works.\n> >\n> > Test will run only on devices that support multistream output, i.e.,\n>\n> I think s/output/capture/ might be better.\n>\n> Also s/i.e./e.g/\n>\n> > devices that use IPU3 and raspberrypi pipeline. This was tested on\n> > a Raspberry Pi 4B+.\n> >\n> > Signed-off-by: Vedant Paranjape \n> > Reviewed-by: Nicolas Dufresne \n> > ---\n> > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> > test/gstreamer/meson.build | 1 +\n> > 2 files changed, 129 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..6f63f1e748e2\n> > --- /dev/null\n> > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > @@ -0,0 +1,128 @@\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> You don't use anything from this.\n>\n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/source_paths.h\"\n>\n> Nor this.\n>\n> (The single stream tests have these too)\n>\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 &camera : cm.cameras()) {\n> > + if (camera->streams().size() > 1) {\n> > + cameraName = camera->id();\n> > + cameraFound = true;\n> > + cm.stop();\n> > + break;\n> > + }\n> > + }\n> > +\n> > + if (!cameraFound) {\n> > + cm.stop();\n> > + return TestSkip;\n> > + }\n> > +\n> > + const gchar *streamDescription = \"queue ! fakesink\";\n> > + g_autoptr(GError) error = NULL;\n> > +\n> > + stream0_ =\n> gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > + NULL,\n> > +\n> GST_PARSE_FLAG_FATAL_ERRORS,\n> > + &error);\n> > + if (!stream0_) {\n> > + g_printerr(\"Stream0 could not be created (%s)\\n\",\n> error->message);\n> > + return TestFail;\n> > + }\n> > + g_object_ref_sink(stream0_);\n> > +\n> > + stream1_ =\n> gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > + NULL,\n> > +\n> GST_PARSE_FLAG_FATAL_ERRORS,\n> > + &error);\n> > + if (!stream1_) {\n> > + g_printerr(\"Stream1 could not be created (%s)\\n\",\n> error->message);\n> > + return TestFail;\n> > + }\n> > + g_object_ref_sink(stream1_);\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> > + stream0_,\n> stream1_, NULL);\n>\n> Indentation.\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> > +\n> > + {\n> > + g_autoptr(GstPad) queue0_sink_pad =\n> gst_element_get_static_pad(stream0_, \"sink\");\n> > + g_autoptr(GstPad) queue1_sink_pad =\n> gst_element_get_static_pad(stream1_, \"sink\");\n> > +\n> > + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> GST_PAD_LINK_OK\n> > + || gst_pad_link(request_pad,\n> queue1_sink_pad) != GST_PAD_LINK_OK) {\n>\n> Indentation.\n>\n> > + g_printerr(\"Pads could not be linked.\\n\");\n> > + return TestFail;\n> > + }\n> > + }\n> > +\n> > + if (startPipeline() != TestPass)\n> > + return TestFail;\n> > +\n> > + if (processEvent() != TestPass)\n> > + return TestFail;\n> > +\n> > + return TestPass;\n> > + }\n> > +\n> > + void cleanup() override\n> > + {\n> > + g_clear_object(&stream0_);\n> > + g_clear_object(&stream1_);\n> > + }\n> > +\n> > +private:\n> > + std::string cameraName;\n>\n> s/cameraName/cameraName_/\n>\n> > + GstElement *stream0_;\n> > + GstElement *stream1_;\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> It would be nice if the second elements lined up.\n>\n>\n> Otherwise, looks good.\n>\n\nCan I add a R-b then?\n\nPaul\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 208DFBF01C\n\tfor ;\n\tFri, 24 Sep 2021 06:12:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7109F6918E;\n\tFri, 24 Sep 2021 08:12:33 +0200 (CEST)","from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com\n\t[IPv6:2607:f8b0:4864:20::b32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B3E1D60242\n\tfor ;\n\tFri, 24 Sep 2021 08:12:31 +0200 (CEST)","by mail-yb1-xb32.google.com with SMTP id w19so739555ybs.3\n\tfor ;\n\tThu, 23 Sep 2021 23:12:31 -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=\"cZN+CfLv\"; 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=Cf+p/qVhKr1KlO0ncbR0wKD8QPsz57+1IYO7SFue+RI=;\n\tb=cZN+CfLvYm3DVXNBzwWoicFbMNpmQ9NO4DZ9hAQbUtcWRZggsWw6Tmvnh9Rn+uGs1B\n\tmwCzdc/pgCLhdiGG90P+LRoIjUR1R8T050cLHbAqMvkEKi09eCz+5KLQ5221FaJGaW+e\n\tprbG0dJL//dmPXzkurrZxMuY3H8BDSA7ouQ7Q/Z2iin9JpRzRWdbULj2EjsAYKHXROUT\n\tzoNTK4CR8WdAga4hu69W6z5H0gIBOP9h7KmeyeLRqQYoP8RVhN7AFi7Jssrt7b+YLosI\n\tH2ZD14GBJFfgC9CMnF2xoFjSL+/2WhZelaqHODPzzLVPE2h+b1P5bivIFzkubcsbj+QF\n\t+7qg==","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=Cf+p/qVhKr1KlO0ncbR0wKD8QPsz57+1IYO7SFue+RI=;\n\tb=HK8IwN/szMKI89A1RQrvkoNnzK54mgqnV8vHzzFueCQGHpWsRvegY9CV2LW9YL5Pdw\n\t4LeHf9cEZfEqdPoL1aXMC217lvO6hF+7QnVkCQ7mf0oSK+1dYJSXB+1EpP3Tf9Xv6J3n\n\tzK/kPQoh+QAJ7Bw1EdXLlK/g/vu1vACnJIXDb7CA5eozQ/IVN9rqLbJIKrc7CaohiA6H\n\tRwwbL+xXVq7bSce8L8OY62ZxGmHItd17frTgAbFUN+ztp/7r6faWePVdOQ35FOUsT9vR\n\tFdnZYDMjR0O1hyULt0dVCPfd3ObqXvfqDsBh4QrFngLzydQS+ULh9207a3r6XXcWu5/+\n\tv80A==","X-Gm-Message-State":"AOAM531xsPTtiHvPISI9h2D6TPS8/vFq4sDnidnZVw+AgKKljsyv0hv9\n\t3jw7guGG/pn6Y8K3oMVTVGsaqH6/ingsgZMGgz0=","X-Google-Smtp-Source":"ABdhPJzaMfDMzTLXwoszqBEcwF4pVeepPsInUQ7VcbAm9FefwLxRiu6KEKIf1uecqdnCcPPfFjkejNpybdaqwpi/L3s=","X-Received":"by 2002:a05:6902:1247:: with SMTP id\n\tt7mr10765686ybu.285.1632463950442; \n\tThu, 23 Sep 2021 23:12:30 -0700 (PDT)","MIME-Version":"1.0","References":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>\n\t<20210924060851.GU4382@pyrite.rasen.tech>","In-Reply-To":"<20210924060851.GU4382@pyrite.rasen.tech>","From":"Vedant Paranjape ","Date":"Fri, 24 Sep 2021 11:42:18 +0530","Message-ID":"","To":"Paul Elder ","Content-Type":"multipart/alternative; boundary=\"000000000000dff3ce05ccb7a126\"","Subject":"Re: [libcamera-devel] [PATCH v3] 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 ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19828,"web_url":"https://patchwork.libcamera.org/comment/19828/","msgid":"<20210924070459.GV4382@pyrite.rasen.tech>","date":"2021-09-24T07:04:59","subject":"Re: [libcamera-devel] [PATCH v3] 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":"On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote:\n> Hi Paul,\n> Can you also tell me the expected indentation ?\n> \n> checkstyle suggestions didn't adhere with the 80 char width thing\n\n80 char can be broken (up to 120) as long as it increases readability.\n\n> \n> \n> \n> \n> On Fri, 24 Sep, 2021, 11:39 , wrote:\n> \n> Hi Vedant,\n> \n> On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:\n> > This patch adds a test to test if multi stream using libcamera's\n> > gstreamer element works.\n> >\n> > Test will run only on devices that support multistream output, i.e.,\n> \n> I think s/output/capture/ might be better.\n> \n> Also s/i.e./e.g/\n> \n> > devices that use IPU3 and raspberrypi pipeline. This was tested on\n> > a Raspberry Pi 4B+.\n> >\n> > Signed-off-by: Vedant Paranjape \n> > Reviewed-by: Nicolas Dufresne \n> > ---\n> >  .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> >  test/gstreamer/meson.build                    |   1 +\n> >  2 files changed, 129 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/\n> gstreamer/gstreamer_multi_stream_test.cpp\n> > new file mode 100644\n> > index 000000000000..6f63f1e748e2\n> > --- /dev/null\n> > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > @@ -0,0 +1,128 @@\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> You don't use anything from this.\n> \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/source_paths.h\"\n> \n> Nor this.\n> \n> (The single stream tests have these too)\n> \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 &camera : cm.cameras()) {\n> > +                     if (camera->streams().size() > 1) {\n> > +                             cameraName = camera->id();\n> > +                             cameraFound = true;\n> > +                             cm.stop();\n> > +                             break;\n> > +                     }\n> > +             }\n> > +\n> > +             if (!cameraFound) {\n> > +                     cm.stop();\n> > +                     return TestSkip;\n> > +             }\n> > +\n> > +             const gchar *streamDescription = \"queue ! fakesink\";\n> > +             g_autoptr(GError) error = NULL;\n> > +\n> > +             stream0_ = gst_parse_bin_from_description_full\n> (streamDescription, TRUE,\n> > +                                             NULL,\n> > +                                           \n>  GST_PARSE_FLAG_FATAL_ERRORS,\n> > +                                             &error);\n\nOh I let this and the next one go because it goes over, but I think I\nagree with checkstyle here:\n\n\n \t\tstream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n\t\t\t\t\t\t\t NULL,\n\t\t\t\t\t\t\t GST_PARSE_FLAG_FATAL_ERRORS,\n\t\t\t\t\t\t\t &error);\n\n\n> > +             if (!stream0_) {\n> > +                     g_printerr(\"Stream0 could not be created (%s)\\n\",\n> error->message);\n> > +                     return TestFail;\n> > +             }\n> > +             g_object_ref_sink(stream0_);\n> > +\n> > +             stream1_ = gst_parse_bin_from_description_full\n> (streamDescription, TRUE,\n> > +                                             NULL,\n> > +                                           \n>  GST_PARSE_FLAG_FATAL_ERRORS,\n> > +                                             &error);\n> > +             if (!stream1_) {\n> > +                     g_printerr(\"Stream1 could not be created (%s)\\n\",\n> error->message);\n> > +                     return TestFail;\n> > +             }\n> > +             g_object_ref_sink(stream1_);\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\", cameraName.c_str\n> (), NULL);\n> > +\n> > +             /* Build the pipeline */\n> > +             gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> > +                                                     stream0_, stream1_,\n> NULL);\n> \n> Indentation.\n\ncheckstyle has this one\n\n> \n> > +\n> > +             g_autoptr(GstPad) src_pad = gst_element_get_static_pad\n> (libcameraSrc_, \"src\");\n> > +             g_autoptr(GstPad) request_pad = gst_element_get_request_pad\n> (libcameraSrc_, \"src_%u\");\n> > +\n> > +             {\n> > +                     g_autoptr(GstPad) queue0_sink_pad =\n> gst_element_get_static_pad(stream0_, \"sink\");\n> > +                     g_autoptr(GstPad) queue1_sink_pad =\n> gst_element_get_static_pad(stream1_, \"sink\");\n> > +\n> > +                     if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> GST_PAD_LINK_OK\n> > +                             || gst_pad_link(request_pad,\n> queue1_sink_pad) != GST_PAD_LINK_OK) {\n> \n> Indentation.\n\n\n\t\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\n> \n> > +                             g_printerr(\"Pads could not be linked.\\n\");\n> > +                             return TestFail;\n> > +                     }\n> > +             }\n> > +\n> > +             if (startPipeline() != TestPass)\n> > +                     return TestFail;\n> > +\n> > +             if (processEvent() != TestPass)\n> > +                     return TestFail;\n> > +\n> > +             return TestPass;\n> > +     }\n> > +\n> > +     void cleanup() override\n> > +     {\n> > +             g_clear_object(&stream0_);\n> > +             g_clear_object(&stream1_);\n> > +     }\n> > +\n> > +private:\n> > +     std::string cameraName;\n> \n> s/cameraName/cameraName_/\n> \n> > +     GstElement *stream0_;\n> > +     GstElement *stream1_;\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> It would be nice if the second elements lined up.\n> \n> \n> Otherwise, looks good.\n> \n> \n> Can I add a R-b then?\n\nNot unless I give you one. I'll check in a new ersion.\n\n\nPaul\n\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 9EDEEBF01C\n\tfor ;\n\tFri, 24 Sep 2021 07:05:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E08866918C;\n\tFri, 24 Sep 2021 09:05:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04A0269186\n\tfor ;\n\tFri, 24 Sep 2021 09:05:07 +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 5FC8E45E;\n\tFri, 24 Sep 2021 09:05:05 +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=\"P3KNFcqY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632467106;\n\tbh=YaMsFdRcgCgmiJE4K2TiD4G23Yegt223sKZXxpLo5JQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=P3KNFcqYHF0NqunsEZYo53E3LwuxKGDcNtCVdjcdr+2qCc6xWOLNsNd4UcNMJ4EtK\n\tSIn4wJD50GTIZXyVJHXjXVsU4h3gAf1KhOQxJNO9l6it33D7rLoPmBGpGONtDUKNfT\n\tE5XCyuRgL5o8NvsVYwU2pN4pSsIomlrCuEq+V+j4=","Date":"Fri, 24 Sep 2021 16:04:59 +0900","From":"paul.elder@ideasonboard.com","To":"Vedant Paranjape ","Message-ID":"<20210924070459.GV4382@pyrite.rasen.tech>","References":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>\n\t<20210924060851.GU4382@pyrite.rasen.tech>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3] 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 ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19832,"web_url":"https://patchwork.libcamera.org/comment/19832/","msgid":"","date":"2021-09-24T08:32:07","subject":"Re: [libcamera-devel] [PATCH v3] 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":"On Fri, Sep 24, 2021 at 12:35 PM wrote:\n>\n> On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote:\n> > Hi Paul,\n> > Can you also tell me the expected indentation ?\n> >\n> > checkstyle suggestions didn't adhere with the 80 char width thing\n>\n> 80 char can be broken (up to 120) as long as it increases readability.\n>\n> >\n> >\n> >\n> >\n> > On Fri, 24 Sep, 2021, 11:39 , wrote:\n> >\n> > Hi Vedant,\n> >\n> > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:\n> > > This patch adds a test to test if multi stream using libcamera's\n> > > gstreamer element works.\n> > >\n> > > Test will run only on devices that support multistream output, i.e.,\n> >\n> > I think s/output/capture/ might be better.\n> >\n> > Also s/i.e./e.g/\n> >\n> > > devices that use IPU3 and raspberrypi pipeline. This was tested on\n> > > a Raspberry Pi 4B+.\n> > >\n> > > Signed-off-by: Vedant Paranjape \n> > > Reviewed-by: Nicolas Dufresne \n> > > ---\n> > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> > > test/gstreamer/meson.build | 1 +\n> > > 2 files changed, 129 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/\n> > gstreamer/gstreamer_multi_stream_test.cpp\n> > > new file mode 100644\n> > > index 000000000000..6f63f1e748e2\n> > > --- /dev/null\n> > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > > @@ -0,0 +1,128 @@\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> > You don't use anything from this.\n> >\n> > > +\n> > > +#include \n> > > +\n> > > +#include \"libcamera/internal/source_paths.h\"\n> >\n> > Nor this.\n> >\n> > (The single stream tests have these too)\n> >\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 &camera : cm.cameras()) {\n> > > + if (camera->streams().size() > 1) {\n> > > + cameraName = camera->id();\n> > > + cameraFound = true;\n> > > + cm.stop();\n> > > + break;\n> > > + }\n> > > + }\n> > > +\n> > > + if (!cameraFound) {\n> > > + cm.stop();\n> > > + return TestSkip;\n> > > + }\n> > > +\n> > > + const gchar *streamDescription = \"queue ! fakesink\";\n> > > + g_autoptr(GError) error = NULL;\n> > > +\n> > > + stream0_ = gst_parse_bin_from_description_full\n> > (streamDescription, TRUE,\n> > > + NULL,\n> > > +\n> > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > + &error);\n>\n> Oh I let this and the next one go because it goes over, but I think I\n> agree with checkstyle here:\n>\n>\n> stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> NULL,\n> GST_PARSE_FLAG_FATAL_ERRORS,\n> &error);\n>\n>\n> > > + if (!stream0_) {\n> > > + g_printerr(\"Stream0 could not be created (%s)\\n\",\n> > error->message);\n> > > + return TestFail;\n> > > + }\n> > > + g_object_ref_sink(stream0_);\n> > > +\n> > > + stream1_ = gst_parse_bin_from_description_full\n> > (streamDescription, TRUE,\n> > > + NULL,\n> > > +\n> > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > + &error);\n> > > + if (!stream1_) {\n> > > + g_printerr(\"Stream1 could not be created (%s)\\n\",\n> > error->message);\n> > > + return TestFail;\n> > > + }\n> > > + g_object_ref_sink(stream1_);\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\", cameraName.c_str\n> > (), NULL);\n> > > +\n> > > + /* Build the pipeline */\n> > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> > > + stream0_, stream1_,\n> > NULL);\n> >\n> > Indentation.\n>\n> checkstyle has this one\n>\n> >\n> > > +\n> > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad\n> > (libcameraSrc_, \"src\");\n> > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad\n> > (libcameraSrc_, \"src_%u\");\n\nThis looks bad.\n\n> > > +\n> > > + {\n> > > + g_autoptr(GstPad) queue0_sink_pad =\n> > gst_element_get_static_pad(stream0_, \"sink\");\n> > > + g_autoptr(GstPad) queue1_sink_pad =\n> > gst_element_get_static_pad(stream1_, \"sink\");\n\neven this.\n\n> > > +\n> > > + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> > GST_PAD_LINK_OK\n> > > + || gst_pad_link(request_pad,\n> > queue1_sink_pad) != GST_PAD_LINK_OK) {\n> >\n> > Indentation.\n>\n>\n> if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK ||\n> gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n>\n> >\n> > > + g_printerr(\"Pads could not be linked.\\n\");\n> > > + return TestFail;\n> > > + }\n> > > + }\n> > > +\n> > > + if (startPipeline() != TestPass)\n> > > + return TestFail;\n> > > +\n> > > + if (processEvent() != TestPass)\n> > > + return TestFail;\n> > > +\n> > > + return TestPass;\n> > > + }\n> > > +\n> > > + void cleanup() override\n> > > + {\n> > > + g_clear_object(&stream0_);\n> > > + g_clear_object(&stream1_);\n> > > + }\n> > > +\n> > > +private:\n> > > + std::string cameraName;\n> >\n> > s/cameraName/cameraName_/\n> >\n> > > + GstElement *stream0_;\n> > > + GstElement *stream1_;\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> > It would be nice if the second elements lined up.\n> >\n> >\n> > Otherwise, looks good.\n> >\n> >\n> > Can I add a R-b then?\n>\n> Not unless I give you one. I'll check in a new ersion.\n>\n>\n> Paul\n>\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 62CF8BF01C\n\tfor ;\n\tFri, 24 Sep 2021 08:32:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B953A6918E;\n\tFri, 24 Sep 2021 10:32:22 +0200 (CEST)","from mail-yb1-xb2b.google.com (mail-yb1-xb2b.google.com\n\t[IPv6:2607:f8b0:4864:20::b2b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CC9B69186\n\tfor ;\n\tFri, 24 Sep 2021 10:32:20 +0200 (CEST)","by mail-yb1-xb2b.google.com with SMTP id h2so4148784ybi.13\n\tfor ;\n\tFri, 24 Sep 2021 01:32:20 -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=\"oIAJC0fz\"; 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=o2pequFZJZLpyIiOOCHwym/TRiOcZ0y+mEnidsufQ2A=;\n\tb=oIAJC0fzoi3wnBVFHXXFPeSAGrC0vw5pFehNPWtYH8n7D/S2aypHkQQBoYBvM5IPFg\n\tgaEhCGnukb2p5bV/7r99A/aOfe6YD9J2QwkY/s9mNkThFfpKUA5AdcbSHP2bhg3a+lKJ\n\t1+bSXgfVxSnh7cVYi+GMkmSa7jEnxv8SnmjaXVjwISgFfhrUvcNU9qH77rU+pmwy8Tlg\n\tuEFHkjJQemTIaSY8bEciXvuNjXsFW4OSZRm8tSn75t31wg+YJhVTk66yeNGCYXewqd+8\n\t8QNhbjjaLsLgwM0GW2v0r/QW8lvzuRegzs+WoAziplKMMkh8RKuZERFfnclZmuzKF/Zr\n\tEXBw==","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=o2pequFZJZLpyIiOOCHwym/TRiOcZ0y+mEnidsufQ2A=;\n\tb=D/uJd5RbagVYC2FFrrC4fbx1s4JnhGxHJjPARgMXai/XlOlEdynanxy8ddAOk27FjQ\n\tEjPznxG+Mti0dCXquE+Fqp5F9ScbCpjEBwq59NWJ198/Xin6y7O3VifoZuZ0JjFVwZuh\n\t4I1+s5LYWy6A3G5Xt+z48vbTqhl9XHXoaJrOxVv83eKQrcHBneI6EQlj/MidX1CtyEXt\n\tp59bsUGVLFihdzrqRjmmuTDh4eBAxNqQwZWzNxqhB78kfLNQaHjFwOqCNW49MHJwOJp6\n\t+Zh0XdAX73mPknQj1weRWGyOe968nXLFU9D2niFdRQKVEBkj2zgObIGY+wMx0hbsslQJ\n\tygLQ==","X-Gm-Message-State":"AOAM532O4YfXoIfSp08qUydotcKKGEirPmxPDf8t6rboyZ3frhy1IsGc\n\tRyIsoPSD9iQ0r0CJ19xMvsP6PdBCJFiCbB6iSmlnDGeK6zAwtw==","X-Google-Smtp-Source":"ABdhPJwsIwonUhNuBJ2IMncS2+0aBWPpmLt+AvQo9e9wZGWX1PVrj6tq10QvH+4Xt4n0lNnSlcfrVtYXk9ZR6Co6Kiw=","X-Received":"by 2002:a25:4507:: with SMTP id s7mr10759936yba.4.1632472339378; \n\tFri, 24 Sep 2021 01:32:19 -0700 (PDT)","MIME-Version":"1.0","References":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>\n\t<20210924060851.GU4382@pyrite.rasen.tech>\n\t\n\t<20210924070459.GV4382@pyrite.rasen.tech>","In-Reply-To":"<20210924070459.GV4382@pyrite.rasen.tech>","From":"Vedant Paranjape ","Date":"Fri, 24 Sep 2021 14:02:07 +0530","Message-ID":"","To":"Paul Elder ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3] 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 ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19833,"web_url":"https://patchwork.libcamera.org/comment/19833/","msgid":"<20210924084125.GW4382@pyrite.rasen.tech>","date":"2021-09-24T08:41:25","subject":"Re: [libcamera-devel] [PATCH v3] 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":"On Fri, Sep 24, 2021 at 02:02:07PM +0530, Vedant Paranjape wrote:\n> On Fri, Sep 24, 2021 at 12:35 PM wrote:\n> >\n> > On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote:\n> > > Hi Paul,\n> > > Can you also tell me the expected indentation ?\n> > >\n> > > checkstyle suggestions didn't adhere with the 80 char width thing\n> >\n> > 80 char can be broken (up to 120) as long as it increases readability.\n> >\n> > >\n> > >\n> > >\n> > >\n> > > On Fri, 24 Sep, 2021, 11:39 , wrote:\n> > >\n> > > Hi Vedant,\n> > >\n> > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:\n> > > > This patch adds a test to test if multi stream using libcamera's\n> > > > gstreamer element works.\n> > > >\n> > > > Test will run only on devices that support multistream output, i.e.,\n> > >\n> > > I think s/output/capture/ might be better.\n> > >\n> > > Also s/i.e./e.g/\n> > >\n> > > > devices that use IPU3 and raspberrypi pipeline. This was tested on\n> > > > a Raspberry Pi 4B+.\n> > > >\n> > > > Signed-off-by: Vedant Paranjape \n> > > > Reviewed-by: Nicolas Dufresne \n> > > > ---\n> > > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> > > > test/gstreamer/meson.build | 1 +\n> > > > 2 files changed, 129 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/\n> > > gstreamer/gstreamer_multi_stream_test.cpp\n> > > > new file mode 100644\n> > > > index 000000000000..6f63f1e748e2\n> > > > --- /dev/null\n> > > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > > > @@ -0,0 +1,128 @@\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> > > You don't use anything from this.\n> > >\n> > > > +\n> > > > +#include \n> > > > +\n> > > > +#include \"libcamera/internal/source_paths.h\"\n> > >\n> > > Nor this.\n> > >\n> > > (The single stream tests have these too)\n> > >\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 &camera : cm.cameras()) {\n> > > > + if (camera->streams().size() > 1) {\n> > > > + cameraName = camera->id();\n> > > > + cameraFound = true;\n> > > > + cm.stop();\n> > > > + break;\n> > > > + }\n> > > > + }\n> > > > +\n> > > > + if (!cameraFound) {\n> > > > + cm.stop();\n> > > > + return TestSkip;\n> > > > + }\n> > > > +\n> > > > + const gchar *streamDescription = \"queue ! fakesink\";\n> > > > + g_autoptr(GError) error = NULL;\n> > > > +\n> > > > + stream0_ = gst_parse_bin_from_description_full\n> > > (streamDescription, TRUE,\n> > > > + NULL,\n> > > > +\n> > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > + &error);\n> >\n> > Oh I let this and the next one go because it goes over, but I think I\n> > agree with checkstyle here:\n> >\n> >\n> > stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > NULL,\n> > GST_PARSE_FLAG_FATAL_ERRORS,\n> > &error);\n> >\n> >\n> > > > + if (!stream0_) {\n> > > > + g_printerr(\"Stream0 could not be created (%s)\\n\",\n> > > error->message);\n> > > > + return TestFail;\n> > > > + }\n> > > > + g_object_ref_sink(stream0_);\n> > > > +\n> > > > + stream1_ = gst_parse_bin_from_description_full\n> > > (streamDescription, TRUE,\n> > > > + NULL,\n> > > > +\n> > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > + &error);\n> > > > + if (!stream1_) {\n> > > > + g_printerr(\"Stream1 could not be created (%s)\\n\",\n> > > error->message);\n> > > > + return TestFail;\n> > > > + }\n> > > > + g_object_ref_sink(stream1_);\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\", cameraName.c_str\n> > > (), NULL);\n> > > > +\n> > > > + /* Build the pipeline */\n> > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> > > > + stream0_, stream1_,\n> > > NULL);\n> > >\n> > > Indentation.\n> >\n> > checkstyle has this one\n> >\n> > >\n> > > > +\n> > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad\n> > > (libcameraSrc_, \"src\");\n> > > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad\n> > > (libcameraSrc_, \"src_%u\");\n> \n> This looks bad.\n\nGet rid of the html indentation...\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\nI think it's fine. If you're concerned about the line length you can\nindent it.\n\n+\t\tg_autoptr(GstPad) src_pad =\n+\t\t\tgst_element_get_static_pad(libcameraSrc_, \"src\");\n+\t\tg_autoptr(GstPad) request_pad =\n+\t\t\tgst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n\nEither way is fine.\n\n> \n> > > > +\n> > > > + {\n> > > > + g_autoptr(GstPad) queue0_sink_pad =\n> > > gst_element_get_static_pad(stream0_, \"sink\");\n> > > > + g_autoptr(GstPad) queue1_sink_pad =\n> > > gst_element_get_static_pad(stream1_, \"sink\");\n> \n> even this.\n\nSame here.\n\n\nPaul\n\n> \n> > > > +\n> > > > + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> > > GST_PAD_LINK_OK\n> > > > + || gst_pad_link(request_pad,\n> > > queue1_sink_pad) != GST_PAD_LINK_OK) {\n> > >\n> > > Indentation.\n> >\n> >\n> > if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK ||\n> > gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n> >\n> > >\n> > > > + g_printerr(\"Pads could not be linked.\\n\");\n> > > > + return TestFail;\n> > > > + }\n> > > > + }\n> > > > +\n> > > > + if (startPipeline() != TestPass)\n> > > > + return TestFail;\n> > > > +\n> > > > + if (processEvent() != TestPass)\n> > > > + return TestFail;\n> > > > +\n> > > > + return TestPass;\n> > > > + }\n> > > > +\n> > > > + void cleanup() override\n> > > > + {\n> > > > + g_clear_object(&stream0_);\n> > > > + g_clear_object(&stream1_);\n> > > > + }\n> > > > +\n> > > > +private:\n> > > > + std::string cameraName;\n> > >\n> > > s/cameraName/cameraName_/\n> > >\n> > > > + GstElement *stream0_;\n> > > > + GstElement *stream1_;\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> > > It would be nice if the second elements lined up.\n> > >\n> > >\n> > > Otherwise, looks good.\n> > >\n> > >\n> > > Can I add a R-b then?\n> >\n> > Not unless I give you one. I'll check in a new ersion.\n> >\n> >\n> > Paul\n> >\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 7750CBF01C\n\tfor ;\n\tFri, 24 Sep 2021 08:41:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E56C76918C;\n\tFri, 24 Sep 2021 10:41:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 14D8469186\n\tfor ;\n\tFri, 24 Sep 2021 10:41:34 +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 2BF5A45E;\n\tFri, 24 Sep 2021 10:41:31 +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=\"Pp0jXCFk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632472893;\n\tbh=gYa33EJ84vRDI3jxPTmaiSC8Cw/PaLwmdfA8RyczznU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Pp0jXCFkkPmxQbpgWzgOc0UQBI9Nh45MqCyKGKpxzXpU+ZsOXuKOCtScfBING1rw8\n\teZCeTIr0uT6zsMuJGAN7MLNQU+HweSo4ZDrsYXAB2beFYOVp1V0ZJZm74Xzf3AiTBf\n\tCDbWgJ+P7tOum5wNVJd/A6PP6uZk1eSnzrK6rQ0A=","Date":"Fri, 24 Sep 2021 17:41:25 +0900","From":"paul.elder@ideasonboard.com","To":"Vedant Paranjape ","Message-ID":"<20210924084125.GW4382@pyrite.rasen.tech>","References":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>\n\t<20210924060851.GU4382@pyrite.rasen.tech>\n\t\n\t<20210924070459.GV4382@pyrite.rasen.tech>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3] 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 ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19834,"web_url":"https://patchwork.libcamera.org/comment/19834/","msgid":"","date":"2021-09-24T08:44:11","subject":"Re: [libcamera-devel] [PATCH v3] 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":"On Fri, Sep 24, 2021 at 2:11 PM wrote:\n>\n> On Fri, Sep 24, 2021 at 02:02:07PM +0530, Vedant Paranjape wrote:\n> > On Fri, Sep 24, 2021 at 12:35 PM wrote:\n> > >\n> > > On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote:\n> > > > Hi Paul,\n> > > > Can you also tell me the expected indentation ?\n> > > >\n> > > > checkstyle suggestions didn't adhere with the 80 char width thing\n> > >\n> > > 80 char can be broken (up to 120) as long as it increases readability.\n> > >\n> > > >\n> > > >\n> > > >\n> > > >\n> > > > On Fri, 24 Sep, 2021, 11:39 , wrote:\n> > > >\n> > > > Hi Vedant,\n> > > >\n> > > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:\n> > > > > This patch adds a test to test if multi stream using libcamera's\n> > > > > gstreamer element works.\n> > > > >\n> > > > > Test will run only on devices that support multistream output, i.e.,\n> > > >\n> > > > I think s/output/capture/ might be better.\n> > > >\n> > > > Also s/i.e./e.g/\n> > > >\n> > > > > devices that use IPU3 and raspberrypi pipeline. This was tested on\n> > > > > a Raspberry Pi 4B+.\n> > > > >\n> > > > > Signed-off-by: Vedant Paranjape \n> > > > > Reviewed-by: Nicolas Dufresne \n> > > > > ---\n> > > > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> > > > > test/gstreamer/meson.build | 1 +\n> > > > > 2 files changed, 129 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/\n> > > > gstreamer/gstreamer_multi_stream_test.cpp\n> > > > > new file mode 100644\n> > > > > index 000000000000..6f63f1e748e2\n> > > > > --- /dev/null\n> > > > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > > > > @@ -0,0 +1,128 @@\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> > > > You don't use anything from this.\n> > > >\n> > > > > +\n> > > > > +#include \n> > > > > +\n> > > > > +#include \"libcamera/internal/source_paths.h\"\n> > > >\n> > > > Nor this.\n> > > >\n> > > > (The single stream tests have these too)\n> > > >\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 &camera : cm.cameras()) {\n> > > > > + if (camera->streams().size() > 1) {\n> > > > > + cameraName = camera->id();\n> > > > > + cameraFound = true;\n> > > > > + cm.stop();\n> > > > > + break;\n> > > > > + }\n> > > > > + }\n> > > > > +\n> > > > > + if (!cameraFound) {\n> > > > > + cm.stop();\n> > > > > + return TestSkip;\n> > > > > + }\n> > > > > +\n> > > > > + const gchar *streamDescription = \"queue ! fakesink\";\n> > > > > + g_autoptr(GError) error = NULL;\n> > > > > +\n> > > > > + stream0_ = gst_parse_bin_from_description_full\n> > > > (streamDescription, TRUE,\n> > > > > + NULL,\n> > > > > +\n> > > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > > + &error);\n> > >\n> > > Oh I let this and the next one go because it goes over, but I think I\n> > > agree with checkstyle here:\n> > >\n> > >\n> > > stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > > NULL,\n> > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > &error);\n> > >\n> > >\n> > > > > + if (!stream0_) {\n> > > > > + g_printerr(\"Stream0 could not be created (%s)\\n\",\n> > > > error->message);\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > + g_object_ref_sink(stream0_);\n> > > > > +\n> > > > > + stream1_ = gst_parse_bin_from_description_full\n> > > > (streamDescription, TRUE,\n> > > > > + NULL,\n> > > > > +\n> > > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > > + &error);\n> > > > > + if (!stream1_) {\n> > > > > + g_printerr(\"Stream1 could not be created (%s)\\n\",\n> > > > error->message);\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > + g_object_ref_sink(stream1_);\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\", cameraName.c_str\n> > > > (), NULL);\n> > > > > +\n> > > > > + /* Build the pipeline */\n> > > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> > > > > + stream0_, stream1_,\n> > > > NULL);\n> > > >\n> > > > Indentation.\n> > >\n> > > checkstyle has this one\n> > >\n> > > >\n> > > > > +\n> > > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad\n> > > > (libcameraSrc_, \"src\");\n> > > > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad\n> > > > (libcameraSrc_, \"src_%u\");\n> >\n> > This looks bad.\n>\n> Get rid of the html indentation...\nWhat ? I put in the indentation myself.\n>\n> + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, \"src\");\n> + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n>\n> I think it's fine. If you're concerned about the line length you can\n> indent it.\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>\n> Either way is fine.\n>\n> >\n> > > > > +\n> > > > > + {\n> > > > > + g_autoptr(GstPad) queue0_sink_pad =\n> > > > gst_element_get_static_pad(stream0_, \"sink\");\n> > > > > + g_autoptr(GstPad) queue1_sink_pad =\n> > > > gst_element_get_static_pad(stream1_, \"sink\");\n> >\n> > even this.\n>\n> Same here.\n>\n>\n> Paul\n>\n> >\n> > > > > +\n> > > > > + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> > > > GST_PAD_LINK_OK\n> > > > > + || gst_pad_link(request_pad,\n> > > > queue1_sink_pad) != GST_PAD_LINK_OK) {\n> > > >\n> > > > Indentation.\n> > >\n> > >\n> > > if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK ||\n> > > gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n> > >\n> > > >\n> > > > > + g_printerr(\"Pads could not be linked.\\n\");\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > + }\n> > > > > +\n> > > > > + if (startPipeline() != TestPass)\n> > > > > + return TestFail;\n> > > > > +\n> > > > > + if (processEvent() != TestPass)\n> > > > > + return TestFail;\n> > > > > +\n> > > > > + return TestPass;\n> > > > > + }\n> > > > > +\n> > > > > + void cleanup() override\n> > > > > + {\n> > > > > + g_clear_object(&stream0_);\n> > > > > + g_clear_object(&stream1_);\n> > > > > + }\n> > > > > +\n> > > > > +private:\n> > > > > + std::string cameraName;\n> > > >\n> > > > s/cameraName/cameraName_/\n> > > >\n> > > > > + GstElement *stream0_;\n> > > > > + GstElement *stream1_;\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> > > > It would be nice if the second elements lined up.\n> > > >\n> > > >\n> > > > Otherwise, looks good.\n> > > >\n> > > >\n> > > > Can I add a R-b then?\n> > >\n> > > Not unless I give you one. I'll check in a new ersion.\n> > >\n> > >\n> > > Paul\n> > >\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 1C95FBDC71\n\tfor ;\n\tFri, 24 Sep 2021 08:44:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7FC756918E;\n\tFri, 24 Sep 2021 10:44:26 +0200 (CEST)","from mail-yb1-xb2a.google.com (mail-yb1-xb2a.google.com\n\t[IPv6:2607:f8b0:4864:20::b2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7E58D69186\n\tfor ;\n\tFri, 24 Sep 2021 10:44:24 +0200 (CEST)","by mail-yb1-xb2a.google.com with SMTP id w19so1459947ybs.3\n\tfor ;\n\tFri, 24 Sep 2021 01:44:24 -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=\"lU9FTixL\"; 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=j0rvLDcpKDtWNebKSznFC65yVIo5mvDzo2o58bokcNM=;\n\tb=lU9FTixLENXTt9c7wfLsM16ygVK4Xzzba56zAZWmbOnZROJwWqosAHL+oYsj972EL+\n\tstuYZ/Q6KK/vSX/meolZriqYlCYRf7NqG2iIP0AqYPwPnuazAdYxIVoam6WnHwJVGWTB\n\tI4QZKNZ9x17wbhOvDfZc2heBALB4xpS/jhVIHIiSxCaLvWa9GVxbbU8vmLp0bQ4Bg6UE\n\t7JQgeecCg4CqCii7rzeia313E2X1LGDY84s213a0/gC5f61m18S08LHgOZPofS5orGR0\n\t9w5/k5GyQ3GlcwbYVZpThTH+nj4eS9/g33DNDz3HxJOpvZtLHBEYQq4q1RzHb6zbHZiG\n\tnVGQ==","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=j0rvLDcpKDtWNebKSznFC65yVIo5mvDzo2o58bokcNM=;\n\tb=01PDfi0DQAt8sHh0PMOtx2RMH+eWWN2DWIyBRHq8yFUju9c0A4P12jfVF/qD7f1nJN\n\tURVGXGklwRxTqRSryLMmhfrLyR2gMFgUsTBZz3nPDHt3sSH8H0srL16t8erK1P1+ETdg\n\t6LxZEOXmeOnPiPX5NazuxFEppmWIPWk4lZkK53VOBR7qfvfaGPHlH9uq1GttefZTWSVE\n\tEPWadK6GpncCHMy3kxeNVm7DcWWp9Hk2WMKyj7Fx+EWcH/NKyRq2ojY8DqhN7VsFLmfT\n\tdG7q+1NmG3eihIR2cittoGyG1sVSTR8UVqi8Hqa0rTbnpq6CjAtyuVz4CYBzUypz7/Yp\n\tMx9Q==","X-Gm-Message-State":"AOAM531iq34MFxj1kQglcgG+UVIP90khm1imJK4bunLPw03lm1hD52EF\n\tqs4R8U8p92mIkd1TpU5lCXqrCXzs7AbQS+Vu4PNWMQX+f8dtIg==","X-Google-Smtp-Source":"ABdhPJyaCIzSqMMe03JKTqNTOeHeM6BDpc8ss5+pWODHF8Ynt3WQ9ZiB28LKSsqh2oMWlyLObK28lXe0q9FAkSYsah4=","X-Received":"by 2002:a25:4507:: with SMTP id s7mr10806324yba.4.1632473063314; \n\tFri, 24 Sep 2021 01:44:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>\n\t<20210924060851.GU4382@pyrite.rasen.tech>\n\t\n\t<20210924070459.GV4382@pyrite.rasen.tech>\n\t\n\t<20210924084125.GW4382@pyrite.rasen.tech>","In-Reply-To":"<20210924084125.GW4382@pyrite.rasen.tech>","From":"Vedant Paranjape ","Date":"Fri, 24 Sep 2021 14:14:11 +0530","Message-ID":"","To":"Paul Elder ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3] 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 ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19838,"web_url":"https://patchwork.libcamera.org/comment/19838/","msgid":"<20210924093438.GZ4382@pyrite.rasen.tech>","date":"2021-09-24T09:34:38","subject":"Re: [libcamera-devel] [PATCH v3] 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":"On Fri, Sep 24, 2021 at 02:14:11PM +0530, Vedant Paranjape wrote:\n> On Fri, Sep 24, 2021 at 2:11 PM wrote:\n> >\n> > On Fri, Sep 24, 2021 at 02:02:07PM +0530, Vedant Paranjape wrote:\n> > > On Fri, Sep 24, 2021 at 12:35 PM wrote:\n> > > >\n> > > > On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote:\n> > > > > Hi Paul,\n> > > > > Can you also tell me the expected indentation ?\n> > > > >\n> > > > > checkstyle suggestions didn't adhere with the 80 char width thing\n> > > >\n> > > > 80 char can be broken (up to 120) as long as it increases readability.\n> > > >\n> > > > >\n> > > > >\n> > > > >\n> > > > >\n> > > > > On Fri, 24 Sep, 2021, 11:39 , wrote:\n> > > > >\n> > > > > Hi Vedant,\n> > > > >\n> > > > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:\n> > > > > > This patch adds a test to test if multi stream using libcamera's\n> > > > > > gstreamer element works.\n> > > > > >\n> > > > > > Test will run only on devices that support multistream output, i.e.,\n> > > > >\n> > > > > I think s/output/capture/ might be better.\n> > > > >\n> > > > > Also s/i.e./e.g/\n> > > > >\n> > > > > > devices that use IPU3 and raspberrypi pipeline. This was tested on\n> > > > > > a Raspberry Pi 4B+.\n> > > > > >\n> > > > > > Signed-off-by: Vedant Paranjape \n> > > > > > Reviewed-by: Nicolas Dufresne \n> > > > > > ---\n> > > > > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++\n> > > > > > test/gstreamer/meson.build | 1 +\n> > > > > > 2 files changed, 129 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/\n> > > > > gstreamer/gstreamer_multi_stream_test.cpp\n> > > > > > new file mode 100644\n> > > > > > index 000000000000..6f63f1e748e2\n> > > > > > --- /dev/null\n> > > > > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp\n> > > > > > @@ -0,0 +1,128 @@\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> > > > > You don't use anything from this.\n> > > > >\n> > > > > > +\n> > > > > > +#include \n> > > > > > +\n> > > > > > +#include \"libcamera/internal/source_paths.h\"\n> > > > >\n> > > > > Nor this.\n> > > > >\n> > > > > (The single stream tests have these too)\n> > > > >\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 &camera : cm.cameras()) {\n> > > > > > + if (camera->streams().size() > 1) {\n> > > > > > + cameraName = camera->id();\n> > > > > > + cameraFound = true;\n> > > > > > + cm.stop();\n> > > > > > + break;\n> > > > > > + }\n> > > > > > + }\n> > > > > > +\n> > > > > > + if (!cameraFound) {\n> > > > > > + cm.stop();\n> > > > > > + return TestSkip;\n> > > > > > + }\n> > > > > > +\n> > > > > > + const gchar *streamDescription = \"queue ! fakesink\";\n> > > > > > + g_autoptr(GError) error = NULL;\n> > > > > > +\n> > > > > > + stream0_ = gst_parse_bin_from_description_full\n> > > > > (streamDescription, TRUE,\n> > > > > > + NULL,\n> > > > > > +\n> > > > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > > > + &error);\n> > > >\n> > > > Oh I let this and the next one go because it goes over, but I think I\n> > > > agree with checkstyle here:\n> > > >\n> > > >\n> > > > stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > > > NULL,\n> > > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > &error);\n> > > >\n> > > >\n> > > > > > + if (!stream0_) {\n> > > > > > + g_printerr(\"Stream0 could not be created (%s)\\n\",\n> > > > > error->message);\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > + g_object_ref_sink(stream0_);\n> > > > > > +\n> > > > > > + stream1_ = gst_parse_bin_from_description_full\n> > > > > (streamDescription, TRUE,\n> > > > > > + NULL,\n> > > > > > +\n> > > > > GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > > > + &error);\n> > > > > > + if (!stream1_) {\n> > > > > > + g_printerr(\"Stream1 could not be created (%s)\\n\",\n> > > > > error->message);\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > + g_object_ref_sink(stream1_);\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\", cameraName.c_str\n> > > > > (), NULL);\n> > > > > > +\n> > > > > > + /* Build the pipeline */\n> > > > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> > > > > > + stream0_, stream1_,\n> > > > > NULL);\n> > > > >\n> > > > > Indentation.\n> > > >\n> > > > checkstyle has this one\n> > > >\n> > > > >\n> > > > > > +\n> > > > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad\n> > > > > (libcameraSrc_, \"src\");\n> > > > > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad\n> > > > > (libcameraSrc_, \"src_%u\");\n> > >\n> > > This looks bad.\n> >\n> > Get rid of the html indentation...\n> What ? I put in the indentation myself.\n\nI was talking to myself and re-showing the code block without the html\nindentation. With the html indentation it's very nice to read:\n\n+ g_autoptr(GstPad) src_pad = gst_element_get_static_pad\n(libcameraSrc_, \"src\");\n+ g_autoptr(GstPad) request_pad = gst_element_get_request_pad\n(libcameraSrc_, \"src_%u\");\n\nSee, it's even missing the + to show that it's adding new lines!\n\nThis is why html email is the best :)\n\n(I haven't even showed the extra tab that the html email added before >)\n\n> >\n> > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, \"src\");\n> > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, \"src_%u\");\n> >\n> > I think it's fine. If you're concerned about the line length you can\n> > indent it.\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> >\n> > Either way is fine.\n\nAnyway, either way is fine, and I already merged it. Not that big of a\ndeal.\n\n\nPaul\n\n> >\n> > >\n> > > > > > +\n> > > > > > + {\n> > > > > > + g_autoptr(GstPad) queue0_sink_pad =\n> > > > > gst_element_get_static_pad(stream0_, \"sink\");\n> > > > > > + g_autoptr(GstPad) queue1_sink_pad =\n> > > > > gst_element_get_static_pad(stream1_, \"sink\");\n> > >\n> > > even this.\n> >\n> > Same here.\n> >\n> >\n> > Paul\n> >\n> > >\n> > > > > > +\n> > > > > > + if (gst_pad_link(src_pad, queue0_sink_pad) !=\n> > > > > GST_PAD_LINK_OK\n> > > > > > + || gst_pad_link(request_pad,\n> > > > > queue1_sink_pad) != GST_PAD_LINK_OK) {\n> > > > >\n> > > > > Indentation.\n> > > >\n> > > >\n> > > > if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK ||\n> > > > gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {\n> > > >\n> > > > >\n> > > > > > + g_printerr(\"Pads could not be linked.\\n\");\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > + }\n> > > > > > +\n> > > > > > + if (startPipeline() != TestPass)\n> > > > > > + return TestFail;\n> > > > > > +\n> > > > > > + if (processEvent() != TestPass)\n> > > > > > + return TestFail;\n> > > > > > +\n> > > > > > + return TestPass;\n> > > > > > + }\n> > > > > > +\n> > > > > > + void cleanup() override\n> > > > > > + {\n> > > > > > + g_clear_object(&stream0_);\n> > > > > > + g_clear_object(&stream1_);\n> > > > > > + }\n> > > > > > +\n> > > > > > +private:\n> > > > > > + std::string cameraName;\n> > > > >\n> > > > > s/cameraName/cameraName_/\n> > > > >\n> > > > > > + GstElement *stream0_;\n> > > > > > + GstElement *stream1_;\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> > > > > It would be nice if the second elements lined up.\n> > > > >\n> > > > >\n> > > > > Otherwise, looks good.\n> > > > >\n> > > > >\n> > > > > Can I add a R-b then?\n> > > >\n> > > > Not unless I give you one. I'll check in a new ersion.\n> > > >\n> > > >\n> > > > Paul\n> > > >\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 28A53BF01C\n\tfor ;\n\tFri, 24 Sep 2021 09:34:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C2AE6918E;\n\tFri, 24 Sep 2021 11:34:48 +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 4ADB869186\n\tfor ;\n\tFri, 24 Sep 2021 11:34:46 +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 67B5645E;\n\tFri, 24 Sep 2021 11:34:44 +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=\"qK0XX0Ok\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632476085;\n\tbh=9AQG8Gn8LR13YSzP1jXbIllscc6iTbQCRl3018Kt4FM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qK0XX0OkKW1G44im6Q26kIIVvMUR75LI6nBQGa2L/ORsZR/A7e3APSa32Cuu6gbW1\n\toPY/ZwN0e4XrQspwDHZU9Q2w2PlV+P4k9zRhNYQH0/6160S3tSjHkBI5lscz0TO2Jh\n\tAR14sKDFc3fkUEbmY809onjxN6sJepDfjI5S49mM=","Date":"Fri, 24 Sep 2021 18:34:38 +0900","From":"paul.elder@ideasonboard.com","To":"Vedant Paranjape ","Message-ID":"<20210924093438.GZ4382@pyrite.rasen.tech>","References":"<20210923145637.1685366-1-vedantparanjape160201@gmail.com>\n\t<20210924060851.GU4382@pyrite.rasen.tech>\n\t\n\t<20210924070459.GV4382@pyrite.rasen.tech>\n\t\n\t<20210924084125.GW4382@pyrite.rasen.tech>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3] 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 ,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]