9 files changed, 145 insertions, 89 deletions

diff --git a/apps/buffering.c b/apps/buffering.c
index bf41544c56..b9f30fc6df 100644
--- a/apps/buffering.c
+++ b/apps/buffering.c
@@ -652,11 +652,9 @@ static bool buffer_handle(int handle_id, size_t to_buffer)
     trigger_cpu_boost();
 
     if (h->type == TYPE_ID3) {
-        if (!get_metadata(ringbuf_ptr(h->data), h->fd, h->path)) {
-            /* metadata parsing failed: clear the buffer. */
-            wipe_mp3entry(ringbuf_ptr(h->data));
-        }
-        close_fd(&h->fd);
+        get_metadata_ex(ringbuf_ptr(h->data),
+                        h->fd, h->path, METADATA_CLOSE_FD_ON_EXIT);
+        h->fd = -1; /* with above, behavior same as close_fd */
         h->widx = ringbuf_add(h->data, h->filesize);
         h->end  = h->filesize;
         send_event(BUFFER_EVENT_FINISHED, &handle_id);
diff --git a/apps/iap/iap-core.c b/apps/iap/iap-core.c
index da04a67311..3faf8237bf 100644
--- a/apps/iap/iap-core.c
+++ b/apps/iap/iap-core.c
@@ -685,7 +685,6 @@ bool iap_getc(const unsigned char x)
 void iap_get_trackinfo(const unsigned int track, struct mp3entry* id3)
 {
     int tracknum;
-    int fd;
     struct playlist_track_info info;
 
     tracknum = track;
@@ -699,10 +698,8 @@ void iap_get_trackinfo(const unsigned int track, struct mp3entry* id3)
     if(playlist_next(0) != tracknum)
     {
         playlist_get_track_info(NULL, tracknum, &info);
-        fd = open(info.filename, O_RDONLY);
-        memset(id3, 0, sizeof(*id3));
-        get_metadata(id3, fd, info.filename);
-        close(fd);
+        /* memset(id3, 0, sizeof(*id3)) --get_metadata does this for us */
+        get_metadata(id3, -1, info.filename);
     } else {
         memcpy(id3, audio_current_track(), sizeof(*id3));
     }
diff --git a/apps/iap/iap-lingo4.c b/apps/iap/iap-lingo4.c
index e1ab150fd9..6eb63003a5 100644
--- a/apps/iap/iap-lingo4.c
+++ b/apps/iap/iap-lingo4.c
@@ -1787,7 +1787,6 @@ void iap_handlepkt_mode4(const unsigned int len, const unsigned char *buf)
         {
             unsigned char data[70] = {0x04, 0x00, 0xFF};
             struct mp3entry id3;
-            int fd;
             size_t len;
             long tracknum = get_u32(&buf[3]);
 
@@ -1802,10 +1801,8 @@ void iap_handlepkt_mode4(const unsigned int len, const unsigned char *buf)
             {
                 struct playlist_track_info info;
                 playlist_get_track_info(NULL, tracknum, &info);
-                fd = open(info.filename, O_RDONLY);
-                memset(&id3, 0, sizeof(struct mp3entry));
-                get_metadata(&id3, fd, info.filename);
-                close(fd);
+                /* memset(&id3, 0, sizeof(struct mp3entry)); --get_metadata does this for us */
+                get_metadata(&id3, -1, info.filename);
             }
             /* Return the requested track data */
             switch(cmd)
diff --git a/apps/playback.c b/apps/playback.c
index 18f62fb69b..8a30af5199 100644
--- a/apps/playback.c
+++ b/apps/playback.c
@@ -2022,8 +2022,7 @@ static int audio_load_track(void)
         if (fd >= 0)
         {
             id3_mutex_lock();
-            if(!get_metadata(ub_id3, fd, path))
-                wipe_mp3entry(ub_id3);
+            get_metadata(ub_id3, fd, path);
             id3_mutex_unlock();
         }
 
diff --git a/apps/playlist_viewer.c b/apps/playlist_viewer.c
index 579eba5a61..d06a3f3df6 100644
--- a/apps/playlist_viewer.c
+++ b/apps/playlist_viewer.c
@@ -51,6 +51,10 @@
 #include "menus/exported_menus.h"
 #include "yesno.h"
 
+#if defined(HAVE_TC_RAMCACHE) && defined(HAVE_DIRCACHE)
+#include "tagcache.h"
+#endif
+
 /* Maximum number of tracks we can have loaded at one time */
 #define MAX_PLAYLIST_ENTRIES 200
 
@@ -60,11 +64,11 @@
 
 /* Information about a specific track */
 struct playlist_entry {
-    char *name;                 /* Formatted track name                     */
+    char *name;                 /* track path                               */
+    char *title;                /* Formatted track name                     */
     int index;                  /* Playlist index                           */
     int display_index;          /* Display index                            */
-    bool queued;                /* Is track queued?                         */
-    bool skipped;               /* Is track marked as bad?                  */
+    int attr;                  /* Is track queued?; Is track marked as bad?*/
 };
 
 enum direction
@@ -86,6 +90,8 @@ enum pv_onplay_result {
 
 struct playlist_buffer
 {
+    struct playlist_entry tracks[MAX_PLAYLIST_ENTRIES];
+
     char *name_buffer;        /* Buffer used to store track names */
     int buffer_size;          /* Size of name buffer */
 
@@ -97,7 +103,6 @@ struct playlist_buffer
                                  the buffer has a real index < to the
                                  real index of the the first track)*/
 
-    struct playlist_entry tracks[MAX_PLAYLIST_ENTRIES];
     int num_loaded;           /* Number of track entries loaded in buffer */
 };
 
@@ -136,7 +141,6 @@ static bool playlist_viewer_init(struct playlist_viewer * viewer,
                                  const char* filename, bool reload,
                                  int *most_recent_selection);
 
-static void format_name(char* dest, const char* src);
 static void format_line(const struct playlist_entry* track, char* str,
                         int len);
 
@@ -221,6 +225,40 @@ static void playlist_buffer_load_entries_screen(struct playlist_buffer * pb,
     }
 }
 
+static bool retrieve_track_metadata(struct mp3entry *id3, const char *filename, int flags)
+{
+    bool success = true;
+#if defined(HAVE_TC_RAMCACHE) && defined(HAVE_DIRCACHE)
+    /* try to get the id3 data from the database */
+    /* the database, doesn't store frequency, file size or codec (g4470) ChrisS*/
+    if ((flags & METADATA_EXCLUDE_ID3_PATH) || !tagcache_fill_tags(id3, filename))
+#endif
+    /* fall back to reading the file from disk */
+    {
+        success = get_metadata_ex(id3, -1, filename, flags);
+    }
+    return success;
+}
+
+static int load_track_title(char* buffer, size_t bufsz, char *filename)
+{
+    struct mp3entry id3;
+    if (retrieve_track_metadata(&id3, filename, METADATA_EXCLUDE_ID3_PATH)
+       && id3.title && id3.title[0] != '\0')
+    {
+        const char *artist = id3.artist;
+        if (!artist)
+            artist = id3.albumartist;
+        if(!artist)
+            artist = str(LANG_TAGNAVI_UNTAGGED);
+
+        size_t len = snprintf(buffer, bufsz, "%s - %s", artist, id3.title) + 1;
+        if (len < bufsz)
+            return len;
+    }
+    return 0; /*Failure*/
+}
+
 static int playlist_entry_load(struct playlist_entry *entry, int index,
                                char* name_buffer, int remaining_size)
 {
@@ -234,17 +272,29 @@ static int playlist_entry_load(struct playlist_entry *entry, int index,
     if (playlist_get_track_info(viewer.playlist, index, &info) < 0)
         return -1;
 
-    len = strlen(info.filename) + 1;
+    len = strlcpy(name_buffer, info.filename, remaining_size) + 1;
 
     if (len <= remaining_size)
     {
-        strcpy(name_buffer, info.filename);
-
         entry->name = name_buffer;
+        entry->title = name_buffer;
         entry->index = info.index;
         entry->display_index = info.display_index;
-        entry->queued = info.attr & PLAYLIST_ATTR_QUEUED;
-        entry->skipped = info.attr & PLAYLIST_ATTR_SKIPPED;
+        entry->attr = info.attr & (PLAYLIST_ATTR_SKIPPED | PLAYLIST_ATTR_QUEUED);
+
+        if (global_settings.playlist_viewer_track_display == 2) /* title */
+        {
+            /* advance buffer position past filename, adjust length remaining */
+            name_buffer += len;
+            remaining_size -= len;
+            int tlen = load_track_title(name_buffer, remaining_size, info.filename);
+            if (tlen > 0)
+            {
+                entry->title = name_buffer;
+                len += tlen;
+            }
+        }
+
         return len;
     }
     return -1;
@@ -433,7 +483,7 @@ static bool playlist_viewer_init(struct playlist_viewer * viewer,
 }
 
 /* Format trackname for display purposes */
-static void format_name(char* dest, const char* src)
+static void format_name(char* dest, const char* src, size_t bufsz)
 {
     switch (global_settings.playlist_viewer_track_display)
     {
@@ -442,17 +492,16 @@ static void format_name(char* dest, const char* src)
         {
             /* Only display the filename */
             char* p = strrchr(src, '/');
-
-            strcpy(dest, p+1);
-
+            strlcpy(dest, p+1, bufsz);
             /* Remove the extension */
             strrsplt(dest, '.');
-
             break;
         }
+        case 2: /* Artist - Title */
+            /*fall-through*/
         case 1:
             /* Full path */
-            strcpy(dest, src);
+            strlcpy(dest, src, bufsz);
             break;
     }
 }
@@ -463,10 +512,9 @@ static void format_line(const struct playlist_entry* track, char* str,
 {
     char name[MAX_PATH];
     char *skipped = "";
+    format_name(name, track->title, sizeof(name));
 
-    format_name(name, track->name);
-
-    if (track->skipped)
+    if (track->attr & PLAYLIST_ATTR_SKIPPED)
         skipped = "(ERR) ";
 
     if (global_settings.playlist_viewer_indices)
@@ -524,13 +572,7 @@ static enum pv_onplay_result show_track_info(const struct playlist_entry *curren
     }
     else
     {
-        int fd = open(current_track->name, O_RDONLY);
-        if (fd >= 0)
-        {
-            if (get_metadata(&id3, fd, current_track->name))
-                id3_retrieval_successful = true;
-            close(fd);
-        }
+        id3_retrieval_successful = retrieve_track_metadata(&id3, current_track->name, 0);
     }
 
     return id3_retrieval_successful &&
@@ -755,7 +797,7 @@ static enum themable_icons playlist_callback_icons(int selected_item,
         /* Track we are moving */
         return Icon_Moving;
     }
-    else if (track->queued)
+    else if (track->attr & PLAYLIST_ATTR_QUEUED)
     {
         /* Queued track */
         return Icon_Queued;
@@ -776,17 +818,27 @@ static int playlist_callback_voice(int selected_item, void *data)
             talk_id(LANG_NOW_PLAYING, true);
         if (track->index == local_viewer->moving_track)
             talk_id(VOICE_TRACK_TO_MOVE, true);
-        if (track->queued)
+        if (track->attr & PLAYLIST_ATTR_QUEUED)
             talk_id(VOICE_QUEUED, true);
     }
-    if (track->skipped)
+    if (track->attr & PLAYLIST_ATTR_SKIPPED)
         talk_id(VOICE_BAD_TRACK, true);
     if (global_settings.playlist_viewer_indices)
         talk_number(track->display_index, true);
 
-    if(global_settings.playlist_viewer_track_display)
-        talk_fullpath(track->name, true);
-    else talk_file_or_spell(NULL, track->name, NULL, true);
+    switch(global_settings.playlist_viewer_track_display)
+    {
+        case 1: /*full path*/
+            talk_fullpath(track->name, true);
+            break;
+        case 2: /*title*/
+            talk_spell(track->title, true);
+            break;
+        default:
+        case 0: /*filename only*/
+            talk_file_or_spell(NULL, track->name, NULL, true);
+            break;
+    }
     if (viewer.moving_track != -1)
         talk_ids(true,VOICE_PAUSE, VOICE_MOVING_TRACK);
 
@@ -1132,11 +1184,11 @@ static void close_playlist_viewer(void)
 static const char* playlist_search_callback_name(int selected_item, void * data,
                                                  char *buffer, size_t buffer_len)
 {
-    (void)buffer_len; /* this should probably be used */
     int *found_indicies = (int*)data;
     static struct playlist_track_info track;
     playlist_get_track_info(viewer.playlist, found_indicies[selected_item], &track);
-    format_name(buffer, track.filename);
+
+    format_name(buffer, track.filename, buffer_len);
     return buffer;
 }
 
@@ -1145,7 +1197,7 @@ static int say_search_item(int selected_item, void *data)
     int *found_indicies = (int*)data;
     static struct playlist_track_info track;
     playlist_get_track_info(viewer.playlist,found_indicies[selected_item],&track);
-    if(global_settings.playlist_viewer_track_display)
+    if(global_settings.playlist_viewer_track_display == 1)
         talk_fullpath(track.filename, false);
     else talk_file_or_spell(NULL, track.filename, NULL, false);
     return 0;
diff --git a/apps/settings_list.c b/apps/settings_list.c
index 09a45f1faa..60a2735e90 100644
--- a/apps/settings_list.c
+++ b/apps/settings_list.c
@@ -1415,9 +1415,9 @@ 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,id3 title",
+                   NULL, 3, ID2P(LANG_DISPLAY_TRACK_NAME_ONLY),
+                   ID2P(LANG_DISPLAY_FULL_PATH), ID2P(LANG_ID3_TITLE)),
     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)),
diff --git a/apps/tagcache.c b/apps/tagcache.c
index 5413e2def0..1412647368 100644
--- a/apps/tagcache.c
+++ b/apps/tagcache.c
@@ -2193,7 +2193,6 @@ static void NO_INLINE add_tagcache(char *path, unsigned long mtime)
     struct mp3entry id3;
     struct temp_file_entry entry;
     bool ret;
-    int fd;
     int idx_id = -1;
     char tracknumfix[3];
     int offset = 0;
@@ -2266,21 +2265,16 @@ static void NO_INLINE add_tagcache(char *path, unsigned long mtime)
         }
     }
 
-    fd = open(path, O_RDONLY);
-    if (fd < 0)
-    {
-        logf("open fail: %s", path);
-        return ;
-    }
-
-    memset(&id3, 0, sizeof(struct mp3entry));
+    /*memset(&id3, 0, sizeof(struct mp3entry)); -- get_metadata does this for us */
     memset(&entry, 0, sizeof(struct temp_file_entry));
     memset(&tracknumfix, 0, sizeof(tracknumfix));
-    ret = get_metadata(&id3, fd, path);
-    close(fd);
+    ret = get_metadata_ex(&id3, -1, path, METADATA_EXCLUDE_ID3_PATH);
 
     if (!ret)
+    {
+        logf("get_metadata fail: %s", path);
         return ;
+    }
 
     logf("-> %s", path);
 
diff --git a/lib/rbcodec/metadata/metadata.c b/lib/rbcodec/metadata/metadata.c
index 19147ccdb3..28cef46d92 100644
--- a/lib/rbcodec/metadata/metadata.c
+++ b/lib/rbcodec/metadata/metadata.c
@@ -394,25 +394,19 @@ unsigned int probe_file_format(const char *filename)
 /* Note, that this returns false for successful, true for error! */
 bool mp3info(struct mp3entry *entry, const char *filename)
 {
-    int fd;
-    bool result;
-
-    fd = open(filename, O_RDONLY);
-    if (fd < 0)
-        return true;
-
-    result = !get_metadata(entry, fd, filename);
-
-    close(fd);
-
-    return result;
+    return !get_metadata(entry, -1, filename);
 }
 
 /* Get metadata for track - return false if parsing showed problems with the
- * file that would prevent playback.
+ * file that would prevent playback. supply a filedescriptor <0 and the file will be opened
+ * and closed automatically within the get_metadata call
+ * get_metadata_ex allows flags to change the way get_metadata behaves
+ * METADATA_EXCLUDE_ID3_PATH  won't copy filename path to the id3 path buffer 
+ * METADATA_CLOSE_FD_ON_EXIT closes the open filedescriptor on exit
  */
-bool get_metadata(struct mp3entry* id3, int fd, const char* trackname)
+bool get_metadata_ex(struct mp3entry* id3, int fd, const char* trackname, int flags)
 {
+    bool success = true;
     const struct afmt_entry *entry;
     int logfd = 0;
     DEBUGF("Read metadata for %s\n", trackname);
@@ -426,10 +420,20 @@ bool get_metadata(struct mp3entry* id3, int fd, const char* trackname)
             close(logfd);
         }
     }
-
     /* Clear the mp3entry to avoid having bogus pointers appear */
     wipe_mp3entry(id3);
 
+    if (fd < 0)
+    {
+        fd = open(trackname, O_RDONLY);
+        flags |= METADATA_CLOSE_FD_ON_EXIT;
+        if (fd < 0)
+        {
+            DEBUGF("Error opening %s\n", trackname);
+            return false; /*Failure*/
+        }
+    }
+
     /* Take our best guess at the codec type based on file extension */
     id3->codectype = probe_file_format(trackname);
 
@@ -443,19 +447,31 @@ bool get_metadata(struct mp3entry* id3, int fd, const char* trackname)
     if (!entry->parse_func)
     {
         DEBUGF("nothing to parse for %s (format %s)\n", trackname, entry->label);
-        return false;
+        success = false;
     }
-
-    if (!entry->parse_func(fd, id3))
+    else if (!entry->parse_func(fd, id3))
     {
         DEBUGF("parsing %s failed (format: %s)\n", trackname, entry->label);
-        return false;
+        success = false;
+        wipe_mp3entry(id3); /* ensure the mp3entry is clear */
     }
 
-    lseek(fd, 0, SEEK_SET);
-    strlcpy(id3->path, trackname, sizeof(id3->path));
-    /* We have successfully read the metadata from the file */
-    return true;
+    if ((flags & METADATA_CLOSE_FD_ON_EXIT))
+        close(fd);
+    else
+        lseek(fd, 0, SEEK_SET);
+
+    if (success && (flags & METADATA_EXCLUDE_ID3_PATH) == 0)
+    {
+        strlcpy(id3->path, trackname, sizeof(id3->path));
+    }
+    /* have we successfully read the metadata from the file? */
+    return success;
+}
+
+bool get_metadata(struct mp3entry* id3, int fd, const char* trackname)
+{
+    return get_metadata_ex(id3, fd, trackname, 0);
 }
 
 #define MOVE_ENTRY(x) if (x) x += offset;
diff --git a/lib/rbcodec/metadata/metadata.h b/lib/rbcodec/metadata/metadata.h
index b30979334a..a0ba0376c6 100644
--- a/lib/rbcodec/metadata/metadata.h
+++ b/lib/rbcodec/metadata/metadata.h
@@ -24,6 +24,8 @@
 
 #include "platform.h"
 
+#define METADATA_EXCLUDE_ID3_PATH (0x01) /* don't copy filename path to the id3 path buffer */
+#define METADATA_CLOSE_FD_ON_EXIT (0x02) /* close the filedescriptor when finished */
 /* Audio file types. */
 /* NOTE: The values of the AFMT_* items are used for the %fc tag in the WPS
          - so new entries MUST be added to the end to maintain compatibility.
@@ -323,6 +325,7 @@ struct mp3entry {
 
 unsigned int probe_file_format(const char *filename);
 bool get_metadata(struct mp3entry* id3, int fd, const char* trackname);
+bool get_metadata_ex(struct mp3entry* id3, int fd, const char* trackname, int flags);
 bool mp3info(struct mp3entry *entry, const char *filename);
 void adjust_mp3entry(struct mp3entry *entry, void *dest, const void *orig);
 void copy_mp3entry(struct mp3entry *dest, const struct mp3entry *orig);