From d2c65fd068a5c2e7cde52bd06e1eadc7e7e37364 Mon Sep 17 00:00:00 2001
From: Andre Noll <maan@systemlinux.org>
Date: Mon, 3 Feb 2014 20:30:11 +0100
Subject: [PATCH] ao_write: Avoid pthread_join().

The call to pthread_join() may block for hundreds of milliseconds,
depending of the amount of buffered data. This is not acceptable in
a post_select hook. Among other problems this leads to an incorrect time
display and fake "time jump" warnings.

This patch drops the call to pthread_join() and makes ->post_select()
return non-negative until the play thread has terminated. In order to
have a way to tell if the thread is still running we let the thread
(i.e. aow_play()) remove its own buffer tree node on exit instead
of doing this in ->post_select() after pthread_join(). Hence if
->thread_btrn is NULL, we know the thread has terminated. It would
be easier to use pthread_tryjoin_np() but this would not be portable,
as this function is a non-standard GNU extension.
---
 ao_write.c | 53 +++++++++++++++++++++++++++++++++++------------------
 error.h    |  1 +
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/ao_write.c b/ao_write.c
index fc65af60..a2e86ed2 100644
--- a/ao_write.c
+++ b/ao_write.c
@@ -51,23 +51,34 @@ static void aow_pre_select(struct sched *s, struct task *t)
 	struct private_aow_data *pawd = wn->private_data;
 	int ret;
 
-	if (pawd)
-		pthread_mutex_lock(&pawd->mutex);
-	ret = btr_node_status(wn->btrn, wn->min_iqs, BTR_NT_LEAF);
-	if (pawd)
-		pthread_mutex_unlock(&pawd->mutex);
-
-	if (ret == 0) {
-		/*
-		 * Even though the node status is zero, we might have data
-		 * available, but the output buffer is full. If we don't set a
-		 * timeout here, we are woken up only if new data arrives,
-		 * which might be too late and result in a buffer underrun in
-		 * the playing thread. To avoid this we never sleep longer than
-		 * the (default) buffer time.
-		 */
-		return sched_request_timeout_ms(20, s);
+	if (!pawd) { /* not yet started */
+		assert(wn->btrn);
+		ret = btr_node_status(wn->btrn, wn->min_iqs, BTR_NT_LEAF);
+		if (ret != 0)
+			goto min_delay;
+		return; /* no data available */
 	}
+	if (!wn->btrn) { /* EOF */
+		if (!pawd->thread_btrn) /* ready to exit */
+			goto min_delay;
+		/* wait for the play thread to terminate */
+		goto timeout;
+	}
+	pthread_mutex_lock(&pawd->mutex);
+	ret = btr_node_status(wn->btrn, wn->min_iqs, BTR_NT_LEAF);
+	pthread_mutex_unlock(&pawd->mutex);
+	if (ret != 0)
+		goto min_delay;
+	/*
+	 * Even though the node status is zero, we might have data available,
+	 * but the output buffer is full. If we don't set a timeout here, we
+	 * are woken up only if new data arrives, which might be too late and
+	 * result in a buffer underrun in the playing thread. To avoid this we
+	 * never sleep longer than the (default) buffer time.
+	 */
+timeout:
+	return sched_request_timeout_ms(20, s);
+min_delay:
 	sched_min_delay(s);
 }
 
@@ -248,6 +259,7 @@ unlock:
 out:
 	assert(ret < 0);
 	PARA_NOTICE_LOG("%s\n", para_strerror(-ret));
+	btr_remove_node(&pawd->thread_btrn);
 	pthread_exit(NULL);
 }
 
@@ -332,6 +344,12 @@ static int aow_post_select(__a_unused struct sched *s,
 			goto remove_thread_btrn;
 		return 0;
 	}
+	if (!wn->btrn) {
+		if (!pawd->thread_btrn)
+			return -E_AO_EOF;
+		PARA_INFO_LOG("waiting for play thread to terminate\n");
+		return 0;
+	}
 	pthread_mutex_lock(&pawd->mutex);
 	ret = btr_node_status(wn->btrn, wn->min_iqs, BTR_NT_LEAF);
 	if (ret > 0) {
@@ -343,10 +361,9 @@ static int aow_post_select(__a_unused struct sched *s,
 		goto out;
 	pthread_mutex_lock(&pawd->mutex);
 	btr_remove_node(&wn->btrn);
-	PARA_INFO_LOG("waiting for thread to terminate\n");
 	pthread_cond_signal(&pawd->data_available);
 	pthread_mutex_unlock(&pawd->mutex);
-	pthread_join(pawd->thread, NULL);
+	return 0;
 remove_thread_btrn:
 	btr_remove_node(&pawd->thread_btrn);
 remove_btrn:
diff --git a/error.h b/error.h
index 301e2ca5..ba8409df 100644
--- a/error.h
+++ b/error.h
@@ -181,6 +181,7 @@ extern const char **para_errlist[];
 	PARA_ERROR(AO_PLAY, "ao_play() failed"), \
 	PARA_ERROR(AO_BAD_SAMPLE_FORMAT, "ao: unsigned sample formats not supported"), \
 	PARA_ERROR(AO_PTHREAD, "pthread error"), \
+	PARA_ERROR(AO_EOF, "ao: end of file"), \
 
 
 #define COMPRESS_FILTER_ERRORS \
-- 
2.39.5