Skip duplicate ID3v2 tags and treat them as an extra blank of the first one.

This enables all the file formats to handle duplicate ID3v2 tags properly and erase them automatically.

diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp
index e41606db462f066eff86adb35e65ad8dfa619ada..f06cfbadac9b2b45d5edf2f580a58c31dfc5cbfc 100644 (file)
--- 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 <algorithm>
 
-#include "tfile.h"
+#include <tfile.h>
+#include <tbytevector.h>
+#include <tpropertymap.h>
+#include <tdebug.h>
 
 #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 9ae1dffaba4574697cb7d0425530436ebecbdb06..c5f9bbf67eb57a3e32abbd93364d3c4de4db5219 100644 (file)
--- 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);
 }
 

diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 5088841f6718a0755c5724f4ddd440b423094652..8786315786ae6bc0135b20b4f2506175abec37c5 100644 (file)
--- 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);