[libcamera-devel] [PATCH v11] test: gstreamer: Add test for gstreamer single stream
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Aug 16 21:43:53 CEST 2021
On 16/08/2021 17:34, Laurent Pinchart wrote:
> On Mon, Aug 16, 2021 at 03:22:29PM +0100, Kieran Bingham wrote:
>> On 16/08/2021 15:14, Vedant Paranjape wrote:
>>> Hi Laurent,
>>> Yes, what's the camera-name with which vimc camera can be selected ?
>>> I could simply set the object property to use it. I think it would be
>>> nice to have two separate tests, one which runs on real camera and other
>>> one on vimc camera.
>>
>> Or runs on all available cameras?
>
> I'm in two minds about this. We don't run all our other tests against
> all cameras, and instead consider that pipeline handler testing is the
> job of lc-compliance. For the GStreamer element, it could be different
> if we wanted to unit-test features that don't work with vimc, but for
> the single stream test, I'm not sure testing all cameras would be the
> best option.
Actually, I agree - for consistancy and reproducability of the unit
tests, I think fixing to VIMC is better.
I believe we have defined VIMC as a requirement for the unit tests, if
not explicitly - then certainly defined by code already.
Using VIMC makes the most sense.
I do think there is some benefit in some test being able to run on 'all
cameras' though, but that's more about being able to test multiple
cameras - which would then /require/ multiple cameras to run the test
... so - lets not worry about that for now ;-)
--
Kieran
>>> On Mon, Aug 16, 2021 at 7:29 PM Laurent Pinchart wrote:
>>>> 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>
>>>>>>
>>>>>> 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>
>>>>>> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>>>>>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>>>>> Tested-by: Kieran Bingham <kieran.bingham@@ideasonboard.com>
>>>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>>>> Signed-off-by: Laurent Pinchart <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')
>
More information about the libcamera-devel
mailing list