| Message ID | 20231204212747.581968-1-nicolas@ndufresne.ca |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi, Le lundi 04 décembre 2023 à 16:27 -0500, Nicolas Dufresne via libcamera-devel a écrit : > From: Nicolas Dufresne <nicolas.dufresne@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/ 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. Nicolas > --- > 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)
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@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/ > > 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. 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. > > --- > > 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)
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@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/ > > > > 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 > > 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. > > > > --- > > > 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) >
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@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@ideasonboard.com> -- Kieran
Hi, Le mardi 02 janvier 2024 à 11:09 +0000, Kieran Bingham a écrit : > 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@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. I'll give it some further thought while making v2, but as said, we can split adding devenv from setting tests env initially. > > > > 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? I did think about this, but the workaround is something that just work without having to interact with meson. What would happen is that some people workflow would be affected. For this reason another subject that is best handled separately. Are you aware of known maintenance issue with the current approach ? > > > > > > > --- > > > > > 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@ideasonboard.com> Thank you, I'll refresh my memory and rebase locally and will soon come back with at least an updated readme, which I want for my self too. Nicolas > > -- > Kieran
Hi Nicolas, On Mon, Jan 08, 2024 at 10:42:06AM -0500, Nicolas Dufresne wrote: > Le mardi 02 janvier 2024 à 11:09 +0000, Kieran Bingham a écrit : > > Quoting Nicolas Dufresne via libcamera-devel (2023-12-05 20:18:47) > > > Le mardi 05 décembre 2023 à 18:19 +0200, Laurent Pinchart a écrit : > > > > 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@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. > > I'll give it some further thought while making v2, but as said, we can split > adding devenv from setting tests env initially. > > > > > 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? > > I did think about this, but the workaround is something that just work without > having to interact with meson. What would happen is that some people workflow > would be affected. For this reason another subject that is best handled > separately. Are you aware of known maintenance issue with the current approach ? Not really. I just think the current mechanism is a bit of a hack, and is likely fragile, I would be happy to drop it if we could. There's no urgency though. When it comes to running binaries from the build directory for quick testing, it's not a workflow we've committed to support in the same way forever. I think requiring usage of meson devenv would be reasonable, if it gives us all we need in a simple way. > > > > > > --- > > > > > > 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@ideasonboard.com> > > Thank you, I'll refresh my memory and rebase locally and will soon come back > with at least an updated readme, which I want for my self too.
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)
From: Nicolas Dufresne <nicolas.dufresne@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. --- src/gstreamer/meson.build | 7 +++++++ 1 file changed, 7 insertions(+)