Closed Bug 986985 Opened 6 years ago Closed 6 years ago

PulseAudio: Server restart permanently breaks audio in Firefox

Categories

(Core :: Audio/Video, defect)

All
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: BenB, Assigned: arun)

Details

Attachments

(2 files)

Reproduction:
1. Start pulseaudio server
2. Start Firefox
3. Play some audio in Firefox
4. Kill pulseaudio server
5. Start pulseaudio server again
6. Play some audio in Firefox
7. Kill Firefox
8. Start Firefox
9. Play some audio in Firefox


Actual result:
Step 3: Audio plays
Step 6: No audio
Step 9: Audio plays

Expected result:
Step 3: Audio plays
Step 6: Audio plays
Step 9: Audio plays

Firefox notices that the Pulse server connection that it holds on to is gone, and re-opens a connection to the Pulse server, based on the now-current Pulse settings, which may be different from the time when Firefox started, so the new pulse server settings should be used automatically.

Regression:
I think this happens only since native PulseAudio support (bug 837563). Before, when Firefox used the ALSA API and the ALSA->pulse bridge sent the audio to Pulse, I don't think I saw this problem.
Assignee: nobody → kinetik
Clarification: (same as above)
Actual result:
Step 3: Audio plays
Step 6: No audio
Step 9: Audio plays
Firefox does not notice that the Pulse server connection that it holds on to is gone.

Expected result:
Step 3: Audio plays
Step 6: Audio plays
Step 9: Audio plays
Firefox notices that the Pulse server connection that it holds on to is gone, and re-opens a connection to the Pulse server, based on the now-current Pulse settings, which may be different from the time when Firefox started, so the new pulse server settings should be used automatically.
Do we want to actually handle this case (considering that PulseAudio going away is supposed to be an exception case)? If yes, is it something PulseAudio-specific, or might other backends also need some sort of reconnection logic?
Well, I'm not sure I know a way to kill, say, OSX or Window's audio subsystem. I've certainly seen them crash in practice, and Firefox would not play sound until it got restarted, for the exact same reason: some kind of connection (probably different across backends, might be pipe, might shm, etc.) got lost. 

This is not critical, but good to have. It means detecting that the server went away (probably catching an error in the pulse api on stream creation), and reinitializing the whole thing.
> considering that PulseAudio going away is supposed to be an exception case

PulseAudio is not like Windows or Mac OS X audio system, but it's a server process. And you're now connecting to that server directly. Before, when Firefox was using the ALSA->Pulse compat libs, these libs were connecting to the server, and they *did* handle the restart gracefully. Now that you connect directly, you have to take that responsibility of handling it.

More importantly, the server can be remote and the remote machine is restarted or whatever. This remote feature is not an edge case, but intrinsic in PulseAudio and one of the main advantages of Pulse. E.g. I have a HTPC connected to my stereo, while the desktop has no loudspeakers whatsoever.

You just have to deal with the fact that another process (potentially on another machine) is being restarted.
Tentatively taking, feel free to steal back if someone has time.
Assignee: kinetik → paul
> the server can be remote

FWIW, because Paul asked: This is configured by making a file
~/.pulse/client.conf with content "default-server = <hostname>"
This seem to works for me, but it's probably not thread safe yet.
I don't think we can reuse pulse_init() as is since that would leak a threaded mainloop. Here's one that I'd written up that only recreates the context (I thought you intended to do this at a higher level rather than in the pulse backend, which is why I didn't post this earlier).
(In reply to Arun Raghavan from comment #8)
> Created attachment 8399471 [details] [diff] [review]
> 0001-pulse-Reconnect-context-if-disconnected.patch

Oh, this one is only compile-tested, btw. I didn't have an FF build handy to test with.
Attachment #8399471 - Flags: review?(kinetik)
This patch works, and passes the tests locally.

I think this is thread safe, but the patch could use a second pair of eyes.
Attachment #8399471 - Flags: review?(kinetik) → review+
ditto. Thanks a lot to all involved! :-)
https://hg.mozilla.org/mozilla-central/rev/e89976cb4b4b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Assignee: paul → arun
You need to log in before you can comment on or make changes to this bug.