function tagcache_fill_tags / performance

[mikeage]

I'm trying to modify the playlist viewer to show information about each song instead of the file name (quite important for an iPod that's also used with iTunes, since all the filenames are things like ER4M.mp3).

I addded the following code to format_name in playlist_viewer.c (to start, I just put it under the case 1 (full path) for testing purposes):

Code: [Select]

struct mp3entry temp_id3;
if (tagcache_fill_tags(&temp_id3, src)) {
    strcpy(dest,temp_id3.title);
}

(yes, it's wrapped in the proper ifdefs in the original, but I'm trying to avoid an extra long post here)

While it does work, it seems to take an extraordinary long time to show a playlist, almost as though it's hitting each file on disk. If I'm reading the tagcache.c file correctly, however, this shouldn't be happening. Tagcache should check for it in RAM, and then be reading from there.

Scrolling is also much slower (about a second or so between songs on a 4g).

Is this a reasonable way to do what I want? Is there another way to get id3 information given a filename?

[mikeage]

Sorry to reply to my own post, but I don't want to modify it, since it's been a while, in case people have already read the original.

I ran some comparison tests between the original and my patch... it appears that the slowdown is only when playing back a file, and it exists in the original (is this the priority change in the UI referred to several days ago?).

Nevertheless, loading a playlist of 100 songs still takes up to 5 seconds (with no background music), whereas without the patch it's almost instant. That still seems excessive.

[nls]

Do you have the "Load to ram setting" set? If you don't then it reads the database from disk, if you do then I don't know what's going on.

http://download.rockbox.org/manual/rockbox-ipod4gray/rockbox-buildch4.html#x7-380004.2

[mikeage]

I have load to RAM enabled.

It's actually not _so_ bad... I'll try and do the patch properly today (make sure I have the right ifdefs, add a real config option, etc), and put it on flyspray so others can take a look at it.

[mikeage]

Ok, I've put a patch that allows one to view the current playlist by title and "title (album)" in addition to file name and full path.

Two major notes:

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.

Patch link: http://www.rockbox.org/tracker/task/6084

[xlarge]

Disregard my post, i don't know what i'm doing right now.

[xlarge]

Can't get your patch to work...

~/rockbox/ $patch -p3 < playlist_viewer.patch

missing header for unified diff at line 4 of patch
... line 39 ...
... line 89 ...

And most changes get rejected... Why do i suck at this? =)

[xlarge]

Ah... I now know what went wrong.
Copying files to my x5 using my non root-account doesn't transfer files.. so i was booting the old rockbox firmware.

Running your patch;
It bugs out with wierd characters when showing title and tagcache is not in ram but when showing title+album and tagcache not in ram it shows the same as "full path" does...