Bug 439847 - [PATCH] Please update PulseAudio driver
Summary: [PATCH] Please update PulseAudio driver
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: SDL
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Thomas Woerner
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 343911 376981
TreeView+ depends on / blocked
 
Reported: 2008-03-31 18:28 UTC by Lennart Poettering
Modified: 2008-04-08 14:47 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-04-07 16:02:29 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Lennart Poettering 2008-03-31 18:28:15 UTC
Please merge this patch I just prepared for SDL, which fixes the buffering in
SDL and thus makes the driver work with all games I tested.

If you merge this please make sure to drop SDL-1.2.12-preferalsa.patch.

http://0pointer.de/public/sdl-pulse-rework.patch

This patch:
- fixes buffering (i.e. reduces number of "fragments" to 2, doesn't defer
filling up of the buffer until the entire buffer ran completely empty.
- Drops $PASERVER and $PADEVICE env var support, since this is a duplication of
$PULSE_SERVER and $PULSE_SINK which the PA libs honor anyway.

I believe this is safe for F9 given the bad quality of the existing driver.
Also, the changes are minimal.

Comment 1 Hans de Goede 2008-03-31 18:33:43 UTC
SDL_sound is an SDL add on library to add support for certain sound formats. The
pulse audiodriver is support of the main SDL package, changing component and
reassigning.

Thomas, I think we _really_ want this for F-9.



Comment 2 Lennart Poettering 2008-03-31 19:39:17 UTC
BTW, posted upstream as http://bugzilla.libsdl.org/show_bug.cgi?id=572

Comment 3 Lennart Poettering 2008-04-01 18:38:52 UTC
Anyone objects if I commit this to Rawhide (of course, assuming the ACLs are
properly set)?

Comment 4 Matthias Clasen 2008-04-02 15:08:42 UTC
Please do.

Comment 5 Thomas Woerner 2008-04-02 16:49:06 UTC
Wait with this.

I have some questions about this:

- Why do you want to remove the preferalsa patch?
- Is possible that this patch is breaking any application?
- Why are you removing $PASERVER and $PADEVICE? Are these variables not in use
at all?


Comment 6 Lennart Poettering 2008-04-02 21:15:16 UTC
(In reply to comment #5)
> Wait with this.
> 
> I have some questions about this:
> 
> - Why do you want to remove the preferalsa patch?

Good question. I am stupid. Ignore this request.

> - Is possible that this patch is breaking any application?

Possible is everything. However, the current SDL driver is very buggy and the
new one is afaics correct. Hence even if the new driver might not be perfect, it
will certainly "unbreak" a lot of apps. Also notice that right now we where
using the ESD driver of SDL to connect to PA because the PA backend for SDL was
as buggy as it was.

So in summary: afaics this very unlikely to break any app.

> - Why are you removing $PASERVER and $PADEVICE? Are these variables not in use
> at all?

As far as I know nobody uses them. They are specific to the old SDL driver for
PA that noone used anyway because it was so buggy. Now, PA has env vars like
that anyway, and they work for all apps, not just SDL, hence to avoid confusion
and for the sake of removing redundant code I removed it with this patch.


Comment 7 Thomas Woerner 2008-04-07 16:02:29 UTC
Fixed in rawhide in package SDL-1.2.13-3.fc9.

Comment 8 Kevin Kofler 2008-04-08 12:17:42 UTC
Shouldn't SDL_pulseaudio_hack.sh be changed to use the native pulse driver 
instead of esd now?

Comment 9 Julian Sikorski 2008-04-08 12:32:18 UTC
I think it should. Also, wouldn't it be better to make it rely on pulseaudio
itself, rather than alsa plugin?

Comment 10 Kevin Kofler 2008-04-08 12:37:37 UTC
That's already the case.

Comment 11 Julian Sikorski 2008-04-08 12:42:08 UTC
Is it? [ -e /usr/lib/alsa-lib/libasound_module_pcm_pulse.so -o -e
/usr/lib64/alsa-lib/libasound_module_pcm_pulse.so ] && export
SDL_AUDIODRIVER=esd seems to check for alsa plugin, not pulseaudio.

Comment 12 Julian Sikorski 2008-04-08 12:45:38 UTC
I think that the best solution would be to drop the hack altogether and just
modify the preference so that pulse is chosen over alsa if the daemon is running.

Comment 13 Kevin Kofler 2008-04-08 12:48:10 UTC
The assumption there is that the ALSA plugin is installed if and only if 
PulseAudio is running. This used to be mostly true because the ALSA plugin made 
ALSA completely unusable if it was installed and PA was not actually 
running. :-) Now that AFAIK this has been fixed, this assumption is probably 
not so true anymore.

Comment 14 Julian Sikorski 2008-04-08 14:47:50 UTC
(In reply to comment #13)
> The assumption there is that the ALSA plugin is installed if and only if 
> PulseAudio is running.
The problem here is that the reverse is not true - one may have PA running
without alsa plugin installed. I use such configureation, sicnce alsa plugin
refuses to work with some apps I use, namely skype and etqw. Well, I guess I can
just set an environmental variable.


Note You need to log in before you can comment on or make changes to this bug.