[libcamera-devel] [PATCH] gstreamer: Add meson devenv support

[Kieran Bingham]

Quoting Nicolas Dufresne via libcamera-devel (2023-12-05 20:18:47)
> Hi,
> 
> Le mardi 05 décembre 2023 à 18:19 +0200, Laurent Pinchart a écrit :
> > Hi Nicolas,
> > 
> > Thank you for the patch.
> > 
> > On Tue, Dec 05, 2023 at 09:00:10AM -0500, Nicolas Dufresne via libcamera-devel wrote:
> > > Le lundi 04 décembre 2023 à 16:27 -0500, Nicolas Dufresne via libcamera-devel a écrit :
> > > > From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > > > 
> > > > This change to the build system will prepend the plugin build directory
> > > > to GST_PLUGIN_PATH environment. This makes the built plugin visible to
> > > > GStreamer inside meson devenv enabling uninstalled testing.
> > > 
> > > In case of positive feedback, I'd like to make a V2 that updates the
> > > documentation. Instead of:
> > > 
> > >   export GST_PLUGIN_PATH=$(pwd)/build/src/gstreamer
> > > 
> > > I'd document:
> > > 
> > >   meson devenv -C build/

Ack.

> > > 
> > > Devenv is nicer since it prepends to the path, it will also blend well if
> > > libcamera is made a subproject of another project using devenv (notably mesa
> > > and/or GStreamer). In cross-compilation, the env can be generated on the build
> > > computer, and used as a script on the target with help of the --dump option.
> > 
> > I didn't know about meson devenv, it seems to be an interesting feature.
> > We have a bit of code in libcamera and in the unit tests to explicitly
> > support running the libcamera test executables and the unit tests from
> > the build directory and find the correct resources. I would be happy to
> > remove that if it could be replaced by meson devenv.
> 
> Tests have their own "env" option, which does not need to match the devenv.
> 
> https://mesonbuild.com/Unit-tests.html

Hrm ... looks like that would need to be added to every test (at least
every test that needs it - but perhaps it's better to add it to all to
be consistent).

So we might have to make this env global to libcamera as a
libcamera_runtime_env or such.

Could be done later though ... Not this patch.

> > Related to this patch, we have the following code in
> > test/gstreamer/gstreamer_test.cpp:
> > 
> >       /*
> >        * 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;
> >       }
> > 
> > Do you think this could be dropped ? I haven't checked if the meson unit
> > tests are run in a similar environment as devenv though.
> 
> We can certainly drop this and just use the same env for devenv and the meson
> test. The registry cache is invalidate when the env changes and path order is
> respected.

Sounds like we could look to removing the isLibcameraInstalled()
DT_RUNPATH/DT_RPATH workaround too?


> > > > ---
> > > >  src/gstreamer/meson.build | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build
> > > > index 20784b71..3810a15b 100644
> > > > --- a/src/gstreamer/meson.build
> > > > +++ b/src/gstreamer/meson.build
> > > > @@ -46,3 +46,10 @@ libcamera_gst = shared_library('gstlibcamera',
> > > >      install : true,
> > > >      install_dir : '@0@/gstreamer-1.0'.format(get_option('libdir')),
> > > >  )
> > > > +
> > > > +# Makes the plugin visble to GStreamer inside meson devenv
> > > > +fs = import('fs')
> > > > +plugin_path = fs.parent(libcamera_gst.full_path())
> > > > +env = environment()
> > > > +env.prepend('GST_PLUGIN_PATH', fs.parent(plugin_path))
> > > > +meson.add_devenv(env)

 - Cc: Tomi - Is this something that should be added for Python too?

This patch does seem fairly self sufficient though, and otherwise
unobtrusive, (and seemingly beneficial).

With or without your proposed update to README.rst

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

--
Kieran