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

13585 diff mbox series

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@gmail.com>
---
 .../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

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@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@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.

> ---
>  .../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)

On Wed, 1 Sep, 2021, 01:59 Nicolas Dufresne, <nicolas@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@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@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?

> ---
> >  .../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)
>
>
>

Le mercredi 01 septembre 2021 à 02:04 +0530, Vedant Paranjape a écrit :
> 
> 
> On Wed, 1 Sep, 2021, 01:59 Nicolas Dufresne, <nicolas@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@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@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.

> 
> > > ---
> > >   .../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)
> > 
> >

On Wed, 1 Sep, 2021, 02:11 Nicolas Dufresne, <nicolas@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@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@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@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?

>
> > > > ---
> > > >   .../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)
> > >
> > >
>
>
>

On Tue, Aug 31, 2021 at 04:41:06PM -0400, Nicolas Dufresne wrote:
> Le mercredi 01 septembre 2021 à 02:04 +0530, Vedant Paranjape a écrit :
> > On Wed, 1 Sep, 2021, 01:59 Nicolas Dufresne 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@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@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.

Please don't forget to include a timeout.

> The second way is thinner I believe.
> 
> > > > ---
> > > >   .../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 > /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 > nitialization
> > > > -      * 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 > San
> > > > -              * enabled, and as GStreamer is most likely not, this causes > he
> > > > -              * ASan link order check to fail when gst-plugin-scanner
> > > > -              * dlopen()s the plugin as many libraries will have already > een
> > > > -              * 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 > rror");
> > > > +             if (status_ != TestPass)
> > > > +                     return status_;
> > > >   
> > > > -                     return TestFail;
> > > > -             }
> > > > +             g_autoptr(GstElement) convert0__ = > st_element_factory_make("videoconvert", "convert0");
> > > > +             g_autoptr(GstElement) sink0__ = > st_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_, > ibcameraSrc_);
> > > > -                     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_, > ink0_, NULL);
> > > >                if (gst_element_link_many(libcameraSrc_, convert0_, sink0_, > ULL) != 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 > tate.\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 | > ST_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, > imeout, 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 > /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 > nitialization
> > > > +      * 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__ = > st_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 > tate.\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 | > ST_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, > sgType);
> > > > +
> > > > +     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 > /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)

Le mercredi 01 septembre 2021 à 02:13 +0530, Vedant Paranjape a écrit :
> 
> 
> On Wed, 1 Sep, 2021, 02:11 Nicolas Dufresne, <nicolas@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@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@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@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)
> > > > 
> > > > 
> > 
> >

Got it, thanks!

On Wed, Sep 1, 2021 at 9:24 PM Nicolas Dufresne <nicolas@ndufresne.ca>
wrote:

> Le mercredi 01 septembre 2021 à 02:13 +0530, Vedant Paranjape a écrit :
> >
> >
> > On Wed, 1 Sep, 2021, 02:11 Nicolas Dufresne, <nicolas@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@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@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@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)
> > > > >
> > > > >
> > >
> > >
>
>
>

Hi Vedant,

On Wed, Sep 01, 2021 at 01:32:17AM +0530, Vedant Paranjape wrote:
> 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@gmail.com>
> ---
>  .../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");

For local variables, you just remove the ending underscore altogether
(also the other instances where you do so).

> +		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);
> +	}

You don't need braces here.

> +
> +	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_);
> +	}

You don't need braces here.

How come you unref libcameraSrc_ but not the others upon failure?
What's the difference between libcameraSrc_ and the other pipeline
components? Is TestFail both sufficient and necessary to need to free
libcameraSrc_?

> +
> +	gst_deinit();
> +}
> +
> +int GstreamerTest::gstreamer_create_pipeline()

All of these functions should be camelCase. Also since they're all
members of GstreamerTest, I think you can drop the gstreamer prefix too.

> +{
> +	g_autoptr(GstElement) libcameraSrc__ = gst_element_factory_make("libcamerasrc", "libcamera");
> +	pipeline_ = gst_pipeline_new("test-pipeline");
> +	g_object_ref_sink(libcameraSrc__);

You can just drop the underscores completely.


Hm other than those, I think it's ready.


Paul

> +
> +	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;
> +	}
> +
> +	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)
> -- 
> 2.25.1
>

Hi Paul,
Thanks for the review.


On Wed, Sep 8, 2021 at 9:30 AM <paul.elder@ideasonboard.com> wrote:

> Hi Vedant,
>
> On Wed, Sep 01, 2021 at 01:32:17AM +0530, Vedant Paranjape wrote:
> > 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@gmail.com>
> > ---
> >  .../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");
>
> For local variables, you just remove the ending underscore altogether
> (also the other instances where you do so).
>

Okay, done !

> +             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);
> > +     }
>
> You don't need braces here.
>
> > +
> > +     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_);
> > +     }
>
> You don't need braces here.
>
> How come you unref libcameraSrc_ but not the others upon failure?
> What's the difference between libcameraSrc_ and the other pipeline
> components? Is TestFail both sufficient and necessary to need to free
> libcameraSrc_?
>

This is because the other elements are added to the pipeline, once that is
done successfully the pipeline owns them and takes care of their existence.
So, there's going to be a minor change here though. But, in this case,
since create pipeline is called before the elements are added to the
pipeline, I need
to take care of libcameraSrc.

> +
> > +     gst_deinit();
> > +}
> > +
> > +int GstreamerTest::gstreamer_create_pipeline()
>
> All of these functions should be camelCase. Also since they're all
> members of GstreamerTest, I think you can drop the gstreamer prefix too.
>

Okay

> +{
> > +     g_autoptr(GstElement) libcameraSrc__ =
> gst_element_factory_make("libcamerasrc", "libcamera");
> > +     pipeline_ = gst_pipeline_new("test-pipeline");
> > +     g_object_ref_sink(libcameraSrc__);
>
> You can just drop the underscores completely.
>
>
> Hm other than those, I think it's ready.
>
>
> Paul
>
> > +
> > +     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;
> > +     }
> > +
> > +     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)
> > --
> > 2.25.1
> >
>

Regards,
*Vedant Paranjape*