diff --git a/NEWS b/NEWS index 1c0d8383..baf214b2 100644 --- a/NEWS +++ b/NEWS @@ -5,7 +5,8 @@ * Added String::clear(). * Added alternative functions to XiphComment::removeField(). * Better handling of duplicate ID3v2 tags in all kinds of files. - * Better handling of duplicate tags in WAV files. + * Better handling of duplicate tag chunks in WAV files. + * Better handling of duplicate tag chunks in AIFF 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. diff --git a/taglib/ape/apefooter.cpp b/taglib/ape/apefooter.cpp index cb27c440..417afef8 100644 --- a/taglib/ape/apefooter.cpp +++ b/taglib/ape/apefooter.cpp @@ -38,14 +38,13 @@ using namespace APE; class APE::Footer::FooterPrivate { public: - FooterPrivate() : version(0), - footerPresent(true), - headerPresent(false), - isHeader(false), - itemCount(0), - tagSize(0) {} - - ~FooterPrivate() {} + FooterPrivate() : + version(0), + footerPresent(true), + headerPresent(false), + isHeader(false), + itemCount(0), + tagSize(0) {} uint version; @@ -56,8 +55,6 @@ public: uint itemCount; uint tagSize; - - static const uint size = 32; }; //////////////////////////////////////////////////////////////////////////////// @@ -66,26 +63,26 @@ public: TagLib::uint APE::Footer::size() { - return FooterPrivate::size; + return 32; } ByteVector APE::Footer::fileIdentifier() { - return ByteVector::fromCString("APETAGEX"); + return ByteVector("APETAGEX"); } //////////////////////////////////////////////////////////////////////////////// // public members //////////////////////////////////////////////////////////////////////////////// -APE::Footer::Footer() +APE::Footer::Footer() : + d(new FooterPrivate()) { - d = new FooterPrivate; } -APE::Footer::Footer(const ByteVector &data) +APE::Footer::Footer(const ByteVector &data) : + d(new FooterPrivate()) { - d = new FooterPrivate; parse(data); } @@ -137,7 +134,7 @@ TagLib::uint APE::Footer::tagSize() const TagLib::uint APE::Footer::completeTagSize() const { if(d->headerPresent) - return d->tagSize + d->size; + return d->tagSize + size(); else return d->tagSize; } @@ -154,13 +151,14 @@ void APE::Footer::setData(const ByteVector &data) ByteVector APE::Footer::renderFooter() const { - return render(false); + return render(false); } ByteVector APE::Footer::renderHeader() const { - if (!d->headerPresent) return ByteVector(); - + if(!d->headerPresent) + return ByteVector(); + else return render(true); } diff --git a/taglib/ape/apeitem.cpp b/taglib/ape/apeitem.cpp index 51b1fccd..d4d22de2 100644 --- a/taglib/ape/apeitem.cpp +++ b/taglib/ape/apeitem.cpp @@ -43,28 +43,32 @@ public: bool readOnly; }; -APE::Item::Item() +//////////////////////////////////////////////////////////////////////////////// +// public members +//////////////////////////////////////////////////////////////////////////////// + +APE::Item::Item() : + d(new ItemPrivate()) { - d = new ItemPrivate; } -APE::Item::Item(const String &key, const String &value) +APE::Item::Item(const String &key, const String &value) : + d(new ItemPrivate()) { - d = new ItemPrivate; d->key = key; d->text.append(value); } -APE::Item::Item(const String &key, const StringList &values) +APE::Item::Item(const String &key, const StringList &values) : + d(new ItemPrivate()) { - d = new ItemPrivate; d->key = key; d->text = values; } -APE::Item::Item(const String &key, const ByteVector &value, bool binary) +APE::Item::Item(const String &key, const ByteVector &value, bool binary) : + d(new ItemPrivate()) { - d = new ItemPrivate; d->key = key; if(binary) { d->type = Binary; @@ -74,9 +78,9 @@ APE::Item::Item(const String &key, const ByteVector &value, bool binary) d->text.append(value); } -APE::Item::Item(const Item &item) +APE::Item::Item(const Item &item) : + d(new ItemPrivate(*item.d)) { - d = new ItemPrivate(*item.d); } APE::Item::~Item() @@ -86,13 +90,17 @@ APE::Item::~Item() Item &APE::Item::operator=(const Item &item) { - if(&item != this) { - delete d; - d = new ItemPrivate(*item.d); - } + Item(item).swap(*this); return *this; } +void APE::Item::swap(Item &item) +{ + using std::swap; + + swap(d, item.d); +} + void APE::Item::setReadOnly(bool readOnly) { d->readOnly = readOnly; diff --git a/taglib/ape/apeitem.h b/taglib/ape/apeitem.h index d75692ff..c8ea0ba6 100644 --- a/taglib/ape/apeitem.h +++ b/taglib/ape/apeitem.h @@ -89,6 +89,11 @@ namespace TagLib { */ Item &operator=(const Item &item); + /*! + * Exchanges the content of this item by the content of \a item. + */ + void swap(Item &item); + /*! * Returns the key. */ diff --git a/taglib/asf/asfattribute.cpp b/taglib/asf/asfattribute.cpp index dfceb1ef..2d9591a9 100644 --- a/taglib/asf/asfattribute.cpp +++ b/taglib/asf/asfattribute.cpp @@ -70,61 +70,61 @@ public: // public members //////////////////////////////////////////////////////////////////////////////// -ASF::Attribute::Attribute() - : d(new AttributePrivate()) +ASF::Attribute::Attribute() : + d(new AttributePrivate()) { d->data->type = UnicodeType; } -ASF::Attribute::Attribute(const ASF::Attribute &other) - : d(new AttributePrivate(*other.d)) +ASF::Attribute::Attribute(const ASF::Attribute &other) : + d(new AttributePrivate(*other.d)) { } -ASF::Attribute::Attribute(const String &value) - : d(new AttributePrivate()) +ASF::Attribute::Attribute(const String &value) : + d(new AttributePrivate()) { d->data->type = UnicodeType; d->data->stringValue = value; } -ASF::Attribute::Attribute(const ByteVector &value) - : d(new AttributePrivate()) +ASF::Attribute::Attribute(const ByteVector &value) : + d(new AttributePrivate()) { d->data->type = BytesType; d->data->byteVectorValue = value; } -ASF::Attribute::Attribute(const ASF::Picture &value) - : d(new AttributePrivate()) +ASF::Attribute::Attribute(const ASF::Picture &value) : + d(new AttributePrivate()) { d->data->type = BytesType; d->data->pictureValue = value; } -ASF::Attribute::Attribute(unsigned int value) - : d(new AttributePrivate()) +ASF::Attribute::Attribute(unsigned int value) : + d(new AttributePrivate()) { d->data->type = DWordType; d->data->intValue = value; } -ASF::Attribute::Attribute(unsigned long long value) - : d(new AttributePrivate()) +ASF::Attribute::Attribute(unsigned long long value) : + d(new AttributePrivate()) { d->data->type = QWordType; d->data->longLongValue = value; } -ASF::Attribute::Attribute(unsigned short value) - : d(new AttributePrivate()) +ASF::Attribute::Attribute(unsigned short value) : + d(new AttributePrivate()) { d->data->type = WordType; d->data->shortValue = value; } -ASF::Attribute::Attribute(bool value) - : d(new AttributePrivate()) +ASF::Attribute::Attribute(bool value) : + d(new AttributePrivate()) { d->data->type = BoolType; d->data->boolValue = value; @@ -137,10 +137,17 @@ ASF::Attribute::~Attribute() ASF::Attribute &ASF::Attribute::operator=(const ASF::Attribute &other) { - *d = *other.d; + Attribute(other).swap(*this); return *this; } +void ASF::Attribute::swap(Attribute &other) +{ + using std::swap; + + swap(d, other.d); +} + ASF::Attribute::AttributeTypes ASF::Attribute::type() const { return d->data->type; @@ -357,4 +364,3 @@ void ASF::Attribute::setStream(int value) { d->data->stream = value; } - diff --git a/taglib/asf/asfattribute.h b/taglib/asf/asfattribute.h index 469490f6..df7485d5 100644 --- a/taglib/asf/asfattribute.h +++ b/taglib/asf/asfattribute.h @@ -114,6 +114,11 @@ namespace TagLib */ ASF::Attribute &operator=(const Attribute &other); + /*! + * Exchanges the content of the Attribute by the content of \a other. + */ + void swap(Attribute &other); + /*! * Destroys the attribute. */ diff --git a/taglib/asf/asfpicture.cpp b/taglib/asf/asfpicture.cpp index 4c48bb09..14b58f10 100644 --- a/taglib/asf/asfpicture.cpp +++ b/taglib/asf/asfpicture.cpp @@ -55,19 +55,18 @@ public: SHARED_PTR data; }; - //////////////////////////////////////////////////////////////////////////////// // Picture class members //////////////////////////////////////////////////////////////////////////////// -ASF::Picture::Picture() - : d(new PicturePrivate()) +ASF::Picture::Picture() : + d(new PicturePrivate()) { d->data->valid = true; } -ASF::Picture::Picture(const Picture& other) - : d(new PicturePrivate(*other.d)) +ASF::Picture::Picture(const Picture& other) : + d(new PicturePrivate(*other.d)) { } @@ -130,10 +129,17 @@ int ASF::Picture::dataSize() const ASF::Picture& ASF::Picture::operator=(const ASF::Picture& other) { - *d = *other.d; + Picture(other).swap(*this); return *this; } +void ASF::Picture::swap(Picture &other) +{ + using std::swap; + + swap(d, other.d); +} + ByteVector ASF::Picture::render() const { if(!isValid()) @@ -184,4 +190,3 @@ ASF::Picture ASF::Picture::fromInvalid() ret.d->data->valid = false; return ret; } - diff --git a/taglib/asf/asfpicture.h b/taglib/asf/asfpicture.h index 13866f2c..1e8466c5 100644 --- a/taglib/asf/asfpicture.h +++ b/taglib/asf/asfpicture.h @@ -119,6 +119,11 @@ namespace TagLib */ Picture& operator=(const Picture& other); + /*! + * Exchanges the content of the Picture by the content of \a other. + */ + void swap(Picture &other); + /*! * Returns true if Picture stores valid picture */ diff --git a/taglib/fileref.cpp b/taglib/fileref.cpp index eb3f5275..21e90be9 100644 --- a/taglib/fileref.cpp +++ b/taglib/fileref.cpp @@ -65,7 +65,8 @@ namespace template inline FileName toFileName(T arg) { - // Should never be called. + debug("FileRef::toFileName(): This version should never be called."); + return FileName(L""); } template <> @@ -84,7 +85,8 @@ namespace inline File *resolveFileType(T arg, bool readProperties, AudioProperties::ReadStyle style) { - // Should never be called. + debug("FileRef::resolveFileType(): This version should never be called."); + return 0; } template <> diff --git a/taglib/mp4/mp4coverart.cpp b/taglib/mp4/mp4coverart.cpp index fb9c2ed7..f837c079 100644 --- a/taglib/mp4/mp4coverart.cpp +++ b/taglib/mp4/mp4coverart.cpp @@ -42,8 +42,8 @@ namespace class MP4::CoverArt::CoverArtPrivate { public: - CoverArtPrivate(Format f, const ByteVector &v) - : data(new CoverArtData()) + CoverArtPrivate(Format f, const ByteVector &v) : + data(new CoverArtData()) { data->format = f; data->data = v; @@ -52,23 +52,35 @@ public: SHARED_PTR data; }; -MP4::CoverArt::CoverArt(Format format, const ByteVector &data) - : d(new CoverArtPrivate(format, data)) +//////////////////////////////////////////////////////////////////////////////// +// public members +//////////////////////////////////////////////////////////////////////////////// + +MP4::CoverArt::CoverArt(Format format, const ByteVector &data) : + d(new CoverArtPrivate(format, data)) { } -MP4::CoverArt::CoverArt(const CoverArt &item) - : d(new CoverArtPrivate(*item.d)) +MP4::CoverArt::CoverArt(const CoverArt &item) : + d(new CoverArtPrivate(*item.d)) { } MP4::CoverArt & MP4::CoverArt::operator=(const CoverArt &item) { - *d = *item.d; + CoverArt(item).swap(*this); return *this; } +void +MP4::CoverArt::swap(CoverArt &item) +{ + using std::swap; + + swap(d, item.d); +} + MP4::CoverArt::~CoverArt() { delete d; @@ -85,4 +97,3 @@ MP4::CoverArt::data() const { return d->data->data; } - diff --git a/taglib/mp4/mp4coverart.h b/taglib/mp4/mp4coverart.h index d87efe05..83f2ac59 100644 --- a/taglib/mp4/mp4coverart.h +++ b/taglib/mp4/mp4coverart.h @@ -53,8 +53,17 @@ namespace TagLib { ~CoverArt(); CoverArt(const CoverArt &item); + + /*! + * Copies the contents of \a item into this CoverArt. + */ CoverArt &operator=(const CoverArt &item); + /*! + * Exchanges the content of the CoverArt by the content of \a item. + */ + void swap(CoverArt &item); + //! Format of the image Format format() const; diff --git a/taglib/mp4/mp4item.cpp b/taglib/mp4/mp4item.cpp index db27d06e..dd072210 100644 --- a/taglib/mp4/mp4item.cpp +++ b/taglib/mp4/mp4item.cpp @@ -57,8 +57,8 @@ namespace class MP4::Item::ItemPrivate { public: - ItemPrivate() - : data(new ItemData()) + ItemPrivate() : + data(new ItemData()) { data->valid = true; data->atomDataType = MP4::TypeUndefined; @@ -68,89 +68,96 @@ public: SHARED_PTR data; }; -MP4::Item::Item() - : d(new ItemPrivate()) +MP4::Item::Item() : + d(new ItemPrivate()) { d->data->valid = false; } -MP4::Item::Item(const Item &item) - : d(new ItemPrivate(*item.d)) +MP4::Item::Item(const Item &item) : + d(new ItemPrivate(*item.d)) { } MP4::Item & - MP4::Item::operator=(const Item &item) +MP4::Item::operator=(const Item &item) { - *d = *item.d; + Item(item).swap(*this); return *this; } +void +MP4::Item::swap(Item &item) +{ + using std::swap; + + swap(d, item.d); +} MP4::Item::~Item() { delete d; } -MP4::Item::Item(bool value) - : d(new ItemPrivate()) +MP4::Item::Item(bool value) : + d(new ItemPrivate()) { d->data->m_bool = value; d->data->type = TypeBool; } -MP4::Item::Item(int value) - : d(new ItemPrivate()) +MP4::Item::Item(int value) : + d(new ItemPrivate()) { d->data->m_int = value; d->data->type = TypeInt; } -MP4::Item::Item(uchar value) - : d(new ItemPrivate()) +MP4::Item::Item(uchar value) : + d(new ItemPrivate()) { d->data->m_byte = value; d->data->type = TypeByte; } -MP4::Item::Item(uint value) - : d(new ItemPrivate()) +MP4::Item::Item(uint value) : + d(new ItemPrivate()) { d->data->m_uint = value; d->data->type = TypeUInt; } -MP4::Item::Item(long long value) - : d(new ItemPrivate()) +MP4::Item::Item(long long value) : + d(new ItemPrivate()) { d->data->m_longlong = value; d->data->type = TypeLongLong; } -MP4::Item::Item(int value1, int value2) - : d(new ItemPrivate()) +MP4::Item::Item(int value1, int value2) : + d(new ItemPrivate()) { d->data->m_intPair.first = value1; d->data->m_intPair.second = value2; d->data->type = TypeIntPair; } -MP4::Item::Item(const ByteVectorList &value) - : d(new ItemPrivate()) +MP4::Item::Item(const ByteVectorList &value) : + d(new ItemPrivate()) { d->data->m_byteVectorList = value; d->data->type = TypeByteVectorList; } -MP4::Item::Item(const StringList &value) - : d(new ItemPrivate()) +MP4::Item::Item(const StringList &value) : + d(new ItemPrivate()) { d->data->m_stringList = value; d->data->type = TypeStringList; } -MP4::Item::Item(const MP4::CoverArtList &value) - : d(new ItemPrivate()) +MP4::Item::Item(const MP4::CoverArtList &value) : + d(new ItemPrivate()) { d->data->m_coverArtList = value; d->data->type = TypeCoverArtList; @@ -262,4 +269,3 @@ MP4::Item::toString() const } return String(); } - diff --git a/taglib/mp4/mp4item.h b/taglib/mp4/mp4item.h index 177463e5..02fd40bb 100644 --- a/taglib/mp4/mp4item.h +++ b/taglib/mp4/mp4item.h @@ -57,7 +57,16 @@ namespace TagLib { Item(); Item(const Item &item); + /*! + * Copies the contents of \a item into this Item. + */ Item &operator=(const Item &item); + + /*! + * Exchanges the content of the Item by the content of \a item. + */ + void swap(Item &item); + ~Item(); Item(int value); diff --git a/taglib/mpeg/id3v2/id3v2footer.cpp b/taglib/mpeg/id3v2/id3v2footer.cpp index defbb17e..abbe054b 100644 --- a/taglib/mpeg/id3v2/id3v2footer.cpp +++ b/taglib/mpeg/id3v2/id3v2footer.cpp @@ -31,30 +31,27 @@ using namespace ID3v2; class Footer::FooterPrivate { -public: - static const uint size = 10; }; -Footer::Footer() +Footer::Footer() : + d(0) { - } Footer::~Footer() { - } TagLib::uint Footer::size() { - return FooterPrivate::size; + return 10; } ByteVector Footer::render(const Header *header) const { - ByteVector headerData = header->render(); - headerData[0] = '3'; - headerData[1] = 'D'; - headerData[2] = 'I'; - return headerData; + ByteVector headerData = header->render(); + headerData[0] = '3'; + headerData[1] = 'D'; + headerData[2] = 'I'; + return headerData; } diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp index 4c5d3af8..593879ad 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.cpp +++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp @@ -54,6 +54,40 @@ using namespace TagLib; using namespace ID3v2; +namespace +{ + void updateGenre(TextIdentificationFrame *frame) + { + StringList fields = frame->fieldList(); + StringList newfields; + + for(StringList::ConstIterator it = fields.begin(); it != fields.end(); ++it) { + String s = *it; + int end = s.find(")"); + + if(s.startsWith("(") && end > 0) { + // "(12)Genre" + String text = s.substr(end + 1); + bool ok; + int number = s.substr(1, end - 1).toInt(&ok); + if(ok && number >= 0 && number <= 255 && !(ID3v1::genre(number) == text)) + newfields.append(s.substr(1, end - 1)); + if(!text.isEmpty()) + newfields.append(text); + } + else { + // "Genre" or "12" + newfields.append(s); + } + } + + if(newfields.isEmpty()) + fields.append(String()); + + frame->setText(newfields); + } +} + class FrameFactory::FrameFactoryPrivate { public: @@ -71,14 +105,13 @@ public: } }; -FrameFactory FrameFactory::factory; - //////////////////////////////////////////////////////////////////////////////// // public members //////////////////////////////////////////////////////////////////////////////// FrameFactory *FrameFactory::instance() { + static FrameFactory factory; return &factory; } @@ -322,9 +355,9 @@ void FrameFactory::setDefaultTextEncoding(String::Type encoding) // protected members //////////////////////////////////////////////////////////////////////////////// -FrameFactory::FrameFactory() +FrameFactory::FrameFactory() : + d(new FrameFactoryPrivate()) { - d = new FrameFactoryPrivate; } FrameFactory::~FrameFactory() @@ -332,9 +365,94 @@ FrameFactory::~FrameFactory() delete d; } +namespace +{ + // Frame conversion table ID3v2.2 -> 2.4 + const char *frameConversion2[][2] = { + { "BUF", "RBUF" }, + { "CNT", "PCNT" }, + { "COM", "COMM" }, + { "CRA", "AENC" }, + { "ETC", "ETCO" }, + { "GEO", "GEOB" }, + { "IPL", "TIPL" }, + { "MCI", "MCDI" }, + { "MLL", "MLLT" }, + { "POP", "POPM" }, + { "REV", "RVRB" }, + { "SLT", "SYLT" }, + { "STC", "SYTC" }, + { "TAL", "TALB" }, + { "TBP", "TBPM" }, + { "TCM", "TCOM" }, + { "TCO", "TCON" }, + { "TCP", "TCMP" }, + { "TCR", "TCOP" }, + { "TDY", "TDLY" }, + { "TEN", "TENC" }, + { "TFT", "TFLT" }, + { "TKE", "TKEY" }, + { "TLA", "TLAN" }, + { "TLE", "TLEN" }, + { "TMT", "TMED" }, + { "TOA", "TOAL" }, + { "TOF", "TOFN" }, + { "TOL", "TOLY" }, + { "TOR", "TDOR" }, + { "TOT", "TOAL" }, + { "TP1", "TPE1" }, + { "TP2", "TPE2" }, + { "TP3", "TPE3" }, + { "TP4", "TPE4" }, + { "TPA", "TPOS" }, + { "TPB", "TPUB" }, + { "TRC", "TSRC" }, + { "TRD", "TDRC" }, + { "TRK", "TRCK" }, + { "TS2", "TSO2" }, + { "TSA", "TSOA" }, + { "TSC", "TSOC" }, + { "TSP", "TSOP" }, + { "TSS", "TSSE" }, + { "TST", "TSOT" }, + { "TT1", "TIT1" }, + { "TT2", "TIT2" }, + { "TT3", "TIT3" }, + { "TXT", "TOLY" }, + { "TXX", "TXXX" }, + { "TYE", "TDRC" }, + { "UFI", "UFID" }, + { "ULT", "USLT" }, + { "WAF", "WOAF" }, + { "WAR", "WOAR" }, + { "WAS", "WOAS" }, + { "WCM", "WCOM" }, + { "WCP", "WCOP" }, + { "WPB", "WPUB" }, + { "WXX", "WXXX" }, + + // Apple iTunes nonstandard frames + { "PCS", "PCST" }, + { "TCT", "TCAT" }, + { "TDR", "TDRL" }, + { "TDS", "TDES" }, + { "TID", "TGID" }, + { "WFD", "WFED" }, + }; + const size_t frameConversion2Size = sizeof(frameConversion2) / sizeof(frameConversion2[0]); + + // Frame conversion table ID3v2.3 -> 2.4 + const char *frameConversion3[][2] = { + { "TORY", "TDOR" }, + { "TYER", "TDRC" }, + { "IPLS", "TIPL" }, + }; + const size_t frameConversion3Size = sizeof(frameConversion3) / sizeof(frameConversion3[0]); +} + bool FrameFactory::updateFrame(Frame::Header *header) const { - TagLib::ByteVector frameID = header->frameID(); + const ByteVector frameID = header->frameID(); switch(header->version()) { @@ -356,75 +474,12 @@ bool FrameFactory::updateFrame(Frame::Header *header) const // ID3v2.2 only used 3 bytes for the frame ID, so we need to convert all of // the frames to their 4 byte ID3v2.4 equivalent. - convertFrame("BUF", "RBUF", header); - convertFrame("CNT", "PCNT", header); - convertFrame("COM", "COMM", header); - convertFrame("CRA", "AENC", header); - convertFrame("ETC", "ETCO", header); - convertFrame("GEO", "GEOB", header); - convertFrame("IPL", "TIPL", header); - convertFrame("MCI", "MCDI", header); - convertFrame("MLL", "MLLT", header); - convertFrame("POP", "POPM", header); - convertFrame("REV", "RVRB", header); - convertFrame("SLT", "SYLT", header); - convertFrame("STC", "SYTC", header); - convertFrame("TAL", "TALB", header); - convertFrame("TBP", "TBPM", header); - convertFrame("TCM", "TCOM", header); - convertFrame("TCO", "TCON", header); - convertFrame("TCP", "TCMP", header); - convertFrame("TCR", "TCOP", header); - convertFrame("TDY", "TDLY", header); - convertFrame("TEN", "TENC", header); - convertFrame("TFT", "TFLT", header); - convertFrame("TKE", "TKEY", header); - convertFrame("TLA", "TLAN", header); - convertFrame("TLE", "TLEN", header); - convertFrame("TMT", "TMED", header); - convertFrame("TOA", "TOAL", header); - convertFrame("TOF", "TOFN", header); - convertFrame("TOL", "TOLY", header); - convertFrame("TOR", "TDOR", header); - convertFrame("TOT", "TOAL", header); - convertFrame("TP1", "TPE1", header); - convertFrame("TP2", "TPE2", header); - convertFrame("TP3", "TPE3", header); - convertFrame("TP4", "TPE4", header); - convertFrame("TPA", "TPOS", header); - convertFrame("TPB", "TPUB", header); - convertFrame("TRC", "TSRC", header); - convertFrame("TRD", "TDRC", header); - convertFrame("TRK", "TRCK", header); - convertFrame("TS2", "TSO2", header); - convertFrame("TSA", "TSOA", header); - convertFrame("TSC", "TSOC", header); - convertFrame("TSP", "TSOP", header); - convertFrame("TSS", "TSSE", header); - convertFrame("TST", "TSOT", header); - convertFrame("TT1", "TIT1", header); - convertFrame("TT2", "TIT2", header); - convertFrame("TT3", "TIT3", header); - convertFrame("TXT", "TOLY", header); - convertFrame("TXX", "TXXX", header); - convertFrame("TYE", "TDRC", header); - convertFrame("UFI", "UFID", header); - convertFrame("ULT", "USLT", header); - convertFrame("WAF", "WOAF", header); - convertFrame("WAR", "WOAR", header); - convertFrame("WAS", "WOAS", header); - convertFrame("WCM", "WCOM", header); - convertFrame("WCP", "WCOP", header); - convertFrame("WPB", "WPUB", header); - convertFrame("WXX", "WXXX", header); - - // Apple iTunes nonstandard frames - convertFrame("PCS", "PCST", header); - convertFrame("TCT", "TCAT", header); - convertFrame("TDR", "TDRL", header); - convertFrame("TDS", "TDES", header); - convertFrame("TID", "TGID", header); - convertFrame("WFD", "WFED", header); + for(size_t i = 0; i < frameConversion2Size; ++i) { + if(frameID == frameConversion2[i][0]) { + header->setFrameID(frameConversion2[i][1]); + break; + } + } break; } @@ -443,9 +498,12 @@ bool FrameFactory::updateFrame(Frame::Header *header) const return false; } - convertFrame("TORY", "TDOR", header); - convertFrame("TYER", "TDRC", header); - convertFrame("IPLS", "TIPL", header); + for(size_t i = 0; i < frameConversion3Size; ++i) { + if(frameID == frameConversion3[i][0]) { + header->setFrameID(frameConversion3[i][1]); + break; + } + } break; } @@ -455,57 +513,11 @@ bool FrameFactory::updateFrame(Frame::Header *header) const // This should catch a typo that existed in TagLib up to and including // version 1.1 where TRDC was used for the year rather than TDRC. - convertFrame("TRDC", "TDRC", header); + if(frameID == "TRDC") + header->setFrameID("TDRC"); + break; } return true; } - -//////////////////////////////////////////////////////////////////////////////// -// private members -//////////////////////////////////////////////////////////////////////////////// - -void FrameFactory::convertFrame(const char *from, const char *to, - Frame::Header *header) const -{ - if(header->frameID() != from) - return; - - // debug("ID3v2.4 no longer supports the frame type " + String(from) + " It has" + - // "been converted to the type " + String(to) + "."); - - header->setFrameID(to); -} - -void FrameFactory::updateGenre(TextIdentificationFrame *frame) const -{ - StringList fields = frame->fieldList(); - StringList newfields; - - for(StringList::ConstIterator it = fields.begin(); it != fields.end(); ++it) { - String s = *it; - const size_t end = s.find(")"); - - if(s.startsWith("(") && end != String::npos()) { - // "(12)Genre" - String text = s.substr(end + 1); - bool ok; - int number = s.substr(1, end - 1).toInt(&ok); - if(ok && number >= 0 && number <= 255 && !(ID3v1::genre(number) == text)) - newfields.append(s.substr(1, end - 1)); - if(!text.isEmpty()) - newfields.append(text); - } - else { - // "Genre" or "12" - newfields.append(s); - } - } - - if(newfields.isEmpty()) - fields.append(String()); - - frame->setText(newfields); - -} diff --git a/taglib/mpeg/id3v2/id3v2framefactory.h b/taglib/mpeg/id3v2/id3v2framefactory.h index 899ac582..1ccc77ea 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.h +++ b/taglib/mpeg/id3v2/id3v2framefactory.h @@ -66,6 +66,7 @@ namespace TagLib { { public: static FrameFactory *instance(); + /*! * Create a frame based on \a data. \a tagHeader should be a valid * ID3v2::Header instance. @@ -131,18 +132,6 @@ namespace TagLib { FrameFactory(const FrameFactory &); FrameFactory &operator=(const FrameFactory &); - /*! - * This method is used internally to convert a frame from ID \a from to ID - * \a to. If the frame matches the \a from pattern and converts the frame - * ID in the \a header or simply does nothing if the frame ID does not match. - */ - void convertFrame(const char *from, const char *to, - Frame::Header *header) const; - - void updateGenre(TextIdentificationFrame *frame) const; - - static FrameFactory factory; - class FrameFactoryPrivate; FrameFactoryPrivate *d; }; diff --git a/taglib/riff/aiff/aifffile.cpp b/taglib/riff/aiff/aifffile.cpp index 165618e9..bd02c067 100644 --- a/taglib/riff/aiff/aifffile.cpp +++ b/taglib/riff/aiff/aifffile.cpp @@ -39,7 +39,6 @@ public: FilePrivate() : properties(0), tag(0), - tagChunkID("ID3 "), hasID3v2(false) {} ~FilePrivate() @@ -50,7 +49,6 @@ public: AudioProperties *properties; ID3v2::Tag *tag; - ByteVector tagChunkID; bool hasID3v2; }; @@ -102,7 +100,10 @@ bool RIFF::AIFF::File::save() return false; } - setChunkData(d->tagChunkID, d->tag->render()); + removeChunk("ID3 "); + removeChunk("id3 "); + + setChunkData("ID3 ", d->tag->render()); d->hasID3v2 = true; return true; @@ -124,7 +125,6 @@ void RIFF::AIFF::File::read(bool readProperties) if(name == "ID3 " || name == "id3 ") { if(!d->tag) { d->tag = new ID3v2::Tag(this, chunkOffset(i)); - d->tagChunkID = name; d->hasID3v2 = true; } else { diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index 09829e24..aadea5d3 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -26,14 +26,14 @@ // This class assumes that std::basic_string has a contiguous and null-terminated buffer. #ifdef HAVE_CONFIG_H -#include +# include #endif -#include "tstring.h" -#include "tdebug.h" -#include "tstringlist.h" -#include "tsmartptr.h" -#include "tutils.h" +#include +#include +#include +#include +#include #include #include @@ -116,6 +116,114 @@ namespace return len; } + + // Returns the native format of std::wstring. + inline String::Type wcharByteOrder() + { + if(Utils::systemByteOrder() == LittleEndian) + return String::UTF16LE; + else + return String::UTF16BE; + } + + // Converts a Latin-1 string into UTF-16(without BOM/CPU byte order) + // and copies it to the internal buffer. + inline void copyFromLatin1(std::wstring &data, const char *s, size_t length) + { + data.resize(length); + + for(size_t i = 0; i < length; ++i) + data[i] = static_cast(s[i]); + } + + // Converts a UTF-8 string into UTF-16(without BOM/CPU byte order) + // and copies it to the internal buffer. + inline void copyFromUTF8(std::wstring &data, const char *s, size_t length) + { + data.resize(length); + + if(length > 0) { + const size_t len = UTF8toUTF16(s, length, &data[0], data.size()); + data.resize(len); + } + } + + // Converts a UTF-16 (with BOM), UTF-16LE or UTF16-BE string into + // UTF-16(without BOM/CPU byte order) and copies it to the internal buffer. + inline void copyFromUTF16(std::wstring &data, const wchar_t *s, size_t length, String::Type t) + { + bool swap; + if(t == String::UTF16) { + if(length >= 1 && s[0] == 0xfeff) + swap = false; // Same as CPU endian. No need to swap bytes. + else if(length >= 1 && s[0] == 0xfffe) + swap = true; // Not same as CPU endian. Need to swap bytes. + else { + debug("String::copyFromUTF16() - Invalid UTF16 string."); + return; + } + + s++; + length--; + } + else { + swap = (t != wcharByteOrder()); + } + + data.resize(length); + if(length > 0) { + if(swap) { + for(size_t i = 0; i < length; ++i) + data[i] = Utils::byteSwap(static_cast(s[i])); + } + else { + ::wmemcpy(&data[0], s, length); + } + } + } + + // Converts a UTF-16 (with BOM), UTF-16LE or UTF16-BE string into + // UTF-16(without BOM/CPU byte order) and copies it to the internal buffer. + inline void copyFromUTF16(std::wstring &data, const char *s, size_t length, String::Type t) + { + bool swap; + if(t == String::UTF16) { + if(length < 2) { + debug("String::copyFromUTF16() - Invalid UTF16 string."); + return; + } + + // Uses memcpy instead of reinterpret_cast to avoid an alignment exception. + ushort bom; + ::memcpy(&bom, s, 2); + + if(bom == 0xfeff) + swap = false; // Same as CPU endian. No need to swap bytes. + else if(bom == 0xfffe) + swap = true; // Not same as CPU endian. Need to swap bytes. + else { + debug("String::copyFromUTF16() - Invalid UTF16 string."); + return; + } + + s += 2; + length -= 2; + } + else { + swap = (t != wcharByteOrder()); + } + + data.resize(length / 2); + for(size_t i = 0; i < length / 2; ++i) { + ushort c; + ::memcpy(&c, s, 2); + if(swap) + c = Utils::byteSwap(c); + + data[i] = static_cast(c); + s += 2; + } + } } namespace TagLib { @@ -164,9 +272,9 @@ String::String(const std::string &s, Type t) : d(new StringPrivate()) { if(t == Latin1) - copyFromLatin1(s.c_str(), s.length()); + copyFromLatin1(*d->data, s.c_str(), s.length()); else if(t == String::UTF8) - copyFromUTF8(s.c_str(), s.length()); + copyFromUTF8(*d->data, s.c_str(), s.length()); else { debug("String::String() -- std::string should not contain UTF16."); } @@ -175,8 +283,11 @@ String::String(const std::string &s, Type t) : String::String(const std::wstring &s, Type t) : d(new StringPrivate()) { + if(t == UTF16Native) + t = wcharByteOrder(); + if(t == UTF16 || t == UTF16BE || t == UTF16LE) - copyFromUTF16(s.c_str(), s.length(), t); + copyFromUTF16(*d->data, s.c_str(), s.length(), t); else { debug("String::String() -- std::wstring should not contain Latin1 or UTF-8."); } @@ -185,8 +296,11 @@ String::String(const std::wstring &s, Type t) : String::String(const wchar_t *s, Type t) : d(new StringPrivate()) { + if(t == UTF16Native) + t = wcharByteOrder(); + if(t == UTF16 || t == UTF16BE || t == UTF16LE) - copyFromUTF16(s, ::wcslen(s), t); + copyFromUTF16(*d->data, s, ::wcslen(s), t); else { debug("String::String() -- const wchar_t * should not contain Latin1 or UTF-8."); } @@ -196,9 +310,9 @@ String::String(const char *s, Type t) : d(new StringPrivate()) { if(t == Latin1) - copyFromLatin1(s, ::strlen(s)); + copyFromLatin1(*d->data, s, ::strlen(s)); else if(t == String::UTF8) - copyFromUTF8(s, ::strlen(s)); + copyFromUTF8(*d->data, s, ::strlen(s)); else { debug("String::String() -- const char * should not contain UTF16."); } @@ -207,8 +321,11 @@ String::String(const char *s, Type t) : String::String(wchar_t c, Type t) : d(new StringPrivate()) { + if(t == UTF16Native) + t = wcharByteOrder(); + if(t == UTF16 || t == UTF16BE || t == UTF16LE) - copyFromUTF16(&c, 1, t); + copyFromUTF16(*d->data, &c, 1, t); else { debug("String::String() -- wchar_t should not contain Latin1 or UTF-8."); } @@ -218,9 +335,9 @@ String::String(char c, Type t) : d(new StringPrivate()) { if(t == Latin1) - copyFromLatin1(&c, 1); + copyFromLatin1(*d->data, &c, 1); else if(t == String::UTF8) - copyFromUTF8(&c, 1); + copyFromUTF8(*d->data, &c, 1); else { debug("String::String() -- char should not contain UTF16."); } @@ -232,12 +349,15 @@ String::String(const ByteVector &v, Type t) : if(v.isEmpty()) return; + if(t == UTF16Native) + t = wcharByteOrder(); + if(t == Latin1) - copyFromLatin1(v.data(), v.size()); + copyFromLatin1(*d->data, v.data(), v.size()); else if(t == UTF8) - copyFromUTF8(v.data(), v.size()); + copyFromUTF8(*d->data, v.data(), v.size()); else - copyFromUTF16(v.data(), v.size(), t); + copyFromUTF16(*d->data, v.data(), v.size(), t); // If we hit a null in the ByteVector, shrink the string again. d->data->resize(::wcslen(d->data->c_str())); @@ -462,9 +582,7 @@ int String::toInt(bool *ok) const // Has wcstol() consumed the entire string and not overflowed? if(ok) { *ok = (errno == 0 && end > begin && *end == L'\0'); -#if LONG_MAX > INT_MAX *ok = (*ok && value > INT_MIN && value < INT_MAX); -#endif } return static_cast(value); @@ -595,55 +713,62 @@ String &String::operator+=(char c) String &String::operator=(const String &s) { - *d = *s.d; + String(s).swap(*this); return *this; } String &String::operator=(const std::string &s) { - *this = String(s); + String(s).swap(*this); return *this; } String &String::operator=(const std::wstring &s) { - *this = String(s); + String(s).swap(*this); return *this; } String &String::operator=(const wchar_t *s) { - *this = String(s); + String(s).swap(*this); return *this; } String &String::operator=(char c) { - *this = String(c); + String(c).swap(*this); return *this; } String &String::operator=(wchar_t c) { - *this = String(c); + String(c).swap(*this); return *this; } String &String::operator=(const char *s) { - *this = String(s); + String(s).swap(*this); return *this; } String &String::operator=(const ByteVector &v) { - *this = String(v); + String(v).swap(*this); return *this; } +void String::swap(String &s) +{ + using std::swap; + + swap(d, s.d); +} + bool String::operator<(const String &s) const { - return *d->data < *s.d->data; + return (*d->data < *s.d->data); } //////////////////////////////////////////////////////////////////////////////// @@ -653,107 +778,11 @@ bool String::operator<(const String &s) const void String::detach() { if(!d->data.unique()) - d->data.reset(new std::wstring(*d->data)); + String(d->data->c_str()).swap(*this); } //////////////////////////////////////////////////////////////////////////////// -// private members -//////////////////////////////////////////////////////////////////////////////// - -void String::copyFromLatin1(const char *s, size_t length) -{ - d->data->resize(length); - - for(size_t i = 0; i < length; ++i) - (*d->data)[i] = static_cast(s[i]); -} - -void String::copyFromUTF8(const char *s, size_t length) -{ - d->data->resize(length); - - if(length > 0) { - const size_t len = UTF8toUTF16(s, length, &(*d->data)[0], d->data->size()); - d->data->resize(len); - } -} - -void String::copyFromUTF16(const wchar_t *s, size_t length, Type t) -{ - bool swap; - if(t == UTF16) { - if(length >= 1 && s[0] == 0xfeff) - swap = false; // Same as CPU endian. No need to swap bytes. - else if(length >= 1 && s[0] == 0xfffe) - swap = true; // Not same as CPU endian. Need to swap bytes. - else { - debug("String::copyFromUTF16() - Invalid UTF16 string."); - return; - } - - s++; - length--; - } - else - swap = (t != WCharByteOrder); - - d->data->resize(length); - if(length > 0) { - if(swap) { - for(size_t i = 0; i < length; ++i) - (*d->data)[i] = Utils::byteSwap(static_cast(s[i])); - } - else { - ::wmemcpy(&(*d->data)[0], s, length); - } - } -} - -void String::copyFromUTF16(const char *s, size_t length, Type t) -{ - bool swap; - if(t == UTF16) { - if(length < 2) { - debug("String::copyFromUTF16() - Invalid UTF16 string."); - return; - } - - // Uses memcpy instead of reinterpret_cast to avoid an alignment exception. - ushort bom; - ::memcpy(&bom, s, 2); - - if(bom == 0xfeff) - swap = false; // Same as CPU endian. No need to swap bytes. - else if(bom == 0xfffe) - swap = true; // Not same as CPU endian. Need to swap bytes. - else { - debug("String::copyFromUTF16() - Invalid UTF16 string."); - return; - } - - s += 2; - length -= 2; - } - else - swap = (t != WCharByteOrder); - - d->data->resize(length / 2); - for(size_t i = 0; i < length / 2; ++i) { - ushort c; - ::memcpy(&c, s, 2); - if(swap) - c = Utils::byteSwap(c); - - (*d->data)[i] = static_cast(c); - s += 2; - } -} - -const String::Type String::WCharByteOrder - = (Utils::systemByteOrder() == BigEndian) ? String::UTF16BE : String::UTF16LE; - -//////////////////////////////////////////////////////////////////////////////// -// related functions +// related non-member functions //////////////////////////////////////////////////////////////////////////////// const String operator+(const String &s1, const String &s2) diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index 2ce70ed2..51048608 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -110,7 +110,11 @@ namespace TagLib { /*! * UTF16 little endian. 16 bit characters. */ - UTF16LE = 4 + UTF16LE = 4, + /*! + * UTF16 in the native byte order of the system. 16 bit characters. + */ + UTF16Native = 5 }; /*! @@ -141,7 +145,7 @@ namespace TagLib { * exit. UTF16BE or UTF16LE is automatically chosen as default according * to the CPU byte order */ - String(const std::wstring &s, Type t = WCharByteOrder); + String(const std::wstring &s, Type t = UTF16Native); /*! * Makes a deep copy of the data in \a s. @@ -151,7 +155,7 @@ namespace TagLib { * exit. UTF16BE or UTF16LE is automatically chosen as default according * to the CPU byte order */ - String(const wchar_t *s, Type t = WCharByteOrder); + String(const wchar_t *s, Type t = UTF16Native); /*! * Makes a deep copy of the data in \a c. @@ -169,7 +173,7 @@ namespace TagLib { * exit. UTF16BE or UTF16LE is automatically chosen as default according * to the CPU byte order */ - String(wchar_t c, Type t = WCharByteOrder); + String(wchar_t c, Type t = UTF16Native); /*! * Makes a deep copy of the data in \a s. @@ -483,6 +487,11 @@ namespace TagLib { */ String &operator=(const ByteVector &v); + /*! + * Exchanges the content of the String by the content of \a s. + */ + void swap(String &s); + /*! * To be able to use this class in a Map, this operator needed to be * implemented. Returns true if \a s is less than this string in a byte-wise @@ -506,37 +515,6 @@ namespace TagLib { void detach(); private: - /*! - * Converts a \e Latin-1 string into \e UTF-16(without BOM/CPU byte order) - * and copies it to the internal buffer. - */ - void copyFromLatin1(const char *s, size_t length); - - /*! - * Converts a \e UTF-8 string into \e UTF-16(without BOM/CPU byte order) - * and copies it to the internal buffer. - */ - void copyFromUTF8(const char *s, size_t length); - - /*! - * Converts a \e UTF-16 (with BOM), UTF-16LE or UTF16-BE string into - * \e UTF-16(without BOM/CPU byte order) and copies it to the internal buffer. - */ - void copyFromUTF16(const wchar_t *s, size_t length, Type t); - - /*! - * Converts a \e UTF-16 (with BOM), UTF-16LE or UTF16-BE string into - * \e UTF-16(without BOM/CPU byte order) and copies it to the internal buffer. - */ - void copyFromUTF16(const char *s, size_t length, Type t); - - /*! - * Indicates which byte order of UTF-16 is used to store strings internally. - * - * \note \e String::UTF16BE or \e String::UTF16LE - */ - static const Type WCharByteOrder; - class StringPrivate; StringPrivate *d; }; diff --git a/tests/test_aiff.cpp b/tests/test_aiff.cpp index 386c7686..3844fa0b 100644 --- a/tests/test_aiff.cpp +++ b/tests/test_aiff.cpp @@ -77,13 +77,18 @@ public: void testDuplicateID3v2() { - RIFF::AIFF::File f(TEST_FILE_PATH_C("duplicate_id3v2.aiff")); + ScopedFileCopy copy("duplicate_id3v2", ".aiff"); - // duplicate_id3v2.aiff has duplicate ID3v2 tags. + // duplicate_id3v2.aiff has duplicate ID3v2 tag chunks. // title() returns "Title2" if can't skip the second tag. + RIFF::AIFF::File f(copy.fileName().c_str()); CPPUNIT_ASSERT(f.hasID3v2Tag()); CPPUNIT_ASSERT_EQUAL(String("Title1"), f.tag()->title()); + + f.save(); + CPPUNIT_ASSERT_EQUAL((offset_t)7030, f.length()); + CPPUNIT_ASSERT_EQUAL((offset_t)-1, f.find("Title2")); } void testFuzzedFile1() diff --git a/tests/test_string.cpp b/tests/test_string.cpp index a9cb7382..17c965e2 100644 --- a/tests/test_string.cpp +++ b/tests/test_string.cpp @@ -257,6 +257,12 @@ public: String("9999999999").toInt(&ok); CPPUNIT_ASSERT_EQUAL(ok, false); + + String("2147483648").toInt(&ok); + CPPUNIT_ASSERT_EQUAL(ok, false); + + String("-2147483649").toInt(&ok); + CPPUNIT_ASSERT_EQUAL(ok, false); } void testFromInt()