[libcamera-devel] [PATCH v2] test: gstreamer: Add a test for gstreamer multi stream

Nicolas Dufresne nicolas at ndufresne.ca
Tue Sep 14 19:49:00 CEST 2021


Le mardi 14 septembre 2021 à 23:16 +0530, Vedant Paranjape a écrit :
> 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.

Well, if you remove the floating ref, ownership is not transferred to the
pipeline and you have to unref when you are done. That is the core principal of
floating references.

> 
> > > +};
> > > +
> > > +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/2a713820/attachment.htm>


More information about the libcamera-devel mailing list