Unify File::save(...) APIs between file formats that support ID3v2

Closes #922

6 files changed, 84 insertions, 7 deletions

diff --git a/taglib/riff/aiff/aifffile.cpp b/taglib/riff/aiff/aifffile.cpp
index 4f9c868e..6ef4c9be 100644
--- a/taglib/riff/aiff/aifffile.cpp
+++ b/taglib/riff/aiff/aifffile.cpp
@@ -118,6 +118,11 @@ RIFF::AIFF::Properties *RIFF::AIFF::File::audioProperties() const
 
 bool RIFF::AIFF::File::save()
 {
+    return save(ID3v2::v4);
+}
+
+bool RIFF::AIFF::File::save(ID3v2::Version version)
+{
   if(readOnly()) {
     debug("RIFF::AIFF::File::save() -- File is read only.");
     return false;
@@ -135,7 +140,7 @@ bool RIFF::AIFF::File::save()
   }
 
   if(tag() && !tag()->isEmpty()) {
-    setChunkData("ID3 ", d->tag->render());
+    setChunkData("ID3 ", d->tag->render(version));
     d->hasID3v2 = true;
   }
 
diff --git a/taglib/riff/aiff/aifffile.h b/taglib/riff/aiff/aifffile.h
index 5ba1a279..ceac5769 100644
--- a/taglib/riff/aiff/aifffile.h
+++ b/taglib/riff/aiff/aifffile.h
@@ -120,6 +120,11 @@ namespace TagLib {
         virtual bool save();
 
         /*!
+         * Save using a specific ID3v2 version (e.g. v3)
+         */
+        bool save(ID3v2::Version version);
+
+        /*!
          * Returns whether or not the file on disk actually has an ID3v2 tag.
          *
          * \see ID3v2Tag()
diff --git a/taglib/riff/wav/wavfile.cpp b/taglib/riff/wav/wavfile.cpp
index 0ebe21c3..3dce70b1 100644
--- a/taglib/riff/wav/wavfile.cpp
+++ b/taglib/riff/wav/wavfile.cpp
@@ -152,6 +152,13 @@ bool RIFF::WAV::File::save()
 
 bool RIFF::WAV::File::save(TagTypes tags, bool stripOthers, int id3v2Version)
 {
+  return save(tags,
+              stripOthers ? StripOthers : StripNone,
+              id3v2Version == 3 ? ID3v2::v3 : ID3v2::v4);
+}
+
+bool RIFF::WAV::File::save(TagTypes tags, StripTags strip, ID3v2::Version version)
+{
   if(readOnly()) {
     debug("RIFF::WAV::File::save() -- File is read only.");
     return false;
@@ -162,14 +169,14 @@ bool RIFF::WAV::File::save(TagTypes tags, bool stripOthers, int id3v2Version)
     return false;
   }
 
-  if(stripOthers)
-    strip(static_cast<TagTypes>(AllTags & ~tags));
+  if(strip == StripOthers)
+    File::strip(static_cast<TagTypes>(AllTags & ~tags));
 
   if(tags & ID3v2) {
     removeTagChunks(ID3v2);
 
     if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) {
-      setChunkData("ID3 ", ID3v2Tag()->render(id3v2Version));
+      setChunkData("ID3 ", ID3v2Tag()->render(version));
       d->hasID3v2 = true;
     }
   }
diff --git a/taglib/riff/wav/wavfile.h b/taglib/riff/wav/wavfile.h
index f6c190ed..bc9ce312 100644
--- a/taglib/riff/wav/wavfile.h
+++ b/taglib/riff/wav/wavfile.h
@@ -159,7 +159,19 @@ namespace TagLib {
          */
         virtual bool save();
 
-        bool save(TagTypes tags, bool stripOthers = true, int id3v2Version = 4);
+        /*!
+         * \deprecated
+         */
+        TAGLIB_DEPRECATED bool save(TagTypes tags, bool stripOthers, int id3v2Version = 4);
+
+        /*!
+         * Save the file.  If \a strip is specified, it is possible to choose if
+         * tags not specified in \a tags should be stripped from the file or
+         * retained.  With \a version, it is possible to specify whether ID3v2.4
+         * or ID3v2.3 should be used.
+         */
+        bool save(TagTypes tags, StripTags strip = StripOthers,
+                  ID3v2::Version version = ID3v2::v4);
 
         /*!
          * Returns whether or not the file on disk actually has an ID3v2 tag.
diff --git a/tests/test_aiff.cpp b/tests/test_aiff.cpp
index cc7df9aa..116c13a6 100644
--- a/tests/test_aiff.cpp
+++ b/tests/test_aiff.cpp
@@ -40,6 +40,7 @@ class TestAIFF : public CppUnit::TestFixture
   CPPUNIT_TEST(testAiffProperties);
   CPPUNIT_TEST(testAiffCProperties);
   CPPUNIT_TEST(testSaveID3v2);
+  CPPUNIT_TEST(testSaveID3v23);
   CPPUNIT_TEST(testDuplicateID3v2);
   CPPUNIT_TEST(testFuzzedFile1);
   CPPUNIT_TEST(testFuzzedFile2);
@@ -105,6 +106,29 @@ public:
     }
   }
 
+  void testSaveID3v23()
+  {
+    ScopedFileCopy copy("empty", ".aiff");
+    string newname = copy.fileName();
+
+    String xxx = ByteVector(254, 'X');
+    {
+      RIFF::AIFF::File f(newname.c_str());
+      CPPUNIT_ASSERT_EQUAL(false, f.hasID3v2Tag());
+
+      f.tag()->setTitle(xxx);
+      f.tag()->setArtist("Artist A");
+      f.save(ID3v2::v3);
+      CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
+    }
+    {
+      RIFF::AIFF::File f2(newname.c_str());
+      CPPUNIT_ASSERT_EQUAL((unsigned int)3, f2.tag()->header()->majorVersion());
+      CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist());
+      CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
+    }
+  }
+
   void testDuplicateID3v2()
   {
     ScopedFileCopy copy("duplicate_id3v2", ".aiff");
diff --git a/tests/test_wav.cpp b/tests/test_wav.cpp
index 8e96795f..fd0c04f7 100644
--- a/tests/test_wav.cpp
+++ b/tests/test_wav.cpp
@@ -44,6 +44,7 @@ class TestWAV : public CppUnit::TestFixture
   CPPUNIT_TEST(testFloatProperties);
   CPPUNIT_TEST(testZeroSizeDataChunk);
   CPPUNIT_TEST(testID3v2Tag);
+  CPPUNIT_TEST(testSaveID3v23);
   CPPUNIT_TEST(testInfoTag);
   CPPUNIT_TEST(testStripTags);
   CPPUNIT_TEST(testDuplicateTags);
@@ -139,6 +140,29 @@ public:
     }
   }
 
+  void testSaveID3v23()
+  {
+    ScopedFileCopy copy("empty", ".wav");
+    string newname = copy.fileName();
+
+    String xxx = ByteVector(254, 'X');
+    {
+      RIFF::WAV::File f(newname.c_str());
+      CPPUNIT_ASSERT_EQUAL(false, f.hasID3v2Tag());
+
+      f.tag()->setTitle(xxx);
+      f.tag()->setArtist("Artist A");
+      f.save(RIFF::WAV::File::AllTags, File::StripOthers, ID3v2::v3);
+      CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
+    }
+    {
+      RIFF::WAV::File f2(newname.c_str());
+      CPPUNIT_ASSERT_EQUAL((unsigned int)3, f2.ID3v2Tag()->header()->majorVersion());
+      CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist());
+      CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
+    }
+  }
+
   void testInfoTag()
   {
     ScopedFileCopy copy("empty", ".wav");
@@ -191,7 +215,7 @@ public:
       RIFF::WAV::File f(filename.c_str());
       CPPUNIT_ASSERT(f.hasID3v2Tag());
       CPPUNIT_ASSERT(f.hasInfoTag());
-      f.save(RIFF::WAV::File::ID3v2, true);
+      f.save(RIFF::WAV::File::ID3v2, File::StripOthers);
     }
     {
       RIFF::WAV::File f(filename.c_str());
@@ -205,7 +229,7 @@ public:
       RIFF::WAV::File f(filename.c_str());
       CPPUNIT_ASSERT(f.hasID3v2Tag());
       CPPUNIT_ASSERT(f.hasInfoTag());
-      f.save(RIFF::WAV::File::Info, true);
+      f.save(RIFF::WAV::File::Info, File::StripOthers);
     }
     {
       RIFF::WAV::File f(filename.c_str());