clementine-player / Clementine

:tangerine: Clementine Music Player
https://www.clementine-player.org/
GNU General Public License v3.0
3.77k stars 677 forks source link

FMPS_RATING of -1 Violates Specification #4128

Closed bill-mcgonigle closed 10 years ago

bill-mcgonigle commented 10 years ago

I'm writing a little command line tagger for my FLAC files (since key shortcuts are currently broken under KDE, #1485) and when I parse the Vorbis comments for FMPS_RATING, I'm seeing a value of -1 for unrated files.

With Audio::FLAC:Header (perl/CPAN):

          '/storage/music/flac/The_Police/Synchronicity/01__Synchronicity_I.flac' => {
                                                                                       'found' => 1,
                                                                                       'rating' => '-1'
                                                                                     },

and metaflac:

$ metaflac --list /storage/music/flac/REO_Speedwagon/The_Hits/09__Cant_Fight_This_Feeling.flac | grep RAT
    comment[5]: FMPS_RATING=-1
    comment[6]: FMPS_RATING_AMAROK_SCORE=0

I can see why '-1' might be useful as 'unrated' but it appears to violate the FMPS Spec:

A file that has no rating tag for a specific purpose is to be considered unrated for that purpose (user, critic or algorithm). 2.2.1 All Rating Tags • Ratings SHALL be a float value between 0.0 and 1.0, inclusive. 0.0 SHALL be the lowest possible rating; 1.0 is the highest possible rating.

I can certainly work with this for my needs, but we should adhere to the spec and omit the tag if a file is not rated to promote interoperability. I'm assuming 0.0 is the semantic value for 'explicitly untagged' and 0.1 would indicate the 'worst rating' under FMPS. I believe in Clementine one can click to the left of the 1st star to set a 0-star rating. I'm unaware of whether this is tracked as being different than unrated.

ArnaudBienner commented 10 years ago

My understanding of the spec is "unrated" = "tag not found", not 0.0, so I changed our code to not write anything in this case.

ArnaudBienner commented 10 years ago

Thanks for reporting the issue btw :)

bill-mcgonigle commented 10 years ago

Agreed - I misread the spec back when I filed this. Thanks for the fix!