From ea5ce8034b4a1ead1227114308889511dbfa9539 Mon Sep 17 00:00:00 2001 From: Christian Soffke Date: Sat, 11 Nov 2023 01:53:36 +0100 Subject: Replace "Reload After Saving" with option to remove queued tracks The "Reload After Saving" setting was added in g3347 (4f83e66) to solve FS#13287, by allowing you to bookmark a modified playlist after saving, without having to manually reload it first. Since the rewrite of playlist_save in g5192 (90e3571), a modified playlist doesn't have to be reloaded anymore in order to be bookmarked after it's been saved, unless it contains queued tracks. To cover the remaining use cases of the previously available option, Rockbox will now offer to remove any queued tracks from a playlist when saving it. Change-Id: I2d6f12bcce14d8ff41a4d921ce84d628774103ac --- apps/filetree.c | 14 +++---- apps/filetree.h | 3 +- apps/iap/iap-lingo4.c | 3 +- apps/lang/english.lang | 22 ++++++++-- apps/menus/playlist_menu.c | 59 +-------------------------- apps/misc.c | 7 ++++ apps/misc.h | 1 + apps/playlist.c | 16 ++++++++ apps/settings.h | 1 - apps/settings_list.c | 2 - manual/configure_rockbox/bookmarking.tex | 5 ++- manual/configure_rockbox/playlist_options.tex | 7 ---- manual/main_menu/main.tex | 6 ++- manual/rockbox_interface/wps.tex | 9 +++- manual/working_with_playlists/main.tex | 4 +- 15 files changed, 69 insertions(+), 90 deletions(-) diff --git a/apps/filetree.c b/apps/filetree.c index dff8f49be9..b5f5dece5a 100644 --- a/apps/filetree.c +++ b/apps/filetree.c @@ -129,8 +129,7 @@ int ft_build_playlist(struct tree_context* c, int start_index) * avoid allocating yet another path buffer on the stack (and save some * code; the caller typically needs to create the full pathname anyway)... */ -bool ft_play_playlist(char* pathname, char* dirname, - char* filename, bool skip_warn_and_bookmarks) +bool ft_play_playlist(char* pathname, char* dirname, char* filename) { if (global_settings.party_mode && audio_status()) { @@ -138,12 +137,9 @@ bool ft_play_playlist(char* pathname, char* dirname, return false; } - if (!skip_warn_and_bookmarks) - { - int res = bookmark_autoload(pathname); - if (res == BOOKMARK_CANCEL || res == BOOKMARK_DO_RESUME || !warn_on_pl_erase()) - return false; - } + int res = bookmark_autoload(pathname); + if (res == BOOKMARK_CANCEL || res == BOOKMARK_DO_RESUME || !warn_on_pl_erase()) + return false; splash(0, ID2P(LANG_WAIT)); @@ -547,7 +543,7 @@ int ft_enter(struct tree_context* c) switch ( file_attr & FILE_ATTR_MASK ) { case FILE_ATTR_M3U: - play = ft_play_playlist(buf, c->currdir, file->name, false); + play = ft_play_playlist(buf, c->currdir, file->name); if (play) { diff --git a/apps/filetree.h b/apps/filetree.h index 3ec7846d2c..ebd9885ec2 100644 --- a/apps/filetree.h +++ b/apps/filetree.h @@ -28,7 +28,6 @@ int ft_exit(struct tree_context* c); int ft_assemble_path(char *buf, size_t bufsz, const char* currdir, const char* filename); int ft_build_playlist(struct tree_context* c, int start_index); -bool ft_play_playlist(char* pathname, char* dirname, - char* filename, bool skip_warn_and_bookmarks); +bool ft_play_playlist(char* pathname, char* dirname, char* filename); #endif diff --git a/apps/iap/iap-lingo4.c b/apps/iap/iap-lingo4.c index eb629407f2..e1ab150fd9 100644 --- a/apps/iap/iap-lingo4.c +++ b/apps/iap/iap-lingo4.c @@ -109,8 +109,7 @@ static void seek_to_playlist(unsigned long index) MAX_PATH); ft_play_playlist(selected_playlist, global_settings.playlist_catalog_dir, - strrchr(selected_playlist, '/') + 1, - false); + strrchr(selected_playlist, '/') + 1); } static unsigned long nbr_total_playlists(void) diff --git a/apps/lang/english.lang b/apps/lang/english.lang index 1287a26c39..c41033ec81 100644 --- a/apps/lang/english.lang +++ b/apps/lang/english.lang @@ -15936,16 +15936,16 @@ id: LANG_PLAYLIST_RELOAD_AFTER_SAVE - desc: reload playlist after saving + desc: deprecated user: core - *: "Reload After Saving" + *: "" - *: "Reload After Saving" + *: "" - *: "Reload After Saving" + *: "" @@ -16655,3 +16655,17 @@ *: "Database Directory" + + id: LANG_REMOVE_QUEUED_TRACKS + desc: Confirmation dialog + user: core + + *: "Remove Queued Tracks?" + + + *: "Remove Queued Tracks?" + + + *: "Remove Queued Tracks?" + + diff --git a/apps/menus/playlist_menu.c b/apps/menus/playlist_menu.c index affe20418d..bfb94ccdad 100644 --- a/apps/menus/playlist_menu.c +++ b/apps/menus/playlist_menu.c @@ -38,7 +38,6 @@ #include "talk.h" #include "playlist_catalog.h" #include "splash.h" -#include "filetree.h" #include "general.h" /* load a screen to save the playlist passed in (or current playlist if NULL is passed) */ @@ -46,11 +45,6 @@ int save_playlist_screen(struct playlist_info* playlist) { char directoryonly[MAX_PATH+3]; - char *filename; - - int resume_index; - uint32_t resume_elapsed; - uint32_t resume_offset; char temp[MAX_PATH+1], *p; int len; @@ -86,55 +80,8 @@ int save_playlist_screen(struct playlist_info* playlist) /* reload in case playlist was saved to cwd */ reload_directory(); - - /* only reload newly saved playlist if: - * playlist is null AND setting is turned on - * - * if playlist is null, we should be dealing with the current playlist, - * and thus we got here from the wps screen. That means we want to reload - * the current playlist so the user can make bookmarks. */ - if ((global_settings.playlist_reload_after_save == true) && - (playlist == NULL)) - { - - /* at least one slash exists in temp */ - if (strrchr(temp, '/') != NULL) - { - filename = strrchr(temp, '/') + 1; - - if (temp[0] == '/') /* most common situation - first char is a slash */ - { - strcpy(directoryonly, temp); - directoryonly[filename - temp] = '\0'; - } else /* there is a slash, but not at the beginning of temp - prepend one */ - { - directoryonly[0] = '/'; - - strcpy(directoryonly+1, temp); - directoryonly[filename - temp + 1] = '\0'; - } - } else /* temp doesn't contain any slashes, uncommon? */ - { - directoryonly[0] = '/'; - directoryonly[1] = '\0'; - filename = temp; - } - - /* can't trust index from id3 (don't know why), get it from playlist */ - resume_index = playlist_get_display_index() - 1; - - struct mp3entry* id3 = audio_current_track(); - - /* record elapsed and offset so they don't change when we load new playlist */ - resume_elapsed = id3->elapsed; - resume_offset = id3->offset; - - ft_play_playlist(temp, directoryonly, filename, true); - playlist_start(resume_index, resume_elapsed, resume_offset); - } - /* cancelled out of name selection */ } else { - return 1; + return 1; /* cancelled out of name selection */ } return 0; @@ -179,14 +126,12 @@ MENUITEM_SETTING(warn_on_erase, &global_settings.warnon_erase_dynplaylist, NULL) MENUITEM_SETTING(keep_current_track_on_replace, &global_settings.keep_current_track_on_replace_playlist, NULL); MENUITEM_SETTING(show_shuffled_adding_options, &global_settings.show_shuffled_adding_options, NULL); MENUITEM_SETTING(show_queue_options, &global_settings.show_queue_options, NULL); -MENUITEM_SETTING(playlist_reload_after_save, &global_settings.playlist_reload_after_save, NULL); MAKE_MENU(currentplaylist_settings_menu, ID2P(LANG_CURRENT_PLAYLIST), NULL, Icon_Playlist, &warn_on_erase, &keep_current_track_on_replace, &show_shuffled_adding_options, - &show_queue_options, - &playlist_reload_after_save); + &show_queue_options); MAKE_MENU(playlist_settings, ID2P(LANG_PLAYLISTS), NULL, Icon_Playlist, diff --git a/apps/misc.c b/apps/misc.c index cb5f1cea9c..4faadb8130 100644 --- a/apps/misc.c +++ b/apps/misc.c @@ -1235,6 +1235,13 @@ int confirm_overwrite_yesno(void) return gui_syncyesno_run(&message, NULL, NULL); } +int confirm_remove_queued_yesno(void) +{ + static const char *lines[] = { ID2P(LANG_REMOVE_QUEUED_TRACKS) }; + static const struct text_message message = { lines, 1 }; + return gui_syncyesno_run(&message, NULL, NULL); +} + /* time_split_units() split time values depending on base unit unit_idx: UNIT_HOUR, UNIT_MIN, UNIT_SEC, UNIT_MS diff --git a/apps/misc.h b/apps/misc.h index 497aa04c78..fd48ccf648 100644 --- a/apps/misc.h +++ b/apps/misc.h @@ -160,6 +160,7 @@ int hex_to_rgb(const char* hex, int* color); int confirm_delete_yesno(const char *name); int confirm_overwrite_yesno(void); +int confirm_remove_queued_yesno(void); char* strrsplt(char* str, int c); char* skip_whitespace(char* const str); diff --git a/apps/playlist.c b/apps/playlist.c index 67d59d1aac..024f98d2f3 100644 --- a/apps/playlist.c +++ b/apps/playlist.c @@ -99,6 +99,7 @@ #include "filetypes.h" #include "icons.h" #include "system.h" +#include "misc.h" #include "lang.h" #include "talk.h" @@ -3977,6 +3978,7 @@ int playlist_save(struct playlist_info* playlist, char *filename) char tmpbuf[MAX_PATH+1]; ssize_t pathlen; int rc = 0; + bool reload_tracks = false; if (!playlist) playlist = ¤t_playlist; @@ -3996,6 +3998,17 @@ int playlist_save(struct playlist_info* playlist, char *filename) goto error; } + /* Ask if queued tracks should be removed, so that + playlist can be bookmarked after it's been saved */ + for (int i = playlist->amount - 1; i >= 0; i--) + if (playlist->indices[i] & PLAYLIST_QUEUED) + { + if (reload_tracks || (reload_tracks = (confirm_remove_queued_yesno() == YESNO_YES))) + remove_track_unlocked(playlist, i, false); + else + break; + } + rc = pl_save_playlist(playlist, save_path, tmpbuf, sizeof(tmpbuf)); if (rc < 0) { @@ -4030,5 +4043,8 @@ error: playlist_write_unlock(playlist); dc_thread_start(playlist, true); cpu_boost(false); + if (reload_tracks && playlist->started && + (audio_status() & AUDIO_STATUS_PLAY)) + audio_flush_and_reload_tracks(); return rc; } diff --git a/apps/settings.h b/apps/settings.h index 13550ffd2a..09a01b40a1 100644 --- a/apps/settings.h +++ b/apps/settings.h @@ -607,7 +607,6 @@ struct user_settings bool constrain_next_folder; /* whether next_folder is constrained to directories within start_directory */ int recursive_dir_insert; /* should directories be inserted recursively */ - bool playlist_reload_after_save; /* reload and resume playlist after saving */ bool fade_on_stop; /* fade on pause/unpause/stop */ bool playlist_shuffle; bool warnon_erase_dynplaylist; /* warn when erasing dynamic playlist */ diff --git a/apps/settings_list.c b/apps/settings_list.c index 74db7550bf..bc3708bfba 100644 --- a/apps/settings_list.c +++ b/apps/settings_list.c @@ -1412,8 +1412,6 @@ const struct settings_list settings[] = { CHOICE_SETTING(0, recursive_dir_insert, LANG_RECURSE_DIRECTORY , RECURSE_ON, "recursive directory insert", off_on_ask, NULL , 3 , ID2P(LANG_OFF), ID2P(LANG_ON), ID2P(LANG_ASK)), - OFFON_SETTING(0, playlist_reload_after_save, LANG_PLAYLIST_RELOAD_AFTER_SAVE, - false, "reload after saving playlist", NULL), /* bookmarks */ CHOICE_SETTING(0, autocreatebookmark, LANG_BOOKMARK_SETTINGS_AUTOCREATE, BOOKMARK_NO, "autocreate bookmarks", diff --git a/manual/configure_rockbox/bookmarking.tex b/manual/configure_rockbox/bookmarking.tex index c1d0f3b8bf..4168545aac 100644 --- a/manual/configure_rockbox/bookmarking.tex +++ b/manual/configure_rockbox/bookmarking.tex @@ -18,7 +18,10 @@ and does not work for tracks launched via the database. In addition, they do not work with dynamic (i.e. modified but not saved) playlists. If making a bookmark is not available, saving the playlist - is sufficient to allow a bookmark to be made.} + is sufficient to allow a bookmark to be made. Queued tracks, which are automatically + removed after playback, do not get stored to the playlist file and must + not be part of the current playlist. You need to confirm the removal of such tracks + when saving, or bookmarking will not be possible.} \begin{description} diff --git a/manual/configure_rockbox/playlist_options.tex b/manual/configure_rockbox/playlist_options.tex index d881ba5b40..a966d4be7e 100644 --- a/manual/configure_rockbox/playlist_options.tex +++ b/manual/configure_rockbox/playlist_options.tex @@ -47,13 +47,6 @@ related to playlists. If set to \setting{In Submenu}, Rockbox will move the options into a separate submenu. - \item[Reload After Saving.] - If set to \setting{Yes}, saving the current playlist from the While Playing Screen's - Context Menu will cause Rockbox to load the newly saved playlist and resume it to the - current position. This is useful for creating bookmarks after modifying or customizing - a playlist. Saving playlists outside of the While Playing Screen's Context Menu will - not be affected by this setting. - \end{description} \end{description} diff --git a/manual/main_menu/main.tex b/manual/main_menu/main.tex index 34868ca759..b017fc4431 100644 --- a/manual/main_menu/main.tex +++ b/manual/main_menu/main.tex @@ -178,8 +178,10 @@ will be saved in the Playlist Catalogue directory. \item[Save Current Playlist:] Saves the current dynamic playlist, excluding queued tracks, to the -specified file. If no path is provided then playlist is saved to the current -directory. +specified file. Asks for confirmation whether to remove queued tracks +from the current playlist, so that bookmarks can be created +(see \reference{ref:createbookmark}). +If no path is provided then playlist is saved to the current directory. \item[Reset Playlist Catalogue Directory:] Will reset the default location for playlists to the \fname{/Playlists} diff --git a/manual/rockbox_interface/wps.tex b/manual/rockbox_interface/wps.tex index 45b3860b0e..5f810261fa 100644 --- a/manual/rockbox_interface/wps.tex +++ b/manual/rockbox_interface/wps.tex @@ -223,8 +223,13 @@ enabled. It allows the assignment of a personal rating value (0 -- 10) to a track which can be displayed in the WPS and used in the Database browser. The value wraps at 10. -\subsubsection{Bookmarks} -This allows you to create a bookmark in the currently-playing track. +\subsubsection{\label{ref:createbookmark}Bookmarks} +This allows you to create a bookmark in the currently-playing track. Note that bookmarks +can only be created for directories, or for playlists stored on disk. Unsaved or modified +lists must be saved first. Queued tracks, which are automatically removed after playback, +do not get stored to the playlist file. When saving, Rockbox will offer to purge the +current playlist of such tracks, so that you'll be able to create bookmarks afterwards. +See \reference{ref:Bookmarkconfigactual} for more information on bookmarks. \subsubsection{\label{ref:trackinfoviewer}Show Track Info} \screenshot{rockbox_interface/images/ss-id3-viewer}{The track info viewer}{} diff --git a/manual/working_with_playlists/main.tex b/manual/working_with_playlists/main.tex index 45c57e34b4..dd25d41c26 100644 --- a/manual/working_with_playlists/main.tex +++ b/manual/working_with_playlists/main.tex @@ -173,7 +173,9 @@ select \setting{Save Current Playlist} or enter the context menu for the \setting{Save Current Playlist}. Either method will bring you to the \setting{Virtual Keyboard} (see \reference{sec:virtual_keyboard}), enter a filename for your playlist and -accept it and you are done. +accept it. If the current playlist contains any queued tracks, you will be +asked whether to remove them, as a prerequisiste for creating bookmarks +(see \reference{ref:createbookmark}). \subsection{Loading saved playlists} \subsubsection{Through the \setting{File Browser}} -- cgit v1.2.3