Bug 5296 - MUSICBRAINZ_SORTNAME - not supported but in code?
: MUSICBRAINZ_SORTNAME - not supported but in code?
Status: RESOLVED WORKSFORME
Product: Logitech Media Server
Classification: Unclassified
Component: Tagging
: 6.5.4
: All All
: P2 enhancement (vote)
: Future
Assigned To: Squeezebox QA Team email alias
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-21 16:01 UTC by Richard Harnwell
Modified: 2007-10-22 16:01 UTC (History)
5 users (show)

See Also:
Category: ---


Attachments
Proposed patch to take MusicBrainz artist sort order into account (797 bytes, patch)
2007-10-07 02:56 UTC, Stuart Hickinbottom
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Harnwell 2007-08-21 16:01:59 UTC
SS doesn't seem to support MUSICBRAINZ_SORTNAME (only ARTISTSORT as far as I can tell - I'm pretty sure a response from support confirmed this some time ago). If that is the case, I don't understand why it is referenced in FLAC.pm:

   my %tagMapping = ( 'TRACKNUMBER' => 'TRACKNUM', 
                       'DISCNUMBER' => 'DISC',
                              'URL' => 'URLTAG',
             'musicbrainz_sortname' => 'ARTISTSORT', 
        'MUSICBRAINZ_ALBUMARTISTID' => 'MUSICBRAINZ_ALBUMARTIST_ID', 
              'MUSICBRAINZ_ALBUMID' => 'MUSICBRAINZ_ALBUM_ID', 
          'MUSICBRAINZ_ALBUMSTATUS' => 'MUSICBRAINZ_ALBUM_STATUS', 
            'MUSICBRAINZ_ALBUMTYPE' => 'MUSICBRAINZ_ALBUM_TYPE', 
             'MUSICBRAINZ_ARTISTID' => 'MUSICBRAINZ_ARTIST_ID', 
             'MUSICBRAINZ_SORTNAME' => 'MUSICBRAINZ_SORTNAME', 
              'MUSICBRAINZ_TRACKID' => 'MUSICBRAINZ_ID', 
                'MUSICBRAINZ_TRMID' => 'MUSICBRAINZ_TRM_ID',


My Perl isn't good enough to work out what is going on here. It looks pretty weird to me - MUSICBRAINZ_SORTNAME being mapped to itself, and musicbrainz_sortname being mapped to ARTISTSORT (why just do this for the lowercase version?).

I'm on the latest (6.5.4 Linux) version of SS, but I believe this situation hasn't changed for some time.

What I'd really like (and I'm sure I'm not alone) is for SS to support MUSICBRAINZ_SORTNAME. If that's not possible, maybe the above code needs tidying up?

Cheers,

Richard
Comment 1 KDF 2007-08-21 16:44:44 UTC
the lowercase mappign was added at change 2900 by Michael.  Michael, do you recall why?

The rest of the MusicBrainz stuff came in at change 4306 from Dan, based on a patch provided by a user.

The original intent was simply to collect the metadata for the purposes of using it in something like SlimScrobbler which can record usage based on musicbrainz_id. Technically, it is supported as collected metadata, just not as a sorting mechanism.

I'm not sure I like the idea of mangling the ARTISTSORT field with potentially multiple sources.  It's already enough of a nightmare with what is supported currently, and with what users end up having in their metadata.
Comment 2 Michael Herger 2007-08-21 23:07:31 UTC
(In reply to comment #1)
> the lowercase mappign was added at change 2900 by Michael.  Michael, do you
> recall why?

I'm sorry I can't help here. That's another Michael (michael at fallenangels dot com if I remember right). I never touched much in that area.
Comment 3 Chris Owens 2007-09-04 11:58:47 UTC
So it sounds like perhaps this should be converted to an enhancement request that we support sorting by MUSICBRAINZ_SORTNAME?  And we will of course take KDF's comments into account.
Comment 4 Richard Harnwell 2007-09-04 13:14:59 UTC

(In reply to comment #3)
> So it sounds like perhaps this should be converted to an enhancement request
> that we support sorting by MUSICBRAINZ_SORTNAME?  And we will of course take
> KDF's comments into account.

Well I'd obviously be very pleased if this could be done, but I understand KDF's comments re. adding confusion to the metadata side of things. I guess MUSICBRAINZ_SORTNAME should only be used if ARTISTSORT is empty.
Comment 5 Stuart Hickinbottom 2007-10-07 02:56:06 UTC
Created attachment 2221 [details]
Proposed patch to take MusicBrainz artist sort order into account

The attached patch works for me. I looked quite hard and couldn't see what the lowercase 'musicbrainz_sortname' was for, and the mapping of 'MUSICBRAINZ_SORTNAME' back to itself just seemed plain wrong.

Could those affected test it?
Comment 6 Stuart Hickinbottom 2007-10-10 00:23:24 UTC
This is currently categorised as an enhancement - is that really the case? It looks like a defect to me as it appears as though the code in there was expected to work but doesn't do what it's supposed to.

Also, it's marked as affecting PCs only (it affects all platforms), and Fedora only (it affects all operating systems).

It would be nice to get this fixed in SqueezeCenter 7.
Comment 7 Andy Grundman 2007-10-10 11:35:11 UTC
I applied this patch as change 13714 as well as adding it for MP3 and Ogg.  MP3 uses a different tag for this: "MUSICBRAINZ ALBUM ARTIST SORTNAME".
Comment 8 Spies Steven 2007-10-12 11:11:01 UTC
See Bug 5764.
Comment 9 Spies Steven 2007-10-12 16:24:15 UTC
From what I can tell according to the MusicBrainz tag spec at http://musicbrainz.org/doc/MusicBrainzTag shouldn't 'XSOP' or 'TSOP' be mapped to 'ARTISTSORT' not 'TXXX MusicBrainz Album Artist Sortname'?
Comment 10 Andy Grundman 2007-10-20 06:21:47 UTC
You're right, I removed the album artist sort mapping for MP3.
Comment 11 Spies Steven 2007-10-22 16:01:49 UTC
Looks good to me.  From what I can tell both MusicBrainz Sortname and
MusicBrainz Album Artist mapping has been added to SqueezeCenter.  Only
MusicBrainz Album Artist Sortname is missing since SqueezeCenter does not
currently support AlbumArtistSort and Bug #4584 covers that.