From 2a40d420120c051b9ee79a0086059b5f2059a013 Mon Sep 17 00:00:00 2001 From: Aidan MacDonald Date: Sat, 21 Jan 2023 18:18:37 +0000 Subject: playlist: Refactor control cache flush Make background control file flushing work on non-dircache targets. It has nothing to do with dircache and doesn't belong in the dircache scan thread. Change-Id: I3f39261ccaebb5dce69e7db3d2e0c0c0c54f640b --- apps/playlist.c | 101 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 53 insertions(+), 48 deletions(-) (limited to 'apps/playlist.c') diff --git a/apps/playlist.c b/apps/playlist.c index 77e3d20151..48933664a0 100644 --- a/apps/playlist.c +++ b/apps/playlist.c @@ -368,26 +368,40 @@ static void sync_control(struct playlist_info* playlist, bool force) } } +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_mutex_lock(&playlist->mutex); + + if (playlist->control_fd >= 0) + flush_cached_control_unlocked(playlist); + + playlist_mutex_unlock(&playlist->mutex); +} +#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(struct playlist_info* playlist) +static int flush_cached_control_unlocked(struct playlist_info* playlist) { int result = 0; - int i; if (playlist->num_cached <= 0) return 0; - playlist_mutex_lock(&(playlist->mutex)); - lseek(playlist->control_fd, 0, SEEK_END); - for (i=0; inum_cached; i++) + for (int i = 0; i < playlist->num_cached; i++) { - struct playlist_control_cache* cache = &(playlist->control_cache[i]); - + struct playlist_control_cache* cache = &playlist->control_cache[i]; switch (cache->command) { case PLAYLIST_COMMAND_PLAYLIST: @@ -432,16 +446,24 @@ static int flush_cached_control(struct playlist_info* playlist) { 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; - splash(HZ*2, ID2P(LANG_PLAYLIST_CONTROL_UPDATE_ERROR)); } - playlist_mutex_unlock(&(playlist->mutex)); +#if USING_STORAGE_CALLBACK + remove_event_ex(DISK_EVENT_SPINUP, flush_control_cache_idle_cb, playlist); +#endif return result; } @@ -457,9 +479,8 @@ static int update_control(struct playlist_info* playlist, struct playlist_control_cache* cache; bool flush = false; - playlist_mutex_lock(&(playlist->mutex)); - - cache = &(playlist->control_cache[playlist->num_cached++]); + playlist_mutex_lock(&playlist->mutex); + cache = &playlist->control_cache[playlist->num_cached++]; cache->command = command; cache->i1 = i1; @@ -473,23 +494,26 @@ static int update_control(struct playlist_info* playlist, case PLAYLIST_COMMAND_PLAYLIST: case PLAYLIST_COMMAND_ADD: case PLAYLIST_COMMAND_QUEUE: -#ifndef HAVE_DIRCACHE - case PLAYLIST_COMMAND_DELETE: - case PLAYLIST_COMMAND_RESET: -#endif + /* + * These commands can use s1/s2, which may point to + * stack allocated buffers, so flush them immediately. + */ flush = true; break; - case PLAYLIST_COMMAND_SHUFFLE: - case PLAYLIST_COMMAND_UNSHUFFLE: - default: /* only flush when needed */ + default: break; } if (flush || playlist->num_cached == PLAYLIST_MAX_CACHE) - result = flush_cached_control(playlist); - - playlist_mutex_unlock(&(playlist->mutex)); + 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 + } + playlist_mutex_unlock(&playlist->mutex); return result; } @@ -1885,30 +1909,16 @@ static int get_next_index(const struct playlist_info* playlist, int steps, } #ifdef HAVE_DIRCACHE -static void dc_flush_playlist_callback(void) -{ - struct playlist_info *playlist; - playlist = ¤t_playlist; - if (playlist->control_fd >= 0) - { - flush_cached_control(playlist); - sync_control(playlist, true); - } - else if (playlist->control_fd < 0 || playlist->num_cached <= 0) - unregister_storage_idle_func(dc_flush_playlist_callback, false); -} - /** * Thread to update filename pointers to dircache on background - * without affecting playlist load up performance. This thread also flushes - * any pending control commands when the disk spins up. + * without affecting playlist load up performance. */ static void dc_thread_playlist(void) { struct queue_event ev; static char tmp[MAX_PATH+1]; - struct playlist_info *playlist; + struct playlist_info *playlist = ¤t_playlist; struct dircache_fileref *dcfrefs; int index; int seek; @@ -1941,13 +1951,6 @@ static void dc_thread_playlist(void) timeout or 5s, whichever is less */ case SYS_TIMEOUT: { - playlist = ¤t_playlist; - if (playlist->control_fd >= 0) - { - if (playlist->num_cached > 0) - register_storage_idle_func(dc_flush_playlist_callback); - } - if (!playlist->dcfrefs_handle || playlist->amount <= 0) break ; @@ -2110,13 +2113,15 @@ void playlist_init(void) void playlist_shutdown(void) { struct playlist_info* playlist = ¤t_playlist; + playlist_mutex_lock(&playlist->mutex); if (playlist->control_fd >= 0) { - flush_cached_control(playlist); - + flush_cached_control_unlocked(playlist); close_playlist_control_file(playlist); } + + playlist_mutex_unlock(&playlist->mutex); } /* -- cgit v1.2.3