aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kellermann <max@duempel.org>2011-09-01 07:13:21 +0200
committerMax Kellermann <max@duempel.org>2011-09-01 07:13:21 +0200
commit8b0b4ff0860ea93850c2f44e72e8a8a5de05e13b (patch)
treeda2adaaa75382333d934869b530ae416a922baf1
parent60f7ff3de594ef6b54a61b6ad630819ce026c760 (diff)
downloadmpd-8b0b4ff0860ea93850c2f44e72e8a8a5de05e13b.tar.gz
mpd-8b0b4ff0860ea93850c2f44e72e8a8a5de05e13b.tar.xz
mpd-8b0b4ff0860ea93850c2f44e72e8a8a5de05e13b.zip
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.
-rw-r--r--NEWS1
-rw-r--r--src/output_all.c7
-rw-r--r--src/output_control.c18
-rw-r--r--src/output_control.h5
-rw-r--r--src/output_init.c1
-rw-r--r--src/output_internal.h9
-rw-r--r--src/output_thread.c8
7 files changed, 35 insertions, 14 deletions
diff --git a/NEWS b/NEWS
index 0bf08d0ab..56a846582 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,7 @@ ver 0.16.4 (2011/??/??)
* fix memory leaks
* don't resume playback when seeking to another song while paused
* apply follow_inside_symlinks to absolute symlinks
+* fix playback discontinuation after seeking
* input:
- curl: limit the receive buffer size
- curl: implement a hard-coded timeout of 10 seconds
diff --git a/src/output_all.c b/src/output_all.c
index 19c0f0166..551736a41 100644
--- a/src/output_all.c
+++ b/src/output_all.c
@@ -206,15 +206,18 @@ static void audio_output_wait_all(void)
}
/**
- * Signals the audio output if it is open. This function locks the
- * mutex.
+ * Signal the audio output if it is open, and set the "allow_play"
+ * flag. This function locks the mutex.
*/
static void
audio_output_lock_signal(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);
}
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);
diff --git a/src/output_control.h b/src/output_control.h
index 7f4f4a53c..2b88d4103 100644
--- a/src/output_control.h
+++ b/src/output_control.h
@@ -70,6 +70,11 @@ void audio_output_pause(struct audio_output *ao);
void
audio_output_drain_async(struct audio_output *ao);
+/**
+ * Clear the "allow_play" flag and send the "CANCEL" command
+ * asynchronously. To finish the operation, the caller has to set the
+ * "allow_play" flag and signal the thread.
+ */
void audio_output_cancel(struct audio_output *ao);
void audio_output_close(struct audio_output *ao);
diff --git a/src/output_init.c b/src/output_init.c
index f4700dfb2..96f87f512 100644
--- a/src/output_init.c
+++ b/src/output_init.c
@@ -189,6 +189,7 @@ audio_output_init(struct audio_output *ao, const struct config_param *param,
ao->really_enabled = false;
ao->open = false;
ao->pause = false;
+ ao->allow_play = true;
ao->fail_timer = NULL;
pcm_buffer_init(&ao->cross_fade_buffer);
diff --git a/src/output_internal.h b/src/output_internal.h
index 18d431352..7102ea5cd 100644
--- a/src/output_internal.h
+++ b/src/output_internal.h
@@ -110,6 +110,15 @@ struct audio_output {
bool pause;
/**
+ * When this flag is set, the output thread will not do any
+ * playback. It will wait until the flag is cleared.
+ *
+ * This is used to synchronize the "clear" operation on the
+ * shared music pipe during the CANCEL command.
+ */
+ bool allow_play;
+
+ /**
* If not NULL, the device has failed, and this timer is used
* to estimate how long it should stay disabled (unless
* explicitly reopened with "play").
diff --git a/src/output_thread.c b/src/output_thread.c
index 2c2b8d116..bf56ca971 100644
--- a/src/output_thread.c
+++ b/src/output_thread.c
@@ -648,12 +648,6 @@ static gpointer audio_output_task(gpointer arg)
}
ao_command_finished(ao);
-
- /* the player thread will now clear our music
- pipe - wait for a notify, to give it some
- time */
- if (ao->command == AO_COMMAND_NONE)
- g_cond_wait(ao->cond, ao->mutex);
continue;
case AO_COMMAND_KILL:
@@ -663,7 +657,7 @@ static gpointer audio_output_task(gpointer arg)
return NULL;
}
- if (ao->open && ao_play(ao))
+ if (ao->open && ao->allow_play && ao_play(ao))
/* don't wait for an event if there are more
chunks in the pipe */
continue;