GNOME Bugzilla – Bug 567657
Support artist sortnames
Last modified: 2009-01-23 00:22:24 UTC
Artist sortnames are a way to better control the ordering of tracks by artist. For example, the sortname for "The Beatles" might be "Beatles, The". TagLib# ought to support retrieving this metadata.
Created attachment 126377 [details] [review] Add sort information to TagLib.Tag First attempt at a patch. It needs additional documentation for the new properties, and an implementation for ID3 tags.
Created attachment 126392 [details] [review] Add sort information to TagLib.Tag (v2) Updated patch. Adds more documentation, and some support for ID3 tags. According to id3.org[1], only artist sort information is supported (in the TSOP frame), so I left the album artist sort unimplemented in ID3 tags. [1] http://www.id3.org/id3v2.4.0-frames
Created attachment 126485 [details] [review] added mpeg4 sort fields to John's v2 Patch I updated John's patch to support mpeg4 Performer Sort and AlbumArtist Sort. I also had to add entries for the overrides into combinedtag.cs, I suspect you didn't bump into the data not making it into taglib.tag completely with ogg files. This diff should include John's entries, effectively obsoleting it. If you'd like me to wait and create another patch after commiting his changes, let me know.
Here are some non-standard id3v2 frames: TSO2: iTunes uses this for Album Artist sort order TSOC: iTunes uses this for Composer sort order
Created attachment 126545 [details] [review] Revised Patch adding Sort Fields This patch has jmillikin's changes and mine to implement all 5 sort fields for Id3v2, Ogg/Xiph, and mpeg4. I also dug up 2 more Sort fields: AlbumSort - TSOA Frames for ID3 - ALBUMSORT for Ogg - soal for mpeg4 TitleSort - TSOT Frame for ID3 - TITLESORT for ogg - sonm for mpeg4 PerformersSort - TSOP Frames for ID3 - ARTISTSORT of Ogg - soar for mpeg4 AlbumArtistsSort - TSO2 Frames for ID3 - ALBUMARTISTSORT of Ogg - soaa for mpeg4 ComposersSort - TSOC Frames for ID3 - COMPOSERSORT for Ogg - soco for mpeg4
Hey guys, looks pretty good. 1. Can we verify that string [] are stored in all these, not just strings? Don't want to expose it as string [] if that's what nobody else does. And let's put the URLs for where we found the codes in their respective files. 2. Would you document the ID3 frame-type literals like you did for mp4? 3. In src/TagLib/Mpeg4/BoxTypes.cs in TitleSort (and others) in the getter, you create the empty array and you don't exit the foreach if the value is != null and with Length > 0 like you do for ComposersSort in src/TagLib/CombinedTag.cs - please change to more like CombinedTag's style. Thanks!
> Can we verify that string [] are stored in all these, not just strings? Don't want to expose it as string [] if that's what nobody else does. The interface for sortnames ought to match the interface for "real" names. It makes no sense to have `string[] Performers`, and `string PerformersSort`. However, I can't think of any reasonable way to handle the case of two ARTIST and two ARTISTSORT (for example) tags in one file. There's no way to know which is which or whether they are in the correct order respective to eachother.
Right, exactly the reason for me questioning it - if you want to sort by artist and it's a string [], what does that mean?
> Right, exactly the reason for me questioning it - if you want to sort by artist > and it's a string [], what does that mean? My opinion is to leave that up to the application; TagLib should just try to accurately represent the state of the file. Since files may be tagged with multiple sortnames, there needs to be a way to retrieve them all. We could add fields like FirstPerformerSort, as an analogue to FirstPerformer.
PerformerSort, ArtistSort, and ComposerSort I can see being string [] as their might be multiple values. TitleSort and AlbumSort really will never have more than one entry, perhaps they should be string. If your creating a list by PerformerSort, each PerformerSort entry should be listed in your list, with perhaps the same track (or album) listed multiple times, for each performer listed. The sort fields really just give the option to provide a more natural list without having to sort on articles (The A An) IE "Beatles, The" and "McCartney, Paul";"Harrison, George" I left them as string[] for consistency.
Gabriel - In #3 you refered to src/TagLib/Mpeg4/BoxTypes.cs, and I don't think that was what you meant. I suspect you meant src/TagLib/Ogg/GroupedComment.cs. My reason for the two very similiar blocks being different was that all the getters from CombinedTag.cs use the method you recomended, while all the getters from GroupedComment.cs use a different method (the one I employed in that file). So If I change them to look like the ones in CombinedTag.cs they won't match the existing entries in GroupedComment.cs. So should the blocks in this patch match across the files, or should they match the style used in the particular file? I suspect they should match across this patch, and perhaps later the block from GroupedComment.cs can be updated to conform to the style. -A
Created attachment 126556 [details] [review] Add sort information to TagLib.Tag (v3) And only now, hours after replying, do I realize which fields you were talking about... Andy's latest patch version, with TitleSort and AlbumSort converted to be single-string properties. Not a big change, but enough to let it be tested in Banshee.
Created attachment 126559 [details] [review] Same string changes as John, updated comments, etc I had just changed the same string[] to string. :-) AlbumSort -> string TitleSort -> string PerformersSort -> string[] (FirstPerformerSort) AlbumArtistsSort -> string[] (FirstAlbumArtistSort) ComposersSort -> string[] (FirstComposerSort) Updated the comments with the best references I could find for urls, a bit light imho as far as references go, but I'll adopt the "this is how xxx is doing it stance." Changed the added code in src/TagLib/Ogg/GroupedComments.cs to better match the entries in CombinedTag.cs. It doesn't match the existing style in that file, but that's another issue.
Created attachment 126633 [details] [review] Same as previous - adds support for asf sort properties (wma) Patch is same as above, except adds changes needed to src/TagLib/Asf/Tag.cs for asf sort fields(excepting composer which I could not find documented). I could not find an implementation for ape tags, excepting to copy vorbis style so I left ape alone. Doesn't necessarily obsolete last patch, it could be re-created as a seperate diff if you've already tested with the last patch and ready to commit.
John, your patch in http://bugzilla.gnome.org/show_bug.cgi?id=567657#c12 was obsolete, right? Committed, thanks a lot guys, great job working together on this!
> John, your patch in http://bugzilla.gnome.org/show_bug.cgi?id=567657#c12 was obsolete, right? Yes, it was just there to provide a version with the type signature changes for testing and was obsoleted by comment #13.