Add unit tests

http://bugs.kde.org/show_bug.cgi?id=254223

NEWS

@@ -4,6 +4,7 @@ TagLib 1.8 (In Development)
  * Support for writing ID3v2.3 tags.
  * Added methods for checking if WMA and MP4 files are DRM-protected.
  * Started using atomic int operations for reference counting.
+ * Find APE tags even if there's a Lyrics3v2 tag present (BUG:254223).
 
 TagLib 1.7 (Mar 11, 2011)
 =========================

taglib/mpeg/mpegfile.cpp

@@ -56,7 +56,9 @@ public:
     ID3v1Location(-1),
     hasID3v2(false),
     hasID3v1(false),
+    hasLyrics3v2(false),
     hasAPE(false),
+    lyrics3v2Size(0),
     properties(0)
   {
 
@@ -85,7 +87,9 @@ public:
 
   bool hasID3v2;
   bool hasID3v1;
+  bool hasLyrics3v2;
   bool hasAPE;
+  long lyrics3v2Size;
 
   Properties *properties;
 };
@@ -198,6 +202,10 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
       if(ID3v1Tag())
         d->ID3v1Location = findID3v1();
 
+      // Lyrics3v2 tag location has changed, update if it exists
+      
+      findLyrics3v2();
+
       // APE tag location has changed, update if it exists
 
       if(APETag())
@@ -226,8 +234,9 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
   // Dont save an APE-tag unless one has been created
 
   if((APE & tags) && APETag()) {
-    if(d->hasAPE)
+    if(d->hasAPE) {
       insert(APETag()->render(), d->APELocation, d->APEOriginalSize);
+    }
     else {
       if(d->hasID3v1) {
         insert(APETag()->render(), d->ID3v1Location, 0);
@@ -249,8 +258,9 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
       }
     }
   }
-  else if(d->hasAPE && stripOthers)
+  else if(d->hasAPE && stripOthers) {
     success = strip(APE, false) && success;
+  }
 
   return success;
 }
@@ -296,6 +306,10 @@ bool MPEG::File::strip(int tags, bool freeMemory)
     if(ID3v1Tag())
       d->ID3v1Location = findID3v1();
 
+    // Lyrics3v2 tag location has changed, update if it exists
+    
+    findLyrics3v2();
+
     // APE tag location has changed, update if it exists
 
    if(APETag())
@@ -432,6 +446,10 @@ void MPEG::File::read(bool readProperties, Properties::ReadStyle propertiesStyle
     d->tag.set(ID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
     d->hasID3v1 = true;
   }
+  
+  // Look for a Lyrics3v2 tag
+  
+  findLyrics3v2();
 
   // Look for an APE tag
 
@@ -576,10 +594,41 @@ long MPEG::File::findID3v1()
   return -1;
 }
 
+void MPEG::File::findLyrics3v2()
+{
+  if(isValid()) {
+    long offset = -9;
+    if( d->hasID3v1 )
+      offset -= 128;
+    seek(offset, End);
+
+    if(readBlock(9) == "LYRICS200") {
+      d->hasLyrics3v2 = true;
+      
+      offset -= 6;
+      seek(offset, End);
+      
+      ByteVector sizeVector = readBlock(6);
+      d->lyrics3v2Size = 15 + readNumber(sizeVector);
+      
+      return;
+    }
+  }
+  
+  d->hasLyrics3v2 = false;
+  d->lyrics3v2Size = 0;
+}
+
 void MPEG::File::findAPE()
 {
   if(isValid()) {
-    seek(d->hasID3v1 ? -160 : -32, End);
+    long offset = -32;
+    if(d->hasID3v1)
+      offset -= 128;
+    if(d->hasLyrics3v2)
+      offset -= d->lyrics3v2Size;
+    
+    seek(offset, End);
 
     long p = tell();
 
@@ -597,6 +646,58 @@ void MPEG::File::findAPE()
   d->APEFooterLocation = -1;
 }
 
+long MPEG::File::readNumber(ByteVector vector)
+{
+  long number = 0;
+  int exp = 1;
+  int value = 0;
+  for(int i=vector.size(); i>0; i--) {
+    switch(vector.at(i-1)) {
+      case '1': {
+        value = 1;
+        break;
+      }
+      case '2': {
+        value = 2;
+        break;
+      }
+      case '3': {
+        value = 3;
+        break;
+      }
+      case '4': {
+        value = 4;
+        break;
+      }
+      case '5': {
+        value = 5;
+        break;
+      }
+      case '6': {
+        value = 6;
+        break;
+      }
+      case '7': {
+        value = 7;
+        break;
+      }
+      case '8': {
+        value = 8;
+        break;
+      }
+      case '9': {
+        value = 9;
+        break;
+      }
+      default:
+        value = 0;
+    }
+    number += value * exp;
+    exp *= 10;
+  }
+  return number;
+}
+
 bool MPEG::File::secondSynchByte(char byte)
 {
   if(uchar(byte) == 0xff)

taglib/mpeg/mpegfile.h

@@ -287,8 +287,14 @@ namespace TagLib {
       void read(bool readProperties, Properties::ReadStyle propertiesStyle);
       long findID3v2();
       long findID3v1();
+      void findLyrics3v2();
       void findAPE();
 
+      /*!
+       * Extracts a number from a ByteVector
+       */
+      long readNumber(ByteVector vector);
+
       /*!
        * MPEG frames can be recognized by the bit pattern 11111111 111, so the
        * first byte is easy to check for, however checking to see if the second byte

tests/test_mpeg.cpp

@@ -3,6 +3,7 @@
 #include <stdio.h>
 #include <mpegfile.h>
 #include <id3v2tag.h>
+#include <apetag.h>
 #include "utils.h"
 
 using namespace std;
@@ -15,6 +16,11 @@ class TestMPEG : public CppUnit::TestFixture
   CPPUNIT_TEST(testSaveID3v24);
   CPPUNIT_TEST(testSaveID3v24WrongParam);
   CPPUNIT_TEST(testSaveID3v23);
+  CPPUNIT_TEST(testReadAPEv2);
+  CPPUNIT_TEST(testReadAPEv2WithLyrics3v2);
+  CPPUNIT_TEST(testSaveAPEv2);
+  CPPUNIT_TEST(testSaveAPEv2WithLyrics3v2);
+  CPPUNIT_TEST(testSaveAPEv2EmptyWithLyrics3v2);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -76,6 +82,75 @@ public:
     CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
   }
 
+  void testReadAPEv2()
+  {
+    ScopedFileCopy copy("apetag-replaygain", ".mp3");
+    string newname = copy.fileName();
+
+    MPEG::File f(newname.c_str());
+    String s = f.APETag()->itemListMap()["MP3GAIN_ALBUM_MINMAX"].toString();
+    CPPUNIT_ASSERT_EQUAL(String("129,170"), s);
+  }
+
+  void testReadAPEv2WithLyrics3v2()
+  {
+    ScopedFileCopy copy("apetag-replaygain-lyrics3v2", ".mp3");
+    string newname = copy.fileName();
+
+    MPEG::File f(newname.c_str());
+    String s = f.APETag()->itemListMap()["MP3GAIN_ALBUM_MINMAX"].toString();
+    CPPUNIT_ASSERT_EQUAL(String("129,170"), s);
+  }
+
+  void testSaveAPEv2()
+  {
+    ScopedFileCopy copy("apetag-replaygain", ".mp3");
+    string newname = copy.fileName();
+
+    MPEG::File f(newname.c_str());
+    f.APETag()->addValue("MP3GAIN_ALBUM_MINMAX", String("xxx"));
+    f.save();
+
+    MPEG::File f2(newname.c_str());
+    String s = f2.APETag()->itemListMap()["MP3GAIN_ALBUM_MINMAX"].toString();
+    CPPUNIT_ASSERT_EQUAL(String("xxx"), s);
+  }
+
+  void testSaveAPEv2WithLyrics3v2()
+  {
+    ScopedFileCopy copy("apetag-replaygain-lyrics3v2", ".mp3");
+    string newname = copy.fileName();
+
+    MPEG::File f(newname.c_str());
+    f.APETag()->addValue("MP3GAIN_ALBUM_MINMAX", String("xxx"));
+    f.save();
+
+    MPEG::File f2(newname.c_str());
+    String s = f2.APETag()->itemListMap()["MP3GAIN_ALBUM_MINMAX"].toString();
+    CPPUNIT_ASSERT_EQUAL(String("xxx"), s);
+    f2.seek(-9, File::End);
+    CPPUNIT_ASSERT_EQUAL(ByteVector("LYRICS200"), f2.readBlock(9));
+  }
+
+  void testSaveAPEv2EmptyWithLyrics3v2()
+  {
+    ScopedFileCopy copy("lyrics3v2", ".mp3", false);
+    string newname = copy.fileName();
+
+    {
+      MPEG::File f(newname.c_str());
+      f.APETag(true)->addValue("MP3GAIN_ALBUM_MINMAX", String("xxx"));
+      f.save();
+    }
+
+    MPEG::File f2(newname.c_str());
+    CPPUNIT_ASSERT(f2.APETag());
+    String s = f2.APETag()->itemListMap()["MP3GAIN_ALBUM_MINMAX"].toString();
+    CPPUNIT_ASSERT_EQUAL(String("xxx"), s);
+    f2.seek(-9, File::End);
+    CPPUNIT_ASSERT_EQUAL(ByteVector("LYRICS200"), f2.readBlock(9));
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);