From: Robin Stocker <robin@nibor.org>
Date: Sun, 10 Jun 2012 16:47:13 +0000 (+0200)
Subject: Don't duplicate from ID3v1 to ID3v2 when saving only ID3v2
X-Git-Tag: v1.8beta~6^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7279b4fb7b30e82a539615869185ff24ebba885c;p=taglib

Don't duplicate from ID3v1 to ID3v2 when saving only ID3v2

When saving only v2 with stripOthers (which means stripping v1), the
data from v1 would still be duplicated to v2. Likewise for the other way
around.

This is not the expected outcome when e.g. a frame was removed in v2,
because it would be added again on save from the v1 data. The test shows
that.

This changes save to only duplicate the data when the other tag type
will not be stripped.
---

diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 28b8fca7..6ebff897 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -207,12 +207,12 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
   }
 
   // Create the tags if we've been asked to.  Copy the values from the tag that
-  // does exist into the new tag.
+  // does exist into the new tag, except if the existing tag is to be stripped.
 
-  if((tags & ID3v2) && ID3v1Tag())
+  if((tags & ID3v2) && ID3v1Tag() && !(stripOthers && !(tags & ID3v1)))
     Tag::duplicate(ID3v1Tag(), ID3v2Tag(true), false);
 
-  if((tags & ID3v1) && d->tag[ID3v2Index])
+  if((tags & ID3v1) && d->tag[ID3v2Index] && !(stripOthers && !(tags & ID3v2)))
     Tag::duplicate(ID3v2Tag(), ID3v1Tag(true), false);
 
   bool success = true;
diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 605ec457..27533655 100644
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -71,6 +71,7 @@ class TestID3v2 : public CppUnit::TestFixture
   CPPUNIT_TEST(testCompressedFrameWithBrokenLength);
   CPPUNIT_TEST(testPropertyInterface);
   CPPUNIT_TEST(testPropertyInterface2);
+  CPPUNIT_TEST(testSaveAndStripID3v1ShouldNotAddFrameFromID3v1ToId3v2);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -628,6 +629,28 @@ public:
     CPPUNIT_ASSERT(tag.frameList("TIPL").isEmpty());
   }
 
+  void testSaveAndStripID3v1ShouldNotAddFrameFromID3v1ToId3v2()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+    string newname = copy.fileName();
+
+    {
+      MPEG::File foo(newname.c_str());
+      foo.tag()->setArtist("Artist");
+      foo.save(MPEG::File::ID3v1 | MPEG::File::ID3v2);
+    }
+
+    {
+      MPEG::File bar(newname.c_str());
+      bar.ID3v2Tag()->removeFrames("TPE1");
+      // Should strip ID3v1 here and not add old values to ID3v2 again
+      bar.save(MPEG::File::ID3v2, true);
+    }
+
+    MPEG::File f(newname.c_str());
+    CPPUNIT_ASSERT(!f.ID3v2Tag()->frameListMap().contains("TPE1"));
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v2);