[libcamera-devel] [PATCH v3] test: gstreamer: Factor out code into a base class

Nicolas Dufresne nicolas at ndufresne.ca
Wed Sep 1 17:54:55 CEST 2021


Le mercredi 01 septembre 2021 à 02:13 +0530, Vedant Paranjape a écrit :
> 
> 
> On Wed, 1 Sep, 2021, 02:11 Nicolas Dufresne, <nicolas at ndufresne.ca> wrote:
> > Le mercredi 01 septembre 2021 à 02:04 +0530, Vedant Paranjape a écrit :
> > > 
> > > 
> > > On Wed, 1 Sep, 2021, 01:59 Nicolas Dufresne, <nicolas at ndufresne.ca> wrote:
> > > > Le mercredi 01 septembre 2021 à 01:32 +0530, Vedant Paranjape a écrit :
> > > > > Lot of code used in single stream test is boiler plate and common
> > > > > across
> > > > > every gstreamer test. Factored out this code into a base class called
> > > > > GstreamerTest.
> > > > > 
> > > > > Also updated the gstreamer_single_stream_test to use the GstreamerTest
> > > > > base class
> > > > > 
> > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > > > 
> > > > For my part, this looks like a good start, we will likely modify and
> > > > adapt
> > > > test
> > > > while adding few more and it will get more stable later.
> > > > 
> > > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > > > 
> > > > p.s. I'm starting a discussion inline about the test design itself, why
> > they
> > > > need to 2s instead of stop whenever a success condition is met. But this
> > > > isn't
> > > > a
> > > > change from the already merged test, so not a blocker for this.
> > > 
> > > I was the one who made it timeout at 2s, as without timeout the test kept
> > > on
> > > running forever, what's the success condition here?
> > 
> > This is just a test run, I'd use pad probe, and after couple of non-zero
> > frames,
> > I'd stop and quit. This can be done with a GMainLoop, using
> > g_main_loop_quit(),
> > or using an application message, that get catched as success in your
> > process_event() function.
> > 
> > The second way is thinner I believe.
> 
> What benefits do these method have over the way I did it?

Consider long term, with growing number of test, if all tests take 2s to run,
and run one after the other, running the test will be very very long.

With a test that only runs the time requires to figure-out if the camera is
spitting non-fully black images, only few ms are needed.

> 
> > > 
> > > > > ---
> > > > >   .../gstreamer_single_stream_test.cpp          | 145 +++-------------
> > > > >   test/gstreamer/gstreamer_test.cpp             | 157
> > > > > ++++++++++++++++++
> > > > >   test/gstreamer/gstreamer_test.h               |  39 +++++
> > > > >   test/gstreamer/meson.build                    |   2 +-
> > > > >   4 files changed, 221 insertions(+), 122 deletions(-)
> > > > >   create mode 100644 test/gstreamer/gstreamer_test.cpp
> > > > >   create mode 100644 test/gstreamer/gstreamer_test.h
> > > > > 
> > > > > diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp
> > > > b/test/gstreamer/gstreamer_single_stream_test.cpp
> > > > > index 4c8d4804..c27e4d36 100644
> > > > > --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> > > > > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> > > > > @@ -14,104 +14,48 @@
> > > > >   
> > > > >   #include <gst/gst.h>
> > > > >   
> > > > > +#include "gstreamer_test.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
> > > > > +class GstreamerSingleStreamTest : public GstreamerTest, public Test
> > > > >   {
> > > > > +public:
> > > > > +     GstreamerSingleStreamTest()
> > > > > +             : GstreamerTest()
> > > > > +     {
> > > > > +     }
> > > > > +
> > > > >   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");
> > > > > +             if (status_ != TestPass)
> > > > > +                     return status_;
> > > > >   
> > > > > -                     return TestFail;
> > > > > -             }
> > > > > +             g_autoptr(GstElement) convert0__ =
> > > > gst_element_factory_make("videoconvert", "convert0");
> > > > > +             g_autoptr(GstElement) sink0__ =
> > > > gst_element_factory_make("fakesink", "sink0");
> > > > > +             g_object_ref_sink(convert0__);
> > > > > +             g_object_ref_sink(sink0__);
> > > > >   
> > > > > -             /*
> > > > > -              * 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);
> > > > > -             }
> > > > > +             if (!convert0__ || !sink0__) {
> > > > > +                     g_printerr("Not all elements could be created.
> > > > %p.%p\n",
> > > > > +                                convert0__, sink0__);
> > > > >   
> > > > > -             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();
> > > > > +             convert0_ = reinterpret_cast<GstElement
> > > > *>(g_steal_pointer(&convert0__));
> > > > > +             sink0_ = reinterpret_cast<GstElement
> > > > *>(g_steal_pointer(&sink0__));
> > > > >   
> > > > > +             if (gstreamer_create_pipeline() != TestPass)
> > > > >                        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) {
> > > > > @@ -119,57 +63,16 @@ protected:
> > > > >                        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");
> > > > > +             if (gstreamer_start_pipeline() != TestPass)
> > > > >                        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;
> > > > > -             }
> > > > > +             if (gstreamer_process_event() != TestPass)
> > > > > +                     return TestFail;
> > > > >   
> > > > > -             return TestFail;
> > > > > +             return TestPass;
> > > > >        }
> > > > >   
> > > > >   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 *convert0_;
> > > > >        GstElement *sink0_;
> > > > >   };
> > > > > diff --git a/test/gstreamer/gstreamer_test.cpp
> > > > b/test/gstreamer/gstreamer_test.cpp
> > > > > new file mode 100644
> > > > > index 00000000..22128c4c
> > > > > --- /dev/null
> > > > > +++ b/test/gstreamer/gstreamer_test.cpp
> > > > > @@ -0,0 +1,157 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > > +/*
> > > > > + * Copyright (C) 2021, Vedant Paranjape
> > > > > + *
> > > > > + * libcamera Gstreamer element API tests
> > > > > + */
> > > > > +
> > > > > +#include "gstreamer_test.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";
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +GstreamerTest::GstreamerTest()
> > > > > +{
> > > > > +     /*
> > > > > +     * 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");
> > > > > +
> > > > > +             status_ = TestFail;
> > > > > +             return;
> > > > > +     }
> > > > > +
> > > > > +     /*
> > > > > +     * Remove the system libcamera plugin, if any, and add the
> > > > > +     * plugin from the build directory.
> > > > > +     */
> > > > > +     GstRegistry *registry = gst_registry_get();
> > > > > +     g_autoptr(GstPlugin) plugin = gst_registry_lookup(registry,
> > > > "libgstlibcamera.so");
> > > > > +     if (plugin) {
> > > > > +             gst_registry_remove_plugin(registry, 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");
> > > > > +
> > > > > +             status_ = TestFail;
> > > > > +             return;
> > > > > +     }
> > > > > +
> > > > > +     status_ = TestPass;
> > > > > +}
> > > > > +
> > > > > +GstreamerTest::~GstreamerTest()
> > > > > +{
> > > > > +     if (pipeline_)
> > > > > +             gst_object_unref(pipeline_);
> > > > > +     if (status_ == TestFail) {
> > > > > +             gst_object_unref(libcameraSrc_);
> > > > > +     }
> > > > > +
> > > > > +     gst_deinit();
> > > > > +}
> > > > > +
> > > > > +int GstreamerTest::gstreamer_create_pipeline()
> > > > > +{
> > > > > +     g_autoptr(GstElement) libcameraSrc__ =
> > > > gst_element_factory_make("libcamerasrc", "libcamera");
> > > > > +     pipeline_ = gst_pipeline_new("test-pipeline");
> > > > > +     g_object_ref_sink(libcameraSrc__);
> > > > > +
> > > > > +     if (!libcameraSrc__ || !pipeline_) {
> > > > > +             g_printerr("Unable to create create pipeline %p.%p\n",
> > > > > +                                             libcameraSrc__,
> > pipeline_);
> > > > > +
> > > > > +             return TestFail;
> > > > > +     }
> > > > > +
> > > > > +     libcameraSrc_ = reinterpret_cast<GstElement
> > > > *>(g_steal_pointer(&libcameraSrc__));
> > > > > +
> > > > > +     return TestPass;
> > > > > +}
> > > > > +
> > > > > +int GstreamerTest::gstreamer_start_pipeline()
> > > > > +{
> > > > > +     GstStateChangeReturn ret;
> > > > > +
> > > > > +     /* 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;
> > > > > +     }
> > > > > +
> > > > > +     return TestPass;
> > > > > +}
> > > > > +
> > > > > +int GstreamerTest::gstreamer_process_event()
> > > > > +{
> > > > > +     /* 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;
> > > > > +     }
> > > > 
> > > > Isn't it unfortunate design that all test must last 2s ? Just opening
> > > > the
> > > > discussion, this was like this before.
> > > > 
> > > > > +
> > > > > +     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;
> > > > > +}
> > > > > +
> > > > > +void GstreamerTest::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");
> > > > > +}
> > > > > +
> > > > > diff --git a/test/gstreamer/gstreamer_test.h
> > > > b/test/gstreamer/gstreamer_test.h
> > > > > new file mode 100644
> > > > > index 00000000..cdffdea9
> > > > > --- /dev/null
> > > > > +++ b/test/gstreamer/gstreamer_test.h
> > > > > @@ -0,0 +1,39 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > > +/*
> > > > > + * Copyright (C) 2021, Vedant Paranjape
> > > > > + *
> > > > > + * gstreamer_test.cpp - GStreamer test base class
> > > > > + */
> > > > > +
> > > > > +#ifndef __LIBCAMERA_GSTREAMER_TEST_H__
> > > > > +#define __LIBCAMERA_GSTREAMER_TEST_H__
> > > > > +
> > > > > +#include <iostream>
> > > > > +#include <unistd.h>
> > > > > +
> > > > > +#include <libcamera/base/utils.h>
> > > > > +
> > > > > +#include "libcamera/internal/source_paths.h"
> > > > > +
> > > > > +#include <gst/gst.h>
> > > > > +
> > > > > +using namespace std;
> > > > > +
> > > > > +class GstreamerTest
> > > > > +{
> > > > > +public:
> > > > > +     GstreamerTest();
> > > > > +     virtual ~GstreamerTest();
> > > > > +
> > > > > +protected:
> > > > > +     virtual int gstreamer_create_pipeline();
> > > > > +     int gstreamer_start_pipeline();
> > > > > +     int gstreamer_process_event();
> > > > > +     void gstreamer_print_error(GstMessage *msg);
> > > > > +
> > > > > +     GstElement *pipeline_;
> > > > > +     GstElement *libcameraSrc_;
> > > > > +     int status_;
> > > > > +};
> > > > > +
> > > > > +#endif /* __LIBCAMERA_GSTREAMER_TEST_H__ */
> > > > > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build
> > > > > index b99aa0da..aca53b92 100644
> > > > > --- a/test/gstreamer/meson.build
> > > > > +++ b/test/gstreamer/meson.build
> > > > > @@ -10,7 +10,7 @@ gstreamer_tests = [
> > > > >   gstreamer_dep = dependency('gstreamer-1.0', required: true)
> > > > >   
> > > > >   foreach t : gstreamer_tests
> > > > > -    exe = executable(t[0], t[1],
> > > > > +    exe = executable(t[0], t[1], 'gstreamer_test.cpp',
> > > > >                        dependencies : [libcamera_private,
> > gstreamer_dep],
> > > > >                        link_with : test_libraries,
> > > > >                        include_directories : test_includes_internal)
> > > > 
> > > > 
> > 
> > 




More information about the libcamera-devel mailing list