After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 623710 - [frei0r] Load frei0r plugins in /usr/lib64/frei0r-1 too
[frei0r] Load frei0r plugins in /usr/lib64/frei0r-1 too
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal blocker
: 0.10.20
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-07-06 21:58 UTC by Olivier Crête
Modified: 2010-08-08 09:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch against git bad (1.67 KB, patch)
2010-07-06 21:58 UTC, Olivier Crête
none Details | Review
updated patch (1.67 KB, patch)
2010-07-06 22:51 UTC, Olivier Crête
none Details | Review
really updated patch (1.57 KB, patch)
2010-07-06 23:53 UTC, Olivier Crête
committed Details | Review

Description Olivier Crête 2010-07-06 21:58:38 UTC
Created attachment 165384 [details] [review]
patch against git bad

The current frei0r plugin fails to detect the frei0r plugins on Fedora where they are installed in /usr/lib64. The attached patch makes it look in more places. Also, the handling of the HOME variable was very wrong.
Comment 1 Tim-Philipp Müller 2010-07-06 22:31:12 UTC
> The current frei0r plugin fails to detect the frei0r plugins on Fedora where
> they are installed in /usr/lib64. The attached patch makes it look in more
> places.

Ok. It's not entirely obvious to me though that it should be loading plugins in /usr/* before it loads plugins in $prefix, or that we should automatically be looking in other prefixes than the one from the frei0r.pc file.
 

> Also, the handling of the HOME variable was very wrong.

Could you elaborate in what way(s) it was wrong?
Comment 2 Olivier Crête 2010-07-06 22:51:07 UTC
Created attachment 165387 [details] [review]
updated patch

ah sorry, I failed to read the doc about the path component thing in the env var (kind of a strange way to do it). Updated patch.
Comment 3 Tim-Philipp Müller 2010-07-06 23:19:06 UTC
> Created an attachment (id=165387) [details] [review]
> updated patch
> 
> ah sorry, I failed to read the doc about the path component thing in the env
> var (kind of a strange way to do it). Updated patch.

Isn't that the exact same patch as the initial patch? :)

There's a reason the env var stuff is done that way. These are criteria by which the core decides whether a cached plugin needs to be re-scanned and updated. This needs to be done when either the environment variable changes (e.g. HOME), or any files or directories in the directory referenced by HOME change. So if you just passed "HOME", then the core would recursively scan your entire home directory on start-up to figure out if it needs to re-introspect that plugin or not. Which isn't a good idea, obviously.
Comment 4 Olivier Crête 2010-07-06 23:53:35 UTC
Created attachment 165392 [details] [review]
really updated patch
Comment 5 Sebastian Dröge (slomo) 2010-07-07 05:42:59 UTC
And there's no problem trying to dlopen() a 32 bit lib on a 64 bit system and the other way around?
Comment 6 Olivier Crête 2010-07-07 14:26:50 UTC
dlopen will just fail and return and error and that plugin will be ignored. I wish there was something in autoconf/configure to find what the standard library directory is, but I couldn't find anything.
Comment 7 Sebastian Dröge (slomo) 2010-07-10 13:55:49 UTC
Ok, well, let's commit this for now but add a FIXME comment please. I don't think autoconf has anything to detect the insane multiarch library locations...
Comment 8 Tim-Philipp Müller 2010-07-10 16:03:13 UTC
Why do we need to cater for random other locations than $prefix and pkg-config --variable=libdir frei0r ? This should be handled via an environment variable IMHO, if needed at all.
Comment 9 Olivier Crête 2010-07-11 03:35:56 UTC
(In reply to comment #8)
> Why do we need to cater for random other locations than $prefix and pkg-config
> --variable=libdir frei0r ? This should be handled via an environment variable
> IMHO, if needed at all.

frei0r is not a library (just a specification) so it doesn't have a pkgconfig file.. and $prefix is /usr, so its doesn't tell us the subdirectory for libraries (I guess that's libdir).
Comment 10 Tim-Philipp Müller 2010-07-11 08:25:08 UTC
Right, I meant variables based on $prefix such as $libdir. (We can prevent people from doing 'make install libdir=xyz' and require this to be set via configure so it's available at build time, as we do already in core and base, see e.g. gst/Makefile.am, check-libexecdir-consistency target).

I seem to have a frei0r.pc file (frei0r-plugins-dev package in debian/sid) and assumed it came from upstream, but didn't check.

Anyway, I don't have any strong opinions on this, just commenting from the sidelines.
Comment 11 Sebastian Dröge (slomo) 2010-07-11 08:35:02 UTC
The frei0r.pc file doesn't help much here (our frei0r plugin doesn't depend on an installed frei0r package btw, we have a copy of the frei0r.h interface definition), the frei0r spec says[0] that these locations should be used in this order:

(1) /usr/lib/frei0r-1/<vendor>
(2) /usr/local/lib/frei0r-1/<vendor>
(3) $HOME/.frei0r-1/lib/<vendor>

Of course this doesn't work for any multiarch hacks, and we can't simply take $(libdir) because it's only (1) or (2) or something completely different but we would need to search in all 3 directories.

I think Olivier's approach to just search in all lib, lib32 and lib64 directories is the only good approach that is compatible to the frei0r spec and works with multiarch.

[0] http://www.piksel.org/frei0r/1.1/spec/group__pluglocations.html
Comment 12 Tim-Philipp Müller 2010-08-06 01:01:50 UTC
If you want to get this in for the release, now would be a good time to push it..
Comment 13 Olivier Crête 2010-08-06 01:21:26 UTC
Fixed:

commit 3579c1164b6420e2aef31028ef09230e3d33925d
Author: Olivier Crête <olivier.crete@collabora.co.uk>
Date:   Fri Aug 6 03:19:16 2010 +0200

    frei0r: Load plugins in /usr/{local/,}lib{32,64}/frei0r-1 too
    
    Loads the plugins in more paths where they could be installed by
    multilib distributions.
    
    Fixes #623710
Comment 14 Tim-Philipp Müller 2010-08-06 09:16:12 UTC
Hrm, this causes crashes for me on 64-bit debian sid, where the same plugins are installed both in /usr/lib/frei0r and /usr/lib64/frei0r.

Program received signal SIGSEGV, Segmentation fault.
__strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31
31	../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.
	in ../sysdeps/x86_64/multiarch/../strlen.S
(gdb) bt
  • #0 __strlen_sse2
    at ../sysdeps/x86_64/multiarch/../strlen.S line 31
  • #1 IA__g_ascii_strdown
    at /tmp/buildd/glib2.0-2.24.1/glib/gstrfuncs.c line 1728
  • #2 gst_frei0r_klass_install_properties
    at gstfrei0r.c line 75
  • #3 gst_frei0r_filter_class_init
    at gstfrei0rfilter.c line 192
  • #4 type_class_init_Wm
    at /tmp/buildd/glib2.0-2.24.1/gobject/gtype.c line 2210
  • #5 IA__g_type_class_ref
    at /tmp/buildd/glib2.0-2.24.1/gobject/gtype.c line 2909
  • #6 get_object_types
    at gst-plugins-bad-plugins-scan.c line 228
  • #7 main
    at gst-plugins-bad-plugins-scan.c line 286
  • #2 gst_frei0r_klass_install_properties
    at gstfrei0r.c line 75
$1 = (f0r_param_info_t *) 0x136dc90
(gdb) print *param_info
$2 = {name = 0x382d465455 <Address 0x382d465455 out of bounds>, type = 1, explanation = 0x382d465455 <Address 0x382d465455 out of bounds>}
(gdb) print param_info->name
$3 = 0x382d465455 <Address 0x382d465455 out of bounds>
Comment 15 Sebastian Dröge (slomo) 2010-08-06 09:23:56 UTC
It's maybe not a good idea to load the same so from two different locations ;) Especially because the frei0r spec says that frei0r_init() of the plugins must only be called once.

Checking the filename is probably not enough to prevent this...
Comment 16 Sebastian Dröge (slomo) 2010-08-07 07:49:06 UTC
This also breaks the docs build of -bad.

Any suggestions how we could fix this? Maybe for now a check of the filename might be enough but it could break later again in theory...

This would only need a static GHashTable or GList in gstfrei0r.c to keep the already handled filenames.
Comment 17 Sebastian Dröge (slomo) 2010-08-08 09:58:32 UTC
commit 1e3ec9e3bd798c7d9c30e328702a293ff53ec344
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Sun Aug 8 11:56:42 2010 +0200

    frei0r: Don't try to register/load the same frei0r plugin at different locations twice
    
    This could happen because for example /usr/lib is linked
    to /usr/lib64 and both are loaded. The frei0r specification
    says that the plugin init function must only be called once
    and for some plugin weird things (including crashes) are
    happening.
    
    Fixes bug #623710.