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 28b8fca747e60603e989a1b18f6e49ff1b7b4963..6ebff897aa8fccb0d073c3f9a4926bfab06b9c97 100644 (file)
--- 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 605ec457fe71a253a4d63fab11badbc5f4157633..275336555b0ce0f572e41f021e136c054f4a927d 100644 (file)
--- 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);