[prev in list] [next in list] [prev in thread] [next in thread]
List: rockbox-cvs
Subject: playlist: Remove control file cache
From: rockbox-gerrit-noreply--- via rockbox-cvs <rockbox-cvs () lists ! haxx ! se>
Date: 2023-03-24 19:39:26
Message-ID: 202303241939.32OJdQNM1253524 () archos ! rockbox ! org
[Download RAW message or body]
commit 2e99e2175b48cc00274b03bb4aecf5d01403110d
Author: Aidan MacDonald <amachronic@protonmail.com>
Date: Fri Mar 24 17:36:28 2023 +0000
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
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;
--
rockbox-cvs mailing list
rockbox-cvs@lists.haxx.se
https://lists.haxx.se/mailman/listinfo/rockbox-cvs
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic