TCON parsing fixes

 * Don't join multiple 2.4 TCON fields together
 * Don't add duplicate fields for 2.3 genres where the refinement matches the ID3v1 genre

Patch by Michael Smith
BUG:188578

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

diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp
index 8faaa5412428f881d27406a5119e8d8b975ad124..18da4c3eb759a40af476ada7b89ab8ed4fa2a3b9 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2framefactory.cpp
+++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp
@@ -31,6 +31,7 @@
 
 #include "id3v2framefactory.h"
 #include "id3v2synchdata.h"
+#include "id3v1genres.h"
 
 #include "frames/attachedpictureframe.h"
 #include "frames/commentsframe.h"
@@ -394,26 +395,31 @@ void FrameFactory::convertFrame(const char *from, const char *to,
 
 void FrameFactory::updateGenre(TextIdentificationFrame *frame) const
 {
-  StringList fields;
-  String s = frame->toString();
-
-  while(s.startsWith("(")) {
-
-    int closing = s.find(")");
-
-    if(closing < 0)
-      break;
-
-    fields.append(s.substr(1, closing - 1));
-
-    s = s.substr(closing + 1);
+  StringList fields = frame->fieldList();
+  StringList newfields;
+
+  for(StringList::Iterator it = fields.begin(); it != fields.end(); ++it) {
+    String s = *it;
+    int end = s.find(")");
+
+    if(s.startsWith("(") && end > 0) {
+      // "(12)Genre"
+      String text = s.substr(end + 1);
+      int number = s.substr(1, end - 1).toInt();
+      if (number > 0 && number <= 255 && !(ID3v1::genre(number) == text))
+        newfields.append(s.substr(1, end - 1));
+      if (!text.isEmpty())
+        newfields.append(text);
+    }
+    else {
+      // "Genre" or "12"
+      newfields.append(s);
+    }
   }
 
-  if(!s.isEmpty())
-    fields.append(s);
-
-  if(fields.isEmpty())
+  if(newfields.isEmpty())
     fields.append(String::null);
 
-  frame->setText(fields);
+  frame->setText(newfields);
+
 }

diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 07b33f1e99b399b96a7499c0c0d0a8a50cde49d6..74701781cc2cd04d3fbe9fead0f7fee98d0ba64c 100644 (file)
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -11,6 +11,7 @@
 #include <relativevolumeframe.h>
 #include <popularimeterframe.h>
 #include <urllinkframe.h>
+#include <tdebug.h>
 #include "utils.h"
 
 using namespace std;
@@ -53,6 +54,9 @@ class TestID3v2 : public CppUnit::TestFixture
   CPPUNIT_TEST(testParseUserUrlLinkFrame);
   CPPUNIT_TEST(testRenderUserUrlLinkFrame);
   CPPUNIT_TEST(testSaveUTF16Comment);
+  CPPUNIT_TEST(testUpdateGenre23_1);
+  CPPUNIT_TEST(testUpdateGenre23_2);
+  CPPUNIT_TEST(testUpdateGenre24);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -336,6 +340,64 @@ public:
     ID3v2::FrameFactory::instance()->setDefaultTextEncoding(defaultEncoding);
   }
 
+  void testUpdateGenre23_1()
+  {
+    // "Refinement" is the same as the ID3v1 genre - duplicate
+    ID3v2::FrameFactory *factory = ID3v2::FrameFactory::instance();
+    ByteVector data = ByteVector("TCON"                 // Frame ID
+                                 "\x00\x00\x00\x10"     // Frame size
+                                 "\x00\x00"             // Frame flags
+                                 "\x00"                 // Encoding
+                                 "(22)Death Metal", 26);     // Text
+    ID3v2::TextIdentificationFrame *frame =
+        static_cast<TagLib::ID3v2::TextIdentificationFrame*>(factory->createFrame(data, TagLib::uint(3)));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), frame->fieldList().size());
+    CPPUNIT_ASSERT_EQUAL(String("Death Metal"), frame->fieldList()[0]);
+
+    ID3v2::Tag tag;
+    tag.addFrame(frame);
+    CPPUNIT_ASSERT_EQUAL(String("Death Metal"), tag.genre());
+  }
+
+  void testUpdateGenre23_2()
+  {
+    // "Refinement" is different from the ID3v1 genre
+    ID3v2::FrameFactory *factory = ID3v2::FrameFactory::instance();
+    ByteVector data = ByteVector("TCON"                 // Frame ID
+                                 "\x00\x00\x00\x13"     // Frame size
+                                 "\x00\x00"             // Frame flags
+                                 "\x00"                 // Encoding
+                                 "(4)Eurodisco", 23);   // Text
+    ID3v2::TextIdentificationFrame *frame =
+        static_cast<TagLib::ID3v2::TextIdentificationFrame*>(factory->createFrame(data, TagLib::uint(3)));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(2), frame->fieldList().size());
+    CPPUNIT_ASSERT_EQUAL(String("4"), frame->fieldList()[0]);
+    CPPUNIT_ASSERT_EQUAL(String("Eurodisco"), frame->fieldList()[1]);
+
+    ID3v2::Tag tag;
+    tag.addFrame(frame);
+    CPPUNIT_ASSERT_EQUAL(String("Disco Eurodisco"), tag.genre());
+  }
+
+  void testUpdateGenre24()
+  {
+    ID3v2::FrameFactory *factory = ID3v2::FrameFactory::instance();
+    ByteVector data = ByteVector("TCON"                   // Frame ID
+                                 "\x00\x00\x00\x0D"       // Frame size
+                                 "\x00\x00"               // Frame flags
+                                 "\0"                   // Encoding
+                                 "14\0Eurodisco", 23);     // Text
+    ID3v2::TextIdentificationFrame *frame =
+        static_cast<TagLib::ID3v2::TextIdentificationFrame*>(factory->createFrame(data, TagLib::uint(4)));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(2), frame->fieldList().size());
+    CPPUNIT_ASSERT_EQUAL(String("14"), frame->fieldList()[0]);
+    CPPUNIT_ASSERT_EQUAL(String("Eurodisco"), frame->fieldList()[1]);
+
+    ID3v2::Tag tag;
+    tag.addFrame(frame);
+    CPPUNIT_ASSERT_EQUAL(String("R&B Eurodisco"), tag.genre());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v2);