diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index e41606db..f06cfbad 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -24,22 +24,22 @@ ***************************************************************************/ #ifdef HAVE_CONFIG_H -#include "config.h" +# include "config.h" #endif #include -#include "tfile.h" +#include +#include +#include +#include #include "id3v2tag.h" #include "id3v2header.h" #include "id3v2extendedheader.h" #include "id3v2footer.h" #include "id3v2synchdata.h" -#include "tbytevector.h" #include "id3v1genres.h" -#include "tpropertymap.h" -#include "tdebug.h" #include "frames/textidentificationframe.h" #include "frames/commentsframe.h" @@ -136,7 +136,6 @@ ID3v2::Tag::~Tag() delete d; } - String ID3v2::Tag::title() const { if(!d->frameListMap["TIT2"].isEmpty()) @@ -564,7 +563,6 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const } } - ByteVector ID3v2::Tag::render(int version) const { // We need to render the "tag data" first so that we have to correct size to @@ -667,18 +665,43 @@ void ID3v2::Tag::setLatin1StringHandler(const Latin1StringHandler *handler) void ID3v2::Tag::read() { - if(d->file && d->file->isOpen()) { + if(!d->file) + return; - d->file->seek(d->tagOffset); - d->header.setData(d->file->readBlock(Header::size())); + if(!d->file->isOpen()) + return; - // if the tag size is 0, then this is an invalid tag (tags must contain at - // least one frame) + d->file->seek(d->tagOffset); + d->header.setData(d->file->readBlock(Header::size())); - if(d->header.tagSize() == 0) - return; + // If the tag size is 0, then this is an invalid tag (tags must contain at + // least one frame) + if(d->header.tagSize() != 0) parse(d->file->readBlock(d->header.tagSize())); + + // Look for duplicate ID3v2 tags and treat them as an extra blank of this one. + // It leads to overwriting them with zero when saving the tag. + + // This is a workaround for some faulty files that have duplicate ID3v2 tags. + // Unfortunately, TagLib itself may write such duplicate tags until v1.10. + + uint extraSize = 0; + + while(true) { + + d->file->seek(d->tagOffset + d->header.completeTagSize() + extraSize); + + const ByteVector data = d->file->readBlock(Header::size()); + if(data.size() < Header::size() || !data.startsWith(Header::fileIdentifier())) + break; + + extraSize += Header(data).completeTagSize(); + } + + if(extraSize != 0) { + debug("ID3v2::Tag::read() - Duplicate ID3v2 tags found."); + d->header.setTagSize(d->header.tagSize() + extraSize); } } diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 9ae1dffa..8b7af692 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -446,24 +446,9 @@ long MPEG::File::firstFrameOffset() { long position = 0; - if(hasID3v2Tag()) { + if(hasID3v2Tag()) 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. - - long location; - while((location = findID3v2(position)) >= 0) { - seek(location); - const ID3v2::Header header(readBlock(ID3v2::Header::size())); - position = location + header.completeTagSize(); - - debug("MPEG::File::firstFrameOffset() - Duplicate ID3v2 tag found."); - } - } - return nextFrameOffset(position); } @@ -504,7 +489,7 @@ void MPEG::File::read(bool readProperties) { // Look for an ID3v2 tag - d->ID3v2Location = findID3v2(0); + d->ID3v2Location = findID3v2(); if(d->ID3v2Location >= 0) { @@ -547,115 +532,40 @@ void MPEG::File::read(bool readProperties) ID3v1Tag(true); } -long MPEG::File::findID3v2(long offset) +long MPEG::File::findID3v2() { - // This method is based on the contents of TagLib::File::find(), but because - // of some subtleties -- specifically the need to look for the bit pattern of - // an MPEG sync, it has been modified for use here. + if(!isValid()) + return -1; - if(isValid() && ID3v2::Header::fileIdentifier().size() <= bufferSize()) { + // An ID3v2 tag or MPEG frame is most likely be at the beginning of the file. - // The position in the file that the current buffer starts at. + const ByteVector headerID = ID3v2::Header::fileIdentifier(); - long bufferOffset = 0; - ByteVector buffer; + seek(0); - // These variables are used to keep track of a partial match that happens at - // the end of a buffer. + const ByteVector data = readBlock(headerID.size()); + if(data.size() < headerID.size()) + return -1; - int previousPartialMatch = -1; - bool previousPartialSynchMatch = false; + if(data == headerID) + return 0; - // Save the location of the current read pointer. We will restore the - // position using seek() before all returns. + if(firstSyncByte(data[0]) && secondSynchByte(data[1])) + return -1; - long originalPosition = tell(); + // Look for the entire file, if neither an MEPG frame or ID3v2 tag was found + // at the beginning of the file. + // We don't care about the inefficiency of the code, since this is a seldom case. - // Start the search at the offset. + const long tagOffset = find(headerID); + if(tagOffset < 0) + return -1; - seek(offset); + const long frameOffset = firstFrameOffset(); + if(frameOffset < tagOffset) + return -1; - // This loop is the crux of the find method. There are three cases that we - // want to account for: - // (1) The previously searched buffer contained a partial match of the search - // pattern and we want to see if the next one starts with the remainder of - // that pattern. - // - // (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 iteration, where we will check for the rest - // of the pattern. - - for(buffer = readBlock(bufferSize()); buffer.size() > 0; buffer = readBlock(bufferSize())) { - - // (1) previous partial match - - if(previousPartialSynchMatch && secondSynchByte(buffer[0])) - return -1; - - if(previousPartialMatch >= 0 && int(bufferSize()) > previousPartialMatch) { - const int patternOffset = (bufferSize() - previousPartialMatch); - if(buffer.containsAt(ID3v2::Header::fileIdentifier(), 0, patternOffset)) { - seek(originalPosition); - return offset + bufferOffset - bufferSize() + previousPartialMatch; - } - } - - // (2) pattern contained in current buffer - - long location = buffer.find(ID3v2::Header::fileIdentifier()); - if(location >= 0) { - seek(originalPosition); - return offset + bufferOffset + location; - } - - int firstSynchByte = buffer.find(char(uchar(255))); - - // Here we have to loop because there could be several of the first - // (11111111) byte, and we want to check all such instances until we find - // a full match (11111111 111) or hit the end of the buffer. - - while(firstSynchByte >= 0) { - - // if this *is not* at the end of the buffer - - if(firstSynchByte < int(buffer.size()) - 1) { - if(secondSynchByte(buffer[firstSynchByte + 1])) { - // We've found the frame synch pattern. - seek(originalPosition); - return -1; - } - else { - - // We found 11111111 at the end of the current buffer indicating a - // partial match of the synch pattern. The find() below should - // return -1 and break out of the loop. - - previousPartialSynchMatch = true; - } - } - - // Check in the rest of the buffer. - - firstSynchByte = buffer.find(char(uchar(255)), firstSynchByte + 1); - } - - // (3) partial match - - previousPartialMatch = buffer.endsWithPartialMatch(ID3v2::Header::fileIdentifier()); - - bufferOffset += bufferSize(); - } - - // Since we hit the end of the file, reset the status before continuing. - - clear(); - - seek(originalPosition); - } - - return -1; + return tagOffset; } long MPEG::File::findID3v1() diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index 858a6a5c..8478d093 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -390,7 +390,7 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties); - long findID3v2(long offset); + long findID3v2(); long findID3v1(); void findAPE(); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 036ab2c7..612dc424 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -93,6 +93,7 @@ class TestID3v2 : public CppUnit::TestFixture CPPUNIT_TEST(testRenderTableOfContentsFrame); CPPUNIT_TEST(testShrinkPadding); CPPUNIT_TEST(testEmptyFrame); + CPPUNIT_TEST(testDuplicateTags); CPPUNIT_TEST_SUITE_END(); public: @@ -1117,6 +1118,41 @@ public: } } + void testDuplicateTags() + { + ScopedFileCopy copy("duplicate_id3v2", ".mp3"); + + ByteVector audioStream; + { + MPEG::File f(copy.fileName().c_str()); + f.seek(f.ID3v2Tag()->header()->completeTagSize()); + audioStream = f.readBlock(2089); + + // duplicate_id3v2.mp3 has duplicate ID3v2 tags. + // Sample rate will be 32000 if we can't skip the second tag. + + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL((TagLib::uint)8049, f.ID3v2Tag()->header()->completeTagSize()); + + CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); + + f.ID3v2Tag()->setArtist("Artist A"); + f.save(MPEG::File::ID3v2, true); + } + { + MPEG::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL((long)3594, f.length()); + CPPUNIT_ASSERT_EQUAL((TagLib::uint)1505, f.ID3v2Tag()->header()->completeTagSize()); + CPPUNIT_ASSERT_EQUAL(String("Artist A"), f.ID3v2Tag()->artist()); + CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); + + f.seek(f.ID3v2Tag()->header()->completeTagSize()); + CPPUNIT_ASSERT_EQUAL(f.readBlock(2089), audioStream); + + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v2);