[Feature] Persist pitch settings through reboots part deux

Revisit this after discussion with chris_s on IRC and forum

Pitch menu now changes icon when pitch has been changed

uses NVRAM to save the pitch settings unconditionally

Manual updated

Change-Id: Idcb4c2b7fe42f7a203dc4bfc46285657f370d0fd

apps/audio_thread.c

@@ -38,11 +38,6 @@
 #include "talk.h"
 #include "settings.h"
 
-#if defined(HAVE_PITCHCONTROL)
-#include "pitchscreen.h"
-#include "misc.h"
-#include "strcasecmp.h"
-#endif
 /* Macros to enable logf for queues
    logging on SYS_TIMEOUT can be disabled */
 #ifdef SIMULATOR
@@ -190,39 +185,4 @@ void INIT_ATTR audio_init(void)
     audio_is_initialized = true;
 
     sound_settings_apply();
-
-#if defined(HAVE_PITCHCONTROL)
-    int fd;
-    char line[64];
-    char* name;
-    char* value;
-    int32_t num;
-    if (file_exists(PITCH_CFG_FILE))
-    {
-        fd = open_utf8(PITCH_CFG_FILE, O_RDONLY);
-        if (fd >= 0)
-        {
-            while (read_line(fd, line, sizeof line) > 0)
-            {
-                if (!settings_parseline(line, &name, &value))
-                    continue;
-                if (strcasecmp(name, "pitch") == 0)
-                {
-                    num = atoi(value);
-                    if (num != PITCH_SPEED_100)
-                        sound_set_pitch(num);
-                }
-                if (strcasecmp(name, "stretch") == 0)
-                {
-                    num = atoi(value);
-                    if (num != PITCH_SPEED_100)
-                        dsp_set_timestretch(num);
-                }
-            }
-            close(fd);
-        }
-    }
-#endif
-
-
 }

apps/gui/pitchscreen.c

@@ -30,6 +30,5 @@ int reset_pitch(void)
 {
     sound_set_pitch(PITCH_SPEED_100);
     dsp_set_timestretch(PITCH_SPEED_100);
-    remove(PITCH_CFG_FILE);
     return 0;
 }

apps/gui/pitchscreen.h

@@ -22,7 +22,6 @@
 #ifndef _PITCHSCREEN_H_
 #define _PITCHSCREEN_H_
 
-#define PITCH_CFG_FILE ROCKBOX_DIR "/pitch.cfg"
 int gui_syncpitchscreen_run(void);
 int reset_pitch(void);
 

apps/onplay.c

@@ -708,23 +708,47 @@ static int browse_id3_wrapper(void)
 /* CONTEXT_WPS items */
 MENUITEM_FUNCTION(browse_id3_item, MENU_FUNC_CHECK_RETVAL, ID2P(LANG_MENU_SHOW_ID3_INFO),
                   browse_id3_wrapper, NULL, Icon_NOICON);
+
 #ifdef HAVE_PITCHCONTROL
 MENUITEM_FUNCTION(pitch_screen_item, 0, ID2P(LANG_PITCH),
                   gui_syncpitchscreen_run, NULL, Icon_Audio);
 MENUITEM_FUNCTION(pitch_reset_item, 0, ID2P(LANG_RESET_SETTING),
                   reset_pitch, NULL, Icon_Submenu_Entered);
 
+static int pitch_callback(int action,
+                          const struct menu_item_ex *this_item,
+                          struct gui_synclist *this_list);
+
+/* need special handling so we can toggle the icon */
+#define MAKE_PITCHMENU( name, str, callback, icon, ... )           \
+    static const struct menu_item_ex *name##_[]  = {__VA_ARGS__};  \
+    struct menu_callback_with_desc name##__ = {callback,str,icon}; \
+    static const struct menu_item_ex name =                        \
+        {MT_MENU|MENU_HAS_DESC|MENU_EXITAFTERTHISMENU|             \
+         MENU_ITEM_COUNT(sizeof( name##_)/sizeof(*name##_)),       \
+            { (void*)name##_},{.callback_and_desc = & name##__}};
+
+MAKE_PITCHMENU(pitch_menu, ID2P(LANG_PITCH),
+                pitch_callback, Icon_Audio,
+                &pitch_screen_item,
+                &pitch_reset_item);
+
 static int pitch_callback(int action,
                           const struct menu_item_ex *this_item,
                           struct gui_synclist *this_list)
 {
-    if (action == ACTION_ENTER_MENUITEM)
+    if (action == ACTION_ENTER_MENUITEM || action == ACTION_REQUEST_MENUITEM)
     {
+        pitch_menu__.icon_id = Icon_Submenu; /* if setting changed show + */
         int32_t ts = dsp_get_timestretch();
         if (sound_get_pitch() == PITCH_SPEED_100 && ts == PITCH_SPEED_100)
-        { /* if default then run pitch screen directly */
+        {
-            gui_syncpitchscreen_run();
+            pitch_menu__.icon_id = Icon_Audio;
-            action = ACTION_EXIT_MENUITEM;
+            if (action == ACTION_ENTER_MENUITEM)
+            { /* if default then run pitch screen directly */
+                gui_syncpitchscreen_run();
+                action = ACTION_EXIT_MENUITEM;
+            }
         }
     }
     return action;
@@ -732,14 +756,8 @@ static int pitch_callback(int action,
     (void)this_item;
     (void)this_list;
 }
-
-/* pitch submenu */
-MAKE_ONPLAYMENU(pitch_menu, ID2P(LANG_PITCH),
-                pitch_callback, Icon_Audio,
-                &pitch_screen_item,
-                &pitch_reset_item);
-
 #endif /*def HAVE_PITCHCONTROL*/
+
 #ifdef HAVE_ALBUMART
 MENUITEM_FUNCTION(view_album_art_item, 0, ID2P(LANG_VIEW_ALBUMART),
                   view_album_art, NULL, Icon_NOICON);

apps/plugins/pitch_screen.c

@@ -22,11 +22,6 @@
 #include "plugin.h"
 #include "lib/icon_helper.h"
 #include "lib/arg_helper.h"
-#include "lib/configfile.h"
-#include "../gui/pitchscreen.h" /*PITCH_CFG_FILE*/
-
-#define CFG_FILE PITCH_CFG_FILE
-#define CFG_VER  1
 
 #define ICON_BORDER 12 /* icons are currently 7x8, so add ~2 pixels  */
                        /*   on both sides when drawing */
@@ -54,12 +49,6 @@ struct pvars
 };
 static struct pvars pitch_vars;
 
-static struct configdata pitchcfg[] =
-{
-    {TYPE_INT, PITCH_MIN,  PITCH_MAX, { .int32_p = &pitch_vars.pitch }, "pitch", NULL},
-    {TYPE_INT, STRETCH_MIN, STRETCH_MAX, { .int32_p = &pitch_vars.stretch }, "stretch", NULL},
-};
-
 enum
 {
     PITCH_TOP = 0,
@@ -1229,9 +1218,6 @@ enum plugin_status plugin_start(const void* parameter)
     bool gui = false;
     rb->pcmbuf_set_low_latency(true);
 
-    struct pvars cur;
-    fill_pitchvars(&cur);
-
     /* Figure out whether to be in timestretch mode */
     if (parameter == NULL) /* gui mode */
     {
@@ -1241,24 +1227,11 @@ enum plugin_status plugin_start(const void* parameter)
             rb->settings_save();
         }
         gui = true;
-        if (rb->file_exists(CFG_FILE))
-        {
-            if (configfile_load(CFG_FILE, pitchcfg, 2, CFG_VER) >= 0)
-            {
-                if (pitch_vars.pitch != cur.pitch || pitch_vars.stretch != cur.stretch)
-                {
-                    if (rb->yesno_pop(ID2P(LANG_REVERT_TO_DEFAULT_SETTINGS)))
-                    {
-                        rb->sound_set_pitch(pitch_vars.pitch);
-                        rb->dsp_set_timestretch(pitch_vars.stretch);
-                    }
-                }
-            }
-        }
-
     }
     else
     {
+        struct pvars cur;
+        fill_pitchvars(&cur);
         fill_pitchvars(&pitch_vars);
         argparse((const char*) parameter, -1, NULL, &arg_callback);
         if (pitch_vars.pitch != cur.pitch)
@@ -1302,17 +1275,6 @@ enum plugin_status plugin_start(const void* parameter)
 
     if (gui && gui_syncpitchscreen_run() == 1)
             return PLUGIN_USB_CONNECTED;
-
-    if (gui)
-    {
-        fill_pitchvars(&pitch_vars);
-
-        if (pitch_vars.pitch != cur.pitch || pitch_vars.stretch != cur.stretch)
-        {
-            if (configfile_save(CFG_FILE, pitchcfg, 2, CFG_VER) < 0)
-                rb->splash(HZ, ID2P(LANG_ERROR_WRITING_CONFIG));
-        }
-    }
     rb->pcmbuf_set_low_latency(false);
     return PLUGIN_OK;
 }

apps/root_menu.c

@@ -321,9 +321,19 @@ static int recscrn(void* param)
 static int wpsscrn(void* param)
 {
     int ret_val = GO_TO_PREVIOUS;
+    int audstatus = audio_status();
     (void)param;
     push_current_activity(ACTIVITY_WPS);
-    if (audio_status())
+
+#ifdef HAVE_PITCHCONTROL
+    if (!audstatus)
+    {
+        sound_set_pitch(global_status.resume_pitch);
+        dsp_set_timestretch(global_status.resume_speed);
+    }
+#endif
+
+    if (audstatus)
     {
         talk_shutup();
         ret_val = gui_wps_show();

apps/settings.h

@@ -326,6 +326,10 @@ struct system_status
     uint32_t resume_crc32; /* crc32 of the name of the file */
     uint32_t resume_elapsed; /* elapsed time in last file */
     uint32_t resume_offset; /* byte offset in mp3 file */
+#ifdef HAVE_PITCHCONTROL
+    int32_t resume_pitch;
+    int32_t resume_speed;
+#endif
     int runtime;       /* current runtime since last charge */
     int topruntime;    /* top known runtime */
 #ifdef HAVE_DIRCACHE

apps/settings_list.c

@@ -977,6 +977,10 @@ const struct settings_list settings[] = {
     SYSTEM_SETTING(NVRAM(4), resume_elapsed, -1),
     SYSTEM_SETTING(NVRAM(4), resume_offset, -1),
     SYSTEM_SETTING(NVRAM(4), resume_modified, false),
+#ifdef HAVE_PITCHCONTROL
+    SYSTEM_SETTING(NVRAM(4), resume_pitch, PITCH_SPEED_100),
+    SYSTEM_SETTING(NVRAM(4), resume_speed, PITCH_SPEED_100),
+#endif
     CHOICE_SETTING(F_CB_ON_SELECT_ONLY|F_CB_ONLY_IF_CHANGED, repeat_mode,
                    LANG_REPEAT, REPEAT_OFF, "repeat", "off,all,one,shuffle"
 #ifdef AB_REPEAT_ENABLE

apps/settings_list.h

@@ -161,7 +161,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   8
+#define NVRAM_CONFIG_VERSION   9
 /* 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

firmware/sound.c

@@ -636,6 +636,8 @@ void sound_set_pitch(int32_t pitch)
         return;
 
     audiohw_set_pitch(pitch);
+    /* filter out invalid by grabbing the value actually set */
+    global_status.resume_pitch = dsp_get_pitch();
 }
 
 int32_t sound_get_pitch(void)

lib/rbcodec/dsp/tdspeed.c

@@ -27,6 +27,9 @@
 #include "dsp-util.h"
 #include "dsp_proc_entry.h"
 #include "tdspeed.h"
+#ifdef ROCKBOX
+#include "settings.h"
+#endif
 
 #ifndef assert
 #define assert(cond)
@@ -402,6 +405,9 @@ void dsp_set_timestretch(int32_t percent)
 
     struct dsp_config *dsp = dsp_get_config(CODEC_IDX_AUDIO);
     dsp_configure(dsp, TIMESTRETCH_SET_FACTOR, percent);
+#ifdef ROCKBOX /* filter out invalid by grabbing the value actually set */
+    global_status.resume_speed = st->factor;
+#endif
 }
 
 /* Return the timestretch ratio */

manual/rockbox_interface/wps.tex

@@ -288,18 +288,13 @@ This may even be the whole track.
 
   The value of the rate, pitch and speed
   is persistent, i.e. when the \dap\ is turned on it will
-  always be set to your last value set by \setting{Pitch Screen}.
+  always be set to the last value set by \setting{Pitch Screen} or Bookmarks.
   Selecting \setting{Pitch} again will now display a menu with
   \setting{Pitch} and \setting{Reset Setting}.
   Selecting \setting{Reset Setting} will reset the pitch to 100\% now and at next boot.
-  However the rate, pitch and speed information will be stored in any bookmarks
+  However the rate, pitch and speed information is stored in any bookmarks
   you may create (see \reference{ref:Bookmarkconfigactual})
-  while the pitch is altered and will be restored upon
+  and will be restored upon playing back those bookmarks.
-  playing back those bookmarks.
-
-  \note{ If a bookmark has changed pitch, settings will remain till
-  changed again or the \dap{} is restarted and your (default) settings will then be
-  restored}
 
   \begin{btnmap}
     \ActionPsToggleMode