mpd version 0.15.14

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.

NEWS

@@ -1,3 +1,22 @@
+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:
+  - httpd: fix random data in stream title
+  - httpd: MIME type audio/ogg for Ogg Vorbis
+* input:
+  - rewind: update MIME not only once
+  - rewind: enable for MMS
+
+
 ver 0.15.12 (2010/07/20)
 * input:
   - curl: remove assertion after curl_multi_fdset()

configure.ac

@@ -1,5 +1,5 @@
 AC_PREREQ(2.60)
-AC_INIT(mpd, 0.15.12, 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)

src/decoder/ffmpeg_plugin.c

@@ -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",

src/decoder/mp4ff_plugin.c

@@ -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 = {

src/decoder_control.c

@@ -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);
 }
 

src/decoder_control.h

@@ -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);

src/icy_server.c

@@ -95,6 +95,7 @@ icy_server_metadata_page(const struct tag *tag, ...)
 	gchar stream_title[(1 + 255 - 28) * 16]; // Length + Metadata -
 						 // "StreamTitle='';StreamUrl='';"
 						 // = 4081 - 28
+	stream_title[0] =  '\0';
 
 	last_item = -1;
 

src/input/curl_input_plugin.c

@@ -259,7 +259,7 @@ input_curl_select(struct input_curl *c)
 		return -1;
 	}
 
-#if LIBCURL_VERSION_NUM >= 0x070f00
+#if LIBCURL_VERSION_NUM >= 0x070f04
 	long timeout2;
 	mcode = curl_multi_timeout(c->multi, &timeout2);
 	if (mcode != CURLM_OK) {

src/input/rewind_input_plugin.c

@@ -20,6 +20,9 @@
 #include "config.h"
 #include "input/rewind_input_plugin.h"
 #include "input/curl_input_plugin.h"
+#ifdef ENABLE_MMS
+#include "input/mms_input_plugin.h"
+#endif
 #include "input_plugin.h"
 #include "tag.h"
 
@@ -80,16 +83,19 @@ 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;
 	dest->size = src->size;
 	dest->offset = src->offset;
 
-	if (dest->mime == NULL && src->mime != NULL)
-		/* this is set only once, and the duplicated pointer
-		   is freed by input_stream_close() */
+	if (src->mime != NULL) {
+		g_free(dest->mime);
 		dest->mime = g_strdup(src->mime);
+	}
 }
 
 static void
@@ -219,7 +225,11 @@ input_rewind_open(struct input_stream *is)
 	assert(is != NULL);
 	assert(is->offset == 0);
 
-	if (is->plugin != &input_plugin_curl)
+	if (is->plugin != &input_plugin_curl
+#ifdef ENABLE_MMS
+	    && is->plugin != &input_plugin_mms
+#endif
+	   )
 		/* due to limitations in the input_plugin API, we only
 		   (explicitly) support the CURL input plugin */
 		return;
@@ -229,10 +239,12 @@ input_rewind_open(struct input_stream *is)
 
 	/* move the CURL input stream to c->input */
 	c->input = *is;
-	input_curl_reinit(&c->input);
+	if (is->plugin == &input_plugin_curl)
+		input_curl_reinit(&c->input);
 
 	/* convert the existing input_stream pointer to a "rewind"
 	   input stream */
 	is->plugin = &rewind_input_plugin;
 	is->data = c;
+	is->mime = g_strdup(c->input.mime);
 }

src/notify.c

@@ -48,3 +48,10 @@ void notify_signal(struct notify *notify)
 	g_cond_signal(notify->cond);
 	g_mutex_unlock(notify->mutex);
 }
+
+void notify_clear(struct notify *notify)
+{
+	g_mutex_lock(notify->mutex);
+	notify->pending = false;
+	g_mutex_unlock(notify->mutex);
+}

src/notify.h

@@ -45,4 +45,9 @@ void notify_wait(struct notify *notify);
  */
 void notify_signal(struct notify *notify);
 
+/**
+ * Clears a pending notification.
+ */
+void notify_clear(struct notify *notify);
+
 #endif

src/output/httpd_output_plugin.c

@@ -70,7 +70,7 @@ httpd_output_init(G_GNUC_UNUSED const struct audio_format *audio_format,
 	}
 
 	if (strcmp(encoder_name, "vorbis") == 0)
-		httpd->content_type = "application/x-ogg";
+		httpd->content_type = "audio/ogg";
 	else if (strcmp(encoder_name, "lame") == 0)
 		httpd->content_type = "audio/mpeg";
 	else

src/output_all.c

@@ -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

src/output_control.c

@@ -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);

src/output_internal.h

@@ -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;
 

src/output_thread.c

@@ -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,
@@ -268,6 +281,16 @@ static gpointer audio_output_task(gpointer arg)
 			ao->chunk = NULL;
 			if (ao->open)
 				ao_plugin_cancel(ao->plugin, ao->data);
+
+			/* we must clear the notification now, because
+			   the notify_wait() call below must wait
+			   until audio_output_all_cancel() has cleared
+			   the pipe; if another notification happens
+			   to be still pending, we get a race
+			   condition with a crash or an assertion
+			   failure */
+			notify_clear(&ao->notify);
+
 			ao_command_finished(ao);
 
 			/* the player thread will now clear our music

src/pipe.h

@@ -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

src/player_thread.c

@@ -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