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

10 files changed, 51 insertions, 77 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 @@
 </phrase>
 <phrase>
   id: LANG_PLAYLIST_RELOAD_AFTER_SAVE
-  desc: reload playlist after saving
+  desc: deprecated
   user: core
   <source>
-    *: "Reload After Saving"
+    *: ""
   </source>
   <dest>
-    *: "Reload After Saving"
+    *: ""
   </dest>
   <voice>
-    *: "Reload After Saving"
+    *: ""
   </voice>
 </phrase>
 <phrase>
@@ -16655,3 +16655,17 @@
     *: "Database Directory"
   </voice>
 </phrase>
+<phrase>
+  id: LANG_REMOVE_QUEUED_TRACKS
+  desc: Confirmation dialog
+  user: core
+  <source>
+    *: "Remove Queued Tracks?"
+  </source>
+  <dest>
+    *: "Remove Queued Tracks?"
+  </dest>
+  <voice>
+    *: "Remove Queued Tracks?"
+  </voice>
+</phrase>
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 = &current_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",