From 5a898c15e79ab87d2466e61549fcc20ce115c67e Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Mon, 23 Feb 2009 09:29:56 +0100
Subject: output_api: play() returns a length

The old API required an output plugin to not return until all data
passed to the play() method is consumed.  Some output plugins have to
loop to fulfill that requirement, and may block during that.  Simplify
these, by letting them consume only part of the buffer: make play()
return the length of the consumed data.
---
 src/output/alsa_plugin.c  | 31 +++++++++++--------------------
 src/output/ao_plugin.c    | 20 +++++++-------------
 src/output/fifo_plugin.c  | 17 +++++++----------
 src/output/jack_plugin.c  | 33 +++++++++++++++------------------
 src/output/mvp_plugin.c   | 12 ++++++------
 src/output/null_plugin.c  |  6 +++---
 src/output/oss_plugin.c   | 17 +++++++----------
 src/output/osx_plugin.c   | 45 ++++++++++++++++++---------------------------
 src/output/pulse_plugin.c |  6 +++---
 src/output/shout_plugin.c |  7 +++++--
 src/output_plugin.h       |  6 ++++--
 src/output_thread.c       | 22 +++++++++++++++++-----
 12 files changed, 103 insertions(+), 119 deletions(-)

(limited to 'src')

diff --git a/src/output/alsa_plugin.c b/src/output/alsa_plugin.c
index bbb03e80b..05e097513 100644
--- a/src/output/alsa_plugin.c
+++ b/src/output/alsa_plugin.c
@@ -441,7 +441,7 @@ alsa_close(void *data)
 	mixer_close(ad->mixer);
 }
 
-static bool
+static size_t
 alsa_play(void *data, const char *chunk, size_t size)
 {
 	struct alsa_data *ad = data;
@@ -449,28 +449,19 @@ alsa_play(void *data, const char *chunk, size_t size)
 
 	size /= ad->frame_size;
 
-	while (size > 0) {
+	while (true) {
 		ret = ad->writei(ad->pcm, chunk, size);
-
-		if (ret == -EAGAIN || ret == -EINTR)
-			continue;
-
-		if (ret < 0) {
-			if (alsa_recover(ad, ret) < 0) {
-				g_warning("closing ALSA device \"%s\" due to write "
-					  "error: %s\n",
-					  alsa_device(ad), snd_strerror(-errno));
-				return false;
-			}
-
-			continue;
+		if (ret > 0)
+			return ret * ad->frame_size;
+
+		if (ret < 0 && ret != -EAGAIN && ret != -EINTR &&
+		    alsa_recover(ad, ret) < 0) {
+			g_warning("closing ALSA device \"%s\" due to write "
+				  "error: %s\n",
+				  alsa_device(ad), snd_strerror(-errno));
+			return 0;
 		}
-
-		chunk += ret * ad->frame_size;
-		size -= ret;
 	}
-
-	return true;
 }
 
 const struct audio_output_plugin alsaPlugin = {
diff --git a/src/output/ao_plugin.c b/src/output/ao_plugin.c
index b0662d7a1..c9d10174e 100644
--- a/src/output/ao_plugin.c
+++ b/src/output/ao_plugin.c
@@ -208,29 +208,23 @@ static int ao_play_deconst(ao_device *device, const void *output_samples,
 	return ao_play(device, u.out, num_bytes);
 }
 
-static bool
+static size_t
 audioOutputAo_play(void *data, const char *playChunk, size_t size)
 {
 	AoData *ad = (AoData *)data;
-	size_t chunk_size;
 
 	if (ad->device == NULL)
 		return false;
 
-	while (size > 0) {
-		chunk_size = ad->writeSize > size
-			? size : ad->writeSize;
-
-		if (ao_play_deconst(ad->device, playChunk, chunk_size) == 0) {
-			audioOutputAo_error("Closing libao device due to play error");
-			return false;
-		}
+	if (size > ad->writeSize)
+		size = ad->writeSize;
 
-		playChunk += chunk_size;
-		size -= chunk_size;
+	if (ao_play_deconst(ad->device, playChunk, size) == 0) {
+		audioOutputAo_error("Closing libao device due to play error");
+		return 0;
 	}
 
-	return true;
+	return size;
 }
 
 const struct audio_output_plugin aoPlugin = {
diff --git a/src/output/fifo_plugin.c b/src/output/fifo_plugin.c
index b0c6d12f4..1be0f2a7d 100644
--- a/src/output/fifo_plugin.c
+++ b/src/output/fifo_plugin.c
@@ -237,11 +237,10 @@ static void fifo_dropBufferedAudio(void *data)
 	}
 }
 
-static bool
+static size_t
 fifo_playAudio(void *data, const char *playChunk, size_t size)
 {
 	FifoData *fd = (FifoData *)data;
-	size_t offset = 0;
 	ssize_t bytes;
 
 	if (!fd->timer->started)
@@ -251,8 +250,11 @@ fifo_playAudio(void *data, const char *playChunk, size_t size)
 
 	timer_add(fd->timer, size);
 
-	while (size) {
-		bytes = write(fd->output, playChunk + offset, size);
+	while (true) {
+		bytes = write(fd->output, playChunk, size);
+		if (bytes > 0)
+			return (size_t)bytes;
+
 		if (bytes < 0) {
 			switch (errno) {
 			case EAGAIN:
@@ -265,14 +267,9 @@ fifo_playAudio(void *data, const char *playChunk, size_t size)
 
 			g_warning("Closing FIFO output \"%s\" due to write error: %s",
 				  fd->path, strerror(errno));
-			return false;
+			return 0;
 		}
-
-		size -= bytes;
-		offset += bytes;
 	}
-
-	return true;
 }
 
 const struct audio_output_plugin fifoPlugin = {
diff --git a/src/output/jack_plugin.c b/src/output/jack_plugin.c
index 2bcdf874f..22841f8ef 100644
--- a/src/output/jack_plugin.c
+++ b/src/output/jack_plugin.c
@@ -387,7 +387,7 @@ mpd_jack_write_samples(struct jack_data *jd, const void *src,
 	}
 }
 
-static bool
+static size_t
 mpd_jack_play(void *data, const char *buff, size_t size)
 {
 	struct jack_data *jd = data;
@@ -396,36 +396,33 @@ mpd_jack_play(void *data, const char *buff, size_t size)
 
 	if (jd->shutdown) {
 		g_warning("Refusing to play, because there is no client thread.");
-		return false;
+		return 0;
 	}
 
 	assert(size % frame_size == 0);
 	size /= frame_size;
-	while (size > 0 && !jd->shutdown) {
+
+	while (!jd->shutdown) {
 		space = jack_ringbuffer_write_space(jd->ringbuffer[0]);
 		space1 = jack_ringbuffer_write_space(jd->ringbuffer[1]);
 		if (space > space1)
 			/* send data symmetrically */
 			space = space1;
 
-		space /= sample_size;
-		if (space > 0) {
-			if (space > size)
-				space = size;
-
-			mpd_jack_write_samples(jd, buff, space);
-
-			buff += space * frame_size;
-			size -= space;
-		} else {
-			/* XXX do something more intelligent to
-			   synchronize */
-			g_usleep(1000);
-		}
+		if (space >= frame_size)
+			break;
 
+		/* XXX do something more intelligent to
+		   synchronize */
+		g_usleep(1000);
 	}
 
-	return true;
+	space /= sample_size;
+	if (space < size)
+		size = space;
+
+	mpd_jack_write_samples(jd, buff, size);
+	return size * frame_size;
 }
 
 const struct audio_output_plugin jackPlugin = {
diff --git a/src/output/mvp_plugin.c b/src/output/mvp_plugin.c
index cb570c8ee..840661ea7 100644
--- a/src/output/mvp_plugin.c
+++ b/src/output/mvp_plugin.c
@@ -248,7 +248,7 @@ static void mvp_dropBufferedAudio(void *data)
 	}
 }
 
-static bool
+static size_t
 mvp_playAudio(void *data, const char *playChunk, size_t size)
 {
 	MvpData *md = data;
@@ -258,19 +258,19 @@ mvp_playAudio(void *data, const char *playChunk, size_t size)
 	if (md->fd < 0)
 		mvp_openDevice(md, &md->audio_format);
 
-	while (size > 0) {
+	while (true) {
 		ret = write(md->fd, playChunk, size);
+		if (ret > 0)
+			return (size_t)ret;
+
 		if (ret < 0) {
 			if (errno == EINTR)
 				continue;
 			g_warning("closing mvp PCM device due to write error: "
 				  "%s\n", strerror(errno));
-			return false;
+			return 0;
 		}
-		playChunk += ret;
-		size -= ret;
 	}
-	return true;
 }
 
 const struct audio_output_plugin mvpPlugin = {
diff --git a/src/output/null_plugin.c b/src/output/null_plugin.c
index 1ae55ca9b..2506feb74 100644
--- a/src/output/null_plugin.c
+++ b/src/output/null_plugin.c
@@ -74,14 +74,14 @@ null_close(void *data)
 	}
 }
 
-static bool
+static size_t
 null_play(void *data, G_GNUC_UNUSED const char *chunk, size_t size)
 {
 	struct null_data *nd = data;
 	Timer *timer = nd->timer;
 
 	if (!nd->sync)
-		return true;
+		return size;
 
 	if (!timer->started)
 		timer_start(timer);
@@ -90,7 +90,7 @@ null_play(void *data, G_GNUC_UNUSED const char *chunk, size_t size)
 
 	timer_add(timer, size);
 
-	return true;
+	return size;
 }
 
 static void
diff --git a/src/output/oss_plugin.c b/src/output/oss_plugin.c
index 007d5bc84..eca6f9101 100644
--- a/src/output/oss_plugin.c
+++ b/src/output/oss_plugin.c
@@ -553,7 +553,7 @@ static void oss_dropBufferedAudio(void *data)
 	}
 }
 
-static bool
+static size_t
 oss_playAudio(void *data, const char *playChunk, size_t size)
 {
 	OssData *od = data;
@@ -563,20 +563,17 @@ oss_playAudio(void *data, const char *playChunk, size_t size)
 	if (od->fd < 0 && !oss_open(od))
 		return false;
 
-	while (size > 0) {
+	while (true) {
 		ret = write(od->fd, playChunk, size);
-		if (ret < 0) {
-			if (errno == EINTR)
-				continue;
+		if (ret > 0)
+			return (size_t)ret;
+
+		if (ret < 0 && errno != EINTR) {
 			g_warning("closing oss device \"%s\" due to write error: "
 				  "%s\n", od->device, strerror(errno));
-			return false;
+			return 0;
 		}
-		playChunk += ret;
-		size -= ret;
 	}
-
-	return true;
 }
 
 const struct audio_output_plugin ossPlugin = {
diff --git a/src/output/osx_plugin.c b/src/output/osx_plugin.c
index a97f56982..ffeb8cd5e 100644
--- a/src/output/osx_plugin.c
+++ b/src/output/osx_plugin.c
@@ -256,13 +256,11 @@ osx_openDevice(void *data, struct audio_format *audioFormat)
 	return true;
 }
 
-static bool
+static size_t
 osx_play(void *data, const char *playChunk, size_t size)
 {
 	OsxData *od = data;
-	size_t bytesToCopy;
-	size_t bytes;
-	size_t curpos;
+	size_t start, nbytes;
 
 	if (!od->started) {
 		int err;
@@ -270,42 +268,35 @@ osx_play(void *data, const char *playChunk, size_t size)
 		err = AudioOutputUnitStart(od->au);
 		if (err) {
 			g_warning("unable to start audio output: %i\n", err);
-			return false;
+			return 0;
 		}
 	}
 
 	g_mutex_lock(od->mutex);
 
-	while (size) {
-		curpos = od->pos + od->len;
-		if (curpos >= od->bufferSize)
-			curpos -= od->bufferSize;
+	while (od->len >= od->bufferSize)
+		/* wait for some free space in the buffer */
+		g_cond_wait(od->condition, od->mutex);
 
-		bytesToCopy = MIN(od->bufferSize, size);
+	start = od->pos + od->len;
+	if (start >= od->bufferSize)
+		start -= od->bufferSize;
 
-		while (od->len > od->bufferSize - bytesToCopy) {
-			g_cond_wait(od->condition, od->mutex);
-		}
+	nbytes = start < od->pos
+		? od->pos - start
+		: od->bufferSize - start;
 
-		size -= bytesToCopy;
-		od->len += bytesToCopy;
+	assert(nbytes > 0);
 
-		bytes = od->bufferSize - curpos;
-		if (bytesToCopy > bytes) {
-			memcpy(od->buffer + curpos, playChunk, bytes);
-			curpos = 0;
-			playChunk += bytes;
-			bytesToCopy -= bytes;
-		}
-
-		memcpy(od->buffer + curpos, playChunk, bytesToCopy);
-		playChunk += bytesToCopy;
+	if (nbytes > size)
+		nbytes = size;
 
-	}
+	memcpy(od->buffer + start, playChunk, nbytes);
+	od->len += nbytes;
 
 	g_mutex_unlock(od->mutex);
 
-	return true;
+	return nbytes;
 }
 
 const struct audio_output_plugin osxPlugin = {
diff --git a/src/output/pulse_plugin.c b/src/output/pulse_plugin.c
index 559d2ee56..0203310e7 100644
--- a/src/output/pulse_plugin.c
+++ b/src/output/pulse_plugin.c
@@ -160,7 +160,7 @@ static void pulse_close(void *data)
 	}
 }
 
-static bool
+static size_t
 pulse_play(void *data, const char *playChunk, size_t size)
 {
 	struct pulse_data *pd = data;
@@ -172,10 +172,10 @@ pulse_play(void *data, const char *playChunk, size_t size)
 			  audio_output_get_name(pd->ao),
 			  pa_strerror(error));
 		pulse_close(pd);
-		return false;
+		return 0;
 	}
 
-	return true;
+	return size;
 }
 
 const struct audio_output_plugin pulse_plugin = {
diff --git a/src/output/shout_plugin.c b/src/output/shout_plugin.c
index 7887e2922..2563e8092 100644
--- a/src/output/shout_plugin.c
+++ b/src/output/shout_plugin.c
@@ -413,7 +413,7 @@ my_shout_open_device(void *data, struct audio_format *audio_format)
 	return true;
 }
 
-static bool
+static size_t
 my_shout_play(void *data, const char *chunk, size_t size)
 {
 	struct shout_data *sd = (struct shout_data *)data;
@@ -427,7 +427,10 @@ my_shout_play(void *data, const char *chunk, size_t size)
 		return false;
 	}
 
-	return write_page(sd);
+	if (!write_page(sd))
+		return 0;
+
+	return size;
 }
 
 static bool
diff --git a/src/output_plugin.h b/src/output_plugin.h
index b652dc44e..8a2bcadac 100644
--- a/src/output_plugin.h
+++ b/src/output_plugin.h
@@ -92,8 +92,10 @@ struct audio_output_plugin {
 
 	/**
 	 * Play a chunk of audio data.
+	 *
+	 * @return the number of bytes played, or 0 on error
 	 */
-	bool (*play)(void *data, const char *playChunk, size_t size);
+	size_t (*play)(void *data, const char *chunk, size_t size);
 
 	/**
 	 * Try to cancel data which may still be in the device's
@@ -167,7 +169,7 @@ ao_plugin_send_tag(const struct audio_output_plugin *plugin,
 		plugin->send_tag(data, tag);
 }
 
-static inline bool
+static inline size_t
 ao_plugin_play(const struct audio_output_plugin *plugin,
 	       void *data, const void *chunk, size_t size)
 {
diff --git a/src/output_thread.c b/src/output_thread.c
index 098e1b427..709d8d1a2 100644
--- a/src/output_thread.c
+++ b/src/output_thread.c
@@ -52,9 +52,9 @@ static void ao_play(struct audio_output *ao)
 {
 	const char *data = ao->args.play.data;
 	size_t size = ao->args.play.size;
-	bool ret;
 
 	assert(size > 0);
+	assert(size % audio_format_frame_size(&ao->out_audio_format) == 0);
 
 	if (!audio_format_equals(&ao->in_audio_format, &ao->out_audio_format)) {
 		data = pcm_convert(&ao->convert_state,
@@ -69,10 +69,22 @@ static void ao_play(struct audio_output *ao)
 			return;
 	}
 
-	ret = ao_plugin_play(ao->plugin, ao->data, data, size);
-	if (!ret) {
-		ao_plugin_cancel(ao->plugin, ao->data);
-		ao_close(ao);
+	while (size > 0) {
+		size_t nbytes;
+
+		nbytes = ao_plugin_play(ao->plugin, ao->data, data, size);
+		if (nbytes == 0) {
+			/* play()==0 means failure */
+			ao_plugin_cancel(ao->plugin, ao->data);
+			ao_close(ao);
+			break;
+		}
+
+		assert(nbytes <= size);
+		assert(nbytes % audio_format_frame_size(&ao->out_audio_format) == 0);
+
+		data += nbytes;
+		size -= nbytes;
 	}
 
 	ao_command_finished(ao);
-- 
cgit v1.2.3