Update the TCON (genre) frame as we create it so that we don't keep ID3v2.3

formatted data sitting around waiting to be written to 2.4 tags.

BUG:132018

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

diff --git a/mpeg/id3v2/frames/textidentificationframe.cpp b/mpeg/id3v2/frames/textidentificationframe.cpp
index d2cebc12608a12f7b3d6085c9d2240a9d23d53c5..26afe10cdc6b7c2211737a0fb0ade2820d4d0364 100644 (file)
--- a/mpeg/id3v2/frames/textidentificationframe.cpp
+++ b/mpeg/id3v2/frames/textidentificationframe.cpp
@@ -215,7 +215,8 @@ void UserTextIdentificationFrame::setDescription(const String &s)
   TextIdentificationFrame::setText(l);
 }
 
-UserTextIdentificationFrame *UserTextIdentificationFrame::find(ID3v2::Tag *tag, const String &description) // static
+UserTextIdentificationFrame *UserTextIdentificationFrame::find(
+  ID3v2::Tag *tag, const String &description) // static
 {
   FrameList l = tag->frameList("TXXX");
   for(FrameList::Iterator it = l.begin(); it != l.end(); ++it) {
@@ -239,7 +240,7 @@ UserTextIdentificationFrame::UserTextIdentificationFrame(const ByteVector &data,
 void UserTextIdentificationFrame::checkFields()
 {
   int fields = fieldList().size();
-
+    
   if(fields == 0)
     setDescription(String::null);
   if(fields <= 1)

diff --git a/mpeg/id3v2/id3v2framefactory.cpp b/mpeg/id3v2/id3v2framefactory.cpp
index 1755b4d3c7f23ce4b61022f1d9987dcd3d1233b3..1fee20876c70b9d8770db0fa05bf7afea3bbee34 100644 (file)
--- a/mpeg/id3v2/id3v2framefactory.cpp
+++ b/mpeg/id3v2/id3v2framefactory.cpp
@@ -121,6 +121,10 @@ Frame *FrameFactory::createFrame(const ByteVector &data, uint version) const
 
     if(d->useDefaultEncoding)
       f->setTextEncoding(d->defaultEncoding);
+
+    if(frameID == "TCON")
+      updateGenre(f);
+
     return f;
   }
 
@@ -311,3 +315,29 @@ void FrameFactory::convertFrame(const char *from, const char *to,
 
   header->setFrameID(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);
+  }
+
+  if(!s.isEmpty())
+    fields.append(s);
+
+  if(fields.isEmpty())
+    fields.append(String::null);
+
+  frame->setText(fields);
+}

diff --git a/mpeg/id3v2/id3v2framefactory.h b/mpeg/id3v2/id3v2framefactory.h
index 4a0d11b4b4956927cc5baa862a2c0437169f0f38..a002d9616c8f181f98961bab0fdf617700a3a615 100644 (file)
--- a/mpeg/id3v2/id3v2framefactory.h
+++ b/mpeg/id3v2/id3v2framefactory.h
@@ -29,6 +29,8 @@ namespace TagLib {
 
   namespace ID3v2 {
 
+    class TextIdentificationFrame;
+
     //! A factory for creating ID3v2 frames
 
     /*!
@@ -128,6 +130,8 @@ namespace TagLib {
       void convertFrame(const char *from, const char *to,
                         Frame::Header *header) const;
 
+      void updateGenre(TextIdentificationFrame *frame) const;
+
       static FrameFactory *factory;
 
       class FrameFactoryPrivate;

diff --git a/mpeg/id3v2/id3v2tag.cpp b/mpeg/id3v2/id3v2tag.cpp
index 1032489fe87b38a4e3ce0558fdedeac3f8119768..6e7a3b9231e2d37047c65bf500268d1ce58222c3 100644 (file)
--- a/mpeg/id3v2/id3v2tag.cpp
+++ b/mpeg/id3v2/id3v2tag.cpp
@@ -124,69 +124,53 @@ String ID3v2::Tag::genre() const
   // should be separated by " / " instead of " ".  For the moment to keep
   // the behavior the same as released versions it is being left with " ".
 
-  if(!d->frameListMap["TCON"].isEmpty() &&
-     dynamic_cast<TextIdentificationFrame *>(d->frameListMap["TCON"].front()))
+  if(d->frameListMap["TCON"].isEmpty() ||
+     !dynamic_cast<TextIdentificationFrame *>(d->frameListMap["TCON"].front()))
   {
-    Frame *frame = d->frameListMap["TCON"].front();
-
-    // ID3v2.4 lists genres as the fields in its frames field list.  If the field
-    // is simply a number it can be assumed that it is an ID3v1 genre number.
-    // Here was assume that if an ID3v1 string is present that it should be
-    // appended to the genre string.  Multiple fields will be appended as the
-    // string is built.
-
-    if(d->header.majorVersion() == 4) {
-      TextIdentificationFrame *f = static_cast<TextIdentificationFrame *>(frame);
-      StringList fields = f->fieldList();
-
-      String genreString;
-      bool hasNumber = false;
-
-      for(StringList::ConstIterator it = fields.begin(); it != fields.end(); ++it) {
-        bool isNumber = true;
-        for(String::ConstIterator charIt = (*it).begin();
-            isNumber && charIt != (*it).end();
-            ++charIt)
-        {
-          isNumber = *charIt >= '0' && *charIt <= '9';
-        }
-
-        if(!genreString.isEmpty())
-          genreString.append(' ');
-
-        if(isNumber) {
-          int number = (*it).toInt();
-          if(number >= 0 && number <= 255) {
-            hasNumber = true;
-            genreString.append(ID3v1::genre(number));
-          }
-        }
-        else
-          genreString.append(*it);
-      }
-      if(hasNumber)
-        return genreString;
+    return String::null;
+  }
+
+  // ID3v2.4 lists genres as the fields in its frames field list.  If the field
+  // is simply a number it can be assumed that it is an ID3v1 genre number.
+  // Here was assume that if an ID3v1 string is present that it should be
+  // appended to the genre string.  Multiple fields will be appended as the
+  // string is built.
+
+  TextIdentificationFrame *f = static_cast<TextIdentificationFrame *>(
+    d->frameListMap["TCON"].front());
+
+  StringList fields = f->fieldList();
+
+  String genreString;
+  bool hasNumber = false;
+
+  for(StringList::ConstIterator it = fields.begin(); it != fields.end(); ++it) {
+
+    bool isNumber = true;
+
+    for(String::ConstIterator charIt = (*it).begin();
+        isNumber && charIt != (*it).end();
+        ++charIt)
+    {
+      isNumber = *charIt >= '0' && *charIt <= '9';
     }
 
-    String s = frame->toString();
-
-    // ID3v2.3 "content type" can contain a ID3v1 genre number in parenthesis at
-    // the beginning of the field.  If this is all that the field contains, do a
-    // translation from that number to the name and return that.  If there is a
-    // string folloing the ID3v1 genre number, that is considered to be
-    // authoritative and we return that instead.  Or finally, the field may
-    // simply be free text, in which case we just return the value.
-
-    int closing = s.find(")");
-    if(s.substr(0, 1) == "(" && closing > 0) {
-      if(closing == int(s.size() - 1))
-        return ID3v1::genre(s.substr(1, s.size() - 2).toInt());
-      else
-        return s.substr(closing + 1);
+    if(!genreString.isEmpty())
+      genreString.append(' ');
+
+    if(isNumber) {
+      int number = (*it).toInt();
+      if(number >= 0 && number <= 255) {
+        hasNumber = true;
+        genreString.append(ID3v1::genre(number));
+      }
+    }
+    else {
+      genreString.append(*it);
     }
-    return s;
   }
-  return String::null;
+
+  return genreString;
 }
 
 TagLib::uint ID3v2::Tag::year() const

diff --git a/toolkit/tstring.cpp b/toolkit/tstring.cpp
index 5d0cf41acf4b80b0fa59fc0d69922eebca3447ac..1371a2218177a521614dc87f0d560de065c1e615 100644 (file)
--- a/toolkit/tstring.cpp
+++ b/toolkit/tstring.cpp
@@ -287,6 +287,14 @@ int String::find(const String &s, int offset) const
     return -1;
 }
 
+bool String::startsWith(const String &s) const
+{
+  if(s.length() > length())
+    return false;
+
+  return substr(0, s.length()) == s;
+}
+
 String String::substr(uint position, uint n) const
 {
   if(n > position + d->data.size())
@@ -325,6 +333,11 @@ TagLib::uint String::size() const
   return d->data.size();
 }
 
+TagLib::uint String::length() const
+{
+  return size();
+}
+
 bool String::isEmpty() const
 {
   return d->data.size() == 0;

diff --git a/toolkit/tstring.h b/toolkit/tstring.h
index 66e71e9124f047908bef19b1438108ea90cc45ec..d36498685c629eb0e881b9290ccf273bdeedc521 100644 (file)
--- a/toolkit/tstring.h
+++ b/toolkit/tstring.h
@@ -219,6 +219,11 @@ namespace TagLib {
      */
     int find(const String &s, int offset = 0) const;
 
+    /*!
+     * Returns true if the strings starts with the substring \a s.
+     */
+    bool startsWith(const String &s) const;
+
     /*!
      * Extract a substring from this string starting at \a position and
      * continuing for \a n characters.
@@ -243,6 +248,11 @@ namespace TagLib {
      */
     uint size() const;
 
+    /*!
+     * Returns the length of the string.  Equivalent to size().
+     */
+    uint length() const;
+
     /*!
      * Returns true if the string is empty.
      *