[libcamera-devel] [PATCH v2] test: gstreamer: Add a test for gstreamer multi stream
Vedant Paranjape
vedantparanjape160201 at gmail.com
Tue Sep 14 19:46:26 CEST 2021
Hi Nicolas,
On Tue, 14 Sep, 2021, 23:08 Nicolas Dufresne, <nicolas at ndufresne.ca> wrote:
> Le vendredi 10 septembre 2021 à 23:41 +0530, Vedant Paranjape a écrit :
> > This patch adds a test to test if multi stream using libcamera's
> > gstreamer element works.
> >
> > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > ---
> > .../gstreamer/gstreamer_multi_stream_test.cpp | 138 ++++++++++++++++++
> > test/gstreamer/meson.build | 1 +
> > 2 files changed, 139 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..e5c909c85da2
> > --- /dev/null
> > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp
> > @@ -0,0 +1,138 @@
> > +/* 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 &i : cm.cameras()) {
> > + if (i->streams().size() > 1) {
> > + cameraName = i->id();
> > + cameraFound = true;
> > + cm.stop();
> > + break;
> > + }
> > + }
> > +
> > + if (!cameraFound) {
> > + cm.stop();
> > + return TestSkip;
> > + }
> > +
> > + g_autoptr(GstElement) convert0 =
> gst_element_factory_make("videoconvert", "convert0");
> > + g_autoptr(GstElement) convert1 =
> gst_element_factory_make("videoconvert", "convert1");
> > + g_autoptr(GstElement) sink0 =
> gst_element_factory_make("fakesink", "sink0");
> > + g_autoptr(GstElement) sink1 =
> gst_element_factory_make("fakesink", "sink1");
> > + g_autoptr(GstElement) queue0 =
> gst_element_factory_make("queue", "queue0");
> > + g_autoptr(GstElement) queue1 =
> gst_element_factory_make("queue", "queue1");
> > + g_object_ref_sink(convert0);
> > + g_object_ref_sink(convert1);
> > + g_object_ref_sink(sink0);
> > + g_object_ref_sink(sink1);
> > + g_object_ref_sink(queue0);
> > + g_object_ref_sink(queue1);
>
> ref_sink should be inline or next to their allocation.
>
Okay, will change.
> > +
> > + if (!convert0 || !convert1 || !sink0 || !sink1 || !queue0
> || !queue1) {
> > + g_printerr("Not all elements could be created.
> %p.%p.%p.%p.%p.%p\n",
> > + convert0, convert1, sink0, sink1,
> queue0, queue1);
> > +
> > + return TestFail;
> > + }
> > +
> > + convert0_ = reinterpret_cast<GstElement
> *>(g_steal_pointer(&convert0));
> > + convert1_ = reinterpret_cast<GstElement
> *>(g_steal_pointer(&convert1));
> > + sink0_ = reinterpret_cast<GstElement
> *>(g_steal_pointer(&sink0));
> > + sink1_ = reinterpret_cast<GstElement
> *>(g_steal_pointer(&sink1));
> > + queue0_ = reinterpret_cast<GstElement
> *>(g_steal_pointer(&queue0));
> > + queue1_ = reinterpret_cast<GstElement
> *>(g_steal_pointer(&queue1));
>
> What is the point of using autoptr here, you will clean them up in your
> destructure, just store them directly in the class. In general, if you
> feel the
> code is ugly, that's likely before you are doing something that is not
> needed.
>
Okay, makes sense.
> +
> > + 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_,
> queue0_, queue1_,
> > +
> convert0_, convert1_, sink0_, sink1_, NULL);
> > + if (gst_element_link_many(queue0_, convert0_, sink0_,
> NULL) != TRUE
> > + || gst_element_link_many(queue1_, convert1_,
> sink1_, NULL) != TRUE) {
> > + g_printerr("Elements could not be linked.\n");
> > + return TestFail;
> > + }
>
> I think we'd gain in readability of using gst_parse_launch_full(). You can
> even
> link requested pad with that. It's also very ackward here, since you have a
> run() function which cannot be run twice.
>
I agree with this, I'll take a look at the function.
> +
> > + 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");
> > + GstPad *queue0_sink_pad =
> gst_element_get_static_pad(queue0_, "sink");
> > + GstPad *queue1_sink_pad =
> gst_element_get_static_pad(queue1_, "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) {
> > + if (queue0_sink_pad)
> > + gst_object_unref(queue0_sink_pad);
> > + if (queue1_sink_pad)
> > + gst_object_unref(queue1_sink_pad);
> > + g_printerr("Pads could not be linked.\n");
> > + return TestFail;
> > + }
> > + gst_object_unref(queue0_sink_pad);
> > + gst_object_unref(queue1_sink_pad);
> > +
> > + if (startPipeline() != TestPass)
> > + return TestFail;
> > +
> > + if (processEvent() != TestPass)
> > + return TestFail;
> > +
> > + return TestPass;
> > + }
> > +
> > +private:
> > + std::string cameraName;
> > + GstElement *convert0_;
> > + GstElement *convert1_;
> > + GstElement *sink0_;
> > + GstElement *sink1_;
> > + GstElement *queue0_;
> > + GstElement *queue1_;
>
> No destructor ? All leaked ?
>
How ? I have added all to pipeline, it will take care of their lifetime.
> +};
> > +
> > +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)
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210914/c15a2c2f/attachment-0001.htm>
More information about the libcamera-devel
mailing list