diff options
author | Aidan MacDonald <amachronic@protonmail.com> | 2023-03-24 17:36:28 +0000 |
---|---|---|
committer | Aidan MacDonald <amachronic@protonmail.com> | 2023-03-24 19:02:56 +0000 |
commit | 2e99e2175b48cc00274b03bb4aecf5d01403110d (patch) | |
tree | a8573f8b3295372dd432e017bb53e3c2b6dda309 | |
parent | a4cfa8ae6a817da719531a8043ea5f24aa70b67e (diff) | |
download | rockbox-2e99e2175b.tar.gz rockbox-2e99e2175b.zip |
playlist: Remove control file cache
Control cache entries cost 24 bytes per command, but cacheable
commands are always less than that when written out to the file.
We can actually cache *more* data by writing commands directly
to the fd (native Rockbox has a 512-byte cache per fd) and it's
much simpler.
Change-Id: Ibb1b9ffa56ef17431b281419a04082e14d0cbd85
-rw-r--r-- | apps/playlist.c | 257 | ||||
-rw-r--r-- | apps/playlist.h | 15 |
2 files changed, 65 insertions, 207 deletions
diff --git a/apps/playlist.c b/apps/playlist.c index c1d280595c..c122dac048 100644 --- a/apps/playlist.c +++ b/apps/playlist.c @@ -387,178 +387,55 @@ static int rotate_index(const struct playlist_info* playlist, int index) return index; } -/* - * sync control file to disk - */ -static void sync_control(struct playlist_info* playlist, bool force) +static void sync_control_unlocked(struct playlist_info* playlist) { -#ifndef HAVE_DIRCACHE /*non dircache targets sync every time */ - force = true; -#endif - - if (playlist->started && force) - { - if (playlist->pending_control_sync) - { - playlist_write_lock(playlist); - - fsync(playlist->control_fd); - playlist->pending_control_sync = false; - - playlist_write_unlock(playlist); - } - } -} - -static int flush_cached_control_unlocked(struct playlist_info* playlist); - -#if USING_STORAGE_CALLBACK -static void flush_control_cache_idle_cb(unsigned short id, void *ev, void *ud) -{ - (void)id; - (void)ev; - - struct playlist_info *playlist = ud; - playlist_write_lock(playlist); - if (playlist->control_fd >= 0) - flush_cached_control_unlocked(playlist); - - playlist_write_unlock(playlist); + fsync(playlist->control_fd); } -#endif -/* - * Flush any cached control commands to disk. Called when playlist is being - * modified. Returns 0 on success and -1 on failure. - */ -static int flush_cached_control_unlocked(struct playlist_info* playlist) +static int update_control_unlocked(struct playlist_info* playlist, + enum playlist_command command, int i1, int i2, + const char* s1, const char* s2, int *seekpos) { - int result = 0; - - if (playlist->num_cached <= 0) - return 0; + int fd = playlist->control_fd; + int result; - lseek(playlist->control_fd, 0, SEEK_END); + lseek(fd, 0, SEEK_END); - for (int i = 0; i < playlist->num_cached; i++) + switch (command) { - struct playlist_control_cache* cache = &playlist->control_cache[i]; - switch (cache->command) + case PLAYLIST_COMMAND_PLAYLIST: + result = fdprintf(fd, "P:%d:%s:%s\n", i1, s1, s2); + break; + case PLAYLIST_COMMAND_ADD: + case PLAYLIST_COMMAND_QUEUE: + result = fdprintf(fd, "%c:%d:%d:", + command == PLAYLIST_COMMAND_ADD ? 'A' : 'Q', i1, i2); + if (result > 0) { - case PLAYLIST_COMMAND_PLAYLIST: - result = fdprintf(playlist->control_fd, "P:%d:%s:%s\n", - cache->i1, cache->s1, cache->s2); - break; - case PLAYLIST_COMMAND_ADD: - case PLAYLIST_COMMAND_QUEUE: - result = fdprintf(playlist->control_fd, "%c:%d:%d:", - (cache->command == PLAYLIST_COMMAND_ADD)?'A':'Q', - cache->i1, cache->i2); - if (result > 0) - { - /* save the position in file where name is written */ - int* seek_pos = (int *)cache->data; - *seek_pos = lseek(playlist->control_fd, 0, SEEK_CUR); - result = fdprintf(playlist->control_fd, "%s\n", cache->s1); - } - break; - case PLAYLIST_COMMAND_DELETE: - result = fdprintf(playlist->control_fd, "D:%d\n", cache->i1); - break; - case PLAYLIST_COMMAND_SHUFFLE: - result = fdprintf(playlist->control_fd, "S:%d:%d\n", - cache->i1, cache->i2); - break; - case PLAYLIST_COMMAND_UNSHUFFLE: - result = fdprintf(playlist->control_fd, "U:%d\n", cache->i1); - break; - case PLAYLIST_COMMAND_RESET: - result = fdprintf(playlist->control_fd, "%s\n", "R"); - break; - case PLAYLIST_COMMAND_CLEAR: - result = fdprintf(playlist->control_fd, "%s\n", "C"); - break; - default: - break; + *seekpos = lseek(fd, 0, SEEK_CUR); + result = fdprintf(fd, "%s\n", s1); } - - if (result <= 0) - break; - } - - if (result > 0) - { - playlist->num_cached = 0; - playlist->pending_control_sync = true; - result = 0; - } - else - { - /* At this point the control file is likely corrupted. We still - * need to clear the cache to avoid a buffer overflow from the - * next command. It's unsafe to splash() because this function - * can be called off the main thread. - * - * TODO: recover from failed playlist control file writes. - */ - playlist->num_cached = 0; - result = -1; - } - -#if USING_STORAGE_CALLBACK - remove_event_ex(DISK_EVENT_SPINUP, flush_control_cache_idle_cb, playlist); -#endif - return result; -} - -/* - * Update control data with new command. Depending on the command, it may be - * cached or flushed to disk. - */ -static int update_control(struct playlist_info* playlist, - enum playlist_command command, int i1, int i2, - const char* s1, const char* s2, void* data) -{ - int result = 0; - struct playlist_control_cache* cache; - bool flush = false; - - playlist_write_lock(playlist); - cache = &playlist->control_cache[playlist->num_cached++]; - - cache->command = command; - cache->i1 = i1; - cache->i2 = i2; - cache->s1 = s1; - cache->s2 = s2; - cache->data = data; - - switch (command) - { - case PLAYLIST_COMMAND_PLAYLIST: - case PLAYLIST_COMMAND_ADD: - case PLAYLIST_COMMAND_QUEUE: - /* - * These commands can use s1/s2, which may point to - * stack allocated buffers, so flush them immediately. - */ - flush = true; - break; - default: - break; - } - - if (flush || playlist->num_cached == PLAYLIST_MAX_CACHE) - result = flush_cached_control_unlocked(playlist); - else - { -#if USING_STORAGE_CALLBACK - add_event_ex(DISK_EVENT_SPINUP, true, flush_control_cache_idle_cb, playlist); -#endif + break; + case PLAYLIST_COMMAND_DELETE: + result = fdprintf(fd, "D:%d\n", i1); + break; + case PLAYLIST_COMMAND_SHUFFLE: + result = fdprintf(fd, "S:%d:%d\n", i1, i2); + break; + case PLAYLIST_COMMAND_UNSHUFFLE: + result = fdprintf(fd, "U:%d\n", i1); + break; + case PLAYLIST_COMMAND_RESET: + result = write(fd, "R\n", 2); + break; + case PLAYLIST_COMMAND_CLEAR: + result = write(fd, "C\n", 2); + break; + default: + return -1; } - playlist_write_unlock(playlist); return result; } @@ -602,7 +479,6 @@ static void empty_playlist_unlocked(struct playlist_info* playlist, bool resume) playlist->filename[0] = '\0'; playlist->seed = 0; - playlist->num_cached = 0; playlist->utf8 = true; playlist->control_created = false; @@ -617,7 +493,6 @@ static void empty_playlist_unlocked(struct playlist_info* playlist, bool resume) playlist->last_insert_pos = -1; playlist->started = false; - playlist->pending_control_sync = false; if (!resume && playlist == ¤t_playlist) { @@ -713,9 +588,9 @@ static void new_playlist_unlocked(struct playlist_info* playlist, if (playlist->control_fd >= 0) { - update_control(playlist, PLAYLIST_COMMAND_PLAYLIST, + update_control_unlocked(playlist, PLAYLIST_COMMAND_PLAYLIST, PLAYLIST_CONTROL_FILE_VERSION, -1, dirused, fileused, NULL); - sync_control(playlist, false); + sync_control_unlocked(playlist); } } @@ -740,9 +615,9 @@ static int check_control(struct playlist_info* playlist) playlist->filename[playlist->dirlen-1] = '\0'; - update_control(playlist, PLAYLIST_COMMAND_PLAYLIST, + update_control_unlocked(playlist, PLAYLIST_COMMAND_PLAYLIST, PLAYLIST_CONTROL_FILE_VERSION, -1, dir, file, NULL); - sync_control(playlist, false); + sync_control_unlocked(playlist); playlist->filename[playlist->dirlen-1] = c; } } @@ -824,9 +699,8 @@ static int recreate_control_unlocked(struct playlist_info* playlist) playlist->filename[playlist->dirlen-1] = '\0'; - /* cannot call update_control() because of mutex */ - result = fdprintf(playlist->control_fd, "P:%d:%s:%s\n", - PLAYLIST_CONTROL_FILE_VERSION, dir, file); + update_control_unlocked(playlist, PLAYLIST_COMMAND_PLAYLIST, + PLAYLIST_CONTROL_FILE_VERSION, -1, dir, file, NULL); playlist->filename[playlist->dirlen-1] = c; @@ -1381,8 +1255,9 @@ static int remove_all_tracks_unlocked(struct playlist_info *playlist, bool write if (write && playlist->control_fd >= 0) { - update_control(playlist, PLAYLIST_COMMAND_CLEAR, -1, -1, NULL, NULL, NULL); - sync_control(playlist, false); + update_control_unlocked(playlist, PLAYLIST_COMMAND_CLEAR, + -1, -1, NULL, NULL, NULL); + sync_control_unlocked(playlist); } return 0; @@ -1534,7 +1409,7 @@ static int add_track_to_playlist_unlocked(struct playlist_info* playlist, if (seek_pos < 0 && playlist->control_fd >= 0) { - int result = update_control(playlist, + int result = update_control_unlocked(playlist, (queue?PLAYLIST_COMMAND_QUEUE:PLAYLIST_COMMAND_ADD), position, playlist->last_insert_pos, filename, NULL, &seek_pos); @@ -1639,10 +1514,10 @@ static int remove_track_unlocked(struct playlist_info* playlist, if (write && playlist->control_fd >= 0) { - result = update_control(playlist, PLAYLIST_COMMAND_DELETE, + result = update_control_unlocked(playlist, PLAYLIST_COMMAND_DELETE, position, -1, NULL, NULL, NULL); if (result >= 0) - sync_control(playlist, false); + sync_control_unlocked(playlist); } return result; @@ -1720,7 +1595,7 @@ static int randomise_playlist_unlocked(struct playlist_info* playlist, if (write) { - update_control(playlist, PLAYLIST_COMMAND_SHUFFLE, seed, + update_control_unlocked(playlist, PLAYLIST_COMMAND_SHUFFLE, seed, playlist->first_index, NULL, NULL, NULL); } @@ -1785,7 +1660,7 @@ static int sort_playlist_unlocked(struct playlist_info* playlist, if (write && playlist->control_fd >= 0) { playlist->first_index = 0; - update_control(playlist, PLAYLIST_COMMAND_UNSHUFFLE, + update_control_unlocked(playlist, PLAYLIST_COMMAND_UNSHUFFLE, playlist->first_index, -1, NULL, NULL, NULL); } @@ -2144,10 +2019,7 @@ void playlist_shutdown(void) playlist_write_lock(playlist); if (playlist->control_fd >= 0) - { - flush_cached_control_unlocked(playlist); pl_close_control(playlist); - } playlist_write_unlock(playlist); } @@ -2645,7 +2517,7 @@ int playlist_insert_directory(struct playlist_info* playlist, result = playlist_directory_tracksearch(dirname, recurse, directory_search_callback, &context); - sync_control(playlist, false); + sync_control_unlocked(playlist); cpu_boost(false); @@ -2779,7 +2651,7 @@ int playlist_insert_playlist(struct playlist_info* playlist, const char *filenam close(fd); - sync_control(playlist, false); + sync_control_unlocked(playlist); display_playlist_count(count, count_str, true); @@ -3092,15 +2964,16 @@ int playlist_next(int steps) playlist->last_insert_pos = -1; if (playlist->control_fd >= 0) { - int result = update_control(playlist, PLAYLIST_COMMAND_RESET, - -1, -1, NULL, NULL, NULL); - + int result = update_control_unlocked(playlist, + PLAYLIST_COMMAND_RESET, + -1, -1, NULL, NULL, NULL); if (result < 0) { index = result; goto out; } - sync_control(playlist, false); + + sync_control_unlocked(playlist); } } } @@ -3928,11 +3801,6 @@ int playlist_set_current(struct playlist_info* playlist) current_playlist.seed = playlist->seed; current_playlist.modified = playlist->modified; - memcpy(current_playlist.control_cache, playlist->control_cache, - sizeof(current_playlist.control_cache)); - - current_playlist.num_cached = playlist->num_cached; - current_playlist.pending_control_sync = playlist->pending_control_sync; result = 0; out: @@ -4034,7 +3902,7 @@ void playlist_start(int start_index, unsigned long elapsed, playlist->index = start_index; playlist->started = true; - sync_control(playlist, false); + sync_control_unlocked(playlist); playlist_write_unlock(playlist); @@ -4047,7 +3915,12 @@ void playlist_sync(struct playlist_info* playlist) if (!playlist) playlist = ¤t_playlist; - sync_control(playlist, false); + playlist_write_lock(playlist); + + sync_control_unlocked(playlist); + + playlist_write_unlock(playlist); + if ((audio_status() & AUDIO_STATUS_PLAY) && playlist->started) audio_flush_and_reload_tracks(); } diff --git a/apps/playlist.h b/apps/playlist.h index bdc2684c18..d01100af59 100644 --- a/apps/playlist.h +++ b/apps/playlist.h @@ -33,7 +33,6 @@ #define PLAYLIST_ATTR_QUEUED 0x01 #define PLAYLIST_ATTR_INSERTED 0x02 #define PLAYLIST_ATTR_SKIPPED 0x04 -#define PLAYLIST_MAX_CACHE 16 #define PLAYLIST_DISPLAY_COUNT 10 @@ -61,15 +60,6 @@ enum { PLAYLIST_INSERT_LAST_SHUFFLED = -7 }; -struct playlist_control_cache { - enum playlist_command command; - int i1; - int i2; - const char* s1; - const char* s2; - void* data; -}; - struct playlist_info { bool utf8; /* playlist is in .m3u8 format */ @@ -87,14 +77,9 @@ struct playlist_info int amount; /* number of tracks in the index */ int last_insert_pos; /* last position we inserted a track */ bool started; /* has playlist been started? */ - bool pending_control_sync; /* control file needs to be synced */ int last_shuffled_start; /* number of tracks when insert last shuffled command start */ int seed; /* shuffle seed */ - /* cache of playlist control commands waiting to be flushed to - to disk */ - struct playlist_control_cache control_cache[PLAYLIST_MAX_CACHE]; - int num_cached; /* number of cached entries */ struct mutex mutex; /* mutex for control file access */ #ifdef HAVE_DIRCACHE int dcfrefs_handle; |