New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SDL_CloseAudio takes 2 seconds to return when using the Pulseaudio driver. #673

Closed

SDLBugzilla opened this issue Feb 10, 2021 · 5 comments

Labels

wontfix

Collaborator

SDLBugzilla commented Feb 10, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 1.2.14
Reported for operating system, platform: Linux, x86

Comments on the original bug report:

On 2012-03-15 17:20:57 +0000, wrote:

Created attachment 838
Testcase.

When SDL_CloseAudio is called on the pulse backend, a delay of around 2 seconds will sometimes occur (about 50% of the time). The rest of the time, the function returns almost immediately. A testcase is attached.

My OS is a fairly vanilla installation of Ubuntu 11.04.

On 2012-03-15 17:40:48 +0000, wrote:

Did a little more testing. Setting the audio driver to alsa rather than pulse has no effect on the bug, presumably because ALSA is emulated as a client of the PulseAudio server. However, setting it to esd removes the bug.

My system has a universal sound latency of around 500ms; I've always presumed it was some sort of driver issue. When terminating the loopwave test with Ctrl+C (using the esd driver), the program will stop running as soon as this latency is over (that is, as soon as sound stops coming out the speakers). Using the pulse and alsa drivers, loopwave will keep running for an additional 1 - 2 seconds beyond this point.

On 2012-03-19 15:43:22 +0000, wrote:

Decided to attempt to fix this bug myself. The delay occurs in SDL_pulseaudio.c, within the call to pa_mainloop_iterate around line 427. This call blocks waiting for completion of an asynchronous pa_stream_drain operation, initiated earlier in the same function.

This bug can be trivially solved by replacing the call to pa_stream_drain with a call to pa_stream_flush, which discards the data rather than waiting for a guarantee that it's finished.

On 2012-03-19 15:47:38 +0000, wrote:

PS: The above fix wouldn't resolve the same problem in the ALSA driver, when ALSA is being emulated on a Pulse system. I haven't tested this, but I believe the issue could be fixed in a similar way by replacing the call to snd_pcm_flush (in ALSA_CloseAudio) with snd_pcm_drop.

On 2012-03-19 16:37:25 +0000, wrote:

Created attachment 839
Patch to fix the delay when closing the ALSA and Pulse drivers.

On 2012-03-19 16:43:19 +0000, wrote:

Sorry for the comment spam, but I forgot to mention: The above patch was generated against the current SDL-1.2 branch of hg. Should also be relevant to SDL2, although I haven't checked that it will apply as-is.

On 2012-03-22 21:24:36 +0000, Ryan C. Gordon wrote:

Is dropping the last buffer of audio the correct approach, though? If you had a music player, should it cut off the last half second of a song so the process can quit quickly?

This bug is real, but I think the culprit is PulseAudio and not SDL:
  http://pulseaudio.org/ticket/866
We mentioned this in Bug # 1013, in regards to SDL_mixer, at one point.

The gist is that we need to tell PulseAudio "wait until you've definitely played what we asked you to and get back to us" and it waits way too long.

I suppose we could work around this in SDL by timing out how long it should take for Pulse to play all our data and just quitting at that point, but that's a goofy solution, too.

--ryan.

On 2012-03-23 15:36:48 +0000, wrote:

Am I right in thinking that patching library or application X to accommodate a difficult-to-fix or widely-deployed bug in library Y isn't usually done in FOSS circles, then? I mean, I'd personally go for the more pragmatic option of patching X, but I can see how that could turn X into a hackjob in the long run.

Philosophical issues aside, though, I'd say that dropping ~500ms of trailing audio will be less harmful in the general case than halting on shutdown for 2s. It's worth noting that without explicitly working around this bug by calling SDL_QuitSubSystem(SDL_INIT_VIDEO) before SDL_CloseAudio, SDL will always close down the audio subsystem before video; therefore, practically all SDL applications that use both audio and video (ie, games) will suffer from an ugly 2-second pause on closing, with an unresponsive window still visible to the user. I haven't tested this broadly, but I can at least confirm that it applies to Frets on Fire.

On 2012-08-06 08:35:35 +0000, Stas Sergeev wrote:

Me too on this bug.

I'd like to note that the problem does NOT depend
on how much audio was played and when. For me it
pauses for 3 seconds even if SDL_PauseAudio(0) was
never called at all, and the sound callback was never
called.
So I am not sure if the Comment 6 applies well.
Or is the PA bug so bad that it hangs even when
nothing was played?

On 2013-11-11 01:21:19 +0000, Sam Lantinga wrote:

Created attachment 1415
SDL 2.0 pulseaudio patch

I don't think we actually need to wait for the drain operation to complete. Can you try out this patch and see if it:
a) solves your issue
b) you actually hear all the audio from the application as it quits?

Thanks!

On 2015-02-16 19:04:15 +0000, Stas Sergeev wrote:

For me the problem no longer happens. I think
because of the new pulseaudio-5.0.
Tested with SDL-1.2.15 and SDL2-2.0.3 without
any patches - all good.

On 2015-08-25 09:38:23 +0000, Ryan C. Gordon wrote:

Hello, and sorry if you're getting several copies of this message by email, since we are closing many bugs at once here.

We have decided to mark all SDL 1.2-related bugs as RESOLVED ENDOFLIFE, as we don't intend to work on SDL 1.2 any further, but didn't want to mark a large quantity of bugs as RESOLVED WONTFIX, to clearly show what was left unattended to and make it easily searchable.

Our current focus is on SDL 2.0.

If you are still having problems with an ENDOFLIFE bug, your absolute best option is to move your program to SDL2, as it will likely fix the problem by default, and give you access to modern platforms and tons of super-cool new features.

Failing that, we will accept small patches to fix these issues, and put them in revision control, although we do not intend to do any further official 1.2 releases.

Failing that, please feel free to contact me directly by email (icculus@icculus.org) and we'll try to find some way to help you out of your situation.

Thank you,
--ryan.

The text was updated successfully, but these errors were encountered:

SDLBugzilla added bug endoflife labels

Feb 10, 2021

SDLBugzilla closed this as completed

Feb 10, 2021

sezero mentioned this issue

Feb 14, 2021

Reevaluate endoflife bugs #837

Closed

sezero removed the endoflife label

Feb 15, 2021

Collaborator

sezero commented Feb 15, 2021

@icculus: Can you see anything wrong with the pulseaudio patch?
(It does work for me (TM)..) Patch inlined below for convenience:

diff --git a/src/audio/pulse/SDL_pulseaudio.c b/src/audio/pulse/SDL_pulseaudio.c
index 29373f3..69e9f21 100644
--- a/src/audio/pulse/SDL_pulseaudio.c
+++ b/src/audio/pulse/SDL_pulseaudio.c
@@ -111,7 +111,7 @@ static pa_stream_state_t (*SDL_NAME(pa_stream_get_state))(pa_stream *s);
 static size_t (*SDL_NAME(pa_stream_writable_size))(pa_stream *s);
 static int (*SDL_NAME(pa_stream_write))(pa_stream *s, const void *data, size_t nbytes,
 	pa_free_cb_t free_cb, int64_t offset, pa_seek_mode_t seek);
-static pa_operation * (*SDL_NAME(pa_stream_drain))(pa_stream *s,
+static pa_operation * (*SDL_NAME(pa_stream_flush))(pa_stream *s,
 	pa_stream_success_cb_t cb, void *userdata);
 static int (*SDL_NAME(pa_stream_disconnect))(pa_stream *s);
 static void (*SDL_NAME(pa_stream_unref))(pa_stream *s);
@@ -162,8 +162,8 @@ static struct {
 		(void **)&SDL_NAME(pa_stream_writable_size)	},
 	{ "pa_stream_write",
 		(void **)&SDL_NAME(pa_stream_write)		},
-	{ "pa_stream_drain",
-		(void **)&SDL_NAME(pa_stream_drain)		},
+	{ "pa_stream_flush",
+		(void **)&SDL_NAME(pa_stream_flush)		},
 	{ "pa_stream_disconnect",
 		(void **)&SDL_NAME(pa_stream_disconnect)	},
 	{ "pa_stream_unref",
@@ -398,7 +398,7 @@ static void PULSE_SetCaption(_THIS, const char *str)
 	}
 }
 
-static void stream_drain_complete(pa_stream *s, int success, void *userdata)
+static void stream_flush_complete(pa_stream *s, int success, void *userdata)
 {
 	/* no-op. */
 }
@@ -407,7 +407,7 @@ static void PULSE_WaitDone(_THIS)
 {
 	pa_operation *o;
 
-	o = SDL_NAME(pa_stream_drain)(stream, stream_drain_complete, NULL);
+	o = SDL_NAME(pa_stream_flush)(stream, stream_flush_complete, NULL);
 	if (!o)
 		return;

sezero reopened this

Feb 15, 2021

Collaborator

icculus commented Feb 15, 2021

My understanding from the Pulse docs is that flush() drops any audio we fed to the device that hasn't been played yet, whereas drain() waits for it to finish playing.

Does this still hang for 2 seconds on modern PulseAudio installs? Someone mentioned a newer (now also old) Pulse fixed this.

Collaborator

sezero commented Feb 15, 2021

My understanding from the Pulse docs is that flush() drops any audio we fed to the device that hasn't been played yet, whereas drain() waits for it to finish playing.

Yes,

Does this still hang for 2 seconds on modern PulseAudio installs? Someone mentioned a newer (now also old) Pulse fixed this.

Tried on a newish (now old) Linux setup with pulseaudio-12.2, no delays.
My old CentOS-6.10 with pulseaudio-0.9.21 does have the annoying delay
and gets fixed by the patch.

Collaborator

icculus commented Feb 15, 2021

My concern is that if you have, say, a command line media player, and you play an mp3 file and once you've given all the audio to SDL, you close the device and terminate the app, the last half-second of audio isn't going to play with this change. I'd be inclined to mark this wontfix and leave the extra delay in older PulseAudio installs.

sezero added the wontfix label

Feb 15, 2021

Collaborator

sezero commented Feb 15, 2021

OK, closing as wontfix.

sezero closed this as completed

Feb 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SDL_CloseAudio takes 2 seconds to return when using the Pulseaudio driver. #673

SDL_CloseAudio takes 2 seconds to return when using the Pulseaudio driver. #673

SDLBugzilla commented Feb 10, 2021

sezero commented Feb 15, 2021

icculus commented Feb 15, 2021

sezero commented Feb 15, 2021

icculus commented Feb 15, 2021

sezero commented Feb 15, 2021