From 212e7808d5f39e1d7fc1bd487de8f13c50d136f2 Mon Sep 17 00:00:00 2001 From: Richard Quirk Date: Sun, 9 Dec 2012 21:04:17 +0100 Subject: [PATCH] Use crc32 of filename to resume tracks As well as using an index, which breaks when a file is added or removed, use the crc32 of the filename. When the crc32 check passes the index is used directly. When it fails, the slow path is taken checking each file name in the playlist until the right crc is found. If that fails the playlist is started from the beginning. See http://www.rockbox.org/tracker/6411 Bump plugin API and nvram version numbers Change-Id: I156f61a9f1ac428b4a682bc680379cb6b60b1b10 Reviewed-on: http://gerrit.rockbox.org/372 Tested-by: Jonathan Gordon Reviewed-by: Jonathan Gordon --- apps/filetree.c | 2 ++ apps/playlist.c | 41 +++++++++++++++++++++++++++++ apps/playlist.h | 3 +++ apps/plugin.c | 14 +++++----- apps/plugin.h | 19 +++++++------ apps/plugins/alarmclock.c | 3 ++- apps/plugins/lib/playback_control.c | 3 ++- apps/plugins/lrcplayer.c | 3 ++- apps/root_menu.c | 6 +++-- apps/settings.h | 1 + apps/settings_list.c | 1 + apps/settings_list.h | 2 +- docs/KNOWN_ISSUES | 3 --- 13 files changed, 74 insertions(+), 27 deletions(-) diff --git a/apps/filetree.c b/apps/filetree.c index 4e7c2745b4..2edcaf3a03 100644 --- a/apps/filetree.c +++ b/apps/filetree.c @@ -703,6 +703,8 @@ int ft_enter(struct tree_context* c) /* the resume_index must always be the index in the shuffled list in case shuffle is enabled */ global_status.resume_index = start_index; + global_status.resume_crc32 = + playlist_get_filename_crc32(NULL, start_index); global_status.resume_offset = 0; status_save(); rc = GO_TO_WPS; diff --git a/apps/playlist.c b/apps/playlist.c index c5e2e83462..35b7e35baa 100755 --- a/apps/playlist.c +++ b/apps/playlist.c @@ -2514,6 +2514,44 @@ int playlist_shuffle(int random_seed, int start_index) return playlist->index; } +/* returns the crc32 of the filename of the track at the specified index */ +unsigned int playlist_get_filename_crc32(struct playlist_info *playlist, + int index) +{ + struct playlist_track_info track_info; + if (playlist_get_track_info(playlist, index, &track_info) == -1) + return -1; + + return crc_32(track_info.filename, strlen(track_info.filename), -1); +} + +/* resume a playlist track with the given crc_32 of the track name. */ +void playlist_resume_track(int start_index, unsigned int crc, int offset) +{ + int i; + unsigned int tmp_crc; + struct playlist_info* playlist = ¤t_playlist; + tmp_crc = playlist_get_filename_crc32(playlist, start_index); + if (tmp_crc == crc) + { + playlist_start(start_index, offset); + return; + } + + for (i = 0 ; i < playlist->amount; i++) + { + tmp_crc = playlist_get_filename_crc32(playlist, i); + if (tmp_crc == crc) + { + playlist_start(i, offset); + return; + } + } + + /* If we got here the file wasnt found, so start from the beginning */ + playlist_start(0,0); +} + /* start playing current playlist at specified index/offset */ void playlist_start(int start_index, int offset) { @@ -2726,7 +2764,9 @@ int playlist_update_resume_info(const struct mp3entry* id3) if (global_status.resume_index != playlist->index || global_status.resume_offset != id3->offset) { + unsigned int crc = crc_32(id3->path, strlen(id3->path), -1); global_status.resume_index = playlist->index; + global_status.resume_crc32 = crc; global_status.resume_offset = id3->offset; status_save(); } @@ -2734,6 +2774,7 @@ int playlist_update_resume_info(const struct mp3entry* id3) else { global_status.resume_index = -1; + global_status.resume_crc32 = -1; global_status.resume_offset = -1; status_save(); } diff --git a/apps/playlist.h b/apps/playlist.h index d19d9a792a..d80d8aa2ee 100644 --- a/apps/playlist.h +++ b/apps/playlist.h @@ -131,6 +131,9 @@ int playlist_create(const char *dir, const char *file); int playlist_resume(void); int playlist_add(const char *filename); int playlist_shuffle(int random_seed, int start_index); +unsigned int playlist_get_filename_crc32(struct playlist_info *playlist, + int index); +void playlist_resume_track(int start_index, unsigned int crc, int offset); void playlist_start(int start_index, int offset); bool playlist_check(int steps); const char *playlist_peek(int steps, char* buf, size_t buf_size); diff --git a/apps/plugin.c b/apps/plugin.c index 9992d85620..24443b58d9 100644 --- a/apps/plugin.c +++ b/apps/plugin.c @@ -155,6 +155,9 @@ static const struct plugin_api rockbox_api = { lcd_double_height, #else &lcd_static_framebuffer[0][0], + lcd_set_viewport, + lcd_set_framebuffer, + lcd_bmp_part, lcd_update_rect, lcd_set_drawmode, lcd_get_drawmode, @@ -287,6 +290,7 @@ static const struct plugin_api rockbox_api = { #ifdef HAVE_LCD_BITMAP viewportmanager_theme_enable, viewportmanager_theme_undo, + viewport_set_fullscreen, #endif /* list */ @@ -583,6 +587,7 @@ static const struct plugin_api rockbox_api = { mixer_channel_stop, mixer_channel_set_amplitude, mixer_channel_get_bytes_waiting, + mixer_channel_set_buffer_hook, system_sound_play, keyclick_click, @@ -590,6 +595,7 @@ static const struct plugin_api rockbox_api = { /* playback control */ playlist_amount, playlist_resume, + playlist_resume_track, playlist_start, playlist_add, playlist_sync, @@ -792,14 +798,6 @@ static const struct plugin_api rockbox_api = { /* new stuff at the end, sort into place next time the API gets incompatible */ -#ifdef HAVE_LCD_BITMAP -#if CONFIG_CODEC == SWCODEC - mixer_channel_set_buffer_hook, -#endif - lcd_set_viewport, - viewport_set_fullscreen, - lcd_set_framebuffer, -#endif }; int plugin_load(const char* plugin, const void* parameter) diff --git a/apps/plugin.h b/apps/plugin.h index b34a5cf0de..cc00b9c60e 100644 --- a/apps/plugin.h +++ b/apps/plugin.h @@ -207,6 +207,10 @@ struct plugin_api { void (*lcd_double_height)(bool on); #else /* HAVE_LCD_BITMAP */ fb_data* lcd_framebuffer; + void (*lcd_set_viewport)(struct viewport* vp); + void (*lcd_set_framebuffer)(fb_data *fb); + void (*lcd_bmp_part)(const struct bitmap *bm, int src_x, int src_y, + int x, int y, int width, int height); void (*lcd_update_rect)(int x, int y, int width, int height); void (*lcd_set_drawmode)(int mode); int (*lcd_get_drawmode)(void); @@ -372,6 +376,8 @@ struct plugin_api { void (*viewportmanager_theme_enable)(enum screen_type screen, bool enable, struct viewport *viewport); void (*viewportmanager_theme_undo)(enum screen_type screen, bool force_redraw); + void (*viewport_set_fullscreen)(struct viewport *vp, + const enum screen_type screen); #endif /* list */ void (*gui_synclist_init)(struct gui_synclist * lists, @@ -707,6 +713,8 @@ struct plugin_api { void (*mixer_channel_set_amplitude)(enum pcm_mixer_channel channel, unsigned int amplitude); size_t (*mixer_channel_get_bytes_waiting)(enum pcm_mixer_channel channel); + void (*mixer_channel_set_buffer_hook)(enum pcm_mixer_channel channel, + chan_buffer_hook_fn_type fn); void (*system_sound_play)(enum system_sound sound); void (*keyclick_click)(bool rawbutton, int action); @@ -715,6 +723,7 @@ struct plugin_api { /* playback control */ int (*playlist_amount)(void); int (*playlist_resume)(void); + void (*playlist_resume_track)(int start_index, unsigned int crc, int offset); void (*playlist_start)(int start_index, int offset); int (*playlist_add)(const char *filename); void (*playlist_sync)(struct playlist_info* playlist); @@ -961,16 +970,6 @@ struct plugin_api { /* new stuff at the end, sort into place next time the API gets incompatible */ -#ifdef HAVE_LCD_BITMAP -#if CONFIG_CODEC == SWCODEC - void (*mixer_channel_set_buffer_hook)(enum pcm_mixer_channel channel, - chan_buffer_hook_fn_type fn); -#endif - void (*lcd_set_viewport)(struct viewport* vp); - void (*viewport_set_fullscreen)(struct viewport *vp, - const enum screen_type screen); - void (*lcd_set_framebuffer)(fb_data *fb); -#endif }; /* plugin header */ diff --git a/apps/plugins/alarmclock.c b/apps/plugins/alarmclock.c index 604c9ceac4..126b837390 100644 --- a/apps/plugins/alarmclock.c +++ b/apps/plugins/alarmclock.c @@ -107,7 +107,8 @@ static void play(void) int audio_status = rb->audio_status(); if (!audio_status && rb->global_status->resume_index != -1) { if (rb->playlist_resume() != -1) { - rb->playlist_start(rb->global_status->resume_index, + rb->playlist_resume_track(rb->global_status->resume_index, + rb->global_status->resume_crc32, rb->global_status->resume_offset); } } diff --git a/apps/plugins/lib/playback_control.c b/apps/plugins/lib/playback_control.c index 7c28230642..47921e52f2 100644 --- a/apps/plugins/lib/playback_control.c +++ b/apps/plugins/lib/playback_control.c @@ -37,7 +37,8 @@ static bool play(void) { if (rb->playlist_resume() != -1) { - rb->playlist_start(rb->global_status->resume_index, + rb->playlist_resume_track(rb->global_status->resume_index, + rb->global_status->resume_crc32, rb->global_status->resume_offset); } } diff --git a/apps/plugins/lrcplayer.c b/apps/plugins/lrcplayer.c index 97385ff047..051d7adbb2 100644 --- a/apps/plugins/lrcplayer.c +++ b/apps/plugins/lrcplayer.c @@ -2682,7 +2682,8 @@ static int handle_button(void) { if (rb->playlist_resume() != -1) { - rb->playlist_start(rb->global_status->resume_index, + rb->playlist_resume_track(rb->global_status->resume_index, + rb->global_status->resume_crc32, rb->global_status->resume_offset); } } diff --git a/apps/root_menu.c b/apps/root_menu.c index 2c72cc47f7..d03fee35f7 100644 --- a/apps/root_menu.c +++ b/apps/root_menu.c @@ -297,12 +297,14 @@ static int wpsscrn(void* param) } else if ( global_status.resume_index != -1 ) { - DEBUGF("Resume index %X offset %lX\n", + DEBUGF("Resume index %X crc32 %lX offset %lX\n", global_status.resume_index, + (unsigned long)global_status.resume_crc32, (unsigned long)global_status.resume_offset); if (playlist_resume() != -1) { - playlist_start(global_status.resume_index, + playlist_resume_track(global_status.resume_index, + global_status.resume_crc32, global_status.resume_offset); ret_val = gui_wps_show(); } diff --git a/apps/settings.h b/apps/settings.h index 2af29ce423..53ad70b24f 100644 --- a/apps/settings.h +++ b/apps/settings.h @@ -262,6 +262,7 @@ bool set_option(const char* string, const void* variable, enum optiontype type, struct system_status { int resume_index; /* index in playlist (-1 for no active resume) */ + uint32_t resume_crc32; /* crc32 of the name of the file */ uint32_t resume_offset; /* byte offset in mp3 file */ int runtime; /* current runtime since last charge */ int topruntime; /* top known runtime */ diff --git a/apps/settings_list.c b/apps/settings_list.c index f27c13c4f1..ef9fe50ece 100644 --- a/apps/settings_list.c +++ b/apps/settings_list.c @@ -721,6 +721,7 @@ const struct settings_list settings[] = { /* playback */ OFFON_SETTING(0, playlist_shuffle, LANG_SHUFFLE, false, "shuffle", NULL), SYSTEM_SETTING(NVRAM(4), resume_index, -1), + SYSTEM_SETTING(NVRAM(4), resume_crc32, -1), SYSTEM_SETTING(NVRAM(4), resume_offset, -1), CHOICE_SETTING(0, repeat_mode, LANG_REPEAT, REPEAT_OFF, "repeat", "off,all,one,shuffle" diff --git a/apps/settings_list.h b/apps/settings_list.h index 9f1805f169..5cefcf6dc2 100644 --- a/apps/settings_list.h +++ b/apps/settings_list.h @@ -142,7 +142,7 @@ struct custom_setting { #define F_NVRAM_BYTES_MASK 0xE0000 /*0-4 bytes can be stored */ #define F_NVRAM_MASK_SHIFT 17 -#define NVRAM_CONFIG_VERSION 6 +#define NVRAM_CONFIG_VERSION 7 /* Above define should be bumped if - a new NVRAM setting is added between 2 other NVRAM settings - number of bytes for a NVRAM setting is changed diff --git a/docs/KNOWN_ISSUES b/docs/KNOWN_ISSUES index 0c44acc685..4ae88cb9fd 100644 --- a/docs/KNOWN_ISSUES +++ b/docs/KNOWN_ISSUES @@ -40,6 +40,3 @@ FS#5796 - Early encoders such as this one employed a floor of type '0', as Files like these require quite a bit of memory to decode, more than what Rockbox has set aside for the purpose. Adding a real malloc for the codecs might help... - -FS#6411 - Resuming a playlist after having deleted one or more files makes - the resume point end up on the wrong song.