[libcamera-devel,v2,2/2] test: gstreamer: Use env instead of registry edit

19418 diff mbox series

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Instead of editing the registry, use gst_env variable provided by the plugin and
already used as part of the devenv shell. This reduces the complexity of the
C++ test code.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
New in V2

 test/gstreamer/gstreamer_test.cpp | 17 -----------------
 test/gstreamer/meson.build        |  2 +-
 2 files changed, 1 insertion(+), 18 deletions(-)

Hi Nicolas,

Thank you for the patch.

On Fri, Jan 19, 2024 at 03:08:48PM -0500, Nicolas Dufresne via libcamera-devel wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> Instead of editing the registry, use gst_env variable provided by the plugin and
> already used as part of the devenv shell. This reduces the complexity of the
> C++ test code.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

I like this :-)

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I'll run this through local and CI tests, and apply it if no issue is
found.

> ---
> New in V2
> 
>  test/gstreamer/gstreamer_test.cpp | 17 -----------------
>  test/gstreamer/meson.build        |  2 +-
>  2 files changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp
> index 091f7bf7..e8119b85 100644
> --- a/test/gstreamer/gstreamer_test.cpp
> +++ b/test/gstreamer/gstreamer_test.cpp
> @@ -51,23 +51,6 @@ GstreamerTest::GstreamerTest(unsigned int numStreams)
>  		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;
> -	}
> -
>  	/*
>  	 * Atleast one camera should be available with numStreams streams,
>  	 * otherwise skip the test entirely.
> diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build
> index a5c003b6..f3ba5a23 100644
> --- a/test/gstreamer/meson.build
> +++ b/test/gstreamer/meson.build
> @@ -17,5 +17,5 @@ foreach test : gstreamer_tests
>                       link_with : test_libraries,
>                       include_directories : test_includes_internal)
>  
> -    test(test['name'], exe, suite : 'gstreamer', is_parallel : false)
> +    test(test['name'], exe, suite : 'gstreamer', is_parallel : false, env : gst_env)
>  endforeach

Quoting Laurent Pinchart (2024-01-22 11:00:33)
> Hi Nicolas,
> 
> Thank you for the patch.
> 
> On Fri, Jan 19, 2024 at 03:08:48PM -0500, Nicolas Dufresne via libcamera-devel wrote:
> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > Instead of editing the registry, use gst_env variable provided by the plugin and
> > already used as part of the devenv shell. This reduces the complexity of the
> > C++ test code.
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> I like this :-)
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I'll run this through local and CI tests, and apply it if no issue is
> found.

Indeed, sounds pretty good - and we can use this as a more standard way
to handle things.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> 
> > ---
> > New in V2
> > 
> >  test/gstreamer/gstreamer_test.cpp | 17 -----------------
> >  test/gstreamer/meson.build        |  2 +-
> >  2 files changed, 1 insertion(+), 18 deletions(-)
> > 
> > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp
> > index 091f7bf7..e8119b85 100644
> > --- a/test/gstreamer/gstreamer_test.cpp
> > +++ b/test/gstreamer/gstreamer_test.cpp
> > @@ -51,23 +51,6 @@ GstreamerTest::GstreamerTest(unsigned int numStreams)
> >               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;
> > -     }
> > -
> >       /*
> >        * Atleast one camera should be available with numStreams streams,
> >        * otherwise skip the test entirely.
> > diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build
> > index a5c003b6..f3ba5a23 100644
> > --- a/test/gstreamer/meson.build
> > +++ b/test/gstreamer/meson.build
> > @@ -17,5 +17,5 @@ foreach test : gstreamer_tests
> >                       link_with : test_libraries,
> >                       include_directories : test_includes_internal)
> >  
> > -    test(test['name'], exe, suite : 'gstreamer', is_parallel : false)
> > +    test(test['name'], exe, suite : 'gstreamer', is_parallel : false, env : gst_env)
> >  endforeach
> 
> -- 
> Regards,
> 
> Laurent Pinchart