From e41dc68a6bc840dad5d9372dfadf25d4439f85aa Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 20 Oct 2014 21:21:32 +0900 Subject: [PATCH 1/4] Skip duplicate ID3v2 tags in MPEG files. --- taglib/mpeg/mpegfile.cpp | 27 +++++++++++++++++++-------- taglib/mpeg/mpegfile.h | 14 +++++++------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index f765befb..e64e8303 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -433,9 +433,20 @@ long MPEG::File::firstFrameOffset() { long position = 0; - if(ID3v2Tag()) + if(ID3v2Tag()) { + // Skip duplicate ID3v2 tags. + position = d->ID3v2Location + ID3v2Tag()->header()->completeTagSize(); + long location; + while((location = findID3v2(position)) >= 0) { + ID3v2::Tag dupTag(this, location); + position = location + dupTag.header()->completeTagSize(); + + debug("MPEG::File::firstFrameOffset() - Duplicate ID3v2 tag found."); + } + } + return nextFrameOffset(position); } @@ -467,7 +478,7 @@ void MPEG::File::read(bool readProperties, Properties::ReadStyle propertiesStyle { // Look for an ID3v2 tag - d->ID3v2Location = findID3v2(); + d->ID3v2Location = findID3v2(0); if(d->ID3v2Location >= 0) { @@ -510,7 +521,7 @@ void MPEG::File::read(bool readProperties, Properties::ReadStyle propertiesStyle ID3v1Tag(true); } -long MPEG::File::findID3v2() +long MPEG::File::findID3v2(long offset) { // This method is based on the contents of TagLib::File::find(), but because // of some subtlteies -- specifically the need to look for the bit pattern of @@ -534,9 +545,9 @@ long MPEG::File::findID3v2() long originalPosition = tell(); - // Start the search at the beginning of the file. + // Start the search at the offset. - seek(0); + seek(offset); // This loop is the crux of the find method. There are three cases that we // want to account for: @@ -547,7 +558,7 @@ long MPEG::File::findID3v2() // (2) The search pattern is wholly contained within the current buffer. // // (3) The current buffer ends with a partial match of the pattern. We will - // note this for use in the next itteration, where we will check for the rest + // note this for use in the next iteration, where we will check for the rest // of the pattern. for(buffer = readBlock(bufferSize()); buffer.size() > 0; buffer = readBlock(bufferSize())) { @@ -561,7 +572,7 @@ long MPEG::File::findID3v2() const int patternOffset = (bufferSize() - previousPartialMatch); if(buffer.containsAt(ID3v2::Header::fileIdentifier(), 0, patternOffset)) { seek(originalPosition); - return bufferOffset - bufferSize() + previousPartialMatch; + return offset + bufferOffset - bufferSize() + previousPartialMatch; } } @@ -570,7 +581,7 @@ long MPEG::File::findID3v2() long location = buffer.find(ID3v2::Header::fileIdentifier()); if(location >= 0) { seek(originalPosition); - return bufferOffset + location; + return offset + bufferOffset + location; } int firstSynchByte = buffer.find(char(uchar(255))); diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index 3fc01e68..a03887a3 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -242,8 +242,8 @@ namespace TagLib { * if there is no valid ID3v2 tag. If \a create is true it will create * an ID3v2 tag if one does not exist and returns a valid pointer. * - * \note This may return a valid pointer regardless of whether or not the - * file on disk has an ID3v2 tag. Use hasID3v2Tag() to check if the file + * \note This may return a valid pointer regardless of whether or not the + * file on disk has an ID3v2 tag. Use hasID3v2Tag() to check if the file * on disk actually has an ID3v2 tag. * * \note The Tag is still owned by the MPEG::File and should not be @@ -261,8 +261,8 @@ namespace TagLib { * if there is no valid ID3v1 tag. If \a create is true it will create * an ID3v1 tag if one does not exist and returns a valid pointer. * - * \note This may return a valid pointer regardless of whether or not the - * file on disk has an ID3v1 tag. Use hasID3v1Tag() to check if the file + * \note This may return a valid pointer regardless of whether or not the + * file on disk has an ID3v1 tag. Use hasID3v1Tag() to check if the file * on disk actually has an ID3v1 tag. * * \note The Tag is still owned by the MPEG::File and should not be @@ -280,8 +280,8 @@ namespace TagLib { * if there is no valid APE tag. If \a create is true it will create * an APE tag if one does not exist and returns a valid pointer. * - * \note This may return a valid pointer regardless of whether or not the - * file on disk has an APE tag. Use hasAPETag() to check if the file + * \note This may return a valid pointer regardless of whether or not the + * file on disk has an APE tag. Use hasAPETag() to check if the file * on disk actually has an APE tag. * * \note The Tag is still owned by the MPEG::File and should not be @@ -370,7 +370,7 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties, Properties::ReadStyle propertiesStyle); - long findID3v2(); + long findID3v2(long offset); long findID3v1(); void findAPE(); From 71acf3b6f705dbcc9c760d2fec664be9d68c5a49 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 20 Oct 2014 23:13:15 +0900 Subject: [PATCH 2/4] Comment on a weird workaround for duplicate ID3v2 tags. --- taglib/mpeg/mpegfile.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index e64e8303..5f953ccb 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -434,8 +434,12 @@ long MPEG::File::firstFrameOffset() long position = 0; if(ID3v2Tag()) { + // Skip duplicate ID3v2 tags. + // Workaround for some faulty files that have duplicate ID3v2 tags. + // Combination of EAC and LAME creates such files when configured incorrectly. + position = d->ID3v2Location + ID3v2Tag()->header()->completeTagSize(); long location; From 269e78f1a013a9673c62075a68d38a14d47fbf34 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 21 Oct 2014 00:16:43 +0900 Subject: [PATCH 3/4] Add a test for duplicate ID3v2 tags. --- tests/data/duplicate_id3v2.mp3 | Bin 0 -> 10138 bytes tests/test_mpeg.cpp | 14 ++++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 tests/data/duplicate_id3v2.mp3 diff --git a/tests/data/duplicate_id3v2.mp3 b/tests/data/duplicate_id3v2.mp3 new file mode 100644 index 0000000000000000000000000000000000000000..34f4f158ce454d71677f450d1db8c40abbb85d30 GIT binary patch literal 10138 zcmeHMc~p~E7Jo@3VOJ2atcINck%Vjn#1=@vpg;&gENC?$0mG6I78L|>Y;nN`3tC(l zD^O^qQp5$+PPGaswTKoK7u2aB!)O(kiddI6bHA|IGjpcX{xRqD^yJHV$@jf`-(7z9 zCFlNL1Uv=-!R54A95O!?Zmbc)X@3qu8AyuqP$pV9{t^rMbP&%N7iXhfq(C|FKB*uO z`%kVHgD@9t$S1`@kzfIcJrPQX@=J&cQ)OhxlXDd+Wkj|rFH68fndvv5Bvjvft5G(? z!)9n{&G>>+-GX?fLY*fMS7gWqGPyEWk)n`iqYxB@B9H_v0cSE4j6{eGFFK+jE@C4l zV!&t6xK`GrmQbfPrpW-U02mWmxszIAoh3S0^VeCVAr3@89+Q$E0*D z#VENeBcMk>kANNlJpy_J z^a$t?&?BHn;QxYvfXA4E873p~7)=H)4KzCvsbDGwDS;^Gz+F5_1=5@kcOfVR2(%n! z!@a-mISP$aaNw7)bUp|%6*5Wy`m2P6WE2i$I0q$<6Z0G-7KP0Nrv@-Mesuac!?&Mg z_!+u23UDF^0*$%P0w*zXppQjM_oIt>qA<7{WBqjC(_CP~RAAT>@8YnSXizZG*%ZQw zffA)7aTK2ila9Jc9)yz$BNu|h97gG!Dd0)GaN$Bw8;d6ng#m#VAR!XN9G0I1mMABb z>1;Ml3jSZX*x z9!m9F%!Gi0i4&FASieeC3d268HVvUHWfa>is5cn<&^=rEE& zhdq&1{{IN){~7`>Mg$=J%1(-l!Oo9<$L<#Lf9C?-e#4P|6$KFhjGbFy-y}KIIsyuV zZMm#pY&JTSVq#OtbS04JpdQ<7GJu>0Ws$<>^4AqD0IacKV9x=%IXXPkP>4_f->zwk zA#FG!N|pPiWyw5=le@LlTK(WMpV;YGr0> zVq$7*Ien^?quop=M>_`xvfC_AvWu^)gM$}`;!C45nar7<+~5HJ*|Qi-f6NHZ$jHdl z#Pkg_vp4*m9i07t_oY37EDYhdCY*qCL3j%s!2+l4Kn@7u@dSwW^+hxw;RsXkh9C`{ zhHwPD0Wu&Nngv6cH z_90Wy!CMe4(3>bEf3W$Rn?iTW4ECPy1`KznJi9Z8b!CGJ_xPvw{z$TDTIYl51V$)ba7mmiE-^tgAXU0-ATdxslahmX8)O}^Q!mfyEaaeL8gK@l!noFlV8*zYcKWD=Dx;jyyuIuul%FHIbIVH z{=MQ>z^%gc^IbiwyC3$=VbL1x%`57|^XKmm4s!Q%KV9r@{wDp|u7~p&v>KzxCu^G5 z)=qokZO)F#>-@Rrzy@{Kk`M1Z2zV89uy}N{hA^`>YICe^Hcx~+NFtcEzJ6~cLZMaYF+jq!D!Bf%qsjLXC!=b_E#6qP;I>y6lnniq#} zC4Htkd2E5{bN{mkzuR_E8tNN=yahe^Y-D{bE-_;D%a7U@5KRVO%=Z1_=cx><<>D0) zi-U!@q1)-2`}wnt50>v{{FQcuyO|VQwByL;2ubqf`dj=!(U@OC@xGH+ddN$#xXU0$;nlM_!@Wi2{iS<`+Oist?gpHG+VsX26? zT0{2gdp11uQzic5?0|mTVnW+sSI6zPBqJIAT9Kq6v$dL)5fFa~XaDf3`{U6D~`!=8!J?NWB%sDmm#E$5Z^^UFKB9?$SHAj+ys@(y2UIuK!B+?#Rs zXeZ-~>lu4Zw^^_E9crzpTVd}kxVp)#twkB#)^_T|?DmsY@0Ofd|1Qhq+>+3_(j<1= zGMA3P@bBN8w=?AYb^UVYIyj~Er#U|4 zPuE%=a~mC9@P09^`)pj*7Di}+M@3gutM69wa+3$ERxmB+cRUR*>MQiBo!M~k!Nq>Q zt8et?klc?e7`}HmJRUYoDDgg1{K-Pkt((->qBZ>w2Fhv)b_EKazb3q0)H>s*^!2ts zsCLjcYECWk3&`uBEVDRwEW~A~y{hcN_2B|(88LZo!UEf3zRzWq)TfDU9VoThQ~pk& zv->O#^mh8LQ?0}%#$Z{DGAOR9gYHHNNH=zL z8L>`_-x!&hoR#@ivz>au%3Y3126u$A(a^hOPEe7ksQP8TRu{W}+*{^8*dhMDvG zFPo5Ysb{W>G))07+=i8ZPxG9r&Iq})-{w*y_ho}k{73)}_ZDH8JgT~1{g(mWiv7I8 zPfPoG-S771^Vz)0sFSmmq?PyGMl(N4s36gO-M@_U6Q~91z#bcEs5tzUSLX4{NAR6H z%T6#3RokD_hLD_n!dQC-NrtAZ%M0rmAs!Z|%2()BQ$^7PTJx`5@ozaUkFPQg(!u!o02W?4EnyKJhVU z?C&njg*TguGp!Q#_8D)b#49e9=GT2q91=(ik9G$pZK3jG zsK|_0RY7g$JZMqo*mm5AP5kHAK4ko{)L4S%d2K1HesEv*!PV-K_dRWPZt$6vq4$~UuuIMA$aOK7v}pUQWwjbbzS%e3&pR)$S@sampleRate()); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG); From 73b9b9b58d5bfee2ea9baba62b4db5d92cb4b42f Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 23 Oct 2014 08:14:10 +0900 Subject: [PATCH 4/4] Avoid reading an entire ID3v2 tag when skipping it. --- taglib/mpeg/mpegfile.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 5f953ccb..0ac87d10 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -434,18 +434,18 @@ long MPEG::File::firstFrameOffset() long position = 0; if(ID3v2Tag()) { + position = d->ID3v2Location + ID3v2Tag()->header()->completeTagSize(); // Skip duplicate ID3v2 tags. // Workaround for some faulty files that have duplicate ID3v2 tags. // Combination of EAC and LAME creates such files when configured incorrectly. - position = d->ID3v2Location + ID3v2Tag()->header()->completeTagSize(); - long location; while((location = findID3v2(position)) >= 0) { - ID3v2::Tag dupTag(this, location); - position = location + dupTag.header()->completeTagSize(); + seek(location); + const ID3v2::Header header(readBlock(ID3v2::Header::size())); + position = location + header.completeTagSize(); debug("MPEG::File::firstFrameOffset() - Duplicate ID3v2 tag found."); }