From 11626e48bfb1dbf265e7eba3777c77d5ab6bd72b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 26 Aug 2011 19:28:09 +0200 Subject: input/curl: implement a hard-coded timeout of 10 seconds Be sure to stop the operation at some point when the server isn't responding. --- src/input/curl_input_plugin.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/input/curl_input_plugin.c b/src/input/curl_input_plugin.c index d6424c2c6..2c4ac2ff8 100644 --- a/src/input/curl_input_plugin.c +++ b/src/input/curl_input_plugin.c @@ -680,6 +680,9 @@ input_curl_easy_init(struct input_curl *c, GError **error_r) curl_easy_setopt(c->easy, CURLOPT_MAXREDIRS, 5); curl_easy_setopt(c->easy, CURLOPT_FAILONERROR, true); curl_easy_setopt(c->easy, CURLOPT_ERRORBUFFER, c->error); + curl_easy_setopt(c->easy, CURLOPT_NOPROGRESS, 1l); + curl_easy_setopt(c->easy, CURLOPT_NOSIGNAL, 1l); + curl_easy_setopt(c->easy, CURLOPT_CONNECTTIMEOUT, 10l); if (proxy != NULL) curl_easy_setopt(c->easy, CURLOPT_PROXY, proxy); -- cgit v1.2.3 From 042c1abc6e1b0023a9f2964573102a332e598237 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 31 Aug 2011 20:58:36 +0200 Subject: output/pulse: use _delete_context() Eliminate duplicate code. --- src/output/pulse_output_plugin.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/output/pulse_output_plugin.c b/src/output/pulse_output_plugin.c index babb8e221..c09b6a6af 100644 --- a/src/output/pulse_output_plugin.c +++ b/src/output/pulse_output_plugin.c @@ -224,6 +224,20 @@ pulse_output_connect(struct pulse_output *po, GError **error_r) return true; } +/** + * Frees and clears the context. + */ +static void +pulse_output_delete_context(struct pulse_output *po) +{ + assert(po != NULL); + assert(po->context != NULL); + + pa_context_disconnect(po->context); + pa_context_unref(po->context); + po->context = NULL; +} + /** * Create, set up and connect a context. * @@ -249,28 +263,13 @@ pulse_output_setup_context(struct pulse_output *po, GError **error_r) pulse_output_subscribe_cb, po); if (!pulse_output_connect(po, error_r)) { - pa_context_unref(po->context); - po->context = NULL; + pulse_output_delete_context(po); return false; } return true; } -/** - * Frees and clears the context. - */ -static void -pulse_output_delete_context(struct pulse_output *po) -{ - assert(po != NULL); - assert(po->context != NULL); - - pa_context_disconnect(po->context); - pa_context_unref(po->context); - po->context = NULL; -} - static void * pulse_output_init(G_GNUC_UNUSED const struct audio_format *audio_format, const struct config_param *param, -- cgit v1.2.3 From e76c752987e0b943e0ee01d6e062e86befd9e8c5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 31 Aug 2011 21:00:55 +0200 Subject: output/pulse: add function _delete_stream() Merge common code. --- src/output/pulse_output_plugin.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/output/pulse_output_plugin.c b/src/output/pulse_output_plugin.c index c09b6a6af..8fcf26992 100644 --- a/src/output/pulse_output_plugin.c +++ b/src/output/pulse_output_plugin.c @@ -224,6 +224,20 @@ pulse_output_connect(struct pulse_output *po, GError **error_r) return true; } +/** + * Frees and clears the stream. + */ +static void +pulse_output_delete_stream(struct pulse_output *po) +{ + assert(po != NULL); + assert(po->stream != NULL); + + pa_stream_disconnect(po->stream); + pa_stream_unref(po->stream); + po->stream = NULL; +} + /** * Frees and clears the context. */ @@ -539,8 +553,7 @@ pulse_output_open(void *data, struct audio_format *audio_format, error = pa_stream_connect_playback(po->stream, po->sink, NULL, 0, NULL, NULL); if (error < 0) { - pa_stream_unref(po->stream); - po->stream = NULL; + pulse_output_delete_stream(po); g_set_error(error_r, pulse_output_quark(), 0, "pa_stream_connect_playback() has failed: %s", @@ -578,9 +591,7 @@ pulse_output_close(void *data) pulse_wait_for_operation(po->mainloop, o); } - pa_stream_disconnect(po->stream); - pa_stream_unref(po->stream); - po->stream = NULL; + pulse_output_delete_stream(po); if (po->context != NULL && pa_context_get_state(po->context) != PA_CONTEXT_READY) -- cgit v1.2.3 From 60f7ff3de594ef6b54a61b6ad630819ce026c760 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 31 Aug 2011 20:55:49 +0200 Subject: output/pulse: reset callbacks before closing stream/context Fixes assertion failure when a stream callback is invoked too late after a format change. --- src/output/pulse_output_plugin.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'src') diff --git a/src/output/pulse_output_plugin.c b/src/output/pulse_output_plugin.c index 8fcf26992..34d736546 100644 --- a/src/output/pulse_output_plugin.c +++ b/src/output/pulse_output_plugin.c @@ -233,6 +233,13 @@ pulse_output_delete_stream(struct pulse_output *po) assert(po != NULL); assert(po->stream != NULL); +#if PA_CHECK_VERSION(0,9,8) + pa_stream_set_suspended_callback(po->stream, NULL, NULL); +#endif + + pa_stream_set_state_callback(po->stream, NULL, NULL); + pa_stream_set_write_callback(po->stream, NULL, NULL); + pa_stream_disconnect(po->stream); pa_stream_unref(po->stream); po->stream = NULL; @@ -247,6 +254,9 @@ pulse_output_delete_context(struct pulse_output *po) assert(po != NULL); assert(po->context != NULL); + pa_context_set_state_callback(po->context, NULL, NULL); + pa_context_set_subscribe_callback(po->context, NULL, NULL); + pa_context_disconnect(po->context); pa_context_unref(po->context); po->context = NULL; -- cgit v1.2.3 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_all.c | 7 +++++-- src/output_control.c | 18 +++++++++++++----- src/output_control.h | 5 +++++ src/output_init.c | 1 + src/output_internal.h | 9 +++++++++ src/output_thread.c | 8 +------- 6 files changed, 34 insertions(+), 14 deletions(-) (limited to 'src') 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 @@ -109,6 +109,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 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; -- 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_all.c | 22 +++------------------- src/output_control.c | 12 ++++++++++++ src/output_control.h | 10 ++++++++-- 3 files changed, 23 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/output_all.c b/src/output_all.c index 551736a41..4e0b2eb22 100644 --- a/src/output_all.c +++ b/src/output_all.c @@ -205,30 +205,14 @@ static void audio_output_wait_all(void) notify_wait(&audio_output_client_notify); } -/** - * 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); -} - /** * Signals all audio outputs which are open. */ static void -audio_output_signal_all(void) +audio_output_allow_play_all(void) { for (unsigned i = 0; i < num_audio_outputs; ++i) - audio_output_lock_signal(&audio_outputs[i]); + audio_output_allow_play(&audio_outputs[i]); } static void @@ -533,7 +517,7 @@ audio_output_all_cancel(void) /* the audio outputs are now waiting for a signal, to synchronize the cleared music pipe */ - audio_output_signal_all(); + audio_output_allow_play_all(); /* invalidate elapsed_time */ 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) { diff --git a/src/output_control.h b/src/output_control.h index 2b88d4103..f0e317d6e 100644 --- a/src/output_control.h +++ b/src/output_control.h @@ -72,11 +72,17 @@ 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. + * asynchronously. To finish the operation, the caller has to call + * audio_output_allow_play(). */ void audio_output_cancel(struct audio_output *ao); +/** + * Set the "allow_play" and signal the thread. + */ +void +audio_output_allow_play(struct audio_output *ao); + void audio_output_close(struct audio_output *ao); /** -- cgit v1.2.3 From e7abdab58dd566d9b80fb5caf5dee867f184d913 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 1 Sep 2011 18:21:40 +0200 Subject: output/osx: signal the GCond while mutex is locked --- src/output/osx_plugin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/output/osx_plugin.c b/src/output/osx_plugin.c index ce82656bd..2c150fc41 100644 --- a/src/output/osx_plugin.c +++ b/src/output/osx_plugin.c @@ -143,8 +143,8 @@ osx_render(void *vdata, if (od->pos >= od->buffer_size) od->pos = 0; - g_mutex_unlock(od->mutex); g_cond_signal(od->condition); + g_mutex_unlock(od->mutex); buffer->mDataByteSize = buffer_size; -- cgit v1.2.3 From 596f36bb78425c8bd6aa4e9a81c796cb78b011c0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 1 Sep 2011 18:13:05 +0200 Subject: output/osx: don't drain the buffer when closing Eliminate an unnecessary source of deadlocks. --- src/output/osx_plugin.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'src') diff --git a/src/output/osx_plugin.c b/src/output/osx_plugin.c index 2c150fc41..7639f3bd9 100644 --- a/src/output/osx_plugin.c +++ b/src/output/osx_plugin.c @@ -95,12 +95,6 @@ static void osx_output_close(void *data) { struct osx_output *od = data; - g_mutex_lock(od->mutex); - while (od->len) { - g_cond_wait(od->condition, od->mutex); - } - g_mutex_unlock(od->mutex); - AudioOutputUnitStop(od->au); AudioUnitUninitialize(od->au); CloseComponent(od->au); -- cgit v1.2.3