From 1b64bb0cb73435555527dd625d9b14dbd3e28f14 Mon Sep 17 00:00:00 2001 From: Urs Fleisch Date: Tue, 18 Oct 2016 20:34:53 +0200 Subject: [PATCH 1/9] Support new classical music frames introduced with iTunes 12.5, #758. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 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")); From f5ca097379b270d1f92368e283d30f8703161645 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 2 Nov 2016 15:44:50 +0900 Subject: [PATCH 2/9] Proper handling of MP4 atoms with zero length. If the size of an atom is 0, it designates the last atom which extends to the end of the file. --- taglib/mp4/mp4atom.cpp | 10 ++++++++-- tests/data/zero-length-mdat.m4a | Bin 0 -> 4517 bytes tests/test_mp4.cpp | 10 ++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/data/zero-length-mdat.m4a diff --git a/taglib/mp4/mp4atom.cpp b/taglib/mp4/mp4atom.cpp index 5758173b..20709212 100644 --- a/taglib/mp4/mp4atom.cpp +++ b/taglib/mp4/mp4atom.cpp @@ -54,10 +54,15 @@ MP4::Atom::Atom(File *file) length = header.toUInt(); - if(length == 1) { + if(length == 0) { + // The last atom which extends to the end of the file. + length = file->length() - offset; + } + else if(length == 1) { + // The atom has a 64-bit length. const long long longLength = file->readBlock(8).toLongLong(); if(longLength <= LONG_MAX) { - // The atom has a 64-bit length, but it's actually a 31-bit value or long is 64-bit. + // The actual length fits in long. That's always the case if long is 64-bit. length = static_cast(longLength); } else { @@ -67,6 +72,7 @@ MP4::Atom::Atom(File *file) return; } } + if(length < 8) { debug("MP4: Invalid atom size"); length = 0; diff --git a/tests/data/zero-length-mdat.m4a b/tests/data/zero-length-mdat.m4a new file mode 100644 index 0000000000000000000000000000000000000000..578d2ef7a8b000addc1e6d5ad9a6b12211f840b9 GIT binary patch literal 4517 zcmeG;i$9ck_ups6W!%OL;~II!{T9*`y@`=guH%+Mv?en$xy_DI>7wyQB-MymGL%9T zwj^!s3M(n)6;f?CiByszmCfb-K4aVMx4+-#_Yb`9nRDix@8$WP%lDk~JOBVRQjo-p z<b1aF!%ipkuB_*iTV0Ny(`K7K0{Vq&*Ov9Xz6$EJLY0P>v(AnAPo0T})T|6>5B z|2C%owe$ayZ~)*O1bkLBTnrUNOLWTqsq^b;nBH$dqT+9Q{(rs5{a7}a1&=^u*?;p3 ztkVY|90ayX6wspBG5oKp65giT-; zL$K#0uoEx=FBA$*AeVw%i>?L`73fhr0PE?}a7I=&W8F}h8<2rA0^Y|;P=5_UmY0bS^d6c z?w-z2?W5l`L@pz*Ev)ZGg`8@b)UAs;Qe_fsTBEMA^)wWNd{?l8MMLs?+6 z+I+7}MNK>I<;jOU&^D#+^ZiFge z>*G&v8(h!CRlh$o9r7^f#}_B}L^SOX zb-Dt&N{J+7(8(l{!YvY3)(HVUW?6=Vok zkRJ&HnjAXPodo0u|eek<` zspqxi_%XXKUAp68vcg4tqO-cD`k*>(!(ijYU?aDDdTi>l^<%pS<%QhxLUvAkg)t^@ z^J(|3xaFZe;SGh{A2+J9Y;UWZoVU9WQOExHa^~c@SoIqf9Z`?BoMqoKs*{8uTxf-bKpQhsNlVe4qC8z0uw2Y0rQ~_YFEaeu;01qA zfnM`ZI@ecKzHB#1YaI$I3f#A2>+-V+>uv^`tu6D*VdT2go0^)Qo{5SGZ)#7;ULeDW z&r}#v)~f2rGdrqr^10~?dYbI%lEPCQTf|9)qHm8=6Pap*((`M zKMI~(*}9BpuT*}TnSN|$Y`1Ck%*e{onb9wD+im)>>P#9Id&v58d*!NJ`G%=Xs-TPd zAiU?eFA}DS$}}NOh_NL1bo_B@DYCP>ADJ~tQTD4pB(+KLWLb>&E5q|kl3hiP_o@$R z>sgm{40HOzzQI*3*N5!sxWpwuifoALpG^g-M|;jXlY?r!%=My({pWS)uI)y&N4{G} z+=Iigl+*&2FeSfLI^cmdS{3Pf%{N?8zU>xu<49V2JB7&;juN{K zPSX1kDUuAKRSq0LU&((+Qn9CiVnHEP4V#ULn265e5%yFcuRQ)rUIfHRnaZeG)F~_ z92mHiM3weEC8Y-OC%tc(w@s3agG)(N5Q|&auDeSueQfE$F*j zS9j~p>>i(rl4DPOR>tpM{`k@Lpf-mE5p6p=m6zH2?;@Bq7e~8mJ`Ld)m6_cnbnfI0 zUtIn^@@Dt>Ct5odJ2rdxIUu>O2;|Fsn>`3Q!5fSPBlkX$=CfRg2QbUg`S|3*9#|dd z|ImsMhV$)apLcXOdF^v$Fz{Amm}xVt*()1IllN>Zd3@zNi_yl1qbry_1{cR0dkhS) zItu8l?3GkhF|YD&Z&GJvXJzYav4?m~aem|b#`TR)wguGN4IZ?+Co@xTwsnr{8JK8A zOXA8KYk9vvSgVcua6rD0JQ|vx`FvKz5FYxezj5F{RXHPdTp+{C?{}Ek&8m$c)d9>zOs;uzy@;|9H-Fyz@s4 z&P}Z%xhvg0b+p#ann$1;$>*T{Y^mEW=Jn!1RYLh{qS5iAEza6qav}zK3+$k1 zZeO~3F~yCHmJ%#-QmIKlps)MWx>BXAH-3rmS>v=`=2oS9U4QAD=pMyfyG>eebC<)L#b|Lz@w{e{yO|yF|}gcMjEO1nu8< ztxv>2Tw>buepELOs%mdpoF#J*FO<>;iZFyW$8J7m7{s>f@^h%iPbf3zM#}PcG`!Px zEbN(k<@Dlp+oj9Xhl-zc(CFx&-@7hbOH)Xa(SGA>|K`KXJfcl-s;Q{~j%A-e4J_XG z<=o!tDDoS#sKR}r3*%n~`-ENRTQE=TEIxSlYJlTo&qLALl~Z1oTe2!?m-30pG}Vul zTZ1pB&jux~51;i}8RL{c-cT|%aH#3m>9Q))ec71y#QkpgGw zuiE4lvViL<|MF*FC7lCCWI-)S5>jUrY(;UEQE~g?2 zLpO)Z_j;q{1AbdZ3}o(ab*0Va&cyGPE2y9!$6dCeV+{m!eZr4z*sL+VYxhaI5J3Q9 HgBSiA+4#i{ literal 0 HcmV?d00001 diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index dd36b21b..bc0ac01d 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -57,6 +57,7 @@ class TestMP4 : public CppUnit::TestFixture CPPUNIT_TEST(testProperties); CPPUNIT_TEST(testFuzzedFile); CPPUNIT_TEST(testRepeatedSave); + CPPUNIT_TEST(testWithZeroLengthAtom); CPPUNIT_TEST_SUITE_END(); public: @@ -395,6 +396,15 @@ public: CPPUNIT_ASSERT_EQUAL(2862L, f.find("0123456789")); CPPUNIT_ASSERT_EQUAL(-1L, f.find("0123456789", 2863)); } + + void testWithZeroLengthAtom() + { + MP4::File f(TEST_FILE_PATH_C("zero-length-mdat.m4a")); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT_EQUAL(1115, f.audioProperties()->lengthInMilliseconds()); + CPPUNIT_ASSERT_EQUAL(22050, f.audioProperties()->sampleRate()); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMP4); From da9df1b2a8c18b1e61f6846aa68b4682e4dc1067 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 7 Nov 2016 00:42:12 +0900 Subject: [PATCH 3/9] Values of FILE_* macros are guaranteed in Win32. --- taglib/toolkit/tfilestream.cpp | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/taglib/toolkit/tfilestream.cpp b/taglib/toolkit/tfilestream.cpp index 4f522a62..219913af 100644 --- a/taglib/toolkit/tfilestream.cpp +++ b/taglib/toolkit/tfilestream.cpp @@ -346,18 +346,7 @@ void FileStream::seek(long offset, Position p) #ifdef _WIN32 - DWORD whence; - switch(p) { - case Beginning: - whence = FILE_BEGIN; - break; - case Current: - whence = FILE_CURRENT; - break; - case End: - whence = FILE_END; - break; - default: + if(p != Beginning && p != Current && p != End) { debug("FileStream::seek() -- Invalid Position value."); return; } @@ -365,7 +354,7 @@ void FileStream::seek(long offset, Position p) LARGE_INTEGER liOffset; liOffset.QuadPart = offset; - if(!SetFilePointerEx(d->file, liOffset, NULL, whence)) { + if(!SetFilePointerEx(d->file, liOffset, NULL, static_cast(p))) { debug("FileStream::seek() -- Failed to set the file pointer."); } From aae23f3c07b08580436c235a9da166232a75fa3a Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 7 Nov 2016 14:12:38 +0900 Subject: [PATCH 4/9] Initialize all the data members of ID3v2::ChapterFrame. --- taglib/mpeg/id3v2/frames/chapterframe.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/taglib/mpeg/id3v2/frames/chapterframe.cpp b/taglib/mpeg/id3v2/frames/chapterframe.cpp index d45af5fb..e5c92b0e 100644 --- a/taglib/mpeg/id3v2/frames/chapterframe.cpp +++ b/taglib/mpeg/id3v2/frames/chapterframe.cpp @@ -37,7 +37,11 @@ class ChapterFrame::ChapterFramePrivate { public: ChapterFramePrivate() : - tagHeader(0) + tagHeader(0), + startTime(0), + endTime(0), + startOffset(0), + endOffset(0) { embeddedFrameList.setAutoDelete(true); } From e390cbac528c66613ef44116a0460beaea85eb93 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 8 Nov 2016 21:17:00 +0900 Subject: [PATCH 5/9] Update NEWS. --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 3e5b192b..3c74bc45 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,9 @@ ============================ * Added support for WinRT. + * Added support for classical music tags of iTunes 12.5. * Dropped support for Windows 9x and NT 4.0 or older. + * Fixed reading MP4 atoms with zero length. * Several smaller bug fixes and performance improvements. TagLib 1.11.1 (Oct 24, 2016) From d81d894d41f8f9b916cdea70b4fdb30adb49a0b4 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 8 Nov 2016 21:39:53 +0900 Subject: [PATCH 6/9] tolower() depends on the current locale. It's much easier to write our own function than to use locales properly. --- taglib/toolkit/tutils.h | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/taglib/toolkit/tutils.h b/taglib/toolkit/tutils.h index e3e4f6c3..365cfcab 100644 --- a/taglib/toolkit/tutils.h +++ b/taglib/toolkit/tutils.h @@ -222,15 +222,27 @@ namespace TagLib } /*! - * Returns whether the two strings s1 and s2 are equal, ignoring the case of - * the characters. + * Converts the letter c to lower case, not depending on the locale. + */ + inline int toLowerCase(char c) + { + if('A' <= c && c <= 'Z') + return c + ('a' - 'A'); + else + return c; + } + + /*! + * Returns whether the two strings s1 and s2 are equal, ignoring the case + * of the characters. This only supports US-ASCII and does not depend on + * the current locale. * * We took the trouble to define this one here, since there are some * incompatible variations of case insensitive strcmp(). */ inline bool equalsIgnoreCase(const char *s1, const char *s2) { - while(*s1 != '\0' && *s2 != '\0' && ::tolower(*s1) == ::tolower(*s2)) { + while(*s1 != '\0' && *s2 != '\0' && toLowerCase(*s1) == toLowerCase(*s2)) { s1++; s2++; } From 56cd3695f73869b3e102a7f5a3aecad62caa2cc2 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 8 Nov 2016 22:50:36 +0900 Subject: [PATCH 7/9] Add README.md. --- README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 00000000..d976db3d --- /dev/null +++ b/README.md @@ -0,0 +1,26 @@ +# TagLib + +[![Build Status](https://travis-ci.org/taglib/taglib.svg?branch=master)](https://travis-ci.org/taglib/taglib) + +### TagLib Audio Meta-Data Library + +http://taglib.org/ + +TagLib is a library for reading and editing the meta-data of several +popular audio formats. Currently it supports both ID3v1 and [ID3v2][] +for MP3 files, [Ogg Vorbis][] comments and ID3 tags and Vorbis comments +in [FLAC][], MPC, Speex, WavPack, TrueAudio, WAV, AIFF, MP4 and ASF +files. + +TagLib is distributed under the [GNU Lesser General Public License][] +(LGPL) and [Mozilla Public License][] (MPL). Essentially that means that +it may be used in proprietary applications, but if changes are made to +TagLib they must be contributed back to the project. Please review the +licenses if you are considering using TagLib in your project. + + [ID3v2]: http://www.id3.org + [Ogg Vorbis]: http://vorbis.com/ + [FLAC]: https://xiph.org/flac/ + [GNU Lesser General Public License]: http://www.gnu.org/licenses/lgpl.html + [Mozilla Public License]: http://www.mozilla.org/MPL/MPL-1.1.html + From 9d58e9f8e8e8734a331b69d4038c981d7351c132 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 8 Nov 2016 23:27:55 +0900 Subject: [PATCH 8/9] Removed a utility function which is used only at one place. --- taglib/ape/apetag.cpp | 24 +++++++++++++----------- taglib/toolkit/tutils.h | 29 ----------------------------- 2 files changed, 13 insertions(+), 40 deletions(-) diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index 89ef8ff4..359fc302 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -47,23 +47,23 @@ using namespace APE; namespace { - bool isKeyValid(const char *key, size_t length) + const unsigned int MinKeyLength = 2; + const unsigned int MaxKeyLength = 255; + + bool isKeyValid(const ByteVector &key) { const char *invalidKeys[] = { "ID3", "TAG", "OGGS", "MP+", 0 }; - if(length < 2 || length > 255) - return false; - // only allow printable ASCII including space (32..126) - for(const char *p = key; p < key + length; ++p) { - const int c = static_cast(*p); + for(ByteVector::ConstIterator it = key.begin(); it != key.end(); ++it) { + const int c = static_cast(*it); if(c < 32 || c > 126) return false; } for(size_t i = 0; invalidKeys[i] != 0; ++i) { - if(Utils::equalsIgnoreCase(key, invalidKeys[i])) + if(String(key).upper() == invalidKeys[i]) return false; } @@ -296,11 +296,10 @@ PropertyMap APE::Tag::setProperties(const PropertyMap &origProps) bool APE::Tag::checkKey(const String &key) { - if(!key.isLatin1()) + if(key.size() < MinKeyLength || key.size() > MaxKeyLength) return false; - const std::string data = key.to8Bit(false); - return isKeyValid(data.c_str(), data.size()); + return isKeyValid(key.data(String::Latin1)); } APE::Footer *APE::Tag::footer() const @@ -419,7 +418,10 @@ void APE::Tag::parse(const ByteVector &data) const unsigned int keyLength = nullPos - pos - 8; const unsigned int valLegnth = data.toUInt(pos, false); - if(isKeyValid(&data[pos + 8], keyLength)){ + if(keyLength >= MinKeyLength + && keyLength <= MaxKeyLength + && isKeyValid(data.mid(pos + 8, keyLength))) + { APE::Item item; item.parse(data.mid(pos)); diff --git a/taglib/toolkit/tutils.h b/taglib/toolkit/tutils.h index 365cfcab..d4051f64 100644 --- a/taglib/toolkit/tutils.h +++ b/taglib/toolkit/tutils.h @@ -221,35 +221,6 @@ namespace TagLib return String(); } - /*! - * Converts the letter c to lower case, not depending on the locale. - */ - inline int toLowerCase(char c) - { - if('A' <= c && c <= 'Z') - return c + ('a' - 'A'); - else - return c; - } - - /*! - * Returns whether the two strings s1 and s2 are equal, ignoring the case - * of the characters. This only supports US-ASCII and does not depend on - * the current locale. - * - * We took the trouble to define this one here, since there are some - * incompatible variations of case insensitive strcmp(). - */ - inline bool equalsIgnoreCase(const char *s1, const char *s2) - { - while(*s1 != '\0' && *s2 != '\0' && toLowerCase(*s1) == toLowerCase(*s2)) { - s1++; - s2++; - } - - return (*s1 == '\0' && *s2 == '\0'); - } - /*! * The types of byte order of the running system. */ From 499f6db9776c9d6f428814af801f84bf93227eab Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 9 Nov 2016 00:28:35 +0900 Subject: [PATCH 9/9] Check invalid Unicode APE keys properly. --- taglib/ape/apetag.cpp | 2 +- tests/test_apetag.cpp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index 359fc302..c31237eb 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -299,7 +299,7 @@ bool APE::Tag::checkKey(const String &key) if(key.size() < MinKeyLength || key.size() > MaxKeyLength) return false; - return isKeyValid(key.data(String::Latin1)); + return isKeyValid(key.data(String::UTF8)); } APE::Footer *APE::Tag::footer() const diff --git a/tests/test_apetag.cpp b/tests/test_apetag.cpp index 653de750..577ec4b0 100644 --- a/tests/test_apetag.cpp +++ b/tests/test_apetag.cpp @@ -114,18 +114,21 @@ public: PropertyMap properties; properties["A"] = String("invalid key: one character"); properties["MP+"] = String("invalid key: forbidden string"); + properties[L"\x1234\x3456"] = String("invalid key: Unicode"); properties["A B~C"] = String("valid key: space and tilde"); properties["ARTIST"] = String("valid key: normal one"); APE::Tag tag; PropertyMap unsuccessful = tag.setProperties(properties); - CPPUNIT_ASSERT_EQUAL((unsigned int)2, unsuccessful.size()); + CPPUNIT_ASSERT_EQUAL((unsigned int)3, unsuccessful.size()); CPPUNIT_ASSERT(unsuccessful.contains("A")); CPPUNIT_ASSERT(unsuccessful.contains("MP+")); + CPPUNIT_ASSERT(unsuccessful.contains(L"\x1234\x3456")); CPPUNIT_ASSERT_EQUAL((unsigned int)2, tag.itemListMap().size()); tag.addValue("VALID KEY", "Test Value 1"); tag.addValue("INVALID KEY \x7f", "Test Value 2"); + tag.addValue(L"INVALID KEY \x1234\x3456", "Test Value 3"); CPPUNIT_ASSERT_EQUAL((unsigned int)3, tag.itemListMap().size()); }