diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp index beb6c70c7..362d9376d 100644 --- a/ext/libclementine-tagreader/tagreader.cpp +++ b/ext/libclementine-tagreader/tagreader.cpp @@ -40,6 +40,7 @@ #include #endif #include +#include #include #include #include @@ -180,6 +181,25 @@ void TagReader::ReadFile(const QString& filename, song); } } + + // Check POPM tags + // We do this after checking FMPS frames, so FMPS have precedence, as we + // will consider POPM tags iff song has no rating/playcount already set. + qLog(Debug) << "POPM"; + if (!map["POPM"].isEmpty()) { + const TagLib::ID3v2::PopularimeterFrame* frame = + dynamic_cast(map["POPM"].front()); + if (frame) { + // Take a user rating only if there's no rating already set + if (song->rating() <= 0 && frame->rating() > 0) { + song->set_rating(ConvertPOPMRating(frame->rating())); + } + if (song->playcount() <= 0 && frame->counter() > 0) { + song->set_playcount(frame->counter()); + } + } + } + } } else if (TagLib::Ogg::Vorbis::File* file = dynamic_cast(fileref->file())) { if (file->tag()) { @@ -486,8 +506,26 @@ bool TagReader::SaveSongStatisticsToFile(const QString& filename, if (TagLib::MPEG::File* file = dynamic_cast(fileref->file())) { TagLib::ID3v2::Tag* tag = file->ID3v2Tag(true); + + // Save as FMPS SetUserTextFrame("FMPS_Rating", QString::number(song.rating()), tag); SetUserTextFrame("FMPS_PlayCount", QString::number(song.playcount()), tag); + + // Also save as POPM + TagLib::ID3v2::PopularimeterFrame* frame = NULL; + + const TagLib::ID3v2::FrameListMap& map = file->ID3v2Tag()->frameListMap(); + if (!map["POPM"].isEmpty()) { + frame = dynamic_cast(map["POPM"].front()); + } + + if (!frame) { + frame = new TagLib::ID3v2::PopularimeterFrame(); + tag->addFrame(frame); + } + + frame->setRating(ConvertToPOPMRating(song.rating())); + frame->setCounter(song.playcount()); } else { // Nothing to save: stop now return true; @@ -734,3 +772,30 @@ bool TagReader::ReadCloudFile(const QUrl& download_url, } #endif // HAVE_GOOGLE_DRIVE +float TagReader::ConvertPOPMRating(const int POPM_rating) { + if (POPM_rating < 0x01) { + return 0.0; + } else if (POPM_rating < 0x40) { + return 0.20; // 1 star + } else if (POPM_rating < 0x80) { + return 0.40; // 2 stars + } else if (POPM_rating < 0xC0) { + return 0.60; // 3 stars + } else if (POPM_rating < 0xFF) { + return 0.80; // 4 stars + } + return 1.0; // 5 stars +} + +int TagReader::ConvertToPOPMRating(const float rating) { + if (rating < 0.20) { + return 0x00; + } else if (rating < 0.40) { + return 0x01; + } else if (rating < 0.60) { + return 0x80; + } else if (rating < 0.80) { + return 0xC0; + } + return 0xFF; +} diff --git a/ext/libclementine-tagreader/tagreader.h b/ext/libclementine-tagreader/tagreader.h index 314f11bb7..e21384dac 100644 --- a/ext/libclementine-tagreader/tagreader.h +++ b/ext/libclementine-tagreader/tagreader.h @@ -93,6 +93,11 @@ class TagReader { TagLib::ID3v2::Tag* tag) const; private: + // Returns a float in [0.0..1.0] corresponding to the rating range we use in Clementine + static float ConvertPOPMRating(const int POPM_rating); + // Reciprocal + static int ConvertToPOPMRating(const float rating); + FileRefFactory* factory_; QNetworkAccessManager* network_; diff --git a/tests/data/fmpspopmrating.mp3 b/tests/data/fmpspopmrating.mp3 new file mode 100644 index 000000000..c1f8bf629 Binary files /dev/null and b/tests/data/fmpspopmrating.mp3 differ diff --git a/tests/data/popmrating.mp3 b/tests/data/popmrating.mp3 new file mode 100644 index 000000000..9408934e0 Binary files /dev/null and b/tests/data/popmrating.mp3 differ diff --git a/tests/data/testdata.qrc b/tests/data/testdata.qrc index 97d7dceb0..42716808b 100644 --- a/tests/data/testdata.qrc +++ b/tests/data/testdata.qrc @@ -11,6 +11,7 @@ fmpsplaycount.mp3 fmpsplaycountboth.mp3 fmpsplaycountuser.mp3 + fmpspopmrating.mp3 fmpsrating.mp3 fmpsratingboth.mp3 fmpsratinguser.mp3 @@ -18,6 +19,7 @@ manyfiles.cue manyfilesbroken.cue onesong.cue + popmrating.mp3 pls_one.pls pls_somafm.pls secretagent.asx diff --git a/tests/song_test.cpp b/tests/song_test.cpp index bb92a3312..cb4d5d10a 100644 --- a/tests/song_test.cpp +++ b/tests/song_test.cpp @@ -146,4 +146,18 @@ TEST_F(SongTest, FMPSPlayCountBoth) { EXPECT_EQ(123, song.playcount()); } +TEST_F(SongTest, POPMRating) { + TemporaryResource r(":/testdata/popmrating.mp3"); + Song song = ReadSongFromFile(r.fileName()); + EXPECT_FLOAT_EQ(0.60, song.rating()); +} + +TEST_F(SongTest, BothFMPSPOPMRating) { + // fmpspopmrating.mp3 contains FMPS with rating 0.42 and POPM with 0x80 + // (corresponds to 0.60 rating for us): check that FMPS tag has precedence + TemporaryResource r(":/testdata/fmpspopmrating.mp3"); + Song song = ReadSongFromFile(r.fileName()); + EXPECT_FLOAT_EQ(0.42, song.rating()); +} + } // namespace