Bug 3069 - Explicit namesort values can be overwritten
: Explicit namesort values can be overwritten
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Database
: 6.5b1
: PC All
: P2 normal with 2 votes (vote)
: ---
Assigned To: Dan Sully
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-02-26 00:09 UTC by Jim McAtee
Modified: 2008-09-15 14:39 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jim McAtee 2006-02-26 00:09:44 UTC
If you define ARTISTSORT for some, but not all albums by a particular artist, 
then SlimServer may overwrite the defined ARTISTSORT value.  What ends up 
stored in the database following a scan will depend on which album gets
scanned last.  For instance, a couple of albums tagged with vorbis comments:

ALBUM=Songs for Swingin' Lovers
ARTIST=Frank Sinatra
ARTISTSORT=Sinatra, Frank

ALBUM=Live in Australia, 1959
ARTIST=Frank Sinatra
ARTIST=Red Norvo

In the second case, you can't define an ARTISTSORT tag, or it may be assigned
to the wrong ARTIST, so it's left blank.  If this album is scanned after the
first one, then the namesort column in the contributors table for the name
'Frank Sinatra' gets changed from 'SINATRA FRANK' to 'FRANK SINATRA', 
overwriting the previously defined sort string.

The logic should be that namesort should be written if the record is new, or
when updating an existing record, only if ARTISTSORT has been explicitly
defined in a track's tags.  This should keep tracks with non-existant
ARTISTSORT tags from wiping out previously defined values.

Note that the same thing can also happen with albums and the ALBUMSORT tag,
although it's less likely since you would usually expect all of the tracks
within an album to have the same ALBUM and ALBUMSORT tags.
Comment 1 Jim McAtee 2006-02-27 11:30:09 UTC
As someone else pointed out in the forums, there are other, more common situations 
where the same problem arises.  If a name appears in a contributor tag other than 
ARTIST, one where there is no recognized sortorder tag (BAND/ALBUMARTIST, COMPOSER, 
CONDUCTOR, etc.) then if the name already exists in the contributors table, the
namesort value for the name will be overwritten.

The logic should then be something like this:

look up name database
if (not found) then
  insert new record, with namesort
else
  if (namesort <> name) then
    update existing record, including namesort
  else
    update existing record, excluding namesort
  endif
endif
Comment 2 Greg Klanderman 2006-04-17 17:57:44 UTC
add myself to CC
Comment 3 Dan Sully 2006-05-02 17:21:51 UTC
Does this patch work for both of you, and not break anything else?

--- Slim/DataStores/DBI/Contributor.pm  (revision 12519)
+++ Slim/DataStores/DBI/Contributor.pm  (local)
@@ -94,6 +94,13 @@
                                'namesort'       => $sort,
                                'musicbrainz_id' => $brainzID,
                        });
+
+               } elsif ($contributorObj && $search ne $sort) {
+
+                       # Bug 3069 - update the namesort only if it's
+                       # different than the name search
+                       $contributorObj->namesort($sort);
+                       $contributorObj->update;
                }
 
                push @contributors, $contributorObj;
Comment 4 Dan Sully 2006-05-08 12:22:42 UTC
Anyone? Bueller?
Comment 5 Jim McAtee 2006-05-08 16:23:15 UTC
Ok, I just finished doing some testing with and without the patch.  It looks very good.  Corrects a number of artists that were out of order.

I only really tested the case with ARTIST tags, as I have very few tracks tagged with contributor roles such as COMPOSER, CONDUCTOR, etc.  Should this patch handle all contributor roles?
Comment 6 Jim McAtee 2006-06-03 21:13:38 UTC
Was this patch ever checked in?
Comment 7 Dan Sully 2006-06-03 21:31:33 UTC
No it hasn't been - still on my radar, I wanted to look at some more possible test cases though.
Comment 8 Dan Sully 2006-06-13 13:17:30 UTC
Fixed in change 7947