From bba562b5570de7815437044b404ca185dcf46669 Mon Sep 17 00:00:00 2001 From: Scott Wheeler Date: Mon, 18 May 2015 21:18:33 +0200 Subject: [PATCH] Add accessors to manipulate MP4 tags without modifying the internal structure This brings the MP4 API in line closer to other tag formats and makes it possible to access the tag data from const functions. "ItemListMap" has been renamed to "ItemMap" (with the old version deprecated). It seems that the "ListMap" notion was copied probably from Allan's ApeTag implementation, which incorrectly copied the term from the XiphTag. Notably, in XiphTag, because a field can have multiple values, the "ListMap" is a map of lists. Calling things a "ListMap" where there can be only one value doesn't fit. Closes #255 --- taglib/mp4/mp4tag.cpp | 34 ++++++++++++++++++--- taglib/mp4/mp4tag.h | 35 ++++++++++++++++++++- tests/test_mp4.cpp | 71 ++++++++++++++++++++++--------------------- 3 files changed, 100 insertions(+), 40 deletions(-) diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index 08e5f411..8e58a1d3 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -39,7 +39,7 @@ public: ~TagPrivate() {} TagLib::File *file; Atoms *atoms; - ItemListMap items; + ItemMap items; }; MP4::Tag::Tag() @@ -451,7 +451,7 @@ bool MP4::Tag::save() { ByteVector data; - for(MP4::ItemListMap::Iterator i = d->items.begin(); i != d->items.end(); i++) { + for(MP4::ItemMap::Iterator i = d->items.begin(); i != d->items.end(); i++) { const String name = i->first; if(name.startsWith("----")) { data.append(renderFreeForm(name, i->second)); @@ -765,12 +765,36 @@ bool MP4::Tag::isEmpty() const return d->items.isEmpty(); } -MP4::ItemListMap & -MP4::Tag::itemListMap() +MP4::ItemMap &MP4::Tag::itemListMap() { return d->items; } +const MP4::ItemMap &MP4::Tag::itemMap() const +{ + return d->items; +} + +MP4::Item MP4::Tag::item(const String &key) const +{ + return d->items[key]; +} + +void MP4::Tag::setItem(const String &key, const Item &value) +{ + d->items[key] = value; +} + +void MP4::Tag::removeItem(const String &key) +{ + d->items.erase(key); +} + +bool MP4::Tag::contains(const String &key) const +{ + return d->items.contains(key); +} + static const char *keyTranslation[][2] = { { "\251nam", "TITLE" }, { "\251ART", "ARTIST" }, @@ -832,7 +856,7 @@ PropertyMap MP4::Tag::properties() const } PropertyMap props; - MP4::ItemListMap::ConstIterator it = d->items.begin(); + MP4::ItemMap::ConstIterator it = d->items.begin(); for(; it != d->items.end(); ++it) { if(keyMap.contains(it->first)) { String key = keyMap[it->first]; diff --git a/taglib/mp4/mp4tag.h b/taglib/mp4/mp4tag.h index cde68964..f74cb7cd 100644 --- a/taglib/mp4/mp4tag.h +++ b/taglib/mp4/mp4tag.h @@ -39,7 +39,11 @@ namespace TagLib { namespace MP4 { + /*! + * \deprecated + */ typedef TagLib::Map ItemListMap; + typedef TagLib::Map ItemMap; class TAGLIB_EXPORT Tag: public TagLib::Tag { @@ -67,7 +71,36 @@ namespace TagLib { virtual bool isEmpty() const; - ItemListMap &itemListMap(); + /*! + * \deprecated Use the item() and setItem() API instead + */ + ItemMap &itemListMap(); + + /*! + * Returns a string-keyed map of the MP4::Items for this tag. + */ + const ItemMap &itemMap() const; + + /*! + * \return The item, if any, corresponding to \a key. + */ + Item item(const String &key) const; + + /*! + * Sets the value of \a key to \a value, overwriting any previous value. + */ + void setItem(const String &key, const Item &value); + + /*! + * Removes the entry with \a key from the tag, or does nothing if it does + * not exist. + */ + void removeItem(const String &key); + + /*! + * \return True if the tag contains an entry for \a key. + */ + bool contains(const String &key) const; PropertyMap properties() const; void removeUnsupportedProperties(const StringList& properties); diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index 9bd2deb7..dd6e3a03 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -71,7 +71,7 @@ public: CPPUNIT_ASSERT(!t1.isEmpty()); MP4::Tag t2; - t2.itemListMap()["foo"] = "bar"; + t2.setItem("foo", "bar"); CPPUNIT_ASSERT(!t2.isEmpty()); } @@ -128,14 +128,15 @@ public: string filename = copy.fileName(); MP4::File *f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT(f->tag()->itemListMap().contains("----:com.apple.iTunes:iTunNORM")); - f->tag()->itemListMap()["----:org.kde.TagLib:Foo"] = StringList("Bar"); + CPPUNIT_ASSERT(f->tag()->contains("----:com.apple.iTunes:iTunNORM")); + f->tag()->setItem("----:org.kde.TagLib:Foo", StringList("Bar")); f->save(); delete f; f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT(f->tag()->itemListMap().contains("----:org.kde.TagLib:Foo")); - CPPUNIT_ASSERT_EQUAL(String("Bar"), f->tag()->itemListMap()["----:org.kde.TagLib:Foo"].toStringList()[0]); + CPPUNIT_ASSERT(f->tag()->contains("----:org.kde.TagLib:Foo")); + CPPUNIT_ASSERT_EQUAL(String("Bar"), + f->tag()->item("----:org.kde.TagLib:Foo").toStringList().front()); f->save(); delete f; } @@ -146,14 +147,16 @@ public: string filename = copy.fileName(); MP4::File *f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT_EQUAL(String("82,164"), f->tag()->itemListMap()["----:com.apple.iTunes:replaygain_track_minmax"].toStringList()[0]); + CPPUNIT_ASSERT_EQUAL(String("82,164"), + f->tag()->item("----:com.apple.iTunes:replaygain_track_minmax").toStringList().front()); CPPUNIT_ASSERT_EQUAL(String("Pearl Jam"), f->tag()->artist()); f->tag()->setComment("foo"); f->save(); delete f; f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT_EQUAL(String("82,164"), f->tag()->itemListMap()["----:com.apple.iTunes:replaygain_track_minmax"].toStringList()[0]); + CPPUNIT_ASSERT_EQUAL(String("82,164"), + f->tag()->item("----:com.apple.iTunes:replaygain_track_minmax").toStringList().front()); CPPUNIT_ASSERT_EQUAL(String("Pearl Jam"), f->tag()->artist()); CPPUNIT_ASSERT_EQUAL(String("foo"), f->tag()->comment()); delete f; @@ -165,20 +168,20 @@ public: string filename = copy.fileName(); MP4::File *f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT_EQUAL(true, f->tag()->itemListMap()["cpil"].toBool()); + CPPUNIT_ASSERT_EQUAL(true, f->tag()->itemMap()["cpil"].toBool()); MP4::Atoms *atoms = new MP4::Atoms(f); MP4::Atom *moov = atoms->atoms[0]; CPPUNIT_ASSERT_EQUAL(long(77), moov->length); - f->tag()->itemListMap()["pgap"] = true; + f->tag()->setItem("pgap", true); f->save(); delete atoms; delete f; f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT_EQUAL(true, f->tag()->itemListMap()["cpil"].toBool()); - CPPUNIT_ASSERT_EQUAL(true, f->tag()->itemListMap()["pgap"].toBool()); + CPPUNIT_ASSERT_EQUAL(true, f->tag()->item("cpil").toBool()); + CPPUNIT_ASSERT_EQUAL(true, f->tag()->item("pgap").toBool()); atoms = new MP4::Atoms(f); moov = atoms->atoms[0]; @@ -198,8 +201,8 @@ public: void testCovrRead() { MP4::File *f = new MP4::File(TEST_FILE_PATH_C("has-tags.m4a")); - CPPUNIT_ASSERT(f->tag()->itemListMap().contains("covr")); - MP4::CoverArtList l = f->tag()->itemListMap()["covr"].toCoverArtList(); + CPPUNIT_ASSERT(f->tag()->contains("covr")); + MP4::CoverArtList l = f->tag()->item("covr").toCoverArtList(); CPPUNIT_ASSERT_EQUAL(TagLib::uint(2), l.size()); CPPUNIT_ASSERT_EQUAL(MP4::CoverArt::PNG, l[0].format()); CPPUNIT_ASSERT_EQUAL(TagLib::uint(79), l[0].data().size()); @@ -214,16 +217,16 @@ public: string filename = copy.fileName(); MP4::File *f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT(f->tag()->itemListMap().contains("covr")); - MP4::CoverArtList l = f->tag()->itemListMap()["covr"].toCoverArtList(); + CPPUNIT_ASSERT(f->tag()->contains("covr")); + MP4::CoverArtList l = f->tag()->item("covr").toCoverArtList(); l.append(MP4::CoverArt(MP4::CoverArt::PNG, "foo")); - f->tag()->itemListMap()["covr"] = l; + f->tag()->setItem("covr", l); f->save(); delete f; f = new MP4::File(filename.c_str()); - CPPUNIT_ASSERT(f->tag()->itemListMap().contains("covr")); - l = f->tag()->itemListMap()["covr"].toCoverArtList(); + CPPUNIT_ASSERT(f->tag()->contains("covr")); + l = f->tag()->item("covr").toCoverArtList(); CPPUNIT_ASSERT_EQUAL(TagLib::uint(3), l.size()); CPPUNIT_ASSERT_EQUAL(MP4::CoverArt::PNG, l[0].format()); CPPUNIT_ASSERT_EQUAL(TagLib::uint(79), l[0].data().size()); @@ -237,8 +240,8 @@ public: void testCovrRead2() { MP4::File *f = new MP4::File(TEST_FILE_PATH_C("covr-junk.m4a")); - CPPUNIT_ASSERT(f->tag()->itemListMap().contains("covr")); - MP4::CoverArtList l = f->tag()->itemListMap()["covr"].toCoverArtList(); + CPPUNIT_ASSERT(f->tag()->contains("covr")); + MP4::CoverArtList l = f->tag()->item("covr").toCoverArtList(); CPPUNIT_ASSERT_EQUAL(TagLib::uint(2), l.size()); CPPUNIT_ASSERT_EQUAL(MP4::CoverArt::PNG, l[0].format()); CPPUNIT_ASSERT_EQUAL(TagLib::uint(79), l[0].data().size()); @@ -264,26 +267,26 @@ public: tags = f.properties(); - CPPUNIT_ASSERT(f.tag()->itemListMap().contains("trkn")); - CPPUNIT_ASSERT_EQUAL(2, f.tag()->itemListMap()["trkn"].toIntPair().first); - CPPUNIT_ASSERT_EQUAL(4, f.tag()->itemListMap()["trkn"].toIntPair().second); + CPPUNIT_ASSERT(f.tag()->contains("trkn")); + CPPUNIT_ASSERT_EQUAL(2, f.tag()->item("trkn").toIntPair().first); + CPPUNIT_ASSERT_EQUAL(4, f.tag()->item("trkn").toIntPair().second); CPPUNIT_ASSERT_EQUAL(StringList("2/4"), tags["TRACKNUMBER"]); - CPPUNIT_ASSERT(f.tag()->itemListMap().contains("disk")); - CPPUNIT_ASSERT_EQUAL(3, f.tag()->itemListMap()["disk"].toIntPair().first); - CPPUNIT_ASSERT_EQUAL(5, f.tag()->itemListMap()["disk"].toIntPair().second); + CPPUNIT_ASSERT(f.tag()->contains("disk")); + CPPUNIT_ASSERT_EQUAL(3, f.tag()->item("disk").toIntPair().first); + CPPUNIT_ASSERT_EQUAL(5, f.tag()->item("disk").toIntPair().second); CPPUNIT_ASSERT_EQUAL(StringList("3/5"), tags["DISCNUMBER"]); - CPPUNIT_ASSERT(f.tag()->itemListMap().contains("tmpo")); - CPPUNIT_ASSERT_EQUAL(123, f.tag()->itemListMap()["tmpo"].toInt()); + CPPUNIT_ASSERT(f.tag()->contains("tmpo")); + CPPUNIT_ASSERT_EQUAL(123, f.tag()->item("tmpo").toInt()); CPPUNIT_ASSERT_EQUAL(StringList("123"), tags["BPM"]); - CPPUNIT_ASSERT(f.tag()->itemListMap().contains("\251ART")); - CPPUNIT_ASSERT_EQUAL(StringList("Foo Bar"), f.tag()->itemListMap()["\251ART"].toStringList()); + CPPUNIT_ASSERT(f.tag()->contains("\251ART")); + CPPUNIT_ASSERT_EQUAL(StringList("Foo Bar"), f.tag()->item("\251ART").toStringList()); CPPUNIT_ASSERT_EQUAL(StringList("Foo Bar"), tags["ARTIST"]); - CPPUNIT_ASSERT(f.tag()->itemListMap().contains("cpil")); - CPPUNIT_ASSERT_EQUAL(true, f.tag()->itemListMap()["cpil"].toBool()); + CPPUNIT_ASSERT(f.tag()->contains("cpil")); + CPPUNIT_ASSERT_EQUAL(true, f.tag()->item("cpil").toBool()); CPPUNIT_ASSERT_EQUAL(StringList("1"), tags["COMPILATION"]); tags["COMPILATION"] = StringList("0"); @@ -291,8 +294,8 @@ public: tags = f.properties(); - CPPUNIT_ASSERT(f.tag()->itemListMap().contains("cpil")); - CPPUNIT_ASSERT_EQUAL(false, f.tag()->itemListMap()["cpil"].toBool()); + CPPUNIT_ASSERT(f.tag()->contains("cpil")); + CPPUNIT_ASSERT_EQUAL(false, f.tag()->item("cpil").toBool()); CPPUNIT_ASSERT_EQUAL(StringList("0"), tags["COMPILATION"]); } -- 2.40.0