Further cleanup and simplification in id3v2dicttools

diff --git a/taglib/mpeg/id3v2/id3v2dicttools.cpp b/taglib/mpeg/id3v2/id3v2dicttools.cpp
index 172b4afbefcb51142c676af93d82006861feca3a..498f92edce2e6b15d561a5916c4031a591981847 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2dicttools.cpp
+++ b/taglib/mpeg/id3v2/id3v2dicttools.cpp
@@ -33,30 +33,27 @@
 #include "frames/unsynchronizedlyricsframe.h"
 #include "id3v1genres.h"
 
-using namespace TagLib;
-using namespace ID3v2;
-
-// list of deprecated frames and their successors
-static const uint deprecatedFramesSize = 4;
-static const char *deprecatedFrames[][2] = {
-  {"TRDA", "TDRC"}, // 2.3 -> 2.4 (http://en.wikipedia.org/wiki/ID3)
-  {"TDAT", "TDRC"}, // 2.3 -> 2.4
-  {"TYER", "TDRC"}, // 2.3 -> 2.4
-  {"TIME", "TDRC"}, // 2.3 -> 2.4
-};
-
-FrameIDMap deprecationMap()
-{
-  static FrameIDMap depMap;
-  if (depMap.isEmpty())
-    for(uint i = 0; i < deprecatedFramesSize; ++i)
-      depMap[deprecatedFrames[i][0]] = deprecatedFrames[i][1];
-  return depMap;
-}
-
 namespace TagLib {
   namespace ID3v2 {
 
+    // list of deprecated frames and their successors
+    static const uint deprecatedFramesSize = 4;
+    static const char *deprecatedFrames[][2] = {
+      {"TRDA", "TDRC"}, // 2.3 -> 2.4 (http://en.wikipedia.org/wiki/ID3)
+      {"TDAT", "TDRC"}, // 2.3 -> 2.4
+      {"TYER", "TDRC"}, // 2.3 -> 2.4
+      {"TIME", "TDRC"}, // 2.3 -> 2.4
+    };
+
+    FrameIDMap &deprecationMap()
+    {
+      static FrameIDMap depMap;
+      if (depMap.isEmpty())
+        for(uint i = 0; i < deprecatedFramesSize; ++i)
+          depMap[deprecatedFrames[i][0]] = deprecatedFrames[i][1];
+      return depMap;
+    }
+
     /*!
      * A map of translations frameID <-> tag used by the unified dictionary interface.
      */
@@ -127,32 +124,24 @@ namespace TagLib {
       { "USLT", "LYRICS" },
     };
 
-    // list of frameIDs that are ignored by the unified dictionary interface
-    static const uint ignoredFramesSize = 7;
-    static const char *ignoredFrames[] = {
-      "TCMP", // illegal 'Part of Compilation' frame set by iTunes (see http://www.id3.org/Compliance_Issues)
-      "GEOB", // no way to handle a general encapsulated object by the dict interface
-      "PRIV", // private frames
-      "APIC", // attached picture -- TODO how could we do this?
-      "POPM", // popularimeter
-      "RVA2", // relative volume
-      "UFID", // unique file identifier
-    };
-
-
-    String frameIDToTagName(const ByteVector &id)
+    Map<ByteVector, String> &idMap()
     {
       static Map<ByteVector, String> m;
-      if (m.isEmpty())
-        for (size_t i = 0; i < numid3frames; ++i)
-          m[id3frames[i][0]] = id3frames[i][1];
+        if (m.isEmpty())
+          for (size_t i = 0; i < numid3frames; ++i)
+            m[id3frames[i][0]] = id3frames[i][1];
+      return m;
+    }
 
+    String frameIDToTagName(const ByteVector &id)
+    {
+      Map<ByteVector, String> &m = idMap();
       if (m.contains(id))
         return m[id];
       if (deprecationMap().contains(id))
         return m[deprecationMap()[id]];
-      debug("unknown frame ID: " + id);
-      return "UNKNOWNID3TAG"; //TODO: implement this nicer
+      debug("unknown frame ID in frameIDToTagName(): " + id);
+      return "UNKNOWNID3TAG#" + String(id) + "#"; //TODO: implement this nicer
     }
 
     ByteVector tagNameToFrameID(const String &s)
@@ -166,18 +155,12 @@ namespace TagLib {
       return "TXXX";
     }
 
-    bool isIgnored(const ByteVector& id)
+    bool ignored(const ByteVector& id)
     {
-      List<ByteVector> ignoredList;
-      if (ignoredList.isEmpty())
-        for (uint i = 0; i < ignoredFramesSize; ++i)
-          ignoredList.append(ignoredFrames[i]);
-      return ignoredList.contains(id);
+      return !(id == "TXXX") && !idMap().contains(id) && !deprecated(id);
     }
 
-
-
-    bool isDeprecated(const ByteVector& id)
+    bool deprecated(const ByteVector& id)
     {
       return deprecationMap().contains(id);
     }

diff --git a/taglib/mpeg/id3v2/id3v2dicttools.h b/taglib/mpeg/id3v2/id3v2dicttools.h
index 5963618aa2dbeb1632b5ec32cd6b1a739f621b4e..1f831c635b2e34710a3728c3b7bd47e134c1c86e 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2dicttools.h
+++ b/taglib/mpeg/id3v2/id3v2dicttools.h
@@ -41,7 +41,6 @@ namespace TagLib {
     typedef Map<ByteVector, ByteVector> FrameIDMap;
     typedef std::pair<String, StringList> KeyValuePair;
 
-    // forward declaration
     class Frame;
     /*!
      * Returns an appropriate ID3 frame ID for the given free-form tag name. This method
@@ -58,11 +57,27 @@ namespace TagLib {
     /*!
      * Tell if the given frame ID is ignored by the unified dictionary subsystem. This is true
      * for frames that don't admit a textual representation, such as pictures or other binary
-     * information.
+     * information, as well as invalid frames that violate the ID3 specification.
+     *
+     * These include:
+     * - illegal frames violating the specification but seem to be used by some applications, e.g.
+     * "TCMP", illegal 'Part of Compilation' frame set by iTunes (see http://www.id3.org/Compliance_Issues)
+     * "NCON", illegal MusicMatch frame (see http://www.id3.org/Compliance_Issues)
+     *
+     *  - frames without a meaningful textual representation -- might be implemented in some future release
+     * "GEOB", no way to handle a general encapsulated object by the dict interface
+     * "PRIV", private frames
+     * "APIC", attached picture
+     * "POPM", popularimeter
+     * "RVA2", relative volume
+     * "UFID", unique file identifier
      */
-    bool TAGLIB_EXPORT isIgnored(const ByteVector &);
+    bool TAGLIB_EXPORT ignored(const ByteVector &);
 
-    bool TAGLIB_EXPORT isDeprecated(const ByteVector&);
+    /*!
+     * Returns true if the given frame ID is deprecated according to the most recent ID3v2 standard.
+     */
+    bool TAGLIB_EXPORT deprecated(const ByteVector&);
 
     /*!
      * Parse the ID3v2::Frame *Frame* to a pair of a human-readable key (e.g. ARTIST) and

diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp
index 84c89a4785f93f569c4ca9a3ce0cd08f42c9599a..9c5e50f41b9fa643e5d50061c5d1b236cb94ace8 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2tag.cpp
+++ b/taglib/mpeg/id3v2/id3v2tag.cpp
@@ -40,6 +40,7 @@
 #include "frames/urllinkframe.h"
 #include "frames/uniquefileidentifierframe.h"
 #include "frames/unsynchronizedlyricsframe.h"
+#include "frames/unknownframe.h"
 
 using namespace TagLib;
 using namespace ID3v2;
@@ -340,9 +341,9 @@ TagDict ID3v2::Tag::toDict() const
   for (; frameIt != frameList().end(); ++frameIt) {
     ByteVector id = (*frameIt)->frameID();
 
-    if (isIgnored(id))
+    if (ignored(id))
       debug("toDict() found ignored id3 frame: " + id);
-    else if (isDeprecated(id))
+    else if (deprecated(id))
       debug("toDict() found deprecated id3 frame: " + id);
     else {
         // in the future, something like dict[frame->tagName()].append(frame->values())
@@ -362,7 +363,7 @@ void ID3v2::Tag::fromDict(const TagDict &dict)
   //
   for (FrameListMap::ConstIterator it = frameListMap().begin(); it != frameListMap().end(); ++it)
     // Remove all frames which are not ignored
-    if (it->second.size() == 0 || !isIgnored(it->first))
+    if (it->second.size() == 0 || !ignored(it->first) )
       toRemove.append(it->second);
 
   for (FrameList::ConstIterator it = toRemove.begin(); it != toRemove.end(); it++)