[MPlayer-dev-eng] [PATCH] ao_nas.c network audio freezes

[Sidik Isani]

Hello again -

  I found a couple of problems with the NAS support.  One can
  cause a permanent hang on a seek, and the other just causes
  general jerkiness because the writer thread was holding a lock
  during the time it was writing to the network.  NAS is working
  great for me now (it even happens to be in sync, which I never
  imagined.)  The patch to fix both problems is attached.  Thanks
  for considering it!

Be seeing you,

- Sidik

P.S.: Network audio presents some interesting challenges for getting
  smooth, synchronized A/V with mplayer, but that's for another message...
-------------- next part --------------
diff -rua MPlayer-cvs/libao2/ao_nas.c MPlayer-isani/libao2/ao_nas.c
--- MPlayer-cvs/libao2/ao_nas.c	Fri May 17 23:15:49 2002
+++ MPlayer-isani/libao2/ao_nas.c	Thu Sep 19 22:57:01 2002
@@ -108,6 +108,7 @@
 	int flow_paused;
 
 	void *client_buffer;
+	void *server_buffer;
 	int client_buffer_size;
 	int client_buffer_used;
 	int server_buffer_size;
@@ -148,15 +149,29 @@
 	if (nas_data->client_buffer_used < num)
 		num = nas_data->client_buffer_used;
 
-	AuWriteElement(nas_data->aud, nas_data->flow, 0, num, nas_data->client_buffer, AuFalse, &as);
+	/*
+	 * It is not appropriate to call AuWriteElement() here because the
+	 * buffer is locked and delays writing to the network will cause
+	 * other threads to block waiting for buffer_mutex.  Instead the
+	 * data is copied to "server_buffer" and written it to the network
+	 * outside of the locked section of code.
+	 *
+	 * (Note: Rather than these two buffers, a single circular buffer
+	 *  could eliminate the memcpy/memmove steps.)
+	 */
+	memcpy(nas_data->server_buffer, nas_data->client_buffer, num);
+
+	nas_data->client_buffer_used -= num;
+	nas_data->server_buffer_used += num;
+	memmove(nas_data->client_buffer, nas_data->client_buffer + num, nas_data->client_buffer_used);
+	pthread_mutex_unlock(&nas_data->buffer_mutex);
+
+	/*
+	 * Now write the new buffer to the network.
+	 */
+	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);
-	else {
-		nas_data->client_buffer_used -= num;
-		nas_data->server_buffer_used += num;
-		memmove(nas_data->client_buffer, nas_data->client_buffer + num, nas_data->client_buffer_used);
-	}
-	pthread_mutex_unlock(&nas_data->buffer_mutex);
 	
 	if (nas_data->flow_paused) {
 		AuPauseFlow(nas_data->aud, nas_data->flow, &as);
@@ -333,6 +348,7 @@
 	nas_data->client_buffer_size = NAS_BUFFER_SIZE;
 	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.samplerate = rate;
 	ao_data.channels = channels;
@@ -403,6 +419,7 @@
 	AuCloseServer(nas_data->aud);
 	nas_data->aud = 0;
 	free(nas_data->client_buffer);
+	free(nas_data->server_buffer);
 }
 
 // stop playing and empty buffers (for seeking/pause)
@@ -478,7 +495,12 @@
 		if (as != AuSuccess)
 			nas_print_error(nas_data->aud, "play(): AuStartFlow", as);
 		nas_data->flow_stopped = 0;
-		while (!nas_empty_event_queue(nas_data)); // wait for first buffer underrun event
+		/*
+		 * Do not continue to wait if there is nothing in the server
+		 * buffer.  (The event never happens and mplayer can freeze.)
+		 */
+		while (nas_data->server_buffer_used > 0 &&
+		       !nas_empty_event_queue(nas_data)); // wait for first buffer underrun event
 	}
 
 	pthread_mutex_lock(&nas_data->buffer_mutex);