Playlist viewer: Add new options to allow formatting using tags

Offer new options to show elegantly your entries in any playlist/dynamic playlist viewer. This is especially important if you dual boot an iPod with Stock OS and want to sync with iTunes; with this very popular setup, file names are obfuscated which results in any Rockbox playlist viewer difficult to enjoy, and it was a long standing issue reported by several Rockbox users over the years. The only way to show the title was to open a contextual menu on each song to get infos about the selected song, which is a very long and anti-ergonomic process to understand what is on your current playlist/randomized playlist. The idea of this patch is to provide new alternatives that the user can select. I personally selected the Title & Album view which provides excellent readability.

This patch was built with performance in mind using lazy loading to load one by one the tags then cache a string and use the little cache as much as possible to make scrolling in the same area as smooth as possible. Performance remains very acceptable even on an iPod 4G with its original hard drive. Using a real compact flash with my iPod Mini 2G reduces the latency even more.

Those new options are disabled by default because they impact noticeably the scrolling performance and are less relevant if your files are decently properly named.

Unfortunately, the search feature in a playlist will need to continue to use the raw filename because reading the tags for a whole playlist is a performance disaster. This works decently while viewing just because I made a code that load those one by one as much as possible.

I focused also on testing that the opening of the playlist viewer UI remained fast, and loading one by one with lazy loading allows to get very little overhead in this regard.

Change-Id: I00d9c802e29f8372447813b035bbae207a016467

6 files changed, 223 insertions, 55 deletions

diff --git a/apps/lang/english.lang b/apps/lang/english.lang
index c518d3573c..1ee1ffac0f 100644
--- a/apps/lang/english.lang
+++ b/apps/lang/english.lang
@@ -2032,6 +2032,34 @@
   </voice>
 </phrase>
 <phrase>
+  id: LANG_DISPLAY_TITLEALBUM_FROMTAGS
+  desc: track display options
+  user: core
+  <source>
+    *: "Title & Album from ID3 tags"
+  </source>
+  <dest>
+    *: "Title & Album from ID3 tags"
+  </dest>
+  <voice>
+    *: "Title and Album from tags"
+  </voice>
+</phrase>
+<phrase>
+  id: LANG_DISPLAY_TITLE_FROMTAGS
+  desc: track display options
+  user: core
+  <source>
+    *: "Title from ID3 tags"
+  </source>
+  <dest>
+    *: "Title from ID3 tags"
+  </dest>
+  <voice>
+    *: "Title from tags"
+  </voice>
+</phrase>
+<phrase>
   id: LANG_BUILDING_DATABASE
   desc: splash database building progress
   user: core
diff --git a/apps/lang/francais.lang b/apps/lang/francais.lang
index 518b48fb96..b8883d4997 100644
--- a/apps/lang/francais.lang
+++ b/apps/lang/francais.lang
@@ -2005,6 +2005,34 @@
   </voice>
 </phrase>
 <phrase>
+  id: LANG_DISPLAY_TITLEALBUM_FROMTAGS
+  desc: track display options
+  user: core
+  <source>
+    *: "Title & Album from ID3 tags"
+  </source>
+  <dest>
+    *: "Titre & Album depuis les tags ID3"
+  </dest>
+  <voice>
+    *: "Titre et Album depuis les tags"
+  </voice>
+</phrase>
+<phrase>
+  id: LANG_DISPLAY_TITLE_FROMTAGS
+  desc: track display options
+  user: core
+  <source>
+    *: "Title from ID3 tags"
+  </source>
+  <dest>
+    *: "Titre depuis les tags ID3"
+  </dest>
+  <voice>
+    *: "Titre depuis les tags"
+  </voice>
+</phrase>
+<phrase>
   id: LANG_BUILDING_DATABASE
   desc: splash database building progress
   user: core
diff --git a/apps/playlist.h b/apps/playlist.h
index f7426df9a3..2fb1ce100e 100644
--- a/apps/playlist.h
+++ b/apps/playlist.h
@@ -33,6 +33,8 @@
 #define PLAYLIST_ATTR_QUEUED    0x01
 #define PLAYLIST_ATTR_INSERTED  0x02
 #define PLAYLIST_ATTR_SKIPPED   0x04
+#define PLAYLIST_ATTR_RETRIEVE_ID3_ATTEMPTED   0x08
+#define PLAYLIST_ATTR_RETRIEVE_ID3_SUCCEEDED   0x10
 
 #define PLAYLIST_DISPLAY_COUNT  10
 
diff --git a/apps/playlist_viewer.c b/apps/playlist_viewer.c
index 5bf547a3fc..d556f3b557 100644
--- a/apps/playlist_viewer.c
+++ b/apps/playlist_viewer.c
@@ -50,6 +50,7 @@
 #include "playlist_menu.h"
 #include "menus/exported_menus.h"
 #include "yesno.h"
+#include "playback.h"
 
 /* Maximum number of tracks we can have loaded at one time */
 #define MAX_PLAYLIST_ENTRIES 200
@@ -142,7 +143,7 @@ static bool playlist_viewer_init(struct playlist_viewer * viewer,
                                  const char* filename, bool reload,
                                  int *most_recent_selection);
 
-static void format_line(const struct playlist_entry* track, char* str,
+static void format_line(struct playlist_entry* track, char* str,
                         int len);
 
 static bool update_playlist(bool force);
@@ -159,6 +160,27 @@ static void playlist_buffer_init(struct playlist_buffer *pb, char *names_buffer,
     pb->num_loaded = 0;
 }
 
+static int playlist_buffer_get_index(struct playlist_buffer *pb, int index)
+{
+    int buffer_index;
+    if (pb->direction == FORWARD)
+    {
+        if (index >= pb->first_index)
+            buffer_index = index-pb->first_index;
+        else /* rotation : track0 in buffer + requested track */
+            buffer_index = viewer.num_tracks-pb->first_index+index;
+    }
+    else
+    {
+        if (index <= pb->first_index)
+            buffer_index = pb->first_index-index;
+        else /* rotation : track0 in buffer + dist from the last track
+                to the requested track (num_tracks-requested track) */
+            buffer_index = pb->first_index+viewer.num_tracks-index;
+    }
+    return buffer_index;
+}
+
 /*
  * Loads the entries following 'index' in the playlist buffer
  */
@@ -227,6 +249,25 @@ static void playlist_buffer_load_entries_screen(struct playlist_buffer * pb,
     playlist_buffer_load_entries(pb, start, direction);
 }
 
+static bool retrieve_id3_tags(const int index, const char* name, struct mp3entry *id3, int flags)
+{
+    bool id3_retrieval_successful = false;
+
+    if (!viewer.playlist &&
+        (audio_status() & AUDIO_STATUS_PLAY) &&
+        (playlist_get_resume_info(&viewer.current_playing_track) == index))
+    {
+        copy_mp3entry(id3, audio_current_track()); /* retrieve id3 from RAM */
+        id3_retrieval_successful = true;
+    }
+    else
+    {
+    /* Read from disk, the database, doesn't store frequency, file size or codec (g4470) ChrisS*/
+        id3_retrieval_successful = get_metadata_ex(id3, -1, name, flags);
+    }
+    return id3_retrieval_successful;
+}
+
 static int playlist_entry_load(struct playlist_entry *entry, int index,
                                char* name_buffer, int remaining_size)
 {
@@ -242,6 +283,13 @@ static int playlist_entry_load(struct playlist_entry *entry, int index,
 
     len = strlcpy(name_buffer, info.filename, remaining_size) + 1;
 
+    if (global_settings.playlist_viewer_track_display >
+        PLAYLIST_VIEWER_ENTRY_SHOW_FULL_PATH && len <= remaining_size)
+    {
+        /* Allocate space for the id3viewc if the option is enabled */
+        len += MAX_PATH + 1;
+    }
+
     if (len <= remaining_size)
     {
         entry->name = name_buffer;
@@ -253,27 +301,6 @@ static int playlist_entry_load(struct playlist_entry *entry, int index,
     return -1;
 }
 
-static int playlist_buffer_get_index(struct playlist_buffer *pb, int index)
-{
-    int buffer_index;
-    if (pb->direction == FORWARD)
-    {
-        if (index >= pb->first_index)
-            buffer_index = index-pb->first_index;
-        else /* rotation : track0 in buffer + requested track */
-            buffer_index = viewer.num_tracks-pb->first_index+index;
-    }
-    else
-    {
-        if (index <= pb->first_index)
-            buffer_index = pb->first_index-index;
-        else /* rotation : track0 in buffer + dist from the last track
-                to the requested track (num_tracks-requested track) */
-            buffer_index = pb->first_index+viewer.num_tracks-index;
-    }
-    return buffer_index;
-}
-
 #define distance(a, b) \
     a>b? (a) - (b) : (b) - (a)
 static bool playlist_buffer_needs_reload(struct playlist_buffer* pb,
@@ -440,7 +467,9 @@ static void format_name(char* dest, const char* src, size_t bufsz)
 {
     switch (global_settings.playlist_viewer_track_display)
     {
-        case 0:
+        case PLAYLIST_VIEWER_ENTRY_SHOW_FILE_NAME:
+        case PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE_AND_ALBUM: /* If loading from tags failed, only display the file name */
+        case PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE: /* If loading from tags failed, only display the file name */
         default:
         {
             /* Only display the filename */
@@ -450,7 +479,7 @@ static void format_name(char* dest, const char* src, size_t bufsz)
             strrsplt(dest, '.');
             break;
         }
-        case 1:
+        case PLAYLIST_VIEWER_ENTRY_SHOW_FULL_PATH:
             /* Full path */
             strlcpy(dest, src, bufsz);
             break;
@@ -458,22 +487,99 @@ static void format_name(char* dest, const char* src, size_t bufsz)
 }
 
 /* Format display line */
-static void format_line(const struct playlist_entry* track, char* str,
+static void format_line(struct playlist_entry* track, char* str,
                         int len)
 {
-    char name[MAX_PATH];
+    char *id3viewc = NULL;
     char *skipped = "";
-    format_name(name, track->name, sizeof(name));
-
     if (track->attr & PLAYLIST_ATTR_SKIPPED)
         skipped = "(ERR) ";
-
-    if (global_settings.playlist_viewer_indices)
-        /* Display playlist index */
-        snprintf(str, len, "%d. %s%s", track->display_index, skipped, name);
+    if (!(track->attr & PLAYLIST_ATTR_RETRIEVE_ID3_ATTEMPTED) &&
+        (global_settings.playlist_viewer_track_display ==
+            PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE_AND_ALBUM ||
+        global_settings.playlist_viewer_track_display ==
+            PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE
+    ))
+    {
+        track->attr |= PLAYLIST_ATTR_RETRIEVE_ID3_ATTEMPTED;
+        struct mp3entry id3;
+        bool retrieve_success = retrieve_id3_tags(track->index, track->name,
+                                            &id3, METADATA_EXCLUDE_ID3_PATH);
+        if (retrieve_success)
+        {
+            if (!id3viewc)
+            {
+                id3viewc = track->name + strlen(track->name) + 1;
+            }
+            struct mp3entry * pid3 = &id3;
+            id3viewc[0] = '\0';
+            if (global_settings.playlist_viewer_track_display ==
+                PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE_AND_ALBUM)
+            {
+                /* Title & Album */
+                if (pid3->title && pid3->title[0] != '\0')
+                {
+                    char* cur_str = id3viewc;
+                    int title_len = strlen(pid3->title);
+                    int rem_space = MAX_PATH;
+                    for (int i = 0; i < title_len && rem_space > 0; i++)
+                    {
+                        cur_str[0] = pid3->title[i];
+                        cur_str++;
+                        rem_space--;
+                    }
+                    if (rem_space > 10)
+                    {
+                        cur_str[0] = (char) ' ';
+                        cur_str[1] = (char) '-';
+                        cur_str[2] = (char) ' ';
+                        cur_str += 3;
+                        rem_space -= 3;
+                        cur_str = strmemccpy(cur_str, pid3->album && pid3->album[0] != '\0' ?
+                            pid3->album : (char*) str(LANG_TAGNAVI_UNTAGGED), rem_space);
+                        if (cur_str)
+                            track->attr |= PLAYLIST_ATTR_RETRIEVE_ID3_SUCCEEDED;
+                    }
+                }
+            }
+            else if (global_settings.playlist_viewer_track_display ==
+                PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE)
+            {
+                /* Just the title */
+                if (pid3->title && pid3->title[0] != '\0' &&
+                    strmemccpy(id3viewc, pid3->title, MAX_PATH)
+                )
+                    track->attr |= PLAYLIST_ATTR_RETRIEVE_ID3_SUCCEEDED;
+            }
+            /* Yield to reduce as much as possible the perceived UI lag, 
+            because retrieving id3 tags is an expensive operation */
+            yield();
+        }
+    }
+    
+    if (!(track->attr & PLAYLIST_ATTR_RETRIEVE_ID3_SUCCEEDED))
+    {
+        /* Simply use a formatted file name */
+        char name[MAX_PATH];
+        format_name(name, track->name, sizeof(name));
+        if (global_settings.playlist_viewer_indices)
+            /* Display playlist index */
+            snprintf(str, len, "%d. %s%s", track->display_index, skipped, name);
+        else
+            snprintf(str, len, "%s%s", skipped, name);
+    }
     else
-        snprintf(str, len, "%s%s", skipped, name);
-
+    {
+        if (!id3viewc)
+        {
+            id3viewc = track->name + strlen(track->name) + 1;
+        }
+        if (global_settings.playlist_viewer_indices)
+            /* Display playlist index */
+            snprintf(str, len, "%d. %s%s", track->display_index, skipped, id3viewc);
+        else
+            snprintf(str, len, "%s%s", skipped, id3viewc);
+    }
 }
 
 /* Update playlist in case something has changed or forced */
@@ -512,20 +618,7 @@ static bool update_playlist(bool force)
 static enum pv_onplay_result show_track_info(const struct playlist_entry *current_track)
 {
     struct mp3entry id3;
-    bool id3_retrieval_successful = false;
-
-    if (!viewer.playlist &&
-        (audio_status() & AUDIO_STATUS_PLAY) &&
-        (playlist_get_resume_info(&viewer.current_playing_track) == current_track->index))
-    {
-        copy_mp3entry(&id3, audio_current_track()); /* retrieve id3 from RAM */
-        id3_retrieval_successful = true;
-    }
-    else
-    {
-    /* Read from disk, the database, doesn't store frequency, file size or codec (g4470) ChrisS*/
-        id3_retrieval_successful = get_metadata(&id3, -1, current_track->name);
-    }
+    bool id3_retrieval_successful = retrieve_id3_tags(current_track->index, current_track->name, &id3, 0);
 
     return id3_retrieval_successful &&
             browse_id3_ex(&id3, viewer.playlist, current_track->display_index,
@@ -790,11 +883,17 @@ static int playlist_callback_voice(int selected_item, void *data)
 
     switch(global_settings.playlist_viewer_track_display)
     {
-        case 1: /*full path*/
+        case PLAYLIST_VIEWER_ENTRY_SHOW_FULL_PATH:
+            /*full path*/
             talk_fullpath(track->name, true);
             break;
         default:
-        case 0: /*filename only*/
+        case PLAYLIST_VIEWER_ENTRY_SHOW_FILE_NAME:
+            /*filename only*/
+        case PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE_AND_ALBUM:
+            /* If loading from tags failed, only talk the file name */
+        case PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE:
+            /* If loading from tags failed, only talk the file name */
             talk_file_or_spell(NULL, track->name, NULL, true);
             break;
     }
@@ -1154,7 +1253,8 @@ static int say_search_item(int selected_item, void *data)
 {
     struct playlist_search_data *s_data = data;
     playlist_get_track_info(viewer.playlist, s_data->found_indicies[selected_item], s_data->track);
-    if(global_settings.playlist_viewer_track_display == 1) /* full path*/
+    if(global_settings.playlist_viewer_track_display == PLAYLIST_VIEWER_ENTRY_SHOW_FULL_PATH)
+        /* full path*/
         talk_fullpath(s_data->track->filename, false);
     else talk_file_or_spell(NULL, s_data->track->filename, NULL, false);
     return 0;
@@ -1197,7 +1297,8 @@ bool search_playlist(void)
 
         playlist_get_track_info(viewer.playlist, i, &track);
         const char *trackname = track.filename;
-        if (track_display == 0) /* if we only display filename only search filename */
+        if (track_display != PLAYLIST_VIEWER_ENTRY_SHOW_FULL_PATH)
+            /* if we only display filename only search filename */
             trackname = strrchr(track.filename, '/');
 
         if (trackname && strcasestr(trackname, search_str))
diff --git a/apps/settings.h b/apps/settings.h
index 1343538b0b..a9a9f647e3 100644
--- a/apps/settings.h
+++ b/apps/settings.h
@@ -78,6 +78,13 @@ enum
     TRIG_TYPE_NEW_FILE
 };
 
+enum {
+    PLAYLIST_VIEWER_ENTRY_SHOW_FILE_NAME = 0,
+    PLAYLIST_VIEWER_ENTRY_SHOW_FULL_PATH = 1,
+    PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE_AND_ALBUM = 2,
+    PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE = 3
+};
+
 #ifdef HAVE_CROSSFADE
 enum {
     CROSSFADE_ENABLE_OFF = 0,
diff --git a/apps/settings_list.c b/apps/settings_list.c
index 3b29703fe9..a9f627bfa6 100644
--- a/apps/settings_list.c
+++ b/apps/settings_list.c
@@ -1421,9 +1421,11 @@ const struct settings_list settings[] = {
     OFFON_SETTING(0,playlist_viewer_indices,LANG_SHOW_INDICES,true,
                   "playlist viewer indices",NULL),
     CHOICE_SETTING(0, playlist_viewer_track_display, LANG_TRACK_DISPLAY, 0,
-                   "playlist viewer track display","track name,full path",
-                   NULL, 2, ID2P(LANG_DISPLAY_TRACK_NAME_ONLY),
-                   ID2P(LANG_DISPLAY_FULL_PATH)),
+                   "playlist viewer track display",
+                   "track name,full path,title and album from tags,title from tags",
+                   NULL, 4, ID2P(LANG_DISPLAY_TRACK_NAME_ONLY),
+                   ID2P(LANG_DISPLAY_FULL_PATH),ID2P(LANG_DISPLAY_TITLEALBUM_FROMTAGS),
+                   ID2P(LANG_DISPLAY_TITLE_FROMTAGS)),
     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)),