From a8632f710f6dca2d9eefb5fa98a8e37ee2f6490e Mon Sep 17 00:00:00 2001 From: Michael Helmling Date: Sun, 22 Jan 2012 22:06:24 +0100 Subject: [PATCH] More progress in ID3 ... setProperties() will get messy :( --- .../id3v2/frames/textidentificationframe.cpp | 46 +++-- .../id3v2/frames/textidentificationframe.h | 13 ++ taglib/mpeg/id3v2/frames/urllinkframe.cpp | 6 +- taglib/mpeg/id3v2/id3v2frame.cpp | 8 +- taglib/mpeg/id3v2/id3v2tag.cpp | 162 ++++++++++++++++-- 5 files changed, 202 insertions(+), 33 deletions(-) diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp index 44528f33..f3aeb083 100644 --- a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp +++ b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp @@ -57,6 +57,17 @@ TextIdentificationFrame::TextIdentificationFrame(const ByteVector &data) : setData(data); } +TextIdentificationFrame *TextIdentificationFrame::createTIPLFrame(const PropertyMap &properties) // static +{ + TextIdentificationFrame* frame = TextIdentificationFrame("TIPL"); + StringList l; + for(PropertyMap::ConstIterator it = properties.begin(); it != properties.end(); ++it){ + l.append(it->first); + l.append(it->second.toString(",")); // comma-separated list of names + } + frame->setText(l); + return frame; +} TextIdentificationFrame::~TextIdentificationFrame() { delete d; @@ -92,6 +103,25 @@ void TextIdentificationFrame::setTextEncoding(String::Type encoding) d->textEncoding = encoding; } +// array of allowed TIPL prefixes and their corresponding key value +static const uint involvedPeopleSize = 5; +static const char* involvedPeople[2] = { + {"ARRANGER", "ARRANGER"}, + {"ENGINEER", "ENGINEER"}, + {"PRODUCER", "PRODUCER"}, + {"DJ-MIX", "DJMIXER"}, + {"MIX", "MIXER"} +}; + +const KeyConversionMap &TextIdentificationFrame::involvedPeopleMap() // static +{ + static KeyConversionMap m; + if(m.isEmpty()) + for(uint i = 0; i < involvedPeopleSize; ++i) + m.insert(involvedPeople[i][1], involvedPeople[i][0]); + return m; +} + PropertyMap TextIdentificationFrame::asProperties() const { if(frameID() == "TIPL") @@ -204,16 +234,6 @@ TextIdentificationFrame::TextIdentificationFrame(const ByteVector &data, Header parseFields(fieldData(data)); } -// array of allowed TIPL prefixes and their corresponding key value -static const uint involvedPeopleSize = 5; -static const char* involvedPeople[2] = { - {"ARRANGER", "ARRANGER"}, - {"ENGINEER", "ENGINEER"}, - {"PRODUCER", "PRODUCER"}, - {"DJ-MIX", "DJMIXER"}, - {"MIX", "MIXER"} -}; - PropertyMap TextIdentificationFrame::makeTIPLProperties() const { PropertyMap map; @@ -249,14 +269,14 @@ PropertyMap TextIdentificationFrame::makeTMCLProperties() const return map; } for(StringList::ConstIterator it = fieldList().begin(); it != fieldList().end(); ++it) { - String key = PropertyMap::prepareKey(*it); - if(key.isNull()) { + String instrument = PropertyMap::prepareKey(*it); + if(instrument.isNull()) { // instrument is not a valid key -> frame unsupported map.clear(); map.unsupportedData().append(frameID()); return map; } - map.insert(key, (++it).split(",")); + map.insert(L"PERFORMER:" + instrument, (++it).split(",")); } return map; } diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.h b/taglib/mpeg/id3v2/frames/textidentificationframe.h index f9348438..fc48a99f 100644 --- a/taglib/mpeg/id3v2/frames/textidentificationframe.h +++ b/taglib/mpeg/id3v2/frames/textidentificationframe.h @@ -36,6 +36,7 @@ namespace TagLib { namespace ID3v2 { class Tag; + typedef Map KeyConversionMap; //! An ID3v2 text identification frame implementation @@ -123,6 +124,13 @@ namespace TagLib { */ explicit TextIdentificationFrame(const ByteVector &data); + /*! + * This is a special factory method to create a TIPL (involved people list) + * frame from the given \a properties. Will parse key=[list of values] data + * into the TIPL format as specified in the ID3 standard. + */ + static TextIdentificationFrame *createTIPLFrame(const PropertyMap &properties); + /*! * Destroys this TextIdentificationFrame instance. */ @@ -173,6 +181,11 @@ namespace TagLib { */ StringList fieldList() const; + /*! + * Returns a KeyConversionMap mapping a role as it would be used in a PropertyMap + * to the corresponding key used in a TIPL ID3 frame to describe that role. + */ + static const KeyConversionMap &involvedPeopleMap(); PropertyMap asProperties() const; protected: diff --git a/taglib/mpeg/id3v2/frames/urllinkframe.cpp b/taglib/mpeg/id3v2/frames/urllinkframe.cpp index 32bfda97..b1800697 100644 --- a/taglib/mpeg/id3v2/frames/urllinkframe.cpp +++ b/taglib/mpeg/id3v2/frames/urllinkframe.cpp @@ -153,10 +153,12 @@ void UserUrlLinkFrame::setDescription(const String &s) PropertyMap UserUrlLinkFrame::asProperties() const { - String key = PropertyMap::prepareKey(description()); PropertyMap map; - if(key.isEmpty()) + String key; + if(description().isEmpty()) key = "URL"; + else + key = PropertyMap::prepareKey(description()); if(key.isNull()) map.unsupportedData().append(L"WXXX/" + description()); else diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 9ec536e0..bf33fa8e 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -263,7 +263,7 @@ String::Type Frame::checkTextEncoding(const StringList &fields, String::Type enc return checkEncoding(fields, encoding, header()->version()); } -static const uint frameTranslationSize = 53; +static const uint frameTranslationSize = 50; static const char *frameTranslation[][2] = { // Text information frames { "TALB", "ALBUM"}, @@ -323,10 +323,10 @@ static const char *frameTranslation[][2] = { { "WORS", "RADIOSTATIONWEBPAGE" }, { "WPAY", "PAYMENTWEBPAGE" }, { "WPUB", "PUBLISHERWEBPAGE" }, - { "WXXX", "URL"}, + //{ "WXXX", "URL"}, handled specially // Other frames - { "COMM", "COMMENT" }, - { "USLT", "LYRICS" }, + //{ "COMM", "COMMENT" }, handled specially + //{ "USLT", "LYRICS" }, handled specially }; Map &idMap() diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 0d7f5c8d..f9b5dfa8 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -342,27 +342,161 @@ PropertyMap ID3v2::Tag::properties() const return properties; } -PropertyMap ID3v2::Tag::setProperties(const PropertyMap &properties) +PropertyMap ID3v2::Tag::setProperties(const PropertyMap &origProps) { FrameList toRemove; + PropertyMap properties = origProps; // first find out what frames to remove; we do not remove in-place // because that would invalidate FrameListMap iterators. - // - for (FrameListMap::ConstIterator it = frameListMap().begin(); it != frameListMap().end(); ++it) - // Remove all frames which are not ignored - if (it->second.size() == 0 || !ignored(it->first) ) - toRemove.append(it->second); + // At the moment, we remove _all_ frames that don't contain unsupported data, + // and create new ones in the next step; this is to avoid clumsy technicalities + // arising when trying to do this more efficient. For example, if the current tag + // contains one URL attribute stored in an WXXX frame, but the given \a properties + // contain two URL values, we would need to remove the WXXX frame (which supports + // only one value), and create a TXXX frame with description=URL. + // The same may happen with COMM and USLT. Additionally, handling of TIPL and TMCL is + // complicated. + // In the future, someone might come up with a more clever sync algorithm. :-) + for(FrameListMap::ConstIterator it = frameListMap().begin(); it != frameListMap().end(); ++it) { + String key = Frame::frameIDToKey(it->first); + // for unsupported (binary) frame types, as well as frames that need special treatment + // (TXXX, WXXX, COMM, TMCL, TIPL, USLT), key will be String::null + if(key.isNull()) + continue; + // else: non-user text or url frame -> there should be only one frame of this type, + // and it's asProperties() method should return a PropertyMap with exactly one key + // and empty unsupportedData(). + if(it->second.size() != 1) + debug("invalid ID3 tag: found more than one " + it->first + " frame"); + PropertyMap frameMap = it->second[0]->asProperties(); + if(properties.contains(key) && frameMap[key] == properties[key]) + properties.erase(key); + else + toRemove.append(it->second[0]); + } - for (FrameList::ConstIterator it = toRemove.begin(); it != toRemove.end(); it++) + // now handle the special cases + // first: TXXX frames + FrameList &userTextFrames = frameList("TXXX"); + for(FrameList::ConstIterator it = userTextFrames.begin(); it != userTextFrames.end(); ++it) { + PropertyMap frameMap = (*it)->asProperties(); + if(!frameMap.unsupportedData().isEmpty()) + // don't touch unsupported frames + continue; + // TXXX frames yield only one key, so it must be begin()->first + String &key = frameMap.begin()->first; + if(!Frame::keyToFrameID(key).isNull()) + // TXXX frame which a description (=key) for which there is a dedicated frame. + // We don't want this, so remove the frame, the appropriate T*** or W*** frame + // will be created later on. + toRemove.append(*it); + if(key.find(":") > 0) + // colon-separated key: this should be inside a TMCL frame. + toRemove.append(*it); + // More (ugly) exceptions: If the user provides more than one COMMENT, LYRICS, or URL + // tag, we store all of these in a TXXX, because COMM, USLT and WXXX. Otherwise there + // should not be such a TXXX frame. + if(key == "COMMENT") { + if(properties.contains("COMMENT") + && properties["COMMENT"].size() >= 2 + && properties["COMMENT"] == frameMap.begin()->second) + properties.erase("COMMENT"); + else + toRemove.append(*it); + }else if(key == "LYRICS") { + if(properties.contains("LYRICS") + && properties["LYRICS"].size() >= 2 + && properties["LYRICS"] == frameMap.begin()->second) + properties.erase("LYRICS"); + else + toRemove.append(*it); + }else if(key == "URL") { + if(properties.contains("URL") + && properties["URL"].size() >= 2 + && properties["URL"] == frameMap.begin()->second) + properties.erase("URL"); + else + toRemove.append(*it); + } + } + + // next: WXXX frames + FrameList &userUrlFrames = frameList("WXXX"); + for(FrameList::ConstIterator it = userUrlFrames.begin(); it != userUrlFrames.end(); ++it) { + PropertyMap frameMap = (*it)->asProperties(); + if(!frameMap.unsupportedData().isEmpty()) + // don't touch unsupported frames + continue; + // WXXX frames yield only one key, so it must be begin()->first + String &key = frameMap.begin()->first; + if(!Frame::keyToFrameID(key).isNull()) + // WXXX frame which a description (=key) for which there is a dedicated frame. + // We don't want this, so remove the frame, the appropriate T*** or W*** frame + // will be created later on. + toRemove.append(*it); + else if(key.find(":") > 0) + // colon-separated key: this should be inside a TMCL frame. + toRemove.append(*it); + // More exceptions: we don't allow COMMENT and LYRICS in WXXX; they should be in COMM and USLT + // (or TXXX, see above). + else if(key == "COMMENT" || key == "LYRICS") + toRemove.append(*it); + // now, the key is either URL or some other string that neither has a dedicated text frame, nor + // a colon. We accept the frame if it's contents match the values in properties. However, if + // key != URL and the values are changed, they will be stored inside a TXXX frame instead, since + // we can't distinguish free-form text from free-form URL keys (possible fix: use URL:REASON like + // in TMCL / TIPL?). + else if(properties.contains(key) && properties[key] == frameMap.begin()->second) + properties.erase(key); + else + toRemove.append(*it); + } + for(FrameList::ConstIterator it = toRemove.begin(); it != toRemove.end(); ++it) removeFrame(*it); - // now create new frames from the TagDict and add them. - for (TagDict::ConstIterator it = dict.begin(); it != dict.end(); ++it) { - Frame *newFrame = createFrame(it->first, it->second); - if (newFrame) - addFrame(newFrame); - else - debug("ERROR: Don't know how to translate tag " + it->first + " to ID3v2!"); + // next: TIPL + PropertyMap existingTipl; + if(!frameList("TIPL").isEmpty()) + existingTipl = frameList("TIPL").front()->asProperties(); + PropertyMap requestedTipl; + KeyConversionMap::ConstIterator it = TextIdentificationFrame::involvedPeopleMap().begin(); + bool rebuildTipl = false; + for(; it != TextIdentificationFrame::involvedPeopleMap().end(); ++it) { + if(properties.contains(it->first)){ + requestedTipl.insert(it->first, properties[it->first]); + properties.erase(it->first); // it's ensured now that this key gets handled correctly + if(!existingTipl.contains(it->first) || existingTipl[it->first] != requestedTipl[it->first]) + rebuildTipl = true; + } else if(existingTipl.contains(it->first)) + rebuildTipl = true; + } + if(rebuildTipl){ + removeFrames("TIPL"); + addFrame(TextIdentificationFrame::createTIPLFrame(requestedTipl)); + } + + // next: create frames for everything still in properties except for TMCL ("PERFORMER:") + // keys, which are collected in a dedicated map + PropertyMap requestedTmcl; + for(PropertyMap::ConstIterator it = properties.begin(); it != properties.end(); ++it){ + if(it->first.find(":") != -1) + requestedTmcl.insert(it->first, it->second); + else{ + // phew. Now we are in the simple case that our key= pair can be represented by a + // single frame, either a T*** (not TIPL, TMCL) or W*** frame. + ByteVector id = Frame::keyToFrameID(it->first); + } + } + + // next: TMCL + PropertyMap existingTmcl; + if(!frameList("TMCL").isEmpty()) + existingTmcl = frameList("TMCL").front()->asProperties(); + bool rebuildTmcl = false; + // search for TMCL keys ("PERFORMER:") in properties + for(PropertyMap::ConstIterator it = properties.begin(); it != properties.end(); ++it){ + if(it->first.find(":") != -1) + requestedTmcl.insert(it->first, it->second); } }