[MPlayer-dev-eng] [PATCH] use libavutil fifo.h in ao_macosx.c

[Reimar Döffinger]

Hello,
I can not test it but this patch changes ao_macosx to use
libavutil/fifo.h like ao_sdl and ao_jack.
I think it could generally use some love, e.g. there is quite
a bit of duplicated code, the write_buffer and read_buffer return
values are incorrectly ignored, malloc results are uselessly cast,
and there are some pointless if (a) free(a); constructs. A close review
probably could find more...

Greetings,
Reimar Döffinger
-------------- next part --------------
Index: libao2/ao_macosx.c
===================================================================
--- libao2/ao_macosx.c	(revision 28916)
+++ libao2/ao_macosx.c	(working copy)
@@ -52,6 +52,7 @@
 #include "audio_out_internal.h"
 #include "libaf/af_format.h"
 #include "osdep/timer.h"
+#include "libavutil/fifo.h"
 
 static const ao_info_t info =
   {
@@ -91,82 +92,30 @@
   int paused;
 
   /* Ring-buffer */
-  /* does not need explicit synchronization, but needs to allocate
-   * (num_chunks + 1) * chunk_size memory to store num_chunks * chunk_size
-   * data */
-  unsigned char *buffer;
-  unsigned int buffer_len; ///< must always be (num_chunks + 1) * chunk_size
+  AVFifoBuffer *buffer;
+  unsigned int buffer_len; ///< must always be num_chunks * chunk_size
   unsigned int num_chunks;
   unsigned int chunk_size;
-  
-  unsigned int buf_read_pos;
-  unsigned int buf_write_pos;
 } ao_macosx_t;
 
 static ao_macosx_t *ao = NULL;
 
 /**
- * \brief return number of free bytes in the buffer
- *    may only be called by mplayer's thread
- * \return minimum number of free bytes in buffer, value may change between
- *    two immediately following calls, and the real number of free bytes
- *    might actually be larger!
- */
-static int buf_free(void) {
-  int free = ao->buf_read_pos - ao->buf_write_pos - ao->chunk_size;
-  if (free < 0) free += ao->buffer_len;
-  return free;
-}
-
-/**
- * \brief return number of buffered bytes
- *    may only be called by playback thread
- * \return minimum number of buffered bytes, value may change between
- *    two immediately following calls, and the real number of buffered bytes
- *    might actually be larger!
- */
-static int buf_used(void) {
-  int used = ao->buf_write_pos - ao->buf_read_pos;
-  if (used < 0) used += ao->buffer_len;
-  return used;
-}
-
-/**
  * \brief add data to ringbuffer
  */
 static int write_buffer(unsigned char* data, int len){
-  int first_len = ao->buffer_len - ao->buf_write_pos;
-  int free = buf_free();
+  int free = ao->buffer_len - av_fifo_size(ao->buffer);
   if (len > free) len = free;
-  if (first_len > len) first_len = len;
-  // till end of buffer
-  memcpy (&ao->buffer[ao->buf_write_pos], data, first_len);
-  if (len > first_len) { // we have to wrap around
-    // remaining part from beginning of buffer
-    memcpy (ao->buffer, &data[first_len], len - first_len);
-  }
-  ao->buf_write_pos = (ao->buf_write_pos + len) % ao->buffer_len;
-  return len;
+  return av_fifo_generic_write(ao->buffer, data, len, NULL);
 }
 
 /**
  * \brief remove data from ringbuffer
  */
 static int read_buffer(unsigned char* data,int len){
-  int first_len = ao->buffer_len - ao->buf_read_pos;
-  int buffered = buf_used();
+  int buffered = av_fifo_size(ao->buffer);
   if (len > buffered) len = buffered;
-  if (first_len > len) first_len = len;
-  // till end of buffer
-  if (data) {
-    memcpy (data, &ao->buffer[ao->buf_read_pos], first_len);
-    if (len > first_len) { // we have to wrap around
-      // remaining part from beginning of buffer
-      memcpy (&data[first_len], ao->buffer, len - first_len);
-    }
-  }
-  ao->buf_read_pos = (ao->buf_read_pos + len) % ao->buffer_len;
-  return len;
+  return av_fifo_generic_read(ao->buffer, data, len, NULL);
 }
 
 OSStatus theRenderProc(void *inRefCon, AudioUnitRenderActionFlags *inActionFlags, const AudioTimeStamp *inTimeStamp, UInt32 inBusNumber, UInt32 inNumFrames, AudioBufferList *ioData)
@@ -493,8 +442,8 @@
 	ao_data.buffersize = ao_data.bps;
 
 	ao->num_chunks = (ao_data.bps+ao->chunk_size-1)/ao->chunk_size;
-    ao->buffer_len = (ao->num_chunks + 1) * ao->chunk_size;
-    ao->buffer = calloc(ao->num_chunks + 1, ao->chunk_size);
+    ao->buffer_len = ao->num_chunks * ao->chunk_size;
+    ao->buffer = av_fifo_alloc(ao->buffer_len);
 	
 	ao_msg(MSGT_AO,MSGL_V, "using %5d chunks of %d bytes (buffer len %d bytes)\n", (int)ao->num_chunks, (int)ao->chunk_size, (int)ao->buffer_len);
 
@@ -506,8 +455,6 @@
 		goto err_out2;
 	}
 
-	reset();
-    
     return CONTROL_OK;
 
 err_out2:
@@ -515,7 +462,7 @@
 err_out1:
     CloseComponent(ao->theOutputUnit);
 err_out:
-    free(ao->buffer);
+    av_fifo_free(ao->buffer);
     free(ao);
     ao = NULL;
     return CONTROL_FALSE; 
@@ -737,8 +684,8 @@
     ao_data.buffersize = ao_data.bps;
 
     ao->num_chunks = (ao_data.bps+ao->chunk_size-1)/ao->chunk_size;
-    ao->buffer_len = (ao->num_chunks + 1) * ao->chunk_size;
-    ao->buffer = calloc(ao->num_chunks + 1, ao->chunk_size);
+    ao->buffer_len = ao->num_chunks * ao->chunk_size;
+    ao->buffer = av_fifo_alloc(ao->buffer_len);
 
     ao_msg(MSGT_AO,MSGL_V, "using %5d chunks of %d bytes (buffer len %d bytes)\n", (int)ao->num_chunks, (int)ao->chunk_size, (int)ao->buffer_len);
 
@@ -753,8 +700,6 @@
         goto err_out1;
     }
 
-    reset();
-
     return CONTROL_TRUE;
 
 err_out1:
@@ -782,7 +727,7 @@
             ao_msg(MSGT_AO, MSGL_WARN, "Could not release hogmode: [%4.4s]\n",
                    (char *)&err);
     }
-    free(ao->buffer);
+    av_fifo_free(ao->buffer);
     free(ao);
     ao = NULL;
     return CONTROL_FALSE; 
@@ -1030,27 +975,22 @@
 static void reset(void)
 {
   audio_pause();
-  /* reset ring-buffer state */
-  ao->buf_read_pos=0;
-  ao->buf_write_pos=0;
-  
-  return;
+  av_fifo_reset(ao->buffer);
 }
 
 
 /* return available space */
 static int get_space(void)
 {
-  return buf_free();
+  return ao->buffer_len - av_fifo_size(ao->buffer);
 }
 
 
 /* return delay until audio is played */
 static float get_delay(void)
 {
-  int buffered = ao->buffer_len - ao->chunk_size - buf_free(); // could be less
   // inaccurate, should also contain the data buffered e.g. by the OS
-  return (float)(buffered)/(float)ao_data.bps;
+  return (float)av_fifo_size(ao->buffer)/(float)ao_data.bps;
 }
 
 
@@ -1115,7 +1055,7 @@
       }
   }
 
-  free(ao->buffer);
+  av_fifo_free(ao->buffer);
   free(ao);
   ao = NULL;
 }