Support for writing structuraly correct ID3v2.3 tags

We don't use the tag footer, extended header or unsynchronization, so we
only need to change the frame size format.

Note that this doesn't write correct ID3v2.3 tags, just tags that have
the correct structure. The content is sill incorrect, unless you add
the right frames manually to the tag instance.

diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp
index 786336b9f821b2392eef2e47a0ac7abb82c04348..ef8e3eed0b451de31695b412c682860d71c25135 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2frame.cpp
+++ b/taglib/mpeg/id3v2/id3v2frame.cpp
@@ -484,6 +484,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 +543,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 661be3d2b26576b469b730e7f2fdebea117e2dbb..cd401c092e55de46399321ce94fcfdbf6ece3cd2 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2frame.h
+++ b/taglib/mpeg/id3v2/id3v2frame.h
@@ -294,11 +294,17 @@ 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.
        *

diff --git a/taglib/mpeg/id3v2/id3v2header.cpp b/taglib/mpeg/id3v2/id3v2header.cpp
index 31a5a1ec14ea892108606b2185a54cdad2c7775a..e21292de513bbe84ffb39119098ef5be14e401df 100644 (file)
--- 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 7e8012b2bf4367b2ac701b134c25d3350f5f24f7..c85ef59d0355e4d1226f98fbe4a80a90e4033e2b 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2tag.cpp
+++ b/taglib/mpeg/id3v2/id3v2tag.cpp
@@ -330,6 +330,11 @@ void ID3v2::Tag::removeFrames(const ByteVector &id)
 }
 
 ByteVector ID3v2::Tag::render() const
+{
+  return render(4);
+}
+
+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
@@ -343,6 +348,7 @@ ByteVector ID3v2::Tag::render() const
   // Loop through the frames rendering them and adding them to the tagData.
 
   for(FrameList::Iterator it = d->frameList.begin(); it != d->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 +370,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 a139d455596e3b9a8b99e3841823750d43da5c0e..137139bd56a33f1612bf9166dec32ae4ef14d4ac 100644 (file)
--- 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

diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp
index 2fc1b3807c0438a4cda8ecbf09b9ca0e52f752ed..3b3513ae4674938120aad0c59a966ffa8c442a0e 100644 (file)
--- a/taglib/mpeg/mpegfile.cpp
+++ b/taglib/mpeg/mpegfile.cpp
@@ -139,6 +139,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 282af775afbfcd7492897c5c123d91ebd72e3608..7b41328ae4a21a930c073e5999f558d83b1e65c2 100644 (file)
--- a/taglib/mpeg/mpegfile.h
+++ b/taglib/mpeg/mpegfile.h
@@ -161,6 +161,21 @@ namespace TagLib {
       // 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.
+       */
+      // BIC: combine with the above method
+      bool save(int tags, bool stripOthers, int id3v2Version);
+
       /*!
        * Returns a pointer to the ID3v2 tag of the file.
        *

diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp
index 6278ff55eef11209164dc166b94506a26999482b..6e85aed33bc5563504d821108bab910d7d82a684 100644 (file)
--- 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,8 @@ class TestMPEG : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestMPEG);
   CPPUNIT_TEST(testVersion2DurationWithXingHeader);
+  CPPUNIT_TEST(testSaveID3v24);
+  CPPUNIT_TEST(testSaveID3v23);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -20,6 +24,38 @@ public:
     CPPUNIT_ASSERT_EQUAL(5387, f.audioProperties()->length());
   }
 
+  void testSaveID3v24()
+  {
+    ScopedFileCopy copy("xing", ".mp3", false);
+    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(String("Artist A"), f2.tag()->artist());
+    CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
+  }
+
+  void testSaveID3v23()
+  {
+    ScopedFileCopy copy("xing", ".mp3", false);
+    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(String("Artist A"), f2.tag()->artist());
+    CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);