From d47d28f0f8a296b177f6c6239cf08ce796c45b92 Mon Sep 17 00:00:00 2001 From: Urs Fleisch Date: Fri, 22 Aug 2025 08:55:51 +0200 Subject: [PATCH] Fix and simplify Matroska simple tag Avoid use of raw pointers, fix property interface. --- examples/matroskareader.cpp | 18 +- examples/matroskawriter.cpp | 17 +- taglib/matroska/ebml/ebmlmktags.cpp | 38 +--- taglib/matroska/matroskasimpletag.cpp | 126 +++++------ taglib/matroska/matroskasimpletag.h | 136 ++++++++---- taglib/matroska/matroskatag.cpp | 305 +++++++++++++------------- taglib/matroska/matroskatag.h | 8 +- 7 files changed, 336 insertions(+), 312 deletions(-) diff --git a/examples/matroskareader.cpp b/examples/matroskareader.cpp index 93a7a3c4..453d583a 100644 --- a/examples/matroskareader.cpp +++ b/examples/matroskareader.cpp @@ -30,23 +30,21 @@ int main(int argc, char *argv[]) const TagLib::Matroska::SimpleTagsList &list = tag->simpleTagsList(); printf("Found %u tag(s):\n", list.size()); - for(TagLib::Matroska::SimpleTag *t : list) { - PRINT_PRETTY("Tag Name", t->name().toCString(true)); + for(const TagLib::Matroska::SimpleTag &t : list) { + PRINT_PRETTY("Tag Name", t.name().toCString(true)); - TagLib::Matroska::SimpleTagString *tString = nullptr; - TagLib::Matroska::SimpleTagBinary *tBinary = nullptr; - if((tString = dynamic_cast(t))) - PRINT_PRETTY("Tag Value", tString->value().toCString(true)); - else if((tBinary = dynamic_cast(t))) + if(t.type() == TagLib::Matroska::SimpleTag::StringType) + PRINT_PRETTY("Tag Value", t.toString().toCString(true)); + else if(t.type() == TagLib::Matroska::SimpleTag::BinaryType) PRINT_PRETTY("Tag Value", - TagLib::Utils::formatString("Binary with size %i", tBinary->value().size()).toCString(false) + TagLib::Utils::formatString("Binary with size %i", t.toByteVector().size()).toCString(false) ); - auto targetTypeValue = static_cast(t->targetTypeValue()); + auto targetTypeValue = static_cast(t.targetTypeValue()); PRINT_PRETTY("Target Type Value", targetTypeValue == 0 ? "None" : TagLib::Utils::formatString("%i", targetTypeValue).toCString(false) ); - const TagLib::String &language = t->language(); + const TagLib::String &language = t.language(); PRINT_PRETTY("Language", !language.isEmpty() ? language.toCString(false) : "Not set"); printf("\n"); diff --git a/examples/matroskawriter.cpp b/examples/matroskawriter.cpp index a5f762fc..70ad8a80 100644 --- a/examples/matroskawriter.cpp +++ b/examples/matroskawriter.cpp @@ -22,18 +22,13 @@ int main(int argc, char *argv[]) auto tag = file.tag(true); tag->clearSimpleTags(); - auto simpleTag = new TagLib::Matroska::SimpleTagString(); - simpleTag->setName("Test Name 1"); - simpleTag->setTargetTypeValue(TagLib::Matroska::SimpleTag::TargetTypeValue::Track); - simpleTag->setValue("Test Value 1"); - simpleTag->setLanguage("en"); - tag->addSimpleTag(simpleTag); + tag->addSimpleTag(TagLib::Matroska::SimpleTag( + "Test Name 1", TagLib::String("Test Value 1"), + TagLib::Matroska::SimpleTag::TargetTypeValue::Track, "en")); - simpleTag = new TagLib::Matroska::SimpleTagString(); - simpleTag->setName("Test Name 2"); - simpleTag->setTargetTypeValue(TagLib::Matroska::SimpleTag::TargetTypeValue::Album); - simpleTag->setValue("Test Value 2"); - tag->addSimpleTag(simpleTag); + tag->addSimpleTag(TagLib::Matroska::SimpleTag( + "Test Name 2", TagLib::String("Test Value 2"), + TagLib::Matroska::SimpleTag::TargetTypeValue::Album)); tag->setTitle("Test title"); tag->setArtist("Test artist"); tag->setYear(1969); diff --git a/taglib/matroska/ebml/ebmlmktags.cpp b/taglib/matroska/ebml/ebmlmktags.cpp index a7dd5709..bbfec2c6 100644 --- a/taglib/matroska/ebml/ebmlmktags.cpp +++ b/taglib/matroska/ebml/ebmlmktags.cpp @@ -68,47 +68,33 @@ std::unique_ptr EBML::MkTags::parse() // Parse each for(auto simpleTag : simpleTags) { - const String *tagName = nullptr; const String *tagValueString = nullptr; const ByteVector *tagValueBinary = nullptr; - const String *language = nullptr; + String tagName; + String language; bool defaultLanguageFlag = true; for(const auto &simpleTagChild : *simpleTag) { Id id = simpleTagChild->getId(); - if(id == Id::MkTagName && !tagName) - tagName = &(element_cast(simpleTagChild)->getValue()); + if(id == Id::MkTagName && tagName.isEmpty()) + tagName = element_cast(simpleTagChild)->getValue(); else if(id == Id::MkTagString && !tagValueString) tagValueString = &(element_cast(simpleTagChild)->getValue()); else if(id == Id::MkTagBinary && !tagValueBinary) tagValueBinary = &(element_cast(simpleTagChild)->getValue()); - else if(id == Id::MkTagsTagLanguage && !language) - language = &(element_cast(simpleTagChild)->getValue()); + else if(id == Id::MkTagsTagLanguage && language.isEmpty()) + language = element_cast(simpleTagChild)->getValue(); else if(id == Id::MkTagsLanguageDefault) defaultLanguageFlag = element_cast(simpleTagChild)->getValue() ? true : false; } - if(!tagName || (tagValueString && tagValueBinary) || (!tagValueString && !tagValueBinary)) + if(tagName.isEmpty() || (tagValueString && tagValueBinary) || (!tagValueString && !tagValueBinary)) continue; - // Create a Simple Tag object and add it to the Tag object - Matroska::SimpleTag *sTag = nullptr; - if(tagValueString) { - auto sTagString = new Matroska::SimpleTagString(); - sTagString->setTargetTypeValue(targetTypeValue); - sTagString->setValue(*tagValueString); - sTag = sTagString; - } - else { // tagValueBinary must be non null - auto sTagBinary = new Matroska::SimpleTagBinary(); - sTagBinary->setTargetTypeValue(targetTypeValue); - sTagBinary->setValue(*tagValueBinary); - sTag = sTagBinary; - } - sTag->setName(*tagName); - if(language) - sTag->setLanguage(*language); - sTag->setDefaultLanguageFlag(defaultLanguageFlag); - mTag->addSimpleTag(sTag); + mTag->addSimpleTag(tagValueString + ? Matroska::SimpleTag(tagName, *tagValueString, + targetTypeValue, language, defaultLanguageFlag) + : Matroska::SimpleTag(tagName, *tagValueBinary, + targetTypeValue, language, defaultLanguageFlag)); } } return mTag; diff --git a/taglib/matroska/matroskasimpletag.cpp b/taglib/matroska/matroskasimpletag.cpp index db0b0979..43fb0fc5 100644 --- a/taglib/matroska/matroskasimpletag.cpp +++ b/taglib/matroska/matroskasimpletag.cpp @@ -19,6 +19,7 @@ ***************************************************************************/ #include "matroskasimpletag.h" +#include #include "matroskatag.h" #include "tstring.h" #include "tbytevector.h" @@ -28,43 +29,68 @@ using namespace TagLib; class Matroska::SimpleTag::SimpleTagPrivate { public: - SimpleTagPrivate() = default; - TargetTypeValue targetTypeValue = None; - String name; - String language; - bool defaultLanguageFlag = true; + explicit SimpleTagPrivate(const String &name, const String& value, + TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) : + value(value), name(name), language(language), + targetTypeValue(targetTypeValue), defaultLanguageFlag(defaultLanguage) {} + explicit SimpleTagPrivate(const String &name, const ByteVector& value, + TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) : + value(value), name(name), language(language), + targetTypeValue(targetTypeValue), defaultLanguageFlag(defaultLanguage) {} + const std::variant value; + const String name; + const String language; + const TargetTypeValue targetTypeValue; + const bool defaultLanguageFlag; }; -class Matroska::SimpleTagString::SimpleTagStringPrivate -{ -public: - SimpleTagStringPrivate() = default; - String value; -}; +//////////////////////////////////////////////////////////////////////////////// +// public members +//////////////////////////////////////////////////////////////////////////////// -class Matroska::SimpleTagBinary::SimpleTagBinaryPrivate -{ -public: - SimpleTagBinaryPrivate() = default; - ByteVector value; -}; - -Matroska::SimpleTag::SimpleTag() : - d(std::make_unique()) +Matroska::SimpleTag::SimpleTag(const String &name, const String &value, + TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) : + d(std::make_unique(name, value, targetTypeValue, + language, defaultLanguage)) { } + +Matroska::SimpleTag::SimpleTag(const String &name, const ByteVector &value, + TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) : + d(std::make_unique(name, value, targetTypeValue, + language, defaultLanguage)) +{ +} + +Matroska::SimpleTag::SimpleTag(const SimpleTag &other) : + d(std::make_unique(*other.d)) +{ +} + +Matroska::SimpleTag::SimpleTag(SimpleTag&& other) noexcept = default; + Matroska::SimpleTag::~SimpleTag() = default; +Matroska::SimpleTag &Matroska::SimpleTag::operator=(SimpleTag &&other) = default; + +Matroska::SimpleTag &Matroska::SimpleTag::operator=(const SimpleTag &other) +{ + SimpleTag(other).swap(*this); + return *this; +} + +void Matroska::SimpleTag::swap(SimpleTag &other) noexcept +{ + using std::swap; + + swap(d, other.d); +} + Matroska::SimpleTag::TargetTypeValue Matroska::SimpleTag::targetTypeValue() const { return d->targetTypeValue; } -void Matroska::SimpleTag::setTargetTypeValue(TargetTypeValue targetTypeValue) -{ - d->targetTypeValue = targetTypeValue; -} - const String &Matroska::SimpleTag::name() const { return d->name; @@ -75,54 +101,28 @@ const String &Matroska::SimpleTag::language() const return d->language; } -void Matroska::SimpleTag::setLanguage(const String &language) -{ - d->language = language; -} - bool Matroska::SimpleTag::defaultLanguageFlag() const { return d->defaultLanguageFlag; } -void Matroska::SimpleTag::setDefaultLanguageFlag(bool flag) +Matroska::SimpleTag::ValueType Matroska::SimpleTag::type() const { - d->defaultLanguageFlag = flag; + return std::holds_alternative(d->value) ? BinaryType : StringType; } -void Matroska::SimpleTag::setName(const String &name) +String Matroska::SimpleTag::toString() const { - d->name = name; + if(std::holds_alternative(d->value)) { + return std::get(d->value); + } + return {}; } -Matroska::SimpleTagString::SimpleTagString() : - dd(std::make_unique()) +ByteVector Matroska::SimpleTag::toByteVector() const { -} -Matroska::SimpleTagString::~SimpleTagString() = default; - -const String &Matroska::SimpleTagString::value() const -{ - return dd->value; -} - -void Matroska::SimpleTagString::setValue(const String &value) -{ - dd->value = value; -} - -Matroska::SimpleTagBinary::SimpleTagBinary() : - dd(std::make_unique()) -{ -} -Matroska::SimpleTagBinary::~SimpleTagBinary() = default; - -const ByteVector &Matroska::SimpleTagBinary::value() const -{ - return dd->value; -} - -void Matroska::SimpleTagBinary::setValue(const ByteVector &value) -{ - dd->value = value; + if(std::holds_alternative(d->value)) { + return std::get(d->value); + } + return {}; } diff --git a/taglib/matroska/matroskasimpletag.h b/taglib/matroska/matroskasimpletag.h index 5026f8fb..def0fea8 100644 --- a/taglib/matroska/matroskasimpletag.h +++ b/taglib/matroska/matroskasimpletag.h @@ -23,70 +23,116 @@ #include #include "tag.h" -//#include "matroskatag.h" namespace TagLib { class String; class ByteVector; namespace Matroska { + //! Attribute of Matroska metadata. class TAGLIB_EXPORT SimpleTag { public: + //! Specifies the level of other elements the tag value applies to. enum TargetTypeValue { - None = 0, - Shot = 10, - Subtrack = 20, - Track = 30, - Part = 40, - Album = 50, - Edition = 60, - Collection = 70 + None = 0, //!< Empty or omitted, everything in the segment + Shot = 10, //!< Shot + Subtrack = 20, //!< Subtrack / movement / scene + Track = 30, //!< Track / song / chapter + Part = 40, //!< Part / session + Album = 50, //!< Album / opera / concert / movie / episode + Edition = 60, //!< Edition / issue / volume / opus / season / sequel + Collection = 70 //!< Collection }; + + //! The types the value can have. + enum ValueType { + StringType = 0, //!< Item contains text information coded in UTF-8 + BinaryType = 1 //!< Item contains binary information + }; + + /*! + * Construct a string simple tag. + */ + SimpleTag(const String &name, const String &value, + TargetTypeValue targetTypeValue = None, + const String &language = String(), bool defaultLanguage = true); + + /*! + * Construct a binary simple tag. + */ + SimpleTag(const String &name, const ByteVector &value, + TargetTypeValue targetTypeValue = None, + const String &language = String(), bool defaultLanguage = true); + + /*! + * Construct a simple tag as a copy of \a other. + */ + SimpleTag(const SimpleTag &other); + + /*! + * Construct a simple tag moving from \a other. + */ + SimpleTag(SimpleTag &&other) noexcept; + + /*! + * Destroys this simple tag. + */ + ~SimpleTag(); + + /*! + * Copies the contents of \a other into this item. + */ + SimpleTag &operator=(const SimpleTag &other); + + /*! + * Moves the contents of \a other into this item. + */ + SimpleTag &operator=(SimpleTag &&other); + + /*! + * Exchanges the content of the simple tag with the content of \a other. + */ + void swap(SimpleTag &other) noexcept; + + /*! + * Returns the name of the simple tag. + */ const String &name() const; + + /*! + * Returns the logical level of the target. + */ TargetTypeValue targetTypeValue() const; + + /*! + * Returns the language of the tag. + */ const String &language() const; + + /*! + * Returns if this is the default/original language to use for the tag. + */ bool defaultLanguageFlag() const; - void setName(const String &name); - void setTargetTypeValue(TargetTypeValue targetTypeValue); - void setLanguage(const String &language); - void setDefaultLanguageFlag(bool flag); - virtual ~SimpleTag(); + + /*! + * Returns the type of the value. + */ + ValueType type() const; + + /*! + * Returns the StringType value. + */ + String toString() const; + + /*! + * Returns the BinaryType value. + */ + ByteVector toByteVector() const; private: class SimpleTagPrivate; TAGLIB_MSVC_SUPPRESS_WARNING_NEEDS_TO_HAVE_DLL_INTERFACE std::unique_ptr d; - - protected: - SimpleTag(); - }; - - class TAGLIB_EXPORT SimpleTagString : public SimpleTag - { - public: - SimpleTagString(); - ~SimpleTagString() override; - const String &value() const; - void setValue(const String &value); - - private: - class SimpleTagStringPrivate; - TAGLIB_MSVC_SUPPRESS_WARNING_NEEDS_TO_HAVE_DLL_INTERFACE - std::unique_ptr dd; - }; - - class TAGLIB_EXPORT SimpleTagBinary : public SimpleTag - { - public: - SimpleTagBinary(); - ~SimpleTagBinary() override; - const ByteVector &value() const; - void setValue(const ByteVector &value); - - private: - class SimpleTagBinaryPrivate; - TAGLIB_MSVC_SUPPRESS_WARNING_NEEDS_TO_HAVE_DLL_INTERFACE - std::unique_ptr dd; }; } diff --git a/taglib/matroska/matroskatag.cpp b/taglib/matroska/matroskatag.cpp index b70a45c8..bbc94bc2 100644 --- a/taglib/matroska/matroskatag.cpp +++ b/taglib/matroska/matroskatag.cpp @@ -41,7 +41,7 @@ public: ~TagPrivate() = default; bool setTag(const String &key, const String &value); - const String *getTag(const String &key) const; + String getTag(const String &key) const; template int removeSimpleTags(T &&p) @@ -51,8 +51,6 @@ public: for(auto it = list.begin(); it != list.end();) { it = std::find_if(it, list.end(), std::forward(p)); if(it != list.end()) { - delete *it; - *it = nullptr; it = list.erase(it); numRemoved++; } @@ -74,23 +72,7 @@ public: return list; } - template - const SimpleTag *findSimpleTag(T &&p) const - { - auto &list = tags; - auto it = std::find_if(list.begin(), list.end(), std::forward(p)); - return it != list.end() ? *it : nullptr; - } - - template - SimpleTag *findSimpleTag(T &&p) - { - return const_cast( - const_cast(this)->findSimpleTag(std::forward(p)) - ); - } - - List tags; + SimpleTagsList tags; ByteVector data; }; @@ -98,20 +80,24 @@ Matroska::Tag::Tag() : Element(static_cast(EBML::Element::Id::MkTags)), d(std::make_unique()) { - d->tags.setAutoDelete(true); } + Matroska::Tag::~Tag() = default; -void Matroska::Tag::addSimpleTag(SimpleTag *tag) +void Matroska::Tag::addSimpleTag(const SimpleTag &tag) { d->tags.append(tag); } -void Matroska::Tag::removeSimpleTag(SimpleTag *tag) +void Matroska::Tag::removeSimpleTag(const String &name, + SimpleTag::TargetTypeValue targetTypeValue) { - auto it = d->tags.find(tag); + auto it = std::find_if(d->tags.begin(), d->tags.end(), + [&name, targetTypeValue](const SimpleTag &t) { + return t.name() == name && t.targetTypeValue() == targetTypeValue; + } + ); if(it != d->tags.end()) { - delete *it; d->tags.erase(it); } } @@ -163,49 +149,44 @@ void Matroska::Tag::setTrack(unsigned int i) String Matroska::Tag::title() const { - const auto value = d->getTag("TITLE"); - return value ? *value : String(); + return d->getTag("TITLE"); } String Matroska::Tag::artist() const { - const auto value = d->getTag("ARTIST"); - return value ? *value : String(); + return d->getTag("ARTIST"); } String Matroska::Tag::album() const { - const auto value = d->getTag("ALBUM"); - return value ? *value : String(); + return d->getTag("ALBUM"); } String Matroska::Tag::comment() const { - const auto value = d->getTag("COMMENT"); - return value ? *value : String(); + return d->getTag("COMMENT"); } String Matroska::Tag::genre() const { - const auto value = d->getTag("GENRE"); - return value ? *value : String(); + return d->getTag("GENRE"); } unsigned int Matroska::Tag::year() const { auto value = d->getTag("DATE"); - if(!value) + if(value.isEmpty()) return 0; - auto list = value->split("-"); + auto list = value.split("-"); return static_cast(list.front().toInt()); } unsigned int Matroska::Tag::track() const { auto value = d->getTag("TRACKNUMBER"); - if(!value) + if(value.isEmpty()) return 0; - auto list = value->split("-"); + auto list = value.split("-"); return static_cast(list.front().toInt()); } @@ -217,30 +198,29 @@ bool Matroska::Tag::isEmpty() const bool Matroska::Tag::render() { EBML::MkTags tags; - List *> targetList; - targetList.setAutoDelete(true); + List targetList; // Build target-based list - for(auto tag : d->tags) { - auto targetTypeValue = tag->targetTypeValue(); + for(const auto &tag : std::as_const(d->tags)) { + auto targetTypeValue = tag.targetTypeValue(); auto it = std::find_if(targetList.begin(), targetList.end(), - [&](auto list) { - const auto *simpleTag = list->front(); - return simpleTag->targetTypeValue() == targetTypeValue; + [&](const auto &list) { + const auto &simpleTag = list.front(); + return simpleTag.targetTypeValue() == targetTypeValue; } ); if(it == targetList.end()) { - auto list = new List(); - list->append(tag); + SimpleTagsList list; + list.append(tag); targetList.append(list); } else - (*it)->append(tag); + it->append(tag); } - for(auto list : targetList) { - auto frontTag = list->front(); - auto targetTypeValue = frontTag->targetTypeValue(); + for(const auto &list : targetList) { + const auto &frontTag = list.front(); + auto targetTypeValue = frontTag.targetTypeValue(); auto tag = EBML::make_unique_element(); // Build element @@ -253,35 +233,33 @@ bool Matroska::Tag::render() tag->appendElement(std::move(targets)); // Build element - for(auto simpleTag : *list) { + for(const auto &simpleTag : list) { auto t = EBML::make_unique_element(); auto tagName = EBML::make_unique_element(); - tagName->setValue(simpleTag->name()); + tagName->setValue(simpleTag.name()); t->appendElement(std::move(tagName)); // Tag Value - SimpleTagString *tStr = nullptr; - SimpleTagBinary *tBin = nullptr; - if((tStr = dynamic_cast(simpleTag))) { + if(simpleTag.type() == SimpleTag::StringType) { auto tagValue = EBML::make_unique_element(); - tagValue->setValue(tStr->value()); + tagValue->setValue(simpleTag.toString()); t->appendElement(std::move(tagValue)); } - else if((tBin = dynamic_cast(simpleTag))) { + else if(simpleTag.type() == SimpleTag::BinaryType) { auto tagValue = EBML::make_unique_element(); - tagValue->setValue(tBin->value()); + tagValue->setValue(simpleTag.toByteVector()); t->appendElement(std::move(tagValue)); } // Language auto language = EBML::make_unique_element(); - const String &lang = simpleTag->language(); + const String &lang = simpleTag.language(); language->setValue(!lang.isEmpty() ? lang : "und"); t->appendElement(std::move(language)); // Default language flag auto dlf = EBML::make_unique_element(); - dlf->setValue(simpleTag->defaultLanguageFlag() ? 1 : 0); + dlf->setValue(simpleTag.defaultLanguageFlag() ? 1 : 0); t->appendElement(std::move(dlf)); tag->appendElement(std::move(t)); @@ -302,51 +280,62 @@ bool Matroska::Tag::render() namespace { - // PropertyMap key, Tag name, Target type value + // PropertyMap key, Tag name, Target type value, strict // If the key is the same as the name and the target type value is Track, // no translation is needed because this is the default mapping. - // Therefore, keys like TITLE, ARTIST, GENRE, COMMENT, etc. are omitted. + // Therefore, keys like TITLE, ARTIST, GENRE, COMMENT, etc. are omitted + // unless they shall have priority over higher level tags with the same name + // when no target type value is given. The strict boolean marks + // entries which shall not be mapped without correct target type value. // For offical tags, see https://www.matroska.org/technical/tagging.html constexpr std::array simpleTagsTranslation { - std::tuple("ALBUM", "TITLE", Matroska::SimpleTag::TargetTypeValue::Album), - std::tuple("ALBUMARTIST", "ARTIST", Matroska::SimpleTag::TargetTypeValue::Album), - std::tuple("TRACKNUMBER", "PART_NUMBER", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("TRACKTOTAL", "TOTAL_PARTS", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("DISCNUMBER", "PART_NUMBER", Matroska::SimpleTag::TargetTypeValue::Part), - std::tuple("DISCTOTAL", "TOTAL_PARTS", Matroska::SimpleTag::TargetTypeValue::Part), - std::tuple("DATE", "DATE_RELEASED", Matroska::SimpleTag::TargetTypeValue::Album), + std::tuple("TITLE", "TITLE", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("ALBUM", "TITLE", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("ARTIST", "ARTIST", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("ALBUMARTIST", "ARTIST", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("TRACKNUMBER", "PART_NUMBER", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("DISCNUMBER", "PART_NUMBER", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("TRACKTOTAL", "TOTAL_PARTS", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("DISCTOTAL", "TOTAL_PARTS", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("DATE", "DATE_RELEASED", Matroska::SimpleTag::TargetTypeValue::Album, false), // Todo - original date - std::tuple("ALBUMSORT", "TITLESORT", Matroska::SimpleTag::TargetTypeValue::Album), - std::tuple("ALBUMARTISTSORT", "ARTISTSORT", Matroska::SimpleTag::TargetTypeValue::Album), - std::tuple("ENCODEDBY", "ENCODED_BY", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("MEDIA", "ORIGINAL_MEDIA_TYPE", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("LABEL", "LABEL_CODE", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("CATALOGNUMBER", "CATALOG_NUMBER", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("DJMIXER", "MIXED_BY", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("REMIXER", "REMIXED_BY", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("INITIALKEY", "INITIAL_KEY", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("RELEASEDATE", "DATE_RELEASED", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("ENCODINGTIME", "DATE_ENCODED", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("TAGGINGDATE", "DATE_TAGGED", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("ENCODEDBY", "ENCODER", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("ENCODING", "ENCODER_SETTINGS", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("OWNER", "PURCHASE_OWNER", Matroska::SimpleTag::TargetTypeValue::Track), - std::tuple("MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ_ALBUMARTISTID", Matroska::SimpleTag::TargetTypeValue::Album), - std::tuple("MUSICBRAINZ_ALBUMID", "MUSICBRAINZ_ALBUMID", Matroska::SimpleTag::TargetTypeValue::Album), - std::tuple("MUSICBRAINZ_RELEASEGROUPID", "MUSICBRAINZ_RELEASEGROUPID", Matroska::SimpleTag::TargetTypeValue::Album), + std::tuple("TITLESORT", "TITLESORT", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("ALBUMSORT", "TITLESORT", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("ARTISTSORT", "ARTISTSORT", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("ALBUMARTISTSORT", "ARTISTSORT", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("ENCODEDBY", "ENCODED_BY", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("MEDIA", "ORIGINAL_MEDIA_TYPE", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("LABEL", "LABEL_CODE", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("CATALOGNUMBER", "CATALOG_NUMBER", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("DJMIXER", "MIXED_BY", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("REMIXER", "REMIXED_BY", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("INITIALKEY", "INITIAL_KEY", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("RELEASEDATE", "DATE_RELEASED", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("ENCODINGTIME", "DATE_ENCODED", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("TAGGINGDATE", "DATE_TAGGED", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("ENCODEDBY", "ENCODER", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("ENCODING", "ENCODER_SETTINGS", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("OWNER", "PURCHASE_OWNER", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("REPLAYGAIN_TRACK_GAIN", "REPLAYGAIN_GAIN", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("REPLAYGAIN_ALBUM_GAIN", "REPLAYGAIN_GAIN", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("REPLAYGAIN_TRACK_PEAK", "REPLAYGAIN_PEAK", Matroska::SimpleTag::TargetTypeValue::Track, false), + std::tuple("REPLAYGAIN_ALBUM_PEAK", "REPLAYGAIN_PEAK", Matroska::SimpleTag::TargetTypeValue::Album, true), + std::tuple("MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ_ALBUMARTISTID", Matroska::SimpleTag::TargetTypeValue::Album, false), + std::tuple("MUSICBRAINZ_ALBUMID", "MUSICBRAINZ_ALBUMID", Matroska::SimpleTag::TargetTypeValue::Album, false), + std::tuple("MUSICBRAINZ_RELEASEGROUPID", "MUSICBRAINZ_RELEASEGROUPID", Matroska::SimpleTag::TargetTypeValue::Album, false), }; - std::pair translateKey(const String &key) + std::tuple translateKey(const String &key) { auto it = std::find_if(simpleTagsTranslation.cbegin(), simpleTagsTranslation.cend(), [&key](const auto &t) { return key == std::get<0>(t); } ); if(it != simpleTagsTranslation.end()) - return { std::get<1>(*it), std::get<2>(*it) }; + return { std::get<1>(*it), std::get<2>(*it), std::get<3>(*it) }; if (!key.isEmpty() && !key.startsWith("_")) - return { key, Matroska::SimpleTag::TargetTypeValue::Track }; - return { String(), Matroska::SimpleTag::TargetTypeValue::None }; + return { key, Matroska::SimpleTag::TargetTypeValue::Track, false }; + return { String(), Matroska::SimpleTag::TargetTypeValue::None, false }; } String translateTag(const String &name, Matroska::SimpleTag::TargetTypeValue targetTypeValue) @@ -355,12 +344,16 @@ namespace simpleTagsTranslation.cend(), [&name, targetTypeValue](const auto &t) { return name == std::get<1>(t) - && targetTypeValue == std::get<2>(t); + && (targetTypeValue == std::get<2>(t) || + (targetTypeValue == Matroska::SimpleTag::TargetTypeValue::None + && !std::get<3>(t))); } ); return it != simpleTagsTranslation.end() ? String(std::get<0>(*it), String::UTF8) - : targetTypeValue == Matroska::SimpleTag::TargetTypeValue::Track && !name.startsWith("_") + : (targetTypeValue == Matroska::SimpleTag::TargetTypeValue::Track || + targetTypeValue == Matroska::SimpleTag::TargetTypeValue::None) && + !name.startsWith("_") ? name : String(); } @@ -368,59 +361,65 @@ namespace bool Matroska::Tag::TagPrivate::setTag(const String &key, const String &value) { - const auto pair = translateKey(key); + const auto tpl = translateKey(key); // Workaround Clang issue - no lambda capture of structured bindings - const String &name = pair.first; - auto targetTypeValue = pair.second; + const String &name = std::get<0>(tpl); + auto targetTypeValue = std::get<1>(tpl); if(name.isEmpty()) return false; removeSimpleTags( - [&name, targetTypeValue] (auto t) { - return t->name() == name - && t->targetTypeValue() == targetTypeValue; + [&name, targetTypeValue] (const auto &t) { + return t.name() == name + && t.targetTypeValue() == targetTypeValue; } ); if(!value.isEmpty()) { - auto t = new SimpleTagString(); - t->setTargetTypeValue(targetTypeValue); - t->setName(name); - t->setValue(value); - tags.append(t); + tags.append(SimpleTag(name, value, targetTypeValue)); } return true; } -const String *Matroska::Tag::TagPrivate::getTag(const String &key) const +String Matroska::Tag::TagPrivate::getTag(const String &key) const { - const auto pair = translateKey(key); + const auto tpl = translateKey(key); // Workaround Clang issue - no lambda capture of structured bindings - const String &name = pair.first; - auto targetTypeValue = pair.second; + const String &name = std::get<0>(tpl); + auto targetTypeValue = std::get<1>(tpl); + bool strict = std::get<2>(tpl); if(name.isEmpty()) - return nullptr; - auto tag = dynamic_cast( - findSimpleTag( - [&name, targetTypeValue] (auto t) { - return t->name() == name - && t->targetTypeValue() == targetTypeValue; - } - ) + return {}; + auto it = std::find_if(tags.begin(), tags.end(), + [&name, targetTypeValue, strict] (const SimpleTag &t) { + return t.name() == name + && t.type() == SimpleTag::StringType + && (t.targetTypeValue() == targetTypeValue || + (t.targetTypeValue() == SimpleTag::TargetTypeValue::None && !strict)); + } ); - return tag ? &tag->value() : nullptr; + return it != tags.end() ? it->toString() : String(); } PropertyMap Matroska::Tag::setProperties(const PropertyMap &propertyMap) { + // Remove all simple tags which would be returned in properties() + for(auto it = d->tags.begin(); it != d->tags.end();) { + String key; + if(it->type() == SimpleTag::StringType && + !(key = translateTag(it->name(), it->targetTypeValue())).isEmpty()) { + it = d->tags.erase(it); + } + else { + ++it; + } + } + + // Add the new properties PropertyMap unsupportedProperties; for(const auto &[key, values] : propertyMap) { for(const auto &value : values) { - if(auto [name, targetTypeValue] = translateKey(key); + if(auto [name, targetTypeValue, _] = translateKey(key); !name.isEmpty()) { - auto t = new SimpleTagString(); - t->setTargetTypeValue(targetTypeValue); - t->setName(name); - t->setValue(value); - d->tags.append(t); + d->tags.append(SimpleTag(name, value, targetTypeValue)); } else { unsupportedProperties[key] = values; @@ -433,8 +432,8 @@ PropertyMap Matroska::Tag::setProperties(const PropertyMap &propertyMap) void Matroska::Tag::removeUnsupportedProperties(const StringList& properties) { d->removeSimpleTags( - [&properties](const SimpleTag* t) { - return properties.contains(t->name()); + [&properties](const SimpleTag &t) { + return properties.contains(t.name()); } ); } @@ -442,9 +441,9 @@ void Matroska::Tag::removeUnsupportedProperties(const StringList& properties) StringList Matroska::Tag::complexPropertyKeys() const { StringList keys; - for(const SimpleTag *t : std::as_const(d->tags)) { - if(auto tBinary = dynamic_cast(t)) { - keys.append(tBinary->name()); + for(const SimpleTag &t : std::as_const(d->tags)) { + if(t.type() == SimpleTag::BinaryType) { + keys.append(t.name()); } } return keys; @@ -454,19 +453,19 @@ List Matroska::Tag::complexProperties(const String& key) const { List props; if(key.upper() != "PICTURE") { // Pictures are handled at the file level - for(const SimpleTag *t : std::as_const(d->tags)) { - if(auto tBinary = dynamic_cast(t)) { + for(const SimpleTag &t : std::as_const(d->tags)) { + if(t.type() == SimpleTag::BinaryType) { VariantMap property; - property.insert("data", tBinary->value()); - property.insert("name", tBinary->name()); - property.insert("targetTypeValue", tBinary->targetTypeValue()); - property.insert("language", tBinary->language()); - property.insert("defaultLanguage", tBinary->defaultLanguageFlag()); + property.insert("data", t.toByteVector()); + property.insert("name", t.name()); + property.insert("targetTypeValue", t.targetTypeValue()); + property.insert("language", t.language()); + property.insert("defaultLanguage", t.defaultLanguageFlag()); props.append(property); } } } - return TagLib::Tag::complexProperties(key); + return props; } bool Matroska::Tag::setComplexProperties(const String& key, const List& value) @@ -476,21 +475,20 @@ bool Matroska::Tag::setComplexProperties(const String& key, const ListremoveSimpleTags( - [&key](const SimpleTag* t) { - return t->name() == key && dynamic_cast(t) != nullptr; + [&key](const SimpleTag &t) { + return t.name() == key && t.type() == SimpleTag::BinaryType; } ); bool result = false; for(const auto &property : value) { if(property.value("name").value() == key && property.contains("data")) { - auto *t = new SimpleTagBinary; - t->setTargetTypeValue(static_cast( - property.value("targetTypeValue", 0).value())); - t->setName(key); - t->setValue(property.value("data").value()); - t->setLanguage(property.value("language").value()); - t->setDefaultLanguageFlag(property.value("defaultLanguage", true).value()); - d->tags.append(t); + d->tags.append(SimpleTag( + key, + property.value("data").value(), + static_cast( + property.value("targetTypeValue", 0).value()), + property.value("language").value(), + property.value("defaultLanguage", true).value())); result = true; } } @@ -500,12 +498,13 @@ bool Matroska::Tag::setComplexProperties(const String& key, const Listtags)) { - if((tStr = dynamic_cast(simpleTag))) { - String key = translateTag(tStr->name(), tStr->targetTypeValue()); + for(const auto &simpleTag : std::as_const(d->tags)) { + if(simpleTag.type() == SimpleTag::StringType) { + String key = translateTag(simpleTag.name(), simpleTag.targetTypeValue()); if(!key.isEmpty()) - properties[key].append(tStr->value()); + properties[key].append(simpleTag.toString()); + else + properties.addUnsupportedData(simpleTag.name()); } } return properties; diff --git a/taglib/matroska/matroskatag.h b/taglib/matroska/matroskatag.h index 5c90a986..8be38424 100644 --- a/taglib/matroska/matroskatag.h +++ b/taglib/matroska/matroskatag.h @@ -37,7 +37,7 @@ namespace TagLib { } namespace Matroska { - using SimpleTagsList = List; + using SimpleTagsList = List; class TAGLIB_EXPORT Tag : public TagLib::Tag #ifndef DO_NOT_DOCUMENT , private Element @@ -76,7 +76,7 @@ namespace TagLib { * The attached files such as pictures with key "PICTURE" are available * with Matroska::File::complexProperties(). */ - List complexProperties(const String& key) const override; + List complexProperties(const String &key) const override; /*! * Set the binary simple tags as maps with keys "data", "name", @@ -88,8 +88,8 @@ namespace TagLib { */ bool setComplexProperties(const String& key, const List& value) override; - void addSimpleTag(SimpleTag *tag); - void removeSimpleTag(SimpleTag *tag); + void addSimpleTag(const SimpleTag &tag); + void removeSimpleTag(const String &name, SimpleTag::TargetTypeValue targetTypeValue); void clearSimpleTags(); const SimpleTagsList &simpleTagsList() const;