Deprecate calls to MPEG::File::save(...) that use boolean params

This uses explicit enums for e.g. the ID3v2 version, making calls more
readable:

  file.save(ID3v1 | ID3v2, StripOthers, ID3v2::v4, Duplicate);

Instead of:

  file.save(ID3v1 | ID3v2, true, 4, true);

Needs to be ported to other types, per #922

9 files changed, 84 insertions, 68 deletions

diff --git a/taglib/CMakeLists.txt b/taglib/CMakeLists.txt
index ec30e14c..f252256c 100644
--- a/taglib/CMakeLists.txt
+++ b/taglib/CMakeLists.txt
@@ -63,6 +63,7 @@ set(tag_HDRS
   mpeg/xingheader.h
   mpeg/id3v1/id3v1tag.h
   mpeg/id3v1/id3v1genres.h
+  mpeg/id3v2/id3v2.h
   mpeg/id3v2/id3v2extendedheader.h
   mpeg/id3v2/id3v2frame.h
   mpeg/id3v2/id3v2header.h
diff --git a/taglib/mpeg/id3v2/id3v2header.h b/taglib/mpeg/id3v2/id3v2header.h
index 12fb6d02..a1f2985e 100644
--- a/taglib/mpeg/id3v2/id3v2header.h
+++ b/taglib/mpeg/id3v2/id3v2header.h
@@ -28,6 +28,7 @@
 
 #include "tbytevector.h"
 #include "taglib_export.h"
+#include "id3v2.h"
 
 namespace TagLib {
 
diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp
index 525e88b6..cf7f191a 100644
--- a/taglib/mpeg/id3v2/id3v2tag.cpp
+++ b/taglib/mpeg/id3v2/id3v2tag.cpp
@@ -462,7 +462,7 @@ PropertyMap ID3v2::Tag::setProperties(const PropertyMap &origProps)
 
 ByteVector ID3v2::Tag::render() const
 {
-  return render(4);
+  return render(ID3v2::v4);
 }
 
 void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const
@@ -569,16 +569,16 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const
 
 ByteVector ID3v2::Tag::render(int version) const
 {
+  return render(version == 3 ? v3 : v4);
+}
+
+ByteVector ID3v2::Tag::render(Version version) const
+{
   // We need to render the "tag data" first so that we have to correct size to
   // render in the tag's header.  The "tag data" -- everything that is included
   // in ID3v2::Header::tagSize() -- includes the extended header, frames and
   // padding, but does not include the tag's header or footer.
 
-  if(version != 3 && version != 4) {
-    debug("Unknown ID3v2 version, using ID3v2.4");
-    version = 4;
-  }
-
   // TODO: Render the extended header.
 
   // Downgrade the frames that ID3v2.3 doesn't support.
@@ -587,7 +587,7 @@ ByteVector ID3v2::Tag::render(int version) const
   newFrames.setAutoDelete(true);
 
   FrameList frameList;
-  if(version == 4) {
+  if(version == v4) {
     frameList = d->frameList;
   }
   else {
@@ -601,7 +601,7 @@ ByteVector ID3v2::Tag::render(int version) const
   // Loop through the frames rendering them and adding them to the tagData.
 
   for(FrameList::ConstIterator it = frameList.begin(); it != frameList.end(); it++) {
-    (*it)->header()->setVersion(version);
+    (*it)->header()->setVersion(version == v3 ? 3 : 4);
     if((*it)->header()->frameID().size() != 4) {
       debug("An ID3v2 frame of unsupported or unknown type \'"
           + String((*it)->header()->frameID()) + "\' has been discarded");
diff --git a/taglib/mpeg/id3v2/id3v2tag.h b/taglib/mpeg/id3v2/id3v2tag.h
index 5811a5f1..96b6fb4a 100644
--- a/taglib/mpeg/id3v2/id3v2tag.h
+++ b/taglib/mpeg/id3v2/id3v2tag.h
@@ -33,21 +33,13 @@
 #include "tmap.h"
 #include "taglib_export.h"
 
+#include "id3v2.h"
 #include "id3v2framefactory.h"
 
 namespace TagLib {
 
   class File;
 
-  //! An ID3v2 implementation
-
-  /*!
-   * This is a relatively complete and flexible framework for working with ID3v2
-   * tags.
-   *
-   * \see ID3v2::Tag
-   */
-
   namespace ID3v2 {
 
     class Header;
@@ -346,13 +338,17 @@ namespace TagLib {
       ByteVector render() const;
 
       /*!
+       * \deprecated
+       */
+      ByteVector render(int version) const;
+
+      /*!
        * Render the tag back to binary data, suitable to be written to disk.
        *
-       * The \a version parameter specifies the version of the rendered
-       * ID3v2 tag. It can be either 4 or 3.
+       * The \a version parameter specifies whether ID3v2.4 (default) or ID3v2.3
+       * should be used.
        */
-      // BIC: combine with the above method
-      ByteVector render(int version) const;
+      ByteVector render(Version version) const;
 
       /*!
        * Gets the current string handler that decides how the "Latin-1" data
diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 29a527e8..ddfe7f9e 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -215,6 +215,14 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
 
 bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags)
 {
+  return save(tags,
+              stripOthers ? StripOthers : StripNone,
+              id3v2Version == 3 ? ID3v2::v3 : ID3v2::v4,
+              duplicateTags ? Duplicate : DoNotDuplicate);
+}
+
+bool MPEG::File::save(int tags, StripTags strip, ID3v2::Version version, DuplicateTags duplicate)
+{
   if(readOnly()) {
     debug("MPEG::File::save() -- File is read only.");
     return false;
@@ -222,22 +230,22 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
 
   // Create the tags if we've been asked to.
 
-  if(duplicateTags) {
+  if(duplicate == Duplicate) {
 
     // Copy the values from the tag that does exist into the new tag,
     // except if the existing tag is to be stripped.
 
-    if((tags & ID3v2) && ID3v1Tag() && !(stripOthers && !(tags & ID3v1)))
+    if((tags & ID3v2) && ID3v1Tag() && !(strip == StripOthers && !(tags & ID3v1)))
       Tag::duplicate(ID3v1Tag(), ID3v2Tag(true), false);
 
-    if((tags & ID3v1) && d->tag[ID3v2Index] && !(stripOthers && !(tags & ID3v2)))
+    if((tags & ID3v1) && d->tag[ID3v2Index] && !(strip == StripOthers && !(tags & ID3v2)))
       Tag::duplicate(ID3v2Tag(), ID3v1Tag(true), false);
   }
 
   // Remove all the tags not going to be saved.
 
-  if(stripOthers)
-    strip(~tags, false);
+  if(strip == StripOthers)
+    File::strip(~tags, false);
 
   if(ID3v2 & tags) {
 
@@ -248,7 +256,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
       if(d->ID3v2Location < 0)
         d->ID3v2Location = 0;
 
-      const ByteVector data = ID3v2Tag()->render(id3v2Version);
+      const ByteVector data = ID3v2Tag()->render(version);
       insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
 
       if(d->APELocation >= 0)
@@ -263,7 +271,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
 
       // ID3v2 tag is empty. Remove the old one.
 
-      strip(ID3v2, false);
+      File::strip(ID3v2, false);
     }
   }
 
@@ -287,7 +295,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
 
       // ID3v1 tag is empty. Remove the old one.
 
-      strip(ID3v1, false);
+      File::strip(ID3v1, false);
     }
   }
 
@@ -316,7 +324,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
 
       // APE tag is empty. Remove the old one.
 
-      strip(APE, false);
+      File::strip(APE, false);
     }
   }
 
diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h
index 2d2dff00..40beefc0 100644
--- a/taglib/mpeg/mpegfile.h
+++ b/taglib/mpeg/mpegfile.h
@@ -32,6 +32,8 @@
 
 #include "mpegproperties.h"
 
+#include "id3v2.h"
+
 namespace TagLib {
 
   namespace ID3v2 { class Tag; class FrameFactory; }
@@ -191,49 +193,39 @@ namespace TagLib {
       bool save(int tags);
 
       /*!
-       * Save the file.  This will attempt to save all of the tag types that are
-       * specified by OR-ing together TagTypes values.  The save() method above
-       * uses AllTags.  This returns true if saving was successful.
-       *
-       * If \a stripOthers is true this strips all tags not included in the mask,
-       * but does not modify them in memory, so later calls to save() which make
-       * use of these tags will remain valid.  This also strips empty tags.
+       * \deprecated
        */
       // BIC: combine with the above method
       bool save(int tags, bool stripOthers);
 
       /*!
-       * Save the file.  This will attempt to save all of the tag types that are
-       * specified by OR-ing together TagTypes values.  The save() method above
-       * uses AllTags.  This returns true if saving was successful.
-       *
-       * If \a stripOthers is true this strips all tags not included in the mask,
-       * but does not modify them in memory, so later calls to save() which make
-       * use of these tags will remain valid.  This also strips empty tags.
-       *
-       * The \a id3v2Version parameter specifies the version of the saved
-       * ID3v2 tag. It can be either 4 or 3.
+       * \deprecated
        */
       // BIC: combine with the above method
       bool save(int tags, bool stripOthers, int id3v2Version);
 
       /*!
+       * \deprecated
+       */
+      // BIC: combine with the above method
+      bool save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags);
+
+      /*!
        * Save the file.  This will attempt to save all of the tag types that are
-       * specified by OR-ing together TagTypes values.  The save() method above
-       * uses AllTags.  This returns true if saving was successful.
+       * specified by OR-ing together TagTypes values.
        *
-       * If \a stripOthers is true this strips all tags not included in the mask,
-       * but does not modify them in memory, so later calls to save() which make
-       * use of these tags will remain valid.  This also strips empty tags.
+       * \a strip can be set to strip all tags except those in \a tags.  Those
+       * tags will not be modified in memory, and thus remain valid.
        *
-       * The \a id3v2Version parameter specifies the version of the saved
-       * ID3v2 tag. It can be either 4 or 3.
+       * \a version specifies the ID3v2 version to be used for writing tags.  By
+       * default, the latest standard, ID3v2.4 is used.
        *
-       * If \a duplicateTags is true and at least one tag -- ID3v1 or ID3v2 --
-       * exists this will duplicate its content into the other tag.
+       * If \a duplicate is set to DuplicateTags and at least one tag -- ID3v1
+       * or ID3v2 -- exists this will duplicate its content into the other tag.
        */
-      // BIC: combine with the above method
-      bool save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags);
+      bool save(int tags, StripTags strip,
+                ID3v2::Version version = ID3v2::v4,
+                DuplicateTags duplicate = Duplicate);
 
       /*!
        * Returns a pointer to the ID3v2 tag of the file.
diff --git a/taglib/toolkit/tfile.h b/taglib/toolkit/tfile.h
index 55f53a52..c9a9d37e 100644
--- a/taglib/toolkit/tfile.h
+++ b/taglib/toolkit/tfile.h
@@ -63,6 +63,24 @@ namespace TagLib {
     };
 
     /*!
+     * Specify which tags to strip either explicitly, or on save.
+     */
+    enum StripTags {
+      StripNone,  //<! Don't strip any tags
+      StripAll,   //<! Strip all tags
+      StripOthers //<! Strip all tags not explicitly referenced in method call
+    };
+
+    /*!
+     * Used to specify if when saving files, if values between different tag
+     * types should be syncronized.
+     */
+    enum DuplicateTags {
+      Duplicate,     //<! Syncronize values between different tag types
+      DoNotDuplicate //<! Do not syncronize values between different tag types
+    };
+
+    /*!
      * Destroys this File instance.
      */
     virtual ~File();
diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 0e36d77f..d42907a5 100644
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -145,7 +145,7 @@ public:
 
     MPEG::File file(newname.c_str());
     file.ID3v2Tag(true)->addFrame(f);
-    file.save(MPEG::File::ID3v2, true, 3);
+    file.save(MPEG::File::ID3v2, File::StripOthers, ID3v2::v3);
     CPPUNIT_ASSERT_EQUAL(true, file.hasID3v2Tag());
 
     ByteVector data = f->render();
@@ -166,7 +166,7 @@ public:
 
     MPEG::File file(copy.fileName().c_str());
     file.ID3v2Tag(true)->addFrame(f);
-    file.save(MPEG::File::ID3v2, true, 3);
+    file.save(MPEG::File::ID3v2, File::StripOthers, ID3v2::v3);
     CPPUNIT_ASSERT(file.hasID3v2Tag());
 
     ByteVector data = f->render();
@@ -756,7 +756,7 @@ public:
       foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOT", String::Latin1));
       foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSST", String::Latin1));
       foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOP", String::Latin1));
-      foo.save(MPEG::File::AllTags, true, 3);
+      foo.save(MPEG::File::AllTags, File::StripOthers, ID3v2::v3);
     }
     {
       MPEG::File bar(newname.c_str());
@@ -1029,7 +1029,7 @@ public:
       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);
+      bar.save(MPEG::File::ID3v2, File::StripOthers);
     }
 
     MPEG::File f(newname.c_str());
@@ -1275,7 +1275,7 @@ public:
       CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
 
       f.ID3v2Tag()->setArtist("Artist A");
-      f.save(MPEG::File::ID3v2, true);
+      f.save(MPEG::File::ID3v2, File::StripOthers);
     }
     {
       MPEG::File f(copy.fileName().c_str());
diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp
index fd46f9ed..45d60ece 100644
--- a/tests/test_mpeg.cpp
+++ b/tests/test_mpeg.cpp
@@ -187,7 +187,7 @@ public:
 
       f.tag()->setTitle(xxx);
       f.tag()->setArtist("Artist A");
-      f.save(MPEG::File::AllTags, true, 4);
+      f.save(MPEG::File::AllTags, File::StripOthers, ID3v2::v4);
       CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
     }
     {
@@ -230,7 +230,7 @@ public:
 
       f.tag()->setTitle(xxx);
       f.tag()->setArtist("Artist A");
-      f.save(MPEG::File::AllTags, true, 3);
+      f.save(MPEG::File::AllTags, File::StripOthers, ID3v2::v3);
       CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
     }
     {
@@ -369,7 +369,7 @@ public:
     {
       MPEG::File f(copy.fileName().c_str());
       f.ID3v2Tag(true)->setTitle("");
-      f.save(MPEG::File::ID3v2, false);
+      f.save(MPEG::File::ID3v2, File::StripNone);
     }
     {
       MPEG::File f(copy.fileName().c_str());
@@ -389,7 +389,7 @@ public:
     {
       MPEG::File f(copy.fileName().c_str());
       f.ID3v1Tag(true)->setTitle("");
-      f.save(MPEG::File::ID3v1, false);
+      f.save(MPEG::File::ID3v1, File::StripNone);
     }
     {
       MPEG::File f(copy.fileName().c_str());
@@ -409,7 +409,7 @@ public:
     {
       MPEG::File f(copy.fileName().c_str());
       f.APETag(true)->setTitle("");
-      f.save(MPEG::File::APE, false);
+      f.save(MPEG::File::APE, File::StripNone);
     }
     {
       MPEG::File f(copy.fileName().c_str());