Merge branch 'master' of github.com:taglib/taglib

15 files changed, 354 insertions, 14 deletions

diff --git a/NEWS b/NEWS
index 5a9603a6..b157c457 100644
--- a/NEWS
+++ b/NEWS
@@ -1,7 +1,9 @@
 TagLib 1.8 (In Development)
 ===========================
 
+ * Support for writing ID3v2.3 tags.
  * Added methods for checking if WMA and MP4 files are DRM-protected.
+ * Started using atomic int operations for reference counting.
 
 TagLib 1.7 (Mar 11, 2011)
 =========================
diff --git a/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp b/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp
index e4e97d01..c31e1076 100644
--- a/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp
+++ b/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp
@@ -152,7 +152,7 @@ ByteVector AttachedPictureFrame::renderFields() const
 {
   ByteVector data;
 
-  String::Type encoding = checkEncoding(d->description, d->textEncoding);
+  String::Type encoding = checkTextEncoding(d->description, d->textEncoding);
 
   data.append(char(encoding));
   data.append(d->mimeType.data(String::Latin1));
diff --git a/taglib/mpeg/id3v2/frames/commentsframe.cpp b/taglib/mpeg/id3v2/frames/commentsframe.cpp
index 406598d9..377a7ee3 100644
--- a/taglib/mpeg/id3v2/frames/commentsframe.cpp
+++ b/taglib/mpeg/id3v2/frames/commentsframe.cpp
@@ -155,8 +155,8 @@ ByteVector CommentsFrame::renderFields() const
 
   String::Type encoding = d->textEncoding;
 
-  encoding = checkEncoding(d->description, encoding);
-  encoding = checkEncoding(d->text, encoding);
+  encoding = checkTextEncoding(d->description, encoding);
+  encoding = checkTextEncoding(d->text, encoding);
 
   v.append(char(encoding));
   v.append(d->language.size() == 3 ? d->language : "XXX");
diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp
index 7c2ab909..cf724922 100644
--- a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp
+++ b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp
@@ -139,7 +139,7 @@ void TextIdentificationFrame::parseFields(const ByteVector &data)
 
 ByteVector TextIdentificationFrame::renderFields() const
 {
-  String::Type encoding = checkEncoding(d->fieldList, d->textEncoding);
+  String::Type encoding = checkTextEncoding(d->fieldList, d->textEncoding);
 
   ByteVector v;
 
diff --git a/taglib/mpeg/id3v2/frames/urllinkframe.cpp b/taglib/mpeg/id3v2/frames/urllinkframe.cpp
index 7756c4ad..09edec40 100644
--- a/taglib/mpeg/id3v2/frames/urllinkframe.cpp
+++ b/taglib/mpeg/id3v2/frames/urllinkframe.cpp
@@ -175,7 +175,7 @@ ByteVector UserUrlLinkFrame::renderFields() const
 {
   ByteVector v;
 
-  String::Type encoding = checkEncoding(d->description, d->textEncoding);
+  String::Type encoding = checkTextEncoding(d->description, d->textEncoding);
 
   v.append(char(encoding));
   v.append(d->description.data(encoding));
diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp
index 786336b9..59559178 100644
--- a/taglib/mpeg/id3v2/id3v2frame.cpp
+++ b/taglib/mpeg/id3v2/id3v2frame.cpp
@@ -230,19 +230,38 @@ String Frame::readStringField(const ByteVector &data, String::Type encoding, int
 
 String::Type Frame::checkEncoding(const StringList &fields, String::Type encoding) // static
 {
+  return checkEncoding(fields, encoding, 4);
+}
+
+String::Type Frame::checkEncoding(const StringList &fields, String::Type encoding, uint version) // static
+{
+  if((encoding == String::UTF8 || encoding == String::UTF16BE) && version != 4)
+    return String::UTF16;
+
   if(encoding != String::Latin1)
     return encoding;
 
   for(StringList::ConstIterator it = fields.begin(); it != fields.end(); ++it) {
     if(!(*it).isLatin1()) {
-      debug("Frame::checkEncoding() -- Rendering using UTF8.");
-      return String::UTF8;
+      if(version == 4) {
+        debug("Frame::checkEncoding() -- Rendering using UTF8.");
+        return String::UTF8;
+      }
+      else {
+        debug("Frame::checkEncoding() -- Rendering using UTF16.");
+        return String::UTF16;
+      }
     }
   }
 
   return String::Latin1;
 }
 
+String::Type Frame::checkTextEncoding(const StringList &fields, String::Type encoding) const
+{
+  return checkEncoding(fields, encoding, header()->version());
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Frame::Header class
 ////////////////////////////////////////////////////////////////////////////////
@@ -484,6 +503,11 @@ TagLib::uint Frame::Header::version() const
   return d->version;
 }
 
+void Frame::Header::setVersion(TagLib::uint version)
+{
+  d->version = version;
+}
+
 bool Frame::Header::tagAlterPreservation() const
 {
   return d->tagAlterPreservation;
@@ -538,7 +562,11 @@ ByteVector Frame::Header::render() const
 {
   ByteVector flags(2, char(0)); // just blank for the moment
 
-  ByteVector v = d->frameID + SynchData::fromUInt(d->frameSize) + flags;
+  ByteVector v = d->frameID +
+    (d->version == 3
+      ? ByteVector::fromUInt(d->frameSize)
+      : SynchData::fromUInt(d->frameSize)) +
+    flags;
 
   return v;
 }
diff --git a/taglib/mpeg/id3v2/id3v2frame.h b/taglib/mpeg/id3v2/id3v2frame.h
index 661be3d2..2b6bcd88 100644
--- a/taglib/mpeg/id3v2/id3v2frame.h
+++ b/taglib/mpeg/id3v2/id3v2frame.h
@@ -199,9 +199,29 @@ namespace TagLib {
        * Checks a the list of string values to see if they can be used with the
        * specified encoding and returns the recommended encoding.
        */
+      // BIC: remove and make non-static
       static String::Type checkEncoding(const StringList &fields,
                                         String::Type encoding);
 
+      /*!
+       * Checks a the list of string values to see if they can be used with the
+       * specified encoding and returns the recommended encoding. This method
+       * also checks the ID3v2 version and makes sure the encoding can be used
+       * in the specified version.
+       */
+      // BIC: remove and make non-static
+      static String::Type checkEncoding(const StringList &fields,
+                                        String::Type encoding, uint version);
+
+      /*!
+       * Checks a the list of string values to see if they can be used with the
+       * specified encoding and returns the recommended encoding. This method
+       * also checks the ID3v2 version and makes sure the encoding can be used
+       * in the version specified by the frame's header.
+       */
+      String::Type checkTextEncoding(const StringList &fields,
+                                     String::Type encoding) const;
+
     private:
       Frame(const Frame &);
       Frame &operator=(const Frame &);
@@ -294,12 +314,18 @@ namespace TagLib {
       void setFrameSize(uint size);
 
       /*!
-       * Returns the ID3v2 version of the header (as passed in from the
-       * construction of the header).
+       * Returns the ID3v2 version of the header, as passed in from the
+       * construction of the header or set via setVersion().
        */
       uint version() const;
 
       /*!
+       * Sets the ID3v2 version of the header, changing has impact on the
+       * correct parsing/rendering of frame data.
+       */
+      void setVersion(uint version);
+
+      /*!
        * Returns the size of the frame header in bytes.
        *
        * \deprecated Please use the version of this method that accepts a
diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp
index 356d60d4..e1850833 100644
--- a/taglib/mpeg/id3v2/id3v2framefactory.cpp
+++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp
@@ -368,6 +368,7 @@ bool FrameFactory::updateFrame(Frame::Header *header) const
 
     convertFrame("TORY", "TDOR", header);
     convertFrame("TYER", "TDRC", header);
+    convertFrame("IPLS", "TIPL", header);
 
     break;
   }
diff --git a/taglib/mpeg/id3v2/id3v2header.cpp b/taglib/mpeg/id3v2/id3v2header.cpp
index 31a5a1ec..e21292de 100644
--- a/taglib/mpeg/id3v2/id3v2header.cpp
+++ b/taglib/mpeg/id3v2/id3v2header.cpp
@@ -164,7 +164,7 @@ ByteVector Header::render() const
   // add the version number -- we always render a 2.4.0 tag regardless of what
   // the tag originally was.
 
-  v.append(char(4));
+  v.append(char(majorVersion()));
   v.append(char(0));
 
   // Currently we don't actually support writing extended headers, footers or
diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp
index 7e8012b2..5b4c5c5b 100644
--- a/taglib/mpeg/id3v2/id3v2tag.cpp
+++ b/taglib/mpeg/id3v2/id3v2tag.cpp
@@ -331,6 +331,106 @@ void ID3v2::Tag::removeFrames(const ByteVector &id)
 
 ByteVector ID3v2::Tag::render() const
 {
+  return render(4);
+}
+
+void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const
+{
+  const char *unsupportedFrames[] = {
+    "ASPI", "EQU2", "RVA2", "SEEK", "SIGN", "TDRL", "TDTG",
+    "TMOO", "TPRO", "TSOA", "TSOT", "TSST", "TSOP", 0
+  };
+  ID3v2::TextIdentificationFrame *frameTDOR = 0;
+  ID3v2::TextIdentificationFrame *frameTDRC = 0;
+  ID3v2::TextIdentificationFrame *frameTIPL = 0;
+  ID3v2::TextIdentificationFrame *frameTMCL = 0;
+  for(FrameList::Iterator it = d->frameList.begin(); it != d->frameList.end(); it++) {
+    ID3v2::Frame *frame = *it;
+    ByteVector frameID = frame->header()->frameID();
+    for(int i = 0; unsupportedFrames[i]; i++) {
+      if(frameID == unsupportedFrames[i]) {
+        debug("A frame that is not supported in ID3v2.3 \'"
+          + String(frameID) + "\' has been discarded");
+        frame = 0;
+        break;
+      }
+    }
+    if(frame && frameID == "TDOR") {
+      frameTDOR = dynamic_cast<ID3v2::TextIdentificationFrame *>(frame);
+      frame = 0;
+    }
+    if(frame && frameID == "TDRC") {
+      frameTDRC = dynamic_cast<ID3v2::TextIdentificationFrame *>(frame);
+      frame = 0;
+    }
+    if(frame && frameID == "TIPL") {
+      frameTIPL = dynamic_cast<ID3v2::TextIdentificationFrame *>(frame);
+      frame = 0;
+    }
+    if(frame && frameID == "TMCL") {
+      frameTMCL = dynamic_cast<ID3v2::TextIdentificationFrame *>(frame);
+      frame = 0;
+    }
+    if(frame) {
+      frames->append(frame);
+    }
+  }
+  if(frameTDOR) {
+    String content = frameTDOR->toString();
+    if(content.size() >= 4) {
+      ID3v2::TextIdentificationFrame *frameTORY = new ID3v2::TextIdentificationFrame("TORY", String::Latin1);
+      frameTORY->setText(content.substr(0, 4));
+      frames->append(frameTORY);
+      newFrames->append(frameTORY);
+    }
+  }
+  if(frameTDRC) {
+    String content = frameTDRC->toString();
+    if(content.size() >= 4) {
+      ID3v2::TextIdentificationFrame *frameTYER = new ID3v2::TextIdentificationFrame("TYER", String::Latin1);
+      frameTYER->setText(content.substr(0, 4));
+      frames->append(frameTYER);
+      newFrames->append(frameTYER);
+      if(content.size() >= 10 && content[4] == '-' && content[7] == '-') {
+        ID3v2::TextIdentificationFrame *frameTDAT = new ID3v2::TextIdentificationFrame("TDAT", String::Latin1);
+        frameTDAT->setText(content.substr(8, 2) + content.substr(5, 2));
+        frames->append(frameTDAT);
+        newFrames->append(frameTDAT);
+        if(content.size() >= 16 && content[10] == 'T' && content[13] == ':') {
+          ID3v2::TextIdentificationFrame *frameTIME = new ID3v2::TextIdentificationFrame("TIME", String::Latin1);
+          frameTIME->setText(content.substr(11, 2) + content.substr(14, 2));
+          frames->append(frameTIME);
+          newFrames->append(frameTIME);
+        }
+      }
+    }
+  }
+  if(frameTIPL || frameTMCL) {
+    ID3v2::TextIdentificationFrame *frameIPLS = new ID3v2::TextIdentificationFrame("IPLS", String::Latin1);
+    StringList people;
+    if(frameTMCL) {
+      StringList v24People = frameTMCL->fieldList();
+      for(uint i = 0; i + 1 < v24People.size(); i += 2) {
+        people.append(v24People[i]);
+        people.append(v24People[i+1]);
+      }
+    }
+    if(frameTIPL) {
+      StringList v24People = frameTIPL->fieldList();
+      for(uint i = 0; i + 1 < v24People.size(); i += 2) {
+        people.append(v24People[i]);
+        people.append(v24People[i+1]);
+      }
+    }
+    frameIPLS->setText(people);
+    frames->append(frameIPLS);
+    newFrames->append(frameIPLS);
+  }
+}
+
+
+ByteVector ID3v2::Tag::render(int 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
@@ -338,11 +438,28 @@ ByteVector ID3v2::Tag::render() const
 
   ByteVector tagData;
 
+  if(version != 3 && version != 4) {
+    debug("Unknown ID3v2 version, using ID3v2.4");
+    version = 4;
+  }
+
   // TODO: Render the extended header.
 
   // Loop through the frames rendering them and adding them to the tagData.
 
-  for(FrameList::Iterator it = d->frameList.begin(); it != d->frameList.end(); it++) {
+  FrameList newFrames;
+  newFrames.setAutoDelete(true);
+
+  FrameList frameList;
+  if(version == 4) {
+    frameList = d->frameList;
+  }
+  else {
+    downgradeFrames(&frameList, &newFrames);
+  }
+
+  for(FrameList::Iterator it = frameList.begin(); it != frameList.end(); it++) {
+    (*it)->header()->setVersion(version);
     if((*it)->header()->frameID().size() != 4) {
       debug("A frame of unsupported or unknown type \'"
           + String((*it)->header()->frameID()) + "\' has been discarded");
@@ -364,7 +481,8 @@ ByteVector ID3v2::Tag::render() const
 
   tagData.append(ByteVector(paddingSize, char(0)));
 
-  // Set the tag size.
+  // Set the version and data size.
+  d->header.setMajorVersion(version);
   d->header.setTagSize(tagData.size());
 
   // TODO: This should eventually include d->footer->render().
diff --git a/taglib/mpeg/id3v2/id3v2tag.h b/taglib/mpeg/id3v2/id3v2tag.h
index a139d455..4a52854a 100644
--- a/taglib/mpeg/id3v2/id3v2tag.h
+++ b/taglib/mpeg/id3v2/id3v2tag.h
@@ -265,6 +265,15 @@ namespace TagLib {
        */
       ByteVector render() 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.
+       */
+      // BIC: combine with the above method
+      ByteVector render(int version) const;
+
     protected:
       /*!
        * Reads data from the file specified in the constructor.  It does basic
@@ -286,6 +295,8 @@ namespace TagLib {
        */
       void setTextFrame(const ByteVector &id, const String &value);
 
+      void downgradeFrames(FrameList *existingFrames, FrameList *newFrames) const;
+
     private:
       Tag(const Tag &);
       Tag &operator=(const Tag &);
diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 2fc1b380..3b3513ae 100644
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -140,6 +140,11 @@ bool MPEG::File::save(int tags)
 
 bool MPEG::File::save(int tags, bool stripOthers)
 {
+  return save(tags, stripOthers, 4);
+}
+
+bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
+{
   if(tags == NoTags && stripOthers)
     return strip(AllTags);
 
@@ -174,7 +179,7 @@ bool MPEG::File::save(int tags, bool stripOthers)
       if(!d->hasID3v2)
         d->ID3v2Location = 0;
 
-      insert(ID3v2Tag()->render(), d->ID3v2Location, d->ID3v2OriginalSize);
+      insert(ID3v2Tag()->render(id3v2Version), d->ID3v2Location, d->ID3v2OriginalSize);
 
       d->hasID3v2 = true;
 
diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h
index 282af775..7b41328a 100644
--- a/taglib/mpeg/mpegfile.h
+++ b/taglib/mpeg/mpegfile.h
@@ -162,6 +162,21 @@ namespace TagLib {
       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.
+       */
+      // BIC: combine with the above method
+      bool save(int tags, bool stripOthers, int id3v2Version);
+
+      /*!
        * Returns a pointer to the ID3v2 tag of the file.
        *
        * If \a create is false (the default) this will return a null pointer
diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 4276963d..8b8939b3 100644
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -1,9 +1,12 @@
 #include <cppunit/extensions/HelperMacros.h>
 #include <string>
 #include <stdio.h>
+// so evil :(
+#define protected public
 #include <id3v2tag.h>
 #include <mpegfile.h>
 #include <id3v2frame.h>
+#undef protected
 #include <uniquefileidentifierframe.h>
 #include <textidentificationframe.h>
 #include <attachedpictureframe.h>
@@ -33,6 +36,7 @@ class TestID3v2 : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestID3v2);
   CPPUNIT_TEST(testUnsynchDecode);
+  CPPUNIT_TEST(testDowngradeUTF8ForID3v23);
   CPPUNIT_TEST(testUTF16BEDelimiter);
   CPPUNIT_TEST(testUTF16Delimiter);
   CPPUNIT_TEST(testReadStringField);
@@ -60,6 +64,7 @@ class TestID3v2 : public CppUnit::TestFixture
   CPPUNIT_TEST(testUpdateGenre23_2);
   CPPUNIT_TEST(testUpdateGenre24);
   CPPUNIT_TEST(testUpdateDate22);
+  CPPUNIT_TEST(testDowngradeTo23);
   // CPPUNIT_TEST(testUpdateFullDate22); TODO TYE+TDA should be upgraded to TDRC together
   CPPUNIT_TEST(testCompressedFrameWithBrokenLength);
   CPPUNIT_TEST_SUITE_END();
@@ -73,6 +78,20 @@ public:
     CPPUNIT_ASSERT_EQUAL(String("My babe just cares for me"), f.tag()->title());
   }
 
+  void testDowngradeUTF8ForID3v23()
+  {
+    ID3v2::TextIdentificationFrame f(ByteVector("TPE1"), String::UTF8);
+    StringList sl;
+    sl.append("Foo");
+    f.setText(sl);
+    f.header()->setVersion(3);
+    ByteVector data = f.render();
+    CPPUNIT_ASSERT_EQUAL((unsigned int)(4+4+2+1+6+2), data.size());
+    ID3v2::TextIdentificationFrame f2(data);
+    CPPUNIT_ASSERT_EQUAL(sl, f2.fieldList());
+    CPPUNIT_ASSERT_EQUAL(String::UTF16, f2.textEncoding());
+  }
+
   void testUTF16BEDelimiter()
   {
     ID3v2::TextIdentificationFrame f(ByteVector("TPE1"), String::UTF16BE);
@@ -456,6 +475,65 @@ public:
     CPPUNIT_ASSERT_EQUAL(String("2010-04-03"), f.ID3v2Tag()->frameListMap()["TDRC"].front()->toString());
   }
 
+  void testDowngradeTo23()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+    string newname = copy.fileName();
+
+    ID3v2::TextIdentificationFrame *tf;
+    MPEG::File foo(newname.c_str());
+    tf = new ID3v2::TextIdentificationFrame("TDOR", String::Latin1);
+    tf->setText("2011-03-16");
+    foo.ID3v2Tag()->addFrame(tf);
+    tf = new ID3v2::TextIdentificationFrame("TDRC", String::Latin1);
+    tf->setText("2012-04-17T12:01");
+    foo.ID3v2Tag()->addFrame(tf);
+    tf = new ID3v2::TextIdentificationFrame("TMCL", String::Latin1);
+    tf->setText(StringList().append("Guitar").append("Artist 1").append("Drums").append("Artist 2"));
+    foo.ID3v2Tag()->addFrame(tf);
+    tf = new ID3v2::TextIdentificationFrame("TIPL", String::Latin1);
+    tf->setText(StringList().append("Producer").append("Artist 3").append("Mastering").append("Artist 4"));
+    foo.ID3v2Tag()->addFrame(tf);
+    foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDRL", String::Latin1));
+    foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDTG", String::Latin1));
+    foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TMOO", String::Latin1));
+    foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TPRO", String::Latin1));
+    foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOA", String::Latin1));
+    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);
+
+    MPEG::File bar(newname.c_str());
+    tf = static_cast<ID3v2::TextIdentificationFrame *>(bar.ID3v2Tag()->frameList("TDOR").front());
+    CPPUNIT_ASSERT(tf);
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), tf->fieldList().size());
+    CPPUNIT_ASSERT_EQUAL(String("2011"), tf->fieldList().front());
+    tf = static_cast<ID3v2::TextIdentificationFrame *>(bar.ID3v2Tag()->frameList("TDRC").front());
+    CPPUNIT_ASSERT(tf);
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), tf->fieldList().size());
+    CPPUNIT_ASSERT_EQUAL(String("2012"), tf->fieldList().front());
+    tf = dynamic_cast<ID3v2::TextIdentificationFrame *>(bar.ID3v2Tag()->frameList("TIPL").front());
+    CPPUNIT_ASSERT(tf);
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(8), tf->fieldList().size());
+    CPPUNIT_ASSERT_EQUAL(String("Guitar"), tf->fieldList()[0]);
+    CPPUNIT_ASSERT_EQUAL(String("Artist 1"), tf->fieldList()[1]);
+    CPPUNIT_ASSERT_EQUAL(String("Drums"), tf->fieldList()[2]);
+    CPPUNIT_ASSERT_EQUAL(String("Artist 2"), tf->fieldList()[3]);
+    CPPUNIT_ASSERT_EQUAL(String("Producer"), tf->fieldList()[4]);
+    CPPUNIT_ASSERT_EQUAL(String("Artist 3"), tf->fieldList()[5]);
+    CPPUNIT_ASSERT_EQUAL(String("Mastering"), tf->fieldList()[6]);
+    CPPUNIT_ASSERT_EQUAL(String("Artist 4"), tf->fieldList()[7]);
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TDRL"));
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TDTG"));
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TMOO"));
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TPRO"));
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOA"));
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOT"));
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSST"));
+    CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOP"));
+  }
+
   void testCompressedFrameWithBrokenLength()
   {
     MPEG::File f("data/compressed_id3_frame.mp3", false);
diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp
index 6278ff55..754caa98 100644
--- a/tests/test_mpeg.cpp
+++ b/tests/test_mpeg.cpp
@@ -2,6 +2,8 @@
 #include <string>
 #include <stdio.h>
 #include <mpegfile.h>
+#include <id3v2tag.h>
+#include "utils.h"
 
 using namespace std;
 using namespace TagLib;
@@ -10,6 +12,9 @@ class TestMPEG : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestMPEG);
   CPPUNIT_TEST(testVersion2DurationWithXingHeader);
+  CPPUNIT_TEST(testSaveID3v24);
+  CPPUNIT_TEST(testSaveID3v24WrongParam);
+  CPPUNIT_TEST(testSaveID3v23);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -20,6 +25,57 @@ public:
     CPPUNIT_ASSERT_EQUAL(5387, f.audioProperties()->length());
   }
 
+  void testSaveID3v24()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+    string newname = copy.fileName();
+
+    String xxx = ByteVector(254, 'X');
+    MPEG::File f(newname.c_str());
+    f.tag()->setTitle(xxx);
+    f.tag()->setArtist("Artist A");
+    f.save(MPEG::File::AllTags, true, 4);
+
+    MPEG::File f2(newname.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f2.ID3v2Tag()->header()->majorVersion());
+    CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist());
+    CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
+  }
+
+  void testSaveID3v24WrongParam()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+    string newname = copy.fileName();
+
+    String xxx = ByteVector(254, 'X');
+    MPEG::File f(newname.c_str());
+    f.tag()->setTitle(xxx);
+    f.tag()->setArtist("Artist A");
+    f.save(MPEG::File::AllTags, true, 8);
+
+    MPEG::File f2(newname.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f2.ID3v2Tag()->header()->majorVersion());
+    CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist());
+    CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
+  }
+
+  void testSaveID3v23()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+    string newname = copy.fileName();
+
+    String xxx = ByteVector(254, 'X');
+    MPEG::File f(newname.c_str());
+    f.tag()->setTitle(xxx);
+    f.tag()->setArtist("Artist A");
+    f.save(MPEG::File::AllTags, true, 3);
+
+    MPEG::File f2(newname.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(3), f2.ID3v2Tag()->header()->majorVersion());
+    CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist());
+    CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);