| Message ID | 20210923145637.1685366-1-vedantparanjape160201@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Le jeudi 23 septembre 2021 à 20:26 +0530, Vedant Paranjape a écrit : > This patch adds a test to test if multi stream using libcamera's > gstreamer element works. > > Test will run only on devices that support multistream output, i.e., > devices that use IPU3 and raspberrypi pipeline. This was tested on > a Raspberry Pi 4B+. > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Ack. > --- > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > test/gstreamer/meson.build | 1 + > 2 files changed, 129 insertions(+) > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp > new file mode 100644 > index 000000000000..6f63f1e748e2 > --- /dev/null > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > @@ -0,0 +1,128 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright (C) 2021, Vedant Paranjape > + * > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > + */ > + > +#include <iostream> > +#include <unistd.h> > + > +#include <libcamera/base/utils.h> > + > +#include <libcamera/libcamera.h> > + > +#include "libcamera/internal/source_paths.h" > + > +#include <gst/gst.h> > + > +#include "gstreamer_test.h" > +#include "test.h" > + > +using namespace std; > + > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > +{ > +public: > + GstreamerMultiStreamTest() > + : GstreamerTest() > + { > + } > + > +protected: > + int init() override > + { > + if (status_ != TestPass) > + return status_; > + > + /* Check if platform support multistream output */ > + libcamera::CameraManager cm; > + cm.start(); > + bool cameraFound = false; > + for (auto &camera : cm.cameras()) { > + if (camera->streams().size() > 1) { > + cameraName = camera->id(); > + cameraFound = true; > + cm.stop(); > + break; > + } > + } > + > + if (!cameraFound) { > + cm.stop(); > + return TestSkip; > + } > + > + const gchar *streamDescription = "queue ! fakesink"; > + g_autoptr(GError) error = NULL; > + > + stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > + NULL, > + GST_PARSE_FLAG_FATAL_ERRORS, > + &error); > + if (!stream0_) { > + g_printerr("Stream0 could not be created (%s)\n", error->message); > + return TestFail; > + } > + g_object_ref_sink(stream0_); > + > + stream1_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > + NULL, > + GST_PARSE_FLAG_FATAL_ERRORS, > + &error); > + if (!stream1_) { > + g_printerr("Stream1 could not be created (%s)\n", error->message); > + return TestFail; > + } > + g_object_ref_sink(stream1_); > + > + if (createPipeline() != TestPass) > + return TestFail; > + > + return TestPass; > + } > + > + int run() override > + { > + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str(), NULL); > + > + /* Build the pipeline */ > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > + stream0_, stream1_, NULL); > + > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, "src"); > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u"); > + > + { > + g_autoptr(GstPad) queue0_sink_pad = gst_element_get_static_pad(stream0_, "sink"); > + g_autoptr(GstPad) queue1_sink_pad = gst_element_get_static_pad(stream1_, "sink"); > + > + if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK > + || gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { > + g_printerr("Pads could not be linked.\n"); > + return TestFail; > + } > + } > + > + if (startPipeline() != TestPass) > + return TestFail; > + > + if (processEvent() != TestPass) > + return TestFail; > + > + return TestPass; > + } > + > + void cleanup() override > + { > + g_clear_object(&stream0_); > + g_clear_object(&stream1_); > + } > + > +private: > + std::string cameraName; > + GstElement *stream0_; > + GstElement *stream1_; > +}; > + > +TEST_REGISTER(GstreamerMultiStreamTest) > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > index aca53b920365..13652e87d05c 100644 > --- a/test/gstreamer/meson.build > +++ b/test/gstreamer/meson.build > @@ -6,6 +6,7 @@ endif > > gstreamer_tests = [ > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], > ] > gstreamer_dep = dependency('gstreamer-1.0', required: true) >
Hi Vedant, On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote: > This patch adds a test to test if multi stream using libcamera's > gstreamer element works. > > Test will run only on devices that support multistream output, i.e., I think s/output/capture/ might be better. Also s/i.e./e.g/ > devices that use IPU3 and raspberrypi pipeline. This was tested on > a Raspberry Pi 4B+. > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > --- > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > test/gstreamer/meson.build | 1 + > 2 files changed, 129 insertions(+) > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp > new file mode 100644 > index 000000000000..6f63f1e748e2 > --- /dev/null > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > @@ -0,0 +1,128 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright (C) 2021, Vedant Paranjape > + * > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > + */ > + > +#include <iostream> > +#include <unistd.h> > + > +#include <libcamera/base/utils.h> You don't use anything from this. > + > +#include <libcamera/libcamera.h> > + > +#include "libcamera/internal/source_paths.h" Nor this. (The single stream tests have these too) > + > +#include <gst/gst.h> > + > +#include "gstreamer_test.h" > +#include "test.h" > + > +using namespace std; > + > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > +{ > +public: > + GstreamerMultiStreamTest() > + : GstreamerTest() > + { > + } > + > +protected: > + int init() override > + { > + if (status_ != TestPass) > + return status_; > + > + /* Check if platform support multistream output */ > + libcamera::CameraManager cm; > + cm.start(); > + bool cameraFound = false; > + for (auto &camera : cm.cameras()) { > + if (camera->streams().size() > 1) { > + cameraName = camera->id(); > + cameraFound = true; > + cm.stop(); > + break; > + } > + } > + > + if (!cameraFound) { > + cm.stop(); > + return TestSkip; > + } > + > + const gchar *streamDescription = "queue ! fakesink"; > + g_autoptr(GError) error = NULL; > + > + stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > + NULL, > + GST_PARSE_FLAG_FATAL_ERRORS, > + &error); > + if (!stream0_) { > + g_printerr("Stream0 could not be created (%s)\n", error->message); > + return TestFail; > + } > + g_object_ref_sink(stream0_); > + > + stream1_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > + NULL, > + GST_PARSE_FLAG_FATAL_ERRORS, > + &error); > + if (!stream1_) { > + g_printerr("Stream1 could not be created (%s)\n", error->message); > + return TestFail; > + } > + g_object_ref_sink(stream1_); > + > + if (createPipeline() != TestPass) > + return TestFail; > + > + return TestPass; > + } > + > + int run() override > + { > + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str(), NULL); > + > + /* Build the pipeline */ > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > + stream0_, stream1_, NULL); Indentation. > + > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, "src"); > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u"); > + > + { > + g_autoptr(GstPad) queue0_sink_pad = gst_element_get_static_pad(stream0_, "sink"); > + g_autoptr(GstPad) queue1_sink_pad = gst_element_get_static_pad(stream1_, "sink"); > + > + if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK > + || gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { Indentation. > + g_printerr("Pads could not be linked.\n"); > + return TestFail; > + } > + } > + > + if (startPipeline() != TestPass) > + return TestFail; > + > + if (processEvent() != TestPass) > + return TestFail; > + > + return TestPass; > + } > + > + void cleanup() override > + { > + g_clear_object(&stream0_); > + g_clear_object(&stream1_); > + } > + > +private: > + std::string cameraName; s/cameraName/cameraName_/ > + GstElement *stream0_; > + GstElement *stream1_; > +}; > + > +TEST_REGISTER(GstreamerMultiStreamTest) > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > index aca53b920365..13652e87d05c 100644 > --- a/test/gstreamer/meson.build > +++ b/test/gstreamer/meson.build > @@ -6,6 +6,7 @@ endif > > gstreamer_tests = [ > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], It would be nice if the second elements lined up. Otherwise, looks good. Paul > ] > gstreamer_dep = dependency('gstreamer-1.0', required: true) > > -- > 2.25.1 >
Hi Paul, Can you also tell me the expected indentation ? checkstyle suggestions didn't adhere with the 80 char width thing On Fri, 24 Sep, 2021, 11:39 , <paul.elder@ideasonboard.com> wrote: > Hi Vedant, > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote: > > This patch adds a test to test if multi stream using libcamera's > > gstreamer element works. > > > > Test will run only on devices that support multistream output, i.e., > > I think s/output/capture/ might be better. > > Also s/i.e./e.g/ > > > devices that use IPU3 and raspberrypi pipeline. This was tested on > > a Raspberry Pi 4B+. > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > --- > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > > test/gstreamer/meson.build | 1 + > > 2 files changed, 129 insertions(+) > > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp > b/test/gstreamer/gstreamer_multi_stream_test.cpp > > new file mode 100644 > > index 000000000000..6f63f1e748e2 > > --- /dev/null > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > > @@ -0,0 +1,128 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright (C) 2021, Vedant Paranjape > > + * > > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > > + */ > > + > > +#include <iostream> > > +#include <unistd.h> > > + > > +#include <libcamera/base/utils.h> > > You don't use anything from this. > > > + > > +#include <libcamera/libcamera.h> > > + > > +#include "libcamera/internal/source_paths.h" > > Nor this. > > (The single stream tests have these too) > > > + > > +#include <gst/gst.h> > > + > > +#include "gstreamer_test.h" > > +#include "test.h" > > + > > +using namespace std; > > + > > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > > +{ > > +public: > > + GstreamerMultiStreamTest() > > + : GstreamerTest() > > + { > > + } > > + > > +protected: > > + int init() override > > + { > > + if (status_ != TestPass) > > + return status_; > > + > > + /* Check if platform support multistream output */ > > + libcamera::CameraManager cm; > > + cm.start(); > > + bool cameraFound = false; > > + for (auto &camera : cm.cameras()) { > > + if (camera->streams().size() > 1) { > > + cameraName = camera->id(); > > + cameraFound = true; > > + cm.stop(); > > + break; > > + } > > + } > > + > > + if (!cameraFound) { > > + cm.stop(); > > + return TestSkip; > > + } > > + > > + const gchar *streamDescription = "queue ! fakesink"; > > + g_autoptr(GError) error = NULL; > > + > > + stream0_ = > gst_parse_bin_from_description_full(streamDescription, TRUE, > > + NULL, > > + > GST_PARSE_FLAG_FATAL_ERRORS, > > + &error); > > + if (!stream0_) { > > + g_printerr("Stream0 could not be created (%s)\n", > error->message); > > + return TestFail; > > + } > > + g_object_ref_sink(stream0_); > > + > > + stream1_ = > gst_parse_bin_from_description_full(streamDescription, TRUE, > > + NULL, > > + > GST_PARSE_FLAG_FATAL_ERRORS, > > + &error); > > + if (!stream1_) { > > + g_printerr("Stream1 could not be created (%s)\n", > error->message); > > + return TestFail; > > + } > > + g_object_ref_sink(stream1_); > > + > > + if (createPipeline() != TestPass) > > + return TestFail; > > + > > + return TestPass; > > + } > > + > > + int run() override > > + { > > + g_object_set(libcameraSrc_, "camera-name", > cameraName.c_str(), NULL); > > + > > + /* Build the pipeline */ > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > > + stream0_, > stream1_, NULL); > > Indentation. > > > + > > + g_autoptr(GstPad) src_pad = > gst_element_get_static_pad(libcameraSrc_, "src"); > > + g_autoptr(GstPad) request_pad = > gst_element_get_request_pad(libcameraSrc_, "src_%u"); > > + > > + { > > + g_autoptr(GstPad) queue0_sink_pad = > gst_element_get_static_pad(stream0_, "sink"); > > + g_autoptr(GstPad) queue1_sink_pad = > gst_element_get_static_pad(stream1_, "sink"); > > + > > + if (gst_pad_link(src_pad, queue0_sink_pad) != > GST_PAD_LINK_OK > > + || gst_pad_link(request_pad, > queue1_sink_pad) != GST_PAD_LINK_OK) { > > Indentation. > > > + g_printerr("Pads could not be linked.\n"); > > + return TestFail; > > + } > > + } > > + > > + if (startPipeline() != TestPass) > > + return TestFail; > > + > > + if (processEvent() != TestPass) > > + return TestFail; > > + > > + return TestPass; > > + } > > + > > + void cleanup() override > > + { > > + g_clear_object(&stream0_); > > + g_clear_object(&stream1_); > > + } > > + > > +private: > > + std::string cameraName; > > s/cameraName/cameraName_/ > > > + GstElement *stream0_; > > + GstElement *stream1_; > > +}; > > + > > +TEST_REGISTER(GstreamerMultiStreamTest) > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > > index aca53b920365..13652e87d05c 100644 > > --- a/test/gstreamer/meson.build > > +++ b/test/gstreamer/meson.build > > @@ -6,6 +6,7 @@ endif > > > > gstreamer_tests = [ > > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], > > It would be nice if the second elements lined up. > > > Otherwise, looks good. > Can I add a R-b then? Paul > > > ] > > gstreamer_dep = dependency('gstreamer-1.0', required: true) > > > > -- > > 2.25.1 > > >
On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote: > Hi Paul, > Can you also tell me the expected indentation ? > > checkstyle suggestions didn't adhere with the 80 char width thing 80 char can be broken (up to 120) as long as it increases readability. > > > > > On Fri, 24 Sep, 2021, 11:39 , <paul.elder@ideasonboard.com> wrote: > > Hi Vedant, > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote: > > This patch adds a test to test if multi stream using libcamera's > > gstreamer element works. > > > > Test will run only on devices that support multistream output, i.e., > > I think s/output/capture/ might be better. > > Also s/i.e./e.g/ > > > devices that use IPU3 and raspberrypi pipeline. This was tested on > > a Raspberry Pi 4B+. > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > --- > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > > test/gstreamer/meson.build | 1 + > > 2 files changed, 129 insertions(+) > > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/ > gstreamer/gstreamer_multi_stream_test.cpp > > new file mode 100644 > > index 000000000000..6f63f1e748e2 > > --- /dev/null > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > > @@ -0,0 +1,128 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright (C) 2021, Vedant Paranjape > > + * > > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > > + */ > > + > > +#include <iostream> > > +#include <unistd.h> > > + > > +#include <libcamera/base/utils.h> > > You don't use anything from this. > > > + > > +#include <libcamera/libcamera.h> > > + > > +#include "libcamera/internal/source_paths.h" > > Nor this. > > (The single stream tests have these too) > > > + > > +#include <gst/gst.h> > > + > > +#include "gstreamer_test.h" > > +#include "test.h" > > + > > +using namespace std; > > + > > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > > +{ > > +public: > > + GstreamerMultiStreamTest() > > + : GstreamerTest() > > + { > > + } > > + > > +protected: > > + int init() override > > + { > > + if (status_ != TestPass) > > + return status_; > > + > > + /* Check if platform support multistream output */ > > + libcamera::CameraManager cm; > > + cm.start(); > > + bool cameraFound = false; > > + for (auto &camera : cm.cameras()) { > > + if (camera->streams().size() > 1) { > > + cameraName = camera->id(); > > + cameraFound = true; > > + cm.stop(); > > + break; > > + } > > + } > > + > > + if (!cameraFound) { > > + cm.stop(); > > + return TestSkip; > > + } > > + > > + const gchar *streamDescription = "queue ! fakesink"; > > + g_autoptr(GError) error = NULL; > > + > > + stream0_ = gst_parse_bin_from_description_full > (streamDescription, TRUE, > > + NULL, > > + > GST_PARSE_FLAG_FATAL_ERRORS, > > + &error); Oh I let this and the next one go because it goes over, but I think I agree with checkstyle here: stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, NULL, GST_PARSE_FLAG_FATAL_ERRORS, &error); > > + if (!stream0_) { > > + g_printerr("Stream0 could not be created (%s)\n", > error->message); > > + return TestFail; > > + } > > + g_object_ref_sink(stream0_); > > + > > + stream1_ = gst_parse_bin_from_description_full > (streamDescription, TRUE, > > + NULL, > > + > GST_PARSE_FLAG_FATAL_ERRORS, > > + &error); > > + if (!stream1_) { > > + g_printerr("Stream1 could not be created (%s)\n", > error->message); > > + return TestFail; > > + } > > + g_object_ref_sink(stream1_); > > + > > + if (createPipeline() != TestPass) > > + return TestFail; > > + > > + return TestPass; > > + } > > + > > + int run() override > > + { > > + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str > (), NULL); > > + > > + /* Build the pipeline */ > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > > + stream0_, stream1_, > NULL); > > Indentation. checkstyle has this one > > > + > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad > (libcameraSrc_, "src"); > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad > (libcameraSrc_, "src_%u"); > > + > > + { > > + g_autoptr(GstPad) queue0_sink_pad = > gst_element_get_static_pad(stream0_, "sink"); > > + g_autoptr(GstPad) queue1_sink_pad = > gst_element_get_static_pad(stream1_, "sink"); > > + > > + if (gst_pad_link(src_pad, queue0_sink_pad) != > GST_PAD_LINK_OK > > + || gst_pad_link(request_pad, > queue1_sink_pad) != GST_PAD_LINK_OK) { > > Indentation. if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK || gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { > > > + g_printerr("Pads could not be linked.\n"); > > + return TestFail; > > + } > > + } > > + > > + if (startPipeline() != TestPass) > > + return TestFail; > > + > > + if (processEvent() != TestPass) > > + return TestFail; > > + > > + return TestPass; > > + } > > + > > + void cleanup() override > > + { > > + g_clear_object(&stream0_); > > + g_clear_object(&stream1_); > > + } > > + > > +private: > > + std::string cameraName; > > s/cameraName/cameraName_/ > > > + GstElement *stream0_; > > + GstElement *stream1_; > > +}; > > + > > +TEST_REGISTER(GstreamerMultiStreamTest) > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > > index aca53b920365..13652e87d05c 100644 > > --- a/test/gstreamer/meson.build > > +++ b/test/gstreamer/meson.build > > @@ -6,6 +6,7 @@ endif > > > > gstreamer_tests = [ > > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], > > It would be nice if the second elements lined up. > > > Otherwise, looks good. > > > Can I add a R-b then? Not unless I give you one. I'll check in a new ersion. Paul > > > ] > > gstreamer_dep = dependency('gstreamer-1.0', required: true) > > > > -- > > 2.25.1 > > >
On Fri, Sep 24, 2021 at 12:35 PM <paul.elder@ideasonboard.com> wrote: > > On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote: > > Hi Paul, > > Can you also tell me the expected indentation ? > > > > checkstyle suggestions didn't adhere with the 80 char width thing > > 80 char can be broken (up to 120) as long as it increases readability. > > > > > > > > > > > On Fri, 24 Sep, 2021, 11:39 , <paul.elder@ideasonboard.com> wrote: > > > > Hi Vedant, > > > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote: > > > This patch adds a test to test if multi stream using libcamera's > > > gstreamer element works. > > > > > > Test will run only on devices that support multistream output, i.e., > > > > I think s/output/capture/ might be better. > > > > Also s/i.e./e.g/ > > > > > devices that use IPU3 and raspberrypi pipeline. This was tested on > > > a Raspberry Pi 4B+. > > > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > --- > > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > > > test/gstreamer/meson.build | 1 + > > > 2 files changed, 129 insertions(+) > > > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > > > > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/ > > gstreamer/gstreamer_multi_stream_test.cpp > > > new file mode 100644 > > > index 000000000000..6f63f1e748e2 > > > --- /dev/null > > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > > > @@ -0,0 +1,128 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > > +/* > > > + * Copyright (C) 2021, Vedant Paranjape > > > + * > > > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > > > + */ > > > + > > > +#include <iostream> > > > +#include <unistd.h> > > > + > > > +#include <libcamera/base/utils.h> > > > > You don't use anything from this. > > > > > + > > > +#include <libcamera/libcamera.h> > > > + > > > +#include "libcamera/internal/source_paths.h" > > > > Nor this. > > > > (The single stream tests have these too) > > > > > + > > > +#include <gst/gst.h> > > > + > > > +#include "gstreamer_test.h" > > > +#include "test.h" > > > + > > > +using namespace std; > > > + > > > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > > > +{ > > > +public: > > > + GstreamerMultiStreamTest() > > > + : GstreamerTest() > > > + { > > > + } > > > + > > > +protected: > > > + int init() override > > > + { > > > + if (status_ != TestPass) > > > + return status_; > > > + > > > + /* Check if platform support multistream output */ > > > + libcamera::CameraManager cm; > > > + cm.start(); > > > + bool cameraFound = false; > > > + for (auto &camera : cm.cameras()) { > > > + if (camera->streams().size() > 1) { > > > + cameraName = camera->id(); > > > + cameraFound = true; > > > + cm.stop(); > > > + break; > > > + } > > > + } > > > + > > > + if (!cameraFound) { > > > + cm.stop(); > > > + return TestSkip; > > > + } > > > + > > > + const gchar *streamDescription = "queue ! fakesink"; > > > + g_autoptr(GError) error = NULL; > > > + > > > + stream0_ = gst_parse_bin_from_description_full > > (streamDescription, TRUE, > > > + NULL, > > > + > > GST_PARSE_FLAG_FATAL_ERRORS, > > > + &error); > > Oh I let this and the next one go because it goes over, but I think I > agree with checkstyle here: > > > stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > NULL, > GST_PARSE_FLAG_FATAL_ERRORS, > &error); > > > > > + if (!stream0_) { > > > + g_printerr("Stream0 could not be created (%s)\n", > > error->message); > > > + return TestFail; > > > + } > > > + g_object_ref_sink(stream0_); > > > + > > > + stream1_ = gst_parse_bin_from_description_full > > (streamDescription, TRUE, > > > + NULL, > > > + > > GST_PARSE_FLAG_FATAL_ERRORS, > > > + &error); > > > + if (!stream1_) { > > > + g_printerr("Stream1 could not be created (%s)\n", > > error->message); > > > + return TestFail; > > > + } > > > + g_object_ref_sink(stream1_); > > > + > > > + if (createPipeline() != TestPass) > > > + return TestFail; > > > + > > > + return TestPass; > > > + } > > > + > > > + int run() override > > > + { > > > + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str > > (), NULL); > > > + > > > + /* Build the pipeline */ > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > > > + stream0_, stream1_, > > NULL); > > > > Indentation. > > checkstyle has this one > > > > > > + > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad > > (libcameraSrc_, "src"); > > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad > > (libcameraSrc_, "src_%u"); This looks bad. > > > + > > > + { > > > + g_autoptr(GstPad) queue0_sink_pad = > > gst_element_get_static_pad(stream0_, "sink"); > > > + g_autoptr(GstPad) queue1_sink_pad = > > gst_element_get_static_pad(stream1_, "sink"); even this. > > > + > > > + if (gst_pad_link(src_pad, queue0_sink_pad) != > > GST_PAD_LINK_OK > > > + || gst_pad_link(request_pad, > > queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > Indentation. > > > if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK || > gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > > > + g_printerr("Pads could not be linked.\n"); > > > + return TestFail; > > > + } > > > + } > > > + > > > + if (startPipeline() != TestPass) > > > + return TestFail; > > > + > > > + if (processEvent() != TestPass) > > > + return TestFail; > > > + > > > + return TestPass; > > > + } > > > + > > > + void cleanup() override > > > + { > > > + g_clear_object(&stream0_); > > > + g_clear_object(&stream1_); > > > + } > > > + > > > +private: > > > + std::string cameraName; > > > > s/cameraName/cameraName_/ > > > > > + GstElement *stream0_; > > > + GstElement *stream1_; > > > +}; > > > + > > > +TEST_REGISTER(GstreamerMultiStreamTest) > > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > > > index aca53b920365..13652e87d05c 100644 > > > --- a/test/gstreamer/meson.build > > > +++ b/test/gstreamer/meson.build > > > @@ -6,6 +6,7 @@ endif > > > > > > gstreamer_tests = [ > > > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > > > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], > > > > It would be nice if the second elements lined up. > > > > > > Otherwise, looks good. > > > > > > Can I add a R-b then? > > Not unless I give you one. I'll check in a new ersion. > > > Paul > > > > > > ] > > > gstreamer_dep = dependency('gstreamer-1.0', required: true) > > > > > > -- > > > 2.25.1 > > > > >
On Fri, Sep 24, 2021 at 02:02:07PM +0530, Vedant Paranjape wrote: > On Fri, Sep 24, 2021 at 12:35 PM <paul.elder@ideasonboard.com> wrote: > > > > On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote: > > > Hi Paul, > > > Can you also tell me the expected indentation ? > > > > > > checkstyle suggestions didn't adhere with the 80 char width thing > > > > 80 char can be broken (up to 120) as long as it increases readability. > > > > > > > > > > > > > > > > > On Fri, 24 Sep, 2021, 11:39 , <paul.elder@ideasonboard.com> wrote: > > > > > > Hi Vedant, > > > > > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote: > > > > This patch adds a test to test if multi stream using libcamera's > > > > gstreamer element works. > > > > > > > > Test will run only on devices that support multistream output, i.e., > > > > > > I think s/output/capture/ might be better. > > > > > > Also s/i.e./e.g/ > > > > > > > devices that use IPU3 and raspberrypi pipeline. This was tested on > > > > a Raspberry Pi 4B+. > > > > > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > --- > > > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > > > > test/gstreamer/meson.build | 1 + > > > > 2 files changed, 129 insertions(+) > > > > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > > > > > > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/ > > > gstreamer/gstreamer_multi_stream_test.cpp > > > > new file mode 100644 > > > > index 000000000000..6f63f1e748e2 > > > > --- /dev/null > > > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > > > > @@ -0,0 +1,128 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > > > +/* > > > > + * Copyright (C) 2021, Vedant Paranjape > > > > + * > > > > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > > > > + */ > > > > + > > > > +#include <iostream> > > > > +#include <unistd.h> > > > > + > > > > +#include <libcamera/base/utils.h> > > > > > > You don't use anything from this. > > > > > > > + > > > > +#include <libcamera/libcamera.h> > > > > + > > > > +#include "libcamera/internal/source_paths.h" > > > > > > Nor this. > > > > > > (The single stream tests have these too) > > > > > > > + > > > > +#include <gst/gst.h> > > > > + > > > > +#include "gstreamer_test.h" > > > > +#include "test.h" > > > > + > > > > +using namespace std; > > > > + > > > > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > > > > +{ > > > > +public: > > > > + GstreamerMultiStreamTest() > > > > + : GstreamerTest() > > > > + { > > > > + } > > > > + > > > > +protected: > > > > + int init() override > > > > + { > > > > + if (status_ != TestPass) > > > > + return status_; > > > > + > > > > + /* Check if platform support multistream output */ > > > > + libcamera::CameraManager cm; > > > > + cm.start(); > > > > + bool cameraFound = false; > > > > + for (auto &camera : cm.cameras()) { > > > > + if (camera->streams().size() > 1) { > > > > + cameraName = camera->id(); > > > > + cameraFound = true; > > > > + cm.stop(); > > > > + break; > > > > + } > > > > + } > > > > + > > > > + if (!cameraFound) { > > > > + cm.stop(); > > > > + return TestSkip; > > > > + } > > > > + > > > > + const gchar *streamDescription = "queue ! fakesink"; > > > > + g_autoptr(GError) error = NULL; > > > > + > > > > + stream0_ = gst_parse_bin_from_description_full > > > (streamDescription, TRUE, > > > > + NULL, > > > > + > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > > + &error); > > > > Oh I let this and the next one go because it goes over, but I think I > > agree with checkstyle here: > > > > > > stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > > NULL, > > GST_PARSE_FLAG_FATAL_ERRORS, > > &error); > > > > > > > > + if (!stream0_) { > > > > + g_printerr("Stream0 could not be created (%s)\n", > > > error->message); > > > > + return TestFail; > > > > + } > > > > + g_object_ref_sink(stream0_); > > > > + > > > > + stream1_ = gst_parse_bin_from_description_full > > > (streamDescription, TRUE, > > > > + NULL, > > > > + > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > > + &error); > > > > + if (!stream1_) { > > > > + g_printerr("Stream1 could not be created (%s)\n", > > > error->message); > > > > + return TestFail; > > > > + } > > > > + g_object_ref_sink(stream1_); > > > > + > > > > + if (createPipeline() != TestPass) > > > > + return TestFail; > > > > + > > > > + return TestPass; > > > > + } > > > > + > > > > + int run() override > > > > + { > > > > + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str > > > (), NULL); > > > > + > > > > + /* Build the pipeline */ > > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > > > > + stream0_, stream1_, > > > NULL); > > > > > > Indentation. > > > > checkstyle has this one > > > > > > > > > + > > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad > > > (libcameraSrc_, "src"); > > > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad > > > (libcameraSrc_, "src_%u"); > > This looks bad. Get rid of the html indentation... + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, "src"); + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u"); I think it's fine. If you're concerned about the line length you can indent it. + g_autoptr(GstPad) src_pad = + gst_element_get_static_pad(libcameraSrc_, "src"); + g_autoptr(GstPad) request_pad = + gst_element_get_request_pad(libcameraSrc_, "src_%u"); Either way is fine. > > > > > + > > > > + { > > > > + g_autoptr(GstPad) queue0_sink_pad = > > > gst_element_get_static_pad(stream0_, "sink"); > > > > + g_autoptr(GstPad) queue1_sink_pad = > > > gst_element_get_static_pad(stream1_, "sink"); > > even this. Same here. Paul > > > > > + > > > > + if (gst_pad_link(src_pad, queue0_sink_pad) != > > > GST_PAD_LINK_OK > > > > + || gst_pad_link(request_pad, > > > queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > > > Indentation. > > > > > > if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK || > > gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > > > > > > + g_printerr("Pads could not be linked.\n"); > > > > + return TestFail; > > > > + } > > > > + } > > > > + > > > > + if (startPipeline() != TestPass) > > > > + return TestFail; > > > > + > > > > + if (processEvent() != TestPass) > > > > + return TestFail; > > > > + > > > > + return TestPass; > > > > + } > > > > + > > > > + void cleanup() override > > > > + { > > > > + g_clear_object(&stream0_); > > > > + g_clear_object(&stream1_); > > > > + } > > > > + > > > > +private: > > > > + std::string cameraName; > > > > > > s/cameraName/cameraName_/ > > > > > > > + GstElement *stream0_; > > > > + GstElement *stream1_; > > > > +}; > > > > + > > > > +TEST_REGISTER(GstreamerMultiStreamTest) > > > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > > > > index aca53b920365..13652e87d05c 100644 > > > > --- a/test/gstreamer/meson.build > > > > +++ b/test/gstreamer/meson.build > > > > @@ -6,6 +6,7 @@ endif > > > > > > > > gstreamer_tests = [ > > > > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > > > > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], > > > > > > It would be nice if the second elements lined up. > > > > > > > > > Otherwise, looks good. > > > > > > > > > Can I add a R-b then? > > > > Not unless I give you one. I'll check in a new ersion. > > > > > > Paul > > > > > > > > > ] > > > > gstreamer_dep = dependency('gstreamer-1.0', required: true) > > > > > > > > -- > > > > 2.25.1 > > > > > > >
On Fri, Sep 24, 2021 at 2:11 PM <paul.elder@ideasonboard.com> wrote: > > On Fri, Sep 24, 2021 at 02:02:07PM +0530, Vedant Paranjape wrote: > > On Fri, Sep 24, 2021 at 12:35 PM <paul.elder@ideasonboard.com> wrote: > > > > > > On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote: > > > > Hi Paul, > > > > Can you also tell me the expected indentation ? > > > > > > > > checkstyle suggestions didn't adhere with the 80 char width thing > > > > > > 80 char can be broken (up to 120) as long as it increases readability. > > > > > > > > > > > > > > > > > > > > > > > On Fri, 24 Sep, 2021, 11:39 , <paul.elder@ideasonboard.com> wrote: > > > > > > > > Hi Vedant, > > > > > > > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote: > > > > > This patch adds a test to test if multi stream using libcamera's > > > > > gstreamer element works. > > > > > > > > > > Test will run only on devices that support multistream output, i.e., > > > > > > > > I think s/output/capture/ might be better. > > > > > > > > Also s/i.e./e.g/ > > > > > > > > > devices that use IPU3 and raspberrypi pipeline. This was tested on > > > > > a Raspberry Pi 4B+. > > > > > > > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > > > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > --- > > > > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > > > > > test/gstreamer/meson.build | 1 + > > > > > 2 files changed, 129 insertions(+) > > > > > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > > > > > > > > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/ > > > > gstreamer/gstreamer_multi_stream_test.cpp > > > > > new file mode 100644 > > > > > index 000000000000..6f63f1e748e2 > > > > > --- /dev/null > > > > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > > > > > @@ -0,0 +1,128 @@ > > > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > > > > +/* > > > > > + * Copyright (C) 2021, Vedant Paranjape > > > > > + * > > > > > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > > > > > + */ > > > > > + > > > > > +#include <iostream> > > > > > +#include <unistd.h> > > > > > + > > > > > +#include <libcamera/base/utils.h> > > > > > > > > You don't use anything from this. > > > > > > > > > + > > > > > +#include <libcamera/libcamera.h> > > > > > + > > > > > +#include "libcamera/internal/source_paths.h" > > > > > > > > Nor this. > > > > > > > > (The single stream tests have these too) > > > > > > > > > + > > > > > +#include <gst/gst.h> > > > > > + > > > > > +#include "gstreamer_test.h" > > > > > +#include "test.h" > > > > > + > > > > > +using namespace std; > > > > > + > > > > > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > > > > > +{ > > > > > +public: > > > > > + GstreamerMultiStreamTest() > > > > > + : GstreamerTest() > > > > > + { > > > > > + } > > > > > + > > > > > +protected: > > > > > + int init() override > > > > > + { > > > > > + if (status_ != TestPass) > > > > > + return status_; > > > > > + > > > > > + /* Check if platform support multistream output */ > > > > > + libcamera::CameraManager cm; > > > > > + cm.start(); > > > > > + bool cameraFound = false; > > > > > + for (auto &camera : cm.cameras()) { > > > > > + if (camera->streams().size() > 1) { > > > > > + cameraName = camera->id(); > > > > > + cameraFound = true; > > > > > + cm.stop(); > > > > > + break; > > > > > + } > > > > > + } > > > > > + > > > > > + if (!cameraFound) { > > > > > + cm.stop(); > > > > > + return TestSkip; > > > > > + } > > > > > + > > > > > + const gchar *streamDescription = "queue ! fakesink"; > > > > > + g_autoptr(GError) error = NULL; > > > > > + > > > > > + stream0_ = gst_parse_bin_from_description_full > > > > (streamDescription, TRUE, > > > > > + NULL, > > > > > + > > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > > > + &error); > > > > > > Oh I let this and the next one go because it goes over, but I think I > > > agree with checkstyle here: > > > > > > > > > stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > > > NULL, > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > &error); > > > > > > > > > > > + if (!stream0_) { > > > > > + g_printerr("Stream0 could not be created (%s)\n", > > > > error->message); > > > > > + return TestFail; > > > > > + } > > > > > + g_object_ref_sink(stream0_); > > > > > + > > > > > + stream1_ = gst_parse_bin_from_description_full > > > > (streamDescription, TRUE, > > > > > + NULL, > > > > > + > > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > > > + &error); > > > > > + if (!stream1_) { > > > > > + g_printerr("Stream1 could not be created (%s)\n", > > > > error->message); > > > > > + return TestFail; > > > > > + } > > > > > + g_object_ref_sink(stream1_); > > > > > + > > > > > + if (createPipeline() != TestPass) > > > > > + return TestFail; > > > > > + > > > > > + return TestPass; > > > > > + } > > > > > + > > > > > + int run() override > > > > > + { > > > > > + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str > > > > (), NULL); > > > > > + > > > > > + /* Build the pipeline */ > > > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > > > > > + stream0_, stream1_, > > > > NULL); > > > > > > > > Indentation. > > > > > > checkstyle has this one > > > > > > > > > > > > + > > > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad > > > > (libcameraSrc_, "src"); > > > > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad > > > > (libcameraSrc_, "src_%u"); > > > > This looks bad. > > Get rid of the html indentation... What ? I put in the indentation myself. > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, "src"); > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u"); > > I think it's fine. If you're concerned about the line length you can > indent it. > > + g_autoptr(GstPad) src_pad = > + gst_element_get_static_pad(libcameraSrc_, "src"); > + g_autoptr(GstPad) request_pad = > + gst_element_get_request_pad(libcameraSrc_, "src_%u"); > > Either way is fine. > > > > > > > > + > > > > > + { > > > > > + g_autoptr(GstPad) queue0_sink_pad = > > > > gst_element_get_static_pad(stream0_, "sink"); > > > > > + g_autoptr(GstPad) queue1_sink_pad = > > > > gst_element_get_static_pad(stream1_, "sink"); > > > > even this. > > Same here. > > > Paul > > > > > > > > + > > > > > + if (gst_pad_link(src_pad, queue0_sink_pad) != > > > > GST_PAD_LINK_OK > > > > > + || gst_pad_link(request_pad, > > > > queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > > > > > Indentation. > > > > > > > > > if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK || > > > gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > > > > > > > > > + g_printerr("Pads could not be linked.\n"); > > > > > + return TestFail; > > > > > + } > > > > > + } > > > > > + > > > > > + if (startPipeline() != TestPass) > > > > > + return TestFail; > > > > > + > > > > > + if (processEvent() != TestPass) > > > > > + return TestFail; > > > > > + > > > > > + return TestPass; > > > > > + } > > > > > + > > > > > + void cleanup() override > > > > > + { > > > > > + g_clear_object(&stream0_); > > > > > + g_clear_object(&stream1_); > > > > > + } > > > > > + > > > > > +private: > > > > > + std::string cameraName; > > > > > > > > s/cameraName/cameraName_/ > > > > > > > > > + GstElement *stream0_; > > > > > + GstElement *stream1_; > > > > > +}; > > > > > + > > > > > +TEST_REGISTER(GstreamerMultiStreamTest) > > > > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > > > > > index aca53b920365..13652e87d05c 100644 > > > > > --- a/test/gstreamer/meson.build > > > > > +++ b/test/gstreamer/meson.build > > > > > @@ -6,6 +6,7 @@ endif > > > > > > > > > > gstreamer_tests = [ > > > > > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > > > > > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], > > > > > > > > It would be nice if the second elements lined up. > > > > > > > > > > > > Otherwise, looks good. > > > > > > > > > > > > Can I add a R-b then? > > > > > > Not unless I give you one. I'll check in a new ersion. > > > > > > > > > Paul > > > > > > > > > > > > ] > > > > > gstreamer_dep = dependency('gstreamer-1.0', required: true) > > > > > > > > > > -- > > > > > 2.25.1 > > > > > > > > >
On Fri, Sep 24, 2021 at 02:14:11PM +0530, Vedant Paranjape wrote: > On Fri, Sep 24, 2021 at 2:11 PM <paul.elder@ideasonboard.com> wrote: > > > > On Fri, Sep 24, 2021 at 02:02:07PM +0530, Vedant Paranjape wrote: > > > On Fri, Sep 24, 2021 at 12:35 PM <paul.elder@ideasonboard.com> wrote: > > > > > > > > On Fri, Sep 24, 2021 at 11:42:18AM +0530, Vedant Paranjape wrote: > > > > > Hi Paul, > > > > > Can you also tell me the expected indentation ? > > > > > > > > > > checkstyle suggestions didn't adhere with the 80 char width thing > > > > > > > > 80 char can be broken (up to 120) as long as it increases readability. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, 24 Sep, 2021, 11:39 , <paul.elder@ideasonboard.com> wrote: > > > > > > > > > > Hi Vedant, > > > > > > > > > > On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote: > > > > > > This patch adds a test to test if multi stream using libcamera's > > > > > > gstreamer element works. > > > > > > > > > > > > Test will run only on devices that support multistream output, i.e., > > > > > > > > > > I think s/output/capture/ might be better. > > > > > > > > > > Also s/i.e./e.g/ > > > > > > > > > > > devices that use IPU3 and raspberrypi pipeline. This was tested on > > > > > > a Raspberry Pi 4B+. > > > > > > > > > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > > > > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > > --- > > > > > > .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++ > > > > > > test/gstreamer/meson.build | 1 + > > > > > > 2 files changed, 129 insertions(+) > > > > > > create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp > > > > > > > > > > > > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/ > > > > > gstreamer/gstreamer_multi_stream_test.cpp > > > > > > new file mode 100644 > > > > > > index 000000000000..6f63f1e748e2 > > > > > > --- /dev/null > > > > > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp > > > > > > @@ -0,0 +1,128 @@ > > > > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > > > > > +/* > > > > > > + * Copyright (C) 2021, Vedant Paranjape > > > > > > + * > > > > > > + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test > > > > > > + */ > > > > > > + > > > > > > +#include <iostream> > > > > > > +#include <unistd.h> > > > > > > + > > > > > > +#include <libcamera/base/utils.h> > > > > > > > > > > You don't use anything from this. > > > > > > > > > > > + > > > > > > +#include <libcamera/libcamera.h> > > > > > > + > > > > > > +#include "libcamera/internal/source_paths.h" > > > > > > > > > > Nor this. > > > > > > > > > > (The single stream tests have these too) > > > > > > > > > > > + > > > > > > +#include <gst/gst.h> > > > > > > + > > > > > > +#include "gstreamer_test.h" > > > > > > +#include "test.h" > > > > > > + > > > > > > +using namespace std; > > > > > > + > > > > > > +class GstreamerMultiStreamTest : public GstreamerTest, public Test > > > > > > +{ > > > > > > +public: > > > > > > + GstreamerMultiStreamTest() > > > > > > + : GstreamerTest() > > > > > > + { > > > > > > + } > > > > > > + > > > > > > +protected: > > > > > > + int init() override > > > > > > + { > > > > > > + if (status_ != TestPass) > > > > > > + return status_; > > > > > > + > > > > > > + /* Check if platform support multistream output */ > > > > > > + libcamera::CameraManager cm; > > > > > > + cm.start(); > > > > > > + bool cameraFound = false; > > > > > > + for (auto &camera : cm.cameras()) { > > > > > > + if (camera->streams().size() > 1) { > > > > > > + cameraName = camera->id(); > > > > > > + cameraFound = true; > > > > > > + cm.stop(); > > > > > > + break; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + if (!cameraFound) { > > > > > > + cm.stop(); > > > > > > + return TestSkip; > > > > > > + } > > > > > > + > > > > > > + const gchar *streamDescription = "queue ! fakesink"; > > > > > > + g_autoptr(GError) error = NULL; > > > > > > + > > > > > > + stream0_ = gst_parse_bin_from_description_full > > > > > (streamDescription, TRUE, > > > > > > + NULL, > > > > > > + > > > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > > > > + &error); > > > > > > > > Oh I let this and the next one go because it goes over, but I think I > > > > agree with checkstyle here: > > > > > > > > > > > > stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, > > > > NULL, > > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > > &error); > > > > > > > > > > > > > > + if (!stream0_) { > > > > > > + g_printerr("Stream0 could not be created (%s)\n", > > > > > error->message); > > > > > > + return TestFail; > > > > > > + } > > > > > > + g_object_ref_sink(stream0_); > > > > > > + > > > > > > + stream1_ = gst_parse_bin_from_description_full > > > > > (streamDescription, TRUE, > > > > > > + NULL, > > > > > > + > > > > > GST_PARSE_FLAG_FATAL_ERRORS, > > > > > > + &error); > > > > > > + if (!stream1_) { > > > > > > + g_printerr("Stream1 could not be created (%s)\n", > > > > > error->message); > > > > > > + return TestFail; > > > > > > + } > > > > > > + g_object_ref_sink(stream1_); > > > > > > + > > > > > > + if (createPipeline() != TestPass) > > > > > > + return TestFail; > > > > > > + > > > > > > + return TestPass; > > > > > > + } > > > > > > + > > > > > > + int run() override > > > > > > + { > > > > > > + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str > > > > > (), NULL); > > > > > > + > > > > > > + /* Build the pipeline */ > > > > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, > > > > > > + stream0_, stream1_, > > > > > NULL); > > > > > > > > > > Indentation. > > > > > > > > checkstyle has this one > > > > > > > > > > > > > > > + > > > > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad > > > > > (libcameraSrc_, "src"); > > > > > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad > > > > > (libcameraSrc_, "src_%u"); > > > > > > This looks bad. > > > > Get rid of the html indentation... > What ? I put in the indentation myself. I was talking to myself and re-showing the code block without the html indentation. With the html indentation it's very nice to read: + g_autoptr(GstPad) src_pad = gst_element_get_static_pad (libcameraSrc_, "src"); + g_autoptr(GstPad) request_pad = gst_element_get_request_pad (libcameraSrc_, "src_%u"); See, it's even missing the + to show that it's adding new lines! This is why html email is the best :) (I haven't even showed the extra tab that the html email added before >) > > > > + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, "src"); > > + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u"); > > > > I think it's fine. If you're concerned about the line length you can > > indent it. > > > > + g_autoptr(GstPad) src_pad = > > + gst_element_get_static_pad(libcameraSrc_, "src"); > > + g_autoptr(GstPad) request_pad = > > + gst_element_get_request_pad(libcameraSrc_, "src_%u"); > > > > Either way is fine. Anyway, either way is fine, and I already merged it. Not that big of a deal. Paul > > > > > > > > > > > + > > > > > > + { > > > > > > + g_autoptr(GstPad) queue0_sink_pad = > > > > > gst_element_get_static_pad(stream0_, "sink"); > > > > > > + g_autoptr(GstPad) queue1_sink_pad = > > > > > gst_element_get_static_pad(stream1_, "sink"); > > > > > > even this. > > > > Same here. > > > > > > Paul > > > > > > > > > > > + > > > > > > + if (gst_pad_link(src_pad, queue0_sink_pad) != > > > > > GST_PAD_LINK_OK > > > > > > + || gst_pad_link(request_pad, > > > > > queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > > > > > > > Indentation. > > > > > > > > > > > > if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK || > > > > gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { > > > > > > > > > > > > > > > + g_printerr("Pads could not be linked.\n"); > > > > > > + return TestFail; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + if (startPipeline() != TestPass) > > > > > > + return TestFail; > > > > > > + > > > > > > + if (processEvent() != TestPass) > > > > > > + return TestFail; > > > > > > + > > > > > > + return TestPass; > > > > > > + } > > > > > > + > > > > > > + void cleanup() override > > > > > > + { > > > > > > + g_clear_object(&stream0_); > > > > > > + g_clear_object(&stream1_); > > > > > > + } > > > > > > + > > > > > > +private: > > > > > > + std::string cameraName; > > > > > > > > > > s/cameraName/cameraName_/ > > > > > > > > > > > + GstElement *stream0_; > > > > > > + GstElement *stream1_; > > > > > > +}; > > > > > > + > > > > > > +TEST_REGISTER(GstreamerMultiStreamTest) > > > > > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build > > > > > > index aca53b920365..13652e87d05c 100644 > > > > > > --- a/test/gstreamer/meson.build > > > > > > +++ b/test/gstreamer/meson.build > > > > > > @@ -6,6 +6,7 @@ endif > > > > > > > > > > > > gstreamer_tests = [ > > > > > > ['single_stream_test', 'gstreamer_single_stream_test.cpp'], > > > > > > + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], > > > > > > > > > > It would be nice if the second elements lined up. > > > > > > > > > > > > > > > Otherwise, looks good. > > > > > > > > > > > > > > > Can I add a R-b then? > > > > > > > > Not unless I give you one. I'll check in a new ersion. > > > > > > > > > > > > Paul > > > > > > > > > > > > > > > ] > > > > > > gstreamer_dep = dependency('gstreamer-1.0', required: true) > > > > > > > > > > > > -- > > > > > > 2.25.1 > > > > > > > > > > >
diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp new file mode 100644 index 000000000000..6f63f1e748e2 --- /dev/null +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp @@ -0,0 +1,128 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021, Vedant Paranjape + * + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test + */ + +#include <iostream> +#include <unistd.h> + +#include <libcamera/base/utils.h> + +#include <libcamera/libcamera.h> + +#include "libcamera/internal/source_paths.h" + +#include <gst/gst.h> + +#include "gstreamer_test.h" +#include "test.h" + +using namespace std; + +class GstreamerMultiStreamTest : public GstreamerTest, public Test +{ +public: + GstreamerMultiStreamTest() + : GstreamerTest() + { + } + +protected: + int init() override + { + if (status_ != TestPass) + return status_; + + /* Check if platform support multistream output */ + libcamera::CameraManager cm; + cm.start(); + bool cameraFound = false; + for (auto &camera : cm.cameras()) { + if (camera->streams().size() > 1) { + cameraName = camera->id(); + cameraFound = true; + cm.stop(); + break; + } + } + + if (!cameraFound) { + cm.stop(); + return TestSkip; + } + + const gchar *streamDescription = "queue ! fakesink"; + g_autoptr(GError) error = NULL; + + stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE, + NULL, + GST_PARSE_FLAG_FATAL_ERRORS, + &error); + if (!stream0_) { + g_printerr("Stream0 could not be created (%s)\n", error->message); + return TestFail; + } + g_object_ref_sink(stream0_); + + stream1_ = gst_parse_bin_from_description_full(streamDescription, TRUE, + NULL, + GST_PARSE_FLAG_FATAL_ERRORS, + &error); + if (!stream1_) { + g_printerr("Stream1 could not be created (%s)\n", error->message); + return TestFail; + } + g_object_ref_sink(stream1_); + + if (createPipeline() != TestPass) + return TestFail; + + return TestPass; + } + + int run() override + { + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str(), NULL); + + /* Build the pipeline */ + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, + stream0_, stream1_, NULL); + + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, "src"); + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u"); + + { + g_autoptr(GstPad) queue0_sink_pad = gst_element_get_static_pad(stream0_, "sink"); + g_autoptr(GstPad) queue1_sink_pad = gst_element_get_static_pad(stream1_, "sink"); + + if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK + || gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) { + g_printerr("Pads could not be linked.\n"); + return TestFail; + } + } + + if (startPipeline() != TestPass) + return TestFail; + + if (processEvent() != TestPass) + return TestFail; + + return TestPass; + } + + void cleanup() override + { + g_clear_object(&stream0_); + g_clear_object(&stream1_); + } + +private: + std::string cameraName; + GstElement *stream0_; + GstElement *stream1_; +}; + +TEST_REGISTER(GstreamerMultiStreamTest) diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build index aca53b920365..13652e87d05c 100644 --- a/test/gstreamer/meson.build +++ b/test/gstreamer/meson.build @@ -6,6 +6,7 @@ endif gstreamer_tests = [ ['single_stream_test', 'gstreamer_single_stream_test.cpp'], + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'], ] gstreamer_dep = dependency('gstreamer-1.0', required: true)