devs/foxbox
1
0
Fork 0
forked from len0rd/rockbox
Code Pull requests Activity

Playlist Viewer: Address problematic index buffer sharing

Browse source 
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
This commit is contained in:
Christian Soffke 2024-11-16 17:05:56 +01:00
parent 3389a08d56
commit fe78b07db6
2 changed files with 21 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

9
apps/playlist.c
View file

@ -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);

29
apps/playlist_viewer.c
View file

@ -375,24 +375,27 @@ 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)
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;