Reimplement dynamic metadata handling

This has been tested for both playback of streams and
outputting to streams, and seems to work fine with minimal
locking.  This reuses the sequence number infrastructure
in OutputBuffer for synchronizing metadata payloads; so
(IMNSHO) should be much more understandable than various
flags being set here and there..

It could still use some cleanup and much testing, but
synchronization issues should be minimal.

12 files changed, 287 insertions, 35 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 6c52dde1e..70306c125 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -55,6 +55,7 @@ mpd_headers = \
 	log.h \
 	ls.h \
 	main_notify.h \
+	metadata_pipe.h \
 	mpd_types.h \
 	myfprintf.h \
 	normalize.h \
@@ -118,6 +119,7 @@ mpd_SOURCES = \
 	ls.c \
 	main.c \
 	main_notify.c \
+	metadata_pipe.c \
 	myfprintf.c \
 	normalize.c \
 	compress.c \
diff --git a/src/inputPlugin.h b/src/inputPlugin.h
index 61cef9bef..2f337acef 100644
--- a/src/inputPlugin.h
+++ b/src/inputPlugin.h
@@ -21,6 +21,7 @@
 
 #include "inputStream.h"
 #include "outputBuffer.h"
+#include "metadata_pipe.h"
 
 /* valid values for streamTypes in the InputPlugin struct: */
 #define INPUT_PLUGIN_STREAM_FILE	0x01
diff --git a/src/inputPlugins/mp3_plugin.c b/src/inputPlugins/mp3_plugin.c
index 5bbd7601f..c36cab6f0 100644
--- a/src/inputPlugins/mp3_plugin.c
+++ b/src/inputPlugins/mp3_plugin.c
@@ -921,7 +921,7 @@ static int mp3Read(mp3DecodeData * data, ReplayGainInfo ** replayGainInfo)
 					data->inStream->metaTitle);
 			free(data->inStream->metaTitle);
 			data->inStream->metaTitle = NULL;
-			freeMpdTag(tag);
+			metadata_pipe_send(tag, data->elapsedTime);
 		}
 
 		samplesLeft = (data->synth).pcm.length;
@@ -1050,20 +1050,19 @@ static int mp3_decode(InputStream * inStream)
 		if (inStream->metaName) {
 			addItemToMpdTag(tag, TAG_ITEM_NAME, inStream->metaName);
 		}
-		freeMpdTag(tag);
 	} else if (tag) {
 		if (inStream->metaName) {
 			clearItemsFromMpdTag(tag, TAG_ITEM_NAME);
 			addItemToMpdTag(tag, TAG_ITEM_NAME, inStream->metaName);
 		}
-		freeMpdTag(tag);
 	} else if (inStream->metaName) {
 		tag = newMpdTag();
 		if (inStream->metaName) {
 			addItemToMpdTag(tag, TAG_ITEM_NAME, inStream->metaName);
 		}
-		freeMpdTag(tag);
 	}
+	if (tag)
+		metadata_pipe_send(tag, 0);
 
 	while (mp3Read(&data, &replayGainInfo) != DECODE_BREAK) ;
 	/* send last little bit if not dc_intr() */
diff --git a/src/inputPlugins/oggvorbis_plugin.c b/src/inputPlugins/oggvorbis_plugin.c
index fcedda54a..cb6eed28e 100644
--- a/src/inputPlugins/oggvorbis_plugin.c
+++ b/src/inputPlugins/oggvorbis_plugin.c
@@ -196,7 +196,8 @@ static MpdTag *oggCommentsParse(char **comments)
 }
 
 static void putOggCommentsIntoOutputBuffer(char *streamName,
-					   char **comments)
+					   char **comments,
+					   float time)
 {
 	MpdTag *tag;
 
@@ -212,7 +213,12 @@ static void putOggCommentsIntoOutputBuffer(char *streamName,
 		addItemToMpdTag(tag, TAG_ITEM_NAME, streamName);
 	}
 
-	freeMpdTag(tag);
+	metadata_pipe_send(tag, time);
+}
+
+static float current_time(OggVorbis_File *vf)
+{
+	return (ov_pcm_tell(vf) / dc.audio_format.sampleRate);
 }
 
 /* public */
@@ -291,7 +297,8 @@ static int oggvorbis_decode(InputStream * inStream)
 			dc.audio_format.sampleRate = vi->rate;
 			comments = ov_comment(&vf, -1)->user_comments;
 			putOggCommentsIntoOutputBuffer(inStream->metaName,
-						       comments);
+						       comments,
+						       current_time(&vf));
 			ogg_getReplayGainInfo(comments, &replayGainInfo);
 		}
 
@@ -310,8 +317,7 @@ static int oggvorbis_decode(InputStream * inStream)
 			if ((test = ov_bitrate_instant(&vf)) > 0) {
 				bitRate = test / 1000;
 			}
-			ob_send(chunk, chunkpos,
-			        ov_pcm_tell(&vf) / dc.audio_format.sampleRate,
+			ob_send(chunk, chunkpos, current_time(&vf),
 			        bitRate, replayGainInfo);
 			chunkpos = 0;
 			if (dc_intr())
diff --git a/src/main.c b/src/main.c
index 239296501..e897cacc1 100644
--- a/src/main.c
+++ b/src/main.c
@@ -44,6 +44,7 @@
 #include "main_notify.h"
 #include "os_compat.h"
 #include "outputBuffer.h"
+#include "metadata_pipe.h"
 
 #define SYSTEM_CONFIG_FILE_LOCATION	"/etc/mpd.conf"
 #define USER_CONFIG_FILE_LOCATION	"/.mpdconf"
@@ -419,6 +420,7 @@ int main(int argc, char *argv[])
 	initReplayGainState();
 	initNormalization();
 	initInputStream();
+	init_metadata_pipe();
 
 	daemonize(&options);
 
diff --git a/src/metadataChunk.c b/src/metadataChunk.c
deleted file mode 100644
index e69de29bb..000000000
--- a/src/metadataChunk.c
+++ /dev/null
diff --git a/src/metadataChunk.h b/src/metadataChunk.h
deleted file mode 100644
index e69de29bb..000000000
--- a/src/metadataChunk.h
+++ /dev/null
diff --git a/src/metadata_pipe.c b/src/metadata_pipe.c
new file mode 100644
index 000000000..aecc331e6
--- /dev/null
+++ b/src/metadata_pipe.c
@@ -0,0 +1,155 @@
+/* the Music Player Daemon (MPD)
+ * Copyright (C) 2003-2007 by Warren Dukes (warren.dukes@gmail.com)
+ * This project's homepage is: http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include "metadata_pipe.h"
+#include "ringbuf.h"
+#include "decode.h"
+#include "os_compat.h"
+#include "log.h"
+#include "outputBuffer.h"
+#include "gcc.h"
+
+/* These are defined in outputBuffer_accessors.h, cleanup is needed */
+mpd_uint8 ob_get_decoder_sequence(void);
+mpd_uint8 ob_get_player_sequence(void);
+
+static struct ringbuf *mp;
+
+/* Each one of these is a packet inside the metadata pipe */
+struct tag_container {
+	float metadata_time;
+	mpd_uint8 seq; /* ob.seq_decoder at time of metadata_pipe_send() */
+	MpdTag *tag; /* our payload */
+};
+
+/*
+ * We have two readers even though ringbuf was designed for one (locklessly),
+ * so we will use a lock to allow readers to safely read.  Writing is only
+ * done from one thread, so it will never block or clobber.
+ */
+static pthread_mutex_t read_lock = PTHREAD_MUTEX_INITIALIZER;
+static MpdTag *current_tag; /* requires read_lock for both r/w access */
+
+static void metadata_pipe_finish(void)
+{
+	ringbuf_free(mp);
+	if (current_tag)
+		freeMpdTag(current_tag);
+}
+
+void init_metadata_pipe(void)
+{
+	mp = ringbuf_create(sizeof(struct tag_container) * 16);
+	atexit(metadata_pipe_finish);
+}
+
+void metadata_pipe_send(MpdTag *tag, float metadata_time)
+{
+	struct tag_container tc;
+	size_t written;
+
+	assert(pthread_equal(pthread_self(), dc.thread));
+
+	if (mpd_unlikely(ringbuf_write_space(mp)
+	                 < sizeof(struct tag_container))) {
+		DEBUG("metadata_pipe: insufficient buffer space, dropping\n");
+		return;
+	}
+
+	tc.tag = tag;
+	tc.metadata_time = metadata_time;
+	tc.seq = ob_get_decoder_sequence();
+	written = ringbuf_write(mp, &tc, sizeof(struct tag_container));
+	assert(written == sizeof(struct tag_container));
+}
+
+static void pipe_clear_unlocked(void)
+{
+	struct tag_container tc;
+	size_t r;
+
+	while ((r = ringbuf_read(mp, &tc, sizeof(struct tag_container)))) {
+		assert(r == sizeof(struct tag_container));
+		freeMpdTag(tc.tag);
+	}
+}
+
+MpdTag * metadata_pipe_recv(void)
+{
+	struct tag_container tc;
+	size_t r;
+	static const size_t mpd_uint8_max = 255; /* XXX CLEANUP */
+	mpd_uint8 expect_seq = ob_get_player_sequence();
+	unsigned long current_time = ob_get_elapsed_time();
+	MpdTag *tag = NULL;
+
+	if (pthread_mutex_trylock(&read_lock) == EBUSY)
+		return NULL;
+retry:
+	if (!(r = ringbuf_peek(mp, &tc, sizeof(struct tag_container))))
+		goto out;
+
+	assert(r == sizeof(struct tag_container));
+	assert(tc.tag);
+	if (expect_seq == tc.seq) {
+		if (current_time < tc.metadata_time)
+			goto out; /* not ready for tag yet */
+		if (mpdTagsAreEqual(tc.tag, current_tag)) {
+			freeMpdTag(tc.tag);
+			ringbuf_read_advance(mp, sizeof(struct tag_container));
+			goto out; /* nothing changed, don't bother */
+		}
+		tag = mpdTagDup(tc.tag);
+		if (current_tag)
+			freeMpdTag(current_tag);
+		current_tag = tc.tag;
+		ringbuf_read_advance(mp, sizeof(struct tag_container));
+	} else if (expect_seq > tc.seq ||
+	           (!expect_seq && tc.seq == mpd_uint8_max)) {
+		DEBUG("metadata_pipe: reader is ahead of writer\n");
+		freeMpdTag(tc.tag);
+		ringbuf_read_advance(mp, sizeof(struct tag_container));
+		goto retry; /* read and skip packets */
+	} else {
+		DEBUG("metadata_pipe: writer is ahead of reader\n");
+		/* not ready for tag yet */
+	}
+out:
+	pthread_mutex_unlock(&read_lock);
+	return tag;
+}
+
+MpdTag *metadata_pipe_current(void)
+{
+	MpdTag *tag;
+
+	assert(! pthread_equal(pthread_self(), dc.thread));
+	if (pthread_mutex_trylock(&read_lock) == EBUSY)
+		return NULL;
+	tag = current_tag ? mpdTagDup(current_tag) : NULL;
+	pthread_mutex_unlock(&read_lock);
+
+	return tag;
+}
+
+void metadata_pipe_clear(void)
+{
+	pthread_mutex_lock(&read_lock);
+	pipe_clear_unlocked();
+	pthread_mutex_unlock(&read_lock);
+}
diff --git a/src/metadata_pipe.h b/src/metadata_pipe.h
new file mode 100644
index 000000000..e54c67584
--- /dev/null
+++ b/src/metadata_pipe.h
@@ -0,0 +1,49 @@
+/* the Music Player Daemon (MPD)
+ * Copyright (C) 2003-2007 by Warren Dukes (warren.dukes@gmail.com)
+ * This project's homepage is: http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef METADATA_PIPE_H
+#define METADATA_PIPE_H
+
+#include "tag.h"
+#include "mpd_types.h"
+
+void init_metadata_pipe(void);
+
+/*
+ * Called by the decoder thread, this inserts a tag pointer into the pipe
+ * DO NOT FREE the tag placed into the pipe; that is that job of the
+ * caller of metadata_pipe_recv() or metadata_pipe_clear().
+ */
+void metadata_pipe_send(MpdTag * tag, float metadata_time);
+
+/*
+ * Reads and consumes one MpdTag pointer off the pipe.  The caller
+ * of this MUST free the MpdTag pointer after it is done using it.
+ */
+MpdTag * metadata_pipe_recv(void);
+
+/*
+ * Returns the last read MpdTag from metadata_pipe_recv(), caller
+ * must free this pointer when it is done using it.
+ */
+MpdTag * metadata_pipe_current(void);
+
+/* Clears all MpdTag pointers on the pipe, freeing all associated elements */
+void metadata_pipe_clear(void);
+
+#endif /* METADATA_PIPE_H */
diff --git a/src/outputBuffer.c b/src/outputBuffer.c
index 7e899e574..ff3a8e901 100644
--- a/src/outputBuffer.c
+++ b/src/outputBuffer.c
@@ -70,8 +70,8 @@ struct output_buffer {
 	size_t conv_buf_len;
 	pthread_t thread;
 	ConvState conv_state;
-	unsigned int seq_drop;
-	unsigned int seq_player; /* only gets changed by ob.thread */
+	mpd_uint8 seq_drop;
+	mpd_uint8 seq_player; /* only gets changed by ob.thread */
 	mpd_uint8 seq_decoder; /* only gets changed by dc.thread */
 	struct ringbuf preseek_index;
 	enum ob_state preseek_state;
@@ -154,7 +154,7 @@ static enum action_status ob_do_drop(void)
 {
 	struct iovec vec[2];
 	long i;
-	unsigned int seq_drop;
+	mpd_uint8 seq_drop;
 
 	cond_enter(&ob_seq_cond);
 	seq_drop = ob.seq_drop;
@@ -202,9 +202,10 @@ static void reader_reset_buffer(void)
 		c->len = 0;
 	}
 	ringbuf_read_advance(ob.index, nr);
+	metadata_pipe_clear();
 }
 
-static void ob_seq_player_set(unsigned int seq_num)
+static void ob_seq_player_set(mpd_uint8 seq_num)
 {
 	cond_enter(&ob_seq_cond);
 	ob.seq_player = seq_num;
@@ -222,7 +223,7 @@ static enum action_status ob_do_reset(int close)
 	if (close)
 		closeAudioDevice();
 	ob.xfade_state = XFADE_DISABLED;
-	ob_seq_player_set((unsigned int)ob.seq_decoder);
+	ob_seq_player_set(ob.seq_decoder);
 	return ob_finalize_action();
 }
 
@@ -277,7 +278,7 @@ static enum action_status ob_do_seek_finish(void)
 		ob.total_time = dc.total_time;
 		reader_reset_buffer();
 		dropBufferedAudio();
-		ob_seq_player_set((unsigned int)ob.seq_decoder);
+		ob_seq_player_set(ob.seq_decoder);
 	}
 	return ob_finalize_action();
 }
@@ -398,12 +399,31 @@ static void new_song_chunk(struct ob_chunk *a)
 	ob.xfade_state = XFADE_DISABLED;
 	ob.total_time = dc.total_time;
 	/* DEBUG("ob.total_time: %f\n", ob.total_time); */
-	ob_seq_player_set((unsigned int)a->seq);
+	ob_seq_player_set(a->seq);
 	wakeup_main_task(); /* sync playlist */
 }
 
 #include "outputBuffer_audio.h"
 
+static void send_next_tag(void)
+{
+	static MpdTag *last_tag;
+	MpdTag *tag;
+
+	if ((tag = metadata_pipe_recv())) { /* streaming tag */
+		DEBUG("Caught new metadata! %p\n", tag);
+		sendMetadataToAudioDevice(tag);
+		freeMpdTag(tag);
+		wakeup_main_task(); /* call sync_metadata() in playlist.c */
+	} else if ((tag = playlist_current_tag())) { /* static file tag */
+		/* shouldn't need mpdTagsAreEqual here for static tags */
+		if (last_tag != tag) {
+			sendMetadataToAudioDevice(tag);
+			last_tag = tag;
+		}
+	}
+}
+
 static void play_next_chunk(void)
 {
 	struct iovec vec[2];
@@ -459,6 +479,8 @@ static void play_next_chunk(void)
 			return;
 		new_song_chunk(a);
 	}
+	send_next_tag();
+
 	/* pcm_volumeChange(a->data, a->len, &ob.audio_format, ob.sw_vol); */
 	if (playAudio(a->data, a->len) < 0)
 		stop_playback();
diff --git a/src/outputBuffer_accessors.h b/src/outputBuffer_accessors.h
index 11c8887c8..2f4116b94 100644
--- a/src/outputBuffer_accessors.h
+++ b/src/outputBuffer_accessors.h
@@ -75,3 +75,14 @@ AudioFormat *ob_audio_format(void)
 {
 	return &ob.audio_format;
 }
+
+mpd_uint8 ob_get_decoder_sequence(void)
+{
+	return ob.seq_decoder;
+}
+
+mpd_uint8 ob_get_player_sequence(void)
+{
+	return ob.seq_player;
+}
+
diff --git a/src/playlist.c b/src/playlist.c
index 4b1042409..48db355a8 100644
--- a/src/playlist.c
+++ b/src/playlist.c
@@ -32,6 +32,7 @@
 #include "myfprintf.h"
 #include "os_compat.h"
 #include "main_notify.h"
+#include "metadata_pipe.h"
 
 enum _playlist_state {
 	PLAYLIST_STATE_STOP = 0,
@@ -966,31 +967,34 @@ int playPlaylistById(int fd, int id, int stopOnError)
 	return playPlaylist(fd, playlist.idToPosition[id], stopOnError);
 }
 
-static void syncCurrentPlayerDecodeMetadata(void)
+/* This is used when we stream data out to shout while playing static files */
+MpdTag *playlist_current_tag(void)
+{
+	Song *song = song_at(playlist.current);
+
+	/* Non-file song tags can get swept out from under us */
+	return (song && song->type == SONG_TYPE_FILE) ? song->tag : NULL;
+}
+
+/* This receives dynamic metadata updates from streams */
+static void sync_metadata(void)
 {
-	Song *songPlayer = song_at(playlist.current);
 	Song *song;
-	int songNum;
-	char path_max_tmp[MPD_PATH_MAX];
+	MpdTag *tag;
 
-	if (!songPlayer)
+	if (!(tag = metadata_pipe_current()))
 		return;
-
-	if (playlist_state != PLAYLIST_STATE_PLAY)
+	song = song_at(playlist.current);
+	if (!song || song->type != SONG_TYPE_URL ||
+	    mpdTagsAreEqual(song->tag, tag)) {
+		freeMpdTag(tag);
 		return;
-
-	songNum = playlist.order[playlist.current];
-	song = playlist.songs[songNum];
-
-	if (song->type == SONG_TYPE_URL &&
-	    0 == strcmp(get_song_url(path_max_tmp, song), songPlayer->url) &&
-	    !mpdTagsAreEqual(song->tag, songPlayer->tag)) {
-		if (song->tag)
-			freeMpdTag(song->tag);
-		song->tag = mpdTagDup(songPlayer->tag);
-		playlist.songMod[songNum] = playlist.version;
-		incrPlaylistVersion();
 	}
+	if (song->tag)
+		freeMpdTag(song->tag);
+	song->tag = tag;
+	playlist.songMod[playlist.order[playlist.current]] = playlist.version;
+	incrPlaylistVersion();
 }
 
 void syncPlayerAndPlaylist(void)
@@ -998,6 +1002,7 @@ void syncPlayerAndPlaylist(void)
 	if (playlist_state != PLAYLIST_STATE_PLAY)
 		return;
 	syncPlaylistWithQueue();
+	sync_metadata();
 	/* DEBUG("queued:%d current:%d\n", playlist.queued, playlist.current); */
 	if (playlist_state == PLAYLIST_STATE_PLAY &&
 	    playlist.queued >= 0 &&