From f5a25182734ba0f15b698f8dc4cc29509df89293 Mon Sep 17 00:00:00 2001 From: Michael Helmling Date: Sun, 26 Feb 2012 19:21:57 +0100 Subject: [PATCH] Fixed handling of UnknownFrames in ID3v2. - If an unknown frame with id "XXXX" occurs, an entry "UNKNOWN/XXXX" is added to unsupportedData(). The removeUnsupportedProperties() method in turn removes all unknown frames with id "XXXX" if it encounters a string "UNKNOWN/XXXX" in the given list. - Implemented findByDescription() to UnsynchronizedLyricsFrame in order to support removal of lyrics frames with unsupported keys. - Adapted id3v2 test case to new QuodLibet policy. --- .../frames/unsynchronizedlyricsframe.cpp | 12 +++++ .../id3v2/frames/unsynchronizedlyricsframe.h | 9 ++++ taglib/mpeg/id3v2/id3v2frame.cpp | 19 ++++---- taglib/mpeg/id3v2/id3v2tag.cpp | 47 ++++++++++--------- taglib/mpeg/id3v2/id3v2tag.h | 11 +++-- tests/test_id3v2.cpp | 9 +++- 6 files changed, 69 insertions(+), 38 deletions(-) diff --git a/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp b/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp index 6719a9b3..9d76164d 100644 --- a/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp +++ b/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp @@ -27,6 +27,7 @@ #include "unsynchronizedlyricsframe.h" #include +#include #include #include @@ -125,6 +126,17 @@ PropertyMap UnsynchronizedLyricsFrame::asProperties() const return map; } +UnsynchronizedLyricsFrame *UnsynchronizedLyricsFrame::findByDescription(const ID3v2::Tag *tag, const String &d) // static +{ + ID3v2::FrameList lyrics = tag->frameList("USLT"); + + for(ID3v2::FrameList::ConstIterator it = lyrics.begin(); it != lyrics.end(); ++it){ + UnsynchronizedLyricsFrame *frame = dynamic_cast(*it); + if(frame && frame->description() == d) + return frame; + } + return 0; +} //////////////////////////////////////////////////////////////////////////////// // protected members //////////////////////////////////////////////////////////////////////////////// diff --git a/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.h b/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.h index 2a3c5f9a..3af354fc 100644 --- a/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.h +++ b/taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.h @@ -147,6 +147,15 @@ namespace TagLib { */ PropertyMap asProperties() const; + /*! + * LyricsFrames each have a unique description. This searches for a lyrics + * frame with the decription \a d and returns a pointer to it. If no + * frame is found that matches the given description null is returned. + * + * \see description() + */ + static UnsynchronizedLyricsFrame *findByDescription(const Tag *tag, const String &d); + protected: // Reimplementations. diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index ed023123..6b45f223 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -43,6 +43,7 @@ #include "frames/urllinkframe.h" #include "frames/unsynchronizedlyricsframe.h" #include "frames/commentsframe.h" +#include "frames/unknownframe.h" using namespace TagLib; using namespace ID3v2; @@ -429,18 +430,16 @@ ByteVector Frame::keyToFrameID(const String &s) PropertyMap Frame::asProperties() const { + if(dynamic_cast< const UnknownFrame *>(this)) { + PropertyMap m; + m.unsupportedData().append("UNKNOWN/" + frameID()); + return m; + } const ByteVector &id = frameID(); // workaround until this function is virtual - if(id == "TXXX") { - const UserTextIdentificationFrame *txxxFrame = dynamic_cast< const UserTextIdentificationFrame* >(this); - if(txxxFrame != NULL) - return txxxFrame->asProperties(); - else { - PropertyMap m; - m.unsupportedData().append("UNKNOWN"); - return m; - } - } else if(id[0] == 'T') + if(id == "TXXX") + return dynamic_cast< const UserTextIdentificationFrame* >(this)->asProperties(); + else if(id[0] == 'T') return dynamic_cast< const TextIdentificationFrame* >(this)->asProperties(); else if(id == "WXXX") return dynamic_cast< const UserUrlLinkFrame* >(this)->asProperties(); diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 49c0c517..ce228330 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -346,35 +346,36 @@ PropertyMap ID3v2::Tag::properties() const void ID3v2::Tag::removeUnsupportedProperties(const StringList &properties) { - // entries of unsupportedData() are usually frame IDs which are not supported - // by the PropertyMap interface. Three special cases exist: TXXX, WXXX, and COMM - // frames may also be unsupported if their description() is not a valid key. for(StringList::ConstIterator it = properties.begin(); it != properties.end(); ++it){ - if(*it == "UNKNOWN") { - // delete all unknown frames - FrameList l = frameList(); + if(it->startsWith("UNKNOWN/")) { + String frameID = it->substr(String("UNKNOWN/").size()); + if(frameID.size() != 4) + continue; // invalid specification + ByteVector id = frameID.data(String::Latin1); + // delete all unknown frames of given type + FrameList l = frameList(id); for(FrameList::ConstIterator fit = l.begin(); fit != l.end(); fit++) if (dynamic_cast(*fit) != NULL) removeFrame(*fit); + } else if(it->size() == 4){ + ByteVector id = it->data(String::Latin1); + removeFrames(id); } else { ByteVector id = it->substr(0,4).data(String::Latin1); - if(id == "TXXX") { - String description = it->substr(5); - Frame *frame = UserTextIdentificationFrame::find(this, description); - if(frame) - removeFrame(frame); - } else if(id == "WXXX") { - String description = it->substr(5); - Frame *frame = UserUrlLinkFrame::find(this, description); - if(frame) - removeFrame(frame); - } else if(id == "COMM") { - String description = it->substr(5); - Frame *frame = CommentsFrame::findByDescription(this, description); - if(frame) - removeFrame(frame); - } else - removeFrames(id); // there should be only one frame with "id" + if(it->size() <= 5) + continue; // invalid specification + String description = it->substr(5); + Frame *frame; + if(id == "TXXX") + frame = UserTextIdentificationFrame::find(this, description); + else if(id == "WXXX") + frame = UserUrlLinkFrame::find(this, description); + else if(id == "COMM") + frame = CommentsFrame::findByDescription(this, description); + else if(id == "USLT") + frame = UnsynchronizedLyricsFrame::findByDescription(this, description); + if(frame) + removeFrame(frame); } } } diff --git a/taglib/mpeg/id3v2/id3v2tag.h b/taglib/mpeg/id3v2/id3v2tag.h index 3728868a..94784e76 100644 --- a/taglib/mpeg/id3v2/id3v2tag.h +++ b/taglib/mpeg/id3v2/id3v2tag.h @@ -293,9 +293,14 @@ namespace TagLib { /*! * Removes unsupported frames given by \a properties. The elements of - * \a properties must be taken from properties().unsupportedData() and - * are the four-byte frame IDs of ID3 frames which are not compatible - * with the PropertyMap schema. + * \a properties must be taken from properties().unsupportedData(); they + * are of one of the following forms: + * - a four-character frame ID, if the ID3 specification allows only one + * frame with that ID (thus, the frame is uniquely determined) + * - frameID + "/" + description(), when the ID is one of "TXXX", "WXXX", + * "COMM", or "USLT", + * - "UNKNOWN/" + frameID, for frames that could not be parsed by TagLib. + * In that case, *all* unknown frames with the given ID will be removed. */ void removeUnsupportedProperties(const StringList &properties); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 530b3f3d..49ffef0d 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -558,10 +558,15 @@ public: MPEG::File f(newname.c_str()); PropertyMap dict = f.ID3v2Tag(false)->properties(); CPPUNIT_ASSERT_EQUAL(uint(6), dict.size()); + + CPPUNIT_ASSERT(dict.contains("USERTEXTDESCRIPTION1")); + CPPUNIT_ASSERT(dict.contains("QuodLibet::USERTEXTDESCRIPTION2")); + CPPUNIT_ASSERT_EQUAL(uint(2), dict["USERTEXTDESCRIPTION1"].size()); + CPPUNIT_ASSERT_EQUAL(uint(2), dict["QuodLibet::USERTEXTDESCRIPTION2"].size()); CPPUNIT_ASSERT_EQUAL(String("userTextData1"), dict["USERTEXTDESCRIPTION1"][0]); CPPUNIT_ASSERT_EQUAL(String("userTextData2"), dict["USERTEXTDESCRIPTION1"][1]); - CPPUNIT_ASSERT_EQUAL(String("userTextData1"), dict["USERTEXTDESCRIPTION2"][0]); - CPPUNIT_ASSERT_EQUAL(String("userTextData2"), dict["USERTEXTDESCRIPTION2"][1]); + CPPUNIT_ASSERT_EQUAL(String("userTextData1"), dict["QuodLibet::USERTEXTDESCRIPTION2"][0]); + CPPUNIT_ASSERT_EQUAL(String("userTextData2"), dict["QuodLibet::USERTEXTDESCRIPTION2"][1]); CPPUNIT_ASSERT_EQUAL(String("Pop"), dict["GENRE"].front());