From 9226fa76b3f0dba015a857836f87753497f82136 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 21 May 2015 12:54:20 +0900 Subject: [PATCH 1/5] MP4: AudioProperties improvements Add lengthInSeconds(), lengthInMilliseconds() properties. (#503) Add some tests for audio properties. Add some supplementary comments. Move parsing code to read() for consistency with other classes. --- taglib/mp4/mp4properties.cpp | 167 +++++++++++++++++++++-------------- taglib/mp4/mp4properties.h | 51 ++++++++++- tests/test_mp4.cpp | 16 +++- 3 files changed, 163 insertions(+), 71 deletions(-) diff --git a/taglib/mp4/mp4properties.cpp b/taglib/mp4/mp4properties.cpp index 5a41c081..e712f5d4 100644 --- a/taglib/mp4/mp4properties.cpp +++ b/taglib/mp4/mp4properties.cpp @@ -34,7 +34,14 @@ using namespace TagLib; class MP4::Properties::PropertiesPrivate { public: - PropertiesPrivate() : length(0), bitrate(0), sampleRate(0), channels(0), bitsPerSample(0), encrypted(false), codec(MP4::Properties::Unknown) {} + PropertiesPrivate() : + length(0), + bitrate(0), + sampleRate(0), + channels(0), + bitsPerSample(0), + encrypted(false), + codec(MP4::Properties::Unknown) {} int length; int bitrate; @@ -45,11 +52,83 @@ public: Codec codec; }; -MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style) - : AudioProperties(style) -{ - d = new PropertiesPrivate; +//////////////////////////////////////////////////////////////////////////////// +// public members +//////////////////////////////////////////////////////////////////////////////// +MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style) : + AudioProperties(style), + d(new PropertiesPrivate()) +{ + read(file, atoms); +} + +MP4::Properties::~Properties() +{ + delete d; +} + +int +MP4::Properties::channels() const +{ + return d->channels; +} + +int +MP4::Properties::sampleRate() const +{ + return d->sampleRate; +} + +int +MP4::Properties::length() const +{ + return lengthInSeconds(); +} + +int +MP4::Properties::lengthInSeconds() const +{ + return d->length / 1000; +} + +int +MP4::Properties::lengthInMilliseconds() const +{ + return d->length; +} + +int +MP4::Properties::bitrate() const +{ + return d->bitrate; +} + +int +MP4::Properties::bitsPerSample() const +{ + return d->bitsPerSample; +} + +bool +MP4::Properties::isEncrypted() const +{ + return d->encrypted; +} + +MP4::Properties::Codec +MP4::Properties::codec() const +{ + return d->codec; +} + +//////////////////////////////////////////////////////////////////////////////// +// private members +//////////////////////////////////////////////////////////////////////////////// + +void +MP4::Properties::read(File *file, Atoms *atoms) +{ MP4::Atom *moov = atoms->find("moov"); if(!moov) { debug("MP4: Atom 'moov' not found"); @@ -59,9 +138,9 @@ MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style) MP4::Atom *trak = 0; ByteVector data; - MP4::AtomList trakList = moov->findall("trak"); - for (unsigned int i = 0; i < trakList.size(); i++) { - trak = trakList[i]; + const MP4::AtomList trakList = moov->findall("trak"); + for(MP4::AtomList::ConstIterator it = trakList.begin(); it != trakList.end(); ++it) { + trak = *it; MP4::Atom *hdlr = trak->find("mdia", "hdlr"); if(!hdlr) { debug("MP4: Atom 'trak.mdia.hdlr' not found"); @@ -74,7 +153,7 @@ MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style) } trak = 0; } - if (!trak) { + if(!trak) { debug("MP4: No audio tracks"); return; } @@ -87,25 +166,28 @@ MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style) file->seek(mdhd->offset); data = file->readBlock(mdhd->length); - uint version = data[8]; + + const uint version = data[8]; + long long unit; + long long length; if(version == 1) { - if (data.size() < 36 + 8) { + if(data.size() < 36 + 8) { debug("MP4: Atom 'trak.mdia.mdhd' is smaller than expected"); return; } - const long long unit = data.toLongLong(28U); - const long long length = data.toLongLong(36U); - d->length = unit ? int(length / unit) : 0; + unit = data.toLongLong(28U); + length = data.toLongLong(36U); } else { - if (data.size() < 24 + 4) { + if(data.size() < 24 + 4) { debug("MP4: Atom 'trak.mdia.mdhd' is smaller than expected"); return; } - const unsigned int unit = data.toUInt(20U); - const unsigned int length = data.toUInt(24U); - d->length = unit ? length / unit : 0; + unit = data.toUInt(20U); + length = data.toUInt(24U); } + if(unit > 0 && length > 0) + d->length = static_cast(length * 1000.0 / unit); MP4::Atom *atom = trak->find("mdia", "minf", "stbl", "stsd"); if(!atom) { @@ -135,7 +217,7 @@ MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style) } } } - else if (data.mid(20, 4) == "alac") { + else if(data.mid(20, 4) == "alac") { if (atom->length == 88 && data.mid(56, 4) == "alac") { d->codec = ALAC; d->bitsPerSample = data.at(69); @@ -150,50 +232,3 @@ MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style) d->encrypted = true; } } - -MP4::Properties::~Properties() -{ - delete d; -} - -int -MP4::Properties::channels() const -{ - return d->channels; -} - -int -MP4::Properties::sampleRate() const -{ - return d->sampleRate; -} - -int -MP4::Properties::length() const -{ - return d->length; -} - -int -MP4::Properties::bitrate() const -{ - return d->bitrate; -} - -int -MP4::Properties::bitsPerSample() const -{ - return d->bitsPerSample; -} - -bool -MP4::Properties::isEncrypted() const -{ - return d->encrypted; -} - -MP4::Properties::Codec MP4::Properties::codec() const -{ - return d->codec; -} - diff --git a/taglib/mp4/mp4properties.h b/taglib/mp4/mp4properties.h index 2607c366..410d5348 100644 --- a/taglib/mp4/mp4properties.h +++ b/taglib/mp4/mp4properties.h @@ -49,17 +49,66 @@ namespace TagLib { Properties(File *file, Atoms *atoms, ReadStyle style = Average); virtual ~Properties(); + /*! + * Returns the length of the file in seconds. The length is rounded down to + * the nearest whole second. + * + * \note This method is just an alias of lengthInSeconds(). + * + * \deprecated + */ virtual int length() const; + + /*! + * Returns the length of the file in seconds. The length is rounded down to + * the nearest whole second. + * + * \see lengthInMilliseconds() + */ + // BIC: make virtual + int lengthInSeconds() const; + + /*! + * Returns the length of the file in milliseconds. + * + * \see lengthInSeconds() + */ + // BIC: make virtual + int lengthInMilliseconds() const; + + /*! + * Returns the average bit rate of the file in kb/s. + */ virtual int bitrate() const; + + /*! + * Returns the sample rate in Hz. + */ virtual int sampleRate() const; + + /*! + * Returns the number of audio channels. + */ virtual int channels() const; + + /*! + * Returns the number of bits per audio sample. + */ virtual int bitsPerSample() const; + + /*! + * Returns whether or not the file is encrypted. + */ bool isEncrypted() const; - //! Audio codec used in the MP4 file + /*! + * Returns the codec used in the file. + */ Codec codec() const; private: + void read(File *file, Atoms *atoms); + class PropertiesPrivate; PropertiesPrivate *d; }; diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index dd6e3a03..77ba6aaa 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -36,23 +36,31 @@ public: void testPropertiesAAC() { MP4::File f(TEST_FILE_PATH_C("has-tags.m4a")); + CPPUNIT_ASSERT(f.audioProperties()); CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->length()); + CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->lengthInSeconds()); + CPPUNIT_ASSERT_EQUAL(3707, f.audioProperties()->lengthInMilliseconds()); CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->bitrate()); CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); - CPPUNIT_ASSERT_EQUAL(16, ((MP4::Properties *)f.audioProperties())->bitsPerSample()); - CPPUNIT_ASSERT_EQUAL(MP4::Properties::AAC, ((MP4::Properties *)f.audioProperties())->codec()); + CPPUNIT_ASSERT_EQUAL(16, f.audioProperties()->bitsPerSample()); + CPPUNIT_ASSERT_EQUAL(false, f.audioProperties()->isEncrypted()); + CPPUNIT_ASSERT_EQUAL(MP4::Properties::AAC, f.audioProperties()->codec()); } void testPropertiesALAC() { MP4::File f(TEST_FILE_PATH_C("empty_alac.m4a")); + CPPUNIT_ASSERT(f.audioProperties()); CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->length()); + CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->lengthInSeconds()); + CPPUNIT_ASSERT_EQUAL(3705, f.audioProperties()->lengthInMilliseconds()); CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->bitrate()); CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); - CPPUNIT_ASSERT_EQUAL(16, ((MP4::Properties *)f.audioProperties())->bitsPerSample()); - CPPUNIT_ASSERT_EQUAL(MP4::Properties::ALAC, ((MP4::Properties *)f.audioProperties())->codec()); + CPPUNIT_ASSERT_EQUAL(16, f.audioProperties()->bitsPerSample()); + CPPUNIT_ASSERT_EQUAL(false, f.audioProperties()->isEncrypted()); + CPPUNIT_ASSERT_EQUAL(MP4::Properties::ALAC, f.audioProperties()->codec()); } void testCheckValid() From da14f67e2c88f55d076274547c327b9e7ef9e953 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 26 May 2015 10:50:43 +0900 Subject: [PATCH 2/5] MP4: Do rounding when calculating the bit rate. --- taglib/mp4/mp4properties.cpp | 6 +++--- tests/test_mp4.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/taglib/mp4/mp4properties.cpp b/taglib/mp4/mp4properties.cpp index e712f5d4..4485dd0e 100644 --- a/taglib/mp4/mp4properties.cpp +++ b/taglib/mp4/mp4properties.cpp @@ -213,16 +213,16 @@ MP4::Properties::read(File *file, Atoms *atoms) pos += 3; } pos += 10; - d->bitrate = (data.toUInt(pos) + 500) / 1000; + d->bitrate = static_cast((data.toUInt(pos) + 500) / 1000.0 + 0.5); } } } else if(data.mid(20, 4) == "alac") { - if (atom->length == 88 && data.mid(56, 4) == "alac") { + if(atom->length == 88 && data.mid(56, 4) == "alac") { d->codec = ALAC; d->bitsPerSample = data.at(69); d->channels = data.at(73); - d->bitrate = data.toUInt(80U) / 1000; + d->bitrate = static_cast(data.toUInt(80U) / 1000.0 + 0.5); d->sampleRate = data.toUInt(84U); } } diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index 77ba6aaa..78e1badf 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -55,7 +55,7 @@ public: CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->length()); CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->lengthInSeconds()); CPPUNIT_ASSERT_EQUAL(3705, f.audioProperties()->lengthInMilliseconds()); - CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->bitrate()); + CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->bitrate()); CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); CPPUNIT_ASSERT_EQUAL(16, f.audioProperties()->bitsPerSample()); From 5d775537591df223b68783fcc39809cd4d235ac6 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 27 May 2015 12:52:02 +0900 Subject: [PATCH 3/5] MP4: Remove useless ByteVector::mid() operations. --- taglib/mp4/mp4properties.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/taglib/mp4/mp4properties.cpp b/taglib/mp4/mp4properties.cpp index 4485dd0e..0ac77342 100644 --- a/taglib/mp4/mp4properties.cpp +++ b/taglib/mp4/mp4properties.cpp @@ -148,7 +148,7 @@ MP4::Properties::read(File *file, Atoms *atoms) } file->seek(hdlr->offset); data = file->readBlock(hdlr->length); - if(data.mid(16, 4) == "soun") { + if(data.containsAt("soun", 16)) { break; } trak = 0; @@ -196,20 +196,20 @@ MP4::Properties::read(File *file, Atoms *atoms) file->seek(atom->offset); data = file->readBlock(atom->length); - if(data.mid(20, 4) == "mp4a") { + if(data.containsAt("mp4a", 20)) { d->codec = AAC; d->channels = data.toShort(40U); d->bitsPerSample = data.toShort(42U); d->sampleRate = data.toUInt(46U); - if(data.mid(56, 4) == "esds" && data[64] == 0x03) { + if(data.containsAt("esds", 56) && data[64] == 0x03) { uint pos = 65; - if(data.mid(pos, 3) == "\x80\x80\x80") { + if(data.containsAt("\x80\x80\x80", pos)) { pos += 3; } pos += 4; if(data[pos] == 0x04) { pos += 1; - if(data.mid(pos, 3) == "\x80\x80\x80") { + if(data.containsAt("\x80\x80\x80", pos)) { pos += 3; } pos += 10; @@ -217,8 +217,8 @@ MP4::Properties::read(File *file, Atoms *atoms) } } } - else if(data.mid(20, 4) == "alac") { - if(atom->length == 88 && data.mid(56, 4) == "alac") { + else if(data.containsAt("alac", 20)) { + if(atom->length == 88 && data.containsAt("alac", 56)) { d->codec = ALAC; d->bitsPerSample = data.at(69); d->channels = data.at(73); From 7f0c547ba64f337680a77a3dc47fdfbcfb7741ee Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 31 Jul 2015 23:55:30 +0900 Subject: [PATCH 4/5] MP4: Remove unused formal parameters. --- taglib/mp4/mp4file.cpp | 27 ++++++++++++++------------- taglib/mp4/mp4file.h | 2 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/taglib/mp4/mp4file.cpp b/taglib/mp4/mp4file.cpp index e3cb02a3..84055c11 100644 --- a/taglib/mp4/mp4file.cpp +++ b/taglib/mp4/mp4file.cpp @@ -35,9 +35,10 @@ using namespace TagLib; class MP4::File::FilePrivate { public: - FilePrivate() : tag(0), atoms(0), properties(0) - { - } + FilePrivate() : + tag(0), + atoms(0), + properties(0) {} ~FilePrivate() { @@ -51,20 +52,20 @@ public: MP4::Properties *properties; }; -MP4::File::File(FileName file, bool readProperties, AudioProperties::ReadStyle audioPropertiesStyle) - : TagLib::File(file) +MP4::File::File(FileName file, bool readProperties, AudioProperties::ReadStyle) : + TagLib::File(file), + d(new FilePrivate()) { - d = new FilePrivate; if(isOpen()) - read(readProperties, audioPropertiesStyle); + read(readProperties); } -MP4::File::File(IOStream *stream, bool readProperties, AudioProperties::ReadStyle audioPropertiesStyle) - : TagLib::File(stream) +MP4::File::File(IOStream *stream, bool readProperties, AudioProperties::ReadStyle) : + TagLib::File(stream), + d(new FilePrivate()) { - d = new FilePrivate; if(isOpen()) - read(readProperties, audioPropertiesStyle); + read(readProperties); } MP4::File::~File() @@ -112,7 +113,7 @@ MP4::File::checkValid(const MP4::AtomList &list) } void -MP4::File::read(bool readProperties, Properties::ReadStyle audioPropertiesStyle) +MP4::File::read(bool readProperties) { if(!isValid()) return; @@ -132,7 +133,7 @@ MP4::File::read(bool readProperties, Properties::ReadStyle audioPropertiesStyle) d->tag = new Tag(this, d->atoms); if(readProperties) { - d->properties = new Properties(this, d->atoms, audioPropertiesStyle); + d->properties = new Properties(this, d->atoms); } } diff --git a/taglib/mp4/mp4file.h b/taglib/mp4/mp4file.h index a19eb074..0d615216 100644 --- a/taglib/mp4/mp4file.h +++ b/taglib/mp4/mp4file.h @@ -116,7 +116,7 @@ namespace TagLib { private: - void read(bool readProperties, Properties::ReadStyle audioPropertiesStyle); + void read(bool readProperties); bool checkValid(const MP4::AtomList &list); class FilePrivate; From 34ab65aa57c19f1c445fddc238e4ccf439618325 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 31 Jul 2015 23:59:22 +0900 Subject: [PATCH 5/5] MP4: Hide an internal function from the public header. --- taglib/mp4/mp4file.cpp | 35 ++++++++++++++++++++--------------- taglib/mp4/mp4file.h | 2 -- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/taglib/mp4/mp4file.cpp b/taglib/mp4/mp4file.cpp index 84055c11..1fc1524f 100644 --- a/taglib/mp4/mp4file.cpp +++ b/taglib/mp4/mp4file.cpp @@ -32,6 +32,23 @@ using namespace TagLib; +namespace +{ + bool checkValid(const MP4::AtomList &list) + { + for(MP4::AtomList::ConstIterator it = list.begin(); it != list.end(); ++it) { + + if((*it)->length == 0) + return false; + + if(!checkValid((*it)->children)) + return false; + } + + return true; + } +} + class MP4::File::FilePrivate { public: @@ -47,8 +64,8 @@ public: delete properties; } - MP4::Tag *tag; - MP4::Atoms *atoms; + MP4::Tag *tag; + MP4::Atoms *atoms; MP4::Properties *properties; }; @@ -100,18 +117,6 @@ MP4::File::audioProperties() const return d->properties; } -bool -MP4::File::checkValid(const MP4::AtomList &list) -{ - for(uint i = 0; i < list.size(); i++) { - if(list[i]->length == 0) - return false; - if(!checkValid(list[i]->children)) - return false; - } - return true; -} - void MP4::File::read(bool readProperties) { @@ -119,7 +124,7 @@ MP4::File::read(bool readProperties) return; d->atoms = new Atoms(this); - if (!checkValid(d->atoms->atoms)) { + if(!checkValid(d->atoms->atoms)) { setValid(false); return; } diff --git a/taglib/mp4/mp4file.h b/taglib/mp4/mp4file.h index 0d615216..28880f84 100644 --- a/taglib/mp4/mp4file.h +++ b/taglib/mp4/mp4file.h @@ -115,9 +115,7 @@ namespace TagLib { bool save(); private: - void read(bool readProperties); - bool checkValid(const MP4::AtomList &list); class FilePrivate; FilePrivate *d;