From e750cb491d4b4de609f6339d6cceb11ab0d4bfa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Tue, 28 Apr 2015 11:56:07 +0200 Subject: [PATCH 01/15] FileRef: Allow an IOStream to be used --- taglib/fileref.cpp | 50 ++++++++++++++++++++++++++++-------------- taglib/fileref.h | 16 +++++++++++++- tests/test_fileref.cpp | 12 ++++++++++ 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/taglib/fileref.cpp b/taglib/fileref.cpp index 2b5d5f23..c2cd31d1 100644 --- a/taglib/fileref.cpp +++ b/taglib/fileref.cpp @@ -54,6 +54,10 @@ using namespace TagLib; +template +static File* createInternal(T arg, const String& filename, bool readAudioProperties, + AudioProperties::ReadStyle audioPropertiesStyle); + class FileRef::FileRefPrivate : public RefCounter { public: @@ -83,6 +87,17 @@ FileRef::FileRef(FileName fileName, bool readAudioProperties, d = new FileRefPrivate(create(fileName, readAudioProperties, audioPropertiesStyle)); } +FileRef::FileRef(IOStream* stream, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) +{ + d = new FileRefPrivate(createInternal(stream, +#ifdef _WIN32 + stream->name().toString() +#else + stream->name() +#endif + , readAudioProperties, audioPropertiesStyle)); +} + FileRef::FileRef(File *file) { d = new FileRefPrivate(file); @@ -213,24 +228,25 @@ File *FileRef::create(FileName fileName, bool readAudioProperties, return file; } + return createInternal(fileName, +#ifdef _WIN32 + fileName.toString() +#else + fileName +#endif + , readAudioProperties, audioPropertiesStyle); +} + +template +static File* createInternal(T fileName, const String& s, bool readAudioProperties, + AudioProperties::ReadStyle audioPropertiesStyle) +{ // Ok, this is really dumb for now, but it works for testing. String ext; - { -#ifdef _WIN32 - - String s = fileName.toString(); - -#else - - String s = fileName; - - #endif - - const int pos = s.rfind("."); - if(pos != -1) - ext = s.substr(pos + 1).upper(); - } + const int pos = s.rfind("."); + if(pos != -1) + ext = s.substr(pos + 1).upper(); // If this list is updated, the method defaultFileExtensions() should also be // updated. However at some point that list should be created at the same time @@ -238,7 +254,7 @@ File *FileRef::create(FileName fileName, bool readAudioProperties, if(!ext.isEmpty()) { if(ext == "MP3") - return new MPEG::File(fileName, readAudioProperties, audioPropertiesStyle); + return new MPEG::File(fileName, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); if(ext == "OGG") return new Ogg::Vorbis::File(fileName, readAudioProperties, audioPropertiesStyle); if(ext == "OGA") { @@ -250,7 +266,7 @@ File *FileRef::create(FileName fileName, bool readAudioProperties, return new Ogg::Vorbis::File(fileName, readAudioProperties, audioPropertiesStyle); } if(ext == "FLAC") - return new FLAC::File(fileName, readAudioProperties, audioPropertiesStyle); + return new FLAC::File(fileName, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); if(ext == "MPC") return new MPC::File(fileName, readAudioProperties, audioPropertiesStyle); if(ext == "WV") diff --git a/taglib/fileref.h b/taglib/fileref.h index 1e965229..abeb0af9 100644 --- a/taglib/fileref.h +++ b/taglib/fileref.h @@ -128,7 +128,21 @@ namespace TagLib { audioPropertiesStyle = AudioProperties::Average); /*! - * Construct a FileRef using \a file. The FileRef now takes ownership of the + * Construct a FileRef from an opened \a IOStream. If \a readAudioProperties is true then + * the audio properties will be read using \a audioPropertiesStyle. If + * \a readAudioProperties is false then \a audioPropertiesStyle will be + * ignored. + * + * Also see the note in the class documentation about why you may not want to + * use this method in your application. + */ + explicit FileRef(IOStream* stream, + bool readAudioProperties = true, + AudioProperties::ReadStyle + audioPropertiesStyle = AudioProperties::Average); + + /*! + * Contruct a FileRef using \a file. The FileRef now takes ownership of the * pointer and will delete the File when it passes out of scope. */ explicit FileRef(File *file); diff --git a/tests/test_fileref.cpp b/tests/test_fileref.cpp index f13eafaa..a51b868c 100644 --- a/tests/test_fileref.cpp +++ b/tests/test_fileref.cpp @@ -6,6 +6,7 @@ #include #include #include "utils.h" +#include using namespace std; using namespace TagLib; @@ -74,6 +75,17 @@ public: CPPUNIT_ASSERT_EQUAL(f->tag()->track(), TagLib::uint(7)); CPPUNIT_ASSERT_EQUAL(f->tag()->year(), TagLib::uint(2080)); delete f; + + FileStream fs(newname.c_str()); + f = new FileRef(&fs); + CPPUNIT_ASSERT(!f->isNull()); + CPPUNIT_ASSERT_EQUAL(f->tag()->artist(), String("ttest artist")); + CPPUNIT_ASSERT_EQUAL(f->tag()->title(), String("ytest title")); + CPPUNIT_ASSERT_EQUAL(f->tag()->genre(), String("uTest!")); + CPPUNIT_ASSERT_EQUAL(f->tag()->album(), String("ialbummmm")); + CPPUNIT_ASSERT_EQUAL(f->tag()->track(), TagLib::uint(7)); + CPPUNIT_ASSERT_EQUAL(f->tag()->year(), TagLib::uint(2080)); + delete f; } void testMusepack() From b3cea6cca7ae2339d3207c9fb9e09dfeae9ad022 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 3 Sep 2015 09:12:05 +0900 Subject: [PATCH 02/15] Fix XiphComment::setComment() for the case that a Vorbis comment has the "COMMENT" field. --- taglib/ogg/xiphcomment.cpp | 9 ++++++++- tests/test_xiphcomment.cpp | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 9462607f..39903b04 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -137,7 +137,14 @@ void Ogg::XiphComment::setAlbum(const String &s) void Ogg::XiphComment::setComment(const String &s) { - addField(d->commentField.isEmpty() ? "DESCRIPTION" : d->commentField, s); + if(d->commentField.isEmpty()) { + if(!d->fieldListMap["DESCRIPTION"].isEmpty()) + d->commentField = "DESCRIPTION"; + else + d->commentField = "COMMENT"; + } + + addField(d->commentField, s); } void Ogg::XiphComment::setGenre(const String &s) diff --git a/tests/test_xiphcomment.cpp b/tests/test_xiphcomment.cpp index 6526229b..e2358333 100644 --- a/tests/test_xiphcomment.cpp +++ b/tests/test_xiphcomment.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -17,6 +18,7 @@ class TestXiphComment : public CppUnit::TestFixture CPPUNIT_TEST(testTrack); CPPUNIT_TEST(testSetTrack); CPPUNIT_TEST(testInvalidKeys); + CPPUNIT_TEST(testClearComment); CPPUNIT_TEST_SUITE_END(); public: @@ -74,6 +76,22 @@ public: CPPUNIT_ASSERT(cmt.properties().isEmpty()); } + void testClearComment() + { + ScopedFileCopy copy("no-tags", ".flac"); + + { + FLAC::File f(copy.fileName().c_str()); + f.xiphComment()->addField("COMMENT", "Comment1"); + f.save(); + } + { + FLAC::File f(copy.fileName().c_str()); + f.xiphComment()->setComment(""); + CPPUNIT_ASSERT_EQUAL(String(""), f.xiphComment()->comment()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestXiphComment); From ba5137bf2d3b1bcca9269e3a7f5f194243c87d23 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 12 Nov 2015 17:37:22 +0900 Subject: [PATCH 03/15] Use always "COMMENT" field when updating XiphComment. The recommended field name for additional comments is "COMMENT". It's the same behavior as "DATE" or "TRACKNUMBER" field. --- taglib/ogg/xiphcomment.cpp | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 39903b04..5630d25b 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -36,7 +36,6 @@ class Ogg::XiphComment::XiphCommentPrivate public: FieldListMap fieldListMap; String vendorID; - String commentField; }; //////////////////////////////////////////////////////////////////////////////// @@ -82,16 +81,10 @@ String Ogg::XiphComment::album() const String Ogg::XiphComment::comment() const { - if(!d->fieldListMap["DESCRIPTION"].isEmpty()) { - d->commentField = "DESCRIPTION"; - return d->fieldListMap["DESCRIPTION"].toString(); - } - - if(!d->fieldListMap["COMMENT"].isEmpty()) { - d->commentField = "COMMENT"; + if(!d->fieldListMap["COMMENT"].isEmpty()) return d->fieldListMap["COMMENT"].toString(); - } - + if(!d->fieldListMap["DESCRIPTION"].isEmpty()) + return d->fieldListMap["DESCRIPTION"].toString(); return String::null; } @@ -137,14 +130,8 @@ void Ogg::XiphComment::setAlbum(const String &s) void Ogg::XiphComment::setComment(const String &s) { - if(d->commentField.isEmpty()) { - if(!d->fieldListMap["DESCRIPTION"].isEmpty()) - d->commentField = "DESCRIPTION"; - else - d->commentField = "COMMENT"; - } - - addField(d->commentField, s); + removeField("DESCRIPTION"); + addField("COMMENT", s); } void Ogg::XiphComment::setGenre(const String &s) From 8f147034d6af7a4c2830b3113d4b257c1836fabe Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 13 Nov 2015 11:07:50 +0900 Subject: [PATCH 04/15] Add a test about handing "COMMENT" and "DESCIPRION" fields in XiphComment. --- tests/test_xiphcomment.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_xiphcomment.cpp b/tests/test_xiphcomment.cpp index e2358333..e21d9807 100644 --- a/tests/test_xiphcomment.cpp +++ b/tests/test_xiphcomment.cpp @@ -17,6 +17,7 @@ class TestXiphComment : public CppUnit::TestFixture CPPUNIT_TEST(testSetYear); CPPUNIT_TEST(testTrack); CPPUNIT_TEST(testSetTrack); + CPPUNIT_TEST(testSetComment); CPPUNIT_TEST(testInvalidKeys); CPPUNIT_TEST(testClearComment); CPPUNIT_TEST_SUITE_END(); @@ -63,6 +64,16 @@ public: CPPUNIT_ASSERT_EQUAL(String("3"), cmt.fieldListMap()["TRACKNUMBER"].front()); } + void testSetComment() + { + Ogg::XiphComment cmt; + cmt.addField("DESCRIPTION", "Test Comment 1"); + cmt.addField("COMMENT", "Test Comment 2"); + cmt.setComment("Test Comment 3"); + CPPUNIT_ASSERT(cmt.fieldListMap()["DESCRIPTION"].isEmpty()); + CPPUNIT_ASSERT_EQUAL(String("Test Comment 3"), cmt.fieldListMap()["COMMENT"].front()); + } + void testInvalidKeys() { PropertyMap map; From 7d0a651f3e1569f554d709c067798df1981c5c8f Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 14:18:23 +0900 Subject: [PATCH 05/15] Revert "Use always "COMMENT" field when updating XiphComment." This reverts commit ba5137bf2d3b1bcca9269e3a7f5f194243c87d23. --- taglib/ogg/xiphcomment.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 5630d25b..39903b04 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -36,6 +36,7 @@ class Ogg::XiphComment::XiphCommentPrivate public: FieldListMap fieldListMap; String vendorID; + String commentField; }; //////////////////////////////////////////////////////////////////////////////// @@ -81,10 +82,16 @@ String Ogg::XiphComment::album() const String Ogg::XiphComment::comment() const { - if(!d->fieldListMap["COMMENT"].isEmpty()) - return d->fieldListMap["COMMENT"].toString(); - if(!d->fieldListMap["DESCRIPTION"].isEmpty()) + if(!d->fieldListMap["DESCRIPTION"].isEmpty()) { + d->commentField = "DESCRIPTION"; return d->fieldListMap["DESCRIPTION"].toString(); + } + + if(!d->fieldListMap["COMMENT"].isEmpty()) { + d->commentField = "COMMENT"; + return d->fieldListMap["COMMENT"].toString(); + } + return String::null; } @@ -130,8 +137,14 @@ void Ogg::XiphComment::setAlbum(const String &s) void Ogg::XiphComment::setComment(const String &s) { - removeField("DESCRIPTION"); - addField("COMMENT", s); + if(d->commentField.isEmpty()) { + if(!d->fieldListMap["DESCRIPTION"].isEmpty()) + d->commentField = "DESCRIPTION"; + else + d->commentField = "COMMENT"; + } + + addField(d->commentField, s); } void Ogg::XiphComment::setGenre(const String &s) From 29b0568decccf28ac53571be7d48ac8a0cf4a747 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 14:18:40 +0900 Subject: [PATCH 06/15] Revert "Add a test about handing "COMMENT" and "DESCIPRION" fields in XiphComment." This reverts commit 8f147034d6af7a4c2830b3113d4b257c1836fabe. --- tests/test_xiphcomment.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/test_xiphcomment.cpp b/tests/test_xiphcomment.cpp index e21d9807..e2358333 100644 --- a/tests/test_xiphcomment.cpp +++ b/tests/test_xiphcomment.cpp @@ -17,7 +17,6 @@ class TestXiphComment : public CppUnit::TestFixture CPPUNIT_TEST(testSetYear); CPPUNIT_TEST(testTrack); CPPUNIT_TEST(testSetTrack); - CPPUNIT_TEST(testSetComment); CPPUNIT_TEST(testInvalidKeys); CPPUNIT_TEST(testClearComment); CPPUNIT_TEST_SUITE_END(); @@ -64,16 +63,6 @@ public: CPPUNIT_ASSERT_EQUAL(String("3"), cmt.fieldListMap()["TRACKNUMBER"].front()); } - void testSetComment() - { - Ogg::XiphComment cmt; - cmt.addField("DESCRIPTION", "Test Comment 1"); - cmt.addField("COMMENT", "Test Comment 2"); - cmt.setComment("Test Comment 3"); - CPPUNIT_ASSERT(cmt.fieldListMap()["DESCRIPTION"].isEmpty()); - CPPUNIT_ASSERT_EQUAL(String("Test Comment 3"), cmt.fieldListMap()["COMMENT"].front()); - } - void testInvalidKeys() { PropertyMap map; From c4fe65787c8302ab7f0475a139b7fc10059a020b Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 20:59:13 +0900 Subject: [PATCH 07/15] Avoid using String::isNull() where it is considered to be confused with isEmpty(). --- taglib/ape/apetag.cpp | 4 ++-- taglib/mod/modtag.cpp | 2 +- taglib/mpeg/id3v2/frames/commentsframe.cpp | 4 +--- taglib/mpeg/id3v2/frames/synchronizedlyricsframe.cpp | 4 ++-- taglib/mpeg/id3v2/frames/textidentificationframe.cpp | 2 +- taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp | 4 +--- taglib/mpeg/id3v2/frames/urllinkframe.cpp | 2 -- taglib/tag.cpp | 10 +++++----- taglib/toolkit/tpropertymap.cpp | 2 +- 9 files changed, 14 insertions(+), 20 deletions(-) diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index c633d575..00e1bc77 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -195,7 +195,7 @@ PropertyMap APE::Tag::properties() const String tagName = it->first.upper(); // if the item is Binary or Locator, or if the key is an invalid string, // add to unsupportedData - if(it->second.type() != Item::Text || tagName.isNull()) + if(it->second.type() != Item::Text || tagName.isEmpty()) properties.unsupportedData().append(it->first); else { // Some tags need to be handled specially @@ -232,7 +232,7 @@ PropertyMap APE::Tag::setProperties(const PropertyMap &origProps) for(; remIt != itemListMap().end(); ++remIt) { String key = remIt->first.upper(); // only remove if a) key is valid, b) type is text, c) key not contained in new properties - if(!key.isNull() && remIt->second.type() == APE::Item::Text && !properties.contains(key)) + if(!key.isEmpty() && remIt->second.type() == APE::Item::Text && !properties.contains(key)) toRemove.append(remIt->first); } diff --git a/taglib/mod/modtag.cpp b/taglib/mod/modtag.cpp index 4ba72117..4b180bbb 100644 --- a/taglib/mod/modtag.cpp +++ b/taglib/mod/modtag.cpp @@ -128,7 +128,7 @@ PropertyMap Mod::Tag::properties() const PropertyMap properties; properties["TITLE"] = d->title; properties["COMMENT"] = d->comment; - if(!(d->trackerName.isNull())) + if(!(d->trackerName.isEmpty())) properties["TRACKERNAME"] = d->trackerName; return properties; } diff --git a/taglib/mpeg/id3v2/frames/commentsframe.cpp b/taglib/mpeg/id3v2/frames/commentsframe.cpp index deaa9d9a..b5618322 100644 --- a/taglib/mpeg/id3v2/frames/commentsframe.cpp +++ b/taglib/mpeg/id3v2/frames/commentsframe.cpp @@ -116,8 +116,6 @@ PropertyMap CommentsFrame::asProperties() const PropertyMap map; if(key.isEmpty() || key == "COMMENT") map.insert("COMMENT", text()); - else if(key.isNull()) - map.unsupportedData().append(L"COMM/" + description()); else map.insert("COMMENT:" + key, text()); return map; @@ -164,7 +162,7 @@ void CommentsFrame::parseFields(const ByteVector &data) } else { d->description = String(l.front(), d->textEncoding); d->text = String(l.back(), d->textEncoding); - } + } } } diff --git a/taglib/mpeg/id3v2/frames/synchronizedlyricsframe.cpp b/taglib/mpeg/id3v2/frames/synchronizedlyricsframe.cpp index 86c11f7a..2ab46481 100644 --- a/taglib/mpeg/id3v2/frames/synchronizedlyricsframe.cpp +++ b/taglib/mpeg/id3v2/frames/synchronizedlyricsframe.cpp @@ -159,7 +159,7 @@ void SynchronizedLyricsFrame::parseFields(const ByteVector &data) int pos = 6; d->description = readStringField(data, d->textEncoding, &pos); - if(d->description.isNull()) + if(d->description.isEmpty()) return; /* @@ -190,7 +190,7 @@ void SynchronizedLyricsFrame::parseFields(const ByteVector &data) } } String text = readStringField(data, enc, &pos); - if(text.isNull() || pos + 4 > end) + if(text.isEmpty() || pos + 4 > end) return; uint time = data.toUInt(pos, true); diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp index 07bf0ede..7eacf932 100644 --- a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp +++ b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp @@ -292,7 +292,7 @@ PropertyMap TextIdentificationFrame::makeTMCLProperties() const StringList l = fieldList(); for(StringList::ConstIterator it = l.begin(); it != l.end(); ++it) { String instrument = it->upper(); - if(instrument.isNull()) { + if(instrument.isEmpty()) { // instrument is not a valid key -> frame unsupported map.clear(); map.unsupportedData().append(frameID()); diff --git a/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp b/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp index 8f96cb8d..f97cdc5a 100644 --- a/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp +++ b/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp @@ -119,8 +119,6 @@ PropertyMap UnsynchronizedLyricsFrame::asProperties() const String key = description().upper(); if(key.isEmpty() || key.upper() == "LYRICS") map.insert("LYRICS", text()); - else if(key.isNull()) - map.unsupportedData().append(L"USLT/" + description()); else map.insert("LYRICS:" + key, text()); return map; @@ -164,7 +162,7 @@ void UnsynchronizedLyricsFrame::parseFields(const ByteVector &data) } else { d->description = String(l.front(), d->textEncoding); d->text = String(l.back(), d->textEncoding); - } + } } } diff --git a/taglib/mpeg/id3v2/frames/urllinkframe.cpp b/taglib/mpeg/id3v2/frames/urllinkframe.cpp index e42d64d1..e59a80dd 100644 --- a/taglib/mpeg/id3v2/frames/urllinkframe.cpp +++ b/taglib/mpeg/id3v2/frames/urllinkframe.cpp @@ -159,8 +159,6 @@ PropertyMap UserUrlLinkFrame::asProperties() const String key = description().upper(); if(key.isEmpty() || key.upper() == "URL") map.insert("URL", url()); - else if(key.isNull()) - map.unsupportedData().append(L"WXXX/" + description()); else map.insert("URL:" + key, url()); return map; diff --git a/taglib/tag.cpp b/taglib/tag.cpp index eb8fab7a..d13b248f 100644 --- a/taglib/tag.cpp +++ b/taglib/tag.cpp @@ -58,15 +58,15 @@ bool Tag::isEmpty() const PropertyMap Tag::properties() const { PropertyMap map; - if(!(title().isNull())) + if(!(title().isEmpty())) map["TITLE"].append(title()); - if(!(artist().isNull())) + if(!(artist().isEmpty())) map["ARTIST"].append(artist()); - if(!(album().isNull())) + if(!(album().isEmpty())) map["ALBUM"].append(album()); - if(!(comment().isNull())) + if(!(comment().isEmpty())) map["COMMENT"].append(comment()); - if(!(genre().isNull())) + if(!(genre().isEmpty())) map["GENRE"].append(genre()); if(!(year() == 0)) map["DATE"].append(String::number(year())); diff --git a/taglib/toolkit/tpropertymap.cpp b/taglib/toolkit/tpropertymap.cpp index 313d7fb4..2c696417 100644 --- a/taglib/toolkit/tpropertymap.cpp +++ b/taglib/toolkit/tpropertymap.cpp @@ -35,7 +35,7 @@ PropertyMap::PropertyMap(const SimplePropertyMap &m) { for(SimplePropertyMap::ConstIterator it = m.begin(); it != m.end(); ++it){ String key = it->first.upper(); - if(!key.isNull()) + if(!key.isEmpty()) insert(it->first, it->second); else unsupported.append(it->first); From 8c6fe4545305472f286fb038d375540695aa49e2 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 22:21:47 +0900 Subject: [PATCH 08/15] Avoid using String::null where an empty string is required. String::null is not necessarily be empty or remains the same instance. Using it in a public header may lead to a linkage error. --- taglib/ape/apeitem.cpp | 2 +- taglib/ape/apetag.cpp | 10 ++-- taglib/asf/asftag.cpp | 12 ++--- taglib/it/itfile.cpp | 4 +- taglib/mod/modfile.cpp | 2 +- taglib/mod/modtag.cpp | 12 ++--- taglib/mp4/mp4tag.cpp | 10 ++-- taglib/mpeg/id3v1/id3v1tag.cpp | 2 +- .../id3v2/frames/tableofcontentsframe.cpp | 2 +- .../id3v2/frames/textidentificationframe.cpp | 14 ++--- .../frames/uniquefileidentifierframe.cpp | 2 +- taglib/mpeg/id3v2/frames/unknownframe.cpp | 2 +- taglib/mpeg/id3v2/id3v2frame.cpp | 2 +- taglib/mpeg/id3v2/id3v2framefactory.cpp | 2 +- taglib/mpeg/id3v2/id3v2tag.cpp | 10 ++-- taglib/ogg/xiphcomment.cpp | 10 ++-- taglib/s3m/s3mfile.cpp | 2 +- taglib/tag.cpp | 10 ++-- taglib/tagunion.cpp | 2 +- taglib/toolkit/tiostream.cpp | 52 +++++++++---------- taglib/toolkit/tpropertymap.cpp | 3 +- taglib/xm/xmfile.cpp | 4 +- tests/test_apetag.cpp | 2 +- tests/test_id3v2.cpp | 2 +- tests/test_it.cpp | 6 +-- tests/test_mod.cpp | 6 +-- tests/test_s3m.cpp | 6 +-- tests/test_string.cpp | 2 +- tests/test_xm.cpp | 16 +++--- 29 files changed, 106 insertions(+), 105 deletions(-) diff --git a/taglib/ape/apeitem.cpp b/taglib/ape/apeitem.cpp index 442dce63..d04cc1d1 100644 --- a/taglib/ape/apeitem.cpp +++ b/taglib/ape/apeitem.cpp @@ -210,7 +210,7 @@ String APE::Item::toString() const if(d->type == Text && !isEmpty()) return d->text.front(); else - return String::null; + return String(); } bool APE::Item::isEmpty() const diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index 00e1bc77..170a77c8 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -91,35 +91,35 @@ ByteVector APE::Tag::fileIdentifier() String APE::Tag::title() const { if(d->itemListMap["TITLE"].isEmpty()) - return String::null; + return String(); return d->itemListMap["TITLE"].values().toString(); } String APE::Tag::artist() const { if(d->itemListMap["ARTIST"].isEmpty()) - return String::null; + return String(); return d->itemListMap["ARTIST"].values().toString(); } String APE::Tag::album() const { if(d->itemListMap["ALBUM"].isEmpty()) - return String::null; + return String(); return d->itemListMap["ALBUM"].values().toString(); } String APE::Tag::comment() const { if(d->itemListMap["COMMENT"].isEmpty()) - return String::null; + return String(); return d->itemListMap["COMMENT"].values().toString(); } String APE::Tag::genre() const { if(d->itemListMap["GENRE"].isEmpty()) - return String::null; + return String(); return d->itemListMap["GENRE"].values().toString(); } diff --git a/taglib/asf/asftag.cpp b/taglib/asf/asftag.cpp index 5b61a024..ea9141a3 100644 --- a/taglib/asf/asftag.cpp +++ b/taglib/asf/asftag.cpp @@ -64,7 +64,7 @@ String ASF::Tag::album() const { if(d->attributeListMap.contains("WM/AlbumTitle")) return d->attributeListMap["WM/AlbumTitle"][0].toString(); - return String::null; + return String(); } String ASF::Tag::copyright() const @@ -107,7 +107,7 @@ String ASF::Tag::genre() const { if(d->attributeListMap.contains("WM/Genre")) return d->attributeListMap["WM/Genre"][0].toString(); - return String::null; + return String(); } void ASF::Tag::setTitle(const String &value) @@ -329,16 +329,16 @@ PropertyMap ASF::Tag::setProperties(const PropertyMap &props) for(; it != origProps.end(); ++it) { if(!props.contains(it->first) || props[it->first].isEmpty()) { if(it->first == "TITLE") { - d->title = String::null; + d->title.clear(); } else if(it->first == "ARTIST") { - d->artist = String::null; + d->artist.clear(); } else if(it->first == "COMMENT") { - d->comment = String::null; + d->comment.clear(); } else if(it->first == "COPYRIGHT") { - d->copyright = String::null; + d->copyright.clear(); } else { d->attributeListMap.erase(reverseKeyMap[it->first]); diff --git a/taglib/it/itfile.cpp b/taglib/it/itfile.cpp index ad5cf0b8..51ad8ac1 100644 --- a/taglib/it/itfile.cpp +++ b/taglib/it/itfile.cpp @@ -118,7 +118,7 @@ bool IT::File::save() if(i < lines.size()) writeString(lines[i], 25); else - writeString(String::null, 25); + writeString(String(), 25); writeByte(0); } @@ -133,7 +133,7 @@ bool IT::File::save() if((TagLib::uint)(i + instrumentCount) < lines.size()) writeString(lines[i + instrumentCount], 25); else - writeString(String::null, 25); + writeString(String(), 25); writeByte(0); } diff --git a/taglib/mod/modfile.cpp b/taglib/mod/modfile.cpp index ce974c16..2741e1bf 100644 --- a/taglib/mod/modfile.cpp +++ b/taglib/mod/modfile.cpp @@ -99,7 +99,7 @@ bool Mod::File::save() } for(uint i = n; i < d->properties.instrumentCount(); ++ i) { - writeString(String::null, 22); + writeString(String(), 22); seek(8, Current); } return true; diff --git a/taglib/mod/modtag.cpp b/taglib/mod/modtag.cpp index 4b180bbb..af98fb92 100644 --- a/taglib/mod/modtag.cpp +++ b/taglib/mod/modtag.cpp @@ -55,12 +55,12 @@ String Mod::Tag::title() const String Mod::Tag::artist() const { - return String::null; + return String(); } String Mod::Tag::album() const { - return String::null; + return String(); } String Mod::Tag::comment() const @@ -70,7 +70,7 @@ String Mod::Tag::comment() const String Mod::Tag::genre() const { - return String::null; + return String(); } TagLib::uint Mod::Tag::year() const @@ -142,19 +142,19 @@ PropertyMap Mod::Tag::setProperties(const PropertyMap &origProps) d->title = properties["TITLE"].front(); oneValueSet.append("TITLE"); } else - d->title = String::null; + d->title.clear(); if(properties.contains("COMMENT")) { d->comment = properties["COMMENT"].front(); oneValueSet.append("COMMENT"); } else - d->comment = String::null; + d->comment.clear(); if(properties.contains("TRACKERNAME")) { d->trackerName = properties["TRACKERNAME"].front(); oneValueSet.append("TRACKERNAME"); } else - d->trackerName = String::null; + d->trackerName.clear(); // for each tag that has been set above, remove the first entry in the corresponding // value list. The others will be returned as unsupported by this format. diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index 005c8a4d..84219196 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -686,7 +686,7 @@ MP4::Tag::title() const { if(d->items.contains("\251nam")) return d->items["\251nam"].toStringList().toString(", "); - return String::null; + return String(); } String @@ -694,7 +694,7 @@ MP4::Tag::artist() const { if(d->items.contains("\251ART")) return d->items["\251ART"].toStringList().toString(", "); - return String::null; + return String(); } String @@ -702,7 +702,7 @@ MP4::Tag::album() const { if(d->items.contains("\251alb")) return d->items["\251alb"].toStringList().toString(", "); - return String::null; + return String(); } String @@ -710,7 +710,7 @@ MP4::Tag::comment() const { if(d->items.contains("\251cmt")) return d->items["\251cmt"].toStringList().toString(", "); - return String::null; + return String(); } String @@ -718,7 +718,7 @@ MP4::Tag::genre() const { if(d->items.contains("\251gen")) return d->items["\251gen"].toStringList().toString(", "); - return String::null; + return String(); } unsigned int diff --git a/taglib/mpeg/id3v1/id3v1tag.cpp b/taglib/mpeg/id3v1/id3v1tag.cpp index 9fc8cfd7..f4004184 100644 --- a/taglib/mpeg/id3v1/id3v1tag.cpp +++ b/taglib/mpeg/id3v1/id3v1tag.cpp @@ -184,7 +184,7 @@ void ID3v1::Tag::setGenre(const String &s) void ID3v1::Tag::setYear(TagLib::uint i) { - d->year = i > 0 ? String::number(i) : String::null; + d->year = i > 0 ? String::number(i) : String(); } void ID3v1::Tag::setTrack(TagLib::uint i) diff --git a/taglib/mpeg/id3v2/frames/tableofcontentsframe.cpp b/taglib/mpeg/id3v2/frames/tableofcontentsframe.cpp index bdcc1183..1e8a1af8 100644 --- a/taglib/mpeg/id3v2/frames/tableofcontentsframe.cpp +++ b/taglib/mpeg/id3v2/frames/tableofcontentsframe.cpp @@ -214,7 +214,7 @@ void TableOfContentsFrame::removeEmbeddedFrames(const ByteVector &id) String TableOfContentsFrame::toString() const { - return String::null; + return String(); } PropertyMap TableOfContentsFrame::asProperties() const diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp index 7eacf932..8cd0d940 100644 --- a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp +++ b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp @@ -312,8 +312,8 @@ UserTextIdentificationFrame::UserTextIdentificationFrame(String::Type encoding) d(0) { StringList l; - l.append(String::null); - l.append(String::null); + l.append(String()); + l.append(String()); setText(l); } @@ -341,7 +341,7 @@ String UserTextIdentificationFrame::description() const { return !TextIdentificationFrame::fieldList().isEmpty() ? TextIdentificationFrame::fieldList().front() - : String::null; + : String(); } StringList UserTextIdentificationFrame::fieldList() const @@ -354,7 +354,7 @@ StringList UserTextIdentificationFrame::fieldList() const void UserTextIdentificationFrame::setText(const String &text) { if(description().isEmpty()) - setDescription(String::null); + setDescription(String()); TextIdentificationFrame::setText(StringList(description()).append(text)); } @@ -362,7 +362,7 @@ void UserTextIdentificationFrame::setText(const String &text) void UserTextIdentificationFrame::setText(const StringList &fields) { if(description().isEmpty()) - setDescription(String::null); + setDescription(String()); TextIdentificationFrame::setText(StringList(description()).append(fields)); } @@ -417,7 +417,7 @@ void UserTextIdentificationFrame::checkFields() int fields = fieldList().size(); if(fields == 0) - setDescription(String::null); + setDescription(String()); if(fields <= 1) - setText(String::null); + setText(String()); } diff --git a/taglib/mpeg/id3v2/frames/uniquefileidentifierframe.cpp b/taglib/mpeg/id3v2/frames/uniquefileidentifierframe.cpp index a0e842e0..a986e4db 100644 --- a/taglib/mpeg/id3v2/frames/uniquefileidentifierframe.cpp +++ b/taglib/mpeg/id3v2/frames/uniquefileidentifierframe.cpp @@ -86,7 +86,7 @@ void UniqueFileIdentifierFrame::setIdentifier(const ByteVector &v) String UniqueFileIdentifierFrame::toString() const { - return String::null; + return String(); } PropertyMap UniqueFileIdentifierFrame::asProperties() const diff --git a/taglib/mpeg/id3v2/frames/unknownframe.cpp b/taglib/mpeg/id3v2/frames/unknownframe.cpp index 4def028b..9d059193 100644 --- a/taglib/mpeg/id3v2/frames/unknownframe.cpp +++ b/taglib/mpeg/id3v2/frames/unknownframe.cpp @@ -51,7 +51,7 @@ UnknownFrame::~UnknownFrame() String UnknownFrame::toString() const { - return String::null; + return String(); } ByteVector UnknownFrame::data() const diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 1e679780..f811ed69 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -317,7 +317,7 @@ String Frame::readStringField(const ByteVector &data, String::Type encoding, int int end = data.find(delimiter, *position, delimiter.size()); if(end < *position) - return String::null; + return String(); String str; if(encoding == String::Latin1) diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp index bf4b0ee8..f387e937 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.cpp +++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp @@ -516,7 +516,7 @@ void FrameFactory::updateGenre(TextIdentificationFrame *frame) const } if(newfields.isEmpty()) - fields.append(String::null); + fields.append(String()); frame->setText(newfields); diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index f06cfbad..be5bfd20 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -140,21 +140,21 @@ String ID3v2::Tag::title() const { if(!d->frameListMap["TIT2"].isEmpty()) return d->frameListMap["TIT2"].front()->toString(); - return String::null; + return String(); } String ID3v2::Tag::artist() const { if(!d->frameListMap["TPE1"].isEmpty()) return d->frameListMap["TPE1"].front()->toString(); - return String::null; + return String(); } String ID3v2::Tag::album() const { if(!d->frameListMap["TALB"].isEmpty()) return d->frameListMap["TALB"].front()->toString(); - return String::null; + return String(); } String ID3v2::Tag::comment() const @@ -162,7 +162,7 @@ String ID3v2::Tag::comment() const const FrameList &comments = d->frameListMap["COMM"]; if(comments.isEmpty()) - return String::null; + return String(); for(FrameList::ConstIterator it = comments.begin(); it != comments.end(); ++it) { @@ -184,7 +184,7 @@ String ID3v2::Tag::genre() const if(d->frameListMap["TCON"].isEmpty() || !dynamic_cast(d->frameListMap["TCON"].front())) { - return String::null; + return String(); } // ID3v2.4 lists genres as the fields in its frames field list. If the field diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 1da0edac..090675c2 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -62,21 +62,21 @@ Ogg::XiphComment::~XiphComment() String Ogg::XiphComment::title() const { if(d->fieldListMap["TITLE"].isEmpty()) - return String::null; + return String(); return d->fieldListMap["TITLE"].toString(); } String Ogg::XiphComment::artist() const { if(d->fieldListMap["ARTIST"].isEmpty()) - return String::null; + return String(); return d->fieldListMap["ARTIST"].toString(); } String Ogg::XiphComment::album() const { if(d->fieldListMap["ALBUM"].isEmpty()) - return String::null; + return String(); return d->fieldListMap["ALBUM"].toString(); } @@ -92,13 +92,13 @@ String Ogg::XiphComment::comment() const return d->fieldListMap["COMMENT"].toString(); } - return String::null; + return String(); } String Ogg::XiphComment::genre() const { if(d->fieldListMap["GENRE"].isEmpty()) - return String::null; + return String(); return d->fieldListMap["GENRE"].toString(); } diff --git a/taglib/s3m/s3mfile.cpp b/taglib/s3m/s3mfile.cpp index 371340a5..f15d740b 100644 --- a/taglib/s3m/s3mfile.cpp +++ b/taglib/s3m/s3mfile.cpp @@ -134,7 +134,7 @@ bool S3M::File::save() if(i < lines.size()) writeString(lines[i], 27); else - writeString(String::null, 27); + writeString(String(), 27); // string terminating NUL is not optional: writeByte(0); } diff --git a/taglib/tag.cpp b/taglib/tag.cpp index d13b248f..3526226f 100644 --- a/taglib/tag.cpp +++ b/taglib/tag.cpp @@ -89,31 +89,31 @@ PropertyMap Tag::setProperties(const PropertyMap &origProps) setTitle(properties["TITLE"].front()); oneValueSet.append("TITLE"); } else - setTitle(String::null); + setTitle(String()); if(properties.contains("ARTIST")) { setArtist(properties["ARTIST"].front()); oneValueSet.append("ARTIST"); } else - setArtist(String::null); + setArtist(String()); if(properties.contains("ALBUM")) { setAlbum(properties["ALBUM"].front()); oneValueSet.append("ALBUM"); } else - setAlbum(String::null); + setAlbum(String()); if(properties.contains("COMMENT")) { setComment(properties["COMMENT"].front()); oneValueSet.append("COMMENT"); } else - setComment(String::null); + setComment(String()); if(properties.contains("GENRE")) { setGenre(properties["GENRE"].front()); oneValueSet.append("GENRE"); } else - setGenre(String::null); + setGenre(String()); if(properties.contains("DATE")) { bool ok; diff --git a/taglib/tagunion.cpp b/taglib/tagunion.cpp index a6743d14..30a53583 100644 --- a/taglib/tagunion.cpp +++ b/taglib/tagunion.cpp @@ -42,7 +42,7 @@ using namespace TagLib; return tag(1)->method(); \ if(tag(2) && !tag(2)->method().isEmpty()) \ return tag(2)->method(); \ - return String::null \ + return String(); \ #define numberUnion(method) \ if(tag(0) && tag(0)->method() > 0) \ diff --git a/taglib/toolkit/tiostream.cpp b/taglib/toolkit/tiostream.cpp index 0284aed6..72fe32a6 100644 --- a/taglib/toolkit/tiostream.cpp +++ b/taglib/toolkit/tiostream.cpp @@ -33,10 +33,10 @@ using namespace TagLib; # include "tdebug.h" # include -namespace +namespace { // Check if the running system has CreateFileW() function. - // Windows9x systems don't have CreateFileW() or can't accept Unicode file names. + // Windows9x systems don't have CreateFileW() or can't accept Unicode file names. bool supportsUnicode() { @@ -49,11 +49,11 @@ namespace } // Indicates whether the system supports Unicode file names. - - const bool SystemSupportsUnicode = supportsUnicode(); + + const bool SystemSupportsUnicode = supportsUnicode(); // Converts a UTF-16 string into a local encoding. - // This function should only be used in Windows9x systems which don't support + // This function should only be used in Windows9x systems which don't support // Unicode file names. std::string unicodeToAnsi(const wchar_t *wstr) @@ -76,52 +76,52 @@ namespace // If WinNT, stores a Unicode string into m_wname directly. // If Win9x, converts and stores it into m_name to avoid calling Unicode version functions. -FileName::FileName(const wchar_t *name) +FileName::FileName(const wchar_t *name) : m_name (SystemSupportsUnicode ? "" : unicodeToAnsi(name)) , m_wname(SystemSupportsUnicode ? name : L"") { } -FileName::FileName(const char *name) - : m_name(name) +FileName::FileName(const char *name) + : m_name(name) { } -FileName::FileName(const FileName &name) - : m_name (name.m_name) +FileName::FileName(const FileName &name) + : m_name (name.m_name) , m_wname(name.m_wname) { } -FileName::operator const wchar_t *() const -{ - return m_wname.c_str(); +FileName::operator const wchar_t *() const +{ + return m_wname.c_str(); } -FileName::operator const char *() const -{ - return m_name.c_str(); +FileName::operator const char *() const +{ + return m_name.c_str(); } -const std::wstring &FileName::wstr() const -{ - return m_wname; +const std::wstring &FileName::wstr() const +{ + return m_wname; } -const std::string &FileName::str() const -{ - return m_name; -} +const std::string &FileName::str() const +{ + return m_name; +} String FileName::toString() const { if(!m_wname.empty()) { return String(m_wname); - } + } else if(!m_name.empty()) { const int len = MultiByteToWideChar(CP_ACP, 0, m_name.c_str(), -1, NULL, 0); if(len == 0) - return String::null; + return String(); std::vector buf(len); MultiByteToWideChar(CP_ACP, 0, m_name.c_str(), -1, &buf[0], len); @@ -129,7 +129,7 @@ String FileName::toString() const return String(&buf[0]); } else { - return String::null; + return String(); } } diff --git a/taglib/toolkit/tpropertymap.cpp b/taglib/toolkit/tpropertymap.cpp index 2c696417..1c6a47d9 100644 --- a/taglib/toolkit/tpropertymap.cpp +++ b/taglib/toolkit/tpropertymap.cpp @@ -144,7 +144,8 @@ bool PropertyMap::operator!=(const PropertyMap &other) const String PropertyMap::toString() const { - String ret = ""; + String ret; + for(ConstIterator it = begin(); it != end(); ++it) ret += it->first+"="+it->second.toString(", ") + "\n"; if(!unsupported.isEmpty()) diff --git a/taglib/xm/xmfile.cpp b/taglib/xm/xmfile.cpp index 3625d213..a0bb5353 100644 --- a/taglib/xm/xmfile.cpp +++ b/taglib/xm/xmfile.cpp @@ -449,7 +449,7 @@ bool XM::File::save() seek(pos + 4); const uint len = std::min(22UL, instrumentHeaderSize - 4U); if(i >= lines.size()) - writeString(String::null, len); + writeString(String(), len); else writeString(lines[i], len); @@ -480,7 +480,7 @@ bool XM::File::save() seek(pos + 18); const uint len = std::min(sampleHeaderSize - 18U, 22UL); if(sampleNameIndex >= lines.size()) - writeString(String::null, len); + writeString(String(), len); else writeString(lines[sampleNameIndex ++], len); } diff --git a/tests/test_apetag.cpp b/tests/test_apetag.cpp index 2f027958..42475766 100644 --- a/tests/test_apetag.cpp +++ b/tests/test_apetag.cpp @@ -108,7 +108,7 @@ public: ByteVector data("Test Data"); item.setBinaryData(data); CPPUNIT_ASSERT(item.values().isEmpty()); - CPPUNIT_ASSERT_EQUAL(String::null, item.toString()); + CPPUNIT_ASSERT_EQUAL(String(), item.toString()); CPPUNIT_ASSERT_EQUAL(data, item.binaryData()); item.setValue("Test Text 2"); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 612dc424..c6592bbe 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -36,7 +36,7 @@ class PublicFrame : public ID3v2::Frame String readStringField(const ByteVector &data, String::Type encoding, int *positon = 0) { return ID3v2::Frame::readStringField(data, encoding, positon); } - virtual String toString() const { return String::null; } + virtual String toString() const { return String(); } virtual void parseFields(const ByteVector &) {} virtual ByteVector renderFields() const { return ByteVector(); } }; diff --git a/tests/test_it.cpp b/tests/test_it.cpp index be5680ec..3270eb25 100644 --- a/tests/test_it.cpp +++ b/tests/test_it.cpp @@ -123,10 +123,10 @@ private: CPPUNIT_ASSERT_EQUAL((TagLib::uchar)128, p->panningSeparation()); CPPUNIT_ASSERT_EQUAL((TagLib::uchar) 0, p->pitchWheelDepth()); CPPUNIT_ASSERT_EQUAL(title, t->title()); - CPPUNIT_ASSERT_EQUAL(String::null, t->artist()); - CPPUNIT_ASSERT_EQUAL(String::null, t->album()); + CPPUNIT_ASSERT_EQUAL(String(), t->artist()); + CPPUNIT_ASSERT_EQUAL(String(), t->album()); CPPUNIT_ASSERT_EQUAL(comment, t->comment()); - CPPUNIT_ASSERT_EQUAL(String::null, t->genre()); + CPPUNIT_ASSERT_EQUAL(String(), t->genre()); CPPUNIT_ASSERT_EQUAL(0U, t->year()); CPPUNIT_ASSERT_EQUAL(0U, t->track()); CPPUNIT_ASSERT_EQUAL(String("Impulse Tracker"), t->trackerName()); diff --git a/tests/test_mod.cpp b/tests/test_mod.cpp index 0a233c97..c62a870b 100644 --- a/tests/test_mod.cpp +++ b/tests/test_mod.cpp @@ -113,10 +113,10 @@ private: CPPUNIT_ASSERT_EQUAL(31U, p->instrumentCount()); CPPUNIT_ASSERT_EQUAL((uchar)1, p->lengthInPatterns()); CPPUNIT_ASSERT_EQUAL(title, t->title()); - CPPUNIT_ASSERT_EQUAL(String::null, t->artist()); - CPPUNIT_ASSERT_EQUAL(String::null, t->album()); + CPPUNIT_ASSERT_EQUAL(String(), t->artist()); + CPPUNIT_ASSERT_EQUAL(String(), t->album()); CPPUNIT_ASSERT_EQUAL(comment, t->comment()); - CPPUNIT_ASSERT_EQUAL(String::null, t->genre()); + CPPUNIT_ASSERT_EQUAL(String(), t->genre()); CPPUNIT_ASSERT_EQUAL(0U, t->year()); CPPUNIT_ASSERT_EQUAL(0U, t->track()); CPPUNIT_ASSERT_EQUAL(String("StarTrekker"), t->trackerName()); diff --git a/tests/test_s3m.cpp b/tests/test_s3m.cpp index 24a4c6e4..997c5653 100644 --- a/tests/test_s3m.cpp +++ b/tests/test_s3m.cpp @@ -110,10 +110,10 @@ private: CPPUNIT_ASSERT_EQUAL((TagLib::uchar)125, p->tempo()); CPPUNIT_ASSERT_EQUAL((TagLib::uchar) 6, p->bpmSpeed()); CPPUNIT_ASSERT_EQUAL(title, t->title()); - CPPUNIT_ASSERT_EQUAL(String::null, t->artist()); - CPPUNIT_ASSERT_EQUAL(String::null, t->album()); + CPPUNIT_ASSERT_EQUAL(String(), t->artist()); + CPPUNIT_ASSERT_EQUAL(String(), t->album()); CPPUNIT_ASSERT_EQUAL(comment, t->comment()); - CPPUNIT_ASSERT_EQUAL(String::null, t->genre()); + CPPUNIT_ASSERT_EQUAL(String(), t->genre()); CPPUNIT_ASSERT_EQUAL(0U, t->year()); CPPUNIT_ASSERT_EQUAL(0U, t->track()); CPPUNIT_ASSERT_EQUAL(String("ScreamTracker III"), t->trackerName()); diff --git a/tests/test_string.cpp b/tests/test_string.cpp index 24816af9..27839618 100644 --- a/tests/test_string.cpp +++ b/tests/test_string.cpp @@ -69,7 +69,7 @@ public: s.clear(); CPPUNIT_ASSERT(s.isEmpty()); - CPPUNIT_ASSERT(!s.isNull()); + CPPUNIT_ASSERT(!s.isNull()); // deprecated, but still worth it to check. String unicode("José Carlos", String::UTF8); CPPUNIT_ASSERT(strcmp(unicode.toCString(), "Jos\xe9 Carlos") == 0); diff --git a/tests/test_xm.cpp b/tests/test_xm.cpp index 28086533..48e6ec2c 100644 --- a/tests/test_xm.cpp +++ b/tests/test_xm.cpp @@ -139,13 +139,13 @@ public: CPPUNIT_ASSERT_EQUAL((TagLib::ushort) 6, p->tempo()); CPPUNIT_ASSERT_EQUAL((TagLib::ushort)125, p->bpmSpeed()); CPPUNIT_ASSERT_EQUAL(titleBefore, t->title()); - CPPUNIT_ASSERT_EQUAL(String::null, t->artist()); - CPPUNIT_ASSERT_EQUAL(String::null, t->album()); - CPPUNIT_ASSERT_EQUAL(String::null, t->comment()); - CPPUNIT_ASSERT_EQUAL(String::null, t->genre()); + CPPUNIT_ASSERT_EQUAL(String(), t->artist()); + CPPUNIT_ASSERT_EQUAL(String(), t->album()); + CPPUNIT_ASSERT_EQUAL(String(), t->comment()); + CPPUNIT_ASSERT_EQUAL(String(), t->genre()); CPPUNIT_ASSERT_EQUAL(0U, t->year()); CPPUNIT_ASSERT_EQUAL(0U, t->track()); - CPPUNIT_ASSERT_EQUAL(String::null, t->trackerName()); + CPPUNIT_ASSERT_EQUAL(String(), t->trackerName()); } void testWriteTagsShort() @@ -185,10 +185,10 @@ private: CPPUNIT_ASSERT_EQUAL((TagLib::ushort) 6, p->tempo()); CPPUNIT_ASSERT_EQUAL((TagLib::ushort)125, p->bpmSpeed()); CPPUNIT_ASSERT_EQUAL(title, t->title()); - CPPUNIT_ASSERT_EQUAL(String::null, t->artist()); - CPPUNIT_ASSERT_EQUAL(String::null, t->album()); + CPPUNIT_ASSERT_EQUAL(String(), t->artist()); + CPPUNIT_ASSERT_EQUAL(String(), t->album()); CPPUNIT_ASSERT_EQUAL(comment, t->comment()); - CPPUNIT_ASSERT_EQUAL(String::null, t->genre()); + CPPUNIT_ASSERT_EQUAL(String(), t->genre()); CPPUNIT_ASSERT_EQUAL(0U, t->year()); CPPUNIT_ASSERT_EQUAL(0U, t->track()); CPPUNIT_ASSERT_EQUAL(trackerName, t->trackerName()); From ce1c03faa38aa38a5719afe9ff135a3151d4b6d4 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 20 Nov 2015 23:08:43 +0900 Subject: [PATCH 09/15] Update NEWS. --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 1b3751bd..84ebd89e 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ * Better handling of duplicate tags in WAV files. * Fixed crash when calling File::properties() after strip(). * Fixed possible file corruptions when saving ASF files. + * Fixed updating the comment field of Vorbis comments. * Marked ByteVector::null and ByteVector::isNull() deprecated. * Marked String::null and ByteVector::isNull() deprecated. * Many smaller bug fixes and performance improvements. From b592f78238830ea99030255c52aa9cc215666040 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sat, 21 Nov 2015 09:30:04 +0900 Subject: [PATCH 10/15] Unify common functions for finding tags. Several classes have exactly identical functions for finding tags. This also hides the functions from public headers. --- taglib/CMakeLists.txt | 1 + taglib/ape/apefile.cpp | 53 ++------------------ taglib/ape/apefile.h | 3 -- taglib/flac/flacfile.cpp | 32 ++---------- taglib/flac/flacfile.h | 2 - taglib/mpc/mpcfile.cpp | 52 ++------------------ taglib/mpc/mpcfile.h | 3 -- taglib/tagutils.cpp | 79 ++++++++++++++++++++++++++++++ taglib/tagutils.h | 49 ++++++++++++++++++ taglib/trueaudio/trueaudiofile.cpp | 32 ++---------- taglib/trueaudio/trueaudiofile.h | 2 - taglib/wavpack/wavpackfile.cpp | 37 ++------------ taglib/wavpack/wavpackfile.h | 2 - 13 files changed, 146 insertions(+), 201 deletions(-) create mode 100644 taglib/tagutils.cpp create mode 100644 taglib/tagutils.h diff --git a/taglib/CMakeLists.txt b/taglib/CMakeLists.txt index 31e2c49b..5a047bf1 100644 --- a/taglib/CMakeLists.txt +++ b/taglib/CMakeLists.txt @@ -334,6 +334,7 @@ set(tag_LIB_SRCS tagunion.cpp fileref.cpp audioproperties.cpp + tagutils.cpp ) add_library(tag ${tag_LIB_SRCS} ${tag_HDRS}) diff --git a/taglib/ape/apefile.cpp b/taglib/ape/apefile.cpp index 1349dfa3..caefe737 100644 --- a/taglib/ape/apefile.cpp +++ b/taglib/ape/apefile.cpp @@ -38,9 +38,9 @@ #include #include #include +#include #include "apefile.h" - #include "apetag.h" #include "apefooter.h" @@ -258,7 +258,7 @@ void APE::File::read(bool readProperties) { // Look for an ID3v2 tag - d->ID3v2Location = findID3v2(); + d->ID3v2Location = Utils::findID3v2(this); if(d->ID3v2Location >= 0) { seek(d->ID3v2Location); @@ -269,7 +269,7 @@ void APE::File::read(bool readProperties) // Look for an ID3v1 tag - d->ID3v1Location = findID3v1(); + d->ID3v1Location = Utils::findID3v1(this); if(d->ID3v1Location >= 0) { d->tag.set(ApeID3v1Index, new ID3v1::Tag(this, d->ID3v1Location)); @@ -278,7 +278,7 @@ void APE::File::read(bool readProperties) // Look for an APE tag - d->APELocation = findAPE(); + d->APELocation = Utils::findAPE(this, d->ID3v1Location); if(d->APELocation >= 0) { d->tag.set(ApeAPEIndex, new APE::Tag(this, d->APELocation)); @@ -314,48 +314,3 @@ void APE::File::read(bool readProperties) d->properties = new Properties(this, streamLength); } } - -long APE::File::findAPE() -{ - if(!isValid()) - return -1; - - if(d->hasID3v1) - seek(-160, End); - else - seek(-32, End); - - long p = tell(); - - if(readBlock(8) == APE::Tag::fileIdentifier()) - return p; - - return -1; -} - -long APE::File::findID3v1() -{ - if(!isValid()) - return -1; - - seek(-128, End); - long p = tell(); - - if(readBlock(3) == ID3v1::Tag::fileIdentifier()) - return p; - - return -1; -} - -long APE::File::findID3v2() -{ - if(!isValid()) - return -1; - - seek(0); - - if(readBlock(3) == ID3v2::Header::fileIdentifier()) - return 0; - - return -1; -} diff --git a/taglib/ape/apefile.h b/taglib/ape/apefile.h index 1d2e5c67..e638d865 100644 --- a/taglib/ape/apefile.h +++ b/taglib/ape/apefile.h @@ -219,9 +219,6 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties); - long findAPE(); - long findID3v1(); - long findID3v2(); class FilePrivate; FilePrivate *d; diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index dc8f4011..252907d0 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -362,7 +363,7 @@ void FLAC::File::read(bool readProperties) { // Look for an ID3v2 tag - d->ID3v2Location = findID3v2(); + d->ID3v2Location = Utils::findID3v2(this); if(d->ID3v2Location >= 0) { @@ -378,7 +379,7 @@ void FLAC::File::read(bool readProperties) // Look for an ID3v1 tag - d->ID3v1Location = findID3v1(); + d->ID3v1Location = Utils::findID3v1(this); if(d->ID3v1Location >= 0) { d->tag.set(FlacID3v1Index, new ID3v1::Tag(this, d->ID3v1Location)); @@ -539,30 +540,3 @@ void FLAC::File::scan() d->scanned = true; } - -long FLAC::File::findID3v1() -{ - if(!isValid()) - return -1; - - seek(-128, End); - long p = tell(); - - if(readBlock(3) == ID3v1::Tag::fileIdentifier()) - return p; - - return -1; -} - -long FLAC::File::findID3v2() -{ - if(!isValid()) - return -1; - - seek(0); - - if(readBlock(3) == ID3v2::Header::fileIdentifier()) - return 0; - - return -1; -} diff --git a/taglib/flac/flacfile.h b/taglib/flac/flacfile.h index 1c055d33..6cbcb5a0 100644 --- a/taglib/flac/flacfile.h +++ b/taglib/flac/flacfile.h @@ -295,8 +295,6 @@ namespace TagLib { void read(bool readProperties); void scan(); - long findID3v2(); - long findID3v1(); class FilePrivate; FilePrivate *d; diff --git a/taglib/mpc/mpcfile.cpp b/taglib/mpc/mpcfile.cpp index 6d8dae5a..1d798212 100644 --- a/taglib/mpc/mpcfile.cpp +++ b/taglib/mpc/mpcfile.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include "mpcfile.h" #include "id3v1tag.h" @@ -266,7 +267,7 @@ void MPC::File::read(bool readProperties) { // Look for an ID3v1 tag - d->ID3v1Location = findID3v1(); + d->ID3v1Location = Utils::findID3v1(this); if(d->ID3v1Location >= 0) { d->tag.set(MPCID3v1Index, new ID3v1::Tag(this, d->ID3v1Location)); @@ -275,7 +276,7 @@ void MPC::File::read(bool readProperties) // Look for an APE tag - d->APELocation = findAPE(); + d->APELocation = Utils::findAPE(this, d->ID3v1Location); if(d->APELocation >= 0) { d->tag.set(MPCAPEIndex, new APE::Tag(this, d->APELocation)); @@ -290,7 +291,7 @@ void MPC::File::read(bool readProperties) // Look for an ID3v2 tag - d->ID3v2Location = findID3v2(); + d->ID3v2Location = Utils::findID3v2(this); if(d->ID3v2Location >= 0) { seek(d->ID3v2Location); @@ -323,48 +324,3 @@ void MPC::File::read(bool readProperties) d->properties = new Properties(this, streamLength); } } - -long MPC::File::findAPE() -{ - if(!isValid()) - return -1; - - if(d->hasID3v1) - seek(-160, End); - else - seek(-32, End); - - long p = tell(); - - if(readBlock(8) == APE::Tag::fileIdentifier()) - return p; - - return -1; -} - -long MPC::File::findID3v1() -{ - if(!isValid()) - return -1; - - seek(-128, End); - long p = tell(); - - if(readBlock(3) == ID3v1::Tag::fileIdentifier()) - return p; - - return -1; -} - -long MPC::File::findID3v2() -{ - if(!isValid()) - return -1; - - seek(0); - - if(readBlock(3) == ID3v2::Header::fileIdentifier()) - return 0; - - return -1; -} diff --git a/taglib/mpc/mpcfile.h b/taglib/mpc/mpcfile.h index 0980a5cd..98e7480f 100644 --- a/taglib/mpc/mpcfile.h +++ b/taglib/mpc/mpcfile.h @@ -222,9 +222,6 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties); - long findAPE(); - long findID3v1(); - long findID3v2(); class FilePrivate; FilePrivate *d; diff --git a/taglib/tagutils.cpp b/taglib/tagutils.cpp new file mode 100644 index 00000000..dc047040 --- /dev/null +++ b/taglib/tagutils.cpp @@ -0,0 +1,79 @@ +/*************************************************************************** + copyright : (C) 2015 by Tsuda Kageyu + email : tsuda.kageyu@gmail.com + ***************************************************************************/ + +/*************************************************************************** + * This library is free software; you can redistribute it and/or modify * + * it under the terms of the GNU Lesser General Public License version * + * 2.1 as published by the Free Software Foundation. * + * * + * This library is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this library; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * + * 02110-1301 USA * + * * + * Alternatively, this file is available under the Mozilla Public * + * License Version 1.1. You may obtain a copy of the License at * + * http://www.mozilla.org/MPL/ * + ***************************************************************************/ + +#include + +#include "id3v1tag.h" +#include "id3v2header.h" +#include "apetag.h" + +#include "tagutils.h" + +using namespace TagLib; + +long Utils::findID3v1(File *file) +{ + if(!file->isValid()) + return -1; + + file->seek(-128, File::End); + const long p = file->tell(); + + if(file->readBlock(3) == ID3v1::Tag::fileIdentifier()) + return p; + + return -1; +} + +long Utils::findID3v2(File *file) +{ + if(!file->isValid()) + return -1; + + file->seek(0); + + if(file->readBlock(3) == ID3v2::Header::fileIdentifier()) + return 0; + + return -1; +} + +long Utils::findAPE(File *file, long id3v1Location) +{ + if(!file->isValid()) + return -1; + + if(id3v1Location >= 0) + file->seek(id3v1Location - 32, File::Beginning); + else + file->seek(-32, File::End); + + const long p = file->tell(); + + if(file->readBlock(8) == APE::Tag::fileIdentifier()) + return p; + + return -1; +} diff --git a/taglib/tagutils.h b/taglib/tagutils.h new file mode 100644 index 00000000..fb11d1e0 --- /dev/null +++ b/taglib/tagutils.h @@ -0,0 +1,49 @@ +/*************************************************************************** + copyright : (C) 2015 by Tsuda Kageyu + email : tsuda.kageyu@gmail.com + ***************************************************************************/ + +/*************************************************************************** + * This library is free software; you can redistribute it and/or modify * + * it under the terms of the GNU Lesser General Public License version * + * 2.1 as published by the Free Software Foundation. * + * * + * This library is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this library; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * + * 02110-1301 USA * + * * + * Alternatively, this file is available under the Mozilla Public * + * License Version 1.1. You may obtain a copy of the License at * + * http://www.mozilla.org/MPL/ * + ***************************************************************************/ + +#ifndef TAGLIB_TAGUTILS_H +#define TAGLIB_TAGUTILS_H + +// THIS FILE IS NOT A PART OF THE TAGLIB API + +#ifndef DO_NOT_DOCUMENT // tell Doxygen not to document this header + +namespace TagLib { + + class File; + + namespace Utils { + + long findID3v1(File *file); + + long findID3v2(File *file); + + long findAPE(File *file, long id3v1Location); + } +} + +#endif + +#endif diff --git a/taglib/trueaudio/trueaudiofile.cpp b/taglib/trueaudio/trueaudiofile.cpp index 4ca60915..0ff3fa5e 100644 --- a/taglib/trueaudio/trueaudiofile.cpp +++ b/taglib/trueaudio/trueaudiofile.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include "trueaudiofile.h" #include "id3v1tag.h" @@ -248,7 +249,7 @@ void TrueAudio::File::read(bool readProperties) { // Look for an ID3v2 tag - d->ID3v2Location = findID3v2(); + d->ID3v2Location = Utils::findID3v2(this); if(d->ID3v2Location >= 0) { @@ -264,7 +265,7 @@ void TrueAudio::File::read(bool readProperties) // Look for an ID3v1 tag - d->ID3v1Location = findID3v1(); + d->ID3v1Location = Utils::findID3v1(this); if(d->ID3v1Location >= 0) { d->tag.set(TrueAudioID3v1Index, new ID3v1::Tag(this, d->ID3v1Location)); @@ -296,30 +297,3 @@ void TrueAudio::File::read(bool readProperties) d->properties = new Properties(readBlock(TrueAudio::HeaderSize), streamLength); } } - -long TrueAudio::File::findID3v1() -{ - if(!isValid()) - return -1; - - seek(-128, End); - long p = tell(); - - if(readBlock(3) == ID3v1::Tag::fileIdentifier()) - return p; - - return -1; -} - -long TrueAudio::File::findID3v2() -{ - if(!isValid()) - return -1; - - seek(0); - - if(readBlock(3) == ID3v2::Header::fileIdentifier()) - return 0; - - return -1; -} diff --git a/taglib/trueaudio/trueaudiofile.h b/taglib/trueaudio/trueaudiofile.h index 3fc515f6..4bcb722a 100644 --- a/taglib/trueaudio/trueaudiofile.h +++ b/taglib/trueaudio/trueaudiofile.h @@ -240,8 +240,6 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties); - long findID3v1(); - long findID3v2(); class FilePrivate; FilePrivate *d; diff --git a/taglib/wavpack/wavpackfile.cpp b/taglib/wavpack/wavpackfile.cpp index 90f79d16..7273e103 100644 --- a/taglib/wavpack/wavpackfile.cpp +++ b/taglib/wavpack/wavpackfile.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include "wavpackfile.h" #include "id3v1tag.h" @@ -243,7 +244,7 @@ void WavPack::File::read(bool readProperties) { // Look for an ID3v1 tag - d->ID3v1Location = findID3v1(); + d->ID3v1Location = Utils::findID3v1(this); if(d->ID3v1Location >= 0) { d->tag.set(WavID3v1Index, new ID3v1::Tag(this, d->ID3v1Location)); @@ -252,7 +253,7 @@ void WavPack::File::read(bool readProperties) // Look for an APE tag - d->APELocation = findAPE(); + d->APELocation = Utils::findAPE(this, d->ID3v1Location); if(d->APELocation >= 0) { d->tag.set(WavAPEIndex, new APE::Tag(this, d->APELocation)); @@ -280,35 +281,3 @@ void WavPack::File::read(bool readProperties) d->properties = new Properties(this, streamLength); } } - -long WavPack::File::findAPE() -{ - if(!isValid()) - return -1; - - if(d->hasID3v1) - seek(-160, End); - else - seek(-32, End); - - long p = tell(); - - if(readBlock(8) == APE::Tag::fileIdentifier()) - return p; - - return -1; -} - -long WavPack::File::findID3v1() -{ - if(!isValid()) - return -1; - - seek(-128, End); - long p = tell(); - - if(readBlock(3) == ID3v1::Tag::fileIdentifier()) - return p; - - return -1; -} diff --git a/taglib/wavpack/wavpackfile.h b/taglib/wavpack/wavpackfile.h index 24511581..abcabd94 100644 --- a/taglib/wavpack/wavpackfile.h +++ b/taglib/wavpack/wavpackfile.h @@ -208,8 +208,6 @@ namespace TagLib { File &operator=(const File &); void read(bool readProperties); - long findID3v1(); - long findAPE(); class FilePrivate; FilePrivate *d; From de0fc83066dedba0366f1b9689e452891be11af7 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sat, 21 Nov 2015 10:29:37 +0900 Subject: [PATCH 11/15] Style fixes in fileref.cpp. --- taglib/fileref.cpp | 222 ++++++++++++++++++++++++--------------------- taglib/fileref.h | 5 + 2 files changed, 124 insertions(+), 103 deletions(-) diff --git a/taglib/fileref.cpp b/taglib/fileref.cpp index c2cd31d1..e5272f3b 100644 --- a/taglib/fileref.cpp +++ b/taglib/fileref.cpp @@ -30,7 +30,7 @@ #include #include #include -#include "trefcounter.h" +#include #include "fileref.h" #include "asffile.h" @@ -54,14 +54,106 @@ using namespace TagLib; -template -static File* createInternal(T arg, const String& filename, bool readAudioProperties, - AudioProperties::ReadStyle audioPropertiesStyle); +namespace +{ + // Templatized internal functions. T should be String or IOStream*. + + template + inline FileName toFileName(T arg) + { + // Should never be called. + } + + template <> + inline FileName toFileName(IOStream *arg) + { + return arg->name(); + } + + template <> + inline FileName toFileName(FileName arg) + { + return arg; + } + + template + File* createInternal(T arg, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) + { + // Ok, this is really dumb for now, but it works for testing. + + const FileName fileName = toFileName(arg); +#ifdef _WIN32 + const String s = fileName.toString(); +#else + const String s(fileName); +#endif + + String ext; + const int pos = s.rfind("."); + if(pos != -1) + ext = s.substr(pos + 1).upper(); + + // If this list is updated, the method defaultFileExtensions() should also be + // updated. However at some point that list should be created at the same time + // that a default file type resolver is created. + + if(!ext.isEmpty()) { + if(ext == "MP3") + return new MPEG::File(arg, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); + if(ext == "OGG") + return new Ogg::Vorbis::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "OGA") { + /* .oga can be any audio in the Ogg container. First try FLAC, then Vorbis. */ + File *file = new Ogg::FLAC::File(arg, readAudioProperties, audioPropertiesStyle); + if(file->isValid()) + return file; + delete file; + return new Ogg::Vorbis::File(arg, readAudioProperties, audioPropertiesStyle); + } + if(ext == "FLAC") + return new FLAC::File(arg, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); + if(ext == "MPC") + return new MPC::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "WV") + return new WavPack::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "SPX") + return new Ogg::Speex::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "OPUS") + return new Ogg::Opus::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "TTA") + return new TrueAudio::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "M4A" || ext == "M4R" || ext == "M4B" || ext == "M4P" || ext == "MP4" || ext == "3G2") + return new MP4::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "WMA" || ext == "ASF") + return new ASF::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "AIF" || ext == "AIFF" || ext == "AFC" || ext == "AIFC") + return new RIFF::AIFF::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "WAV") + return new RIFF::WAV::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "APE") + return new APE::File(arg, readAudioProperties, audioPropertiesStyle); + // module, nst and wow are possible but uncommon extensions + if(ext == "MOD" || ext == "MODULE" || ext == "NST" || ext == "WOW") + return new Mod::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "S3M") + return new S3M::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "IT") + return new IT::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "XM") + return new XM::File(arg, readAudioProperties, audioPropertiesStyle); + } + + return 0; + } +} class FileRef::FileRefPrivate : public RefCounter { public: - FileRefPrivate(File *f) : RefCounter(), file(f) {} + FileRefPrivate(File *f) : + RefCounter(), + file(f) {} + ~FileRefPrivate() { delete file; } @@ -76,34 +168,29 @@ List FileRef::FileRefPrivate::fileTypeResolve // public members //////////////////////////////////////////////////////////////////////////////// -FileRef::FileRef() +FileRef::FileRef() : + d(new FileRefPrivate(0)) { - d = new FileRefPrivate(0); } FileRef::FileRef(FileName fileName, bool readAudioProperties, - AudioProperties::ReadStyle audioPropertiesStyle) + AudioProperties::ReadStyle audioPropertiesStyle) : + d(new FileRefPrivate(createInternal(fileName, readAudioProperties, audioPropertiesStyle))) { - d = new FileRefPrivate(create(fileName, readAudioProperties, audioPropertiesStyle)); } -FileRef::FileRef(IOStream* stream, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) +FileRef::FileRef(IOStream* stream, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) : + d(new FileRefPrivate(createInternal(stream, readAudioProperties, audioPropertiesStyle))) { - d = new FileRefPrivate(createInternal(stream, -#ifdef _WIN32 - stream->name().toString() -#else - stream->name() -#endif - , readAudioProperties, audioPropertiesStyle)); } -FileRef::FileRef(File *file) +FileRef::FileRef(File *file) : + d(new FileRefPrivate(file)) { - d = new FileRefPrivate(file); } -FileRef::FileRef(const FileRef &ref) : d(ref.d) +FileRef::FileRef(const FileRef &ref) : + d(ref.d) { d->ref(); } @@ -189,31 +276,30 @@ StringList FileRef::defaultFileExtensions() bool FileRef::isNull() const { - return !d->file || !d->file->isValid(); + return (!d->file || !d->file->isValid()); } FileRef &FileRef::operator=(const FileRef &ref) { - if(&ref == this) - return *this; - - if(d->deref()) - delete d; - - d = ref.d; - d->ref(); - + FileRef(ref).swap(*this); return *this; } +void FileRef::swap(FileRef &ref) +{ + using std::swap; + + swap(d, ref.d); +} + bool FileRef::operator==(const FileRef &ref) const { - return ref.d->file == d->file; + return (ref.d->file == d->file); } bool FileRef::operator!=(const FileRef &ref) const { - return ref.d->file != d->file; + return (ref.d->file != d->file); } File *FileRef::create(FileName fileName, bool readAudioProperties, @@ -228,75 +314,5 @@ File *FileRef::create(FileName fileName, bool readAudioProperties, return file; } - return createInternal(fileName, -#ifdef _WIN32 - fileName.toString() -#else - fileName -#endif - , readAudioProperties, audioPropertiesStyle); -} - -template -static File* createInternal(T fileName, const String& s, bool readAudioProperties, - AudioProperties::ReadStyle audioPropertiesStyle) -{ - // Ok, this is really dumb for now, but it works for testing. - - String ext; - const int pos = s.rfind("."); - if(pos != -1) - ext = s.substr(pos + 1).upper(); - - // If this list is updated, the method defaultFileExtensions() should also be - // updated. However at some point that list should be created at the same time - // that a default file type resolver is created. - - if(!ext.isEmpty()) { - if(ext == "MP3") - return new MPEG::File(fileName, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); - if(ext == "OGG") - return new Ogg::Vorbis::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "OGA") { - /* .oga can be any audio in the Ogg container. First try FLAC, then Vorbis. */ - File *file = new Ogg::FLAC::File(fileName, readAudioProperties, audioPropertiesStyle); - if (file->isValid()) - return file; - delete file; - return new Ogg::Vorbis::File(fileName, readAudioProperties, audioPropertiesStyle); - } - if(ext == "FLAC") - return new FLAC::File(fileName, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); - if(ext == "MPC") - return new MPC::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "WV") - return new WavPack::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "SPX") - return new Ogg::Speex::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "OPUS") - return new Ogg::Opus::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "TTA") - return new TrueAudio::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "M4A" || ext == "M4R" || ext == "M4B" || ext == "M4P" || ext == "MP4" || ext == "3G2") - return new MP4::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "WMA" || ext == "ASF") - return new ASF::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "AIF" || ext == "AIFF" || ext == "AFC" || ext == "AIFC") - return new RIFF::AIFF::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "WAV") - return new RIFF::WAV::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "APE") - return new APE::File(fileName, readAudioProperties, audioPropertiesStyle); - // module, nst and wow are possible but uncommon extensions - if(ext == "MOD" || ext == "MODULE" || ext == "NST" || ext == "WOW") - return new Mod::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "S3M") - return new S3M::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "IT") - return new IT::File(fileName, readAudioProperties, audioPropertiesStyle); - if(ext == "XM") - return new XM::File(fileName, readAudioProperties, audioPropertiesStyle); - } - - return 0; + return createInternal(fileName, readAudioProperties, audioPropertiesStyle); } diff --git a/taglib/fileref.h b/taglib/fileref.h index abeb0af9..4c431180 100644 --- a/taglib/fileref.h +++ b/taglib/fileref.h @@ -240,6 +240,11 @@ namespace TagLib { */ FileRef &operator=(const FileRef &ref); + /*! + * Exchanges the content of the FileRef by the content of \a ref. + */ + void swap(FileRef &ref); + /*! * Returns true if this FileRef and \a ref point to the same File object. */ From 61dabe61a743fce2ca6eaa5343b71901e150135b Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sat, 21 Nov 2015 10:30:48 +0900 Subject: [PATCH 12/15] Update NEWS. --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 84ebd89e..24d4525c 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ ========================== + * New API for creating FileRef from IOStream. * Added support for ID3v2 PCST and WFED frames. * Added String::clear(). * Better handling of duplicate ID3v2 tags in all kinds of files. From 8724184d5693624cf1097f4d3b8dd83aa3b490c8 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sat, 21 Nov 2015 10:32:32 +0900 Subject: [PATCH 13/15] Fix a typo in a comment. --- taglib/fileref.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/fileref.h b/taglib/fileref.h index 4c431180..387b2f51 100644 --- a/taglib/fileref.h +++ b/taglib/fileref.h @@ -142,7 +142,7 @@ namespace TagLib { audioPropertiesStyle = AudioProperties::Average); /*! - * Contruct a FileRef using \a file. The FileRef now takes ownership of the + * Construct a FileRef using \a file. The FileRef now takes ownership of the * pointer and will delete the File when it passes out of scope. */ explicit FileRef(File *file); From ef3ce1e38a47853fd4ce96b9e33455318ae5583f Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sat, 21 Nov 2015 17:06:26 +0900 Subject: [PATCH 14/15] Style fixes in fileref.cpp. --- taglib/fileref.cpp | 107 ++++++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 55 deletions(-) diff --git a/taglib/fileref.cpp b/taglib/fileref.cpp index e5272f3b..0be65603 100644 --- a/taglib/fileref.cpp +++ b/taglib/fileref.cpp @@ -56,6 +56,8 @@ using namespace TagLib; namespace { + List fileTypeResolvers; + // Templatized internal functions. T should be String or IOStream*. template @@ -79,13 +81,10 @@ namespace template File* createInternal(T arg, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) { - // Ok, this is really dumb for now, but it works for testing. - - const FileName fileName = toFileName(arg); #ifdef _WIN32 - const String s = fileName.toString(); + const String s = toFileName(arg).toString(); #else - const String s(fileName); + const String s(toFileName(arg)); #endif String ext; @@ -97,51 +96,52 @@ namespace // updated. However at some point that list should be created at the same time // that a default file type resolver is created. - if(!ext.isEmpty()) { - if(ext == "MP3") - return new MPEG::File(arg, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); - if(ext == "OGG") - return new Ogg::Vorbis::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "OGA") { - /* .oga can be any audio in the Ogg container. First try FLAC, then Vorbis. */ - File *file = new Ogg::FLAC::File(arg, readAudioProperties, audioPropertiesStyle); - if(file->isValid()) - return file; - delete file; - return new Ogg::Vorbis::File(arg, readAudioProperties, audioPropertiesStyle); - } - if(ext == "FLAC") - return new FLAC::File(arg, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); - if(ext == "MPC") - return new MPC::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "WV") - return new WavPack::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "SPX") - return new Ogg::Speex::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "OPUS") - return new Ogg::Opus::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "TTA") - return new TrueAudio::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "M4A" || ext == "M4R" || ext == "M4B" || ext == "M4P" || ext == "MP4" || ext == "3G2") - return new MP4::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "WMA" || ext == "ASF") - return new ASF::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "AIF" || ext == "AIFF" || ext == "AFC" || ext == "AIFC") - return new RIFF::AIFF::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "WAV") - return new RIFF::WAV::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "APE") - return new APE::File(arg, readAudioProperties, audioPropertiesStyle); - // module, nst and wow are possible but uncommon extensions - if(ext == "MOD" || ext == "MODULE" || ext == "NST" || ext == "WOW") - return new Mod::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "S3M") - return new S3M::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "IT") - return new IT::File(arg, readAudioProperties, audioPropertiesStyle); - if(ext == "XM") - return new XM::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext.isEmpty()) + return 0; + + if(ext == "MP3") + return new MPEG::File(arg, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); + if(ext == "OGG") + return new Ogg::Vorbis::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "OGA") { + /* .oga can be any audio in the Ogg container. First try FLAC, then Vorbis. */ + File *file = new Ogg::FLAC::File(arg, readAudioProperties, audioPropertiesStyle); + if(file->isValid()) + return file; + delete file; + return new Ogg::Vorbis::File(arg, readAudioProperties, audioPropertiesStyle); } + if(ext == "FLAC") + return new FLAC::File(arg, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle); + if(ext == "MPC") + return new MPC::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "WV") + return new WavPack::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "SPX") + return new Ogg::Speex::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "OPUS") + return new Ogg::Opus::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "TTA") + return new TrueAudio::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "M4A" || ext == "M4R" || ext == "M4B" || ext == "M4P" || ext == "MP4" || ext == "3G2") + return new MP4::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "WMA" || ext == "ASF") + return new ASF::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "AIF" || ext == "AIFF" || ext == "AFC" || ext == "AIFC") + return new RIFF::AIFF::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "WAV") + return new RIFF::WAV::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "APE") + return new APE::File(arg, readAudioProperties, audioPropertiesStyle); + // module, nst and wow are possible but uncommon extensions + if(ext == "MOD" || ext == "MODULE" || ext == "NST" || ext == "WOW") + return new Mod::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "S3M") + return new S3M::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "IT") + return new IT::File(arg, readAudioProperties, audioPropertiesStyle); + if(ext == "XM") + return new XM::File(arg, readAudioProperties, audioPropertiesStyle); return 0; } @@ -159,11 +159,8 @@ public: } File *file; - static List fileTypeResolvers; }; -List FileRef::FileRefPrivate::fileTypeResolvers; - //////////////////////////////////////////////////////////////////////////////// // public members //////////////////////////////////////////////////////////////////////////////// @@ -235,7 +232,7 @@ bool FileRef::save() const FileRef::FileTypeResolver *FileRef::addFileTypeResolver(const FileRef::FileTypeResolver *resolver) // static { - FileRefPrivate::fileTypeResolvers.prepend(resolver); + fileTypeResolvers.prepend(resolver); return resolver; } @@ -306,9 +303,9 @@ File *FileRef::create(FileName fileName, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) // static { - List::ConstIterator it = FileRefPrivate::fileTypeResolvers.begin(); + List::ConstIterator it = fileTypeResolvers.begin(); - for(; it != FileRefPrivate::fileTypeResolvers.end(); ++it) { + for(; it != fileTypeResolvers.end(); ++it) { File *file = (*it)->createFile(fileName, readAudioProperties, audioPropertiesStyle); if(file) return file; From f34d73d319ceb4d9baccf8b626dcd185f4a631e0 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sat, 21 Nov 2015 18:29:41 +0900 Subject: [PATCH 15/15] Make FileRef::FileTypeResolver work properly. --- taglib/fileref.cpp | 47 ++++++++++++++++++++++++++++++++---------- tests/test_fileref.cpp | 31 +++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/taglib/fileref.cpp b/taglib/fileref.cpp index 0be65603..7cf78803 100644 --- a/taglib/fileref.cpp +++ b/taglib/fileref.cpp @@ -56,7 +56,8 @@ using namespace TagLib; namespace { - List fileTypeResolvers; + typedef List ResolverList; + ResolverList fileTypeResolvers; // Templatized internal functions. T should be String or IOStream*. @@ -79,8 +80,41 @@ namespace } template - File* createInternal(T arg, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) + inline File *resolveFileType(T arg, bool readProperties, + AudioProperties::ReadStyle style) { + // Should never be called. + } + + template <> + inline File *resolveFileType(IOStream *arg, bool readProperties, + AudioProperties::ReadStyle style) + { + return 0; + } + + template <> + inline File *resolveFileType(FileName arg, bool readProperties, + AudioProperties::ReadStyle style) + { + ResolverList::ConstIterator it = fileTypeResolvers.begin(); + for(; it != fileTypeResolvers.end(); ++it) { + File *file = (*it)->createFile(arg, readProperties, style); + if(file) + return file; + } + + return 0; + } + + template + File* createInternal(T arg, bool readAudioProperties, + AudioProperties::ReadStyle audioPropertiesStyle) + { + File *file = resolveFileType(arg, readAudioProperties, audioPropertiesStyle); + if(file) + return file; + #ifdef _WIN32 const String s = toFileName(arg).toString(); #else @@ -302,14 +336,5 @@ bool FileRef::operator!=(const FileRef &ref) const File *FileRef::create(FileName fileName, bool readAudioProperties, AudioProperties::ReadStyle audioPropertiesStyle) // static { - - List::ConstIterator it = fileTypeResolvers.begin(); - - for(; it != fileTypeResolvers.end(); ++it) { - File *file = (*it)->createFile(fileName, readAudioProperties, audioPropertiesStyle); - if(file) - return file; - } - return createInternal(fileName, readAudioProperties, audioPropertiesStyle); } diff --git a/tests/test_fileref.cpp b/tests/test_fileref.cpp index a51b868c..a389cfd3 100644 --- a/tests/test_fileref.cpp +++ b/tests/test_fileref.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "utils.h" #include @@ -11,6 +12,18 @@ using namespace std; using namespace TagLib; +namespace +{ + class DummyResolver : public FileRef::FileTypeResolver + { + public: + virtual File *createFile(FileName fileName, bool, AudioProperties::ReadStyle) const + { + return new Ogg::Vorbis::File(fileName); + } + }; +} + class TestFileRef : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TestFileRef); @@ -29,6 +42,7 @@ class TestFileRef : public CppUnit::TestFixture CPPUNIT_TEST(testAPE); CPPUNIT_TEST(testWav); CPPUNIT_TEST(testUnsupported); + CPPUNIT_TEST(testFileResolver); CPPUNIT_TEST_SUITE_END(); public: @@ -168,10 +182,25 @@ public: { FileRef f1(TEST_FILE_PATH_C("no-extension")); CPPUNIT_ASSERT(f1.isNull()); - + FileRef f2(TEST_FILE_PATH_C("unsupported-extension.xxx")); CPPUNIT_ASSERT(f2.isNull()); } + + void testFileResolver() + { + FileRef *f = new FileRef(TEST_FILE_PATH_C("xing.mp3")); + CPPUNIT_ASSERT(dynamic_cast(f->file()) != NULL); + delete f; + + DummyResolver resolver; + FileRef::addFileTypeResolver(&resolver); + + f = new FileRef(TEST_FILE_PATH_C("xing.mp3")); + CPPUNIT_ASSERT(dynamic_cast(f->file()) != NULL); + delete f; + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestFileRef);