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);