[libcamera-devel] [PATCH v11] test: gstreamer: Add test for gstreamer single stream

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Aug 16 15:58:54 CEST 2021


Hi Vedant,

On Mon, Aug 16, 2021 at 07:04:00PM +0530, Vedant Paranjape wrote:
> Hi Umang,
> Thanks for the Review. I feel the changes are appropriate. I will send a
> patch with the changes.

While at it, I'm wondering if it would make sense to select the vimc
camera explicitly from the libcamerasrc element instead of using
whatever camera happens to be the first. It would make tests more
reproducible.

> On Mon, Aug 16, 2021 at 11:17 AM Umang Jain wrote:
> 
> > Hi Vedant and Laurent,
> >
> > Thanks for the patch. Overall looks good to me,
> >
> > a few comments below for cleanup paths :)
> > On 8/14/21 5:16 AM, Laurent Pinchart wrote:
> >
> > From: Vedant Paranjape <vedantparanjape160201 at gmail.com> <vedantparanjape160201 at gmail.com>
> >
> > This patch adds a test to test if single stream using libcamera's
> > gstreamer element works.
> >
> > We need to work around two distinct issues 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.
> >
> > - GStreamer spawns a child process to scan plugins. If GStreamer is
> >   compiled without ASan (which is likely) but libcamera is, dlopen()ing
> >   the libcamera plugin will cause an ASan link order verification
> >   failure. Disable the verification child processes to work around the
> >   problem. This requires gcc 8 or newer.
> >
> > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com> <vedantparanjape160201 at gmail.com>
> > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com> <paul.elder at ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com> <kieran.bingham at ideasonboard.com>
> > Tested-by: Kieran Bingham <kieran.bingham@@ideasonboard.com> <kieran.bingham@@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com> <laurent.pinchart at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com> <laurent.pinchart at ideasonboard.com>
> > ---
> > This version incorporates changes coming from my review of v10, and
> > fixes for ASan issues. ASan is a bit of a pain with GStreamer, for two
> > independent reasons as explained in the commit message. I'd like to find
> > a way to use leak suppression files to handle the glib initialization
> > leak, but that's a big rabbit hole. The workaround for the second issue
> > is acceptable in my opinion.
> >
> > The biggest trouble with these workarounds is that they don't work with
> > gcc version older than 8. As the stable version of the most common Linux
> > distributions ship gcc 8 or newer, skipping this test for gcc 7 (which
> > is the oldest version that libcamera supports) is also acceptable in my
> > opinion.
> >
> > Changes since v10:
> >
> > - Disable ASan leak detection
> > - Disable ASan link order verification for child processes
> > - Include source_path.h
> > - Remove unneeded explicit std::string construction
> > - Declare variables at usage site
> > - Make constants constexpr
> > - Add a variable for the message type
> > - Blank space fixes
> > ---
> >  .../gstreamer_single_stream_test.cpp          | 188 ++++++++++++++++++
> >  test/gstreamer/meson.build                    |  19 ++
> >  test/meson.build                              |   1 +
> >  3 files changed, 208 insertions(+)
> >  create mode 100644 test/gstreamer/gstreamer_single_stream_test.cpp
> >  create mode 100644 test/gstreamer/meson.build
> >
> > diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
> > new file mode 100644
> > index 000000000000..e26673b3471a
> > --- /dev/null
> > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> > @@ -0,0 +1,188 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2021, Vedant Paranjape
> > + *
> > + * ipa_interface_test.cpp - Test the IPA interface
> > + */
> > +
> > +#include <iostream>
> > +#include <unistd.h>
> > +
> > +#include <libcamera/base/utils.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 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 will cause 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. Work around this issue by disabling the link order
> > +		 * check. This will only affect child processes, as ASan is
> > +		 * already loaded for this process by the time this code is
> > +		 * executed, and should thus hopefully be safe.
> > +		 *
> > +		 * This option is not available in gcc older than 8, the only
> > +		 * option in that case is to skip the test.
> > +		 */
> > +#if defined(__SANITIZE_ADDRESS__) && !defined(__clang__) && __GNUC__ < 8
> > +		return TestSkip;
> > +#endif
> > +		setenv("ASAN_OPTIONS", "verify_asan_link_order=0", 1);
> > +
> > +		/* Initialize GStreamer */
> > +		GError *errInit = nullptr;
> >
> > One can use:
> >
> >     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");
> > +			if (errInit)
> > +				g_error_free(errInit);
> >
> > And remove manual free here.
> >
> > +
> > +			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;
> > +		}
> > +
> > +		/* Create the elements */
> > +		libcameraSrc_ = gst_element_factory_make("libcamerasrc", "libcamera");
> > +		convert0_ = gst_element_factory_make("videoconvert", "convert0");
> > +		sink0_ = gst_element_factory_make("fakesink", "sink0");
> > +
> > +		/* Create the empty pipeline_ */
> > +		pipeline_ = gst_pipeline_new("test-pipeline");
> > +
> > +		if (!pipeline_ || !convert0_ || !sink0_ || !libcameraSrc_) {
> > +			g_printerr("Not all elements could be created. %p.%p.%p.%p\n",
> > +				   pipeline_, convert0_, sink0_, libcameraSrc_);
> > +			if (pipeline_)
> > +				gst_object_unref(pipeline_);
> > +			if (convert0_)
> > +				gst_object_unref(convert0_);
> > +			if (sink0_)
> > +				gst_object_unref(sink0_);
> > +			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_, convert0_, sink0_, NULL);
> > +		if (gst_element_link_many(libcameraSrc_, convert0_, sink0_, NULL) != TRUE) {
> > +			g_printerr("Elements could not be linked.\n");
> > +			return TestFail;
> > +		}
> > +
> > +		/* 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 = 2000000000;
> > +
> > +		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");
> >
> > EOS doesn't sound like a error/Test-fail condition to me. If there is a
> > possibility that EOS will be reached before timeout, what's ensures(in the
> > first place) that the pipeline will be in playing state for entire timeout,
> > before we request masked EOS/Error messages from the bus? What am I missing?
> 
> I am not a gstreamer expert, to the best of my knowledge, EOS won't be ever
> sent by libcamerasrc, as it is the element which drives the pipeline. I
> think this event can be dropped, I just want to confirm it with Nicolas
> once he's back.
> 
> > It is important to note that *only elements driving the pipeline should
> ever send an EOS event*. If your element is chain-based, it is not driving
> the pipeline. Chain-based elements should just return GST_FLOW_EOS from
> their chain function at the end of the stream (or the configured segment)
> 
> https://gstreamer.freedesktop.org/documentation/plugin-development/advanced/events.html?gi-language=c#end-of-stream-eos
> 
> +			break;
> > +		default:
> > +			g_printerr("Unexpected message received.\n");
> > +			break;
> > +		}
> > +
> > +		return TestFail;
> > +	}
> > +
> > +private:
> > +	void gstreamer_print_error(GstMessage *msg)
> > +	{
> > +		GError *err;
> >
> > This can use:
> >
> >     g_autoptr(GError) err = NULL;
> >
> > +		gchar *debug_info;
> >
> > and
> >
> >      g_autofree char *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");
> > +		g_clear_error(&err);
> > +		g_free(debug_info);
> >
> > So, that will lead us to dropping manual free of err and debug info
> >
> >
> > Minor things, so:
> >
> > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> >
> > +	}
> > +
> > +	GstElement *pipeline_;
> > +	GstElement *libcameraSrc_;
> > +	GstElement *convert0_;
> > +	GstElement *sink0_;
> > +};
> > +
> > +TEST_REGISTER(GstreamerSingleStreamTest)
> > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build
> > new file mode 100644
> > index 000000000000..b99aa0da0ba3
> > --- /dev/null
> > +++ b/test/gstreamer/meson.build
> > @@ -0,0 +1,19 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +if not gst_enabled
> > +    subdir_done()
> > +endif
> > +
> > +gstreamer_tests = [
> > +    ['single_stream_test',   'gstreamer_single_stream_test.cpp'],
> > +]
> > +gstreamer_dep = dependency('gstreamer-1.0', required: true)
> > +
> > +foreach t : gstreamer_tests
> > +    exe = executable(t[0], t[1],
> > +                     dependencies : [libcamera_private, gstreamer_dep],
> > +                     link_with : test_libraries,
> > +                     include_directories : test_includes_internal)
> > +
> > +    test(t[0], exe, suite : 'gstreamer', is_parallel : false)
> > +endforeach
> > diff --git a/test/meson.build b/test/meson.build
> > index 3bceb5df586f..d0466f17d7b6 100644
> > --- a/test/meson.build
> > +++ b/test/meson.build
> > @@ -11,6 +11,7 @@ subdir('libtest')
> >
> >  subdir('camera')
> >  subdir('controls')
> > +subdir('gstreamer')
> >  subdir('ipa')
> >  subdir('ipc')
> >  subdir('log')

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list