diff options
author | Christian Soffke <christian.soffke@gmail.com> | 2024-11-16 17:05:56 +0100 |
---|---|---|
committer | Christian Soffke <christian.soffke@gmail.com> | 2024-12-31 10:08:31 -0500 |
commit | fe78b07db6bd2806dbbdeca05f263c9e0cf0a57e (patch) | |
tree | 1cad797e98bfa9d13375505cb83b3148829630fd | |
parent | 3389a08d569544c133a02159a4b3d0d543d60be4 (diff) | |
download | rockbox-fe78b07db6.tar.gz rockbox-fe78b07db6.zip |
Playlist Viewer: Address problematic index buffer sharing
Borrowing the index buffer of the current playlist
can be problematic, since it is possible to start
playback of a different playlist from the playlist
viewer without leaving the viewer, thereby causing
a collision.
As long as we have a sufficiently large plugin
buffer, take advantage of it, regardless of
playback state.
When playback is stopped, always resume the playlist
from the control file before loading it, even if the
playlist has finished playing, to prevent running
into invalid indices.
Note:
dcfrefs_handle is initialized to 0 automatically for
the static on_disk_playlist struct.
Change-Id: I2a05a6a51a088ea9ba73858c353525db9e61c36e
-rw-r--r-- | apps/playlist.c | 9 | ||||
-rw-r--r-- | apps/playlist_viewer.c | 37 |
2 files changed, 21 insertions, 25 deletions
diff --git a/apps/playlist.c b/apps/playlist.c index 00343e839e..8de6b6c430 100644 --- a/apps/playlist.c +++ b/apps/playlist.c @@ -2024,7 +2024,7 @@ size_t playlist_get_index_bufsz(size_t max_sz) * provided, the current playlist's index buffer is shared. * FIXME: When using the shared buffer, you must ensure that playback is * stopped and that no other playlist will be started while this - * one is loaded. + * one is loaded. The current playlist's indices will be trashed! * * The temp_buffer (if not NULL) is used as a scratchpad when loading indices. */ @@ -2052,18 +2052,11 @@ struct playlist_info* playlist_load(const char* dir, const char* file, playlist->max_playlist_size = num_indices; playlist->indices = index_buffer; -#ifdef HAVE_DIRCACHE - playlist->dcfrefs_handle = 0; -#endif } else { - /* FIXME not sure if it's safe to share index buffers */ playlist->max_playlist_size = current_playlist.max_playlist_size; playlist->indices = current_playlist.indices; -#ifdef HAVE_DIRCACHE - playlist->dcfrefs_handle = 0; -#endif } new_playlist_unlocked(playlist, dir, file); diff --git a/apps/playlist_viewer.c b/apps/playlist_viewer.c index 75efc27ce5..07a923ce3c 100644 --- a/apps/playlist_viewer.c +++ b/apps/playlist_viewer.c @@ -375,23 +375,26 @@ static bool playlist_viewer_init(struct playlist_viewer * viewer, char *buffer, *index_buffer = NULL; size_t buffer_size, index_buffer_size = 0; bool is_playing = audio_status() & (AUDIO_STATUS_PLAY | AUDIO_STATUS_PAUSE); - bool have_list = filename || is_playing; - if (!have_list && (global_status.resume_index != -1)) + + /* FIXME: On devices with a plugin buffer smaller than 512 KiB, the index buffer + is shared with the current playlist when playback is stopped, to enable + displaying larger playlists. This is generally unsafe, since it is possible + to start playback of a new playlist while another list is still open in the + viewer. Devices affected by this, as of the time of writing, appear to be: + - 80 KiB plugin buffer: Sansa c200v2 + - 64 KiB plugin buffer: Sansa m200v4, Sansa Clip + */ + bool require_index_buffer = filename && (is_playing || PLUGIN_BUFFER_SIZE >= 0x80000); + + if (!filename && !is_playing) { /* Try to restore the list from control file */ - have_list = (playlist_resume() != -1); - } - if (!have_list && (playlist_amount() > 0)) - { - /*If dynamic playlist still exists, view it anyway even - if playback has reached the end of the playlist */ - have_list = true; - } - if (!have_list) - { - /* Nothing to view, exit */ - splash(HZ, ID2P(LANG_CATALOG_NO_PLAYLISTS)); - return false; + if (playlist_resume() == -1) + { + /* Nothing to view, exit */ + splash(HZ, ID2P(LANG_CATALOG_NO_PLAYLISTS)); + return false; + } } size_t id3_size = ALIGN_UP(sizeof(*viewer->id3), 4); @@ -402,7 +405,7 @@ static bool playlist_viewer_init(struct playlist_viewer * viewer, /* Index buffer is required, unless playback is stopped or we're showing the current playlist (i.e. filename == NULL) */ - if (filename && is_playing) + if (require_index_buffer) index_buffer_size = playlist_get_index_bufsz(buffer_size - id3_size - (MAX_PATH + 1)); /* Check for unused space in the plugin buffer to run @@ -452,7 +455,7 @@ static bool playlist_viewer_init(struct playlist_viewer * viewer, } viewer->title = file; - if (is_playing) + if (require_index_buffer) { index_buffer = buffer; buffer += index_buffer_size; |