Compare commits

...

14 Commits

Author SHA1 Message Date
Avuton Olrich
dec7090198 mpd version 0.15.14 2010-11-06 14:42:02 -07:00
Max Kellermann
83ec0e5552 player_thread: fix assertion failure due to wrong music pipe on seek
When one song is played twice, and the decoder is working on the
second "instance", but the first should be seeked, the check in
player_seek_decoder() may assume that it can reuse the decoder without
exchanging pipes.  The last thing was the mistake: the pipe pointer
was different, which led to an assertion failure.  This patch adds
another check which exchanges the player pipe.
2010-11-05 19:24:42 +01:00
Max Kellermann
cc261872c2 decoder_control: pass music_pipe to dc_start()
More abstraction for decoder_control.pipe.
2010-11-05 19:18:44 +01:00
Max Kellermann
5223261f19 player_thread: add helper function player_dc_at_next_song()
Some abstraction for decoder_control.pipe access.
2010-11-05 19:08:59 +01:00
Max Kellermann
c594afeee7 pipe: add helper function music_pipe_empty() 2010-11-05 18:40:23 +01:00
Max Kellermann
32d10eedbd input/rewind: remove redundant NULL check before g_free() call 2010-11-05 18:40:14 +01:00
Max Kellermann
dfd98eede7 input/rewind: add two assertions 2010-11-05 18:40:07 +01:00
Max Kellermann
a728d7a026 input/rewind: fix double free bug
Duplicate the "mime" attribute of the inner input_stream object,
instead of copying the pointer.
2010-11-05 18:39:40 +01:00
Max Kellermann
e8d8bd4c0d decoder/{mp4ff,ffmpeg}: add extension ".m4b" (audio book)
Same as ".m4a".
2010-11-05 02:01:35 +01:00
Max Kellermann
8d5fa754e8 output_thread: fix assertion failure due to race condition in OPEN
Change the assertion on "fail_timer==NULL" in OPEN to a runtime check.
This assertion crashed when the output thread failed while the player
thread was calling audio_output_open().
2010-11-04 23:44:23 +01:00
Max Kellermann
2ee047a1dd output_internal: protect attribute "fail_timer" with mutex 2010-11-04 23:40:43 +01:00
Max Kellermann
9562f66741 output_control: lock object in audio_output_open()
Protect the attributes "open" and "fail_timer".
2010-11-04 23:28:18 +01:00
Max Kellermann
21223154aa output_control: lock object in audio_output_close()
Protect the attributes "open" and "fail_timer".
2010-11-04 21:51:02 +01:00
Avuton Olrich
a549d871f3 Modify version string to post-release version 0.15.14~git 2010-10-10 09:57:57 -07:00
13 changed files with 157 additions and 37 deletions

9
NEWS

@@ -1,3 +1,12 @@
ver 0.15.14 (2010/11/06)
* player_thread: fix assertion failure due to wrong music pipe on seek
* output_thread: fix assertion failure due to race condition in OPEN
* input:
- rewind: fix double free bug
* decoders:
- mp4ff, ffmpeg: add extension ".m4b" (audio book)
ver 0.15.13 (2010/10/10)
* output_thread: fix race condition after CANCEL command
* output:

@@ -1,5 +1,5 @@
AC_PREREQ(2.60)
AC_INIT(mpd, 0.15.13, musicpd-dev-team@lists.sourceforge.net)
AC_INIT(mpd, 0.15.14, musicpd-dev-team@lists.sourceforge.net)
AC_CONFIG_SRCDIR([src/main.c])
AM_INIT_AUTOMAKE([foreign 1.9 dist-bzip2])
AM_CONFIG_HEADER(config.h)

@@ -501,7 +501,9 @@ static const char *const ffmpeg_suffixes[] = {
"atrac", "au", "aud", "avi", "avm2", "avs", "bap", "bfi", "c93", "cak",
"cin", "cmv", "cpk", "daud", "dct", "divx", "dts", "dv", "dvd", "dxa",
"eac3", "film", "flac", "flc", "fli", "fll", "flx", "flv", "g726",
"gsm", "gxf", "iss", "m1v", "m2v", "m2t", "m2ts", "m4a", "m4v", "mad",
"gsm", "gxf", "iss", "m1v", "m2v", "m2t", "m2ts",
"m4a", "m4b", "m4v",
"mad",
"mj2", "mjpeg", "mjpg", "mka", "mkv", "mlp", "mm", "mmf", "mov", "mp+",
"mp1", "mp2", "mp3", "mp4", "mpc", "mpeg", "mpg", "mpga", "mpp", "mpu",
"mve", "mvi", "mxf", "nc", "nsv", "nut", "nuv", "oga", "ogm", "ogv",

@@ -425,7 +425,13 @@ mp4_tag_dup(const char *file)
return ret;
}
static const char *const mp4_suffixes[] = { "m4a", "mp4", NULL };
static const char *const mp4_suffixes[] = {
"m4a",
"m4b",
"mp4",
NULL
};
static const char *const mp4_mime_types[] = { "audio/mp4", "audio/m4a", NULL };
const struct decoder_plugin mp4ff_decoder_plugin = {

@@ -18,6 +18,7 @@
*/
#include "decoder_control.h"
#include "pipe.h"
#include <assert.h>
@@ -58,22 +59,28 @@ static void dc_command_async(enum decoder_command cmd)
}
void
dc_start(struct notify *notify, struct song *song)
dc_start(struct notify *notify, struct song *song, struct music_pipe *pipe)
{
assert(dc.pipe != NULL);
assert(dc.pipe == NULL);
assert(song != NULL);
assert(pipe != NULL);
assert(music_pipe_empty(pipe));
dc.next_song = song;
dc.pipe = pipe;
dc_command(notify, DECODE_COMMAND_START);
}
void
dc_start_async(struct song *song)
dc_start_async(struct song *song, struct music_pipe *pipe)
{
assert(dc.pipe != NULL);
assert(dc.pipe == NULL);
assert(song != NULL);
assert(pipe != NULL);
assert(music_pipe_empty(pipe));
dc.next_song = song;
dc.pipe = pipe;
dc_command_async(DECODE_COMMAND_START);
}

@@ -118,10 +118,10 @@ void
dc_command_wait(struct notify *notify);
void
dc_start(struct notify *notify, struct song *song);
dc_start(struct notify *notify, struct song *song, struct music_pipe *pipe);
void
dc_start_async(struct song *song);
dc_start_async(struct song *song, struct music_pipe *pipe);
void
dc_stop(struct notify *notify);

@@ -83,6 +83,9 @@ copy_attributes(struct input_stream *dest)
const struct input_rewind *r = dest->data;
const struct input_stream *src = &r->input;
assert(dest != src);
assert(dest->mime != src->mime);
dest->ready = src->ready;
dest->seekable = src->seekable;
dest->error = src->error;
@@ -90,8 +93,7 @@ copy_attributes(struct input_stream *dest)
dest->offset = src->offset;
if (src->mime != NULL) {
if (dest->mime != NULL)
g_free(dest->mime);
g_free(dest->mime);
dest->mime = g_strdup(src->mime);
}
}
@@ -244,4 +246,5 @@ input_rewind_open(struct input_stream *is)
input stream */
is->plugin = &rewind_input_plugin;
is->data = c;
is->mime = g_strdup(c->input.mime);
}

@@ -266,7 +266,7 @@ audio_output_all_open(const struct audio_format *audio_format,
else
/* if the pipe hasn't been cleared, the the audio
format must not have changed */
assert(music_pipe_size(g_mp) == 0 ||
assert(music_pipe_empty(g_mp) ||
audio_format_equals(audio_format,
&input_audio_format));
@@ -378,7 +378,7 @@ audio_output_all_check(void)
assert(g_mp != NULL);
while ((chunk = music_pipe_peek(g_mp)) != NULL) {
assert(music_pipe_size(g_mp) > 0);
assert(!music_pipe_empty(g_mp));
if (!chunk_is_consumed(chunk))
/* at least one output is not finished playing

@@ -50,6 +50,20 @@ static void ao_command(struct audio_output *ao, enum audio_output_command cmd)
ao_command_wait(ao);
}
/**
* Like ao_command(), but assumes the object is locked by the caller.
*/
static void
ao_command_locked(struct audio_output *ao, enum audio_output_command cmd)
{
assert(ao->command == AO_COMMAND_NONE);
ao->command = cmd;
g_mutex_unlock(ao->mutex);
ao_command_wait(ao);
g_mutex_lock(ao->mutex);
}
static void ao_command_async(struct audio_output *ao,
enum audio_output_command cmd)
{
@@ -58,6 +72,12 @@ static void ao_command_async(struct audio_output *ao,
notify_signal(&ao->notify);
}
static void
audio_output_close_locked(struct audio_output *ao);
/**
* Object must be locked (and unlocked) by the caller.
*/
static bool
audio_output_open(struct audio_output *ao,
const struct audio_format *audio_format,
@@ -84,7 +104,7 @@ 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);
ao_command_locked(ao, AO_COMMAND_CANCEL);
}
return true;
@@ -95,7 +115,7 @@ audio_output_open(struct audio_output *ao,
if (!ao->config_audio_format) {
if (ao->open)
audio_output_close(ao);
audio_output_close_locked(ao);
/* no audio format is configured: copy in->out, let
the output's open() method determine the effective
@@ -110,7 +130,7 @@ audio_output_open(struct audio_output *ao,
open = ao->open;
if (!open) {
ao_command(ao, AO_COMMAND_OPEN);
ao_command_locked(ao, AO_COMMAND_OPEN);
open = ao->open;
}
@@ -127,12 +147,19 @@ audio_output_update(struct audio_output *ao,
{
assert(mp != NULL);
g_mutex_lock(ao->mutex);
if (ao->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 ret = audio_output_open(ao, audio_format, mp);
g_mutex_unlock(ao->mutex);
return ret;
}
} else if (audio_output_is_open(ao))
audio_output_close(ao);
audio_output_close_locked(ao);
g_mutex_unlock(ao->mutex);
return false;
}
@@ -162,21 +189,33 @@ void audio_output_cancel(struct audio_output *ao)
ao_command_async(ao, AO_COMMAND_CANCEL);
}
void audio_output_close(struct audio_output *ao)
static void
audio_output_close_locked(struct audio_output *ao)
{
assert(ao != NULL);
assert(!ao->open || ao->fail_timer == NULL);
if (ao->mixer != NULL)
mixer_auto_close(ao->mixer);
if (ao->open)
ao_command(ao, AO_COMMAND_CLOSE);
ao_command_locked(ao, AO_COMMAND_CLOSE);
else if (ao->fail_timer != NULL) {
g_timer_destroy(ao->fail_timer);
ao->fail_timer = NULL;
}
}
void audio_output_close(struct audio_output *ao)
{
assert(ao != NULL);
assert(!ao->open || ao->fail_timer == NULL);
g_mutex_lock(ao->mutex);
audio_output_close_locked(ao);
g_mutex_unlock(ao->mutex);
}
void audio_output_finish(struct audio_output *ao)
{
audio_output_close(ao);

@@ -131,7 +131,8 @@ struct audio_output {
const struct music_pipe *pipe;
/**
* This mutex protects #open, #chunk and #chunk_finished.
* This mutex protects #open, #fail_timer, #chunk and
* #chunk_finished.
*/
GMutex *mutex;

@@ -105,7 +105,12 @@ ao_play_chunk(struct audio_output *ao, const struct music_chunk *chunk)
/* don't automatically reopen this device for
10 seconds */
g_mutex_lock(ao->mutex);
assert(ao->fail_timer == NULL);
ao->fail_timer = g_timer_new();
g_mutex_unlock(ao->mutex);
return false;
}
@@ -192,10 +197,18 @@ static gpointer audio_output_task(gpointer arg)
case AO_COMMAND_OPEN:
assert(!ao->open);
assert(ao->fail_timer == NULL);
assert(ao->pipe != NULL);
assert(ao->chunk == NULL);
if (ao->fail_timer != NULL) {
/* this can only happen when this
output thread fails while
audio_output_open() is run in the
player thread */
g_timer_destroy(ao->fail_timer);
ao->fail_timer = NULL;
}
error = NULL;
ret = ao_plugin_open(ao->plugin, ao->data,
&ao->out_audio_format,

@@ -99,4 +99,10 @@ music_pipe_push(struct music_pipe *mp, struct music_chunk *chunk);
unsigned
music_pipe_size(const struct music_pipe *mp);
static inline bool
music_pipe_empty(const struct music_pipe *mp)
{
return music_pipe_size(mp) == 0;
}
#endif

@@ -118,6 +118,33 @@ static void player_command_finished(void)
notify_signal(&main_notify);
}
/**
* Is the decoder still busy on the same song as the player?
*
* Note: this function does not check if the decoder is already
* finished.
*/
static bool
player_dc_at_current_song(const struct player *player)
{
assert(player != NULL);
assert(player->pipe != NULL);
return dc.pipe == player->pipe;
}
/**
* Has the decoder already begun decoding the next song?
*
* Note: this function does not check if the decoder is already
* finished.
*/
static bool
player_dc_at_next_song(const struct player *player)
{
return dc.pipe != NULL && !player_dc_at_current_song(player);
}
/**
* Stop the decoder and clears (and frees) its music pipe.
*/
@@ -297,10 +324,9 @@ static bool player_seek_decoder(struct player *player)
/* clear music chunks which might still reside in the
pipe */
music_pipe_clear(player->pipe, player_buffer);
dc.pipe = player->pipe;
/* re-start the decoder */
dc_start_async(pc.next_song);
dc_start_async(pc.next_song, player->pipe);
ret = player_wait_for_decoder(player);
if (!ret) {
/* decoder failure */
@@ -308,6 +334,14 @@ static bool player_seek_decoder(struct player *player)
return false;
}
} else {
if (!player_dc_at_current_song(player)) {
/* the decoder is already decoding the "next" song,
but it is the same song file; exchange the pipe */
music_pipe_clear(player->pipe, player_buffer);
music_pipe_free(player->pipe);
player->pipe = dc.pipe;
}
pc.next_song = NULL;
player->queued = false;
}
@@ -364,7 +398,7 @@ static void player_process_command(struct player *player)
case PLAYER_COMMAND_QUEUE:
assert(pc.next_song != NULL);
assert(!player->queued);
assert(dc.pipe == NULL || dc.pipe == player->pipe);
assert(!player_dc_at_next_song(player));
player->queued = true;
player_command_finished();
@@ -409,7 +443,7 @@ static void player_process_command(struct player *player)
return;
}
if (dc.pipe != NULL && dc.pipe != player->pipe)
if (player_dc_at_next_song(player))
/* the decoder is already decoding the song -
stop it and reset the position */
player_dc_stop(player);
@@ -505,7 +539,7 @@ play_next_chunk(struct player *player)
return true;
if (player->xfade == XFADE_ENABLED &&
dc.pipe != NULL && dc.pipe != player->pipe &&
player_dc_at_next_song(player) &&
(cross_fade_position = music_pipe_size(player->pipe))
<= player->cross_fade_chunks) {
/* perform cross fade */
@@ -638,8 +672,7 @@ static void do_play(void)
player.pipe = music_pipe_new();
dc.buffer = player_buffer;
dc.pipe = player.pipe;
dc_start(&pc.notify, pc.next_song);
dc_start(&pc.notify, pc.next_song, player.pipe);
if (!player_wait_for_decoder(&player)) {
player_dc_stop(&player);
player_command_finished();
@@ -706,14 +739,15 @@ static void do_play(void)
/* the decoder has finished the current song;
make it decode the next song */
assert(pc.next_song != NULL);
assert(dc.pipe == NULL || dc.pipe == player.pipe);
assert(!player_dc_at_next_song(&player));
dc.pipe = NULL;
player.queued = false;
dc.pipe = music_pipe_new();
dc_start_async(pc.next_song);
dc_start_async(pc.next_song, music_pipe_new());
}
if (dc.pipe != NULL && dc.pipe != player.pipe &&
if (player_dc_at_next_song(&player) &&
player.xfade == XFADE_UNKNOWN &&
!decoder_is_starting()) {
/* enable cross fading in this song? if yes,
@@ -736,7 +770,7 @@ static void do_play(void)
if (player.paused)
notify_wait(&pc.notify);
else if (music_pipe_size(player.pipe) > 0) {
else if (!music_pipe_empty(player.pipe)) {
/* at least one music chunk is ready - send it
to the audio output */
@@ -748,7 +782,7 @@ static void do_play(void)
/* XXX synchronize in a better way */
g_usleep(10000);
} else if (dc.pipe != NULL && dc.pipe != player.pipe) {
} else if (player_dc_at_next_song(&player)) {
/* at the beginning of a new song */
if (!player_song_border(&player))
@@ -757,7 +791,7 @@ static void do_play(void)
/* check the size of the pipe again, because
the decoder thread may have added something
since we last checked */
if (music_pipe_size(player.pipe) == 0)
if (music_pipe_empty(player.pipe))
break;
} else {
/* the decoder is too busy and hasn't provided