Rockbox

Tasklist

FS#9613 - Allow formatting of track names for the default <All Tracks> database menus

Attached to Project: Rockbox
Opened by Justin Gan (jgrg1) - Monday, 08 December 2008, 12:50 GMT
Last edited by sideral (sideral) - Monday, 01 August 2011, 15:31 GMT
Task Type Patches
Category Database
Status New
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.0
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

By default, whenever you choose the <All Tracks> option from the database menu, the list of results returned defaults to the standard track name format, i.e. "%02d. %s" tracknum title. What I wanted was to be able to define the format that gets applied to the <All Tracks> option, dependent on the current menu level. So for example, if I had browsed to genre->artist-><All Tracks> it would return the list of all tracks for all artists for the given genre and I wanted the track names to be formated with:

"%s %s %02d. %s" artist album tracknum title

and then sorted into order, since I already know what genre the tracks are for.

Similarly, if I simply browsed to genre->artist->album-><All Tracks>, it would return the list of all tracks for the selected genre and artist, but I want the track names to be formated with:

"%s %02d. %s" album tracknum title

so when it's sorted, I get the tracks grouped only by album and tracknumber, since I already know what artist the tracks are for.

This patch attempts to address this issue by allowing users to define custom <All Tracks> formats in the tagnavi_custom.config file. Thus, sort order can be controlled by how you format the track names, just as it works in base Rockbox.

The premise behind the code is that if the <All Tracks> menu option is chosen, Rockbox will look for formats with a certain naming convention defined by concatenating the tag names of the parent menu levels with "." characters and finally appending an ".All" identifier. So in the first example above where we had browsed to genre->artist-><All Tracks>, Rockbox will look for all formats called:

"genre.artist.All"

and then cycle through them to see if the appropriate clauses defined on the format are met. Similarly, for the second example where we had browsed to genre->artist->album-><All Tracks>, Rockbox will look for the format called:

"genre.artist.album.All"

As in base Rockbox, it will cycle through all applicable formats and apply the first format it comes across that meets all the clauses defined on the format. One minor change I had to make to get the relevant tags loaded for the search results was to add the tags defined in the clauses of all matching format names to the tagcache search, but not add the clauses themselves.

So now, in the tagnavi_custom.config file to illustrate the first example, I have defined 4 formats called "genre.artist.All" to format the final display name based on whether the artist/album tags are actually defined on the mp3's in the database.

%format "genre.artist.All" "%s - %s - %02d. %s" artist album tracknum title ? artist !~ "<Untagged>" album !~ "<Untagged>" tracknum > "0"
%format "genre.artist.All" "%s - %02d. %s" album tracknum title ? album !~ "<Untagged>" tracknum > "0"
%format "genre.artist.All" "%s - %02d. %s" artist tracknum title ? artist !~ "<Untagged>" tracknum > "0"
%format "genre.artist.All" "%02d. %s" tracknum title ? tracknum > "0"

The downside of this approach is that in order to have this style of functionality for every <All Tracks> menu option, I defined a lot of formats. As such, I had to increase the maximum number of formats from 32 to 96 in order to cope with my formatting schemes. I also increased the maximum number of tag filters from 4 to 8. Now this obviously increases the amount of memory allocated for formats, which will have an impact on available audio buffer size. The attached tagnavi_custom.config file shows the various "All" formats I've added and it's pretty comprehensive in the way it handles missing tags, so technically, if your music library is properly tagged, you should never need all of the formats so you could easily reduce the maximum number of formats.

My principal concern with this patch is in terms of the memory usage of increasing the maximum number of formats and number of tag filters allowed. I'm not aware of a memory profiler for cygwin, but then I hadn't used cygwin before playing with Rockbox - so any pointers would be helpful.

The second piece of functionality I wanted was in the handling of the "strip" functionality. Base Rockbox has an all or nothing approach; if the global format found for the list of tracks contains a strip modifier, then it applies the strip to ALL track names, even if the subsequent formatting of the track does not contain a strip modifier. I wanted Rockbox to only strip tracknames if a strip modifier was defined on the format that is applied to the track. This patch fixes this problem.


This works fine on my Sansa e280 with a full 8Gb drive and a full 2Gb micro-SD card. It's formatted close to 1000 tracks with no noticeable difference in processing time, and I've not come across any out of memory errors on my e280, but I haven't tested it on any other mp3 player so I would recommend you test this first using the appropriate simulator.

Note that this patch incorporates the year album patch  FS#8051  as the code is all tied up in the same files.
This task depends upon

Comment by Paul Louden (Llorean) - Monday, 08 December 2008, 21:59 GMT
Please do not post patches that are dependent on other patches if they don't need to be. As it stands, if someone's interested in your patch but not the year album patch, they'll have to strip its code out if they want to commit yours.

Also, you mention it does a lot of work to deal with missing tags: I think we should assume those using the database either have proper tags or are willing to fix their tags, and attempt the minimum memory usage.

You can see how much memory is used by Rockbox I believe in the rockbox-info.txt you get after compiling.
Comment by Paul Louden (Llorean) - Monday, 08 December 2008, 22:02 GMT
Also, you report this as against version 3.0? Your patch file at least seems to suggest that you made the diff against a much more recent revision of Rockbox. Is this correct?
Comment by Justin Gan (jgrg1) - Tuesday, 09 December 2008, 13:04 GMT
Duly noted.

Find attached a patch file that does not contain the yearalbum patch ( FS#8051 ), and also a maximum of 48 filters, 6 filters per format and 6 tags per format. The associated tagnavi_custom.config file has been fixed, but it does assume that your music collection is properly tagged and at the very least each track has an artist defined.

With regards to RAM usage (Rockbox version: r19369M-081209):

1) Base Rockbox r19369M-081209 (max 32 formats, 5 tags per format, 4 filters per format) : 1617600
2) Original AllTracks_Database_051222008.patch (max 96 formats, 8 filters per format, 10 tags per format) : 1619536
3) AllTracks_Database_09122008_NoYearAlbum.patch (max 48 formats, 6 tags per format, 6 filters per format) : 1618448

The latest patch was built against r19369M-081209 as taken from SVN on 9 December 2008. The original patch was built against the version taken from 5 December 2008.
Comment by Dieter (dip) - Tuesday, 27 January 2009, 16:12 GMT
Great patch! I always looked for a way to have my own sorting for the "All tracks" item. With this patch it works perfectly.
Thanks.
Comment by Dieter (dip) - Thursday, 18 June 2009, 22:54 GMT
out of sync!
Justin, can you please have a look at the patch.
Comment by Justin Gan (jgrg1) - Wednesday, 29 July 2009, 20:24 GMT
Updated for release r22076M taken from SVN on 28Jul2009.

You can define a maximum of 48 formats in your tagnavi_custom file (any more and Rockbox will crash). Each item can have at most 6 filters and 6 tags.

I did find that if there were too many files as a result of a database selection (e.g. Genre), then the code will return a number of files but would NOT sort them, so you get a jumbled mix of results. To fix this, increase the "Max Entries in File Browser" setting on your device by going to Settings/General Settings/System/Limits/MaxEntriesinFileBrowser. By default it's set to 1000. I have a 16Gb microSD card in my Sansa E280 with a total of about 4,500 mp3s. I cranked the max file entries up to 5000 and it works fine for me. A little experimentation is in order to find the optimum level as I wouldn't recommend just blindly setting the max entries to an arbitrarily large number.

Please note, I have only tested this with the Sansa E280 v1 simulator and mp3 player. I cannot vouch for any other players, so I strongly recommend you try it out using the appropriate simulator first.
Comment by Dieter (dip) - Wednesday, 11 November 2009, 23:08 GMT
Out of sync
Justin can you please sync that patch. Thanks a lot.
Comment by Justin Gan (jgrg1) - Tuesday, 12 January 2010, 22:49 GMT
Patched for release r24214M-100112 taken from SVN on 11 January 2010 as requested.

Same restrictions and disclaimer as before - check it thoroughly using the appropriate simulator before putting it on your device.
Comment by Justin Gan (jgrg1) - Wednesday, 13 January 2010, 00:59 GMT
Ooops. Don't use the 12012010.patch, it has a bug in that crashes my sansa e280. Use the 13012010.patch instead - for revision r24216M.
Comment by borre (borre) - Monday, 28 June 2010, 22:22 GMT
I Try to use your last patch, but get this error:

borre:/rockbox# patch --binary -p0 < AllTracks_Database_13012010.patch
patching file apps/tagtree.h
patching file apps/tagcache.c
Hunk #1 succeeded at 951 (offset 23 lines).
Hunk #2 succeeded at 982 (offset 23 lines).
Hunk #3 succeeded at 1294 (offset 23 lines).
patching file apps/tagcache.h
Hunk #3 succeeded at 215 (offset 3 lines).
patching file apps/tagtree.c
Hunk #5 FAILED at 1096.
Hunk #6 succeeded at 1277 (offset 6 lines).
Hunk #7 succeeded at 1346 (offset 6 lines).
1 out of 7 hunks FAILED -- saving rejects to file apps/tagtree.c.rej
borre:/rockbox#

dont know how to fix this.
I placed the apps/tagtree.c.rej as attachment.
Comment by Justin Gan (jgrg1) - Tuesday, 03 August 2010, 16:17 GMT
Ok, fixed for version r27668M as taken on 02 Aug 2010. Enjoy.
Comment by sideral (sideral) - Monday, 01 August 2011, 15:31 GMT
  • Field changed: Status (Unconfirmed → New)
I like the idea of this patch, and would like to shepherd it into the trunk. This change would allow us to close the last gap where track titles can still appear as “<Untagged>” due to formats not being applied. (I've closed the other holes in  FS#12132 .)

However, I find the need to define formats for all instances of <All tracks> a bit excessive, as it is also quite resource intensive (as you've noted), but I do like that it is possible. Wouldn't it be possible to fall back to more generically defined formats if a specific one is not defined? For example, in Genre → Artist → Album → <All Tracks>, we could first look for "genre.artist.album.All", then "genre.artist.All", and so on, all the way up to a format named "All". This would allow us to cut down on the maximum number of formats.

The patch also seems to address one of my pet peeves, which is that %strip is taken only from the first format of each group and then applied to all formats of that group. This patch seems to make %strip per format. Is that right?

We need your real name if this is to be committed. Is it Justin Gan?
Comment by Justin Gan (jgrg1) - Monday, 01 August 2011, 21:10 GMT
Glad to hear it! I still get <Untagged> entries in the lists if there are no tags on the track, however, I haven't synchronised with the source code since Aug 2010.

You're right of course, defining all instances is excessive. Looking at the code again, it would be possible to roll look up the hierarchy and find the first matching format. If there was a more efficient way of describing the format and tag names, with the format of a specific element being optional if the tag doesn't exist, that would also cut down on the format specification. E.g. if you had something like:

%format "fulltitle" "[artist:%s - ][album%s - ][tracknum:%02d. ][title:%s]"

so only if the artist tag is populated, does it apply the format defined after the ':' to the tag, and so on. The majority of the format entries in the tagnavi_custom.config in the patch are to handle the variations of missing/present track tags. I suppose you could also embed the logic within the "[tagname:format]" entries, e.g.

%format "fulltitle" "[artist(!~"<Untagged>"):%s - ][album%s - ][tracknum:%02d. ][title:%s]"

so if artist is assigned and not <Untagged>, then it applies the formatting after the ':'. Just an idea, and not something I think I could do.

You are correct, the %strip is on a per format basis. I only ever used it so I could sort by year, and then hide the year from the display. With the above scheme, you could also define the %strip at the tag level within each "[tagname:format]" if you wanted - probably not necessary though.

Ok, so what's the process? Do you take ownership of this, or should I implement your suggestions and you review it? It's been a while since I've looked at this.

And yes, that is my real name.
Comment by sideral (sideral) - Tuesday, 02 August 2011, 11:52 GMT
> I still get <Untagged> entries in the lists if there are no tags on
> the track, however, I haven't synchronised with the source code since
> Aug 2010.

I've committed this just a couple of days ago.

> You're right of course, defining all instances is excessive. Looking
> at the code again, it would be possible to roll look up the hierarchy
> and find the first matching format.

After some more pondering, I think we should go an even simpler route for the first version of this: Let's just use one special format group "All" for all instances of <All tracks>. Once that's in, we can consider extending it one way or another.

> If there was a more efficient way of describing the format and tag
> names, with the format of a specific element being optional if the tag
> doesn't exist, that would also cut down on the format
> specification. E.g. if you had something like:
>
> %format "fulltitle" "[artist:%s - ][album%s - ][tracknum:%02d. ][title:%s]"

That sounds like a nice idea, but would be even more ambitious. Let's postpone this discussion until the basic support is in.

> You are correct, the %strip is on a per format basis.

You even mention this in your original message, which I apparently didn't complete reading before looking the actual patch. :-)

Could you split the %strip fix out into a separate patch? This could be committed pretty quickly.

Also from the original message:
> I'm not aware of a memory profiler for cygwin, but then I hadn't used
> cygwin before playing with Rockbox - so any pointers would be helpful.

There's a tool in the source tree that can compare two versions of an object file: utils/analysis/bloat-o-meter.py. It reports size differences split by functions and statically allocated objects. I use a small shell snippet to compute a summary for the entire source tree; see the attached Makefile snippet for inspiration.

> Ok, so what's the process? Do you take ownership of this, or should I
> implement your suggestions and you review it?

I don't think there's a well-defined process. Ideally, you'd do the implementation work until the patch is fit for inclusion. But as I'm interested in this feature as well, I may pick this up as well; but I won't make any promises as to when this will happen, if at all.
Comment by Justin Gan (jgrg1) - Saturday, 17 August 2013, 15:32 GMT
Updated patch file for V3.13 as checked out on 17/08/2013.

Loading...