Fix another nasty rebuffering bug, and ensure that buffer and unbuffer events are triggered correctly. Also remove another unused varialbe track_info->filepos

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

2 files changed, 62 insertions, 58 deletions

diff --git a/apps/playback.c b/apps/playback.c
index ccdaced..43a13ba 100644
--- a/apps/playback.c
+++ b/apps/playback.c
@@ -225,6 +225,9 @@ static bool v1first = false;
 static void mp3_set_elapsed(struct mp3entry* id3);
 int mp3_get_file_pos(void);
 
+static void audio_clear_track_entries(bool buffered_only);
+static void initialize_buffer_fill(bool start_play);
+
 #ifdef TIME_CODEC
 bool is_filling(void)
 {
@@ -633,16 +636,12 @@ void* codec_request_buffer_callback(size_t *realsize, size_t reqsize)
 static void rebuffer_and_seek(size_t newpos)
 {
     int fd;
-    size_t filepos;
-
-    /* Prevent multiple rebuffer calls on codecs that 'hunt' when seeking */
-    if (newpos > AUDIO_REBUFFER_GUESS_SIZE)
-        filepos = newpos - AUDIO_REBUFFER_GUESS_SIZE;
-    else
-        filepos = 0;
+    char *trackname;
 
+    trackname = playlist_peek(0);
     /* (Re-)open current track's file handle. */
-    fd = open(playlist_peek(0), O_RDONLY);
+
+    fd = open(trackname, O_RDONLY);
     if (fd < 0) {
         logf("Open failed!");
         mutex_unlock(&mutex_rebuffer);
@@ -652,19 +651,28 @@ static void rebuffer_and_seek(size_t newpos)
         close(current_fd);
     current_fd = fd;
 
+    playlist_end = false;
+
+    ci.curpos = newpos;
+
     /* Clear codec buffer. */
-    audio_invalidate_tracks();
     filebufused = 0;
-    playlist_end = false;
     buf_ridx = buf_widx = 0;
-    cur_ti->filerem = cur_ti->filesize - filepos;
-    cur_ti->filepos = filepos;
-    cur_ti->start_pos = filepos;
-    ci.curpos = newpos;
+
+    /* Make sure we are reading the cur_ti */
+    while (&tracks[track_ridx] != cur_ti)
+        if (--track_ridx < 0)
+            track_ridx += MAX_TRACK;
+    /* Write to the now current track */
+    track_widx = track_ridx;
+
+    initialize_buffer_fill(false);
+
+    cur_ti->filerem = cur_ti->filesize - cur_ti->start_pos;
     cur_ti->available = 0;
-    lseek(current_fd, filepos, SEEK_SET);
 
-    queue_post(&audio_queue, Q_AUDIO_FILL_BUFFER, 0);
+    lseek(current_fd, cur_ti->start_pos, SEEK_SET);
+
     mutex_unlock(&mutex_rebuffer);
 }
 
@@ -745,8 +753,8 @@ bool codec_seek_buffer_callback(size_t newpos)
         newpos = cur_ti->filesize - 1;
 
     difference = newpos - ci.curpos;
-    /* Seeking forward */
     if (difference >= 0) {
+        /* Seeking forward */
         logf("seek: +%d", difference);
         codec_advance_buffer_callback(difference);
         return true;
@@ -760,7 +768,10 @@ bool codec_seek_buffer_callback(size_t newpos)
     /* We need to reload the song. */
     if (newpos < cur_ti->start_pos)
     {
+        mutex_lock(&mutex_rebuffer);
         queue_post(&audio_queue, Q_AUDIO_REBUFFER_SEEK, (void *)newpos);
+        mutex_lock(&mutex_rebuffer);
+        mutex_unlock(&mutex_rebuffer);
         return true;
     }
 
@@ -943,7 +954,6 @@ static void audio_read_file(void)
             buf_widx -= filebuflen;
         tracks[track_widx].available += rc;
         tracks[track_widx].filerem -= rc;
-        tracks[track_widx].filepos += rc;
         filebufused += rc;
         fill_bytesleft -= rc;
 
@@ -1126,18 +1136,17 @@ static bool read_next_metadata(void)
 {
     int fd;
     char *trackname;
-    int next_track;
+    int next_idx;
     int status;
 
-    next_track = track_widx;
-    if (tracks[track_widx].taginfo_ready)
-        next_track++;
-
-    if (next_track >= MAX_TRACK)
-        next_track -= MAX_TRACK;
-
-    if (tracks[next_track].taginfo_ready)
-        return true;
+    next_idx = track_widx;
+    if (tracks[next_idx].taginfo_ready)
+    {
+        if (++next_idx >= MAX_TRACK)
+            next_idx -= MAX_TRACK;
+        if (tracks[next_idx].taginfo_ready)
+            return true;
+    }
 
     trackname = playlist_peek(last_peek_offset + 1);
     if (!trackname)
@@ -1147,19 +1156,15 @@ static bool read_next_metadata(void)
     if (fd < 0)
         return false;
 
-    /** Start buffer refilling also because we need to spin-up the disk.
-     * In fact, it might be better not to start filling here, because if user
-     * is manipulating the playlist a lot, we will just lose battery. */
-    // filling = true;
-    status = get_metadata(&tracks[next_track],fd,trackname,v1first);
+    status = get_metadata(&tracks[next_idx],fd,trackname,v1first);
     /* Preload the glyphs in the tags */
     if (status) {
-        if (tracks[next_track].id3.title)
-            lcd_getstringsize(tracks[next_track].id3.title, NULL, NULL);
-        if (tracks[next_track].id3.artist)
-            lcd_getstringsize(tracks[next_track].id3.artist, NULL, NULL);
-        if (tracks[next_track].id3.album)
-            lcd_getstringsize(tracks[next_track].id3.album, NULL, NULL);
+        if (tracks[next_idx].id3.title)
+            lcd_getstringsize(tracks[next_idx].id3.title, NULL, NULL);
+        if (tracks[next_idx].id3.artist)
+            lcd_getstringsize(tracks[next_idx].id3.artist, NULL, NULL);
+        if (tracks[next_idx].id3.album)
+            lcd_getstringsize(tracks[next_idx].id3.album, NULL, NULL);
     }
     track_changed = true;
     close(fd);
@@ -1210,7 +1215,6 @@ static bool audio_load_track(int offset, bool start_play, int peek_offset)
     size = filesize(current_fd);
     tracks[track_widx].filerem = size;
     tracks[track_widx].filesize = size;
-    tracks[track_widx].filepos = 0;
     tracks[track_widx].available = 0;
     tracks[track_widx].playlist_offset = peek_offset;
     last_peek_offset = peek_offset;
@@ -1229,7 +1233,7 @@ static bool audio_load_track(int offset, bool start_play, int peek_offset)
     /* Get track metadata if we don't already have it. */
     if (!tracks[track_widx].taginfo_ready) {
         if (!get_metadata(&tracks[track_widx],current_fd,trackname,v1first)) {
-            logf("Metadata error!");
+            logf("Metadata error:%s!",trackname);
             tracks[track_widx].filesize = 0;
             tracks[track_widx].filerem = 0;
             tracks[track_widx].taginfo_ready = false;
@@ -1276,7 +1280,6 @@ static bool audio_load_track(int offset, bool start_play, int peek_offset)
             lseek(current_fd, offset, SEEK_SET);
             tracks[track_widx].id3.offset = offset;
             mp3_set_elapsed(&tracks[track_widx].id3);
-            tracks[track_widx].filepos = offset;
             tracks[track_widx].filerem = size - offset;
             ci.curpos = offset;
             tracks[track_widx].start_pos = offset;
@@ -1287,7 +1290,6 @@ static bool audio_load_track(int offset, bool start_play, int peek_offset)
             tracks[track_widx].id3.offset = offset;
             tracks[track_widx].id3.elapsed =
                 tracks[track_widx].id3.length / 2;
-            tracks[track_widx].filepos = offset;
             tracks[track_widx].filerem = size - offset;
             ci.curpos = offset;
             tracks[track_widx].start_pos = offset;
@@ -1311,26 +1313,29 @@ static bool audio_load_track(int offset, bool start_play, int peek_offset)
 
 static void audio_clear_track_entries(bool buffered_only)
 {
-    int cur_idx;
-    int last_idx = 0;
+    int cur_idx = track_widx;
+    int last_idx = -1;
     
-    /* Loop over all tracks from write index to read index. */
-    cur_idx = track_widx;
+    logf("Clearing tracks:%d/%d", track_ridx, track_widx);
+    /* Loop over all tracks from write-to-read */
     while (1) {
         if (++cur_idx >= MAX_TRACK)
-            cur_idx = 0;
+            cur_idx -= MAX_TRACK;
         if (cur_idx == track_ridx)
             break;
 
         /* If the track is buffered, conditionally clear/notify,
          * otherwise clear the track if that option is selected */
         if (tracks[cur_idx].event_sent) {
-            /* If there is an unbuffer callback, call it, otherwise, just
-             * clear the track */
-            if (track_unbuffer_callback)
-                track_unbuffer_callback(&tracks[last_idx].id3, false);
+            if (last_idx >= 0)
+            {
+                /* If there is an unbuffer callback, call it, otherwise, just
+                 * clear the track */
+                if (track_unbuffer_callback)
+                    track_unbuffer_callback(&tracks[last_idx].id3, false);
 
-            memset(&tracks[last_idx], 0, sizeof(struct track_info));
+                memset(&tracks[last_idx], 0, sizeof(struct track_info));
+            }
             last_idx = cur_idx;
         } else if (!buffered_only)
             memset(&tracks[cur_idx], 0, sizeof(struct track_info));
@@ -1339,7 +1344,7 @@ static void audio_clear_track_entries(bool buffered_only)
     /* We clear the previous instance of a buffered track throughout
      * the above loop to facilitate 'last' detection.  Clear/notify
      * the last track here */
-    if (last_idx)
+    if (last_idx >= 0)
     {
         if (track_unbuffer_callback)
             track_unbuffer_callback(&tracks[last_idx].id3, true);
@@ -1401,13 +1406,13 @@ static void audio_play_start(size_t offset)
 static void generate_postbuffer_events(void)
 {
     int cur_idx;
-    int last_idx = 0;
+    int last_idx = -1;
 
     if (have_tracks()) {
         cur_idx = track_ridx;
         while (1) {
             if (!tracks[cur_idx].event_sent) {
-                if (last_idx) {
+                if (last_idx >= 0) {
                     /* Mark the event 'sent' even if we don't really send one */
                     tracks[last_idx].event_sent = true;
                     if (track_buffer_callback)
@@ -1421,7 +1426,7 @@ static void generate_postbuffer_events(void)
                 cur_idx -= MAX_TRACK;
         }
 
-        if (last_idx) {
+        if (last_idx >= 0) {
             tracks[last_idx].event_sent = true;
             if (track_buffer_callback)
                 track_buffer_callback(&tracks[last_idx].id3, true);
diff --git a/apps/playback.h b/apps/playback.h
index 2504652..46fbf77 100644
--- a/apps/playback.h
+++ b/apps/playback.h
@@ -41,7 +41,6 @@ struct track_info {
     
     size_t filerem;            /* Remaining bytes of file NOT in buffer */
     size_t filesize;           /* File total length */
-    size_t filepos;            /* Read position of file for next buffer fill */
     size_t start_pos;          /* Position to first bytes of file in buffer */
     volatile size_t available; /* Available bytes to read from buffer */
     bool taginfo_ready;        /* Is metadata read */