Developed a new patch (ID3 tag data in playlist rather than filename) - now...

[chiark]

Hi Folks

I've been using Rockbox since Monday, when I got my first iPod - the OEM UI is not bad, however the flexibility of RockBox is brilliant!

I'm impressed that Rockbox is able to work with iTunes-loaded music, however the filename obfuscation is frustrating when viewing playlists.

Apologies if this is the wrong place to ask - I realise it's not compiling related, but...

I've written a patch to playlist_viewer.c, specifically the format_name routine, that does the following:

 - if the option to display full pathname is used, do the following check otherwise skip and just show the standard short filename

 - compare the start of the filename with "/ipod_control" - if not, just use the filename otherwise:

 - request information from the tagcache and use this as the playlist name.  If this fails
 - request information from the file's metadata, if this fails
 - just use the filename


When the tagcache is not in RAM or the code drops into inspecting the file itself it's quite slow so should check that the tagcache is available.

Next step would be to modify the playlist options to allow the user to control whether to use the tagcache (or ID3) for all files, just for ipodcontrol files, or for none, and the format of the ID3-derived string as artist, album, track number etc could all be used.

Is this something that I should develop further as I think it's incredibly useful for people who do use iTunes...

Cheers,
Nick.

[Llorean]

Wasn't there already a patch like this that was denied? Does yours offer a significant performance benefit over it?

[chiark]

I've searched through for flyspray for playlist and id3 and couldn't see anything...

Performance isn't an issue providing the tagcache is in RAM it seems - when hitting the disk, it is slow though.  I've tried to mitigate this by only applying the lookup to stuff in /ipod_control which is the only place it's absolutely needed.

On etiquette, is it OK to continue discussing this in this forum?  

[bluebrother]

There was a patch doing something similar by getting the information from the database. It has a major speed penalty though, and because of that it was rejected. Note that the default search on Flyspray doesn't look into tasks that are closed.

Also, restricting such a thing to /iPod_Control only doesn't make sense to me -- if such a feature gets in people will want to use it for e.g. other badly names files as well (and I bet user will want to be able customizing what information is displayed).

[chiark]

Thanks for the info.

As long as the data is in the tag cache, the speed penalty on the ipod 5g that I've got is not noticeable.  When it pulls the information from the disk, it's fairly painful.

As for ipodcontrol only, that was an idea to address the performance issue of reading from disk in a practical way - it's a good point though, this is something that is useful for all files providing there's no performance hit.

From debugging prints, it seemed that the format_name routine gets called twice for everything visible in the playlist: is there a reason for this, and could the second call be eliminated?  Eliminating the double call out to tagcache or metadata would help for sure.

This is something that I'm finding useful as I'm using iTunes to load files on.  If I stop doing that, I guess the issue goes away!

I'll develop it a little further to include a preferences section, and investigate the performance further.  

Is there any simple way I can check at runtime that the tagcache is loaded into RAM?


Prefs could be amended to:

Code: [Select]

                                                          
 From                              To
Track Display                 Track Display
 - Track Name Only             - File name only
 - Full Path                   - Full File Path
                               - Track name from ID3 (needs RAM tagcache)
                               - Track and Album from ID3 (needs RAM tagcache)

I'll see if I can find the other patch...

[bascule]

FS #6084

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

[chiark]

Thanks!

hmm, that looks horribly familiar!  Weird that I couldn't find that - I suppose searching for ID3 and tagcache was the problem?  It appears that the author has written the bits around config changes, too.

I've never had garbage returned from the system, but it's good to see it's not me going mad with respect to dual calls of the routine.  There's a simple way around that though...

It seems there's no issue with performance providing stuff is in RAM: if I can check for that and not call the tag cache then there's no peformance issue (but there's no functionality, either )

[bluebrother]

hmm, that looks horribly familiar!  Weird that I couldn't find that - I suppose searching for ID3 and tagcache was the problem?

There are quite some file types that don't use ID3 for metadata but something different   (vorbis comments just comes in my mind)

[polygonal]

Thanks!

hmm, that looks horribly familiar!  Weird that I couldn't find that - I suppose searching for ID3 and tagcache was the problem?  It appears that the author has written the bits around config changes, too.

I've never had garbage returned from the system, but it's good to see it's not me going mad with respect to dual calls of the routine.  There's a simple way around that though...

It seems there's no issue with performance providing stuff is in RAM: if I can check for that and not call the tag cache then there's no peformance issue (but there's no functionality, either )

I wonder about your patch. I wrote a similiar one for myself but the performance is horrible - seems to take forever for tagcache to retrieve info. What did you do exactly? (is your patch in FS?) The double call problem should be eliminatable as long as you don't ask for tag info in format_name, since the gui_list will possibly call that function more than once, especially there is a remote screen as well.

[chiark]

hi, I am calling tagcache_fill_tags in the format_name routine.  It runs like a dog if the RAM cache is not enabled...

It's not in flyspray - I'll finish off the setting stuff before I put it in there.

[kendubya]

I've been thinking this could be done another way.
What if the tag information (artist and title) were written to the playlist itself during creation, then read back when needed?
Perhaps as a comment after each line.  For example:

/MUSIC/Some Artist - Some Song.mp3
#Some Artist - Some Song
/MUSIC/A badly named track.mp3
#Some Other Artist - Some other Song
/MUSIC/Another Artist - Another Song.mp3
#Another Artist  - Another Song
/ipod_control/FE35/AB67.aac
#An Artist from iTunes - A Song from iTunes

Presumably, rockbox would have to handle the situation where it  has not created the playlist and therefore doesn't have these comments to rely on, obviously falling back to listing just the filename.

I'm not sure if this would be practical, but I thought I'd throw it out there.

[polygonal]

hi, I am calling tagcache_fill_tags in the format_name routine.  It runs like a dog if the RAM cache is not enabled...

It's not in flyspray - I'll finish off the setting stuff before I put it in there.

Then I don't see how your patch can be fast then (even for RAM cache), unless there are hardware differences between your DAP and mine. But I'll wait to see your patch (I assume it is quite close to the old patch in flyspray tha was closed?)

I've been thinking this could be done another way.
What if the tag information (artist and title) were written to the playlist itself during creation, then read back when needed?
Perhaps as a comment after each line.  For example:

/MUSIC/Some Artist - Some Song.mp3
#Some Artist - Some Song
/MUSIC/A badly named track.mp3
#Some Other Artist - Some other Song
/MUSIC/Another Artist - Another Song.mp3
#Another Artist  - Another Song
/ipod_control/FE35/AB67.aac
#An Artist from iTunes - A Song from iTunes

Presumably, rockbox would have to handle the situation where it  has not created the playlist and therefore doesn't have these comments to rely on, obviously falling back to listing just the filename.

I'm not sure if this would be practical, but I thought I'd throw it out there.

There is already a playlist format (that is, not a new format but basic M3U plus comments) called EXTM3U that does what you are describing. There are widely available tools to convert normal M3U playlist to EXTM3U already (on the PC side), and a patch that implements this feature in rockbox is already is in flyspray.

As far as I understand, the current issue is with time but not implementation problem, since whatever method you go for to get title instead of filename, you will have to get the information somehow (either through database/tagcache or through reading the files, and both of the methods are unacceptably slow), unless you have an already-made playlist (then EXTM3U support should be competent and quick enough to solve the problem).

[chiark]

Just FYI, my hardware is a 5.5g 30GB ipod...  That could explain the performance difference, maybe?

[KindOfBlues71]

Any progress updates on this patch or is it dead as well?  I personally would rather create a playlist within Rockbox using my iTunes-loaded music files and be able to see the track names in that playlist.  I'd rather not deal with the hassle of connecting the iPod to my computer to convert a playlist to another format.  Unless, of course, there is no other way to do this, especially if there is a major performance issue.

There are quite some file types that don't use ID3 for metadata but something different  (vorbis comments just comes in my mind)

Yet that doesn't make any difference for this patch (or the previous one) because they're iPod-specific and the default iPod firmware only supports WAV/MP3/AAC audio files.  Songs added to an iPod via iTunes are renamed something like TB6F.mp3 and located in a folder named F01, F02, etc.  These patches would allow those using iTunes the ability to see tracknames based on ID3 tags in Playlists rather than Apple's strange naming system.

-KindOfBlues71