Merge pull request #208 from Rachus/taglib2

A few Matroska testcases and an issue

taglib/ebml/ebmlelement.cpp

@@ -101,7 +101,7 @@ public:
   // for the id)
   ByteVector createVInt(ulli number, bool addOne = true, bool shortest = true)
   {
-    ByteVector vint = ByteVector::fromUInt64BE(static_cast<signed long long>(number));
+    ByteVector vint = ByteVector::fromUInt64BE(number);
     
     // Do we actually need to calculate the length of the variable length
     // integer? If not, then prepend the 0b0000 0001 if necessary and return the
@@ -388,7 +388,7 @@ EBML::Element *EBML::Element::addElement(EBML::ulli id, signed long long number)
 
 EBML::Element *EBML::Element::addElement(EBML::ulli id, EBML::ulli number)
 {
-  return addElement(id, ByteVector::fromUInt64BE(static_cast<signed long long>(number)));
+  return addElement(id, ByteVector::fromUInt64BE(number));
 }
 
 EBML::Element *EBML::Element::addElement(EBML::ulli id, long double number)
@@ -457,7 +457,7 @@ void EBML::Element::setAsInt(signed long long number)
 
 void EBML::Element::setAsUnsigned(EBML::ulli number)
 {
-  setAsBinary(ByteVector::fromUInt64BE(static_cast<signed long long>(number)));
+  setAsBinary(ByteVector::fromUInt64BE(number));
 }
 
 void EBML::Element::setAsFloat(long double)

taglib/ebml/ebmlfile.cpp

@@ -40,7 +40,7 @@ public:
   {
     document->seek(0);
     ByteVector magical = document->readBlock(4);
-    if(static_cast<ulli>(magical.toInt64BE(0)) != Header::EBML)
+    if(static_cast<ulli>(magical.toUInt32BE(0)) != Header::EBML)
       return 0;
     FilePrivate *d = new FilePrivate(document);
     Element *head = d->root.getChild(Header::EBML);

taglib/ebml/matroska/ebmlmatroskaaudio.cpp

@@ -44,10 +44,11 @@ public:
     Element *value;
     
     if(info && (value = info->getChild(Constants::Duration))) {
-      length = static_cast<int>(value->getAsFloat());
-      if((value = info->getChild(Constants::TimecodeScale))){
-        length = static_cast<int>(length / (value->getAsUnsigned() * (1.0 / 1000000000)));
-      }
+      length = static_cast<int>(value->getAsFloat() / 1000000000.L);
+      if((value = info->getChild(Constants::TimecodeScale)))
+        length *= value->getAsUnsigned();
+      else
+        length *= 1000000;
     }
     
     info = elem->getChild(Constants::Tracks);
@@ -59,7 +60,7 @@ public:
     
     // Dirty bitrate:
     document->seek(0, File::End);
-    bitrate = static_cast<int>(8 * document->tell() / length / 1000);
+    bitrate = static_cast<int>(8 * document->tell() / ((length > 0) ? length : 1));
     
     if((value = info->getChild(Constants::Channels)))
       channels = static_cast<int>(value->getAsUnsigned());

taglib/ebml/matroska/ebmlmatroskafile.cpp

@@ -64,8 +64,8 @@ public:
     List<Element *> entries = elem->getChildren(Constants::Tag);
     for(List<Element *>::Iterator i = entries.begin(); i != entries.end(); ++i) {
       Element *target = (*i)->getChild(Constants::Targets);
-      ulli ttvalue = 0;
-      if(target && (target = (*i)->getChild(Constants::TargetTypeValue)))
+      ulli ttvalue = 50; // 50 is default (see spec.)
+      if(target && (target = target->getChild(Constants::TargetTypeValue)))
         ttvalue = target->getAsUnsigned();
       
       // Load all SimpleTags
@@ -126,9 +126,11 @@ public:
 
 EBML::Matroska::File::~File()
 {
-  delete d->tag;
-  delete d->audio;
-  delete d;
+  if (d) {
+    delete d->tag;
+    delete d->audio;
+    delete d;
+  }
 }
 
 EBML::Matroska::File::File(FileName file) : EBML::File(file), d(0)
@@ -196,7 +198,7 @@ bool EBML::Matroska::File::save()
     return false;
   
   // C++11 features would be nice: for(auto &i : d->tags) { /* ... */ }
-  // Well, here we just iterate over each extracted element.
+  // Well, here we just iterate each extracted element.
   for(List<std::pair<PropertyMap, std::pair<Element *, ulli> > >::Iterator i = d->tags.begin();
     i != d->tags.end(); ++i) {
     
@@ -216,7 +218,7 @@ bool EBML::Matroska::File::save()
           target->addElement(Constants::TargetType, Constants::TRACK);
         else if(i->second.second == Constants::MostCommonGroupingValue)
           target->addElement(Constants::TargetType, Constants::ALBUM);
-          
+        
         target->addElement(Constants::TargetTypeValue, i->second.second);
       }
       
@@ -373,6 +375,7 @@ public:
       if(i->second.second == ttv)
         return i;
     }
+    return document->d->tags.end();
   }
   
   // Updates the given information

tests/CMakeLists.txt

@@ -24,6 +24,8 @@ INCLUDE_DIRECTORIES(
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/s3m
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/it
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/xm
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/ebml
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/ebml/matroska
 )
 
 SET(test_runner_SRCS
@@ -64,6 +66,7 @@ SET(test_runner_SRCS
   test_xm.cpp
   test_mpc.cpp
   test_opus.cpp
+  test_matroska.cpp
 )
 
 INCLUDE_DIRECTORIES(${CPPUNIT_INCLUDE_DIR})

tests/test_matroska.cpp

@@ -0,0 +1,108 @@
+#include <string>
+#include <stdio.h>
+#include <tag.h>
+#include <tpropertymap.h>
+#include <cppunit/extensions/HelperMacros.h>
+#include "utils.h"
+
+#include <ebmlmatroskafile.h>
+#include <ebmlmatroskaaudio.h>
+
+using namespace std;
+using namespace TagLib;
+
+class TestMatroska : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestMatroska);
+  CPPUNIT_TEST(testPredefined);
+  CPPUNIT_TEST(testInsertAndExtract);
+  CPPUNIT_TEST(testAudioProperties);
+  CPPUNIT_TEST_SUITE_END();
+  
+public:
+  void testPredefined()
+  {
+    ScopedFileCopy copy("matroska", ".mka");
+    string filename = copy.fileName();
+    
+    EBML::Matroska::File f(filename.c_str());
+    CPPUNIT_ASSERT(f.isValid());
+    
+    PropertyMap pm = f.properties();
+    PropertyMap::Iterator i = pm.find("ENCODER");
+    CPPUNIT_ASSERT(i != f.properties().end());
+    
+    CPPUNIT_ASSERT_EQUAL(String("Lavf54.63.104"), i->second.front());
+  }
+	
+  void testInsertAndExtract()
+  {
+    ScopedFileCopy copy("matroska", ".mka");
+    string filename = copy.fileName();
+    
+    EBML::Matroska::File f1(filename.c_str());
+    CPPUNIT_ASSERT(f1.isValid());
+    
+    Tag* t = f1.tag();
+    
+    CPPUNIT_ASSERT(t != 0);
+    t->setTitle("Seconds of Silence");
+    t->setArtist("Nobody");
+    t->setAlbum("TagLib Test Suite");
+    t->setComment("Well, there's nothing to say - a few special signs: ©’…ä–€ſ");
+    t->setGenre("Air");
+    t->setYear(2013);
+    t->setTrack(15);
+    
+    CPPUNIT_ASSERT(f1.save());
+    
+    EBML::Matroska::File f2(filename.c_str());
+    CPPUNIT_ASSERT(f2.isValid());
+    
+    t = f2.tag();
+    
+    CPPUNIT_ASSERT(t != 0);
+    CPPUNIT_ASSERT_EQUAL(String("Seconds of Silence"), t->title());
+    CPPUNIT_ASSERT_EQUAL(String("Nobody"), t->artist());
+    CPPUNIT_ASSERT_EQUAL(String("TagLib Test Suite"), t->album());
+    CPPUNIT_ASSERT_EQUAL(String("Well, there's nothing to say - a few special signs: ©’…ä–€ſ"), t->comment());
+    CPPUNIT_ASSERT_EQUAL(String("Air"), t->genre());
+    CPPUNIT_ASSERT_EQUAL(2013u, t->year());
+    CPPUNIT_ASSERT_EQUAL(15u, t->track());
+    
+    PropertyMap pm = f2.properties();
+    pm.erase("COMMENT");
+    f2.setProperties(pm);
+    
+    CPPUNIT_ASSERT(f2.save());
+    
+    EBML::Matroska::File f3(filename.c_str());
+    CPPUNIT_ASSERT(f3.isValid());
+    
+    pm = f3.properties();
+    PropertyMap::Iterator i = pm.find("GENRE");
+    
+    CPPUNIT_ASSERT(i != pm.end());
+    CPPUNIT_ASSERT_EQUAL(String("Air"), i->second.front());
+  }
+	
+  void testAudioProperties()
+  {
+    ScopedFileCopy copy("matroska", ".mka");
+    string filename = copy.fileName();
+    
+    EBML::Matroska::File f(filename.c_str());
+    CPPUNIT_ASSERT(f.isValid());
+    
+    AudioProperties* a = f.audioProperties();
+    CPPUNIT_ASSERT(a != 0);
+    
+    // Not a very nice assertion...
+    CPPUNIT_ASSERT_EQUAL(a->length(), 0);
+    // Bitrate is not nice and thus not tested.
+    CPPUNIT_ASSERT_EQUAL(a->sampleRate(), 44100);
+    CPPUNIT_ASSERT_EQUAL(a->channels(), 2);
+  }
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestMatroska);