[Bugfix] shuffle shenanigans from g5288 Fix #13369 shuffle & repeat callbacks

shuffle and sort were called on startup before playlist_init
and also on setting switch even without select

repeat is also now handled in settings_list as well

after moving the callbacks to settings_list.c
there was then a problem of unintended callbacks on exit of the menus
fixed that with F_CB_ON_SELECT_ONLY

since the callback was called regardless of the setting being changed
on F_CB_ON_SELECT_ONLY which is preferable in some circumstances
I co-opted F_TEMPVAR to allow the callback only when the setting was changed
with the flag F_CB_ON_SELECT_ONLY_IF_CHANGED

Change-Id: I5265233bbb556dc06c45273e742be5d78510a806

apps/gui/option_select.c

@@ -470,7 +470,10 @@ bool option_screen(const struct settings_list *setting,
     int *variable;
     bool allow_wrap = setting->flags & F_NO_WRAP ? false : true;
     bool cb_on_select_only =
-                ((setting->flags & F_CB_ON_SELECT_ONLY) == F_CB_ON_SELECT_ONLY);
+        ((setting->flags & F_CB_ON_SELECT_ONLY) == F_CB_ON_SELECT_ONLY);
+    bool cb_on_changed =
+        ((setting->flags & F_CB_ON_SELECT_ONLY_IF_CHANGED) == F_CB_ON_SELECT_ONLY_IF_CHANGED);
+
     int var_type = setting->flags&F_T_MASK;
     void (*function)(int) = NULL;
     char *title;
@@ -561,8 +564,12 @@ bool option_screen(const struct settings_list *setting,
             }
             settings_save();
             done = true;
+
             if (cb_on_select_only && function)
-                function(*variable);
+            {
+                if (!cb_on_changed || (*variable != oldvalue))
+                    function(*variable);
+            }
         }
         else if(default_event_handler(action) == SYS_USB_CONNECTED)
         {

apps/gui/quickscreen.c

@@ -418,8 +418,6 @@ static int gui_syncquickscreen_run(struct gui_quickscreen * qs, int button_enter
 int quick_screen_quick(int button_enter)
 {
     struct gui_quickscreen qs;
-    bool oldshuffle = global_settings.playlist_shuffle;
-    int oldrepeat = global_settings.repeat_mode;
 #ifdef HAVE_ALBUMART
     int old_album_art = global_settings.album_art;
 #endif
@@ -438,21 +436,6 @@ int quick_screen_quick(int button_enter)
     if (ret & QUICKSCREEN_CHANGED)
     {
         settings_save();
-        /* make sure repeat/shuffle/any other nasty ones get updated */
-        if ( oldrepeat != global_settings.repeat_mode &&
-             (audio_status() & AUDIO_STATUS_PLAY) )
-        {
-            audio_flush_and_reload_tracks();
-        }
-        if (oldshuffle != global_settings.playlist_shuffle
-            && audio_status() & AUDIO_STATUS_PLAY)
-        {
-            replaygain_update();
-            if (global_settings.playlist_shuffle)
-                playlist_randomise(NULL, current_tick, true);
-            else
-                playlist_sort(NULL, true);
-        }
 #ifdef HAVE_ALBUMART
         if (old_album_art != global_settings.album_art)
             set_albumart_mode(global_settings.album_art);

apps/menus/playback_menu.c

@@ -244,19 +244,9 @@ static int playback_callback(int action,
                              struct gui_synclist *this_list)
 {
     (void)this_list;
-    static bool old_shuffle = false;
-    static int old_repeat = 0;
     switch (action)
     {
         case ACTION_ENTER_MENUITEM:
-            if (this_item == &shuffle_item)
-            {
-                old_shuffle = global_settings.playlist_shuffle;
-            }
-            else if (this_item == &repeat_mode)
-            {
-                old_repeat = global_settings.repeat_mode;
-            }
             break;
 
         case ACTION_EXIT_MENUITEM: /* on exit */
@@ -268,35 +258,6 @@ static int playback_callback(int action,
                 break;
             }
 #endif /* HAVE_PLAY_FREQ */
-
-            if (!(audio_status() & AUDIO_STATUS_PLAY))
-                break;
-
-            /* Playing only */
-            if (this_item == &shuffle_item)
-            {
-                if (old_shuffle == global_settings.playlist_shuffle)
-                    break;
-
-                replaygain_update();
-
-                if (global_settings.playlist_shuffle)
-                {
-                    playlist_randomise(NULL, current_tick, true);
-                }
-                else
-                {
-                    playlist_sort(NULL, true);
-                }
-            }
-            else if (this_item == &repeat_mode)
-            {
-                if (old_repeat == global_settings.repeat_mode)
-                    break;
-
-                audio_flush_and_reload_tracks();
-            }
-
             break;
     }
     return action;

apps/settings_list.c

@@ -622,14 +622,31 @@ static void eq_set_default(void* setting, void* defaultval)
 /* perform shuffle/unshuffle of the current playlist based on the boolean provided */
 static void shuffle_playlist_callback(bool shuffle)
 {
-    if (shuffle)
+    struct playlist_info *playlist = playlist_get_current();
+    if (playlist->started)
     {
-        playlist_randomise(NULL, current_tick, true);
+        if ((audio_status() & AUDIO_STATUS_PLAY) == AUDIO_STATUS_PLAY)
+        {
+            replaygain_update();
+            if (shuffle)
+            {
+                playlist_randomise(playlist, current_tick, true);
+            }
+            else
+            {
+                playlist_sort(playlist, true);
+            }
+        }
     }
-    else
+}
+
+static void repeat_mode_callback(int repeat)
+{
+    if ((audio_status() & AUDIO_STATUS_PLAY) == AUDIO_STATUS_PLAY)
     {
-        playlist_sort(NULL, true);
+        audio_flush_and_reload_tracks();
     }
+    (void)repeat;
 }
 
 #ifdef HAVE_QUICKSCREEN
@@ -912,17 +929,19 @@ const struct settings_list settings[] = {
 #endif
 
     /* playback */
-    OFFON_SETTING(0, playlist_shuffle, LANG_SHUFFLE, false, "shuffle", shuffle_playlist_callback),
+    OFFON_SETTING(F_CB_ON_SELECT_ONLY_IF_CHANGED, playlist_shuffle, LANG_SHUFFLE,
+                  false, "shuffle", shuffle_playlist_callback),
+
     SYSTEM_SETTING(NVRAM(4), resume_index, -1),
     SYSTEM_SETTING(NVRAM(4), resume_crc32, -1),
     SYSTEM_SETTING(NVRAM(4), resume_elapsed, -1),
     SYSTEM_SETTING(NVRAM(4), resume_offset, -1),
-    CHOICE_SETTING(0, repeat_mode, LANG_REPEAT, REPEAT_OFF, "repeat",
+    CHOICE_SETTING(F_CB_ON_SELECT_ONLY_IF_CHANGED, repeat_mode, LANG_REPEAT,
-                   "off,all,one,shuffle"
+                   REPEAT_OFF, "repeat", "off,all,one,shuffle"
 #ifdef AB_REPEAT_ENABLE
                    ",ab"
 #endif
-                   , NULL,
+                   , repeat_mode_callback,
 #ifdef AB_REPEAT_ENABLE
                    5,
 #else

apps/settings_list.h

@@ -102,6 +102,7 @@ struct table_setting {
 #define F_TABLE_SETTING 0x2000
 #define F_ALLOW_ARBITRARY_VALS 0x4000
 #define F_CB_ON_SELECT_ONLY 0x20000
+#define F_CB_ON_SELECT_ONLY_IF_CHANGED (F_CB_ON_SELECT_ONLY|F_TEMPVAR)
 /* these use the _isfunc_type type for the function */
 /* typedef int (*_isfunc_type)(void); */
 #define F_MIN_ISFUNC    0x100000 /* min(above) is function pointer to above type */