Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Playlists
  • Assigned To No-one
  • Operating System All players
  • Severity Medium
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Mike Miller - 2006-09-28
Last edited by Jonathan Gordon - 2007-08-05

FS#6084 - Show playlist entries' Titles (instead of file names)

This patch allows one to view the current playlist by title and “title (album)” in addition to file name and full path.

Three major notes, and one minor note:

1. It’s slow to load – don’t know why
2. If dircache and tagcache’s load to ram are not enabled, it returns garbage. My quick perusal of the tagcache code suggests that this should not happen, but it does. I don’t know why.
3. It increases the config file version, so you may need to clear settings (since the display format is now 2 bits instead of 1).
4. The strings only exist in the english language file. I didn’t test any other languages (I’m too afraid of getting stuck in a weird menu somewhere <g>)

If anyone can help fix these, I’d like to know how.

Closed by  Jonathan Gordon
2007-08-05 08:02
Reason for closing:  Out of Date
Additional comments about closing:  

and probably would be rejected anyway if it makes the viewer slow

2008-02-19 : A request to re-open the task has been made. Reason for request: I have spent some time with this patch as well as FS 7652 which would be another way of getting tag data into the playlist view display. I altered it to add a third view option to the playlist view so you would have fliename only (default), full path, and tag data. Tag data extracted is display as "artist - filename" . I tested performance of the list view with the patch, without it, and with FS 7652. scrolling a very large list (2450 entries) I found no lag difference between no patch at all, and with the tag data retrieve. FS7652 performed worse, presumably because it doubles the size of the list that must be read into memory. It has some other issues as well.
Miika Pekkarinen commented on 2006-09-28 17:02

1. It seems to be a problem with list widget code to use the callback function to retrieve file names more often than necessary.

2. Currently tagcache_fill_tags(…) function is intended to work only when tagcache is loaded to ram because it would be a slow thing to do otherwise. But I can fix the tagcache code so that it will always work, but it will be very slow for big playlists.

3. That should be ok.

4. Language seems to be handled correctly also.

5. You should fix the patch so that entries point to the same directory level.

Not this way:
— ../../rockbox-daily-20060927.ref/apps/lang/english.lang 2006-09-27 07:00:45.000000000 +0300
+++ ./lang/english.lang 2006-09-28 14:11:06.000000000 +0300

But that way:
— apps/lang/english.lang 2006-09-27 07:00:45.000000000 +0300
+++ apps/lang/english.lang 2006-09-28 14:11:06.000000000 +0300

You can just use the command: cvs diff -u

Mike Miller commented on 2006-09-28 17:12

Miika,

Thanks for the response.

1. Is there any easy way to fix this?

2. Is there a way I can detect if TagCache in RAM / dircache are enabled? Alternatively, is there a better API for me to use?

3. Was just an FYI

4. Yep.. I noticed that it defaults to English

5. I did that because I run the patch from within the rockbox directory. I’ll modify it, though, if that’s the standard for rockbox patches.

5. You should fix the patch so that entries point to the same directory level.

Magnus Holmgren commented on 2006-09-28 17:32

The GUI list code calls the get item name callback each time it wants to redraw an item in the list. And if you’re using a device that supports LCD remotes, some lines will be drawn twice. One solution could be to use parts of the name buffer to cache (!) the tagcache results. That would also improve things when the tagcache isn’t loaded in RAM.

Mike Miller commented on 2006-09-28 18:16

I don’t (yet) know enough about the available buffers to implement this. If someone wants to, I won’t object <g>

Oh, and in my previous comment, I accidently pasted Miika’s #5 below my #5 response. Please ignore that :)

Miika Pekkarinen commented on 2006-09-29 07:43

tagcache_fill_tags(…) should return false if it was unable to retrieve tag data. If you would like to try other api functions that can be used whether tagcache is loaded in ram or not, you might want to try something like this:

static struct tagcache_search tcs;

if (!tagcache_find_index(&tcs, filename))

  return error;

idxid = tcs.idx_id;
tagcache_search_finish(&tcs);

if (!tagcache_search(&tcs, tag_album))

  error..

if (tagcache_retrieve(&tcs, idxid, buffer, buffer_length))
{

 /* buffer = the search results */

}

tagcache_search_finish(&tcs);

Mike Miller commented on 2006-09-29 10:49

I’m not convinced that it’s wise to even try if it’s not already TagCached in RAM. I assume the load times will be really bad if it has to get it from disk.

That said, in the patch I made, do you have any thoughts on why TagCache occasionally returns garbage (or, at least, the string is populated with garbage)? All songs have the same data… usually a few bytes (1-3) of non printable characters, and it’s the same for all files. It tends to happen when I play around with the tagcache settings, although I also saw it once when I changed the language (not sure how that could’ve happened).

Flid commented on 2006-11-10 09:32

Hi. I’ve only just stumbled across this patch. Just thought I’d say that I’d love for this to be implemented, but possibly with a configurable option of “Artist - Title”, as I like genre based playlists.

Has this completely ground to a halt?

Also… just for reference, I don’t see why loading tagcache from disk would be obstructively slow. Surely it’s just got to read the file, not rebuild the database?

Mike Miller commented on 2006-11-10 10:41

I can gladly add that option for you, if you’d like (although probably not until Sunday, as my iPod is on loan to a friend for the weekend).

I haven’t pursued this lately, since I have no idea why I often get garbage in the tagcache struct (from tagcache_fill_tags), although it returns true. Maybe Miika has an idea why…

Haseeb commented on 2007-01-25 17:51

Is work being done on this? haven’t seen any activity lately :(

Mike Miller commented on 2007-01-25 19:37

I haven’t touched it… I stopped using Database (nee tagcache) for a while.

It had a problem which I don’t quite understand: tagcache_fill_tags returns an ok status, but it fills the fields with garbage (every track gets the same gibberish).

Mike Miller commented on 2007-01-28 13:52

Ok, I just dusted off this patch.

It works nicely now (new patch attached that replaces the “full path” view with the tagcache derived view).

It’s still slow, and search doesn’t work. I have to think about where to go from here.

Flid commented on 2007-01-29 18:42

“… where to go from here” Maybe, this could be merged with the actual Database view root “menu”, allowing you to view “Current Playlist” based on tag info, and the configuration of this view could be user adjustable (to allow for Artist - Title / Tile / Title(Album) / Filename etc).

Thanks for keeping an eye on this one Mike.

Dieter commented on 2007-02-11 23:46

Does not work for me. Although the patch could be applied without errors there are still the filenames displayed for the current playlist. I can switch between display with file names only or full path and the filenames only or the full path are shown but not the tags. Any idea why?

Mike Miller commented on 2007-02-12 02:03

Did you:
* enable TagCache
* configure it to load to RAM
* wait for it to finish loading

Dieter commented on 2007-02-12 22:23

I did all this but still see only the complete path.
By the way, I use the simulator with the latest svn since I have an iPod 80GB 5.5g and am waiting for a running rockbox version. In the latest svn version I don’t any longer find the option to use full path in the playlist context menu, but only in the general settings menu. Is that correct?

Dieter commented on 2007-02-12 22:26

Sorry, my last sentence was not right, I found the option to use full path, but nevertheless it does not work.

Matthew Schneider commented on 2007-06-09 20:46

Does this patch just not work, or is it out of sync?

Mike Miller commented on 2007-06-09 20:54

out of sync

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing