Fixed handling of UnknownFrames in ID3v2.

- If an unknown frame with id "XXXX" occurs, an entry
"UNKNOWN/XXXX" is added to unsupportedData().
The removeUnsupportedProperties() method in turn
removes all unknown frames with id "XXXX" if it
encounters a string "UNKNOWN/XXXX" in the given list.

- Implemented findByDescription() to UnsynchronizedLyricsFrame
in order to support removal of lyrics frames with unsupported
keys.

- Adapted id3v2 test case to new QuodLibet policy.

taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp

@@ -27,6 +27,7 @@
 
 #include "unsynchronizedlyricsframe.h"
 #include <tbytevectorlist.h>
+#include <id3v2tag.h>
 #include <tdebug.h>
 #include <tpropertymap.h>
 
@@ -125,6 +126,17 @@ PropertyMap UnsynchronizedLyricsFrame::asProperties() const
   return map;
 }
 
+UnsynchronizedLyricsFrame *UnsynchronizedLyricsFrame::findByDescription(const ID3v2::Tag *tag, const String &d) // static
+{
+  ID3v2::FrameList lyrics = tag->frameList("USLT");
+
+  for(ID3v2::FrameList::ConstIterator it = lyrics.begin(); it != lyrics.end(); ++it){
+    UnsynchronizedLyricsFrame *frame = dynamic_cast<UnsynchronizedLyricsFrame *>(*it);
+    if(frame && frame->description() == d)
+      return frame;
+  }
+  return 0;
+}
 ////////////////////////////////////////////////////////////////////////////////
 // protected members
 ////////////////////////////////////////////////////////////////////////////////

taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.h

@@ -147,6 +147,15 @@ namespace TagLib {
        */
       PropertyMap asProperties() const;
 
+      /*!
+       * LyricsFrames each have a unique description.  This searches for a lyrics
+       * frame with the decription \a d and returns a pointer to it.  If no
+       * frame is found that matches the given description null is returned.
+       *
+       * \see description()
+       */
+      static UnsynchronizedLyricsFrame *findByDescription(const Tag *tag, const String &d);
+
     protected:
       // Reimplementations.
 

taglib/mpeg/id3v2/id3v2frame.cpp

@@ -43,6 +43,7 @@
 #include "frames/urllinkframe.h"
 #include "frames/unsynchronizedlyricsframe.h"
 #include "frames/commentsframe.h"
+#include "frames/unknownframe.h"
 
 using namespace TagLib;
 using namespace ID3v2;
@@ -429,18 +430,16 @@ ByteVector Frame::keyToFrameID(const String &s)
 
 PropertyMap Frame::asProperties() const
 {
+  if(dynamic_cast< const UnknownFrame *>(this)) {
+    PropertyMap m;
+    m.unsupportedData().append("UNKNOWN/" + frameID());
+    return m;
+  }
   const ByteVector &id = frameID();
   // workaround until this function is virtual
-  if(id == "TXXX") {
-    const UserTextIdentificationFrame *txxxFrame = dynamic_cast< const UserTextIdentificationFrame* >(this);
-    if(txxxFrame != NULL)
-      return txxxFrame->asProperties();
-    else {
-      PropertyMap m;
-      m.unsupportedData().append("UNKNOWN");
-      return m;
-    }
-  } else if(id[0] == 'T')
+  if(id == "TXXX")
+    return dynamic_cast< const UserTextIdentificationFrame* >(this)->asProperties();
+  else if(id[0] == 'T')
     return dynamic_cast< const TextIdentificationFrame* >(this)->asProperties();
   else if(id == "WXXX")
     return dynamic_cast< const UserUrlLinkFrame* >(this)->asProperties();

taglib/mpeg/id3v2/id3v2tag.cpp

@@ -346,35 +346,36 @@ PropertyMap ID3v2::Tag::properties() const
 
 void ID3v2::Tag::removeUnsupportedProperties(const StringList &properties)
 {
-  // entries of unsupportedData() are usually frame IDs which are not supported
-  // by the PropertyMap interface. Three special cases exist: TXXX, WXXX, and COMM
-  // frames may also be unsupported if their description() is not a valid key.
   for(StringList::ConstIterator it = properties.begin(); it != properties.end(); ++it){
-    if(*it == "UNKNOWN") {
-      // delete all unknown frames
-      FrameList l = frameList();
+    if(it->startsWith("UNKNOWN/")) {
+      String frameID = it->substr(String("UNKNOWN/").size());
+      if(frameID.size() != 4)
+        continue; // invalid specification
+      ByteVector id = frameID.data(String::Latin1);
+      // delete all unknown frames of given type
+      FrameList l = frameList(id);
       for(FrameList::ConstIterator fit = l.begin(); fit != l.end(); fit++)
         if (dynamic_cast<const UnknownFrame *>(*fit) != NULL)
           removeFrame(*fit);
+    } else if(it->size() == 4){
+      ByteVector id = it->data(String::Latin1);
+      removeFrames(id);
     } else {
       ByteVector id = it->substr(0,4).data(String::Latin1);
-      if(id == "TXXX") {
-        String description = it->substr(5);
-        Frame *frame = UserTextIdentificationFrame::find(this, description);
-        if(frame)
-          removeFrame(frame);
-      } else if(id == "WXXX") {
-        String description = it->substr(5);
-        Frame *frame = UserUrlLinkFrame::find(this, description);
-        if(frame)
-          removeFrame(frame);
-      } else if(id == "COMM") {
-        String description = it->substr(5);
-        Frame *frame = CommentsFrame::findByDescription(this, description);
-        if(frame)
-          removeFrame(frame);
-      } else
-        removeFrames(id); // there should be only one frame with "id"
+      if(it->size() <= 5)
+        continue; // invalid specification
+      String description = it->substr(5);
+      Frame *frame;
+      if(id == "TXXX")
+        frame = UserTextIdentificationFrame::find(this, description);
+      else if(id == "WXXX")
+        frame = UserUrlLinkFrame::find(this, description);
+      else if(id == "COMM")
+        frame = CommentsFrame::findByDescription(this, description);
+      else if(id == "USLT")
+        frame = UnsynchronizedLyricsFrame::findByDescription(this, description);
+      if(frame)
+        removeFrame(frame);
     }
   }
 }

taglib/mpeg/id3v2/id3v2tag.h

@@ -293,9 +293,14 @@ namespace TagLib {
 
       /*!
        * Removes unsupported frames given by \a properties. The elements of
-       * \a properties must be taken from properties().unsupportedData() and
-       * are the four-byte frame IDs of ID3 frames which are not compatible
-       * with the PropertyMap schema.
+       * \a properties must be taken from properties().unsupportedData(); they
+       * are of one of the following forms:
+       *  - a four-character frame ID, if the ID3 specification allows only one
+       *    frame with that ID (thus, the frame is uniquely determined)
+       *  - frameID + "/" + description(), when the ID is one of "TXXX", "WXXX",
+       *    "COMM", or "USLT",
+       *  - "UNKNOWN/" + frameID, for frames that could not be parsed by TagLib.
+       *    In that case, *all* unknown frames with the given ID will be removed.
        */
       void removeUnsupportedProperties(const StringList &properties);
 

tests/test_id3v2.cpp

@@ -558,10 +558,15 @@ public:
     MPEG::File f(newname.c_str());
     PropertyMap dict = f.ID3v2Tag(false)->properties();
     CPPUNIT_ASSERT_EQUAL(uint(6), dict.size());
+
+    CPPUNIT_ASSERT(dict.contains("USERTEXTDESCRIPTION1"));
+    CPPUNIT_ASSERT(dict.contains("QuodLibet::USERTEXTDESCRIPTION2"));
+    CPPUNIT_ASSERT_EQUAL(uint(2), dict["USERTEXTDESCRIPTION1"].size());
+    CPPUNIT_ASSERT_EQUAL(uint(2), dict["QuodLibet::USERTEXTDESCRIPTION2"].size());
     CPPUNIT_ASSERT_EQUAL(String("userTextData1"), dict["USERTEXTDESCRIPTION1"][0]);
     CPPUNIT_ASSERT_EQUAL(String("userTextData2"), dict["USERTEXTDESCRIPTION1"][1]);
-    CPPUNIT_ASSERT_EQUAL(String("userTextData1"), dict["USERTEXTDESCRIPTION2"][0]);
-    CPPUNIT_ASSERT_EQUAL(String("userTextData2"), dict["USERTEXTDESCRIPTION2"][1]);
+    CPPUNIT_ASSERT_EQUAL(String("userTextData1"), dict["QuodLibet::USERTEXTDESCRIPTION2"][0]);
+    CPPUNIT_ASSERT_EQUAL(String("userTextData2"), dict["QuodLibet::USERTEXTDESCRIPTION2"][1]);
 
     CPPUNIT_ASSERT_EQUAL(String("Pop"), dict["GENRE"].front());