Bug 41539

Summary: Fix PulseAudio 1.0 build for Solaris
Product: PulseAudio Reporter: Brian Cameron <brian.cameron>
Component: build-systemAssignee: pulseaudio-bugs
Status: RESOLVED FIXED QA Contact: pulseaudio-bugs
Severity: normal    
Priority: medium CC: colin, lennart, mkbosmans
Version: unspecified   
Hardware: Other   
OS: Solaris   
Whiteboard:
i915 platform: i915 features:
Attachments: Patch fixing issue
Updated patch

Description Brian Cameron 2011-10-06 14:25:19 UTC
Created attachment 52060 [details] [review]
Patch fixing issue

The Solaris linker requires that libraries and executables explicitly link against all libraries that it uses.  PulseAudio's pkg-config file does not link in pulsecommon or pulsecore which causes all programs which use PulseAudio to 
fail to link on Solaris.

Adding these two libraries to the "Libs:" (as in the attached patch) fixes this issue.  Adding these creates no problems for other distros, but makes it easier to build PulseAudio on Solaris.  If this patch could go upstream, that would be great.

The "Libs.private:" lines can obviously be removed from the pc.in files if the libraries are added to "Libs:", though I did not make this change in the attached patch.
Comment 1 Tanu Kaskinen 2011-10-07 15:37:27 UTC
I'm not an expert on linking stuff, but I fear that this will cause the libraries and executables that are linked against libpulse to become dependent on the exact version of libpulsecommon and libpulsecore that the linking was made against. But I'm pretty clueless here - if nobody else answers anything, please take this to the mailing list.

(Btw, any chance of sending the patches with "git send-email", or at least formatted with "git format-patch"?)
Comment 2 Arun Raghavan 2011-10-09 20:12:29 UTC
I don't think this we'd want to take this upstream. We don't want applications explicitly linking to libpulsecore and libpulsecommon on Linux and other platforms where the linker is capable of tracking library dependencies.

Leaving this open for now, in case Colin wants to weigh in.
Comment 3 Brian Cameron 2011-10-10 08:43:13 UTC
If the goal is to make it possible for programs to link against these libraries regardless of version number, then it seems odd to embed the version number in the name.  Wouldn't a more straightforward fix be to explicitly link against the libraries but not embed "-1.0" or other version numbers in their name?
Comment 4 Brian Cameron 2011-10-10 08:43:57 UTC
Or, I guess, to embed version numbers in the name, but only change those version numbers when you really want programs to need to relink?
Comment 5 Brian Cameron 2011-10-14 18:40:23 UTC
This bug (the fact that things that link against PulseAudio need to link against libpulsecore to resolve symbols) makes me realize that there is probably a more serious licensing issue here.  I just read the LICENSE file in PulseAudio and it says:

----

All PulseAudio source files are licensed under the GNU Lesser General Public
License. (see file LGPL for details)

However, the server side links to the GPL-only library 'libsamplerate' which
practically downgrades the license of the server part to GPL (see file GPL for
details), exercising section 3 of the LGPL.

Hence you should treat the client library ('libpulse') of PulseAudio as being
LGPL licensed and the server part ('libpulsecore') as being GPL licensed. Since
the PulseAudio daemon and the modules link to 'libpulsecore' they are of course
also GPL licensed.

----

Since it seems now that libpulse and libpulsecommon need symbols from libpulsecore, doesn't this mean that any program that links against Pulse must be a GPL'ed program.  I notice that even the GStreamer PulseAudio plugin needs to link against libpulsecore, which I guess makes all GStreamer based programs GPL.

Is this intentional, or do people just build PulseAudio without libsamplerate if they wish to avoid making everything that uses it GPL?  Or is the LICENSE file wrong?
Comment 6 Tanu Kaskinen 2011-10-15 03:09:15 UTC
(In reply to comment #3)
> If the goal is to make it possible for programs to link against these libraries
> regardless of version number, then it seems odd to embed the version number in
> the name.  Wouldn't a more straightforward fix be to explicitly link against
> the libraries but not embed "-1.0" or other version numbers in their name?

I don't know. Dropping the version number doesn't sound impossible to me, but as I said earlier, I'm not an expert here, and you'll likely get better answers on the mailing list.

(In reply to comment #5)
> Since it seems now that libpulse and libpulsecommon need symbols from
> libpulsecore, doesn't this mean that any program that links against Pulse must
> be a GPL'ed program.  I notice that even the GStreamer PulseAudio plugin needs
> to link against libpulsecore, which I guess makes all GStreamer based programs
> GPL.
> 
> Is this intentional, or do people just build PulseAudio without libsamplerate
> if they wish to avoid making everything that uses it GPL?  Or is the LICENSE
> file wrong?

Clients won't use libsamplerate in any case - resampling is done only at the server end. But the fact still is that if libsamplerate is enabled, both libpulsecore and libpulsecommon will be linked against it. Since the clients won't use libsamplerate in practice at all, and they can't know whether their code will be linked at runtime to a libpulsecore version that is built with libsamplerate support or without, I believe there's no practical possibility for Erik de Castro Lopo to win if he goes mad and sues some company making proprietary software that uses libpulse, claiming that they infringe the libsamplerate license. But to remove any doubt, I think it would be best to change Pulseaudio build system so that libsamplerate won't be linked against any libraries that are used from the client side.

Nowadays Pulseaudio supports also Speex resamplers, which I believe are a pretty good replacement for libsamplerate (the default resampler is currently a speex resampler). So if you want to play safe, you can disable libsamplerate from Solaris Pulseaudio builds without much problems. Or actually, Solaris users may have libsamplerate resamplers configured as the default resampler in /etc/pulse/daemon.conf... If those resamplers cease to be available, there will be some kind of a failure - maybe just an error message, but maybe pulseaudio will fail to start. I'm too lazy to look up the behavior right now. ...Nah, I'm not so lazy after all, I looked it up. Whenever a resampler is needed that isn't available, a warning will be printed to the daemon log and the "speex-float-3" resampler will be used instead.
Comment 7 Brian Cameron 2011-10-15 13:13:20 UTC
Thanks.  I think the license issue and the fact that the LICENSE file seems misleading is really a separate bug, so I filed bug #41539 about it.
Comment 8 Brian Cameron 2011-10-15 13:35:28 UTC
Oops, sorry I meant bug #41822.
Comment 9 Brian Cameron 2011-10-15 14:32:15 UTC
Great, I sent an email to pulseaudio-discuss about this issue and other Solaris build issues as you requested.

http://lists.freedesktop.org/archives/pulseaudio-discuss/2011-October/011753.html
Comment 10 Brian Cameron 2011-10-19 10:57:14 UTC
Created attachment 52533 [details] [review]
Updated patch


You are right.  After doing further research, the right fix is not to modify the pkg-config files to make things like against libpulsecore.  

Instead the issues seem to be caused by the fact that libpulsecommon includes
pulsecore/pstream.c, which includes pulsecore/core-scache.h to gain access to the PA_SCACHE_ENTRY_SIZE_MAX #define.

Moving this #define so it is in pulsecore/memchunk.h (a header already associated with libpulsecommon and also included by pulsecore/scache.h) and fixing pstream.c to include memchunk.h fixes this issue.

I am not sure if this is the best way to fix this issue.  If the PulseAudio maintainers want to rearrange the header files in some other way to avoid the libpulsecommon pstream.c file needing to include pulsecore/core-scache.h, that is obviously okay.

I also notice that src/Makefile needs to be updated so connect_stress links with libpulsecore, so this is also fixed in this patch.

This patch is pretty simple, and it makes PulseAudio link properly on Solaris so libpulsecore does not need to be linked into libpulsecommon.  Can this go upstream?
Comment 12 Colin Guthrie 2011-11-27 08:20:55 UTC
commit 5aedb9b7d3422b9d38a79500d9fbe3a813bc6050
Author: Maarten Bosmans <mkbosmans@gmail.com>
Date:   Tue Nov 22 16:09:18 2011 +0100

    pulsecore: Hardcode FRAME_SIZE_MAX_ALLOW

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.