pulseaudio module depends on libX11
Hi there,
I have a dedicated headless system in my room which I use for audio listening. The system can only be remotely accessed via ssh; of course no X-server is running there and therefore no libX11 is available.
Now vlc-1.0.4 was building (and running) fine on the system, without any GUI of course. I'm using gentoo linux and the vlc-1.0.6 package was marked stable a while back, so I tried to upgrade from 1.0.4 to 1.0.6 - however this does not work, since the pulseaudio output plugin seems to have a mandatory dependency on libX11.
Please see the gentoo bugreport I wrote: http://bugs.gentoo.org/show_bug.cgi?id=319003
I've attached a full build log there, in case it's needed.
The main problem seems to be that libpulse_plugin.so is always linked with -lX11, even when not having enabled any sort of GUI.
Greets, Tobias
Activity
assigned to @Courmisch
added Component::Build system label
Could you test:
diff --git a/configure.ac b/configure.ac index 6e445f6..311d3c5 100644 --- a/configure.ac +++ b/configure.ac @@ -3661,8 +3661,9 @@ dnl dnl Pulseaudio module dnl PKG_ENABLE_MODULES_VLC([PULSE], [], [libpulse >= 0.9.11], [Pulseaudio suppor$ -VLC_ADD_LIBS([pulse], [${X_LIBS} ${X_PRE_LIBS} -lX11]) - +AS_IF([ test "$have_x" = yes],[ + VLC_ADD_LIBS([pulse], [${X_LIBS} ${X_PRE_LIBS} -lX11]) +]) dnl dnl Portaudio module dnlYou need to run ./bootstrap between patching and running configure
Patch doesn't apply to configure.ac from vlc-1.0.6, however this works for me:
+++ configure.ac.new 2010-05-22 15:44:57.000000000 +0200 @@ -4757,7 +4757,7 @@ PKG_CHECK_MODULES(PULSE, libpulse >= 0.9.11, [ VLC_ADD_PLUGIN([pulse]) VLC_ADD_CFLAGS([pulse],[${PULSE_CFLAGS}]) - VLC_ADD_LDFLAGS([pulse],[${PULSE_LIBS} ${X_LIBS} ${X_PRE_LIBS} -lX11])], + VLC_ADD_LDFLAGS([pulse],[${PULSE_LIBS} ${X_LIBS} ${X_PRE_LIBS}])], [AC_MSG_WARN(pulseaudio library not found)]) fiYou patch simply remove the -lX11 unconditionally and hence break for all the people using pulse under X.
So try the folliwing which should be the equivalent for 1.0.6 or try a live ebuild.
--- a/configure.ac +++ b/configure.ac @@ -4752,10 +4752,12 @@ AC_ARG_ENABLE(pulse, PKG_CHECK_MODULES(PULSE, libpulse >= 0.9.11, [ VLC_ADD_PLUGIN([pulse]) VLC_ADD_CFLAGS([pulse],[${PULSE_CFLAGS}]) - VLC_ADD_LDFLAGS([pulse],[${PULSE_LIBS} ${X_LIBS} ${X_PRE_LIBS} -lX11])], + VLC_ADD_LDFLAGS([pulse],[${PULSE_LIBS}]) + AS_IF([ test "$have_x" = yes],[ + VLC_ADD_LIBS([pulse], [${X_LIBS} ${X_PRE_LIBS} -lX11]) + ])], [AC_MSG_WARN(pulseaudio library not found)]) fi - dnl dnl Portaudio module dnl
Please get upstream to fix PulseAudio bug #799 (closed). Until then, this is a wontfix.
added Status::will not fix label
-
The VLC PulseAudio plugin needs to link against Xlib only because of http://www.pulseaudio.org/ticket/799 . If it weren't for this PulseAudio bug, the issue would be moot.
Replying to [comment:13 courmisch]:
This is a fair assumption inasmuch as we cannot determine at compile-time whether or not Xlib will be called at run-time (at least not that I know).
Even if it were possible, determining it at compile time wouldn't work as PA can change after vlc has been built. The previous solution was the most pragmatic one: leave it as a warning and let packagers set proper deps.
Anyway, the bug lies in libpulse not in VLC, and only a corner case is affected (VLC without X). So wontfix.
This is not future proof; what if $distro or a new libpulse fixes the bug? Moreover, it is rather sarcastic to force vlc's libpulse plugin to use libx11 when it's libpulse's x11 support that's broken :)
-
No. The most pragmatic solution is to force Xlib because that's how we ensure that users don't hit weird crashes. I don't expect packagers to grasp why Xlib is needed anyway (the mere fact that we are having this discussion underlines that point).
And it's not sarcastic. That's just how we make sure that XInitThreads() gets called early enough. The only alternative -except for fixing libpulse- is to link LibVLC against Xlib, which would be worse in every perspective. And it's completely future-proof: there is no harm in calling XInitThreads() if libpulse does not use Xlib.
Replying to [comment:15 courmisch]:
I don't expect packagers to grasp why Xlib is needed anyway (the mere fact that we are having this discussion underlines that point).
So you don't expect packagers to read warnings; interesting.
What you call a corner case is something we support on Gentoo; if the reporter is kind enough to attach a patch (mainly reverting your commit) to our bugzilla, I'll add it to our patchset. It's just a bit disappointing not to be able to find a compromise because of your obstinacy in forcing a workaround that is not always needed.
And it's completely future-proof: there is no harm in calling XInitThreads() if libpulse does not use Xlib.
I never meant it will break if it gets fixed, only that it will be useless.
-
Replying to [comment:16 aballier]:
So you don't expect packagers to read warnings; interesting.
I don't expect them to read warnings, because most of the time they don't. And I can't blame them given the length of the configure script output. Not to mention that VLC is often built automatically out of sight of the packager, such as with Debian buildd or some Gentoo user's emerge.
What you call a corner case is something we support on Gentoo;
That's your problem. libpulse has a bug that makes it impossible to support this scenario reliably in VLC. Hence VLC won't support it. If you really "supported" it, then you would already have fixed the -libpulse- bug and this issue would be moot.
if the reporter is kind enough to attach a patch
He wasted his time. I'm sorry for him. It's not my fault if PulseAudio is buggy, and won't accept the blame for it. Heck, I even filed the bug with PulseAudio upstream a long time ago myself.
(mainly reverting your commit) to our bugzilla, I'll add it to our patchset.
You do what you want. However, if you modify VLC, which is an interactive program, you need to clearly state it to the user. Given that this change will basically insert a bug in VLC for Gentoo, I am not going to ignore this requirement of the GPL. I would much rather you get Pulse properly fixed.
It's just a bit disappointing not to be able to find a compromise because of your obstinacy in forcing a workaround that is not always needed.
It's forced because there is no weaker but still safe way to determine when it's needed.
It's "a bit disappointing" that I get flamed for a PulseAudio bug.
And it's completely future-proof: there is no harm in calling XInitThreads() if libpulse does not use Xlib.
I never meant it will break if it gets fixed, only that it will be useless.
That's irrelevant. The work around is needed for those who use PulseAudio with X11, and will -hopefully- become useless indeed. But I don't know the PulseAudio guys plans, so I cannot do anything about that.
Replying to [comment:17 courmisch]:
That's your problem. libpulse has a bug that makes it impossible to support this scenario reliably in VLC. Hence VLC won't support it. If you really "supported" it, then you would already have fixed the -libpulse- bug and this issue would be moot.
I'm not a committer to PA upstream; fixing it in Gentoo won't change vlc and your opinion about this issue, will it?
(mainly reverting your commit) to our bugzilla, I'll add it to our patchset.
You do what you want. However, if you modify VLC, which is an interactive program, you need to clearly state it to the user. Given that this change will basically insert a bug in VLC for Gentoo, I am not going to ignore this requirement of the GPL.
If the combo pulseaudio with x11 and vlc without x11 is disallowed this inserts no bug...
I would much rather you get Pulse properly fixed.
Me too. You probably know best where XInitThreads() calls are needed in PA code; so if you could attach a patch (which would be much smaller than rewriting it to use xcb) to the PA bug I could grab it from there and fix it for us without having to wait for a new PA release; also it will probably help the bug to be fixed quicker at PA upstream.
It's just a bit disappointing not to be able to find a compromise because of your obstinacy in forcing a workaround that is not always needed.
It's forced because there is no weaker but still safe way to determine when it's needed.
For you as vlc upstream there is no weaker way, for me, there is one: http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/media-video/vlc/vlc-9999.ebuild?r1=1.73&r2=1.74
It's "a bit disappointing" that I get flamed for a PulseAudio bug.
Sorry if you understood it that way, it was definitely not my intention to flame you
-
Replying to [comment:18 aballier]:
I'm not a committer to PA upstream; fixing it in Gentoo won't change vlc and your opinion about this issue, will it?
If you fix it in Gentoo, then I have no problem with Gentoo VLC being hacked. Of course, VLC will continue to refer to upstream PA, unless we decide to "bundle" it in contribs, which seems unlikely.
If the combo pulseaudio with x11 and vlc without x11 is disallowed this inserts no bug...
Maybe the gentoo build system can express that "VLC without X11" cannot be used with "PulseAudio with X11", but I doubt it. I would rather expect that you have no way to prevent the user from shooting his own foot by rebuilding libpulse with different options, but not rebuilding VLC accordingly.
I would much rather you get Pulse properly fixed.
Me too. You probably know best where XInitThreads() calls are needed in PA code;
The XInitThreads() solution requires one call before any Xlib call. In practice, it may be sufficient to put one and check errors before each XOpenDisplay().
so if you could attach a patch (which would be much smaller than rewriting it to use xcb) to the PA bug I could grab it from there and fix it for us without having to wait for a new PA release; also it will probably help the bug to be fixed quicker at PA upstream.
That's doable. But while it will fix the race condition for libpulse inside VLC, it will introduce race conditions for any non-threaded Xlib-based app that uses libpulse. Hence I would expect both usptream and Gentoo to reject it.
The only sane fix is to switch to XCB (or rewrite the X11 proto directly or remove the X11 plugin altogether). Xlib is crap and it's not going to change.
