From 1bb06b1f7a2a00ce0e0c119a728d15cde92882ec Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 3 Aug 2015 23:01:15 +0900 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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; }