From 1bb06b1f7a2a00ce0e0c119a728d15cde92882ec Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 3 Aug 2015 23:01:15 +0900 Subject: [PATCH 01/15] Add RIFF::WAV::File::strip() function. Avoid saving an empty tag. --- taglib/riff/wav/wavfile.cpp | 61 +++++++++++++++++-------------------- taglib/riff/wav/wavfile.h | 17 ++++++----- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/taglib/riff/wav/wavfile.cpp b/taglib/riff/wav/wavfile.cpp index e7b8dc8a..cab8b33b 100644 --- a/taglib/riff/wav/wavfile.cpp +++ b/taglib/riff/wav/wavfile.cpp @@ -106,6 +106,17 @@ RIFF::Info::Tag *RIFF::WAV::File::InfoTag() const return d->tag.access(InfoIndex, false); } +void RIFF::WAV::File::strip(TagTypes tags) +{ + removeTagChunks(tags); + + if(tags & ID3v2) + d->tag.set(ID3v2Index, new ID3v2::Tag()); + + if(tags & Info) + d->tag.set(InfoIndex, new RIFF::Info::Tag()); +} + PropertyMap RIFF::WAV::File::properties() const { return tag()->properties(); @@ -146,28 +157,20 @@ bool RIFF::WAV::File::save(TagTypes tags, bool stripOthers, int id3v2Version) if(stripOthers) strip(static_cast(AllTags & ~tags)); - const ID3v2::Tag *id3v2tag = d->tag.access(ID3v2Index, false); if(tags & ID3v2) { - if(d->hasID3v2) { - removeChunk(d->tagChunkID); - d->hasID3v2 = false; - } + removeTagChunks(ID3v2); - if(!id3v2tag->isEmpty()) { - setChunkData(d->tagChunkID, id3v2tag->render(id3v2Version)); + if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) { + setChunkData(d->tagChunkID, ID3v2Tag()->render(id3v2Version)); d->hasID3v2 = true; } } - const Info::Tag *infotag = d->tag.access(InfoIndex, false); if(tags & Info) { - if(d->hasInfo) { - removeChunk(findInfoTagChunk()); - d->hasInfo = false; - } + removeTagChunks(Info); - if(!infotag->isEmpty()) { - setChunkData("LIST", infotag->render(), true); + if(InfoTag() && !InfoTag()->isEmpty()) { + setChunkData("LIST", InfoTag()->render(), true); d->hasInfo = true; } } @@ -227,29 +230,21 @@ void RIFF::WAV::File::read(bool readProperties) d->properties = new Properties(this, Properties::Average); } -void RIFF::WAV::File::strip(TagTypes tags) +void RIFF::WAV::File::removeTagChunks(TagTypes tags) { - if(tags & ID3v2) { - removeChunk(d->tagChunkID); + if((tags & ID3v2) && d->hasID3v2) { + removeChunk("ID3 "); + removeChunk("id3 "); + d->hasID3v2 = false; } - if(tags & Info){ - TagLib::uint chunkId = findInfoTagChunk(); - if(chunkId != TagLib::uint(-1)) { - removeChunk(chunkId); - d->hasInfo = false; + if((tags & Info) && d->hasInfo) { + for(int i = static_cast(chunkCount()) - 1; i >= 0; --i) { + if(chunkName(i) == "LIST" && chunkData(i).startsWith("INFO")) + removeChunk(i); } + + d->hasInfo = false; } } - -TagLib::uint RIFF::WAV::File::findInfoTagChunk() -{ - for(uint i = 0; i < chunkCount(); ++i) { - if(chunkName(i) == "LIST" && chunkData(i).startsWith("INFO")) { - return i; - } - } - - return TagLib::uint(-1); -} diff --git a/taglib/riff/wav/wavfile.h b/taglib/riff/wav/wavfile.h index 129d9537..80f17a85 100644 --- a/taglib/riff/wav/wavfile.h +++ b/taglib/riff/wav/wavfile.h @@ -125,6 +125,15 @@ namespace TagLib { */ Info::Tag *InfoTag() const; + /*! + * This will strip the tags that match the OR-ed together TagTypes from the + * file. By default it strips all tags. It returns true if the tags are + * successfully stripped. + * + * \note This will update the file immediately. + */ + void strip(TagTypes tags = AllTags); + /*! * Implements the unified property interface -- export function. * This method forwards to ID3v2::Tag::properties(). @@ -171,13 +180,7 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties); - - void strip(TagTypes tags); - - /*! - * Returns the index of the chunk that its name is "LIST" and list type is "INFO". - */ - uint findInfoTagChunk(); + void removeTagChunks(TagTypes tags); friend class Properties; From c5cf9b93bcd12e5570c43c3ed41fcaf18e4d5132 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 3 Aug 2015 23:41:26 +0900 Subject: [PATCH 02/15] Fix segfaults when calling File::properties() after strip(). Backport TagUnion::properties() and TagUnion::removeUnsupportedProperties() from taglib2. --- taglib/ape/apefile.cpp | 18 +++----- taglib/flac/flacfile.cpp | 19 ++------- taglib/mpc/mpcfile.cpp | 18 +++----- taglib/mpeg/mpegfile.cpp | 27 ++++-------- taglib/riff/wav/wavfile.cpp | 7 ++-- taglib/tagunion.cpp | 67 +++++++++++++++++++++++++++++- taglib/tagunion.h | 3 ++ taglib/trueaudio/trueaudiofile.cpp | 18 +++----- taglib/wavpack/wavpackfile.cpp | 18 +++----- tests/test_ape.cpp | 25 ++++++++++- tests/test_mpc.cpp | 25 ++++++++++- tests/test_mpeg.cpp | 27 ++++++++++++ tests/test_trueaudio.cpp | 24 +++++++++++ tests/test_wav.cpp | 23 +++++++++- tests/test_wavpack.cpp | 26 +++++++++++- 15 files changed, 253 insertions(+), 92 deletions(-) diff --git a/taglib/ape/apefile.cpp b/taglib/ape/apefile.cpp index c774f516..1349dfa3 100644 --- a/taglib/ape/apefile.cpp +++ b/taglib/ape/apefile.cpp @@ -125,26 +125,20 @@ TagLib::Tag *APE::File::tag() const PropertyMap APE::File::properties() const { - if(d->hasAPE) - return d->tag.access(ApeAPEIndex, false)->properties(); - if(d->hasID3v1) - return d->tag.access(ApeID3v1Index, false)->properties(); - return PropertyMap(); + return d->tag.properties(); } void APE::File::removeUnsupportedProperties(const StringList &properties) { - if(d->hasAPE) - d->tag.access(ApeAPEIndex, false)->removeUnsupportedProperties(properties); - if(d->hasID3v1) - d->tag.access(ApeID3v1Index, false)->removeUnsupportedProperties(properties); + d->tag.removeUnsupportedProperties(properties); } PropertyMap APE::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v1) - d->tag.access(ApeID3v1Index, false)->setProperties(properties); - return d->tag.access(ApeAPEIndex, true)->setProperties(properties); + if(ID3v1Tag()) + ID3v1Tag()->setProperties(properties); + + return APETag(true)->setProperties(properties); } APE::Properties *APE::File::audioProperties() const diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index 4d2d69e6..dc8f4011 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -141,30 +141,17 @@ TagLib::Tag *FLAC::File::tag() const PropertyMap FLAC::File::properties() const { - // once Tag::properties() is virtual, this case distinction could actually be done - // within TagUnion. - if(d->hasXiphComment) - return d->tag.access(FlacXiphIndex, false)->properties(); - if(d->hasID3v2) - return d->tag.access(FlacID3v2Index, false)->properties(); - if(d->hasID3v1) - return d->tag.access(FlacID3v1Index, false)->properties(); - return PropertyMap(); + return d->tag.properties(); } void FLAC::File::removeUnsupportedProperties(const StringList &unsupported) { - if(d->hasXiphComment) - d->tag.access(FlacXiphIndex, false)->removeUnsupportedProperties(unsupported); - if(d->hasID3v2) - d->tag.access(FlacID3v2Index, false)->removeUnsupportedProperties(unsupported); - if(d->hasID3v1) - d->tag.access(FlacID3v1Index, false)->removeUnsupportedProperties(unsupported); + d->tag.removeUnsupportedProperties(unsupported); } PropertyMap FLAC::File::setProperties(const PropertyMap &properties) { - return d->tag.access(FlacXiphIndex, true)->setProperties(properties); + return xiphComment(true)->setProperties(properties); } FLAC::Properties *FLAC::File::audioProperties() const diff --git a/taglib/mpc/mpcfile.cpp b/taglib/mpc/mpcfile.cpp index cf5f38d8..6d8dae5a 100644 --- a/taglib/mpc/mpcfile.cpp +++ b/taglib/mpc/mpcfile.cpp @@ -116,26 +116,20 @@ TagLib::Tag *MPC::File::tag() const PropertyMap MPC::File::properties() const { - if(d->hasAPE) - return d->tag.access(MPCAPEIndex, false)->properties(); - if(d->hasID3v1) - return d->tag.access(MPCID3v1Index, false)->properties(); - return PropertyMap(); + return d->tag.properties(); } void MPC::File::removeUnsupportedProperties(const StringList &properties) { - if(d->hasAPE) - d->tag.access(MPCAPEIndex, false)->removeUnsupportedProperties(properties); - if(d->hasID3v1) - d->tag.access(MPCID3v1Index, false)->removeUnsupportedProperties(properties); + d->tag.removeUnsupportedProperties(properties); } PropertyMap MPC::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v1) - d->tag.access(MPCID3v1Index, false)->setProperties(properties); - return d->tag.access(MPCAPEIndex, true)->setProperties(properties); + if(ID3v1Tag()) + ID3v1Tag()->setProperties(properties); + + return APETag(true)->setProperties(properties); } MPC::Properties *MPC::File::audioProperties() const diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 43075cfc..5ced51ef 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -149,33 +149,22 @@ TagLib::Tag *MPEG::File::tag() const PropertyMap MPEG::File::properties() const { - // once Tag::properties() is virtual, this case distinction could actually be done - // within TagUnion. - if(d->hasID3v2) - return d->tag.access(ID3v2Index, false)->properties(); - if(d->hasAPE) - return d->tag.access(APEIndex, false)->properties(); - if(d->hasID3v1) - return d->tag.access(ID3v1Index, false)->properties(); - return PropertyMap(); + return d->tag.properties(); } void MPEG::File::removeUnsupportedProperties(const StringList &properties) { - if(d->hasID3v2) - d->tag.access(ID3v2Index, false)->removeUnsupportedProperties(properties); - else if(d->hasAPE) - d->tag.access(APEIndex, false)->removeUnsupportedProperties(properties); - else if(d->hasID3v1) - d->tag.access(ID3v1Index, false)->removeUnsupportedProperties(properties); + d->tag.removeUnsupportedProperties(properties); } PropertyMap MPEG::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v1) - // update ID3v1 tag if it exists, but ignore the return value - d->tag.access(ID3v1Index, false)->setProperties(properties); - return d->tag.access(ID3v2Index, true)->setProperties(properties); + // update ID3v1 tag if it exists, but ignore the return value + + if(ID3v1Tag()) + ID3v1Tag()->setProperties(properties); + + return ID3v2Tag(true)->setProperties(properties); } MPEG::Properties *MPEG::File::audioProperties() const diff --git a/taglib/riff/wav/wavfile.cpp b/taglib/riff/wav/wavfile.cpp index cab8b33b..d387d79e 100644 --- a/taglib/riff/wav/wavfile.cpp +++ b/taglib/riff/wav/wavfile.cpp @@ -119,17 +119,18 @@ void RIFF::WAV::File::strip(TagTypes tags) PropertyMap RIFF::WAV::File::properties() const { - return tag()->properties(); + return d->tag.properties(); } void RIFF::WAV::File::removeUnsupportedProperties(const StringList &unsupported) { - tag()->removeUnsupportedProperties(unsupported); + d->tag.removeUnsupportedProperties(unsupported); } PropertyMap RIFF::WAV::File::setProperties(const PropertyMap &properties) { - return tag()->setProperties(properties); + InfoTag()->setProperties(properties); + return ID3v2Tag()->setProperties(properties); } RIFF::WAV::Properties *RIFF::WAV::File::audioProperties() const diff --git a/taglib/tagunion.cpp b/taglib/tagunion.cpp index 52d7136b..a6743d14 100644 --- a/taglib/tagunion.cpp +++ b/taglib/tagunion.cpp @@ -23,8 +23,15 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ -#include "tagunion.h" -#include "tstringlist.h" +#include +#include +#include + +#include "id3v1tag.h" +#include "id3v2tag.h" +#include "apetag.h" +#include "xiphcomment.h" +#include "infotag.h" using namespace TagLib; @@ -102,6 +109,62 @@ void TagUnion::set(int index, Tag *tag) d->tags[index] = tag; } +PropertyMap TagUnion::properties() const +{ + // This is an ugly workaround but we can't add a virtual function. + // Should be virtual in taglib2. + + for(size_t i = 0; i < 3; ++i) { + + if(d->tags[i] && !d->tags[i]->isEmpty()) { + + if(dynamic_cast(d->tags[i])) + return dynamic_cast(d->tags[i])->properties(); + + else if(dynamic_cast(d->tags[i])) + return dynamic_cast(d->tags[i])->properties(); + + else if(dynamic_cast(d->tags[i])) + return dynamic_cast(d->tags[i])->properties(); + + else if(dynamic_cast(d->tags[i])) + return dynamic_cast(d->tags[i])->properties(); + + else if(dynamic_cast(d->tags[i])) + return dynamic_cast(d->tags[i])->properties(); + } + } + + return PropertyMap(); +} + +void TagUnion::removeUnsupportedProperties(const StringList &unsupported) +{ + // This is an ugly workaround but we can't add a virtual function. + // Should be virtual in taglib2. + + for(size_t i = 0; i < 3; ++i) { + + if(d->tags[i]) { + + if(dynamic_cast(d->tags[i])) + dynamic_cast(d->tags[i])->removeUnsupportedProperties(unsupported); + + else if(dynamic_cast(d->tags[i])) + dynamic_cast(d->tags[i])->removeUnsupportedProperties(unsupported); + + else if(dynamic_cast(d->tags[i])) + dynamic_cast(d->tags[i])->removeUnsupportedProperties(unsupported); + + else if(dynamic_cast(d->tags[i])) + dynamic_cast(d->tags[i])->removeUnsupportedProperties(unsupported); + + else if(dynamic_cast(d->tags[i])) + dynamic_cast(d->tags[i])->removeUnsupportedProperties(unsupported); + } + } +} + String TagUnion::title() const { stringUnion(title); diff --git a/taglib/tagunion.h b/taglib/tagunion.h index e94d523a..0a3b3a20 100644 --- a/taglib/tagunion.h +++ b/taglib/tagunion.h @@ -56,6 +56,9 @@ namespace TagLib { void set(int index, Tag *tag); + PropertyMap properties() const; + void removeUnsupportedProperties(const StringList &unsupported); + virtual String title() const; virtual String artist() const; virtual String album() const; diff --git a/taglib/trueaudio/trueaudiofile.cpp b/taglib/trueaudio/trueaudiofile.cpp index 8f40564c..4ca60915 100644 --- a/taglib/trueaudio/trueaudiofile.cpp +++ b/taglib/trueaudio/trueaudiofile.cpp @@ -130,26 +130,20 @@ TagLib::Tag *TrueAudio::File::tag() const PropertyMap TrueAudio::File::properties() const { - // once Tag::properties() is virtual, this case distinction could actually be done - // within TagUnion. - if(d->hasID3v2) - return d->tag.access(TrueAudioID3v2Index, false)->properties(); - if(d->hasID3v1) - return d->tag.access(TrueAudioID3v1Index, false)->properties(); - return PropertyMap(); + return d->tag.properties(); } void TrueAudio::File::removeUnsupportedProperties(const StringList &unsupported) { - if(d->hasID3v2) - d->tag.access(TrueAudioID3v2Index, false)->removeUnsupportedProperties(unsupported); + d->tag.removeUnsupportedProperties(unsupported); } PropertyMap TrueAudio::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v1) - d->tag.access(TrueAudioID3v1Index, false)->setProperties(properties); - return d->tag.access(TrueAudioID3v2Index, true)->setProperties(properties); + if(ID3v1Tag()) + ID3v1Tag()->setProperties(properties); + + return ID3v2Tag(true)->setProperties(properties); } TrueAudio::Properties *TrueAudio::File::audioProperties() const diff --git a/taglib/wavpack/wavpackfile.cpp b/taglib/wavpack/wavpackfile.cpp index de0ba415..90f79d16 100644 --- a/taglib/wavpack/wavpackfile.cpp +++ b/taglib/wavpack/wavpackfile.cpp @@ -110,26 +110,20 @@ TagLib::Tag *WavPack::File::tag() const PropertyMap WavPack::File::properties() const { - if(d->hasAPE) - return d->tag.access(WavAPEIndex, false)->properties(); - if(d->hasID3v1) - return d->tag.access(WavID3v1Index, false)->properties(); - return PropertyMap(); + return d->tag.properties(); } - void WavPack::File::removeUnsupportedProperties(const StringList &unsupported) { - if(d->hasAPE) - d->tag.access(WavAPEIndex, false)->removeUnsupportedProperties(unsupported); + d->tag.removeUnsupportedProperties(unsupported); } - PropertyMap WavPack::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v1) - d->tag.access(WavID3v1Index, false)->setProperties(properties); - return d->tag.access(WavAPEIndex, true)->setProperties(properties); + if(ID3v1Tag()) + ID3v1Tag()->setProperties(properties); + + return APETag(true)->setProperties(properties); } WavPack::Properties *WavPack::File::audioProperties() const diff --git a/tests/test_ape.cpp b/tests/test_ape.cpp index 3ab3197b..64869c4c 100644 --- a/tests/test_ape.cpp +++ b/tests/test_ape.cpp @@ -1,8 +1,10 @@ #include #include -#include +#include +#include #include #include +#include #include #include #include "utils.h" @@ -20,6 +22,7 @@ class TestAPE : public CppUnit::TestFixture CPPUNIT_TEST(testProperties390); CPPUNIT_TEST(testFuzzedFile1); CPPUNIT_TEST(testFuzzedFile2); + CPPUNIT_TEST(testStripAndProperties); CPPUNIT_TEST_SUITE_END(); public: @@ -111,6 +114,26 @@ public: CPPUNIT_ASSERT(f.isValid()); } + void testStripAndProperties() + { + ScopedFileCopy copy("mac-399", ".ape"); + + { + APE::File f(copy.fileName().c_str()); + f.APETag(true)->setTitle("APE"); + f.ID3v1Tag(true)->setTitle("ID3v1"); + f.save(); + } + { + APE::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(String("APE"), f.properties()["TITLE"].front()); + f.strip(APE::File::APE); + CPPUNIT_ASSERT_EQUAL(String("ID3v1"), f.properties()["TITLE"].front()); + f.strip(APE::File::ID3v1); + CPPUNIT_ASSERT(f.properties().isEmpty()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestAPE); diff --git a/tests/test_mpc.cpp b/tests/test_mpc.cpp index 50a62d22..ec1e9f1c 100644 --- a/tests/test_mpc.cpp +++ b/tests/test_mpc.cpp @@ -1,8 +1,10 @@ #include #include -#include +#include +#include #include #include +#include #include #include #include "utils.h" @@ -21,6 +23,7 @@ class TestMPC : public CppUnit::TestFixture CPPUNIT_TEST(testFuzzedFile2); CPPUNIT_TEST(testFuzzedFile3); CPPUNIT_TEST(testFuzzedFile4); + CPPUNIT_TEST(testStripAndProperties); CPPUNIT_TEST_SUITE_END(); public: @@ -109,6 +112,26 @@ public: CPPUNIT_ASSERT(f.isValid()); } + void testStripAndProperties() + { + ScopedFileCopy copy("click", ".mpc"); + + { + MPC::File f(copy.fileName().c_str()); + f.APETag(true)->setTitle("APE"); + f.ID3v1Tag(true)->setTitle("ID3v1"); + f.save(); + } + { + MPC::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(String("APE"), f.properties()["TITLE"].front()); + f.strip(MPC::File::APE); + CPPUNIT_ASSERT_EQUAL(String("ID3v1"), f.properties()["TITLE"].front()); + f.strip(MPC::File::ID3v1); + CPPUNIT_ASSERT(f.properties().isEmpty()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMPC); diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index 8d49a8c7..ee3575e3 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -1,8 +1,11 @@ #include #include #include +#include #include #include +#include +#include #include #include #include @@ -26,6 +29,7 @@ class TestMPEG : public CppUnit::TestFixture CPPUNIT_TEST(testDuplicateID3v2); CPPUNIT_TEST(testFuzzedFile); CPPUNIT_TEST(testFrameOffset); + CPPUNIT_TEST(testStripAndProperties); CPPUNIT_TEST_SUITE_END(); public: @@ -212,6 +216,29 @@ public: } } + void testStripAndProperties() + { + ScopedFileCopy copy("xing", ".mp3"); + + { + MPEG::File f(copy.fileName().c_str()); + f.ID3v2Tag(true)->setTitle("ID3v2"); + f.APETag(true)->setTitle("APE"); + f.ID3v1Tag(true)->setTitle("ID3v1"); + f.save(); + } + { + MPEG::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(String("ID3v2"), f.properties()["TITLE"].front()); + f.strip(MPEG::File::ID3v2); + CPPUNIT_ASSERT_EQUAL(String("APE"), f.properties()["TITLE"].front()); + f.strip(MPEG::File::APE); + CPPUNIT_ASSERT_EQUAL(String("ID3v1"), f.properties()["TITLE"].front()); + f.strip(MPEG::File::ID3v1); + CPPUNIT_ASSERT(f.properties().isEmpty()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG); diff --git a/tests/test_trueaudio.cpp b/tests/test_trueaudio.cpp index 00b81ede..8468e0ec 100644 --- a/tests/test_trueaudio.cpp +++ b/tests/test_trueaudio.cpp @@ -1,5 +1,8 @@ #include #include +#include +#include +#include #include #include #include "utils.h" @@ -12,6 +15,7 @@ class TestTrueAudio : public CppUnit::TestFixture CPPUNIT_TEST_SUITE(TestTrueAudio); CPPUNIT_TEST(testReadPropertiesWithoutID3v2); CPPUNIT_TEST(testReadPropertiesWithTags); + CPPUNIT_TEST(testStripAndProperties); CPPUNIT_TEST_SUITE_END(); public: @@ -46,6 +50,26 @@ public: CPPUNIT_ASSERT_EQUAL(1, f.audioProperties()->ttaVersion()); } + void testStripAndProperties() + { + ScopedFileCopy copy("empty", ".tta"); + + { + TrueAudio::File f(copy.fileName().c_str()); + f.ID3v2Tag(true)->setTitle("ID3v2"); + f.ID3v1Tag(true)->setTitle("ID3v1"); + f.save(); + } + { + TrueAudio::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(String("ID3v2"), f.properties()["TITLE"].front()); + f.strip(TrueAudio::File::ID3v2); + CPPUNIT_ASSERT_EQUAL(String("ID3v1"), f.properties()["TITLE"].front()); + f.strip(TrueAudio::File::ID3v1); + CPPUNIT_ASSERT(f.properties().isEmpty()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestTrueAudio); diff --git a/tests/test_wav.cpp b/tests/test_wav.cpp index bcb91753..e2463790 100644 --- a/tests/test_wav.cpp +++ b/tests/test_wav.cpp @@ -1,9 +1,9 @@ #include #include -#include #include #include #include +#include #include #include #include "utils.h" @@ -24,6 +24,7 @@ class TestWAV : public CppUnit::TestFixture CPPUNIT_TEST(testDuplicateTags); CPPUNIT_TEST(testFuzzedFile1); CPPUNIT_TEST(testFuzzedFile2); + CPPUNIT_TEST(testStripAndProperties); CPPUNIT_TEST_SUITE_END(); public: @@ -211,6 +212,26 @@ public: CPPUNIT_ASSERT(f2.isValid()); } + void testStripAndProperties() + { + ScopedFileCopy copy("empty", ".wav"); + + { + RIFF::WAV::File f(copy.fileName().c_str()); + f.ID3v2Tag()->setTitle("ID3v2"); + f.InfoTag()->setTitle("INFO"); + f.save(); + } + { + RIFF::WAV::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(String("ID3v2"), f.properties()["TITLE"].front()); + f.strip(RIFF::WAV::File::ID3v2); + CPPUNIT_ASSERT_EQUAL(String("INFO"), f.properties()["TITLE"].front()); + f.strip(RIFF::WAV::File::Info); + CPPUNIT_ASSERT(f.properties().isEmpty()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestWAV); diff --git a/tests/test_wavpack.cpp b/tests/test_wavpack.cpp index 5befbff3..eddc601d 100644 --- a/tests/test_wavpack.cpp +++ b/tests/test_wavpack.cpp @@ -1,7 +1,9 @@ #include #include -#include +#include +#include #include +#include #include #include #include "utils.h" @@ -16,6 +18,7 @@ class TestWavPack : public CppUnit::TestFixture CPPUNIT_TEST(testMultiChannelProperties); CPPUNIT_TEST(testTaggedProperties); CPPUNIT_TEST(testFuzzedFile); + CPPUNIT_TEST(testStripAndProperties); CPPUNIT_TEST_SUITE_END(); public: @@ -73,6 +76,27 @@ public: WavPack::File f(TEST_FILE_PATH_C("infloop.wv")); CPPUNIT_ASSERT(f.isValid()); } + + void testStripAndProperties() + { + ScopedFileCopy copy("click", ".wv"); + + { + WavPack::File f(copy.fileName().c_str()); + f.APETag(true)->setTitle("APE"); + f.ID3v1Tag(true)->setTitle("ID3v1"); + f.save(); + } + { + WavPack::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(String("APE"), f.properties()["TITLE"].front()); + f.strip(WavPack::File::APE); + CPPUNIT_ASSERT_EQUAL(String("ID3v1"), f.properties()["TITLE"].front()); + f.strip(WavPack::File::ID3v1); + CPPUNIT_ASSERT(f.properties().isEmpty()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestWavPack); From c7231c58a32d4caef7b7085dab8042c9786d7d92 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 7 Aug 2015 13:36:35 +0900 Subject: [PATCH 03/15] Improve a test about handling duplicate tags in WAV files. --- tests/test_wav.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_wav.cpp b/tests/test_wav.cpp index e2463790..24bb02c5 100644 --- a/tests/test_wav.cpp +++ b/tests/test_wav.cpp @@ -188,7 +188,10 @@ public: void testDuplicateTags() { - RIFF::WAV::File f(TEST_FILE_PATH_C("duplicate_tags.wav")); + ScopedFileCopy copy("duplicate_tags", ".wav"); + + RIFF::WAV::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT_EQUAL(17052L, f.length()); // duplicate_tags.wav has duplicate ID3v2/INFO tags. // title() returns "Title2" if can't skip the second tag. @@ -198,6 +201,10 @@ public: CPPUNIT_ASSERT(f.hasInfoTag()); CPPUNIT_ASSERT_EQUAL(String("Title1"), f.InfoTag()->title()); + + f.save(); + CPPUNIT_ASSERT_EQUAL(15898L, f.length()); + CPPUNIT_ASSERT_EQUAL(-1L, f.find("Title2")); } void testFuzzedFile1() From b6adb1f1081ccb39fe5bb8d3808dd765c4155d6b Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 7 Aug 2015 13:42:01 +0900 Subject: [PATCH 04/15] Remove a private data member not needed to carry. --- taglib/riff/wav/wavfile.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/taglib/riff/wav/wavfile.cpp b/taglib/riff/wav/wavfile.cpp index d387d79e..bdcfa474 100644 --- a/taglib/riff/wav/wavfile.cpp +++ b/taglib/riff/wav/wavfile.cpp @@ -45,11 +45,8 @@ class RIFF::WAV::File::FilePrivate public: FilePrivate() : properties(0), - tagChunkID("ID3 "), hasID3v2(false), - hasInfo(false) - { - } + hasInfo(false) {} ~FilePrivate() { @@ -57,9 +54,6 @@ public: } Properties *properties; - - ByteVector tagChunkID; - TagUnion tag; bool hasID3v2; @@ -162,7 +156,7 @@ bool RIFF::WAV::File::save(TagTypes tags, bool stripOthers, int id3v2Version) removeTagChunks(ID3v2); if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) { - setChunkData(d->tagChunkID, ID3v2Tag()->render(id3v2Version)); + setChunkData("ID3 ", ID3v2Tag()->render(id3v2Version)); d->hasID3v2 = true; } } @@ -199,7 +193,6 @@ void RIFF::WAV::File::read(bool readProperties) const ByteVector name = chunkName(i); if(name == "ID3 " || name == "id3 ") { if(!d->tag[ID3v2Index]) { - d->tagChunkID = name; d->tag.set(ID3v2Index, new ID3v2::Tag(this, chunkOffset(i))); d->hasID3v2 = true; } From 11fbf394a345dd29b68d6e2d2d90b78471db6ee1 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 13 Nov 2015 11:55:56 +0900 Subject: [PATCH 05/15] Skip duplicate ID3v2 tags and treat them as an extra blank of the first one. This enables all the file formats to handle duplicate ID3v2 tags properly and erase them automatically. --- taglib/mpeg/id3v2/id3v2tag.cpp | 51 ++++++++++++++++++++++++---------- taglib/mpeg/mpegfile.cpp | 17 +----------- tests/test_id3v2.cpp | 36 ++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 30 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index e41606db..f06cfbad 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -24,22 +24,22 @@ ***************************************************************************/ #ifdef HAVE_CONFIG_H -#include "config.h" +# include "config.h" #endif #include -#include "tfile.h" +#include +#include +#include +#include #include "id3v2tag.h" #include "id3v2header.h" #include "id3v2extendedheader.h" #include "id3v2footer.h" #include "id3v2synchdata.h" -#include "tbytevector.h" #include "id3v1genres.h" -#include "tpropertymap.h" -#include "tdebug.h" #include "frames/textidentificationframe.h" #include "frames/commentsframe.h" @@ -136,7 +136,6 @@ ID3v2::Tag::~Tag() delete d; } - String ID3v2::Tag::title() const { if(!d->frameListMap["TIT2"].isEmpty()) @@ -564,7 +563,6 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const } } - ByteVector ID3v2::Tag::render(int version) const { // We need to render the "tag data" first so that we have to correct size to @@ -667,18 +665,43 @@ void ID3v2::Tag::setLatin1StringHandler(const Latin1StringHandler *handler) void ID3v2::Tag::read() { - if(d->file && d->file->isOpen()) { + if(!d->file) + return; - d->file->seek(d->tagOffset); - d->header.setData(d->file->readBlock(Header::size())); + if(!d->file->isOpen()) + return; - // if the tag size is 0, then this is an invalid tag (tags must contain at - // least one frame) + d->file->seek(d->tagOffset); + d->header.setData(d->file->readBlock(Header::size())); - if(d->header.tagSize() == 0) - return; + // If the tag size is 0, then this is an invalid tag (tags must contain at + // least one frame) + if(d->header.tagSize() != 0) parse(d->file->readBlock(d->header.tagSize())); + + // Look for duplicate ID3v2 tags and treat them as an extra blank of this one. + // It leads to overwriting them with zero when saving the tag. + + // This is a workaround for some faulty files that have duplicate ID3v2 tags. + // Unfortunately, TagLib itself may write such duplicate tags until v1.10. + + uint extraSize = 0; + + while(true) { + + d->file->seek(d->tagOffset + d->header.completeTagSize() + extraSize); + + const ByteVector data = d->file->readBlock(Header::size()); + if(data.size() < Header::size() || !data.startsWith(Header::fileIdentifier())) + break; + + extraSize += Header(data).completeTagSize(); + } + + if(extraSize != 0) { + debug("ID3v2::Tag::read() - Duplicate ID3v2 tags found."); + d->header.setTagSize(d->header.tagSize() + extraSize); } } diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 9ae1dffa..c5f9bbf6 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -446,24 +446,9 @@ long MPEG::File::firstFrameOffset() { long position = 0; - if(hasID3v2Tag()) { + if(hasID3v2Tag()) position = d->ID3v2Location + ID3v2Tag()->header()->completeTagSize(); - // Skip duplicate ID3v2 tags. - - // Workaround for some faulty files that have duplicate ID3v2 tags. - // Combination of EAC and LAME creates such files when configured incorrectly. - - long location; - while((location = findID3v2(position)) >= 0) { - seek(location); - const ID3v2::Header header(readBlock(ID3v2::Header::size())); - position = location + header.completeTagSize(); - - debug("MPEG::File::firstFrameOffset() - Duplicate ID3v2 tag found."); - } - } - return nextFrameOffset(position); } diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 5088841f..87863157 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -93,6 +93,7 @@ class TestID3v2 : public CppUnit::TestFixture CPPUNIT_TEST(testRenderTableOfContentsFrame); CPPUNIT_TEST(testShrinkPadding); CPPUNIT_TEST(testEmptyFrame); + CPPUNIT_TEST(testDuplicateTags); CPPUNIT_TEST_SUITE_END(); public: @@ -1117,6 +1118,41 @@ public: } } + void testDuplicateTags() + { + ScopedFileCopy copy("duplicate_id3v2", ".mp3"); + + ByteVector audioStream; + { + MPEG::File f(copy.fileName().c_str()); + f.seek(f.ID3v2Tag()->header()->completeTagSize()); + audioStream = f.readBlock(2089); + + // duplicate_id3v2.mp3 has duplicate ID3v2 tags. + // Sample rate will be 32000 if we can't skip the second tag. + + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL((TagLib::uint)8049, f.ID3v2Tag()->header()->completeTagSize()); + + CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); + + f.ID3v2Tag()->setArtist("Artist A"); + f.save(MPEG::File::ID3v2, true); + } + { + MPEG::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL((long)3594, f.length()); + CPPUNIT_ASSERT_EQUAL((TagLib::uint)1505, f.ID3v2Tag()->header()->completeTagSize()); + CPPUNIT_ASSERT_EQUAL(String("Artist A"), f.ID3v2Tag()->artist()); + CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); + + f.seek(f.ID3v2Tag()->header()->completeTagSize()); + CPPUNIT_ASSERT_EQUAL(f.readBlock(2089), audioStream); + + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v2); From 72a807def1375debb7f10b6815533113c37b1daf Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 11:50:39 +0900 Subject: [PATCH 06/15] Fix a string conversion bug in tag_c.cpp. --- bindings/c/tag_c.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bindings/c/tag_c.cpp b/bindings/c/tag_c.cpp index 41dd15eb..b0fd8c0b 100644 --- a/bindings/c/tag_c.cpp +++ b/bindings/c/tag_c.cpp @@ -159,7 +159,7 @@ BOOL taglib_file_save(TagLib_File *file) char *taglib_tag_title(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = stringToCharArray(t->title().toCString(unicodeStrings)); + char *s = stringToCharArray(t->title()); if(stringManagementEnabled) strings.append(s); return s; @@ -168,7 +168,7 @@ char *taglib_tag_title(const TagLib_Tag *tag) char *taglib_tag_artist(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = stringToCharArray(t->artist().toCString(unicodeStrings)); + char *s = stringToCharArray(t->artist()); if(stringManagementEnabled) strings.append(s); return s; @@ -177,7 +177,7 @@ char *taglib_tag_artist(const TagLib_Tag *tag) char *taglib_tag_album(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = stringToCharArray(t->album().toCString(unicodeStrings)); + char *s = stringToCharArray(t->album()); if(stringManagementEnabled) strings.append(s); return s; @@ -186,7 +186,7 @@ char *taglib_tag_album(const TagLib_Tag *tag) char *taglib_tag_comment(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = stringToCharArray(t->comment().toCString(unicodeStrings)); + char *s = stringToCharArray(t->comment()); if(stringManagementEnabled) strings.append(s); return s; @@ -195,7 +195,7 @@ char *taglib_tag_comment(const TagLib_Tag *tag) char *taglib_tag_genre(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = stringToCharArray(t->genre().toCString(unicodeStrings)); + char *s = stringToCharArray(t->genre()); if(stringManagementEnabled) strings.append(s); return s; From a3564d8c68643dcddbb6f166171a26653398a010 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 16:35:55 +0900 Subject: [PATCH 07/15] Efficient lookup for the MP4/ASF field name and ID3v1 genre tables. Linear lookup is much faster and memory efficient when an array is very small. --- taglib/asf/asftag.cpp | 108 ++++---- taglib/mp4/mp4tag.cpp | 124 ++++----- taglib/mpeg/id3v1/id3v1genres.cpp | 424 +++++++++++++++--------------- taglib/mpeg/id3v2/id3v2frame.cpp | 183 ++++++------- tests/test_id3v1.cpp | 10 +- 5 files changed, 437 insertions(+), 412 deletions(-) diff --git a/taglib/asf/asftag.cpp b/taglib/asf/asftag.cpp index bdf3c5da..5b61a024 100644 --- a/taglib/asf/asftag.cpp +++ b/taglib/asf/asftag.cpp @@ -210,58 +210,64 @@ bool ASF::Tag::isEmpty() const d->attributeListMap.isEmpty(); } -static const char *keyTranslation[][2] = { - { "WM/AlbumTitle", "ALBUM" }, - { "WM/AlbumArtist", "ALBUMARTIST" }, - { "WM/Composer", "COMPOSER" }, - { "WM/Writer", "WRITER" }, - { "WM/Conductor", "CONDUCTOR" }, - { "WM/ModifiedBy", "REMIXER" }, - { "WM/Year", "DATE" }, - { "WM/OriginalReleaseYear", "ORIGINALDATE" }, - { "WM/Producer", "PRODUCER" }, - { "WM/ContentGroupDescription", "GROUPING" }, - { "WM/SubTitle", "SUBTITLE" }, - { "WM/SetSubTitle", "DISCSUBTITLE" }, - { "WM/TrackNumber", "TRACKNUMBER" }, - { "WM/PartOfSet", "DISCNUMBER" }, - { "WM/Genre", "GENRE" }, - { "WM/BeatsPerMinute", "BPM" }, - { "WM/Mood", "MOOD" }, - { "WM/ISRC", "ISRC" }, - { "WM/Lyrics", "LYRICS" }, - { "WM/Media", "MEDIA" }, - { "WM/Publisher", "LABEL" }, - { "WM/CatalogNo", "CATALOGNUMBER" }, - { "WM/Barcode", "BARCODE" }, - { "WM/EncodedBy", "ENCODEDBY" }, - { "WM/AlbumSortOrder", "ALBUMSORT" }, - { "WM/AlbumArtistSortOrder", "ALBUMARTISTSORT" }, - { "WM/ArtistSortOrder", "ARTISTSORT" }, - { "WM/TitleSortOrder", "TITLESORT" }, - { "WM/Script", "SCRIPT" }, - { "WM/Language", "LANGUAGE" }, - { "MusicBrainz/Track Id", "MUSICBRAINZ_TRACKID" }, - { "MusicBrainz/Artist Id", "MUSICBRAINZ_ARTISTID" }, - { "MusicBrainz/Album Id", "MUSICBRAINZ_ALBUMID" }, - { "MusicBrainz/Album Artist Id", "MUSICBRAINZ_ALBUMARTISTID" }, - { "MusicBrainz/Release Group Id", "MUSICBRAINZ_RELEASEGROUPID" }, - { "MusicBrainz/Work Id", "MUSICBRAINZ_WORKID" }, - { "MusicIP/PUID", "MUSICIP_PUID" }, - { "Acoustid/Id", "ACOUSTID_ID" }, - { "Acoustid/Fingerprint", "ACOUSTID_FINGERPRINT" }, -}; +namespace +{ + const char *keyTranslation[][2] = { + { "WM/AlbumTitle", "ALBUM" }, + { "WM/AlbumArtist", "ALBUMARTIST" }, + { "WM/Composer", "COMPOSER" }, + { "WM/Writer", "WRITER" }, + { "WM/Conductor", "CONDUCTOR" }, + { "WM/ModifiedBy", "REMIXER" }, + { "WM/Year", "DATE" }, + { "WM/OriginalReleaseYear", "ORIGINALDATE" }, + { "WM/Producer", "PRODUCER" }, + { "WM/ContentGroupDescription", "GROUPING" }, + { "WM/SubTitle", "SUBTITLE" }, + { "WM/SetSubTitle", "DISCSUBTITLE" }, + { "WM/TrackNumber", "TRACKNUMBER" }, + { "WM/PartOfSet", "DISCNUMBER" }, + { "WM/Genre", "GENRE" }, + { "WM/BeatsPerMinute", "BPM" }, + { "WM/Mood", "MOOD" }, + { "WM/ISRC", "ISRC" }, + { "WM/Lyrics", "LYRICS" }, + { "WM/Media", "MEDIA" }, + { "WM/Publisher", "LABEL" }, + { "WM/CatalogNo", "CATALOGNUMBER" }, + { "WM/Barcode", "BARCODE" }, + { "WM/EncodedBy", "ENCODEDBY" }, + { "WM/AlbumSortOrder", "ALBUMSORT" }, + { "WM/AlbumArtistSortOrder", "ALBUMARTISTSORT" }, + { "WM/ArtistSortOrder", "ARTISTSORT" }, + { "WM/TitleSortOrder", "TITLESORT" }, + { "WM/Script", "SCRIPT" }, + { "WM/Language", "LANGUAGE" }, + { "MusicBrainz/Track Id", "MUSICBRAINZ_TRACKID" }, + { "MusicBrainz/Artist Id", "MUSICBRAINZ_ARTISTID" }, + { "MusicBrainz/Album Id", "MUSICBRAINZ_ALBUMID" }, + { "MusicBrainz/Album Artist Id", "MUSICBRAINZ_ALBUMARTISTID" }, + { "MusicBrainz/Release Group Id", "MUSICBRAINZ_RELEASEGROUPID" }, + { "MusicBrainz/Work Id", "MUSICBRAINZ_WORKID" }, + { "MusicIP/PUID", "MUSICIP_PUID" }, + { "Acoustid/Id", "ACOUSTID_ID" }, + { "Acoustid/Fingerprint", "ACOUSTID_FINGERPRINT" }, + }; + const size_t keyTranslationSize = sizeof(keyTranslation) / sizeof(keyTranslation[0]); + + String translateKey(const String &key) + { + for(size_t i = 0; i < keyTranslationSize; ++i) { + if(key == keyTranslation[i][0]) + return keyTranslation[i][1]; + } + + return String(); + } +} PropertyMap ASF::Tag::properties() const { - static Map keyMap; - if(keyMap.isEmpty()) { - int numKeys = sizeof(keyTranslation) / sizeof(keyTranslation[0]); - for(int i = 0; i < numKeys; i++) { - keyMap[keyTranslation[i][0]] = keyTranslation[i][1]; - } - } - PropertyMap props; if(!d->title.isEmpty()) { @@ -279,8 +285,8 @@ PropertyMap ASF::Tag::properties() const ASF::AttributeListMap::ConstIterator it = d->attributeListMap.begin(); for(; it != d->attributeListMap.end(); ++it) { - if(keyMap.contains(it->first)) { - String key = keyMap[it->first]; + const String key = translateKey(it->first); + if(!key.isEmpty()) { AttributeList::ConstIterator it2 = it->second.begin(); for(; it2 != it->second.end(); ++it2) { if(key == "TRACKNUMBER") { diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index 47c802ce..005c8a4d 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -814,70 +814,76 @@ bool MP4::Tag::contains(const String &key) const return d->items.contains(key); } -static const char *keyTranslation[][2] = { - { "\251nam", "TITLE" }, - { "\251ART", "ARTIST" }, - { "\251alb", "ALBUM" }, - { "\251cmt", "COMMENT" }, - { "\251gen", "GENRE" }, - { "\251day", "DATE" }, - { "\251wrt", "COMPOSER" }, - { "\251grp", "GROUPING" }, - { "aART", "ALBUMARTIST" }, - { "trkn", "TRACKNUMBER" }, - { "disk", "DISCNUMBER" }, - { "cpil", "COMPILATION" }, - { "tmpo", "BPM" }, - { "cprt", "COPYRIGHT" }, - { "\251lyr", "LYRICS" }, - { "\251too", "ENCODEDBY" }, - { "soal", "ALBUMSORT" }, - { "soaa", "ALBUMARTISTSORT" }, - { "soar", "ARTISTSORT" }, - { "sonm", "TITLESORT" }, - { "soco", "COMPOSERSORT" }, - { "sosn", "SHOWSORT" }, - { "----:com.apple.iTunes:MusicBrainz Track Id", "MUSICBRAINZ_TRACKID" }, - { "----:com.apple.iTunes:MusicBrainz Artist Id", "MUSICBRAINZ_ARTISTID" }, - { "----:com.apple.iTunes:MusicBrainz Album Id", "MUSICBRAINZ_ALBUMID" }, - { "----:com.apple.iTunes:MusicBrainz Album Artist Id", "MUSICBRAINZ_ALBUMARTISTID" }, - { "----:com.apple.iTunes:MusicBrainz Release Group Id", "MUSICBRAINZ_RELEASEGROUPID" }, - { "----:com.apple.iTunes:MusicBrainz Work Id", "MUSICBRAINZ_WORKID" }, - { "----:com.apple.iTunes:ASIN", "ASIN" }, - { "----:com.apple.iTunes:LABEL", "LABEL" }, - { "----:com.apple.iTunes:LYRICIST", "LYRICIST" }, - { "----:com.apple.iTunes:CONDUCTOR", "CONDUCTOR" }, - { "----:com.apple.iTunes:REMIXER", "REMIXER" }, - { "----:com.apple.iTunes:ENGINEER", "ENGINEER" }, - { "----:com.apple.iTunes:PRODUCER", "PRODUCER" }, - { "----:com.apple.iTunes:DJMIXER", "DJMIXER" }, - { "----:com.apple.iTunes:MIXER", "MIXER" }, - { "----:com.apple.iTunes:SUBTITLE", "SUBTITLE" }, - { "----:com.apple.iTunes:DISCSUBTITLE", "DISCSUBTITLE" }, - { "----:com.apple.iTunes:MOOD", "MOOD" }, - { "----:com.apple.iTunes:ISRC", "ISRC" }, - { "----:com.apple.iTunes:CATALOGNUMBER", "CATALOGNUMBER" }, - { "----:com.apple.iTunes:BARCODE", "BARCODE" }, - { "----:com.apple.iTunes:SCRIPT", "SCRIPT" }, - { "----:com.apple.iTunes:LANGUAGE", "LANGUAGE" }, - { "----:com.apple.iTunes:LICENSE", "LICENSE" }, - { "----:com.apple.iTunes:MEDIA", "MEDIA" }, -}; +namespace +{ + const char *keyTranslation[][2] = { + { "\251nam", "TITLE" }, + { "\251ART", "ARTIST" }, + { "\251alb", "ALBUM" }, + { "\251cmt", "COMMENT" }, + { "\251gen", "GENRE" }, + { "\251day", "DATE" }, + { "\251wrt", "COMPOSER" }, + { "\251grp", "GROUPING" }, + { "aART", "ALBUMARTIST" }, + { "trkn", "TRACKNUMBER" }, + { "disk", "DISCNUMBER" }, + { "cpil", "COMPILATION" }, + { "tmpo", "BPM" }, + { "cprt", "COPYRIGHT" }, + { "\251lyr", "LYRICS" }, + { "\251too", "ENCODEDBY" }, + { "soal", "ALBUMSORT" }, + { "soaa", "ALBUMARTISTSORT" }, + { "soar", "ARTISTSORT" }, + { "sonm", "TITLESORT" }, + { "soco", "COMPOSERSORT" }, + { "sosn", "SHOWSORT" }, + { "----:com.apple.iTunes:MusicBrainz Track Id", "MUSICBRAINZ_TRACKID" }, + { "----:com.apple.iTunes:MusicBrainz Artist Id", "MUSICBRAINZ_ARTISTID" }, + { "----:com.apple.iTunes:MusicBrainz Album Id", "MUSICBRAINZ_ALBUMID" }, + { "----:com.apple.iTunes:MusicBrainz Album Artist Id", "MUSICBRAINZ_ALBUMARTISTID" }, + { "----:com.apple.iTunes:MusicBrainz Release Group Id", "MUSICBRAINZ_RELEASEGROUPID" }, + { "----:com.apple.iTunes:MusicBrainz Work Id", "MUSICBRAINZ_WORKID" }, + { "----:com.apple.iTunes:ASIN", "ASIN" }, + { "----:com.apple.iTunes:LABEL", "LABEL" }, + { "----:com.apple.iTunes:LYRICIST", "LYRICIST" }, + { "----:com.apple.iTunes:CONDUCTOR", "CONDUCTOR" }, + { "----:com.apple.iTunes:REMIXER", "REMIXER" }, + { "----:com.apple.iTunes:ENGINEER", "ENGINEER" }, + { "----:com.apple.iTunes:PRODUCER", "PRODUCER" }, + { "----:com.apple.iTunes:DJMIXER", "DJMIXER" }, + { "----:com.apple.iTunes:MIXER", "MIXER" }, + { "----:com.apple.iTunes:SUBTITLE", "SUBTITLE" }, + { "----:com.apple.iTunes:DISCSUBTITLE", "DISCSUBTITLE" }, + { "----:com.apple.iTunes:MOOD", "MOOD" }, + { "----:com.apple.iTunes:ISRC", "ISRC" }, + { "----:com.apple.iTunes:CATALOGNUMBER", "CATALOGNUMBER" }, + { "----:com.apple.iTunes:BARCODE", "BARCODE" }, + { "----:com.apple.iTunes:SCRIPT", "SCRIPT" }, + { "----:com.apple.iTunes:LANGUAGE", "LANGUAGE" }, + { "----:com.apple.iTunes:LICENSE", "LICENSE" }, + { "----:com.apple.iTunes:MEDIA", "MEDIA" }, + }; + const size_t keyTranslationSize = sizeof(keyTranslation) / sizeof(keyTranslation[0]); + + String translateKey(const String &key) + { + for(size_t i = 0; i < keyTranslationSize; ++i) { + if(key == keyTranslation[i][0]) + return keyTranslation[i][1]; + } + + return String(); + } +} PropertyMap MP4::Tag::properties() const { - static Map keyMap; - if(keyMap.isEmpty()) { - int numKeys = sizeof(keyTranslation) / sizeof(keyTranslation[0]); - for(int i = 0; i < numKeys; i++) { - keyMap[keyTranslation[i][0]] = keyTranslation[i][1]; - } - } - PropertyMap props; for(MP4::ItemMap::ConstIterator it = d->items.begin(); it != d->items.end(); ++it) { - if(keyMap.contains(it->first)) { - String key = keyMap[it->first]; + const String key = translateKey(it->first); + if(!key.isEmpty()) { if(key == "TRACKNUMBER" || key == "DISCNUMBER") { MP4::Item::IntPair ip = it->second.toIntPair(); String value = String::number(ip.first); diff --git a/taglib/mpeg/id3v1/id3v1genres.cpp b/taglib/mpeg/id3v1/id3v1genres.cpp index 074c8bff..def91910 100644 --- a/taglib/mpeg/id3v1/id3v1genres.cpp +++ b/taglib/mpeg/id3v1/id3v1genres.cpp @@ -27,237 +27,239 @@ using namespace TagLib; -namespace TagLib { - namespace ID3v1 { - - static const int genresSize = 192; - static const String genres[] = { - "Blues", - "Classic Rock", - "Country", - "Dance", - "Disco", - "Funk", - "Grunge", - "Hip-Hop", - "Jazz", - "Metal", - "New Age", - "Oldies", - "Other", - "Pop", - "R&B", - "Rap", - "Reggae", - "Rock", - "Techno", - "Industrial", - "Alternative", - "Ska", - "Death Metal", - "Pranks", - "Soundtrack", - "Euro-Techno", - "Ambient", - "Trip-Hop", - "Vocal", - "Jazz+Funk", - "Fusion", - "Trance", - "Classical", - "Instrumental", - "Acid", - "House", - "Game", - "Sound Clip", - "Gospel", - "Noise", - "Alternative Rock", - "Bass", - "Soul", - "Punk", - "Space", - "Meditative", - "Instrumental Pop", - "Instrumental Rock", - "Ethnic", - "Gothic", - "Darkwave", - "Techno-Industrial", - "Electronic", - "Pop-Folk", - "Eurodance", - "Dream", - "Southern Rock", - "Comedy", - "Cult", - "Gangsta", - "Top 40", - "Christian Rap", - "Pop/Funk", - "Jungle", - "Native American", - "Cabaret", - "New Wave", - "Psychedelic", - "Rave", - "Showtunes", - "Trailer", - "Lo-Fi", - "Tribal", - "Acid Punk", - "Acid Jazz", - "Polka", - "Retro", - "Musical", - "Rock & Roll", - "Hard Rock", - "Folk", - "Folk/Rock", - "National Folk", - "Swing", - "Fusion", - "Bebob", - "Latin", - "Revival", - "Celtic", - "Bluegrass", - "Avantgarde", - "Gothic Rock", - "Progressive Rock", - "Psychedelic Rock", - "Symphonic Rock", - "Slow Rock", - "Big Band", - "Chorus", - "Easy Listening", - "Acoustic", - "Humour", - "Speech", - "Chanson", - "Opera", - "Chamber Music", - "Sonata", - "Symphony", - "Booty Bass", - "Primus", - "Porn Groove", - "Satire", - "Slow Jam", - "Club", - "Tango", - "Samba", - "Folklore", - "Ballad", - "Power Ballad", - "Rhythmic Soul", - "Freestyle", - "Duet", - "Punk Rock", - "Drum Solo", - "A Cappella", - "Euro-House", - "Dance Hall", - "Goa", - "Drum & Bass", - "Club-House", - "Hardcore", - "Terror", - "Indie", - "BritPop", - "Negerpunk", - "Polsk Punk", - "Beat", - "Christian Gangsta Rap", - "Heavy Metal", - "Black Metal", - "Crossover", - "Contemporary Christian", - "Christian Rock", - "Merengue", - "Salsa", - "Thrash Metal", - "Anime", - "Jpop", - "Synthpop", - "Abstract", - "Art Rock", - "Baroque", - "Bhangra", - "Big Beat", - "Breakbeat", - "Chillout", - "Downtempo", - "Dub", - "EBM", - "Eclectic", - "Electro", - "Electroclash", - "Emo", - "Experimental", - "Garage", - "Global", - "IDM", - "Illbient", - "Industro-Goth", - "Jam Band", - "Krautrock", - "Leftfield", - "Lounge", - "Math Rock", - "New Romantic", - "Nu-Breakz", - "Post-Punk", - "Post-Rock", - "Psytrance", - "Shoegaze", - "Space Rock", - "Trop Rock", - "World Music", - "Neoclassical", - "Audiobook", - "Audio Theatre", - "Neue Deutsche Welle", - "Podcast", - "Indie Rock", - "G-Funk", - "Dubstep", - "Garage Rock", - "Psybient" - }; - } +namespace +{ + const wchar *genres[] = { + L"Blues", + L"Classic Rock", + L"Country", + L"Dance", + L"Disco", + L"Funk", + L"Grunge", + L"Hip-Hop", + L"Jazz", + L"Metal", + L"New Age", + L"Oldies", + L"Other", + L"Pop", + L"R&B", + L"Rap", + L"Reggae", + L"Rock", + L"Techno", + L"Industrial", + L"Alternative", + L"Ska", + L"Death Metal", + L"Pranks", + L"Soundtrack", + L"Euro-Techno", + L"Ambient", + L"Trip-Hop", + L"Vocal", + L"Jazz+Funk", + L"Fusion", + L"Trance", + L"Classical", + L"Instrumental", + L"Acid", + L"House", + L"Game", + L"Sound Clip", + L"Gospel", + L"Noise", + L"Alternative Rock", + L"Bass", + L"Soul", + L"Punk", + L"Space", + L"Meditative", + L"Instrumental Pop", + L"Instrumental Rock", + L"Ethnic", + L"Gothic", + L"Darkwave", + L"Techno-Industrial", + L"Electronic", + L"Pop-Folk", + L"Eurodance", + L"Dream", + L"Southern Rock", + L"Comedy", + L"Cult", + L"Gangsta", + L"Top 40", + L"Christian Rap", + L"Pop/Funk", + L"Jungle", + L"Native American", + L"Cabaret", + L"New Wave", + L"Psychedelic", + L"Rave", + L"Showtunes", + L"Trailer", + L"Lo-Fi", + L"Tribal", + L"Acid Punk", + L"Acid Jazz", + L"Polka", + L"Retro", + L"Musical", + L"Rock & Roll", + L"Hard Rock", + L"Folk", + L"Folk/Rock", + L"National Folk", + L"Swing", + L"Fusion", + L"Bebob", + L"Latin", + L"Revival", + L"Celtic", + L"Bluegrass", + L"Avantgarde", + L"Gothic Rock", + L"Progressive Rock", + L"Psychedelic Rock", + L"Symphonic Rock", + L"Slow Rock", + L"Big Band", + L"Chorus", + L"Easy Listening", + L"Acoustic", + L"Humour", + L"Speech", + L"Chanson", + L"Opera", + L"Chamber Music", + L"Sonata", + L"Symphony", + L"Booty Bass", + L"Primus", + L"Porn Groove", + L"Satire", + L"Slow Jam", + L"Club", + L"Tango", + L"Samba", + L"Folklore", + L"Ballad", + L"Power Ballad", + L"Rhythmic Soul", + L"Freestyle", + L"Duet", + L"Punk Rock", + L"Drum Solo", + L"A Cappella", + L"Euro-House", + L"Dance Hall", + L"Goa", + L"Drum & Bass", + L"Club-House", + L"Hardcore", + L"Terror", + L"Indie", + L"BritPop", + L"Negerpunk", + L"Polsk Punk", + L"Beat", + L"Christian Gangsta Rap", + L"Heavy Metal", + L"Black Metal", + L"Crossover", + L"Contemporary Christian", + L"Christian Rock", + L"Merengue", + L"Salsa", + L"Thrash Metal", + L"Anime", + L"Jpop", + L"Synthpop", + L"Abstract", + L"Art Rock", + L"Baroque", + L"Bhangra", + L"Big Beat", + L"Breakbeat", + L"Chillout", + L"Downtempo", + L"Dub", + L"EBM", + L"Eclectic", + L"Electro", + L"Electroclash", + L"Emo", + L"Experimental", + L"Garage", + L"Global", + L"IDM", + L"Illbient", + L"Industro-Goth", + L"Jam Band", + L"Krautrock", + L"Leftfield", + L"Lounge", + L"Math Rock", + L"New Romantic", + L"Nu-Breakz", + L"Post-Punk", + L"Post-Rock", + L"Psytrance", + L"Shoegaze", + L"Space Rock", + L"Trop Rock", + L"World Music", + L"Neoclassical", + L"Audiobook", + L"Audio Theatre", + L"Neue Deutsche Welle", + L"Podcast", + L"Indie Rock", + L"G-Funk", + L"Dubstep", + L"Garage Rock", + L"Psybient" + }; + const int genresSize = sizeof(genres) / sizeof(genres[0]); } StringList ID3v1::genreList() { - static StringList l; - if(l.isEmpty()) { - for(int i = 0; i < genresSize; i++) - l.append(genres[i]); + StringList l; + for(int i = 0; i < genresSize; i++) { + l.append(genres[i]); } + return l; } ID3v1::GenreMap ID3v1::genreMap() { - static GenreMap m; - if(m.isEmpty()) { - for(int i = 0; i < genresSize; i++) - m.insert(genres[i], i); + GenreMap m; + for(int i = 0; i < genresSize; i++) { + m.insert(genres[i], i); } + return m; } String ID3v1::genre(int i) { if(i >= 0 && i < genresSize) - return genres[i] + String::null; // always make a copy - return String::null; + return String(genres[i]); // always make a copy + else + return String(); } int ID3v1::genreIndex(const String &name) { - if(genreMap().contains(name)) - return genreMap()[name]; + for(int i = 0; i < genresSize; ++i) { + if(name == genres[i]) + return i; + } + return 255; } diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 91eec328..1e679780 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -364,98 +364,101 @@ String::Type Frame::checkTextEncoding(const StringList &fields, String::Type enc return checkEncoding(fields, encoding, header()->version()); } -static const size_t frameTranslationSize = 51; -static const char *frameTranslation[][2] = { - // Text information frames - { "TALB", "ALBUM"}, - { "TBPM", "BPM" }, - { "TCOM", "COMPOSER" }, - { "TCON", "GENRE" }, - { "TCOP", "COPYRIGHT" }, - { "TDEN", "ENCODINGTIME" }, - { "TDLY", "PLAYLISTDELAY" }, - { "TDOR", "ORIGINALDATE" }, - { "TDRC", "DATE" }, - // { "TRDA", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 - // { "TDAT", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 - // { "TYER", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 - // { "TIME", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 - { "TDRL", "RELEASEDATE" }, - { "TDTG", "TAGGINGDATE" }, - { "TENC", "ENCODEDBY" }, - { "TEXT", "LYRICIST" }, - { "TFLT", "FILETYPE" }, - //{ "TIPL", "INVOLVEDPEOPLE" }, handled separately - { "TIT1", "CONTENTGROUP" }, - { "TIT2", "TITLE"}, - { "TIT3", "SUBTITLE" }, - { "TKEY", "INITIALKEY" }, - { "TLAN", "LANGUAGE" }, - { "TLEN", "LENGTH" }, - //{ "TMCL", "MUSICIANCREDITS" }, handled separately - { "TMED", "MEDIA" }, - { "TMOO", "MOOD" }, - { "TOAL", "ORIGINALALBUM" }, - { "TOFN", "ORIGINALFILENAME" }, - { "TOLY", "ORIGINALLYRICIST" }, - { "TOPE", "ORIGINALARTIST" }, - { "TOWN", "OWNER" }, - { "TPE1", "ARTIST"}, - { "TPE2", "ALBUMARTIST" }, // id3's spec says 'PERFORMER', but most programs use 'ALBUMARTIST' - { "TPE3", "CONDUCTOR" }, - { "TPE4", "REMIXER" }, // could also be ARRANGER - { "TPOS", "DISCNUMBER" }, - { "TPRO", "PRODUCEDNOTICE" }, - { "TPUB", "LABEL" }, - { "TRCK", "TRACKNUMBER" }, - { "TRSN", "RADIOSTATION" }, - { "TRSO", "RADIOSTATIONOWNER" }, - { "TSOA", "ALBUMSORT" }, - { "TSOP", "ARTISTSORT" }, - { "TSOT", "TITLESORT" }, - { "TSO2", "ALBUMARTISTSORT" }, // non-standard, used by iTunes - { "TSRC", "ISRC" }, - { "TSSE", "ENCODING" }, - // URL frames - { "WCOP", "COPYRIGHTURL" }, - { "WOAF", "FILEWEBPAGE" }, - { "WOAR", "ARTISTWEBPAGE" }, - { "WOAS", "AUDIOSOURCEWEBPAGE" }, - { "WORS", "RADIOSTATIONWEBPAGE" }, - { "WPAY", "PAYMENTWEBPAGE" }, - { "WPUB", "PUBLISHERWEBPAGE" }, - //{ "WXXX", "URL"}, handled specially - // Other frames - { "COMM", "COMMENT" }, - //{ "USLT", "LYRICS" }, handled specially - // Apple iTunes proprietary frames - { "PCST", "PODCAST" }, - { "TCAT", "PODCASTCATEGORY" }, - { "TDES", "PODCASTDESC" }, - { "TGID", "PODCASTID" }, - { "WFED", "PODCASTURL" }, -}; +namespace +{ + const char *frameTranslation[][2] = { + // Text information frames + { "TALB", "ALBUM"}, + { "TBPM", "BPM" }, + { "TCOM", "COMPOSER" }, + { "TCON", "GENRE" }, + { "TCOP", "COPYRIGHT" }, + { "TDEN", "ENCODINGTIME" }, + { "TDLY", "PLAYLISTDELAY" }, + { "TDOR", "ORIGINALDATE" }, + { "TDRC", "DATE" }, + // { "TRDA", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 + // { "TDAT", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 + // { "TYER", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 + // { "TIME", "DATE" }, // id3 v2.3, replaced by TDRC in v2.4 + { "TDRL", "RELEASEDATE" }, + { "TDTG", "TAGGINGDATE" }, + { "TENC", "ENCODEDBY" }, + { "TEXT", "LYRICIST" }, + { "TFLT", "FILETYPE" }, + //{ "TIPL", "INVOLVEDPEOPLE" }, handled separately + { "TIT1", "CONTENTGROUP" }, + { "TIT2", "TITLE"}, + { "TIT3", "SUBTITLE" }, + { "TKEY", "INITIALKEY" }, + { "TLAN", "LANGUAGE" }, + { "TLEN", "LENGTH" }, + //{ "TMCL", "MUSICIANCREDITS" }, handled separately + { "TMED", "MEDIA" }, + { "TMOO", "MOOD" }, + { "TOAL", "ORIGINALALBUM" }, + { "TOFN", "ORIGINALFILENAME" }, + { "TOLY", "ORIGINALLYRICIST" }, + { "TOPE", "ORIGINALARTIST" }, + { "TOWN", "OWNER" }, + { "TPE1", "ARTIST"}, + { "TPE2", "ALBUMARTIST" }, // id3's spec says 'PERFORMER', but most programs use 'ALBUMARTIST' + { "TPE3", "CONDUCTOR" }, + { "TPE4", "REMIXER" }, // could also be ARRANGER + { "TPOS", "DISCNUMBER" }, + { "TPRO", "PRODUCEDNOTICE" }, + { "TPUB", "LABEL" }, + { "TRCK", "TRACKNUMBER" }, + { "TRSN", "RADIOSTATION" }, + { "TRSO", "RADIOSTATIONOWNER" }, + { "TSOA", "ALBUMSORT" }, + { "TSOP", "ARTISTSORT" }, + { "TSOT", "TITLESORT" }, + { "TSO2", "ALBUMARTISTSORT" }, // non-standard, used by iTunes + { "TSRC", "ISRC" }, + { "TSSE", "ENCODING" }, + // URL frames + { "WCOP", "COPYRIGHTURL" }, + { "WOAF", "FILEWEBPAGE" }, + { "WOAR", "ARTISTWEBPAGE" }, + { "WOAS", "AUDIOSOURCEWEBPAGE" }, + { "WORS", "RADIOSTATIONWEBPAGE" }, + { "WPAY", "PAYMENTWEBPAGE" }, + { "WPUB", "PUBLISHERWEBPAGE" }, + //{ "WXXX", "URL"}, handled specially + // Other frames + { "COMM", "COMMENT" }, + //{ "USLT", "LYRICS" }, handled specially + // Apple iTunes proprietary frames + { "PCST", "PODCAST" }, + { "TCAT", "PODCASTCATEGORY" }, + { "TDES", "PODCASTDESC" }, + { "TGID", "PODCASTID" }, + { "WFED", "PODCASTURL" }, + }; + const size_t frameTranslationSize = sizeof(frameTranslation) / sizeof(frameTranslation[0]); -static const size_t txxxFrameTranslationSize = 8; -static const char *txxxFrameTranslation[][2] = { - { "MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID" }, - { "MUSICBRAINZ ARTIST ID", "MUSICBRAINZ_ARTISTID" }, - { "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ_ALBUMARTISTID" }, - { "MUSICBRAINZ RELEASE GROUP ID", "MUSICBRAINZ_RELEASEGROUPID" }, - { "MUSICBRAINZ WORK ID", "MUSICBRAINZ_WORKID" }, - { "ACOUSTID ID", "ACOUSTID_ID" }, - { "ACOUSTID FINGERPRINT", "ACOUSTID_FINGERPRINT" }, - { "MUSICIP PUID", "MUSICIP_PUID" }, -}; + const char *txxxFrameTranslation[][2] = { + { "MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID" }, + { "MUSICBRAINZ ARTIST ID", "MUSICBRAINZ_ARTISTID" }, + { "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ_ALBUMARTISTID" }, + { "MUSICBRAINZ RELEASE GROUP ID", "MUSICBRAINZ_RELEASEGROUPID" }, + { "MUSICBRAINZ WORK ID", "MUSICBRAINZ_WORKID" }, + { "ACOUSTID ID", "ACOUSTID_ID" }, + { "ACOUSTID FINGERPRINT", "ACOUSTID_FINGERPRINT" }, + { "MUSICIP PUID", "MUSICIP_PUID" }, + }; + const size_t txxxFrameTranslationSize = sizeof(txxxFrameTranslation) / sizeof(txxxFrameTranslation[0]); -// list of deprecated frames and their successors -static const size_t deprecatedFramesSize = 4; -static const char *deprecatedFrames[][2] = { - {"TRDA", "TDRC"}, // 2.3 -> 2.4 (http://en.wikipedia.org/wiki/ID3) - {"TDAT", "TDRC"}, // 2.3 -> 2.4 - {"TYER", "TDRC"}, // 2.3 -> 2.4 - {"TIME", "TDRC"}, // 2.3 -> 2.4 -}; + // list of deprecated frames and their successors + const char *deprecatedFrames[][2] = { + {"TRDA", "TDRC"}, // 2.3 -> 2.4 (http://en.wikipedia.org/wiki/ID3) + {"TDAT", "TDRC"}, // 2.3 -> 2.4 + {"TYER", "TDRC"}, // 2.3 -> 2.4 + {"TIME", "TDRC"}, // 2.3 -> 2.4 + }; + const size_t deprecatedFramesSize = sizeof(deprecatedFrames) / sizeof(deprecatedFrames[0]);; +} String Frame::frameIDToKey(const ByteVector &id) { diff --git a/tests/test_id3v1.cpp b/tests/test_id3v1.cpp index 7db2dbbf..e778419a 100644 --- a/tests/test_id3v1.cpp +++ b/tests/test_id3v1.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include "utils.h" @@ -13,6 +14,7 @@ class TestID3v1 : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TestID3v1); CPPUNIT_TEST(testStripWhiteSpace); + CPPUNIT_TEST(testGenres); CPPUNIT_TEST_SUITE_END(); public: @@ -27,7 +29,7 @@ public: f.ID3v1Tag(true)->setArtist("Artist "); f.save(); } - + { MPEG::File f(newname.c_str()); CPPUNIT_ASSERT(f.ID3v1Tag(false)); @@ -35,6 +37,12 @@ public: } } + void testGenres() + { + CPPUNIT_ASSERT_EQUAL(String("Darkwave"), ID3v1::genre(50)); + CPPUNIT_ASSERT_EQUAL(100, ID3v1::genreIndex("Humour")); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v1); From e10684312e641f0d4da668a2851e97e174f8abee Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 17:29:25 +0900 Subject: [PATCH 08/15] Efficient lookup for an ID3v2 tag in a MPEG file. An ID3v2 tag or MPEG frame is most likely be at the beginning of the file. --- taglib/mpeg/mpegfile.cpp | 123 ++++++++------------------------------- taglib/mpeg/mpegfile.h | 2 +- 2 files changed, 25 insertions(+), 100 deletions(-) diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index c5f9bbf6..8b7af692 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -489,7 +489,7 @@ void MPEG::File::read(bool readProperties) { // Look for an ID3v2 tag - d->ID3v2Location = findID3v2(0); + d->ID3v2Location = findID3v2(); if(d->ID3v2Location >= 0) { @@ -532,115 +532,40 @@ void MPEG::File::read(bool readProperties) ID3v1Tag(true); } -long MPEG::File::findID3v2(long offset) +long MPEG::File::findID3v2() { - // This method is based on the contents of TagLib::File::find(), but because - // of some subtleties -- specifically the need to look for the bit pattern of - // an MPEG sync, it has been modified for use here. + if(!isValid()) + return -1; - if(isValid() && ID3v2::Header::fileIdentifier().size() <= bufferSize()) { + // An ID3v2 tag or MPEG frame is most likely be at the beginning of the file. - // The position in the file that the current buffer starts at. + const ByteVector headerID = ID3v2::Header::fileIdentifier(); - long bufferOffset = 0; - ByteVector buffer; + seek(0); - // These variables are used to keep track of a partial match that happens at - // the end of a buffer. + const ByteVector data = readBlock(headerID.size()); + if(data.size() < headerID.size()) + return -1; - int previousPartialMatch = -1; - bool previousPartialSynchMatch = false; + if(data == headerID) + return 0; - // Save the location of the current read pointer. We will restore the - // position using seek() before all returns. + if(firstSyncByte(data[0]) && secondSynchByte(data[1])) + return -1; - long originalPosition = tell(); + // Look for the entire file, if neither an MEPG frame or ID3v2 tag was found + // at the beginning of the file. + // We don't care about the inefficiency of the code, since this is a seldom case. - // Start the search at the offset. + const long tagOffset = find(headerID); + if(tagOffset < 0) + return -1; - seek(offset); + const long frameOffset = firstFrameOffset(); + if(frameOffset < tagOffset) + return -1; - // This loop is the crux of the find method. There are three cases that we - // want to account for: - // (1) The previously searched buffer contained a partial match of the search - // pattern and we want to see if the next one starts with the remainder of - // that pattern. - // - // (2) The search pattern is wholly contained within the current buffer. - // - // (3) The current buffer ends with a partial match of the pattern. We will - // note this for use in the next iteration, where we will check for the rest - // of the pattern. - - for(buffer = readBlock(bufferSize()); buffer.size() > 0; buffer = readBlock(bufferSize())) { - - // (1) previous partial match - - if(previousPartialSynchMatch && secondSynchByte(buffer[0])) - return -1; - - if(previousPartialMatch >= 0 && int(bufferSize()) > previousPartialMatch) { - const int patternOffset = (bufferSize() - previousPartialMatch); - if(buffer.containsAt(ID3v2::Header::fileIdentifier(), 0, patternOffset)) { - seek(originalPosition); - return offset + bufferOffset - bufferSize() + previousPartialMatch; - } - } - - // (2) pattern contained in current buffer - - long location = buffer.find(ID3v2::Header::fileIdentifier()); - if(location >= 0) { - seek(originalPosition); - return offset + bufferOffset + location; - } - - int firstSynchByte = buffer.find(char(uchar(255))); - - // Here we have to loop because there could be several of the first - // (11111111) byte, and we want to check all such instances until we find - // a full match (11111111 111) or hit the end of the buffer. - - while(firstSynchByte >= 0) { - - // if this *is not* at the end of the buffer - - if(firstSynchByte < int(buffer.size()) - 1) { - if(secondSynchByte(buffer[firstSynchByte + 1])) { - // We've found the frame synch pattern. - seek(originalPosition); - return -1; - } - else { - - // We found 11111111 at the end of the current buffer indicating a - // partial match of the synch pattern. The find() below should - // return -1 and break out of the loop. - - previousPartialSynchMatch = true; - } - } - - // Check in the rest of the buffer. - - firstSynchByte = buffer.find(char(uchar(255)), firstSynchByte + 1); - } - - // (3) partial match - - previousPartialMatch = buffer.endsWithPartialMatch(ID3v2::Header::fileIdentifier()); - - bufferOffset += bufferSize(); - } - - // Since we hit the end of the file, reset the status before continuing. - - clear(); - - seek(originalPosition); - } - - return -1; + return tagOffset; } long MPEG::File::findID3v1() diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index 858a6a5c..8478d093 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -390,7 +390,7 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties); - long findID3v2(long offset); + long findID3v2(); long findID3v1(); void findAPE(); From f310b1810daaadd125dd6b32844c4d316664149a Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 17:55:18 +0900 Subject: [PATCH 09/15] Update NEWS. --- NEWS | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS b/NEWS index b5a27901..f0ac3e57 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,11 @@ +========================== + + * Added support for ID3v2 PCST and WFED frames. + * Added String::clear(). + * Marked ByteVector::null and ByteVector::isNull() deprecated. + * Marked String::null and ByteVector::isNull() deprecated. + * Many smaller bug fixes and performance improvements. + TagLib 1.10 (Nov 11, 2015) ========================== From 1cc3e4cc5730adc298e3fcac5313203d3fa1c5f7 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 18:24:20 +0900 Subject: [PATCH 10/15] Consistent rounding when calculating the MP4 audio length. --- taglib/mp4/mp4properties.cpp | 2 +- tests/test_mp4.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/mp4/mp4properties.cpp b/taglib/mp4/mp4properties.cpp index 0ac77342..ba821547 100644 --- a/taglib/mp4/mp4properties.cpp +++ b/taglib/mp4/mp4properties.cpp @@ -187,7 +187,7 @@ MP4::Properties::read(File *file, Atoms *atoms) length = data.toUInt(24U); } if(unit > 0 && length > 0) - d->length = static_cast(length * 1000.0 / unit); + d->length = static_cast(length * 1000.0 / unit + 0.5); MP4::Atom *atom = trak->find("mdia", "minf", "stbl", "stsd"); if(!atom) { diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index c110a612..58dbae42 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -40,7 +40,7 @@ public: 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(3708, f.audioProperties()->lengthInMilliseconds()); CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->bitrate()); CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); From 93d55dafd65293dafc2abd6188787e19c7baf7b9 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 10:50:21 +0900 Subject: [PATCH 11/15] Update NEWS. --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index f0ac3e57..709971d5 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,7 @@ * Added support for ID3v2 PCST and WFED frames. * Added String::clear(). + * Better handling of duplicate ID3v2 tags in all kinds of files. * Marked ByteVector::null and ByteVector::isNull() deprecated. * Marked String::null and ByteVector::isNull() deprecated. * Many smaller bug fixes and performance improvements. From 4161746d895e0e3de2e74bf79adc085cd5924de5 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 13:25:28 +0900 Subject: [PATCH 12/15] Update NEWS. --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 709971d5..c979c571 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ * Added support for ID3v2 PCST and WFED frames. * Added String::clear(). * Better handling of duplicate ID3v2 tags in all kinds of files. + * Fixed crash when calling File::properties() after strip(). * Marked ByteVector::null and ByteVector::isNull() deprecated. * Marked String::null and ByteVector::isNull() deprecated. * Many smaller bug fixes and performance improvements. From 779f904940f7d2f19e75dc233666e32032f66f49 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 13:35:07 +0900 Subject: [PATCH 13/15] Update NEWS. --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index c979c571..e4d1c9a5 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ * Added support for ID3v2 PCST and WFED frames. * Added String::clear(). * Better handling of duplicate ID3v2 tags in all kinds of files. + * Better handling of duplicate tags in WAV files. * Fixed crash when calling File::properties() after strip(). * Marked ByteVector::null and ByteVector::isNull() deprecated. * Marked String::null and ByteVector::isNull() deprecated. From 6dc8d701a8db9a39799f5dea3aedb7104143eb1d Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 4 Aug 2015 13:50:09 +0900 Subject: [PATCH 14/15] Avoid writing duplicate tags when saving ASF files. Reduce memory reallocations and copies when saving ASF files. --- taglib/asf/asffile.cpp | 20 +++++++++++++++----- tests/test_asf.cpp | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/taglib/asf/asffile.cpp b/taglib/asf/asffile.cpp index 02fe83c3..a5e5ea84 100644 --- a/taglib/asf/asffile.cpp +++ b/taglib/asf/asffile.cpp @@ -50,7 +50,7 @@ public: class MetadataLibraryObject; FilePrivate(): - size(0), + headerSize(0), tag(0), properties(0), contentDescriptionObject(0), @@ -68,7 +68,7 @@ public: delete properties; } - unsigned long long size; + unsigned long long headerSize; ASF::Tag *tag; ASF::Properties *properties; @@ -556,6 +556,10 @@ bool ASF::File::save() d->headerExtensionObject->objects.append(d->metadataLibraryObject); } + d->extendedContentDescriptionObject->attributeData.clear(); + d->metadataObject->attributeData.clear(); + d->metadataLibraryObject->attributeData.clear(); + const AttributeListMap allAttributes = d->tag->attributeListMap(); for(AttributeListMap::ConstIterator it = allAttributes.begin(); it != allAttributes.end(); ++it) { @@ -591,8 +595,14 @@ bool ASF::File::save() data.append((*it)->render(this)); } - data = headerGuid + ByteVector::fromLongLong(data.size() + 30, false) + ByteVector::fromUInt(d->objects.size(), false) + ByteVector("\x01\x02", 2) + data; - insert(data, 0, (TagLib::ulong)d->size); + seek(16); + writeBlock(ByteVector::fromLongLong(data.size() + 30, false)); + writeBlock(ByteVector::fromUInt(d->objects.size(), false)); + writeBlock(ByteVector("\x01\x02", 2)); + + insert(data, 30, static_cast(d->headerSize - 30)); + + d->headerSize = data.size() + 30; return true; } @@ -617,7 +627,7 @@ void ASF::File::read() d->properties = new ASF::Properties(); bool ok; - d->size = readQWORD(this, &ok); + d->headerSize = readQWORD(this, &ok); if(!ok) { setValid(false); return; diff --git a/tests/test_asf.cpp b/tests/test_asf.cpp index 663ae583..37614b0c 100644 --- a/tests/test_asf.cpp +++ b/tests/test_asf.cpp @@ -25,6 +25,7 @@ class TestASF : public CppUnit::TestFixture CPPUNIT_TEST(testSavePicture); CPPUNIT_TEST(testSaveMultiplePictures); CPPUNIT_TEST(testProperties); + CPPUNIT_TEST(testRepeatedSave); CPPUNIT_TEST_SUITE_END(); public: @@ -270,6 +271,21 @@ public: CPPUNIT_ASSERT_EQUAL(StringList("3"), tags["DISCNUMBER"]); } + void testRepeatedSave() + { + ScopedFileCopy copy("silence-1", ".wma"); + + { + ASF::File f(copy.fileName().c_str()); + f.tag()->setTitle(std::string(128 * 1024, 'X').c_str()); + f.save(); + CPPUNIT_ASSERT_EQUAL(297578L, f.length()); + f.tag()->setTitle(std::string(16 * 1024, 'X').c_str()); + f.save(); + CPPUNIT_ASSERT_EQUAL(68202L, f.length()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestASF); From f956c891418003c5ae2e4850a9af8062b50b2db5 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 11:12:37 +0900 Subject: [PATCH 15/15] Remove a warning from a comment and update NEWS. --- NEWS | 1 + taglib/asf/asffile.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/NEWS b/NEWS index e4d1c9a5..1b3751bd 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ * Better handling of duplicate ID3v2 tags in all kinds of files. * Better handling of duplicate tags in WAV files. * Fixed crash when calling File::properties() after strip(). + * Fixed possible file corruptions when saving ASF files. * Marked ByteVector::null and ByteVector::isNull() deprecated. * Marked String::null and ByteVector::isNull() deprecated. * Many smaller bug fixes and performance improvements. diff --git a/taglib/asf/asffile.h b/taglib/asf/asffile.h index f1ae431f..b674da79 100644 --- a/taglib/asf/asffile.h +++ b/taglib/asf/asffile.h @@ -112,9 +112,6 @@ namespace TagLib { * Save the file. * * This returns true if the save was successful. - * - * \warning In the current implementation, it's dangerous to call save() - * repeatedly. At worst it will corrupt the file. */ virtual bool save();