oysteikt
/
mpd
0
0
Fork 0
Code Issues Pull Requests Releases Activity

output: consistently lock audio output objects 

Always keep the audio_output object locked within the output thread,
unless a plugin method is called.  This fixes several race conditions.
This commit is contained in:
Max Kellermann 2009-10-29 17:06:40 +01:00
parent 1403172ef3
commit bde3d14339
7 changed files with 105 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
NEWS
View File

@ -36,6 +36,7 @@ ver 0.16 (20??/??/??)
- jack: connect to server on MPD startup
- wildcards allowed in audio_format configuration
- alsa: don't recover on CANCEL
- consistently lock audio output objects
* mixers:
- removed support for legacy mixer configuration
- reimplemented software volume as mixer+filter plugin

26
src/output_all.c
View File

@ -180,10 +180,18 @@ audio_output_all_enable_disable(void)
static bool
audio_output_all_finished(void)
{
for (unsigned i = 0; i < num_audio_outputs; ++i)
if (audio_output_is_open(&audio_outputs[i]) &&
!audio_output_command_is_finished(&audio_outputs[i]))
for (unsigned i = 0; i < num_audio_outputs; ++i) {
struct audio_output *ao = &audio_outputs[i];
bool not_finished;
g_mutex_lock(ao->mutex);
not_finished = audio_output_is_open(ao) &&
!audio_output_command_is_finished(ao);
g_mutex_unlock(ao->mutex);
if (not_finished)
return false;
}
return true;
}
@ -261,8 +269,7 @@ audio_output_all_play(struct music_chunk *chunk)
music_pipe_push(g_mp, chunk);
for (i = 0; i < num_audio_outputs; ++i)
if (audio_output_is_open(&audio_outputs[i]))
audio_output_play(&audio_outputs[i]);
audio_output_play(&audio_outputs[i]);
return true;
}
@ -454,8 +461,7 @@ audio_output_all_pause(void)
audio_output_all_update();
for (i = 0; i < num_audio_outputs; ++i)
if (audio_output_is_open(&audio_outputs[i]))
audio_output_pause(&audio_outputs[i]);
audio_output_pause(&audio_outputs[i]);
audio_output_wait_all();
}
@ -467,10 +473,8 @@ audio_output_all_cancel(void)
/* send the cancel() command to all audio outputs */
for (i = 0; i < num_audio_outputs; ++i) {
if (audio_output_is_open(&audio_outputs[i]))
audio_output_cancel(&audio_outputs[i]);
}
for (i = 0; i < num_audio_outputs; ++i)
audio_output_cancel(&audio_outputs[i]);
audio_output_wait_all();

51
src/output_control.c
View File

@ -24,6 +24,7 @@
#include "mixer_control.h"
#include "mixer_plugin.h"
#include "filter_plugin.h"
#include "notify.h"
#include <assert.h>
#include <stdlib.h>
@ -39,8 +40,10 @@ struct notify audio_output_client_notify;
static void ao_command_wait(struct audio_output *ao)
{
while (ao->command != AO_COMMAND_NONE) {
notify_signal(&ao->notify);
g_cond_signal(ao->cond);
g_mutex_unlock(ao->mutex);
notify_wait(&audio_output_client_notify);
g_mutex_lock(ao->mutex);
}
}
@ -56,7 +59,7 @@ static void ao_command_async(struct audio_output *ao,
{
assert(ao->command == AO_COMMAND_NONE);
ao->command = cmd;
notify_signal(&ao->notify);
g_cond_signal(ao->cond);
}
void
@ -74,7 +77,9 @@ audio_output_enable(struct audio_output *ao)
audio_output_thread_start(ao);
}
g_mutex_lock(ao->mutex);
ao_command(ao, AO_COMMAND_ENABLE);
g_mutex_unlock(ao->mutex);
}
void
@ -91,7 +96,9 @@ audio_output_disable(struct audio_output *ao)
return;
}
g_mutex_lock(ao->mutex);
ao_command(ao, AO_COMMAND_DISABLE);
g_mutex_unlock(ao->mutex);
}
static bool
@ -157,23 +164,31 @@ audio_output_update(struct audio_output *ao,
{
assert(mp != NULL);
g_mutex_lock(ao->mutex);
if (ao->enabled && ao->really_enabled) {
if (ao->fail_timer == NULL ||
g_timer_elapsed(ao->fail_timer, NULL) > REOPEN_AFTER)
return audio_output_open(ao, audio_format, mp);
g_timer_elapsed(ao->fail_timer, NULL) > REOPEN_AFTER) {
bool success = audio_output_open(ao, audio_format, mp);
g_mutex_unlock(ao->mutex);
return success;
}
} else if (audio_output_is_open(ao))
audio_output_close(ao);
g_mutex_unlock(ao->mutex);
return false;
}
void
audio_output_play(struct audio_output *ao)
{
if (!ao->open)
return;
g_mutex_lock(ao->mutex);
notify_signal(&ao->notify);
if (audio_output_is_open(ao))
g_cond_signal(ao->cond);
g_mutex_unlock(ao->mutex);
}
void audio_output_pause(struct audio_output *ao)
@ -184,27 +199,37 @@ void audio_output_pause(struct audio_output *ao)
mixer_auto_close()) */
mixer_auto_close(ao->mixer);
ao_command_async(ao, AO_COMMAND_PAUSE);
g_mutex_lock(ao->mutex);
if (audio_output_is_open(ao))
ao_command_async(ao, AO_COMMAND_PAUSE);
g_mutex_unlock(ao->mutex);
}
void audio_output_cancel(struct audio_output *ao)
{
ao_command_async(ao, AO_COMMAND_CANCEL);
g_mutex_lock(ao->mutex);
if (audio_output_is_open(ao))
ao_command_async(ao, AO_COMMAND_CANCEL);
g_mutex_unlock(ao->mutex);
}
void audio_output_close(struct audio_output *ao)
{
assert(!ao->open || ao->fail_timer == NULL);
if (ao->mixer != NULL)
mixer_auto_close(ao->mixer);
g_mutex_lock(ao->mutex);
assert(!ao->open || ao->fail_timer == NULL);
if (ao->open)
ao_command(ao, AO_COMMAND_CLOSE);
else if (ao->fail_timer != NULL) {
g_timer_destroy(ao->fail_timer);
ao->fail_timer = NULL;
}
g_mutex_unlock(ao->mutex);
}
void audio_output_finish(struct audio_output *ao)
@ -214,7 +239,9 @@ void audio_output_finish(struct audio_output *ao)
assert(ao->fail_timer == NULL);
if (ao->thread != NULL) {
g_mutex_lock(ao->mutex);
ao_command(ao, AO_COMMAND_KILL);
g_mutex_unlock(ao->mutex);
g_thread_join(ao->thread);
}
@ -223,7 +250,7 @@ void audio_output_finish(struct audio_output *ao)
ao_plugin_finish(ao->plugin, ao->data);
notify_deinit(&ao->notify);
g_cond_free(ao->cond);
g_mutex_free(ao->mutex);
filter_free(ao->filter);

2
src/output_init.c
View File

@ -191,9 +191,9 @@ audio_output_init(struct audio_output *ao, const struct config_param *param,
assert(ao->filter != NULL);
ao->thread = NULL;
notify_init(&ao->notify);
ao->command = AO_COMMAND_NONE;
ao->mutex = g_mutex_new();
ao->cond = g_cond_new();
ao->data = ao_plugin_init(plugin,
&ao->config_audio_format,

14
src/output_internal.h
View File

@ -21,7 +21,8 @@
#define MPD_OUTPUT_INTERNAL_H
#include "audio_format.h"
#include "notify.h"
#include <glib.h>
#include <time.h>
@ -140,11 +141,6 @@ struct audio_output {
*/
GThread *thread;
/**
* Notify object for the thread.
*/
struct notify notify;
/**
* The next command to be performed by the output thread.
*/
@ -160,6 +156,12 @@ struct audio_output {
*/
GMutex *mutex;
/**
* This condition object wakes up the output thread after
* #command has been set.
*/
GCond *cond;
/**
* The #music_chunk which is currently being played. All
* chunks before this one may be returned to the

59
src/output_thread.c
View File

@ -39,18 +39,25 @@ static void ao_command_finished(struct audio_output *ao)
{
assert(ao->command != AO_COMMAND_NONE);
ao->command = AO_COMMAND_NONE;
g_mutex_unlock(ao->mutex);
notify_signal(&audio_output_client_notify);
g_mutex_lock(ao->mutex);
}
static bool
ao_enable(struct audio_output *ao)
{
GError *error = NULL;
bool success;
if (ao->really_enabled)
return true;
if (!ao_plugin_enable(ao->plugin, ao->data, &error)) {
g_mutex_unlock(ao->mutex);
success = ao_plugin_enable(ao->plugin, ao->data, &error);
g_mutex_lock(ao->mutex);
if (!success) {
g_warning("Failed to enable \"%s\" [%s]: %s\n",
ao->name, ao->plugin->name, error->message);
g_error_free(error);
@ -72,7 +79,10 @@ ao_disable(struct audio_output *ao)
if (ao->really_enabled) {
ao->really_enabled = false;
g_mutex_unlock(ao->mutex);
ao_plugin_disable(ao->plugin, ao->data);
g_mutex_lock(ao->mutex);
}
}
@ -111,9 +121,11 @@ ao_open(struct audio_output *ao)
audio_format_mask_apply(&ao->out_audio_format,
&ao->config_audio_format);
g_mutex_unlock(ao->mutex);
success = ao_plugin_open(ao->plugin, ao->data,
&ao->out_audio_format,
&error);
g_mutex_lock(ao->mutex);
assert(!ao->open);
@ -129,9 +141,7 @@ ao_open(struct audio_output *ao)
convert_filter_set(ao->convert_filter, &ao->out_audio_format);
g_mutex_lock(ao->mutex);
ao->open = true;
g_mutex_unlock(ao->mutex);
g_debug("opened plugin=%s name=\"%s\" "
"audio_format=%u:%u:%u:%u",
@ -157,9 +167,9 @@ ao_close(struct audio_output *ao, bool drain)
ao->pipe = NULL;
g_mutex_lock(ao->mutex);
ao->chunk = NULL;
ao->open = false;
g_mutex_unlock(ao->mutex);
if (drain)
@ -170,6 +180,8 @@ ao_close(struct audio_output *ao, bool drain)
ao_plugin_close(ao->plugin, ao->data);
filter_close(ao->filter);
g_mutex_lock(ao->mutex);
g_debug("closed plugin=%s name=\"%s\"", ao->plugin->name, ao->name);
}
@ -193,14 +205,14 @@ ao_reopen_filter(struct audio_output *ao)
ao->pipe = NULL;
g_mutex_lock(ao->mutex);
ao->chunk = NULL;
ao->open = false;
g_mutex_unlock(ao->mutex);
ao_plugin_close(ao->plugin, ao->data);
ao->fail_timer = g_timer_new();
g_mutex_unlock(ao->mutex);
ao_plugin_close(ao->plugin, ao->data);
g_mutex_lock(ao->mutex);
return;
}
@ -246,8 +258,11 @@ ao_play_chunk(struct audio_output *ao, const struct music_chunk *chunk)
assert(music_chunk_check_format(chunk, &ao->in_audio_format));
assert(size % audio_format_frame_size(&ao->in_audio_format) == 0);
if (chunk->tag != NULL)
if (chunk->tag != NULL) {
g_mutex_unlock(ao->mutex);
ao_plugin_send_tag(ao->plugin, ao->data, chunk->tag);
g_mutex_lock(ao->mutex);
}
if (size == 0)
return true;
@ -269,8 +284,10 @@ ao_play_chunk(struct audio_output *ao, const struct music_chunk *chunk)
while (size > 0 && ao->command == AO_COMMAND_NONE) {
size_t nbytes;
g_mutex_unlock(ao->mutex);
nbytes = ao_plugin_play(ao->plugin, ao->data, data, size,
&error);
g_mutex_lock(ao->mutex);
if (nbytes == 0) {
/* play()==0 means failure */
g_warning("\"%s\" [%s] failed to play: %s",
@ -302,7 +319,6 @@ static void ao_play(struct audio_output *ao)
assert(ao->pipe != NULL);
g_mutex_lock(ao->mutex);
chunk = ao->chunk;
if (chunk != NULL)
/* continue the previous play() call */
@ -315,12 +331,8 @@ static void ao_play(struct audio_output *ao)
assert(!ao->chunk_finished);
ao->chunk = chunk;
g_mutex_unlock(ao->mutex);
success = ao_play_chunk(ao, chunk);
g_mutex_lock(ao->mutex);
if (!success) {
assert(ao->chunk == NULL);
break;
@ -331,21 +343,28 @@ static void ao_play(struct audio_output *ao)
}
ao->chunk_finished = true;
g_mutex_unlock(ao->mutex);
g_mutex_unlock(ao->mutex);
notify_signal(&pc.notify);
g_mutex_lock(ao->mutex);
}
static void ao_pause(struct audio_output *ao)
{
bool ret;
g_mutex_unlock(ao->mutex);
ao_plugin_cancel(ao->plugin, ao->data);
g_mutex_lock(ao->mutex);
ao->pause = true;
ao_command_finished(ao);
do {
g_mutex_unlock(ao->mutex);
ret = ao_plugin_pause(ao->plugin, ao->data);
g_mutex_lock(ao->mutex);
if (!ret) {
ao_close(ao, false);
break;
@ -359,6 +378,8 @@ static gpointer audio_output_task(gpointer arg)
{
struct audio_output *ao = arg;
g_mutex_lock(ao->mutex);
while (1) {
switch (ao->command) {
case AO_COMMAND_NONE:
@ -409,19 +430,21 @@ static gpointer audio_output_task(gpointer arg)
/* the player thread will now clear our music
pipe - wait for a notify, to give it some
time */
notify_wait(&ao->notify);
g_cond_wait(ao->cond, ao->mutex);
continue;
case AO_COMMAND_KILL:
ao->chunk = NULL;
ao_command_finished(ao);
g_mutex_unlock(ao->mutex);
return NULL;
}
if (ao->open)
ao_play(ao);
notify_wait(&ao->notify);
if (ao->command == AO_COMMAND_NONE)
g_cond_wait(ao->cond, ao->mutex);
}
}

4
test/run_output.c
View File

@ -58,10 +58,6 @@ pcm_convert(G_GNUC_UNUSED struct pcm_convert_state *state,
return NULL;
}
void notify_init(G_GNUC_UNUSED struct notify *notify)
{
}
const struct filter_plugin *
filter_plugin_by_name(G_GNUC_UNUSED const char *name)
{