From 08a4ceaae7ac756b8815b230d12ba7992a0cb120 Mon Sep 17 00:00:00 2001 From: Brandon Low Date: Tue, 17 Oct 2006 12:56:22 +0000 Subject: Remove fill_bytesleft, simplify some buffering code, fix bug 5906 git-svn-id: svn://svn.rockbox.org/rockbox/trunk@11247 a1c6a512-1295-4272-9138-f99709370657 --- apps/playback.c | 220 +++++++++++++++++++++++++------------------------------- 1 file changed, 99 insertions(+), 121 deletions(-) (limited to 'apps/playback.c') diff --git a/apps/playback.c b/apps/playback.c index feba1d1c6c..5cf059f3c6 100644 --- a/apps/playback.c +++ b/apps/playback.c @@ -194,6 +194,7 @@ static volatile size_t buf_widx IDATA_ATTR; /* Ring buffer arithmetic */ #define RINGBUF_ADD(p,v) ((p+v)=v) ? p-v : p+filebuflen-v) +#define RINGBUF_ADD_CROSS(p1,v,p2) ((p1 audio Q_AUDIO_FILL_BUFFER"); queue_post(&audio_queue, Q_AUDIO_FILL_BUFFER, 0); @@ -2034,9 +2032,7 @@ static bool audio_yield_codecs(void) } /* Note that this function might yield(). */ -static void audio_clear_track_entries( - bool clear_buffered, bool clear_unbuffered, - bool may_yield) +static void audio_clear_track_entries(bool clear_unbuffered) { int cur_idx = track_widx; int last_idx = -1; @@ -2056,23 +2052,16 @@ static void audio_clear_track_entries( * otherwise clear the track if that option is selected */ if (tracks[cur_idx].event_sent) { - if (clear_buffered) + if (last_idx >= 0) { - if (last_idx >= 0) - { - /* If there is an unbuffer callback, call it, otherwise, - * just clear the track */ - if (track_unbuffer_callback) - { - if (may_yield) - audio_yield_codecs(); - track_unbuffer_callback(&tracks[last_idx].id3, false); - } - - memset(&tracks[last_idx], 0, sizeof(struct track_info)); - } - last_idx = cur_idx; + /* 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)); } + last_idx = cur_idx; } else if (clear_unbuffered) memset(&tracks[cur_idx], 0, sizeof(struct track_info)); @@ -2118,68 +2107,71 @@ static void audio_strip_id3v1_tag(void) } } -static void audio_read_file(bool quick) +/* Returns true if a whole file is read, false otherwise */ +static bool audio_read_file(size_t minimum) { - size_t copy_n; - int rc; + bool ret_val = false; /* If we're called and no file is open, this is an error */ if (current_fd < 0) { logf("Bad fd in arf"); - /* Stop this buffer cycle immediately */ - fill_bytesleft = 0; /* Give some hope of miraculous recovery by forcing a track reload */ tracks[track_widx].filesize = 0; - return ; + /* Stop this buffering run */ + return ret_val; } trigger_cpu_boost(); while (tracks[track_widx].filerem > 0) { + size_t copy_n; int overlap; - - if (fill_bytesleft == 0) - break ; + int rc; /* copy_n is the largest chunk that is safe to read */ copy_n = MIN(conf_filechunk, filebuflen - buf_widx); - copy_n = MIN(copy_n, fill_bytesleft); + + if (RINGBUF_ADD_CROSS(buf_widx,copy_n,buf_ridx) >= 0) + break; /* rc is the actual amount read */ rc = read(current_fd, &filebuf[buf_widx], copy_n); - if (rc <= 0) + if (rc < 0) { - /* Reached the end of the file */ + logf("File ended %dB early", tracks[track_widx].filerem); + tracks[track_widx].filesize -= tracks[track_widx].filerem; tracks[track_widx].filerem = 0; - break ; + break; } - overlap = buf_widx + rc - CUR_TI->buf_idx; - buf_widx = RINGBUF_ADD(buf_widx, rc); - - if (overlap > 0 && (unsigned) overlap >= filebuflen) - overlap -= filebuflen; - - if (overlap > 0 && overlap <= rc && CUR_TI->available != 0) { - CUR_TI->buf_idx = buf_widx; - CUR_TI->start_pos += overlap; - } - tracks[track_widx].available += rc; tracks[track_widx].filerem -= rc; - if (fill_bytesleft > (unsigned)rc) - fill_bytesleft -= rc; - else - fill_bytesleft = 0; + /* How much of the playing track did we overwrite */ + overlap = RINGBUF_ADD_CROSS(buf_widx,rc,CUR_TI->buf_idx); + + /* Advance buffer */ + buf_widx = RINGBUF_ADD(buf_widx, rc); - /* Let the codec process until it is out of the danger zone, or there - * is an event to handle. In the latter case, break this fill cycle - * immediately */ - if (quick || audio_yield_codecs()) + /* If we write into the playing track, adjust it's buffer info */ + if (overlap > 0 && CUR_TI->available != 0) { + CUR_TI->buf_idx += overlap; + CUR_TI->start_pos += overlap; + } + + /* For a rebuffer, fill at least this minimum */ + if (minimum > (unsigned)rc) + minimum -= rc; + /* Let the codec process up to the watermark */ + /* Break immediately if this is a quick buffer, or there is an event */ + else if (minimum || audio_yield_codecs()) + { + /* Exit quickly, but don't stop the overall buffering process */ + ret_val = true; break; + } } if (tracks[track_widx].filerem == 0) @@ -2193,17 +2185,19 @@ static void audio_read_file(bool quick) track_widx &= MAX_TRACK_MASK; tracks[track_widx].filesize = 0; + return true; } else { - logf("Partially buf:%dB", + logf("%s buf:%dB", ret_val?"Quick":"Partially", tracks[track_widx].filesize - tracks[track_widx].filerem); + return ret_val; } } static bool audio_loadcodec(bool start_play) { - size_t size; + size_t size = 0; int fd; int rc; size_t copy_n; @@ -2215,7 +2209,6 @@ static bool audio_loadcodec(bool start_play) return false; tracks[track_widx].has_codec = false; - tracks[track_widx].codecsize = 0; if (start_play) { @@ -2257,35 +2250,34 @@ static bool audio_loadcodec(bool start_play) return false; } - size = filesize(fd); + tracks[track_widx].codecsize = filesize(fd); /* Never load a partial codec */ - if (fill_bytesleft < size) + if (RINGBUF_ADD_CROSS(buf_widx,tracks[track_widx].codecsize,buf_ridx) >= 0) { logf("Not enough space"); - fill_bytesleft = 0; close(fd); return false; } - while (tracks[track_widx].codecsize < size) + while (size < tracks[track_widx].codecsize) { copy_n = MIN(conf_filechunk, filebuflen - buf_widx); rc = read(fd, &filebuf[buf_widx], copy_n); if (rc < 0) { close(fd); + /* This is an error condition, likely the codec file is corrupt */ + logf("Partial codec loaded"); + /* Must undo the buffer write of the partial codec */ + buf_widx = RINGBUF_SUB(buf_widx, size); + tracks[track_widx].codecsize = 0; return false; } - if (fill_bytesleft > (unsigned)rc) - fill_bytesleft -= rc; - else - fill_bytesleft = 0; - buf_widx = RINGBUF_ADD(buf_widx, rc); - tracks[track_widx].codecsize += rc; + size += rc; } tracks[track_widx].has_codec = true; @@ -2446,35 +2438,28 @@ static bool audio_load_track(int offset, bool start_play, bool rebuffer) tracks[track_widx].codecbuf = &filebuf[buf_widx]; if (!audio_loadcodec(start_play)) { - if (tracks[track_widx].codecsize) - { - /* Must undo the buffer write of the partial codec */ - logf("Partial codec loaded"); - fill_bytesleft += tracks[track_widx].codecsize; - buf_widx = RINGBUF_SUB(buf_widx, tracks[track_widx].codecsize); - tracks[track_widx].codecsize = 0; - } - /* Set filesize to zero to indicate no file was loaded. */ tracks[track_widx].filesize = 0; tracks[track_widx].filerem = 0; close(current_fd); current_fd = -1; - /* Try skipping to next track if there is space. */ - if (fill_bytesleft > 0) + if (tracks[track_widx].codecsize) { - /* This is an error condition unless the fill_bytesleft is 0 */ - snprintf(msgbuf, sizeof(msgbuf)-1, "No codec for: %s", trackname); - /* We should not use gui_syncplash from audio thread! */ - gui_syncsplash(HZ*2, true, msgbuf); - /* Skip invalid entry from playlist. */ - playlist_skip_entry(NULL, last_peek_offset); - tracks[track_widx].taginfo_ready = false; - goto peek_again; + /* No space for codec on buffer, not an error */ + tracks[track_widx].codecsize = 0; + return false; } - - return false; + + /* This is an error condition, either no codec was found, or reading + * the codec file failed part way through, either way, skip the track */ + snprintf(msgbuf, sizeof(msgbuf)-1, "No codec for: %s", trackname); + /* We should not use gui_syncplash from audio thread! */ + gui_syncsplash(HZ*2, true, msgbuf); + /* Skip invalid entry from playlist. */ + playlist_skip_entry(NULL, last_peek_offset); + tracks[track_widx].taginfo_ready = false; + goto peek_again; } tracks[track_widx].start_pos = 0; @@ -2518,9 +2503,7 @@ static bool audio_load_track(int offset, bool start_play, bool rebuffer) logf("alt:%s", trackname); tracks[track_widx].buf_idx = buf_widx; - audio_read_file(rebuffer); - - return true; + return audio_read_file(rebuffer); } static bool audio_read_next_metadata(void) @@ -2612,28 +2595,14 @@ static bool audio_initialize_buffer_fill(bool clear_tracks) if (filling) return true; - /* Don't start buffer fill if buffer is already full. */ - if (FILEBUFUSED > conf_watermark && !filling) - return false; - logf("Starting buffer fill"); - fill_bytesleft = filebuflen - FILEBUFUSED; - /* TODO: This doesn't look right, and might explain some problems with - * seeking in large files (to offsets larger than filebuflen). - * And what about buffer wraps? - * - * This really doesn't look right, so don't use it. - */ - // if (buf_ridx > CUR_TI->buf_idx) - // CUR_TI->start_pos = buf_ridx - CUR_TI->buf_idx; - /* Set the filling flag true before calling audio_clear_tracks as that * function can yield and we start looping. */ filling = true; if (clear_tracks) - audio_clear_track_entries(true, false, true); + audio_clear_track_entries(false); /* Save the current resume position once. */ playlist_update_resume_info(audio_current_track()); @@ -2645,6 +2614,7 @@ static void audio_fill_file_buffer( bool start_play, bool rebuffer, size_t offset) { bool had_next_track = audio_next_track() != NULL; + bool continue_buffering; if (!audio_initialize_buffer_fill(!start_play)) return ; @@ -2652,15 +2622,15 @@ static void audio_fill_file_buffer( /* If we have a partially buffered track, continue loading, * otherwise load a new track */ if (tracks[track_widx].filesize > 0) - audio_read_file(false); - else if (!audio_load_track(offset, start_play, rebuffer)) - fill_bytesleft = 0; + continue_buffering = audio_read_file(rebuffer); + else + continue_buffering = audio_load_track(offset, start_play, rebuffer); if (!had_next_track && audio_next_track()) track_changed = true; /* If we're done buffering */ - if (fill_bytesleft == 0) + if (!continue_buffering) { audio_read_next_metadata(); @@ -2672,6 +2642,7 @@ static void audio_fill_file_buffer( ata_sleep(); #endif } + } static void audio_rebuffer(void) @@ -2708,7 +2679,7 @@ static void audio_rebuffer(void) /* Reset buffer and track pointers */ CUR_TI->buf_idx = buf_ridx = buf_widx = 0; track_widx = track_ridx; - audio_clear_track_entries(true, true, false); + audio_clear_track_entries(true); CUR_TI->available = 0; /* Fill the buffer */ @@ -2893,23 +2864,30 @@ static void audio_rebuffer_and_seek(size_t newpos) last_peek_offset = 0; filling = false; audio_initialize_buffer_fill(true); - filling = true; - if (newpos > conf_preseek) { - buf_ridx = RINGBUF_ADD(buf_ridx, conf_preseek); + if (newpos > conf_preseek) + { CUR_TI->start_pos = newpos - conf_preseek; + CUR_TI->filerem = CUR_TI->filesize - CUR_TI->start_pos; + newpos = conf_preseek; } else { - buf_ridx = RINGBUF_ADD(buf_ridx, newpos); CUR_TI->start_pos = 0; + CUR_TI->filerem = CUR_TI->filesize; } - CUR_TI->filerem = CUR_TI->filesize - CUR_TI->start_pos; CUR_TI->available = 0; lseek(current_fd, CUR_TI->start_pos, SEEK_SET); + audio_read_file(newpos); + + /* Account for the data we just read that is 'behind' us now */ + CUR_TI->available -= newpos; + + buf_ridx = RINGBUF_ADD(buf_ridx, newpos); + LOGFQUEUE("audio > codec Q_CODEC_REQUEST_COMPLETE"); queue_post(&codec_callback_queue, Q_CODEC_REQUEST_COMPLETE, 0); } @@ -2967,7 +2945,7 @@ static void audio_stop_playback(void) } /* Mark all entries null. */ - audio_clear_track_entries(true, false, false); + audio_clear_track_entries(false); memset(tracks, 0, sizeof(struct track_info) * MAX_TRACK); } @@ -3015,13 +2993,13 @@ void audio_invalidate_tracks(void) playlist_end = false; track_widx = track_ridx; - audio_clear_track_entries(true, true, true); + /* Mark all other entries null (also buffered wrong metadata). */ + audio_clear_track_entries(true); /* If the current track is fully buffered, advance the write pointer */ if (tracks[track_widx].filerem == 0) track_widx = (track_widx + 1) & MAX_TRACK_MASK; - /* Mark all other entries null (also buffered wrong metadata). */ buf_widx = RINGBUF_ADD(buf_ridx, CUR_TI->available); audio_read_next_metadata(); @@ -3035,7 +3013,7 @@ static void audio_new_playlist(void) if (audio_have_tracks()) { playlist_end = false; track_widx = track_ridx; - audio_clear_track_entries(true, true, true); + audio_clear_track_entries(true); track_widx++; track_widx &= MAX_TRACK_MASK; @@ -3239,7 +3217,7 @@ static void audio_thread(void) case Q_AUDIO_PLAY: LOGFQUEUE("audio < Q_AUDIO_PLAY"); - audio_clear_track_entries(true, false, true); + audio_clear_track_entries(false); audio_play_start((size_t)ev.data); break ; -- cgit v1.2.3