MPEGPlyaer: A bit of audio mutation. Remove a useless thread state. Take some control over the buffer away from the audio thread itself. Some atomicity corrections.

git-svn-id: svn://svn.rockbox.org/rockbox/trunk@28984 a1c6a512-1295-4272-9138-f99709370657

5 files changed, 141 insertions, 84 deletions

diff --git a/apps/plugins/mpegplayer/audio_thread.c b/apps/plugins/mpegplayer/audio_thread.c
index 9e3968007f..ecb8b1ffae 100644
--- a/apps/plugins/mpegplayer/audio_thread.c
+++ b/apps/plugins/mpegplayer/audio_thread.c
@@ -112,7 +112,7 @@ static inline void audiodesc_queue_add_tail(void)
     audio_queue.write++;
 }
 
-/* Increments the queue tail position - leaves one slot as current */
+/* Increments the queue head position - leaves one slot as current */
 static inline bool audiodesc_queue_remove_head(void)
 {
     if (audio_queue.write == audio_queue.read)
@@ -375,8 +375,6 @@ static void audio_thread_msg(struct audio_thread_data *td)
             case TSTATE_INIT:
                 td->state = TSTATE_DECODE;
             case TSTATE_DECODE:
-            case TSTATE_RENDER_WAIT:
-            case TSTATE_RENDER_WAIT_END:
                 break;
 
             case TSTATE_EOS:
@@ -455,7 +453,6 @@ static void audio_thread_msg(struct audio_thread_data *td)
             {
             case TSTATE_DECODE:
             case TSTATE_RENDER_WAIT:
-            case TSTATE_RENDER_WAIT_END:
                 /* These return when in playing state */
                 return;
             }
@@ -512,7 +509,6 @@ static void audio_thread(void)
             /* These states are the only ones that should return */
             case TSTATE_DECODE:          goto audio_decode;
             case TSTATE_RENDER_WAIT:     goto render_wait;
-            case TSTATE_RENDER_WAIT_END: goto render_wait_end;
             /* Anything else is interpreted as an exit */
             default:
             {
@@ -538,29 +534,28 @@ static void audio_thread(void)
         case STREAM_DATA_END:
         {
             if (audio_queue.used > MAD_BUFFER_GUARD)
-                break;
+                break; /* Still have frames to decode */
 
-            /* Used up remainder of compressed audio buffer.
-             * Force any residue to play if audio ended before
-             * reaching the threshold */
-            td.state = TSTATE_RENDER_WAIT_END;
+            /* Used up remainder of compressed audio buffer. Wait for
+             * samples on PCM buffer to finish playing. */
             audio_queue_reset();
 
-        render_wait_end:
-            pcm_output_drain();
-
-            while (pcm_output_used() > (ssize_t)PCMOUT_LOW_WM)
+            while (1)
             {
+                if (pcm_output_empty())
+                {
+                    td.state = TSTATE_EOS;
+                    stream_generate_event(&audio_str, STREAM_EV_COMPLETE, 0);
+                    break;
+                }
+
+                pcm_output_drain();
                 str_get_msg_w_tmo(&audio_str, &td.ev, 1);
+
                 if (td.ev.id != SYS_TIMEOUT)
-                    goto message_process;
+                    break;
             }
 
-            td.state = TSTATE_EOS;
-            if (td.status == STREAM_PLAYING)
-                stream_generate_event(&audio_str, STREAM_EV_COMPLETE, 0);
-
-            rb->yield();
             goto message_wait;
             } /* STREAM_DATA_END: */
         }
@@ -606,11 +601,9 @@ static void audio_thread(void)
 
             /* This is too hard - bail out */
             td.state = TSTATE_EOS;
-
-            if (td.status == STREAM_PLAYING)
-                stream_generate_event(&audio_str, STREAM_EV_COMPLETE, 0);
-
             td.status = STREAM_ERROR;
+            stream_generate_event(&audio_str, STREAM_EV_COMPLETE, 0);
+
             goto message_wait;
         }
 
@@ -643,15 +636,15 @@ static void audio_thread(void)
     render_wait:
         if (synth.pcm.length > 0)
         {
-            struct pcm_frame_header *dst_hdr = pcm_output_get_buffer();
             const char *src[2] =
                 { (char *)synth.pcm.samples[0], (char *)synth.pcm.samples[1] };
             int out_count = (synth.pcm.length * CLOCK_RATE
                                 + (td.samplerate - 1)) / td.samplerate;
-            ssize_t size = sizeof(*dst_hdr) + out_count*4;
+            unsigned char *out_buf;
+            ssize_t size = out_count*4;
 
             /* Wait for required amount of free buffer space */
-            while (pcm_output_free() < size)
+            while ((out_buf = pcm_output_get_buffer(&size)) == NULL)
             {
                 /* Wait one frame */
                 int timeout = out_count*HZ / td.samplerate;
@@ -660,21 +653,17 @@ static void audio_thread(void)
                     goto message_process;
             }
 
-            out_count = rb->dsp_process(td.dsp, dst_hdr->data, src,
-                                        synth.pcm.length);
+            out_count = rb->dsp_process(td.dsp, out_buf, src, synth.pcm.length);
 
             if (out_count <= 0)
                 break;
 
-            dst_hdr->size = sizeof(*dst_hdr) + out_count*4;
-            dst_hdr->time = audio_queue.curr->time;
+            /* Make this data available to DMA */
+            pcm_output_commit_data(out_count*4, audio_queue.curr->time);
 
             /* As long as we're on this timestamp, the time is just
                incremented by the number of samples */
             audio_queue.curr->time += out_count;
-
-            /* Make this data available to DMA */
-            pcm_output_add_data();
         }
 
         rb->yield();
diff --git a/apps/plugins/mpegplayer/mpegplayer.h b/apps/plugins/mpegplayer/mpegplayer.h
index 79c25f6109..7333d87d6e 100644
--- a/apps/plugins/mpegplayer/mpegplayer.h
+++ b/apps/plugins/mpegplayer/mpegplayer.h
@@ -51,14 +51,13 @@
 /* Define this as "1" to have a test tone instead of silence clip */
 #define SILENCE_TEST_TONE 0
 
+/* NOTE: Sizes make no frame header allowance when considering duration */
 #define PCMOUT_BUFSIZE       (CLOCK_RATE/2*4) /* 1/2s */
-#define PCMOUT_GUARD_SAMPLES ((CLOCK_RATE*576+7999)/8000) /* Worst upsampling case */
-#define PCMOUT_GUARD_SIZE    (PCMOUT_GUARD_SAMPLES*4 + sizeof (struct pcm_frame_header))
+#define PCMOUT_GUARD_SIZE    (PCMOUT_BUFSIZE) /* guarantee contiguous sizes */
 #define PCMOUT_ALLOC_SIZE    (PCMOUT_BUFSIZE + PCMOUT_GUARD_SIZE)
                              /* Start pcm playback @ 25% full */
 #define PCMOUT_PLAY_WM       (PCMOUT_BUFSIZE/4)
-                             /* No valid audio frame is smaller */
-#define PCMOUT_LOW_WM        (sizeof (struct pcm_frame_header))
+#define PCMOUT_LOW_WM        (0)
 
 /** disk buffer **/
 #define DISK_BUF_LOW_WATERMARK (1024*1024)
diff --git a/apps/plugins/mpegplayer/pcm_output.c b/apps/plugins/mpegplayer/pcm_output.c
index a5d8f86e5b..0b8ad7701a 100644
--- a/apps/plugins/mpegplayer/pcm_output.c
+++ b/apps/plugins/mpegplayer/pcm_output.c
@@ -36,21 +36,30 @@ static struct pcm_frame_header * ALIGNED_ATTR(4) pcmbuf_tail IBSS_ATTR;
 
 /* Bytes */
 static ssize_t  pcmbuf_curr_size IBSS_ATTR; /* Size of currently playing frame */
-static uint64_t pcmbuf_read      IBSS_ATTR; /* Number of bytes read by DMA */
-static uint64_t pcmbuf_written   IBSS_ATTR; /* Number of bytes written by source */
+static ssize_t  pcmbuf_read      IBSS_ATTR; /* Number of bytes read by DMA */
+static ssize_t  pcmbuf_written   IBSS_ATTR; /* Number of bytes written by source */
 static ssize_t  pcmbuf_threshold IBSS_ATTR; /* Non-silence threshold */
 
 /* Clock */
-static uint32_t clock_base   IBSS_ATTR; /* Our base clock */
 static uint32_t clock_start  IBSS_ATTR; /* Clock at playback start */
-static int32_t  clock_adjust IBSS_ATTR; /* Clock drift adjustment */
+static uint32_t volatile clock_tick IBSS_ATTR; /* Our base clock */
+static uint32_t volatile clock_time IBSS_ATTR; /* Timestamp adjusted */
 
-int pcm_skipped = 0;
-int pcm_underruns = 0;
+static int pcm_skipped = 0;
+static int pcm_underruns = 0;
 
 /* Small silence clip. ~5.80ms @ 44.1kHz */
 static int16_t silence[256*2] ALIGNED_ATTR(4) = { 0 };
 
+/* Delete all buffer contents */
+static void pcm_reset_buffer(void)
+{
+    pcmbuf_threshold = PCMOUT_PLAY_WM;
+    pcmbuf_curr_size = pcmbuf_read = pcmbuf_written = 0;
+    pcmbuf_head = pcmbuf_tail = pcm_buffer;
+    pcm_skipped = pcm_underruns = 0;
+}
+
 /* Advance a PCM buffer pointer by size bytes circularly */
 static inline void pcm_advance_buffer(struct pcm_frame_header **p,
                                       size_t size)
@@ -60,15 +69,16 @@ static inline void pcm_advance_buffer(struct pcm_frame_header **p,
         *p = SKIPBYTES(*p, -PCMOUT_BUFSIZE);
 }
 
-/* Inline internally but not externally */
-inline ssize_t pcm_output_used(void)
+/* Return physical space used */
+static inline ssize_t pcm_output_bytes_used(void)
 {
-    return (ssize_t)(pcmbuf_written - pcmbuf_read);
+    return pcmbuf_written - pcmbuf_read; /* wrap-safe */
 }
 
-inline ssize_t pcm_output_free(void)
+/* Return physical space free */
+static inline ssize_t pcm_output_bytes_free(void)
 {
-    return (ssize_t)(PCMOUT_BUFSIZE - pcmbuf_written + pcmbuf_read);
+    return PCMOUT_BUFSIZE - pcm_output_bytes_used();
 }
 
 /* Audio DMA handler */
@@ -80,7 +90,7 @@ static void get_more(unsigned char **start, size_t *size)
     pcmbuf_read += pcmbuf_curr_size;
     pcmbuf_curr_size = 0;
 
-    sz = pcm_output_used();
+    sz = pcm_output_bytes_used();
 
     if (sz > pcmbuf_threshold)
     {
@@ -89,16 +99,15 @@ static void get_more(unsigned char **start, size_t *size)
         while (1)
         {
             uint32_t time = pcmbuf_head->time;
-            int32_t offset = time - (clock_base + clock_adjust);
+            int32_t offset = time - clock_time;
 
             sz = pcmbuf_head->size;
 
-            if (sz < (ssize_t)(sizeof(pcmbuf_head) + 4) ||
+            if (sz < (ssize_t)(PCM_HDR_SIZE + 4) ||
                 (sz & 3) != 0)
             {
                 /* Just show a warning about this - will never happen
-                 * without a bug in the audio thread code or a clobbered
-                 * buffer */
+                 * without a corrupted buffer */
                 DEBUGF("get_more: invalid size (%ld)\n", (long)sz);
             }
 
@@ -108,11 +117,12 @@ static void get_more(unsigned char **start, size_t *size)
                 pcm_advance_buffer(&pcmbuf_head, sz);
                 pcmbuf_read += sz;
                 pcm_skipped++;
-                if (pcmbuf_read < pcmbuf_written)
+                if (pcm_output_bytes_used() > 0)
                     continue;
 
                 /* Ran out so revert to default watermark */
                 pcmbuf_threshold = PCMOUT_PLAY_WM;
+                pcm_underruns++;
             }
             else if (offset < 100*CLOCK_RATE/1000)
             {
@@ -122,15 +132,15 @@ static void get_more(unsigned char **start, size_t *size)
                 pcm_advance_buffer(&pcmbuf_head, sz);
                 pcmbuf_curr_size = sz;
 
-                sz -= sizeof (struct pcm_frame_header);
+                sz -= PCM_HDR_SIZE;
 
                 *size = sz;
 
                 /* Audio is time master - keep clock synchronized */
-                clock_adjust = time - clock_base;
+                clock_time = time + (sz >> 2);
 
                 /* Update base clock */
-                clock_base += sz >> 2;
+                clock_tick += sz >> 2;
                 return;
             }
             /* Frame will be dropped - play silence clip */
@@ -150,22 +160,57 @@ static void get_more(unsigned char **start, size_t *size)
     *start = (unsigned char *)silence;
     *size = sizeof (silence);
 
-    clock_base += sizeof (silence) / 4;
+    clock_tick += sizeof (silence) / 4;
+    clock_time += sizeof (silence) / 4;
 
-    if (pcmbuf_read > pcmbuf_written)
+    if (sz < 0)
         pcmbuf_read = pcmbuf_written;
 }
 
-struct pcm_frame_header * pcm_output_get_buffer(void)
+/** Public interface **/
+
+/* Return a buffer pointer if at least size bytes are available and if so,
+ * give the actual free space */
+unsigned char * pcm_output_get_buffer(ssize_t *size)
 {
-    return pcmbuf_tail;
+    ssize_t sz = *size;
+    ssize_t free = pcm_output_bytes_free() - PCM_HDR_SIZE;
+
+    if (sz >= 0 && free >= sz)
+    {
+        *size = free; /* return actual free space (- header) */
+        return pcmbuf_tail->data;
+    }
+
+    /* Leave *size alone so caller doesn't have to reinit */
+    return NULL;
 }
 
-void pcm_output_add_data(void)
+/* Commit the buffer returned by pcm_ouput_get_buffer; timestamp is PCM
+ * clock time units, not video format time units */
+bool pcm_output_commit_data(ssize_t size, uint32_t timestamp)
 {
-    size_t size = pcmbuf_tail->size;
+    if (size <= 0 || (size & 3))
+        return false; /* invalid */
+
+    size += PCM_HDR_SIZE;
+
+    if (size > pcm_output_bytes_free())
+        return false; /* too big */
+
+    pcmbuf_tail->size = size;
+    pcmbuf_tail->time = timestamp;
+
     pcm_advance_buffer(&pcmbuf_tail, size);
     pcmbuf_written += size;
+
+    return true;
+}
+
+/* Returns 'true' if the buffer is completely empty */
+bool pcm_output_empty(void)
+{
+    return pcm_output_bytes_used() <= 0;
 }
 
 /* Flushes the buffer - clock keeps counting */
@@ -182,10 +227,7 @@ void pcm_output_flush(void)
     if (playing)
         rb->pcm_play_stop();
 
-    pcmbuf_threshold = PCMOUT_PLAY_WM;
-    pcmbuf_curr_size = pcmbuf_read = pcmbuf_written = 0;
-    pcmbuf_head = pcmbuf_tail = pcm_buffer;
-    pcm_skipped = pcm_underruns = 0;
+    pcm_reset_buffer();
 
     /* Restart if playing state was current */
     if (playing && !paused)
@@ -201,25 +243,54 @@ void pcm_output_set_clock(uint32_t time)
 {
     rb->pcm_play_lock();
 
-    clock_base = time;
     clock_start = time;
-    clock_adjust = 0;
+    clock_tick = time;
+    clock_time = time;
 
     rb->pcm_play_unlock();
 }
 
+/* Return the clock as synchronized by audio frame timestamps */
 uint32_t pcm_output_get_clock(void)
 {
-    return clock_base + clock_adjust
-        - (rb->pcm_get_bytes_waiting() >> 2);
+    uint32_t time, rem;
+
+    /* Reread if data race detected - rem will be 0 if driver hasn't yet
+     * updated to the new buffer size. Also be sure pcm state doesn't
+     * cause indefinite loop.
+     *
+     * FYI: NOT scrutinized for rd/wr reordering on different cores. */
+    do
+    {
+        time = clock_time;
+        rem = rb->pcm_get_bytes_waiting() >> 2;
+    }
+    while (UNLIKELY(time != clock_time ||
+        (rem == 0 && rb->pcm_is_playing() && !rb->pcm_is_paused())));
+
+    return time - rem;
+
 }
 
+/* Return the raw clock as counted from the last pcm_output_set_clock
+ * call */
 uint32_t pcm_output_get_ticks(uint32_t *start)
 {
+    uint32_t tick, rem;
+
+    /* Same procedure as pcm_output_get_clock */
+    do
+    {
+        tick = clock_tick;
+        rem = rb->pcm_get_bytes_waiting() >> 2;
+    }
+    while (UNLIKELY(tick != clock_tick ||
+        (rem == 0 && rb->pcm_is_playing() && !rb->pcm_is_paused())));
+
     if (start)
         *start = clock_start;
 
-    return clock_base - (rb->pcm_get_bytes_waiting() >> 2);
+    return tick - rem;
 }
 
 /* Pauses/Starts pcm playback - and the clock */
@@ -267,12 +338,13 @@ bool pcm_output_init(void)
     if (pcm_buffer == NULL)
         return false;
 
-    pcmbuf_threshold = PCMOUT_PLAY_WM;
-    pcmbuf_head = pcm_buffer;
-    pcmbuf_tail = pcm_buffer;
     pcmbuf_end  = SKIPBYTES(pcm_buffer, PCMOUT_BUFSIZE);
-    pcmbuf_curr_size = pcmbuf_read = pcmbuf_written = 0;
 
+    pcm_reset_buffer();
+
+    /* Some targets could play at the movie frequency without resampling but
+     * as of now DSP assumes a certain frequency (always 44100Hz) so
+     * resampling will be needed for other movie audio rates. */
     rb->pcm_set_frequency(NATIVE_FREQUENCY);
 
 #if INPUT_SRC_CAPS != 0
diff --git a/apps/plugins/mpegplayer/pcm_output.h b/apps/plugins/mpegplayer/pcm_output.h
index 9335235daa..1a00ac48e6 100644
--- a/apps/plugins/mpegplayer/pcm_output.h
+++ b/apps/plugins/mpegplayer/pcm_output.h
@@ -23,6 +23,7 @@
 #ifndef PCM_OUTPUT_H
 #define PCM_OUTPUT_H
 
+#define PCM_HDR_SIZE        (sizeof (struct pcm_frame_header))
 struct pcm_frame_header   /* Header added to pcm data every time a decoded
                              audio frame is sent out */
 {
@@ -31,8 +32,6 @@ struct pcm_frame_header   /* Header added to pcm data every time a decoded
     unsigned char data[]; /* open array of audio data */
 } ALIGNED_ATTR(4);
 
-extern int pcm_skipped, pcm_underruns;
-
 bool pcm_output_init(void);
 void pcm_output_exit(void);
 void pcm_output_flush(void);
@@ -42,9 +41,8 @@ uint32_t pcm_output_get_ticks(uint32_t *start);
 void pcm_output_play_pause(bool play);
 void pcm_output_stop(void);
 void pcm_output_drain(void);
-struct pcm_frame_header * pcm_output_get_buffer(void);
-void pcm_output_add_data(void);
-ssize_t pcm_output_used(void);
-ssize_t pcm_output_free(void);
+unsigned char * pcm_output_get_buffer(ssize_t *size);
+bool pcm_output_commit_data(ssize_t size, uint32_t timestamp);
+bool pcm_output_empty(void);
 
 #endif /* PCM_OUTPUT_H */
diff --git a/apps/plugins/mpegplayer/stream_thread.h b/apps/plugins/mpegplayer/stream_thread.h
index 5791a49e7f..dfa6e8c9a1 100644
--- a/apps/plugins/mpegplayer/stream_thread.h
+++ b/apps/plugins/mpegplayer/stream_thread.h
@@ -73,7 +73,6 @@ enum thread_states
     TSTATE_DECODE,          /* is in a decoding state */
     TSTATE_RENDER,          /* is in a rendering state */
     TSTATE_RENDER_WAIT,     /* is waiting to render */
-    TSTATE_RENDER_WAIT_END, /* is waiting on remaining data */
 };
 
 /* Commands that streams respond to */