From 1802237c75b7a188629bb5e24b75a8f1a0a0354f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Tue, 15 Mar 2011 20:50:47 +0100 Subject: [PATCH 1/9] Support for writing structuraly correct ID3v2.3 tags We don't use the tag footer, extended header or unsynchronization, so we only need to change the frame size format. Note that this doesn't write correct ID3v2.3 tags, just tags that have the correct structure. The content is sill incorrect, unless you add the right frames manually to the tag instance. --- taglib/mpeg/id3v2/id3v2frame.cpp | 11 +++++++++- taglib/mpeg/id3v2/id3v2frame.h | 10 +++++++-- taglib/mpeg/id3v2/id3v2header.cpp | 2 +- taglib/mpeg/id3v2/id3v2tag.cpp | 9 +++++++- taglib/mpeg/id3v2/id3v2tag.h | 9 ++++++++ taglib/mpeg/mpegfile.cpp | 7 +++++- taglib/mpeg/mpegfile.h | 15 +++++++++++++ tests/test_mpeg.cpp | 36 +++++++++++++++++++++++++++++++ 8 files changed, 93 insertions(+), 6 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 786336b9..ef8e3eed 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -484,6 +484,11 @@ TagLib::uint Frame::Header::version() const return d->version; } +void Frame::Header::setVersion(TagLib::uint version) +{ + d->version = version; +} + bool Frame::Header::tagAlterPreservation() const { return d->tagAlterPreservation; @@ -538,7 +543,11 @@ ByteVector Frame::Header::render() const { ByteVector flags(2, char(0)); // just blank for the moment - ByteVector v = d->frameID + SynchData::fromUInt(d->frameSize) + flags; + ByteVector v = d->frameID + + (d->version == 3 + ? ByteVector::fromUInt(d->frameSize) + : SynchData::fromUInt(d->frameSize)) + + flags; return v; } diff --git a/taglib/mpeg/id3v2/id3v2frame.h b/taglib/mpeg/id3v2/id3v2frame.h index 661be3d2..cd401c09 100644 --- a/taglib/mpeg/id3v2/id3v2frame.h +++ b/taglib/mpeg/id3v2/id3v2frame.h @@ -294,11 +294,17 @@ namespace TagLib { void setFrameSize(uint size); /*! - * Returns the ID3v2 version of the header (as passed in from the - * construction of the header). + * Returns the ID3v2 version of the header, as passed in from the + * construction of the header or set via setVersion(). */ uint version() const; + /*! + * Sets the ID3v2 version of the header, changing has impact on the + * correct parsing/rendering of frame data. + */ + void setVersion(uint version); + /*! * Returns the size of the frame header in bytes. * diff --git a/taglib/mpeg/id3v2/id3v2header.cpp b/taglib/mpeg/id3v2/id3v2header.cpp index 31a5a1ec..e21292de 100644 --- a/taglib/mpeg/id3v2/id3v2header.cpp +++ b/taglib/mpeg/id3v2/id3v2header.cpp @@ -164,7 +164,7 @@ ByteVector Header::render() const // add the version number -- we always render a 2.4.0 tag regardless of what // the tag originally was. - v.append(char(4)); + v.append(char(majorVersion())); v.append(char(0)); // Currently we don't actually support writing extended headers, footers or diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 7e8012b2..c85ef59d 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -330,6 +330,11 @@ void ID3v2::Tag::removeFrames(const ByteVector &id) } ByteVector ID3v2::Tag::render() const +{ + return render(4); +} + +ByteVector ID3v2::Tag::render(int version) const { // We need to render the "tag data" first so that we have to correct size to // render in the tag's header. The "tag data" -- everything that is included @@ -343,6 +348,7 @@ ByteVector ID3v2::Tag::render() const // Loop through the frames rendering them and adding them to the tagData. for(FrameList::Iterator it = d->frameList.begin(); it != d->frameList.end(); it++) { + (*it)->header()->setVersion(version); if((*it)->header()->frameID().size() != 4) { debug("A frame of unsupported or unknown type \'" + String((*it)->header()->frameID()) + "\' has been discarded"); @@ -364,7 +370,8 @@ ByteVector ID3v2::Tag::render() const tagData.append(ByteVector(paddingSize, char(0))); - // Set the tag size. + // Set the version and data size. + d->header.setMajorVersion(version); d->header.setTagSize(tagData.size()); // TODO: This should eventually include d->footer->render(). diff --git a/taglib/mpeg/id3v2/id3v2tag.h b/taglib/mpeg/id3v2/id3v2tag.h index a139d455..137139bd 100644 --- a/taglib/mpeg/id3v2/id3v2tag.h +++ b/taglib/mpeg/id3v2/id3v2tag.h @@ -265,6 +265,15 @@ namespace TagLib { */ ByteVector render() const; + /*! + * Render the tag back to binary data, suitable to be written to disk. + * + * The \a version parameter specifies the version of the rendered + * ID3v2 tag. It can be either 4 or 3. + */ + // BIC: combine with the above method + ByteVector render(int version) const; + protected: /*! * Reads data from the file specified in the constructor. It does basic diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 2fc1b380..3b3513ae 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -139,6 +139,11 @@ bool MPEG::File::save(int tags) } bool MPEG::File::save(int tags, bool stripOthers) +{ + return save(tags, stripOthers, 4); +} + +bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version) { if(tags == NoTags && stripOthers) return strip(AllTags); @@ -174,7 +179,7 @@ bool MPEG::File::save(int tags, bool stripOthers) if(!d->hasID3v2) d->ID3v2Location = 0; - insert(ID3v2Tag()->render(), d->ID3v2Location, d->ID3v2OriginalSize); + insert(ID3v2Tag()->render(id3v2Version), d->ID3v2Location, d->ID3v2OriginalSize); d->hasID3v2 = true; diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index 282af775..7b41328a 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -161,6 +161,21 @@ namespace TagLib { // BIC: combine with the above method bool save(int tags, bool stripOthers); + /*! + * Save the file. This will attempt to save all of the tag types that are + * specified by OR-ing together TagTypes values. The save() method above + * uses AllTags. This returns true if saving was successful. + * + * If \a stripOthers is true this strips all tags not included in the mask, + * but does not modify them in memory, so later calls to save() which make + * use of these tags will remain valid. This also strips empty tags. + * + * The \a id3v2Version parameter specifies the version of the saved + * ID3v2 tag. It can be either 4 or 3. + */ + // BIC: combine with the above method + bool save(int tags, bool stripOthers, int id3v2Version); + /*! * Returns a pointer to the ID3v2 tag of the file. * diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index 6278ff55..6e85aed3 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -2,6 +2,8 @@ #include #include #include +#include +#include "utils.h" using namespace std; using namespace TagLib; @@ -10,6 +12,8 @@ class TestMPEG : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TestMPEG); CPPUNIT_TEST(testVersion2DurationWithXingHeader); + CPPUNIT_TEST(testSaveID3v24); + CPPUNIT_TEST(testSaveID3v23); CPPUNIT_TEST_SUITE_END(); public: @@ -20,6 +24,38 @@ public: CPPUNIT_ASSERT_EQUAL(5387, f.audioProperties()->length()); } + void testSaveID3v24() + { + ScopedFileCopy copy("xing", ".mp3", false); + string newname = copy.fileName(); + + String xxx = ByteVector(254, 'X'); + MPEG::File f(newname.c_str()); + f.tag()->setTitle(xxx); + f.tag()->setArtist("Artist A"); + f.save(MPEG::File::AllTags, true, 4); + + MPEG::File f2(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist()); + CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title()); + } + + void testSaveID3v23() + { + ScopedFileCopy copy("xing", ".mp3", false); + string newname = copy.fileName(); + + String xxx = ByteVector(254, 'X'); + MPEG::File f(newname.c_str()); + f.tag()->setTitle(xxx); + f.tag()->setArtist("Artist A"); + f.save(MPEG::File::AllTags, true, 3); + + MPEG::File f2(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist()); + CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title()); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG); From 061b381ea835edd14c8a2e86cf26c97dba00f68f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Tue, 15 Mar 2011 21:57:49 +0100 Subject: [PATCH 2/9] Make sure we don't write UTF8 or UTF16BE to ID3v2.3 tags --- .../id3v2/frames/attachedpictureframe.cpp | 2 +- taglib/mpeg/id3v2/frames/commentsframe.cpp | 4 ++-- .../id3v2/frames/textidentificationframe.cpp | 2 +- taglib/mpeg/id3v2/frames/urllinkframe.cpp | 2 +- taglib/mpeg/id3v2/id3v2frame.cpp | 23 +++++++++++++++++-- taglib/mpeg/id3v2/id3v2frame.h | 20 ++++++++++++++++ tests/test_id3v2.cpp | 18 +++++++++++++++ 7 files changed, 64 insertions(+), 7 deletions(-) diff --git a/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp b/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp index e4e97d01..c31e1076 100644 --- a/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp +++ b/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp @@ -152,7 +152,7 @@ ByteVector AttachedPictureFrame::renderFields() const { ByteVector data; - String::Type encoding = checkEncoding(d->description, d->textEncoding); + String::Type encoding = checkTextEncoding(d->description, d->textEncoding); data.append(char(encoding)); data.append(d->mimeType.data(String::Latin1)); diff --git a/taglib/mpeg/id3v2/frames/commentsframe.cpp b/taglib/mpeg/id3v2/frames/commentsframe.cpp index 406598d9..377a7ee3 100644 --- a/taglib/mpeg/id3v2/frames/commentsframe.cpp +++ b/taglib/mpeg/id3v2/frames/commentsframe.cpp @@ -155,8 +155,8 @@ ByteVector CommentsFrame::renderFields() const String::Type encoding = d->textEncoding; - encoding = checkEncoding(d->description, encoding); - encoding = checkEncoding(d->text, encoding); + encoding = checkTextEncoding(d->description, encoding); + encoding = checkTextEncoding(d->text, encoding); v.append(char(encoding)); v.append(d->language.size() == 3 ? d->language : "XXX"); diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp index 7c2ab909..cf724922 100644 --- a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp +++ b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp @@ -139,7 +139,7 @@ void TextIdentificationFrame::parseFields(const ByteVector &data) ByteVector TextIdentificationFrame::renderFields() const { - String::Type encoding = checkEncoding(d->fieldList, d->textEncoding); + String::Type encoding = checkTextEncoding(d->fieldList, d->textEncoding); ByteVector v; diff --git a/taglib/mpeg/id3v2/frames/urllinkframe.cpp b/taglib/mpeg/id3v2/frames/urllinkframe.cpp index 7756c4ad..09edec40 100644 --- a/taglib/mpeg/id3v2/frames/urllinkframe.cpp +++ b/taglib/mpeg/id3v2/frames/urllinkframe.cpp @@ -175,7 +175,7 @@ ByteVector UserUrlLinkFrame::renderFields() const { ByteVector v; - String::Type encoding = checkEncoding(d->description, d->textEncoding); + String::Type encoding = checkTextEncoding(d->description, d->textEncoding); v.append(char(encoding)); v.append(d->description.data(encoding)); diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index ef8e3eed..59559178 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -230,19 +230,38 @@ String Frame::readStringField(const ByteVector &data, String::Type encoding, int String::Type Frame::checkEncoding(const StringList &fields, String::Type encoding) // static { + return checkEncoding(fields, encoding, 4); +} + +String::Type Frame::checkEncoding(const StringList &fields, String::Type encoding, uint version) // static +{ + if((encoding == String::UTF8 || encoding == String::UTF16BE) && version != 4) + return String::UTF16; + if(encoding != String::Latin1) return encoding; for(StringList::ConstIterator it = fields.begin(); it != fields.end(); ++it) { if(!(*it).isLatin1()) { - debug("Frame::checkEncoding() -- Rendering using UTF8."); - return String::UTF8; + if(version == 4) { + debug("Frame::checkEncoding() -- Rendering using UTF8."); + return String::UTF8; + } + else { + debug("Frame::checkEncoding() -- Rendering using UTF16."); + return String::UTF16; + } } } return String::Latin1; } +String::Type Frame::checkTextEncoding(const StringList &fields, String::Type encoding) const +{ + return checkEncoding(fields, encoding, header()->version()); +} + //////////////////////////////////////////////////////////////////////////////// // Frame::Header class //////////////////////////////////////////////////////////////////////////////// diff --git a/taglib/mpeg/id3v2/id3v2frame.h b/taglib/mpeg/id3v2/id3v2frame.h index cd401c09..2b6bcd88 100644 --- a/taglib/mpeg/id3v2/id3v2frame.h +++ b/taglib/mpeg/id3v2/id3v2frame.h @@ -199,9 +199,29 @@ namespace TagLib { * Checks a the list of string values to see if they can be used with the * specified encoding and returns the recommended encoding. */ + // BIC: remove and make non-static static String::Type checkEncoding(const StringList &fields, String::Type encoding); + /*! + * Checks a the list of string values to see if they can be used with the + * specified encoding and returns the recommended encoding. This method + * also checks the ID3v2 version and makes sure the encoding can be used + * in the specified version. + */ + // BIC: remove and make non-static + static String::Type checkEncoding(const StringList &fields, + String::Type encoding, uint version); + + /*! + * Checks a the list of string values to see if they can be used with the + * specified encoding and returns the recommended encoding. This method + * also checks the ID3v2 version and makes sure the encoding can be used + * in the version specified by the frame's header. + */ + String::Type checkTextEncoding(const StringList &fields, + String::Type encoding) const; + private: Frame(const Frame &); Frame &operator=(const Frame &); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 4276963d..3460168c 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -1,9 +1,12 @@ #include #include #include +// so evil :( +#define protected public #include #include #include +#undef protected #include #include #include @@ -33,6 +36,7 @@ class TestID3v2 : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TestID3v2); CPPUNIT_TEST(testUnsynchDecode); + CPPUNIT_TEST(testDowngradeUTF8ForID3v23); CPPUNIT_TEST(testUTF16BEDelimiter); CPPUNIT_TEST(testUTF16Delimiter); CPPUNIT_TEST(testReadStringField); @@ -73,6 +77,20 @@ public: CPPUNIT_ASSERT_EQUAL(String("My babe just cares for me"), f.tag()->title()); } + void testDowngradeUTF8ForID3v23() + { + ID3v2::TextIdentificationFrame f(ByteVector("TPE1"), String::UTF8); + StringList sl; + sl.append("Foo"); + f.setText(sl); + f.header()->setVersion(3); + ByteVector data = f.render(); + CPPUNIT_ASSERT_EQUAL((unsigned int)(4+4+2+1+6+2), data.size()); + ID3v2::TextIdentificationFrame f2(data); + CPPUNIT_ASSERT_EQUAL(sl, f2.fieldList()); + CPPUNIT_ASSERT_EQUAL(String::UTF16, f2.textEncoding()); + } + void testUTF16BEDelimiter() { ID3v2::TextIdentificationFrame f(ByteVector("TPE1"), String::UTF16BE); From e8d0551c9a4188ea4f93c54fed2913fd5d6b18b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Wed, 16 Mar 2011 17:14:36 +0100 Subject: [PATCH 3/9] Simple ID3v2.4 => ID3v2.3 frame migration --- taglib/mpeg/id3v2/id3v2tag.cpp | 68 +++++++++++++++++++++++++++++++++- taglib/mpeg/id3v2/id3v2tag.h | 2 + tests/test_id3v2.cpp | 43 +++++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index c85ef59d..6908c1b0 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -334,6 +334,61 @@ ByteVector ID3v2::Tag::render() const return render(4); } +void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const +{ + const char *unsupportedFrames[] = { + "ASPI", "EQU2", "RVA2", "SEEK", "SIGN", "TDRL", "TDTG", + "TMOO", "TPRO", "TSOA", "TSOT", "TSST", "TSOP", "TIPL", + "TMCL", 0 + }; + ID3v2::TextIdentificationFrame *frameTDOR = 0; + ID3v2::TextIdentificationFrame *frameTDRC = 0; + for(FrameList::Iterator it = d->frameList.begin(); it != d->frameList.end(); it++) { + ID3v2::Frame *frame = *it; + ByteVector frameID = frame->header()->frameID(); + for(int i = 0; unsupportedFrames[i]; i++) { + if(frameID == unsupportedFrames[i]) { + debug("A frame that is not supported in ID3v2.3 \'" + + String(frameID) + "\' has been discarded"); + frame = 0; + break; + } + } + if(frame && frameID == "TDOR") { + frameTDOR = dynamic_cast(frame); + frame = 0; + } + if(frame && frameID == "TDRC") { + frameTDRC = dynamic_cast(frame); + frame = 0; + } + if(frame) { + frames->append(frame); + } + } + if(frameTDOR) { + String content = frameTDOR->toString(); + if(content.size() >= 4) { + ID3v2::TextIdentificationFrame *frameTORY = new ID3v2::TextIdentificationFrame("TORY", String::Latin1); + frameTORY->setText(content.substr(0, 4)); + frames->append(frameTORY); + newFrames->append(frameTORY); + } + } + if(frameTDRC) { + // FIXME we can do better here, define also TDAT and TIME + String content = frameTDRC->toString(); + if(content.size() >= 4) { + ID3v2::TextIdentificationFrame *frameTYER = new ID3v2::TextIdentificationFrame("TYER", String::Latin1); + frameTYER->setText(content.substr(0, 4)); + frames->append(frameTYER); + newFrames->append(frameTYER); + } + } + // FIXME migrate TIPL and TMCL to IPLS +} + + ByteVector ID3v2::Tag::render(int version) const { // We need to render the "tag data" first so that we have to correct size to @@ -347,7 +402,18 @@ ByteVector ID3v2::Tag::render(int version) const // Loop through the frames rendering them and adding them to the tagData. - for(FrameList::Iterator it = d->frameList.begin(); it != d->frameList.end(); it++) { + FrameList newFrames; + newFrames.setAutoDelete(true); + + FrameList frameList; + if(version == 4) { + frameList = d->frameList; + } + else { + downgradeFrames(&frameList, &newFrames); + } + + for(FrameList::Iterator it = frameList.begin(); it != frameList.end(); it++) { (*it)->header()->setVersion(version); if((*it)->header()->frameID().size() != 4) { debug("A frame of unsupported or unknown type \'" diff --git a/taglib/mpeg/id3v2/id3v2tag.h b/taglib/mpeg/id3v2/id3v2tag.h index 137139bd..4a52854a 100644 --- a/taglib/mpeg/id3v2/id3v2tag.h +++ b/taglib/mpeg/id3v2/id3v2tag.h @@ -295,6 +295,8 @@ namespace TagLib { */ void setTextFrame(const ByteVector &id, const String &value); + void downgradeFrames(FrameList *existingFrames, FrameList *newFrames) const; + private: Tag(const Tag &); Tag &operator=(const Tag &); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 3460168c..84074585 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -64,6 +64,7 @@ class TestID3v2 : public CppUnit::TestFixture CPPUNIT_TEST(testUpdateGenre23_2); CPPUNIT_TEST(testUpdateGenre24); CPPUNIT_TEST(testUpdateDate22); + CPPUNIT_TEST(testDowngradeTo23); // CPPUNIT_TEST(testUpdateFullDate22); TODO TYE+TDA should be upgraded to TDRC together CPPUNIT_TEST(testCompressedFrameWithBrokenLength); CPPUNIT_TEST_SUITE_END(); @@ -474,6 +475,48 @@ public: CPPUNIT_ASSERT_EQUAL(String("2010-04-03"), f.ID3v2Tag()->frameListMap()["TDRC"].front()->toString()); } + void testDowngradeTo23() + { + ScopedFileCopy copy("xing", ".mp3"); + string newname = copy.fileName(); + + ID3v2::TextIdentificationFrame *tf; + MPEG::File foo(newname.c_str()); + tf = new ID3v2::TextIdentificationFrame("TDOR", String::Latin1); + tf->setText("2011-03-16"); + foo.ID3v2Tag()->addFrame(tf); + tf = new ID3v2::TextIdentificationFrame("TDRC", String::Latin1); + tf->setText("2012-04-17"); + foo.ID3v2Tag()->addFrame(tf); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDRL", String::Latin1)); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDTG", String::Latin1)); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TMOO", String::Latin1)); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TPRO", String::Latin1)); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOA", String::Latin1)); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOT", String::Latin1)); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSST", String::Latin1)); + foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOP", String::Latin1)); + foo.save(MPEG::File::AllTags, true, 3); + + MPEG::File bar(newname.c_str()); + tf = static_cast(bar.ID3v2Tag()->frameList("TDOR").front()); + CPPUNIT_ASSERT(tf); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), tf->fieldList().size()); + CPPUNIT_ASSERT_EQUAL(String("2011"), tf->fieldList().front()); + tf = static_cast(bar.ID3v2Tag()->frameList("TDRC").front()); + CPPUNIT_ASSERT(tf); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), tf->fieldList().size()); + CPPUNIT_ASSERT_EQUAL(String("2012"), tf->fieldList().front()); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TDRL")); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TDTG")); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TMOO")); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TPRO")); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOA")); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOT")); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSST")); + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOP")); + } + void testCompressedFrameWithBrokenLength() { MPEG::File f("data/compressed_id3_frame.mp3", false); From 1453a7b157fc88bc6eb28742f67e0ecc70d6dd64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Wed, 16 Mar 2011 17:19:11 +0100 Subject: [PATCH 4/9] Clean-up temporary files --- tests/test_mpeg.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index 6e85aed3..745bcf6b 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -26,7 +26,7 @@ public: void testSaveID3v24() { - ScopedFileCopy copy("xing", ".mp3", false); + ScopedFileCopy copy("xing", ".mp3"); string newname = copy.fileName(); String xxx = ByteVector(254, 'X'); @@ -42,7 +42,7 @@ public: void testSaveID3v23() { - ScopedFileCopy copy("xing", ".mp3", false); + ScopedFileCopy copy("xing", ".mp3"); string newname = copy.fileName(); String xxx = ByteVector(254, 'X'); From d3df66f196fe3580de6d1a78f779d4a4309a65b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Wed, 16 Mar 2011 22:54:58 +0100 Subject: [PATCH 5/9] Convert TDRC to TYER+TDAT+TIME --- taglib/mpeg/id3v2/id3v2tag.cpp | 12 ++++++++++++ tests/test_id3v2.cpp | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 6908c1b0..7286d3c1 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -383,6 +383,18 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const frameTYER->setText(content.substr(0, 4)); frames->append(frameTYER); newFrames->append(frameTYER); + if(content.size() >= 10 && content[4] == '-' && content[7] == '-') { + ID3v2::TextIdentificationFrame *frameTDAT = new ID3v2::TextIdentificationFrame("TDAT", String::Latin1); + frameTDAT->setText(content.substr(8, 2) + content.substr(5, 2)); + frames->append(frameTDAT); + newFrames->append(frameTDAT); + if(content.size() >= 16 && content[10] == 'T' && content[13] == ':') { + ID3v2::TextIdentificationFrame *frameTIME = new ID3v2::TextIdentificationFrame("TIME", String::Latin1); + frameTIME->setText(content.substr(11, 2) + content.substr(14, 2)); + frames->append(frameTIME); + newFrames->append(frameTIME); + } + } } } // FIXME migrate TIPL and TMCL to IPLS diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 84074585..d3adc632 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -486,7 +486,7 @@ public: tf->setText("2011-03-16"); foo.ID3v2Tag()->addFrame(tf); tf = new ID3v2::TextIdentificationFrame("TDRC", String::Latin1); - tf->setText("2012-04-17"); + tf->setText("2012-04-17T12:01"); foo.ID3v2Tag()->addFrame(tf); foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDRL", String::Latin1)); foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDTG", String::Latin1)); From aa57db3a391d6a00473e006431b6acb78bfd47b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Tue, 5 Apr 2011 15:16:17 +0200 Subject: [PATCH 6/9] Convert frames TIPL and TMCL (2.4) to IPLS (2.3) --- taglib/mpeg/id3v2/id3v2framefactory.cpp | 2 +- taglib/mpeg/id3v2/id3v2tag.cpp | 36 ++++++++++++++++++++++--- tests/test_id3v2.cpp | 17 ++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp index 356d60d4..77bd85d5 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.cpp +++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp @@ -153,7 +153,7 @@ Frame *FrameFactory::createFrame(const ByteVector &origData, Header *tagHeader) // Text Identification (frames 4.2) - if(frameID.startsWith("T")) { + if(frameID.startsWith("T") || frameID == "IPLS") { TextIdentificationFrame *f = frameID != "TXXX" ? new TextIdentificationFrame(data, header) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 7286d3c1..8a27a20e 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -338,11 +338,12 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const { const char *unsupportedFrames[] = { "ASPI", "EQU2", "RVA2", "SEEK", "SIGN", "TDRL", "TDTG", - "TMOO", "TPRO", "TSOA", "TSOT", "TSST", "TSOP", "TIPL", - "TMCL", 0 + "TMOO", "TPRO", "TSOA", "TSOT", "TSST", "TSOP", 0 }; ID3v2::TextIdentificationFrame *frameTDOR = 0; ID3v2::TextIdentificationFrame *frameTDRC = 0; + ID3v2::TextIdentificationFrame *frameTIPL = 0; + ID3v2::TextIdentificationFrame *frameTMCL = 0; for(FrameList::Iterator it = d->frameList.begin(); it != d->frameList.end(); it++) { ID3v2::Frame *frame = *it; ByteVector frameID = frame->header()->frameID(); @@ -362,6 +363,14 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const frameTDRC = dynamic_cast(frame); frame = 0; } + if(frame && frameID == "TIPL") { + frameTIPL = dynamic_cast(frame); + frame = 0; + } + if(frame && frameID == "TMCL") { + frameTMCL = dynamic_cast(frame); + frame = 0; + } if(frame) { frames->append(frame); } @@ -376,7 +385,6 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const } } if(frameTDRC) { - // FIXME we can do better here, define also TDAT and TIME String content = frameTDRC->toString(); if(content.size() >= 4) { ID3v2::TextIdentificationFrame *frameTYER = new ID3v2::TextIdentificationFrame("TYER", String::Latin1); @@ -397,7 +405,27 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const } } } - // FIXME migrate TIPL and TMCL to IPLS + if(frameTIPL || frameTMCL) { + ID3v2::TextIdentificationFrame *frameIPLS = new ID3v2::TextIdentificationFrame("IPLS", String::Latin1); + StringList people; + if(frameTMCL) { + StringList v24People = frameTMCL->fieldList(); + for(uint i = 0; i + 1 < v24People.size(); i += 2) { + people.append(v24People[i]); + people.append(v24People[i+1]); + } + } + if(frameTIPL) { + StringList v24People = frameTIPL->fieldList(); + for(uint i = 0; i + 1 < v24People.size(); i += 2) { + people.append(v24People[i]); + people.append(v24People[i+1]); + } + } + frameIPLS->setText(people); + frames->append(frameIPLS); + newFrames->append(frameIPLS); + } } diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index d3adc632..440e4f1e 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -488,6 +488,12 @@ public: tf = new ID3v2::TextIdentificationFrame("TDRC", String::Latin1); tf->setText("2012-04-17T12:01"); foo.ID3v2Tag()->addFrame(tf); + tf = new ID3v2::TextIdentificationFrame("TMCL", String::Latin1); + tf->setText(StringList().append("Guitar").append("Artist 1").append("Drums").append("Artist 2")); + foo.ID3v2Tag()->addFrame(tf); + tf = new ID3v2::TextIdentificationFrame("TIPL", String::Latin1); + tf->setText(StringList().append("Producer").append("Artist 3").append("Mastering").append("Artist 4")); + foo.ID3v2Tag()->addFrame(tf); foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDRL", String::Latin1)); foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TDTG", String::Latin1)); foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TMOO", String::Latin1)); @@ -507,6 +513,17 @@ public: CPPUNIT_ASSERT(tf); CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), tf->fieldList().size()); CPPUNIT_ASSERT_EQUAL(String("2012"), tf->fieldList().front()); + tf = dynamic_cast(bar.ID3v2Tag()->frameList("IPLS").front()); + CPPUNIT_ASSERT(tf); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(8), tf->fieldList().size()); + CPPUNIT_ASSERT_EQUAL(String("Guitar"), tf->fieldList()[0]); + CPPUNIT_ASSERT_EQUAL(String("Artist 1"), tf->fieldList()[1]); + CPPUNIT_ASSERT_EQUAL(String("Drums"), tf->fieldList()[2]); + CPPUNIT_ASSERT_EQUAL(String("Artist 2"), tf->fieldList()[3]); + CPPUNIT_ASSERT_EQUAL(String("Producer"), tf->fieldList()[4]); + CPPUNIT_ASSERT_EQUAL(String("Artist 3"), tf->fieldList()[5]); + CPPUNIT_ASSERT_EQUAL(String("Mastering"), tf->fieldList()[6]); + CPPUNIT_ASSERT_EQUAL(String("Artist 4"), tf->fieldList()[7]); CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TDRL")); CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TDTG")); CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TMOO")); From 3715b9647779b9956f649e6475af47e7bf2704fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Tue, 5 Apr 2011 15:36:23 +0200 Subject: [PATCH 7/9] Protect against incorrect ID3v2 version parameter --- taglib/mpeg/id3v2/id3v2tag.cpp | 5 +++++ tests/test_mpeg.cpp | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 8a27a20e..5b4c5c5b 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -438,6 +438,11 @@ ByteVector ID3v2::Tag::render(int version) const ByteVector tagData; + if(version != 3 && version != 4) { + debug("Unknown ID3v2 version, using ID3v2.4"); + version = 4; + } + // TODO: Render the extended header. // Loop through the frames rendering them and adding them to the tagData. diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index 745bcf6b..754caa98 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -13,6 +13,7 @@ class TestMPEG : public CppUnit::TestFixture CPPUNIT_TEST_SUITE(TestMPEG); CPPUNIT_TEST(testVersion2DurationWithXingHeader); CPPUNIT_TEST(testSaveID3v24); + CPPUNIT_TEST(testSaveID3v24WrongParam); CPPUNIT_TEST(testSaveID3v23); CPPUNIT_TEST_SUITE_END(); @@ -36,6 +37,24 @@ public: f.save(MPEG::File::AllTags, true, 4); MPEG::File f2(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f2.ID3v2Tag()->header()->majorVersion()); + CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist()); + CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title()); + } + + void testSaveID3v24WrongParam() + { + ScopedFileCopy copy("xing", ".mp3"); + string newname = copy.fileName(); + + String xxx = ByteVector(254, 'X'); + MPEG::File f(newname.c_str()); + f.tag()->setTitle(xxx); + f.tag()->setArtist("Artist A"); + f.save(MPEG::File::AllTags, true, 8); + + MPEG::File f2(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f2.ID3v2Tag()->header()->majorVersion()); CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist()); CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title()); } @@ -52,6 +71,7 @@ public: f.save(MPEG::File::AllTags, true, 3); MPEG::File f2(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(3), f2.ID3v2Tag()->header()->majorVersion()); CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist()); CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title()); } From 8878c9158cc7061c95d0054ab7a024ad9287a1be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Tue, 5 Apr 2011 17:08:25 +0200 Subject: [PATCH 8/9] Upgrade IPLS (2.3) to TIPL (2.4) --- taglib/mpeg/id3v2/id3v2framefactory.cpp | 3 ++- tests/test_id3v2.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp index 77bd85d5..e1850833 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.cpp +++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp @@ -153,7 +153,7 @@ Frame *FrameFactory::createFrame(const ByteVector &origData, Header *tagHeader) // Text Identification (frames 4.2) - if(frameID.startsWith("T") || frameID == "IPLS") { + if(frameID.startsWith("T")) { TextIdentificationFrame *f = frameID != "TXXX" ? new TextIdentificationFrame(data, header) @@ -368,6 +368,7 @@ bool FrameFactory::updateFrame(Frame::Header *header) const convertFrame("TORY", "TDOR", header); convertFrame("TYER", "TDRC", header); + convertFrame("IPLS", "TIPL", header); break; } diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 440e4f1e..8b8939b3 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -513,7 +513,7 @@ public: CPPUNIT_ASSERT(tf); CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), tf->fieldList().size()); CPPUNIT_ASSERT_EQUAL(String("2012"), tf->fieldList().front()); - tf = dynamic_cast(bar.ID3v2Tag()->frameList("IPLS").front()); + tf = dynamic_cast(bar.ID3v2Tag()->frameList("TIPL").front()); CPPUNIT_ASSERT(tf); CPPUNIT_ASSERT_EQUAL(TagLib::uint(8), tf->fieldList().size()); CPPUNIT_ASSERT_EQUAL(String("Guitar"), tf->fieldList()[0]); From 5eda17aa961950565e8aa2bfa301a3c12b8fae9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Wed, 6 Apr 2011 00:08:42 +0200 Subject: [PATCH 9/9] NEWS entries --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 5a9603a6..b157c457 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,9 @@ TagLib 1.8 (In Development) =========================== + * Support for writing ID3v2.3 tags. * Added methods for checking if WMA and MP4 files are DRM-protected. + * Started using atomic int operations for reference counting. TagLib 1.7 (Mar 11, 2011) =========================