[libcamera-devel] [PATCH v2] test: gstreamer: Add a test for gstreamer multi stream
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Aug 29 11:25:51 CEST 2021
Hi Vedant,
On Sun, Aug 29, 2021 at 10:53:34AM +0530, Vedant Paranjape wrote:
> On Sun, Aug 29, 2021 at 6:39 AM Laurent Pinchart wrote:
> > On Sat, Aug 28, 2021 at 11:07:53PM +0530, Vedant Paranjape wrote:
> > > This patch adds a test to test if multi stream using libcamera's
> > > gstreamer element works.
> > >
> > > We need to work around one issue with ASan when enabled in the
> > > build:
> > >
> > > - glib has a known leak at initialization time. This is covered by the
> > > suppression file shipped with glib, but it's not clear how to use it
> > > automatically. For now, disable leak detection to avoid test failures.
> > >
> > > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > > ---
> > > .../gstreamer/gstreamer_multi_stream_test.cpp | 225 ++++++++++++++++++
> > > test/gstreamer/meson.build | 1 +
> > > 2 files changed, 226 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 00000000..e32b07a4
> > > --- /dev/null
> > > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp
> > > @@ -0,0 +1,225 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/*
> > > + * Copyright (C) 2021, Vedant Paranjape
> > > + *
> > > + * gstreamer_single_stream_test.cpp - GStreamer single 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 "test.h"
> > > +
> > > +using namespace std;
> > > +
> > > +extern "C" {
> > > +const char *__asan_default_options()
> > > +{
> > > + /*
> > > + * Disable leak detection due to a known global variable initialization
> > > + * leak in glib's g_quark_init(). This should ideally be handled by
> > > + * using a suppression file instead of disabling leak detection.
> > > + */
> > > + return "detect_leaks=false";
> > > +}
> > > +}
> > > +
> > > +class GstreamerSingleStreamTest : public Test
> > > +{
> > > +protected:
> > > + int init() override
> > > + {
> > > + /*
> > > + * GStreamer by default spawns a process to run the
> > > + * gst-plugin-scanner helper. If libcamera is compiled with ASan
> > > + * enabled, and as GStreamer is most likely not, this causes the
> > > + * ASan link order check to fail when gst-plugin-scanner
> > > + * dlopen()s the plugin as many libraries will have already been
> > > + * loaded by then. Fix this issue by disabling spawning of a
> > > + * child helper process when scanning the build directory for
> > > + * plugins.
> > > + */
> > > + gst_registry_fork_set_enabled(false);
> > > +
> > > + /* Initialize GStreamer */
> > > + g_autoptr(GError) errInit = NULL;
> > > + if (!gst_init_check(nullptr, nullptr, &errInit)) {
> > > + g_printerr("Could not initialize GStreamer: %s\n",
> > > + errInit ? errInit->message : "unknown error");
> > > +
> > > + return TestFail;
> > > + }
> > > +
> > > + /*
> > > + * Remove the system libcamera plugin, if any, and add the
> > > + * plugin from the build directory.
> > > + */
> > > + GstRegistry *registry = gst_registry_get();
> > > + GstPlugin *plugin = gst_registry_lookup(registry, "libgstlibcamera.so");
> > > + if (plugin) {
> > > + gst_registry_remove_plugin(registry, plugin);
> > > + gst_object_unref(plugin);
> > > + }
> > > +
> > > + std::string path = libcamera::utils::libcameraBuildPath()
> > > + + "src/gstreamer";
> > > + if (!gst_registry_scan_path(registry, path.c_str())) {
> > > + g_printerr("Failed to add plugin to registry\n");
> > > + gst_deinit();
> > > + return TestFail;
> > > + }
> >
> > There's quite a bit of duplicated code between this and the single
> > stream test. Could you refactor the single stream test to create a base
> > class that can be shared with this one ?
>
> Yes, I am going to do this, but can we go about it as follows, merge this
> patch with duplicated code,
> then I think over it a bit and then submit new patch which refactors a lot
> of the duplicated code,
Why ? :-) I'm fine merging work in progress when there's a reason (for
instance to split very large partch series in chunks, or to avoid
blocking development when other developers depend on that code), but in
this case I think a refactoring before merging the second test is
better.
> also I don't think going ahead with making a base class is a good idea as,
> there's much functional difference between two codes,
> they rather have isolated code that repeats, but no so much as to roll it
> into a class.
I don't agree, I think the tests are very similar. diffstat agrees with
me.
$ diff -u gstreamer_single_stream_test.cpp gstreamer_multi_stream_test.cpp | diffstat
gstreamer_multi_stream_test.cpp | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 55 insertions(+), 7 deletions(-)
> > +
> > > + /* Check if platform support multistream output */
> > > + libcamera::CameraManager cm;
> > > + cm.start();
> > > + if (cm.cameras()[0]->streams().size() <= 1) {
> > > + cm.stop();
> > > + return TestSkip;
> > > + }
> > > + cm.stop();
> >
> > Is there a guarantee that libcamerasrc will pick the same camera ? It
>
> Yes, it is guaranteed if we don't pass it a camera-name which we aren't see
> here:
> https://git.linuxtv.org/libcamera.git/tree/src/gstreamer/gstlibcamerasrc.cpp#n237
>
> > seems quite fragile to me. Isn't there a way to instead query the
> > libcamerasrc element for the number of pads it supports ?
>
> I think this won't be possible unless we add code to the
> gstreamer-element, @Nicolas
> Dufresne <nicolas at ndufresne.ca> can
> you answer this please, I am not sure :)
The GstElement documentation mentions a numsrcpads field. I have little
experience with GStreamer, but it sounds like something that could help.
Have you tried it ?
> > More than
> > that, as not all cameras support multiple streams, it would be nice if
> > the test could iterate over all cameras to find one that does.
>
> This sounds like a nice idea, I could store the camera-name of the specific
> camera and set the camera-name property
> of the libcamera element and start on it, but now I don't a multicamera
> setup.
>
> > +
> > > + /* Create the elements */
> > > + libcameraSrc_ = gst_element_factory_make("libcamerasrc", "libcamera");
> > > + convert0_ = gst_element_factory_make("videoconvert", "convert0");
> > > + convert1_ = gst_element_factory_make("videoconvert", "convert1");
> > > + sink0_ = gst_element_factory_make("fakesink", "sink0");
> > > + sink1_ = gst_element_factory_make("fakesink", "sink1");
> > > + queue0_ = gst_element_factory_make("queue", "camera_queue0");
> > > + queue1_ = gst_element_factory_make("queue", "camera_queue1");
> >
> > Looks like there's room to make the code more generic, posibly storing
> > the elements in containers.
>
> Okay something like a vector to hold the elements and then a map to hold
> <element name, unique name>.
> But this is a test code, is this fancy stuff really needed ? !!
Test code doesn't mean that it has to be dirty and throw away all
software development good practices. Tests are first class citizens.
It's not about "fancy stuff", it's about making the code more readable
and maintainable.
> > > +
> > > + /* Create the empty pipeline_ */
> > > + pipeline_ = gst_pipeline_new("test-pipeline");
> > > +
> > > + if (!pipeline_ || !convert0_ || !convert1_ || !sink0_ ||
> > > + !sink1_ || !queue0_ || !queue1_ || !libcameraSrc_) {
> > > + g_printerr("Not all elements could be created.> %p.%p.%p.%p.%p.%p.%p.%p\n",
> > > + pipeline_, convert0_, convert1_, sink0_,
> > > + sink1_, queue0_, queue1_, libcameraSrc_);
> > > + if (pipeline_)
> > > + gst_object_unref(pipeline_);
> > > + if (convert0_)
> > > + gst_object_unref(convert0_);
> > > + if (convert1_)
> > > + gst_object_unref(convert1_);
> > > + if (sink0_)
> > > + gst_object_unref(sink0_);
> > > + if (sink1_)
> > > + gst_object_unref(sink1_);
> > > + if (queue0_)
> > > + gst_object_unref(queue0_);
> > > + if (queue1_)
> > > + gst_object_unref(queue1_);
> > > + if (libcameraSrc_)
> > > + gst_object_unref(libcameraSrc_);
> > > + gst_deinit();
> > > +
> > > + return TestFail;
> > > + }
> > > +
> > > + return TestPass;
> > > + }
> > > +
> > > + void cleanup() override
> > > + {
> > > + gst_object_unref(pipeline_);
> > > + gst_deinit();
> > > + }
> > > +
> > > + int run() override
> > > + {
> > > + GstStateChangeReturn ret;
> > > +
> > > + /* 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;
> > > + }
> > > +
> > > + 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);
> > > +
> > > + /* Start playing */
> > > + ret = gst_element_set_state(pipeline_, GST_STATE_PLAYING);
> > > + if (ret == GST_STATE_CHANGE_FAILURE) {
> > > + g_printerr("Unable to set the pipeline to the playing state.\n");
> > > + return TestFail;
> > > + }
> > > +
> > > + /* Wait until error or EOS or timeout after 2 seconds */
> > > + constexpr GstMessageType msgType =
> > > + static_cast<GstMessageType>(GST_MESSAGE_ERROR | GST_MESSAGE_EOS);
> > > + constexpr GstClockTime timeout = 2 * GST_SECOND;
> > > +
> > > + g_autoptr(GstBus) bus = gst_element_get_bus(pipeline_);
> > > + g_autoptr(GstMessage) msg = gst_bus_timed_pop_filtered(bus, timeout, msgType);
> > > +
> > > + gst_element_set_state(pipeline_, GST_STATE_NULL);
> > > +
> > > + /* Parse error message */
> > > + if (msg == NULL)
> > > + return TestPass;
> > > +
> > > + switch (GST_MESSAGE_TYPE(msg)) {
> > > + case GST_MESSAGE_ERROR:
> > > + gstreamer_print_error(msg);
> > > + break;
> > > + case GST_MESSAGE_EOS:
> > > + g_print("End-Of-Stream reached.\n");
> > > + break;
> > > + default:
> > > + g_printerr("Unexpected message received.\n");
> > > + break;
> > > + }
> > > +
> > > + return TestFail;
> > > + }
> > > +
> > > +private:
> > > + void gstreamer_print_error(GstMessage *msg)
> > > + {
> > > + g_autoptr(GError) err = NULL;
> > > + g_autofree gchar *debug_info = NULL;
> > > +
> > > + gst_message_parse_error(msg, &err, &debug_info);
> > > + g_printerr("Error received from element %s: %s\n",
> > > + GST_OBJECT_NAME(msg->src), err->message);
> > > + g_printerr("Debugging information: %s\n",
> > > + debug_info ? debug_info : "none");
> > > + }
> > > +
> > > + GstElement *pipeline_;
> > > + GstElement *libcameraSrc_;
> > > + GstElement *queue0_;
> > > + GstElement *queue1_;
> > > + GstElement *convert0_;
> > > + GstElement *convert1_;
> > > + GstElement *sink0_;
> > > + GstElement *sink1_;
> > > +};
> > > +
> > > +TEST_REGISTER(GstreamerSingleStreamTest)
> > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build
> > > index b99aa0da..e28225b2 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)
> > >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list