diff --git a/taglib/asf/asftag.cpp b/taglib/asf/asftag.cpp index 1d37b83b..29a91d3b 100644 --- a/taglib/asf/asftag.cpp +++ b/taglib/asf/asftag.cpp @@ -299,6 +299,7 @@ namespace std::tuple("WM/ToolName", "TOOLNAME", ASF::Attribute::UnicodeType), std::tuple("WM/ToolVersion", "TOOLVERSION", ASF::Attribute::UnicodeType), std::tuple("WM/Provider", "PROVIDER", ASF::Attribute::UnicodeType), + std::tuple("WM/SharedUserRating", "RATING", ASF::Attribute::DWordType), std::tuple("WM/UniqueFileIdentifier", "UNIQUEFILEIDENTIFIER", ASF::Attribute::UnicodeType), std::tuple("WMFSDKVersion", "WMFSDKVERSION", ASF::Attribute::UnicodeType), std::tuple("WMFSDKNeeded", "WMFSDKNEEDED", ASF::Attribute::UnicodeType), @@ -381,8 +382,13 @@ PropertyMap ASF::Tag::properties() const props["COMMENT"] = d->comment; } + AttributeList sharedRatings, popularimeters; for(const auto &[k, attributes] : std::as_const(d->attributeListMap)) { if(const String key = translateKey(k); !key.isEmpty()) { + if(key == "RATING") { + sharedRatings = attributes; + continue; + } for(const auto &attr : attributes) { // The same attribute can occur in both the extended content // description object and the metadata (library) object, skip exact @@ -393,10 +399,38 @@ PropertyMap ASF::Tag::properties() const } } } + else if(k.upper() == "POPULARIMETER") { + popularimeters = attributes; + } else { props.addUnsupportedData(k); } } + + // Expose the rating through the standard RATING property. WM/SharedUserRating + // (the Windows Media rating, a DWORD on the 0-99 scale) takes precedence over + // the POPM-style POPULARIMETER attribute (email|rating|counter, 0-255). + if(!sharedRatings.isEmpty()) { + for(const auto &attr : sharedRatings) { + if(const String value = attributeToString(attr); + !props.value("RATING").contains(value)) { + props.insert("RATING", value); + } + } + } + else if(!popularimeters.isEmpty()) { + // A POPULARIMETER attribute stores a POPM-style value of the form + // "email|rating|counter". Extract the numeric rating field. + for(const auto &attr : popularimeters) { + if(const StringList parts = attr.toString().split("|"); + parts.size() == 3) { + if(const String &value = parts[1]; + !value.isEmpty() && !props.value("RATING").contains(value)) { + props.insert("RATING", value); + } + } + } + } return props; } @@ -431,6 +465,11 @@ PropertyMap ASF::Tag::setProperties(const PropertyMap &props) else if(prop == "COPYRIGHT") { d->copyright.clear(); } + else if(prop == "RATING") { + // Remove both rating sources to keep the round-trip lossless. + eraseAttribute(d->attributeListMap, "WM/SharedUserRating"); + eraseAttribute(d->attributeListMap, "POPULARIMETER"); + } else { eraseAttribute(d->attributeListMap, reverseKeyMap[prop].first); } @@ -441,6 +480,21 @@ PropertyMap ASF::Tag::setProperties(const PropertyMap &props) for(const auto &[prop, attributes] : props) { if(reverseKeyMap.contains(prop)) { const auto &[name, type] = reverseKeyMap[prop]; + if(name == "WM/SharedUserRating" && + !d->attributeListMap.contains("WM/SharedUserRating")) { + if(auto it = d->attributeListMap.find("POPULARIMETER"); + it != d->attributeListMap.end() && !it->second.isEmpty()) { + // Rebuild a POPULARIMETER value, replacing the rating field while keeping + // the email and counter. + const StringList parts = it->second.front().toString().split("|"); + eraseAttribute(d->attributeListMap, "POPULARIMETER"); + for(const auto &str : attributes) { + addAttribute("POPULARIMETER", + parts.size() == 3 ? parts[0] + "|" + str + "|" + parts[2] : str); + } + continue; + } + } eraseAttribute(d->attributeListMap, name); for(const auto &str : attributes) { switch(type) { diff --git a/tests/data/wma_rating.wma b/tests/data/wma_rating.wma new file mode 100644 index 00000000..d2bcec04 Binary files /dev/null and b/tests/data/wma_rating.wma differ diff --git a/tests/test_asf.cpp b/tests/test_asf.cpp index 94088d40..87afbe75 100644 --- a/tests/test_asf.cpp +++ b/tests/test_asf.cpp @@ -53,6 +53,8 @@ class TestASF : public CppUnit::TestFixture CPPUNIT_TEST(testProperties); CPPUNIT_TEST(testPropertiesAllSupported); CPPUNIT_TEST(testPropertiesRealFile); + CPPUNIT_TEST(testRating); + CPPUNIT_TEST(testRatingWma); CPPUNIT_TEST(testCaseInsensitiveAttributeNames); CPPUNIT_TEST(testRepeatedSave); CPPUNIT_TEST_SUITE_END(); @@ -392,6 +394,7 @@ public: tags["ISVBR"] = StringList("1"); tags["PEAKVALUE"] = StringList("32104"); tags["AVERAGELEVEL"] = StringList("2450"); + tags["RATING"] = StringList("80"); ScopedFileCopy copy("silence-1", ".wma"); { @@ -478,6 +481,7 @@ public: {"WM/PromotionURL", ASF::Attribute::UnicodeType}, {"WM/Provider", ASF::Attribute::UnicodeType}, {"WM/Publisher", ASF::Attribute::UnicodeType}, + {"WM/SharedUserRating", ASF::Attribute::DWordType}, {"WM/Script", ASF::Attribute::UnicodeType}, {"WM/SetSubTitle", ASF::Attribute::UnicodeType}, {"WM/SubTitle", ASF::Attribute::UnicodeType}, @@ -551,6 +555,103 @@ public: CPPUNIT_ASSERT_EQUAL(StringList(";"), tags["UNIQUEFILEIDENTIFIER"]); } + void testRating() + { + // The example of the bug report: a file carrying both a Windows Media + // rating and a foobar2000-style POPM attribute. + ScopedFileCopy copy("silence-1", ".wma"); + { + ASF::File f(copy.fileName().c_str()); + f.tag()->setAttribute("WM/SharedUserRating", 75U); + f.tag()->setAttribute("POPULARIMETER", String("player@example.com|128|0")); + f.save(); + } + { + const ASF::File f(copy.fileName().c_str()); + PropertyMap tags = f.properties(); + + // WM/SharedUserRating is exposed and takes precedence over POPULARIMETER. + CPPUNIT_ASSERT_EQUAL(StringList("75"), tags["RATING"]); + CPPUNIT_ASSERT(!tags.unsupportedData().contains("WM/SharedUserRating")); + CPPUNIT_ASSERT(!tags.unsupportedData().contains("POPULARIMETER")); + } + { + // Without WM/SharedUserRating, the numeric POPM rating is extracted. + ASF::File f(copy.fileName().c_str()); + f.tag()->removeItem("WM/SharedUserRating"); + f.save(); + } + { + ASF::File f(copy.fileName().c_str()); + const PropertyMap tags = f.properties(); + CPPUNIT_ASSERT_EQUAL(StringList("128"), tags["RATING"]); + + // Setting RATING keeps the existing POPULARIMETER convention while + // preserving the email and counter. + PropertyMap props = f.properties(); + props["RATING"] = StringList("200"); + f.setProperties(props); + CPPUNIT_ASSERT_EQUAL(String("player@example.com|200|0"), + f.tag()->attribute("POPULARIMETER").front().toString()); + f.save(); + } + { + const ASF::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(StringList("200"), f.properties()["RATING"]); + } + { + // Once WM/SharedUserRating exists, writes go to it. + ASF::File f(copy.fileName().c_str()); + f.tag()->setAttribute("WM/SharedUserRating", 99U); + PropertyMap props = f.properties(); + CPPUNIT_ASSERT_EQUAL(StringList("99"), props["RATING"]); + props["RATING"] = StringList("50"); + f.setProperties(props); + CPPUNIT_ASSERT_EQUAL(50U, + f.tag()->attribute("WM/SharedUserRating").front().toUInt()); + f.save(); + } + { + // Removing RATING removes both rating sources. + ASF::File f(copy.fileName().c_str()); + PropertyMap props = f.properties(); + props.erase("RATING"); + f.setProperties(props); + CPPUNIT_ASSERT(!f.tag()->contains("WM/SharedUserRating")); + CPPUNIT_ASSERT(!f.tag()->contains("POPULARIMETER")); + } + } + + void testRatingWma() + { + // Regression test on a real-world Windows Media file carrying both the + // Windows Media rating (WM/SharedUserRating, DWORD on the 0-99 scale) and + // a foobar2000-style POPULARIMETER attribute (email|rating|counter) whose + // numeric rating is higher. WM/SharedUserRating must win, and neither + // attribute may be dropped into the unsupported data. + const ASF::File f(TEST_FILE_PATH_C("wma_rating.wma")); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(f.audioProperties()); + CPPUNIT_ASSERT(f.tag()); + + const PropertyMap tags = f.properties(); + CPPUNIT_ASSERT_EQUAL(StringList("75"), tags["RATING"]); + CPPUNIT_ASSERT(!tags.unsupportedData().contains("WM/SharedUserRating")); + CPPUNIT_ASSERT(!tags.unsupportedData().contains("POPULARIMETER")); + + // The two rating sources are present in the raw attribute map with the + // expected types: a 0-99 DWORD and a POPM-style string. + const ASF::AttributeListMap &map = f.tag()->attributeListMap(); + CPPUNIT_ASSERT(map.find("WM/SharedUserRating") != map.end()); + CPPUNIT_ASSERT_EQUAL(ASF::Attribute::DWordType, + map["WM/SharedUserRating"].front().type()); + CPPUNIT_ASSERT_EQUAL(75U, + map["WM/SharedUserRating"].front().toUInt()); + CPPUNIT_ASSERT(map.find("POPULARIMETER") != map.end()); + CPPUNIT_ASSERT_EQUAL(ASF::Attribute::UnicodeType, + map["POPULARIMETER"].front().type()); + } + void testCaseInsensitiveAttributeNames() { ScopedFileCopy copy("silence-1", ".wma");