Bug 469 - sort order tags being ignored again
: sort order tags being ignored again
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Tagging
: 5.x or older
: PC All
: P2 normal (vote)
: ---
Assigned To: Dan Sully
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-08-02 17:31 UTC by michael
Modified: 2008-12-18 11:54 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
hack to illustrate sort order problem (1.30 KB, patch)
2004-08-05 16:46 UTC, michael
Details | Diff
alternate patch (2.96 KB, patch)
2005-01-04 21:32 UTC, Greg Klanderman
Details | Diff
MP3 with TSOP (5.13 MB, audio/mpeg)
2005-01-06 07:25 UTC, Greg Klanderman
Details
id3lib patch for sort tag support (9.11 KB, patch)
2005-01-06 07:30 UTC, Greg Klanderman
Details | Diff
fixed sort order screenshot (11.71 KB, image/png)
2005-01-19 00:17 UTC, Dan Sully
Details

Note You need to log in before you can comment on or make changes to this bug.
Description michael 2004-08-02 17:31:22 UTC
Recent nightly builds have not been heeding sort order tags (TSOP, TSOA, etc.)
This change seems to have crept into cvs around the beginning of july. A cvs
checkout from 2004-06-30 will adhere to the sort order tags, while one from
2004-07-02 will not. 

This seems to also have been where much text processing moved from
Slim::Music::Info to Slim::Utils::Text but I haven't found anything obvious in
that move that would've broken sort tags.
Comment 1 michael 2004-08-02 19:10:40 UTC
ok, some additional info...

I think I've narrowed down where this is coming from, but I still haven't
figured out exactly why. It seems to be some sort of scope issue. However, if
you do a cvs checkout of 2004-07-02 (which doesn't properly heed sort order
tags, as mentioned in previous comment) you can restore the old (correct)
behavior by doing the following...

cut and paste the function sortuniq_ignore_articles() from Slim::Utils::Text.pm
back into Slim::Music::Info.pm

within that function, change the call to ignoreCaseArticles() to
Slim::Utils::Text::ignoreCaseArticles() 
(or cut n paste that function into Info.pm as well. either way)

at this point nothing has changed, except that we can call
sortuniq_ignore_articles() from Text.pm or from Info.pm at our choosing.
now in Info.pm find the artists() function, and near the bottom of the function
you'll see the line
return fixCase(Slim::Utils::Text::sortuniq_ignore_articles(@artists));
change this to 
return fixCase(sortuniq_ignore_articles(@artists));

voila! slimserver now uses the artist sort order tags (i.e. TSOP or equivelent.)
revert that last line we changed back to calling into Text.pm and once again
slimserver ignores these tags.

I haven't tested the album and track versions of this, but I imagine we'll see
similar behaviour there.

I'm not sure what the proper fix for this is (I'm sure we don't want to migrate
stuff from Text.pm back into Info.pm outside of this sort of debugging
experiment). Any suggestions?
Comment 2 michael 2004-08-02 20:21:13 UTC
Just for the record, the above hack works on recent (5.3.0) versions as well.
However, it also severely breaks the alphabet bar/pager.
  
Comment 3 KDF 2004-08-03 22:08:28 UTC
can you add a diff as an attachement for this?  I'm sure between what is there
now, and the diff we can sort out the pagebar/sorting problems.
Comment 4 michael 2004-08-05 16:46:20 UTC
Created attachment 91 [details]
hack to illustrate sort order problem

ok, here's a diff that basically implements the workaround/hack I outlined
above. of course this isn't a fix, but just a tool to help locate the problem.
also, this diff only affects artist sorting, so a real fix would need to take
album and track into account as well.
Comment 5 Blackketter Dean 2004-08-12 09:57:28 UTC
I believe that this has been addressed in the latest nightly.  Please verify.
Comment 6 michael 2004-08-12 12:39:49 UTC
Just checked latest cvs (which I assume has everything that would be in the
latest nightly) and did a wipe cache. 
Nope, still broken. 
Comment 7 Ben Sandee 2004-09-23 14:05:43 UTC
I see this too with SlimServer 5.3.0 -- I noticed it immediately after I
upgraded from 5.2.1.
Comment 8 michael 2005-01-03 02:45:49 UTC
just for the record, the current cvs trunk (i.e. the 6.0 branch) is also broken
in regard to sorting, although with a new behaviour of it's own.

In the 5.4.x branch, this is broken in the same way as mentioned above for
5.3.0. Sort order tags are ignored, and items are sorted by the main artist (or
album or title) tag. 

In the trunk/6,0 branch, items with no sort order tags preceed items that do
contain sort order tags. So, for example when browsing by artist, All artist's
beginning with the letter "A" who don't have a sort order tag will appear first,
followed by artists who's sort order tag begins with "A", followed by artists
that begin with "B" and have no sort tag, etc.
Comment 9 Blackketter Dean 2005-01-03 17:02:26 UTC
Dan: Can you take a look at this in the 6.0 branch?
Comment 10 Greg Klanderman 2005-01-04 21:32:57 UTC
Created attachment 239 [details]
alternate patch

I tried Michael's patch against 5.4.1 and it works.  The problem seems to be
related to accessing the "my %sortCache" variable defined in Slim/Music/Info.pm

from the function sortuniq_ignore_articles in Slim/Utils/Text.pm.  Here's an
alternate patch that also works; all it does is define that variable
differently.
Comment 11 Greg Klanderman 2005-01-05 15:40:16 UTC
if this is targeted for 6.0.0, what does that mean in terms of expected release
date?
Comment 12 Dan Sully 2005-01-05 17:05:20 UTC
Could someone send me (or attach) a few files with these ordering tags?

Thanks.
Comment 13 Greg Klanderman 2005-01-06 07:25:34 UTC
Created attachment 243 [details]
MP3 with TSOP

Dan, here's an MP3 with an artist sort tag:
TPE1="Bob Dylan"
TSOP="Dylan, Bob"
Comment 14 Greg Klanderman 2005-01-06 07:26:58 UTC
if you want any more, I'll re-rip a few songs at lower bitrate tonight when I
have my CDs handy.
Comment 15 Greg Klanderman 2005-01-06 07:30:45 UTC
Created attachment 244 [details]
id3lib patch for sort tag support

Here's a patch for id3lib-3.8.3 that adds support for printing and setting the
sort order tags with the demo command line utilities "id3info" and "id3tag"
that come with id3lib.
Comment 16 Dan Sully 2005-01-19 00:17:32 UTC
Created attachment 256 [details]
fixed sort order screenshot

Ok - This should be fixed in the 6.0 tree in change #1768

I've attached a screenshot of the results.
Comment 17 michael 2005-01-21 00:11:18 UTC
Well, the good news is that this seems to be sorting as expected again with
recent 6.0/trunk updates. The bad news, is the "alphabet bar" pager is borked as
a result. It seems the pager is still being generated from the display name and
not the sort name. For example, if you have a sorting like:

Alien Sex Fiend
All About Eve
Marc Almond
Amber Asylum
Tori Amos
And One
Laurie Anderson
Apocalyptica
Bauhaus

You'll see a pager bar that reads "A M A T A L A B"  instead of "A B"

(I can post a screen shot if that's not a clear description.)
Comment 18 Dan Sully 2005-01-23 15:54:48 UTC
Ok - this should be fixed in svn change #1823
Comment 19 michael 2005-01-24 17:30:57 UTC
yep, seems to be finally working right again with recent 6.0 checkouts.

Thanks!
Comment 20 James Richardson 2008-12-15 11:57:03 UTC
This bug has been fixed in the latest release of SqueezeCenter!

Please download the new version from http://www.slimdevices.com/su_downloads.html if you haven't already.  

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.
Comment 21 Chris Owens 2008-12-18 11:54:35 UTC
Routine bug db maintenance; removing old versions which cause confusion.  I apologize for the inconvenience.