Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#8051 - "year album" tag for tag cache

Attached to Project: Rockbox
Opened by Robert Kukla (roolku) - Tuesday, 30 October 2007, 15:40 GMT+2
Last edited by Paul Louden (Llorean) - Sunday, 15 March 2009, 04:50 GMT+2
Task Type Patches
Category Database
Status Closed
Assigned To Robert Kukla (roolku)
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This patch addresses the often heard desire to display albums as "year album" in the Database (see for example: http://forums.rockbox.org/index.php?topic=13191.0)

It implements option 3b) from the discussion here:

http://forums.rockbox.org/index.php?topic=13191.msg100433#msg100433

The new tag is called yearalbum and can be used as a filter in the database. It can not be used in the WPS and use in a database conditional clause is discouraged (but possible).

I have edited the tagnavi.config to use the new tag where appropriate.

Warning: a database rebuild is required, so export your runtime data.
   yearalbum.patch (12.1 KiB) 
 apps/tagcache.c     |   21 ++++++++++--
 apps/tagcache.h     |    9 +++--
 apps/tagnavi.config |   86 ++++++++++++++++++++++++++--------------------------
 apps/tagtree.c      |    1 
 4 files changed, 66 insertions(+), 51 deletions(-)

This task depends upon

Closed by  Paul Louden (Llorean)
Sunday, 15 March 2009, 04:50 GMT+2
Reason for closing:  Rejected
Additional comments about closing:  Needs to be reimplemented in a better way to be considered.
Comment by Mark Fawcus (yapper) - Friday, 02 November 2007, 00:55 GMT+2
This works well for me - a very nice feature.
Comment by Travis Tooke (tdtooke) - Sunday, 04 November 2007, 23:45 GMT+2
Sync, sorta.. I just manually edited the patch for that slight change in tagnavi.config and got rid of those pesky trailing CRs.
   yearalbum-20071104.patch (12 KiB) 
 apps/tagcache.c     |   21 ++++++++++--
 apps/tagcache.h     |    9 +++--
 apps/tagnavi.config |   86 ++++++++++++++++++++++++++--------------------------
 apps/tagtree.c      |    1 
 4 files changed, 66 insertions(+), 51 deletions(-)

Comment by Shiloh Hawley (gree665) - Monday, 05 November 2007, 06:24 GMT+2
Thanks for the cant-do-without patch, I love it!
Comment by Travis Tooke (tdtooke) - Tuesday, 08 January 2008, 21:17 GMT+2
I went ahead and synced this, hope you don't mind me butting in. I love this patch as do many of the people that use my build.
   yearalbum-20080108.patch (12.1 KiB) 
 apps/tagcache.c     |   20 +++++++++---
 apps/tagcache.h     |    8 +---
 apps/tagnavi.config |   86 ++++++++++++++++++++++++++--------------------------
 apps/tagtree.c      |    1 
 4 files changed, 63 insertions(+), 52 deletions(-)

Comment by Shiloh Hawley (gree665) - Friday, 23 January 2009, 17:23 GMT+2
A few minor changes caused this not to build, here is a synced version
   yearalbum-20090122.patch (11.7 KiB) 
 apps/tagcache.c     |   20 +++++++++---
 apps/tagcache.h     |    4 +-
 apps/tagnavi.config |   86 ++++++++++++++++++++++++++--------------------------
 apps/tagtree.c      |    1 
 4 files changed, 62 insertions(+), 49 deletions(-)

Comment by Sadurní (sadur) - Saturday, 21 February 2009, 17:03 GMT+2
I'm using this patch for a while and it works perfectly, with my personal tagnavi_custom.config, and it works very nice.
Why it can't be submitted to svn? It's a shame to have to rebuild the database if I want to try some other patch cleanly or if I must switch to a daily build.
Is there some bug or something that prevents it to be submitted?
Anyway, thanks for this patch.
Comment by Christian Engels (Russel-Athletic) - Thursday, 12 March 2009, 14:19 GMT+2
Just another update for the latest head.
   yearalbum-20090312.patch (11.7 KiB) 
 apps/tagcache.c     |   20 +++++++++---
 apps/tagcache.h     |    4 +-
 apps/tagnavi.config |   86 ++++++++++++++++++++++++++--------------------------
 apps/tagtree.c      |    1 
 4 files changed, 62 insertions(+), 49 deletions(-)

Comment by Paul Louden (Llorean) - Thursday, 12 March 2009, 14:46 GMT+2
I don't think this patch is a good idea for SVN inclusion. It increases database size in RAM for people who don't use it, and at most saves you a single keypress during navigation over simply having a Year filter before having an Album filter. I don't think its RAM cost is worth it for a single keypress, especially since the sort tags patch could allow you to have your albums in year order anyway.
Comment by Thomas Martitz (kugel.) - Thursday, 12 March 2009, 16:41 GMT+2
This should be done solely using the tagnavi imo.
Currently, it seems that there's a limitation so that only the track views can be formatted. This limitation should be resolved, so that a fomatted album name like
"%format "fmt_album" "(%4d) %s" year album" works too. This would be a nice implementation of year album, which also adds the possiblity to combine everything with everything.
Comment by Robert Kukla (roolku) - Thursday, 12 March 2009, 18:03 GMT+2
This can't be done with the current implementation of the database as it would require the equivalent of an Sql group-by clause. There is some discussion in the forum post (linked to in the FS entry) about it.
Comment by Paul Louden (Llorean) - Thursday, 12 March 2009, 19:29 GMT+2
The problem with this patch is it passes the cost of adding this feature entirely on to users who don't use it by inflating the database size. Some other solution needs to be found, because otherwise its cost is a good deal larger than what it offers, in a practical sense. People can already organize their filetree this way, and their database can *nearly* be accessed this way (by filtering year, then album, meaning this only really simplifies the display). It's just really not a good "cost vs gain" at all.
Comment by Robert Kukla (roolku) - Thursday, 12 March 2009, 20:09 GMT+2
I am not arguing for svn inclusion (I don't use it myself and only did it as a favour to someone at the forums). I am only pointing out that the idea to do it via the tagnavi, as appealing as it might seem to the user, requires some redesigning of how the database works internally and is not just a matter of adding a format string.
Comment by Paul Louden (Llorean) - Thursday, 12 March 2009, 20:16 GMT+2
Ah, well if this patch isn't a candidate for SVN inclusion, it should be closed as per tracker policy.
Comment by Thomas Martitz (kugel.) - Thursday, 12 March 2009, 23:04 GMT+2
Hence I said the limitation should be resolved.
Comment by Sadurní (sadur) - Saturday, 14 March 2009, 15:04 GMT+2
Thanks Paul for your clarification about svn inclusion for this patch.
Comment by Shiloh Hawley (gree665) - Saturday, 14 March 2009, 18:28 GMT+2
I wonder if another tracker could be set up for patches like this one that people use, but are not candidates for SVN (the "ignore the" patch also comes to mind).
Comment by Paul Louden (Llorean) - Sunday, 15 March 2009, 04:49 GMT+2
You don't need to post the same message more than once.

If you want to start your own unofficial tracker, nobody's stopping you.

Loading...