Fix a number of bugs to get things working with ID3v2.4 tags generated by

libid3tag:

*) Handle the case of there being an extended header where the first byte
   is zero (this was running into a check to see if the padding had been
   reached)
*) Add support for reading ID3v2::Frame::Header flags.  Previously this
   was not implemented, but was needed to...
*) Properly adjust the reading position for the existance of a data length
   indicator at the beginning of frames

Things now seem to work with the test files that I have here.  Since this
is the only other ID3v2.4 implmentation that I know of "in the wild" it's
nice that they now play nice together.  libid3tag uses many more of the
unique ID3v2.4 features than TagLib does.

CCMAIL:82867-done@bugs.kde.org

git-svn-id: svn://anonsvn.kde.org/home/kde/trunk/kdesupport/taglib@332778 283d02a7-25f6-0310-bc7c-ecb5cbfe19da

diff --git a/mpeg/id3v2/frames/attachedpictureframe.cpp b/mpeg/id3v2/frames/attachedpictureframe.cpp
index 35a6a7efc252c25def363181743cb151a91493a3..5700dfe6466537c6ee7c17b223d9e7c56ad69df5 100644 (file)
--- a/mpeg/id3v2/frames/attachedpictureframe.cpp
+++ b/mpeg/id3v2/frames/attachedpictureframe.cpp
@@ -161,5 +161,5 @@ ByteVector AttachedPictureFrame::renderFields() const
 AttachedPictureFrame::AttachedPictureFrame(const ByteVector &data, Header *h) : Frame(h)
 {
   d = new AttachedPictureFramePrivate;
-  parseFields(data.mid(Header::size(h->version()), size()));
+  parseFields(fieldData(data));
 }

diff --git a/mpeg/id3v2/frames/commentsframe.cpp b/mpeg/id3v2/frames/commentsframe.cpp
index 99af7a4ad9ec91550f7c9435717ebbe6dfd7cfcf..066fed0718c225a3b18582741bb1241bee11f304 100644 (file)
--- a/mpeg/id3v2/frames/commentsframe.cpp
+++ b/mpeg/id3v2/frames/commentsframe.cpp
@@ -148,5 +148,5 @@ ByteVector CommentsFrame::renderFields() const
 CommentsFrame::CommentsFrame(const ByteVector &data, Header *h) : Frame(h)
 {
   d = new CommentsFramePrivate();
-  parseFields(data.mid(Header::size(h->version()), size()));
+  parseFields(fieldData(data));
 }

diff --git a/mpeg/id3v2/frames/relativevolumeframe.cpp b/mpeg/id3v2/frames/relativevolumeframe.cpp
index acbd689a24932016d1b68bcccc9e4d1ad1247d7c..171ba91dc2d12c9c92f57762d2ea09331d6f9add 100644 (file)
--- a/mpeg/id3v2/frames/relativevolumeframe.cpp
+++ b/mpeg/id3v2/frames/relativevolumeframe.cpp
@@ -133,5 +133,5 @@ ByteVector RelativeVolumeFrame::renderFields() const
 RelativeVolumeFrame::RelativeVolumeFrame(const ByteVector &data, Header *h) : Frame(h)
 {
   d = new RelativeVolumeFramePrivate;
-  parseFields(data.mid(Header::size(h->version()), size()));
+  parseFields(fieldData(data));
 }

diff --git a/mpeg/id3v2/frames/textidentificationframe.cpp b/mpeg/id3v2/frames/textidentificationframe.cpp
index 3d1582973184ca165ae8bae2a124a7e19f1aa12e..90e06e5fd6f12b18964a1b75c3af3e3fdbda8d2f 100644 (file)
--- a/mpeg/id3v2/frames/textidentificationframe.cpp
+++ b/mpeg/id3v2/frames/textidentificationframe.cpp
@@ -146,5 +146,5 @@ ByteVector TextIdentificationFrame::renderFields() const
 TextIdentificationFrame::TextIdentificationFrame(const ByteVector &data, Header *h) : Frame(h)
 {
   d = new TextIdentificationFramePrivate;
-  parseFields(data.mid(Header::size(h->version()), size()));
+  parseFields(fieldData(data));
 }

diff --git a/mpeg/id3v2/frames/unknownframe.cpp b/mpeg/id3v2/frames/unknownframe.cpp
index 96723e3e59e81fbfb667837ba73755f867a81c5b..0f4ed2e82e7b0c27fdd73de039bef8a33fc2f511 100644 (file)
--- a/mpeg/id3v2/frames/unknownframe.cpp
+++ b/mpeg/id3v2/frames/unknownframe.cpp
@@ -76,5 +76,5 @@ ByteVector UnknownFrame::renderFields() const
 UnknownFrame::UnknownFrame(const ByteVector &data, Header *h) : Frame(h)
 {
   d = new UnknownFramePrivate;
-  parseFields(data.mid(Header::size(h->version()), size()));
+  parseFields(fieldData(data));
 }

diff --git a/mpeg/id3v2/id3v2frame.cpp b/mpeg/id3v2/id3v2frame.cpp
index 2180e6ef935b57b0680cce127298172a2f21f7b9..8b7c1192d6c414afff24f5bea883ed4f5d04c766 100644 (file)
--- a/mpeg/id3v2/id3v2frame.cpp
+++ b/mpeg/id3v2/id3v2frame.cpp
@@ -19,6 +19,8 @@
  *   USA                                                                   *
  ***************************************************************************/
 
+#include <bitset>
+
 #include <tdebug.h>
 
 #include "id3v2frame.h"
@@ -30,11 +32,12 @@ using namespace ID3v2;
 class Frame::FramePrivate
 {
 public:
-  FramePrivate() {
-    header = 0;
-  }
+  FramePrivate() :
+    header(0)
+    {}
 
-  ~FramePrivate() {
+  ~FramePrivate()
+  {
     delete header;
   }
 
@@ -143,9 +146,22 @@ void Frame::parse(const ByteVector &data)
   else
     d->header = new Header(data);
 
-  // size() is the lenght of the field data
+  parseFields(fieldData(data));
+}
+
+ByteVector Frame::fieldData(const ByteVector &frameData) const
+{
+  uint headerSize = Header::size(d->header->version());
+
+  uint frameDataOffset = headerSize;
+  uint frameDataLength = size();
 
-  parseFields(data.mid(Header::size(d->header->version()), size()));
+  if(d->header->dataLengthIndicator()) {
+    frameDataLength = frameData.mid(headerSize, 4).toUInt();
+    frameDataOffset += 4;
+  }
+
+  return frameData.mid(frameDataOffset, frameDataLength);  
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -155,11 +171,33 @@ void Frame::parse(const ByteVector &data)
 class Frame::Header::HeaderPrivate
 {
 public:
-  HeaderPrivate() : frameSize(0), version(4) {}
+  HeaderPrivate() :
+    frameSize(0),
+    version(4),
+    tagAlterPreservation(false),
+    frameAlterPreservation(false),
+    readOnly(false),
+    groupingIdentity(false),
+    compression(false),
+    encryption(false),
+    unsyncronisation(false),
+    dataLengthIndicator(false)
+    {}
 
   ByteVector frameID;
   uint frameSize;
   uint version;
+
+  // flags
+
+  bool tagAlterPreservation;
+  bool frameAlterPreservation;
+  bool readOnly;
+  bool groupingIdentity;
+  bool compression;
+  bool encryption;
+  bool unsyncronisation;
+  bool dataLengthIndicator;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -213,7 +251,7 @@ void Frame::Header::setData(const ByteVector &data, bool synchSafeInts)
 
 void Frame::Header::setData(const ByteVector &data, uint version)
 {
-  d->version = version;
+ d->version = version;
 
   switch(version) {
   case 0:
@@ -275,8 +313,21 @@ void Frame::Header::setData(const ByteVector &data, uint version)
     else
       d->frameSize = data.mid(4, 4).toUInt();
 
-    // TODO: read flags
+    { // read the first byte of flags
+      std::bitset<8> flags(data[8]);
+      d->tagAlterPreservation   = flags[6]; // (structure 4.1.1.a)
+      d->frameAlterPreservation = flags[5]; // (structure 4.1.1.b)
+      d->readOnly               = flags[4]; // (structure 4.1.1.c)
+    }
 
+    { // read the second byte of flags
+      std::bitset<8> flags(data[9]);
+      d->groupingIdentity    = flags[6]; // (structure 4.1.2.h)
+      d->compression         = flags[3]; // (structure 4.1.2.k)
+      d->encryption          = flags[2]; // (structure 4.1.2.m)
+      d->unsyncronisation    = flags[1]; // (structure 4.1.2.n)
+      d->dataLengthIndicator = flags[0]; // (structure 4.1.2.p)
+    }
     break;
   }
   }
@@ -307,6 +358,46 @@ TagLib::uint Frame::Header::version() const
   return d->version;
 }
 
+bool Frame::Header::tagAlterPreservation() const
+{
+  return d->tagAlterPreservation;
+}
+
+bool Frame::Header::frameAlterPreservation() const
+{
+  return d->frameAlterPreservation;
+}
+
+bool Frame::Header::readOnly() const
+{
+  return d->readOnly;
+}
+
+bool Frame::Header::groupingIdentity() const
+{
+  return d->groupingIdentity;
+}
+
+bool Frame::Header::compression() const
+{
+  return d->compression;
+}
+
+bool Frame::Header::encryption() const
+{
+  return d->encryption;
+}
+
+bool Frame::Header::unsycronisation() const
+{
+  return d->unsyncronisation;
+}
+
+bool Frame::Header::dataLengthIndicator() const
+{
+  return d->dataLengthIndicator;
+}
+
 ByteVector Frame::Header::render() const
 {
   ByteVector flags(2, char(0)); // just blank for the moment

diff --git a/mpeg/id3v2/id3v2frame.h b/mpeg/id3v2/id3v2frame.h
index b78fa300e8697ec3787ed4094469538c88c470a9..cdcc76d5b4f88e834d71fbffed8972ba7b5fb3e4 100644 (file)
--- a/mpeg/id3v2/id3v2frame.h
+++ b/mpeg/id3v2/id3v2frame.h
@@ -170,6 +170,13 @@ namespace TagLib {
        */
       virtual ByteVector renderFields() const = 0;
 
+      /*!
+       * Returns a ByteVector containing the field data given the frame data.
+       * This correctly adjusts for the header size plus any additional frame
+       * data that's specified in the frame header flags.
+       */
+      ByteVector fieldData(const ByteVector &frameData) const;
+
     private:
       Frame(const Frame &);
       Frame &operator=(const Frame &);
@@ -284,6 +291,60 @@ namespace TagLib {
        */
       static uint size(uint version);
 
+      /*!
+       * Returns true if the flag for tag alter preservation is set.
+       *
+       * \note This flag is currently ignored internally in TagLib.
+       */
+      bool tagAlterPreservation() const;
+
+      /*!
+       * Returns true if the flag for frame alter preservation is set.
+       *
+       * \note This flag is currently ignored internally in TagLib.
+       */
+      bool frameAlterPreservation() const;
+
+      /*!
+       * Returns true if the frame is meant to be read only.
+       *
+       * \note This flag is currently ignored internally in TagLib.
+       */
+      bool readOnly() const;
+
+      /*!
+       * Returns true if the flag for the grouping identifity is set.
+       *
+       * \note This flag is currently ignored internally in TagLib.
+       */
+      bool groupingIdentity() const;
+
+      /*!
+       * Returns true if compression is enabled for this frame.
+       *
+       * \note This flag is currently ignored internally in TagLib.
+       */
+      bool compression() const;
+
+      /*!
+       * Returns true if encryption is enabled for this frame.
+       *
+       * \note This flag is currently ignored internally in TagLib.
+       */
+      bool encryption() const;
+
+      /*!
+       * Returns true if unsyncronisation is enabled for this frame.
+       *
+       * \note This flag is currently ignored internally in TagLib.
+       */
+      bool unsycronisation() const;
+
+      /*!
+       * Returns true if the flag for a data lenght indicator is set.
+       */
+      bool dataLengthIndicator() const;
+
       /*!
        * Render the Header back to binary format in a ByteVector.
        */

diff --git a/mpeg/id3v2/id3v2tag.cpp b/mpeg/id3v2/id3v2tag.cpp
index c629bdd937a338eecf82176c2f1be8aa87257ba1..046db563a2d4bf729b8845335a8a197b57261e0f 100644 (file)
--- a/mpeg/id3v2/id3v2tag.cpp
+++ b/mpeg/id3v2/id3v2tag.cpp
@@ -386,7 +386,7 @@ void ID3v2::Tag::read()
 
 void ID3v2::Tag::parse(const ByteVector &data)
 {
-  uint frameDataOffset = 0;
+  uint frameDataPosition = 0;
   uint frameDataLength = data.size();
 
   // check for extended header
@@ -396,7 +396,7 @@ void ID3v2::Tag::parse(const ByteVector &data)
       d->extendedHeader = new ExtendedHeader;
     d->extendedHeader->setData(data);
     if(d->extendedHeader->size() <= data.size()) {
-      frameDataOffset += d->extendedHeader->size();
+      frameDataPosition += d->extendedHeader->size();
       frameDataLength -= d->extendedHeader->size();
     }
   }
@@ -410,8 +410,6 @@ void ID3v2::Tag::parse(const ByteVector &data)
 
   // parse frames
 
-  uint frameDataPosition = 0;
-
   // Make sure that there is at least enough room in the remaining frame data for
   // a frame header.
 
@@ -421,7 +419,6 @@ void ID3v2::Tag::parse(const ByteVector &data)
     // portion of the frame data.
 
     if(data.at(frameDataPosition) == 0) {
-
       if(d->header.footerPresent())
         debug("Padding *and* a footer found.  This is not allowed by the spec.");
 
@@ -429,7 +426,7 @@ void ID3v2::Tag::parse(const ByteVector &data)
       return;
     }
 
-    Frame *frame = d->factory->createFrame(data.mid(frameDataOffset + frameDataPosition),
+    Frame *frame = d->factory->createFrame(data.mid(frameDataPosition),
                                            d->header.majorVersion());
 
     if(!frame)