> On Aug. 25, 2013, 12:06 p.m., Matěj Laitl wrote: > > shared/tag_helpers/APETagHelper.cpp, lines 33-37 > > <http://git.reviewboard.kde.org/r/112266/diff/1/?file=184486#file184486line33> > > > > Hmm, is there a specification somewhere that says what the proper > > identifiers are? I fear of backwards compatibility, perhaps there are files > > out there that use the title-cased identifiers? > > Bruno Léon wrote: > I did not find specifications for this. > Actually when using "Album Artist" instead of "ALBUM ARTIST", the tag is > not read at all. > Taglib output it as "ALBUM ARTIST" which made me make this correction > (and checked it actually works). > > Same this for compilation. This makes Amarok compliant with Taglib output > format. > > Matěj Laitl wrote: > I don't think that TagLib itself is origin of the string, I think it just > reads the identifier that is in the actual tag (which can vary file by file). > Or perhaps you can prove me wrong by pointing out relevant part of TagLib > source code? > > Mark Kretschmann wrote: > Here's the spec: > http://wiki.hydrogenaudio.org/index.php?title=APE_Tag_Item > > And here is the list of valid keys: > http://wiki.hydrogenaudio.org/index.php?title=APE_key > > Keys are case sensitive, first character uppercase, rest lowercase (e.g. > "Artist"). So it looks like the all-uppercase stuff must come from TagLib, > and that our original code used the keys from the actual spec (minus "Album > Artist" and "BPM", which don't exist in the spec at all).
Ergo, probably Matej is right, TagLib just reads the identifiers from the file, and the files you have are actually incompatible with the spec. We could simply toLower() the strings before comparing. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112266/#review38527 ----------------------------------------------------------- On Aug. 25, 2013, 11:17 a.m., Bruno Léon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112266/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2013, 11:17 a.m.) > > > Review request for Amarok. > > > Description > ------- > > Fix reading of Album Artist and Compilation tag in APE tags. > Add support for reading Disc Number in APE tags. > > > Diffs > ----- > > shared/tag_helpers/APETagHelper.cpp c628694 > > Diff: http://git.reviewboard.kde.org/r/112266/diff/ > > > Testing > ------- > > Tested with Musepack files (that do use APE tags) > > > Thanks, > > Bruno Léon > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
