From 8b0b4ff0860ea93850c2f44e72e8a8a5de05e13b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 1 Sep 2011 07:13:21 +0200 Subject: output_thread: reimplement CANCEL synchronization The output thread could hang indefinitely after finishing CANCEL, because it could have missed the signal while the output was not unlocked in ao_command_finished(). This patch removes the wait() call after CANCEL, and adds the flag "allow_play" instead. While this flag is set, playback is skipped. With this flag, there will not be any excess wait() call after the pipe has been cleared. This patch fixes a bug that causes mpd to discontinue playback after seeking, due to the race condition described above. --- src/output_control.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'src/output_control.c') diff --git a/src/output_control.c b/src/output_control.c index 0823b667b..14976dbfb 100644 --- a/src/output_control.c +++ b/src/output_control.c @@ -115,6 +115,8 @@ audio_output_open(struct audio_output *ao, { bool open; + assert(ao != NULL); + assert(ao->allow_play); assert(audio_format_valid(audio_format)); assert(mp != NULL); @@ -140,10 +142,6 @@ audio_output_open(struct audio_output *ao, /* we're not using audio_output_cancel() here, because that function is asynchronous */ ao_command(ao, AO_COMMAND_CANCEL); - - /* the audio output is now waiting for a - signal; wake it up immediately */ - g_cond_signal(ao->cond); } return true; @@ -181,6 +179,7 @@ static void audio_output_close_locked(struct audio_output *ao) { assert(ao != NULL); + assert(ao->allow_play); if (ao->mixer != NULL) mixer_auto_close(ao->mixer); @@ -223,6 +222,8 @@ audio_output_play(struct audio_output *ao) { g_mutex_lock(ao->mutex); + assert(ao->allow_play); + if (audio_output_is_open(ao)) g_cond_signal(ao->cond); @@ -238,6 +239,7 @@ void audio_output_pause(struct audio_output *ao) mixer_auto_close(ao->mixer); g_mutex_lock(ao->mutex); + assert(ao->allow_play); if (audio_output_is_open(ao)) ao_command_async(ao, AO_COMMAND_PAUSE); g_mutex_unlock(ao->mutex); @@ -247,6 +249,7 @@ void audio_output_drain_async(struct audio_output *ao) { g_mutex_lock(ao->mutex); + assert(ao->allow_play); if (audio_output_is_open(ao)) ao_command_async(ao, AO_COMMAND_DRAIN); g_mutex_unlock(ao->mutex); @@ -255,8 +258,12 @@ audio_output_drain_async(struct audio_output *ao) void audio_output_cancel(struct audio_output *ao) { g_mutex_lock(ao->mutex); - if (audio_output_is_open(ao)) + + if (audio_output_is_open(ao)) { + ao->allow_play = false; ao_command_async(ao, AO_COMMAND_CANCEL); + } + g_mutex_unlock(ao->mutex); } @@ -287,6 +294,7 @@ void audio_output_finish(struct audio_output *ao) if (ao->thread != NULL) { g_mutex_lock(ao->mutex); + assert(ao->allow_play); ao_command(ao, AO_COMMAND_KILL); g_mutex_unlock(ao->mutex); g_thread_join(ao->thread); -- cgit v1.2.3 From 2be6184c8d274a5b99cc2c8c86a7aebe46187320 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 1 Sep 2011 07:53:42 +0200 Subject: output_all: move _lock_signal() to output_control.c Better name, better documentation. --- src/output_control.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'src/output_control.c') diff --git a/src/output_control.c b/src/output_control.c index 14976dbfb..f8c5cd873 100644 --- a/src/output_control.c +++ b/src/output_control.c @@ -267,6 +267,18 @@ void audio_output_cancel(struct audio_output *ao) g_mutex_unlock(ao->mutex); } +void +audio_output_allow_play(struct audio_output *ao) +{ + g_mutex_lock(ao->mutex); + + ao->allow_play = true; + if (audio_output_is_open(ao)) + g_cond_signal(ao->cond); + + g_mutex_unlock(ao->mutex); +} + void audio_output_release(struct audio_output *ao) { -- cgit v1.2.3