Fixes:

- missing check in init
- missing brackets causing failure
- nas_aformat_to_auformat not working properly
- fix hang that was finally reproducible with high disk activity
- don't cut of audio on uninit(), wait for buffer to empty

It also simplifies the event_handler, making it more readable and
implements Sidik Isani's suggestion to make the buffer size dependent on
bytes per second. I've been using it for two days and found no further
problems.

patch by Tobias Diedrich <td@sim.uni-hannover.de>


git-svn-id: svn://svn.mplayerhq.hu/mplayer/trunk@7733 b3059339-0415-0410-9bf9-f77b7e298cf2

1 files changed, 111 insertions, 112 deletions

diff --git a/libao2/ao_nas.c b/libao2/ao_nas.c
index bd299f3793..1b22f1254b 100644
--- a/libao2/ao_nas.c
+++ b/libao2/ao_nas.c
@@ -13,8 +13,8 @@
  * Theory of operation:
  *
  * The NAS consists of two parts, a server daemon and a client.
- * We setup the server to use a buffer of size NAS_BUFFER_SIZE
- * with a low watermark of NAS_BUFFER_SIZE - NAS_FRAG_SIZE.
+ * We setup the server to use a buffer of size bytes_per_second
+ * with a low watermark of buffer_size - NAS_FRAG_SIZE.
  * Upon starting the flow the server will generate a buffer underrun
  * event and the event handler will fill the buffer for the first time.
  * Now the server will generate a lowwater event when the server buffer
@@ -26,8 +26,10 @@
  * accounting of what we think how much of the server buffer is filled)
  */
 
+#include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <pthread.h>
 #include <audio/audiolib.h>
 
@@ -38,8 +40,6 @@
 #include "afmt.h"
 
 #define NAS_FRAG_SIZE 4096
-#define NAS_FRAG_COUNT 8
-#define NAS_BUFFER_SIZE NAS_FRAG_SIZE * NAS_FRAG_COUNT
 
 static char *nas_event_types[] = {
 	"Undefined",
@@ -110,8 +110,7 @@ struct ao_nas_data {
 	AuFlowID	flow;
 	AuDeviceID	dev;
 
-	int flow_stopped;
-	int flow_paused;
+	unsigned int state;
 	int expect_underrun;
 
 	void *client_buffer;
@@ -178,12 +177,6 @@ static int nas_readBuffer(struct ao_nas_data *nas_data, int num)
 	AuWriteElement(nas_data->aud, nas_data->flow, 0, num, nas_data->server_buffer, AuFalse, &as);
 	if (as != AuSuccess) 
 		nas_print_error(nas_data->aud, "nas_readBuffer(): AuWriteElement", as);
-	
-	if (nas_data->flow_paused) {
-		AuPauseFlow(nas_data->aud, nas_data->flow, &as);
-		if (as != AuSuccess)
-			nas_print_error(nas_data->aud, "nas_readBuffer(): AuPauseFlow", as);
-	}
 
 	return num;
 }
@@ -217,13 +210,16 @@ static int nas_empty_event_queue(struct ao_nas_data *nas_data)
 static void *nas_event_thread_start(void *data)
 {
 	struct ao_nas_data *nas_data = data;
-	AuEvent ev;
-	AuBool result;
 
 	do {
+		mp_msg(MSGT_AO, MSGL_DBG2,
+		       "ao_nas: event thread heartbeat (state=%s)\n",
+		       nas_state(nas_data->state));
 		nas_empty_event_queue(nas_data);
-		usleep(10000);
+		usleep(1000);
 	} while (!nas_data->stop_thread);
+
+	return NULL;
 }
 
 static AuBool nas_error_handler(AuServer* aud, AuErrorEvent* ev)
@@ -245,56 +241,53 @@ static AuBool nas_error_handler(AuServer* aud, AuErrorEvent* ev)
 static AuBool nas_event_handler(AuServer *aud, AuEvent *ev, AuEventHandlerRec *hnd)
 {
 	AuElementNotifyEvent *event = (AuElementNotifyEvent *) ev;
-	AuStatus as;
 	struct ao_nas_data *nas_data = hnd->data;
 
-	switch (ev->type) {
-	case AuEventTypeElementNotify:
-		mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: event_handler(): kind %s state %s->%s reason %s numbytes %d expect_underrun %d\n",
-			nas_elementnotify_kind(event->kind),
-			nas_state(event->prev_state),
-			nas_state(event->cur_state),
-			nas_reason(event->reason),
-			event->num_bytes,
-			nas_data->expect_underrun);
-
-		nas_data->server_buffer_used -= event->num_bytes;
-		if (nas_data->server_buffer_used < 0)
-			nas_data->server_buffer_used = 0;
-
-		switch (event->kind) {
-		case AuElementNotifyKindLowWater:
-			nas_readBuffer(nas_data, event->num_bytes);
-			break;
-		case AuElementNotifyKindState:
-			if (event->cur_state == AuStatePause) {
-				switch (event->reason) {
-				case AuReasonUnderrun:
-					// buffer underrun -> refill buffer
-					if (nas_data->expect_underrun)
-						nas_data->expect_underrun = 0;
-					else {
-						mp_msg(MSGT_AO, MSGL_WARN, "ao_nas: Buffer underrun.\n");
-						mp_msg(MSGT_AO, MSGL_HINT, "Possible reasons are network congestion or your NAS server is too slow.\n"
-							"Try renicing your nasd to e.g. -15.\n");
-					}
-					nas_data->server_buffer_used = 0;
-					if (nas_readBuffer(nas_data, nas_data->server_buffer_size - nas_data->server_buffer_used) == 0)
-						nas_data->flow_stopped = 1;
-					break;
-				default:
-					break;
-				}
-			}
-			break;
-		default: // silently ignored
+	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: event_handler(): type %s kind %s state %s->%s reason %s numbytes %d expect_underrun %d\n",
+		nas_event_type(event->type),
+		nas_elementnotify_kind(event->kind),
+		nas_state(event->prev_state),
+		nas_state(event->cur_state),
+		nas_reason(event->reason),
+		event->num_bytes,
+		nas_data->expect_underrun);
+
+	nas_data->server_buffer_used -= event->num_bytes;
+	if (nas_data->server_buffer_used < 0)
+		nas_data->server_buffer_used = 0;
+
+	switch (event->reason) {
+	case AuReasonWatermark:
+		nas_readBuffer(nas_data, event->num_bytes);
+		break;
+	case AuReasonUnderrun:
+		// buffer underrun -> refill buffer
+		nas_data->server_buffer_used = 0;
+		if (nas_data->expect_underrun) {
+			nas_data->expect_underrun = 0;
+		} else {
+			mp_msg(MSGT_AO, MSGL_WARN,
+			       "ao_nas: Buffer underrun.\n");
+			mp_msg(MSGT_AO, MSGL_HINT,
+			       "Possible reasons are:"
+			       "1) Network congestion."
+			       "2) Your NAS server is too slow."
+			       "Try renicing your nasd to e.g. -15.\n");
+		}
+		if (nas_readBuffer(nas_data,
+		                   nas_data->server_buffer_size -
+		                   nas_data->server_buffer_used) != 0) {
+			event->cur_state = AuStateStart;
 			break;
 		}
+		mp_msg(MSGT_AO, MSGL_DBG2,
+			"ao_nas: Can't refill buffer, stopping flow.\n");
+		AuStopFlow(nas_data->aud, nas_data->flow, NULL);
 		break;
-	default: 
-		mp_msg(MSGT_AO, MSGL_WARN, "ao_nas: nas_event_handler(): unhandled event type %d\n", ev->type);
+	default:
 		break;
 	}
+	nas_data->state=event->cur_state;
 	return AuTrue;
 }
 
@@ -311,30 +304,31 @@ static AuDeviceID nas_find_device(AuServer *aud, int nch)
 	return AuNone;
 }
 
-static unsigned int nas_aformat_to_auformat(unsigned int format)
+static unsigned int nas_aformat_to_auformat(unsigned int *format)
 {
-	unsigned int res=format << 8;
-	switch (format) {
+	switch (*format) {
 	case	AFMT_U8:
-		return res + AuFormatLinearUnsigned8;
+		return AuFormatLinearUnsigned8;
 	case	AFMT_S8:
-		return res + AuFormatLinearSigned8;
+		return AuFormatLinearSigned8;
 	case	AFMT_U16_LE:
-		return res + AuFormatLinearUnsigned16LSB;
+		return AuFormatLinearUnsigned16LSB;
 	case	AFMT_U16_BE:
-		return res + AuFormatLinearUnsigned16MSB;
+		return AuFormatLinearUnsigned16MSB;
 #ifndef WORDS_BIGENDIAN
 	default:
+		*format=AFMT_S16_LE;
 #endif
 	case	AFMT_S16_LE:
-		return (AFMT_S16_LE << 8) + AuFormatLinearSigned16LSB;
+		return AuFormatLinearSigned16LSB;
 #ifdef WORDS_BIGENDIAN
 	default:
+		*format=AFMT_S16_BE;
 #endif
 	case	AFMT_S16_BE:
-		return (AFMT_S16_BE << 8) + AuFormatLinearSigned16MSB;
+		return AuFormatLinearSigned16MSB;
 	case	AFMT_MU_LAW:
-		return res + AuFormatULAW8;
+		return AuFormatULAW8;
 	}
 }
 
@@ -349,8 +343,9 @@ static int init(int rate,int channels,int format,int flags)
 {
 	AuElement elms[3];
 	AuStatus as;
-	unsigned char auformat = nas_aformat_to_auformat(format);
+	unsigned char auformat = nas_aformat_to_auformat(&format);
 	int bytes_per_sample = channels * AuSizeofFormat(auformat);
+	int buffer_size;
 	char *server;
 
 	nas_data=malloc(sizeof(struct ao_nas_data));
@@ -359,17 +354,23 @@ static int init(int rate,int channels,int format,int flags)
 	mp_msg(MSGT_AO, MSGL_V, "ao2: %d Hz  %d chans  %s\n",rate,channels,
 		audio_out_format_name(format));
 
-	nas_data->client_buffer_size = NAS_BUFFER_SIZE*2;
-	nas_data->client_buffer = malloc(nas_data->client_buffer_size);
-	nas_data->server_buffer_size = NAS_BUFFER_SIZE;
-	nas_data->server_buffer = malloc(nas_data->server_buffer_size);
-
-	ao_data.format = auformat >> 8;
+	ao_data.format = format;
 	ao_data.samplerate = rate;
 	ao_data.channels = channels;
-	ao_data.buffersize = NAS_BUFFER_SIZE * 2;
 	ao_data.outburst = NAS_FRAG_SIZE;
 	ao_data.bps = rate * bytes_per_sample;
+	buffer_size = ao_data.bps; /* buffer 1 second */
+	/*
+	 * round up to multiple of NAS_FRAG_SIZE
+	 * divide by 3 first because of 2:1 split
+	 */
+	buffer_size = (buffer_size/3 + NAS_FRAG_SIZE-1) & ~(NAS_FRAG_SIZE-1);
+	ao_data.buffersize = buffer_size*3;
+
+	nas_data->client_buffer_size = buffer_size*2;
+	nas_data->client_buffer = malloc(nas_data->client_buffer_size);
+	nas_data->server_buffer_size = buffer_size;
+	nas_data->server_buffer = malloc(nas_data->server_buffer_size);
 
 	if (!bytes_per_sample) {
 		mp_msg(MSGT_AO, MSGL_ERR, "ao_nas: init(): Zero bytes per sample -> nosound\n");
@@ -393,7 +394,7 @@ static int init(int rate,int channels,int format,int flags)
 	while (channels>1) {
 		nas_data->dev = nas_find_device(nas_data->aud, channels);
 		if (nas_data->dev != AuNone &&
-		    (nas_data->flow = AuCreateFlow(nas_data->aud, NULL) != 0))
+		    ((nas_data->flow = AuCreateFlow(nas_data->aud, NULL)) != 0))
 			break;
 		channels--;
 	}
@@ -405,21 +406,25 @@ static int init(int rate,int channels,int format,int flags)
 		return 0;
 	}
 
-	AuMakeElementImportClient(elms, rate, auformat & 0xff, channels, AuTrue,
-				NAS_BUFFER_SIZE / bytes_per_sample,
-				(NAS_BUFFER_SIZE - NAS_FRAG_SIZE) / bytes_per_sample,
-				0, NULL);
+	AuMakeElementImportClient(elms, rate, auformat, channels, AuTrue,
+				buffer_size / bytes_per_sample,
+				(buffer_size - NAS_FRAG_SIZE) /
+				bytes_per_sample, 0, NULL);
 	AuMakeElementExportDevice(elms+1, 0, nas_data->dev, rate,
 				AuUnlimitedSamples, 0, NULL);
 	AuSetElements(nas_data->aud, nas_data->flow, AuTrue, 2, elms, &as);
-	if (as != AuSuccess)
+	if (as != AuSuccess) {
 		nas_print_error(nas_data->aud, "init(): AuSetElements", as);
+		AuCloseServer(nas_data->aud);
+		nas_data->aud = 0;
+		return 0;
+	}
 	AuRegisterEventHandler(nas_data->aud, AuEventHandlerIDMask |
 				AuEventHandlerTypeMask,
 				AuEventTypeElementNotify, nas_data->flow,
 				nas_event_handler, (AuPointer) nas_data);
 	AuSetErrorHandler(nas_data->aud, nas_error_handler);
-	nas_data->flow_stopped=1;
+	nas_data->state=AuStateStop;
 	nas_data->expect_underrun=0;
 
 	pthread_mutex_init(&nas_data->buffer_mutex, NULL);
@@ -430,17 +435,13 @@ static int init(int rate,int channels,int format,int flags)
 
 // close audio device
 static void uninit(){
-	AuStatus as;
 
-	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: uninit()\n");
+	mp_msg(MSGT_AO, MSGL_DBG3, "ao_nas: uninit()\n");
 
+	nas_data->expect_underrun = 1;
+	while (nas_data->state != AuStateStop) usleep(1000);
 	nas_data->stop_thread = 1;
 	pthread_join(nas_data->event_thread, NULL);
-	if (!nas_data->flow_stopped) {
-		AuStopFlow(nas_data->aud, nas_data->flow, &as);
-		if (as != AuSuccess)
-			nas_print_error(nas_data->aud, "uninit(): AuStopFlow", as);
-	}
 	AuCloseServer(nas_data->aud);
 	nas_data->aud = 0;
 	free(nas_data->client_buffer);
@@ -451,45 +452,39 @@ static void uninit(){
 static void reset(){
 	AuStatus as;
 
-	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: reset()\n");
+	mp_msg(MSGT_AO, MSGL_DBG3, "ao_nas: reset()\n");
 
 	pthread_mutex_lock(&nas_data->buffer_mutex);
 	nas_data->client_buffer_used = 0;
-	if (!nas_data->flow_stopped) {
+	pthread_mutex_unlock(&nas_data->buffer_mutex);
+	while (nas_data->state != AuStateStop) {
 		AuStopFlow(nas_data->aud, nas_data->flow, &as);
 		if (as != AuSuccess)
 			nas_print_error(nas_data->aud, "reset(): AuStopFlow", as);
-		nas_data->flow_stopped = 1;
+		usleep(1000);
 	}
-	nas_data->server_buffer_used = 0;
-	AuSync(nas_data->aud, AuTrue);
-	pthread_mutex_unlock(&nas_data->buffer_mutex);
 }
 
 // stop playing, keep buffers (for pause)
 static void audio_pause()
 {
 	AuStatus as;
+	mp_msg(MSGT_AO, MSGL_DBG3, "ao_nas: audio_pause()\n");
 
-	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: audio_pause()\n");
-
-	nas_data->flow_paused = 1;
+	AuStopFlow(nas_data->aud, nas_data->flow, &as);
 }
 
 // resume playing, after audio_pause()
 static void audio_resume()
 {
 	AuStatus as;
-	AuEvent ev;
 
-	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: audio_resume()\n");
+	mp_msg(MSGT_AO, MSGL_DBG3, "ao_nas: audio_resume()\n");
 
-	nas_data->flow_stopped = 0;
-	nas_data->flow_paused = 0;
-	nas_data->expect_underrun = 1;
 	AuStartFlow(nas_data->aud, nas_data->flow, &as);
 	if (as != AuSuccess)
-		nas_print_error(nas_data->aud, "play(): AuStartFlow", as);
+		nas_print_error(nas_data->aud,
+		                "play(): AuStartFlow", as);
 }
 
 
@@ -498,7 +493,7 @@ static int get_space()
 {
 	int result;
 	
-	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: get_space()\n");
+	mp_msg(MSGT_AO, MSGL_DBG3, "ao_nas: get_space()\n");
 
 	pthread_mutex_lock(&nas_data->buffer_mutex);
 	result = nas_data->client_buffer_size - nas_data->client_buffer_used;
@@ -515,7 +510,12 @@ static int play(void* data,int len,int flags)
 	int maxbursts, playbursts, writelen;
 	AuStatus as;
 
-	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: play()\n");
+	mp_msg(MSGT_AO, MSGL_DBG3,
+	       "ao_nas: play(%p, %d, %d)\n",
+	       data, len, flags);
+
+	if (len == 0)
+		return 0;
 
 	pthread_mutex_lock(&nas_data->buffer_mutex);
 	maxbursts = (nas_data->client_buffer_size -
@@ -527,14 +527,13 @@ static int play(void* data,int len,int flags)
 
 	nas_writeBuffer(nas_data, data, writelen);
 
-	if (nas_data->flow_stopped) {
-		AuEvent ev;
-
+	if (nas_data->state != AuStateStart &&
+	    maxbursts == playbursts) {
+		mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: play(): Starting flow.\n");
 		nas_data->expect_underrun = 1;
 		AuStartFlow(nas_data->aud, nas_data->flow, &as);
 		if (as != AuSuccess)
 			nas_print_error(nas_data->aud, "play(): AuStartFlow", as);
-		nas_data->flow_stopped = 0;
 	}
 
 	return writelen;
@@ -545,7 +544,7 @@ static float get_delay()
 {
 	float result;
 	
-	mp_msg(MSGT_AO, MSGL_DBG2, "ao_nas: get_delay()\n");
+	mp_msg(MSGT_AO, MSGL_DBG3, "ao_nas: get_delay()\n");
 
 	pthread_mutex_lock(&nas_data->buffer_mutex);
 	result = ((float)(nas_data->client_buffer_used +