Unify File::save(...) APIs between file formats that support ID3v2
authorScott Wheeler <scott@directededge.com>
Wed, 11 Sep 2019 03:55:30 +0000 (05:55 +0200)
committerScott Wheeler <scott@directededge.com>
Wed, 11 Sep 2019 04:48:27 +0000 (06:48 +0200)
Closes #922

taglib/riff/aiff/aifffile.cpp
taglib/riff/aiff/aifffile.h
taglib/riff/wav/wavfile.cpp
taglib/riff/wav/wavfile.h
tests/test_aiff.cpp
tests/test_wav.cpp

index 4f9c868e6b0ba52a20d56f641cacd2be2a1a0a2f..6ef4c9be5dc9442a14d499a5a03a3a55d9516216 100644 (file)
@@ -117,6 +117,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.");
@@ -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;
   }
 
index 5ba1a27915ae9a022cd1bde6572482a694b2ffd1..ceac5769e661830b0786ed271a07e98963610f35 100644 (file)
@@ -119,6 +119,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.
          *
index 0ebe21c3207d3583f71ebc0ba4fd490e1335f7f0..3dce70b1ce463fd95c96a45c8ad93c5d9a561d55 100644 (file)
@@ -151,6 +151,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.");
@@ -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;
     }
   }
index f6c190ed65c9ff90d3fc34d8100c064dd26f052c..bc9ce3126247e0c0a4e1f5066def507d075510bb 100644 (file)
@@ -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.
index cc7df9aac5848f601539950ab7f8421c4e9b2b18..116c13a620794960c5b365297a95697cef1ac28a 100644 (file)
@@ -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");
index 8e96795fdc3cd94f03342be5178511486a610264..fd0c04f711ad2d2346bac071e52f0b7738281b72 100644 (file)
@@ -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());