diff --git a/taglib/ebml/ebmlelement.cpp b/taglib/ebml/ebmlelement.cpp index 32ce46de..00a9b7e7 100644 --- a/taglib/ebml/ebmlelement.cpp +++ b/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(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(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(number))); + setAsBinary(ByteVector::fromUInt64BE(number)); } void EBML::Element::setAsFloat(long double) diff --git a/taglib/ebml/ebmlfile.cpp b/taglib/ebml/ebmlfile.cpp index 9dbb73e0..e9f4ce9f 100644 --- a/taglib/ebml/ebmlfile.cpp +++ b/taglib/ebml/ebmlfile.cpp @@ -40,7 +40,7 @@ public: { document->seek(0); ByteVector magical = document->readBlock(4); - if(static_cast(magical.toInt64BE(0)) != Header::EBML) + if(static_cast(magical.toUInt32BE(0)) != Header::EBML) return 0; FilePrivate *d = new FilePrivate(document); Element *head = d->root.getChild(Header::EBML); diff --git a/taglib/ebml/matroska/ebmlmatroskaaudio.cpp b/taglib/ebml/matroska/ebmlmatroskaaudio.cpp index 8f3cdd00..e2a8251d 100644 --- a/taglib/ebml/matroska/ebmlmatroskaaudio.cpp +++ b/taglib/ebml/matroska/ebmlmatroskaaudio.cpp @@ -44,10 +44,11 @@ public: Element *value; if(info && (value = info->getChild(Constants::Duration))) { - length = static_cast(value->getAsFloat()); - if((value = info->getChild(Constants::TimecodeScale))){ - length = static_cast(length / (value->getAsUnsigned() * (1.0 / 1000000000))); - } + length = static_cast(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(8 * document->tell() / length / 1000); + bitrate = static_cast(8 * document->tell() / ((length > 0) ? length : 1)); if((value = info->getChild(Constants::Channels))) channels = static_cast(value->getAsUnsigned()); diff --git a/taglib/ebml/matroska/ebmlmatroskafile.cpp b/taglib/ebml/matroska/ebmlmatroskafile.cpp index a43a9783..5325ca3a 100644 --- a/taglib/ebml/matroska/ebmlmatroskafile.cpp +++ b/taglib/ebml/matroska/ebmlmatroskafile.cpp @@ -64,8 +64,8 @@ public: List entries = elem->getChildren(Constants::Tag); for(List::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 > >::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 diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a369b5a1..7a5bbbf9 100644 --- a/tests/CMakeLists.txt +++ b/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}) diff --git a/tests/data/matroska.mka b/tests/data/matroska.mka new file mode 100644 index 00000000..e85bd338 Binary files /dev/null and b/tests/data/matroska.mka differ diff --git a/tests/test_matroska.cpp b/tests/test_matroska.cpp new file mode 100644 index 00000000..339f9a78 --- /dev/null +++ b/tests/test_matroska.cpp @@ -0,0 +1,108 @@ +#include +#include +#include +#include +#include +#include "utils.h" + +#include +#include + +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);