From 1b64bb0cb73435555527dd625d9b14dbd3e28f14 Mon Sep 17 00:00:00 2001 From: Urs Fleisch Date: Tue, 18 Oct 2016 20:34:53 +0200 Subject: [PATCH] Support new classical music frames introduced with iTunes 12.5, #758. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit M4A: ©wrk: Work (string) ©mvn: Movement Name (string) ©mvi: Movement Number (number) ©mvc: Movement Count (number) shwm: Show Work & Movement (0/1) ID3 (2.3, 2.4; MVN, MVI for 2.2): MVNM: Movement Name MVIN: Movement Number/Count --- taglib/mp4/mp4tag.cpp | 22 ++++++---- taglib/mpeg/id3v2/id3v2frame.cpp | 10 +++-- taglib/mpeg/id3v2/id3v2framefactory.cpp | 6 ++- tests/test_id3v2.cpp | 42 ++++++++++++++++++++ tests/test_mp4.cpp | 53 +++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 14 deletions(-) diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index a8e2e7d3..0c2e5cbc 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -71,10 +71,10 @@ MP4::Tag::Tag(TagLib::File *file, MP4::Atoms *atoms) : parseIntPair(atom); } else if(atom->name == "cpil" || atom->name == "pgap" || atom->name == "pcst" || - atom->name == "hdvd") { + atom->name == "hdvd" || atom->name == "shwm") { parseBool(atom); } - else if(atom->name == "tmpo") { + else if(atom->name == "tmpo" || atom->name == "\251mvi" || atom->name == "\251mvc") { parseInt(atom); } else if(atom->name == "tvsn" || atom->name == "tves" || atom->name == "cnID" || @@ -472,10 +472,11 @@ MP4::Tag::save() else if(name == "disk") { data.append(renderIntPairNoTrailing(name.data(String::Latin1), it->second)); } - else if(name == "cpil" || name == "pgap" || name == "pcst" || name == "hdvd") { + else if(name == "cpil" || name == "pgap" || name == "pcst" || name == "hdvd" || + name == "shwm") { data.append(renderBool(name.data(String::Latin1), it->second)); } - else if(name == "tmpo") { + else if(name == "tmpo" || name == "\251mvi" || name == "\251mvc") { data.append(renderInt(name.data(String::Latin1), it->second)); } else if(name == "tvsn" || name == "tves" || name == "cnID" || @@ -844,6 +845,11 @@ namespace { "sonm", "TITLESORT" }, { "soco", "COMPOSERSORT" }, { "sosn", "SHOWSORT" }, + { "shwm", "SHOWWORKMOVEMENT" }, + { "\251wrk", "WORK" }, + { "\251mvn", "MOVEMENTNAME" }, + { "\251mvi", "MOVEMENTNUMBER" }, + { "\251mvc", "MOVEMENTCOUNT" }, { "----:com.apple.iTunes:MusicBrainz Track Id", "MUSICBRAINZ_TRACKID" }, { "----:com.apple.iTunes:MusicBrainz Artist Id", "MUSICBRAINZ_ARTISTID" }, { "----:com.apple.iTunes:MusicBrainz Album Id", "MUSICBRAINZ_ALBUMID" }, @@ -897,10 +903,10 @@ PropertyMap MP4::Tag::properties() const } props[key] = value; } - else if(key == "BPM") { + else if(key == "BPM" || key == "MOVEMENTNUMBER" || key == "MOVEMENTCOUNT") { props[key] = String::number(it->second.toInt()); } - else if(key == "COMPILATION") { + else if(key == "COMPILATION" || key == "SHOWWORKMOVEMENT") { props[key] = String::number(it->second.toBool()); } else { @@ -952,11 +958,11 @@ PropertyMap MP4::Tag::setProperties(const PropertyMap &props) d->items[name] = MP4::Item(first, second); } } - else if(it->first == "BPM" && !it->second.isEmpty()) { + else if((it->first == "BPM" || it->first == "MOVEMENTNUMBER" || it->first == "MOVEMENTCOUNT") && !it->second.isEmpty()) { int value = it->second.front().toInt(); d->items[name] = MP4::Item(value); } - else if(it->first == "COMPILATION" && !it->second.isEmpty()) { + else if((it->first == "COMPILATION" || it->first == "SHOWWORKMOVEMENT") && !it->second.isEmpty()) { bool value = (it->second.front().toInt() != 0); d->items[name] = MP4::Item(value); } diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 1f896fa6..ec3b9310 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -111,8 +111,8 @@ Frame *Frame::createTextualFrame(const String &key, const StringList &values) // // check if the key is contained in the key<=>frameID mapping ByteVector frameID = keyToFrameID(key); if(!frameID.isEmpty()) { - // Apple proprietary WFED (Podcast URL) is in fact a text frame. - if(frameID[0] == 'T' || frameID == "WFED"){ // text frame + // Apple proprietary WFED (Podcast URL), MVNM (Movement Name), MVIN (Movement Number) are in fact text frames. + if(frameID[0] == 'T' || frameID == "WFED" || frameID == "MVNM" || frameID == "MVIN"){ // text frame TextIdentificationFrame *frame = new TextIdentificationFrame(frameID, String::UTF8); frame->setText(values); return frame; @@ -392,6 +392,8 @@ namespace { "TDES", "PODCASTDESC" }, { "TGID", "PODCASTID" }, { "WFED", "PODCASTURL" }, + { "MVNM", "MOVEMENTNAME" }, + { "MVIN", "MOVEMENTNUMBER" }, }; const size_t frameTranslationSize = sizeof(frameTranslation) / sizeof(frameTranslation[0]); @@ -474,8 +476,8 @@ PropertyMap Frame::asProperties() const // workaround until this function is virtual if(id == "TXXX") return dynamic_cast< const UserTextIdentificationFrame* >(this)->asProperties(); - // Apple proprietary WFED (Podcast URL) is in fact a text frame. - else if(id[0] == 'T' || id == "WFED") + // Apple proprietary WFED (Podcast URL), MVNM (Movement Name), MVIN (Movement Number) are in fact text frames. + else if(id[0] == 'T' || id == "WFED" || id == "MVNM" || id == "MVIN") return dynamic_cast< const TextIdentificationFrame* >(this)->asProperties(); else if(id == "WXXX") return dynamic_cast< const UserUrlLinkFrame* >(this)->asProperties(); diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp index 0fbb87d0..759a9b7b 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.cpp +++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp @@ -198,8 +198,8 @@ Frame *FrameFactory::createFrame(const ByteVector &origData, Header *tagHeader) // Text Identification (frames 4.2) - // Apple proprietary WFED (Podcast URL) is in fact a text frame. - if(frameID.startsWith("T") || frameID == "WFED") { + // Apple proprietary WFED (Podcast URL), MVNM (Movement Name), MVIN (Movement Number) are in fact text frames. + if(frameID.startsWith("T") || frameID == "WFED" || frameID == "MVNM" || frameID == "MVIN") { TextIdentificationFrame *f = frameID != "TXXX" ? new TextIdentificationFrame(data, header) @@ -456,6 +456,8 @@ namespace { "TDS", "TDES" }, { "TID", "TGID" }, { "WFD", "WFED" }, + { "MVN", "MVNM" }, + { "MVI", "MVIN" }, }; const size_t frameConversion2Size = sizeof(frameConversion2) / sizeof(frameConversion2[0]); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 4f58bdd6..27a2189a 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -109,6 +109,7 @@ class TestID3v2 : public CppUnit::TestFixture CPPUNIT_TEST(testW000); CPPUNIT_TEST(testPropertyInterface); CPPUNIT_TEST(testPropertyInterface2); + CPPUNIT_TEST(testPropertiesMovement); CPPUNIT_TEST(testDeleteFrame); CPPUNIT_TEST(testSaveAndStripID3v1ShouldNotAddFrameFromID3v1ToId3v2); CPPUNIT_TEST(testParseChapterFrame); @@ -921,6 +922,47 @@ public: CPPUNIT_ASSERT_EQUAL(frame6, ID3v2::UniqueFileIdentifierFrame::findByOwner(&tag, "http://musicbrainz.org")); } + void testPropertiesMovement() + { + ID3v2::Tag tag; + ID3v2::TextIdentificationFrame *frameMvnm = new ID3v2::TextIdentificationFrame("MVNM"); + frameMvnm->setText("Movement Name"); + tag.addFrame(frameMvnm); + + ID3v2::TextIdentificationFrame *frameMvin = new ID3v2::TextIdentificationFrame("MVIN"); + frameMvin->setText("2/3"); + tag.addFrame(frameMvin); + + PropertyMap properties = tag.properties(); + CPPUNIT_ASSERT(properties.contains("MOVEMENTNAME")); + CPPUNIT_ASSERT(properties.contains("MOVEMENTNUMBER")); + CPPUNIT_ASSERT_EQUAL(String("Movement Name"), properties["MOVEMENTNAME"].front()); + CPPUNIT_ASSERT_EQUAL(String("2/3"), properties["MOVEMENTNUMBER"].front()); + + ByteVector frameDataMvnm("MVNM" + "\x00\x00\x00\x0e" + "\x00\x00" + "\x00" + "Movement Name", 24); + CPPUNIT_ASSERT_EQUAL(frameDataMvnm, frameMvnm->render()); + ByteVector frameDataMvin("MVIN" + "\x00\x00\x00\x04" + "\x00\x00" + "\x00" + "2/3", 14); + CPPUNIT_ASSERT_EQUAL(frameDataMvin, frameMvin->render()); + + ID3v2::FrameFactory *factory = ID3v2::FrameFactory::instance(); + ID3v2::TextIdentificationFrame *parsedFrameMvnm = + dynamic_cast(factory->createFrame(frameDataMvnm)); + ID3v2::TextIdentificationFrame *parsedFrameMvin = + dynamic_cast(factory->createFrame(frameDataMvin)); + CPPUNIT_ASSERT(parsedFrameMvnm); + CPPUNIT_ASSERT(parsedFrameMvin); + CPPUNIT_ASSERT_EQUAL(String("Movement Name"), parsedFrameMvnm->toString()); + CPPUNIT_ASSERT_EQUAL(String("2/3"), parsedFrameMvin->toString()); + } + void testDeleteFrame() { ScopedFileCopy copy("rare_frames", ".mp3"); diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index dd36b21b..ebfb4bab 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -55,6 +55,7 @@ class TestMP4 : public CppUnit::TestFixture CPPUNIT_TEST(testCovrWrite); CPPUNIT_TEST(testCovrRead2); CPPUNIT_TEST(testProperties); + CPPUNIT_TEST(testPropertiesMovement); CPPUNIT_TEST(testFuzzedFile); CPPUNIT_TEST(testRepeatedSave); CPPUNIT_TEST_SUITE_END(); @@ -378,6 +379,58 @@ public: f.setProperties(tags); } + void testPropertiesMovement() + { + MP4::File f(TEST_FILE_PATH_C("has-tags.m4a")); + + PropertyMap tags = f.properties(); + + tags["WORK"] = StringList("Foo"); + tags["MOVEMENTNAME"] = StringList("Bar"); + tags["MOVEMENTNUMBER"] = StringList("2"); + tags["MOVEMENTCOUNT"] = StringList("3"); + tags["SHOWWORKMOVEMENT"] = StringList("1"); + f.setProperties(tags); + + tags = f.properties(); + + CPPUNIT_ASSERT(f.tag()->contains("\251wrk")); + CPPUNIT_ASSERT_EQUAL(StringList("Foo"), f.tag()->item("\251wrk").toStringList()); + CPPUNIT_ASSERT_EQUAL(StringList("Foo"), tags["WORK"]); + + CPPUNIT_ASSERT(f.tag()->contains("\251mvn")); + CPPUNIT_ASSERT_EQUAL(StringList("Bar"), f.tag()->item("\251mvn").toStringList()); + CPPUNIT_ASSERT_EQUAL(StringList("Bar"), tags["MOVEMENTNAME"]); + + CPPUNIT_ASSERT(f.tag()->contains("\251mvi")); + CPPUNIT_ASSERT_EQUAL(2, f.tag()->item("\251mvi").toInt()); + CPPUNIT_ASSERT_EQUAL(StringList("2"), tags["MOVEMENTNUMBER"]); + + CPPUNIT_ASSERT(f.tag()->contains("\251mvc")); + CPPUNIT_ASSERT_EQUAL(3, f.tag()->item("\251mvc").toInt()); + CPPUNIT_ASSERT_EQUAL(StringList("3"), tags["MOVEMENTCOUNT"]); + + CPPUNIT_ASSERT(f.tag()->contains("shwm")); + CPPUNIT_ASSERT_EQUAL(true, f.tag()->item("shwm").toBool()); + CPPUNIT_ASSERT_EQUAL(StringList("1"), tags["SHOWWORKMOVEMENT"]); + + tags["SHOWWORKMOVEMENT"] = StringList("0"); + f.setProperties(tags); + + tags = f.properties(); + + CPPUNIT_ASSERT(f.tag()->contains("shwm")); + CPPUNIT_ASSERT_EQUAL(false, f.tag()->item("shwm").toBool()); + CPPUNIT_ASSERT_EQUAL(StringList("0"), tags["SHOWWORKMOVEMENT"]); + + tags["WORK"] = StringList(); + tags["MOVEMENTNAME"] = StringList(); + tags["MOVEMENTNUMBER"] = StringList(); + tags["MOVEMENTCOUNT"] = StringList(); + tags["SHOWWORKMOVEMENT"] = StringList(); + f.setProperties(tags); + } + void testFuzzedFile() { MP4::File f(TEST_FILE_PATH_C("infloop.m4a")); -- 2.40.0