Commit 8b0b4ff0 authored by Max Kellermann's avatar Max Kellermann

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.
parent 60f7ff3d
......@@ -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
......
......@@ -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);
}
......
......@@ -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);
......
......@@ -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);
......
......@@ -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);
......
......@@ -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").
......
......@@ -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;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment