From fdbaf7df597b404be04cecbdc83dbc0551a5b996 Mon Sep 17 00:00:00 2001 From: William Wilgus Date: Fri, 5 Jul 2024 17:13:10 -0400 Subject: [Feature] playlist_viewer id3 title display Not sure this is a great idea from disk and battery standpoint but there is no reason you can't.. using the name buffer to fill title data prevent hitting the disk for each screen scroll add get_metadata_ex to allow flags METADATA_EXCLUDE_ID3_PATH prevent copying the filename to the ID3 struct METADATA_CLOSE_FD_ON_EXIT instead of seeking to the beginning the file is closed before get_metadata returns add logic to allow a invalid fd to signal that get_metadata should open and close the file within its call bugfix per Chris_s don't use the tagcache for the trackinfo Change-Id: Ic7a595b39a8d7a57f975312bc9c8bb4111f22a88 --- apps/buffering.c | 8 ++-- apps/iap/iap-core.c | 7 +-- apps/iap/iap-lingo4.c | 7 +-- apps/playback.c | 3 +- apps/playlist_viewer.c | 122 +++++++++++++++++++++++++++++++++++-------------- apps/settings_list.c | 6 +-- apps/tagcache.c | 16 ++----- 7 files changed, 103 insertions(+), 66 deletions(-) (limited to 'apps') 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); -- cgit v1.2.3