From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Fri, 13 Nov 2015 02:55:56 +0000 (+0900)
Subject: Skip duplicate ID3v2 tags and treat them as an extra blank of the first one.
X-Git-Tag: v1.11beta~124^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=11fbf394a345dd29b68d6e2d2d90b78471db6ee1;p=taglib

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 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 <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 9ae1dffa..c5f9bbf6 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);
 }
 
diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 5088841f..87863157 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);