Preserve multiple strings in ID3v2 text frames

CMakeLists.txt

@@ -172,8 +172,8 @@ set(META_APP_NAME "Tag Parser")
 set(META_APP_AUTHOR "Martchus")
 set(META_APP_URL "https://github.com/${META_APP_AUTHOR}/${META_PROJECT_NAME}")
 set(META_APP_DESCRIPTION "C++ library for reading and writing MP4 (iTunes), ID3, Vorbis, Opus, FLAC and Matroska tags")
-set(META_VERSION_MAJOR 7)
+set(META_VERSION_MAJOR 8)
-set(META_VERSION_MINOR 2)
+set(META_VERSION_MINOR 0)
 set(META_VERSION_PATCH 0)
 set(META_PUBLIC_SHARED_LIB_DEPENDS c++utilities)
 set(META_PUBLIC_STATIC_LIB_DEPENDS c++utilities_static)

id3/id3v2frame.cpp

@@ -250,7 +250,6 @@ void Id3v2Frame::parse(BinaryReader &reader, uint32 version, uint32 maximalSize,
         TagTextEncoding dataEncoding = parseTextEncodingByte(static_cast<byte>(*buffer.get()), diag);
 
         // parse string values (since ID3v2.4 a text frame may contain multiple strings)
-        std::vector<TagValue> additionalValues;
         const char *currentOffset = buffer.get() + 1;
         for (size_t currentIndex = 1; currentIndex < m_dataSize;) {
             // determine the next substring
@@ -271,8 +270,8 @@ void Id3v2Frame::parse(BinaryReader &reader, uint32 version, uint32 maximalSize,
                 if (this->value().isEmpty()) {
                     return &this->value();
                 }
-                additionalValues.emplace_back();
+                m_additionalValues.emplace_back();
-                return &additionalValues.back();
+                return &m_additionalValues.back();
             }();
 
             // apply further parsing for some text frame types (eg. convert track number to PositionInSet)
@@ -334,24 +333,9 @@ void Id3v2Frame::parse(BinaryReader &reader, uint32 version, uint32 maximalSize,
         }
 
         // add warning about additional values
-        if (!additionalValues.empty()) {
+        if (version < 4 && !m_additionalValues.empty()) {
-            // format list of values
+            diag.emplace_back(
-            const auto valuesString = [&]() -> string {
+                DiagLevel::Warning, "Multiple strings found though the tag is pre-ID3v2.4. " + ignoreAdditionalValuesDiagMsg(), context);
-                if (additionalValues.size() == 1) {
-                    return argsToString("value \"", additionalValues.front().toString(TagTextEncoding::Utf8), "\" is ignored.");
-                }
-                return argsToString("values ", DiagMessage::formatList(TagValue::toStrings(additionalValues)), " are ignored.");
-            }();
-
-            // emplace diag message
-            if (version < 4) {
-                diag.emplace_back(
-                    DiagLevel::Warning, argsToString("Multiple strings found though the tag is pre-ID3v2.4. Additional ", valuesString), context);
-                additionalValues.clear();
-            } else {
-                diag.emplace_back(DiagLevel::Warning,
-                    argsToString("Multiple strings found. This is not supported so far. Hence the additional ", valuesString), context);
-            }
         }
 
     } else if (version >= 3 && id() == Id3v2FrameIds::lCover) {
@@ -416,6 +400,18 @@ void Id3v2Frame::reset()
     m_dataSize = 0;
     m_totalSize = 0;
     m_padding = false;
+    m_additionalValues.clear();
+}
+
+/*!
+ * \brief Returns a diag message that additional values are ignored.
+ */
+std::string Id3v2Frame::ignoreAdditionalValuesDiagMsg() const
+{
+    if (m_additionalValues.size() == 1) {
+        return argsToString("Additional value \"", m_additionalValues.front().toString(TagTextEncoding::Utf8), "\" is supposed to be ignored.");
+    }
+    return argsToString("Additional values ", DiagMessage::formatList(TagValue::toStrings(m_additionalValues)), " are supposed to be ignored.");
 }
 
 /*!
@@ -436,9 +432,20 @@ Id3v2FrameMaker::Id3v2FrameMaker(Id3v2Frame &frame, byte version, Diagnostics &d
 {
     const string context("making " % m_frame.frameIdString() + " frame");
 
+    // get non-empty, assigned values
+    vector<const TagValue *> values;
+    values.reserve(1 + frame.additionalValues().size());
+    if (!frame.value().isEmpty()) {
+        values.emplace_back(&frame.value());
+    }
+    for (const auto &value : frame.additionalValues()) {
+        if (!value.isEmpty()) {
+            values.emplace_back(&value);
+        }
+    }
+
     // validate assigned data
-    const auto value(m_frame.value());
+    if (values.empty()) {
-    if (value.isEmpty()) {
         diag.emplace_back(DiagLevel::Critical, "Cannot make an empty frame.", context);
         throw InvalidDataException();
     }
@@ -457,6 +464,16 @@ Id3v2FrameMaker::Id3v2FrameMaker(Id3v2Frame &frame, byte version, Diagnostics &d
         diag.emplace_back(DiagLevel::Warning,
             "The existing flag and group information is not supported by the version of ID3v2 and will be ignored/discarted.", context);
     }
+    const bool isTextFrame = Id3v2FrameIds::isTextFrame(m_frameId);
+    if (values.size() != 1) {
+        if (!isTextFrame) {
+            diag.emplace_back(DiagLevel::Critical, "Multiple values are not supported for non-text-frames.", context);
+            throw InvalidDataException();
+        } else if (version < 4) {
+            diag.emplace_back(
+                DiagLevel::Warning, "Multiple strings assigned to pre-ID3v2.4 text frame. " + frame.ignoreAdditionalValuesDiagMsg(), context);
+        }
+    }
 
     // convert frame ID if necessary
     if (version >= 3) {
@@ -483,61 +500,117 @@ Id3v2FrameMaker::Id3v2FrameMaker(Id3v2Frame &frame, byte version, Diagnostics &d
 
     // make actual data depending on the frame ID
     try {
-        if (Id3v2FrameIds::isTextFrame(m_frameId)) {
+        if (isTextFrame) {
-            // make text frames
+            // make text frame
+            vector<string> substrings;
+            substrings.reserve(1 + frame.additionalValues().size());
+            TagTextEncoding encoding = TagTextEncoding::Unspecified;
+
             if ((version >= 3 && (m_frameId == Id3v2FrameIds::lTrackPosition || m_frameId == Id3v2FrameIds::lDiskPosition))
                 || (version < 3 && (m_frameId == Id3v2FrameIds::sTrackPosition || m_frameId == Id3v2FrameIds::sDiskPosition))) {
                 // make track number or disk number frame
-                // -> convert the position to string
+                encoding = version >= 4 ? TagTextEncoding::Utf8 : TagTextEncoding::Latin1;
-                const auto positionStr(value.toString(TagTextEncoding::Latin1));
+                for (const auto *const value : values) {
-                // -> warn if value is no valid position (although we just store a string after all)
+                    // convert the position to string
-                if (value.type() != TagDataType::PositionInSet) {
+                    substrings.emplace_back(value->toString(encoding));
+                    // warn if value is no valid position (although we just store a string after all)
+                    if (value->type() == TagDataType::PositionInSet) {
+                        continue;
+                    }
                     try {
-                        value.toPositionInSet();
+                        value->toPositionInSet();
                     } catch (const ConversionException &) {
                         diag.emplace_back(DiagLevel::Warning,
-                            argsToString("The track/disk number \"", positionStr, "\" is not of the expected form, eg. \"4/10\"."), context);
+                            argsToString("The track/disk number \"", substrings.back(), "\" is not of the expected form, eg. \"4/10\"."), context);
                     }
                 }
-                m_frame.makeString(m_data, m_decompressedSize, positionStr, TagTextEncoding::Latin1);
+
             } else if ((version >= 3 && m_frameId == Id3v2FrameIds::lLength) || (version < 3 && m_frameId == Id3v2FrameIds::sLength)) {
                 // make length frame
-                const auto duration(value.toTimeSpan());
+                encoding = TagTextEncoding::Latin1;
-                if (duration.isNegative()) {
+                for (const auto *const value : values) {
-                    diag.emplace_back(DiagLevel::Critical, argsToString("Assigned duration \"", duration.toString(), "\" is negative."), context);
+                    const auto duration(value->toTimeSpan());
-                    throw InvalidDataException();
+                    if (duration.isNegative()) {
+                        diag.emplace_back(DiagLevel::Critical, argsToString("Assigned duration \"", duration.toString(), "\" is negative."), context);
+                        throw InvalidDataException();
+                    }
+                    substrings.emplace_back(ConversionUtilities::numberToString(static_cast<uint64>(duration.totalMilliseconds())));
                 }
-                m_frame.makeString(m_data, m_decompressedSize, ConversionUtilities::numberToString(static_cast<uint64>(duration.totalMilliseconds())),
+
-                    TagTextEncoding::Latin1);
-            } else if (value.type() == TagDataType::StandardGenreIndex
-                && ((version >= 3 && m_frameId == Id3v2FrameIds::lGenre) || (version < 3 && m_frameId == Id3v2FrameIds::sGenre))) {
-                // make pre-defined genre frame
-                m_frame.makeString(
-                    m_data, m_decompressedSize, ConversionUtilities::numberToString(value.toStandardGenreIndex()), TagTextEncoding::Latin1);
             } else {
-                // make other text frames
+                // make standard genre index and other text frames
-                if (version <= 3 && value.dataEncoding() == TagTextEncoding::Utf8) {
+                // -> find text encoding suitable for all assigned values
-                    // UTF-8 is only supported by ID3v2.4, so convert back to UTF-16
+                for (const auto *const value : values) {
-                    m_frame.makeString(
+                    switch (encoding) {
-                        m_data, m_decompressedSize, value.toString(TagTextEncoding::Utf16LittleEndian), TagTextEncoding::Utf16LittleEndian);
+                    case TagTextEncoding::Unspecified:
-                } else {
+                        switch (value->type()) {
-                    // just keep encoding of the assigned value
+                        case TagDataType::StandardGenreIndex:
-                    m_frame.makeString(m_data, m_decompressedSize, value.toString(), value.dataEncoding());
+                            encoding = TagTextEncoding::Latin1;
+                            break;
+                        default:
+                            encoding = value->dataEncoding();
+                        }
+                        break;
+                    case TagTextEncoding::Latin1:
+                        switch (value->dataEncoding()) {
+                        case TagTextEncoding::Latin1:
+                            break;
+                        default:
+                            encoding = value->dataEncoding();
+                        }
+                        break;
+                    default:;
+                    }
+                }
+                if (version <= 3 && encoding == TagTextEncoding::Utf8) {
+                    encoding = TagTextEncoding::Utf16LittleEndian;
+                }
+                // -> format values
+                for (const auto *const value : values) {
+                    if ((value->type() == TagDataType::StandardGenreIndex)
+                        && ((version >= 3 && m_frameId == Id3v2FrameIds::lGenre) || (version < 3 && m_frameId == Id3v2FrameIds::sGenre))) {
+                        // make standard genere index
+                        substrings.emplace_back(ConversionUtilities::numberToString(value->toStandardGenreIndex()));
+
+                    } else {
+                        // make other text frame
+                        substrings.emplace_back(value->toString(encoding));
+                    }
                 }
             }
 
+            // concatenate substrings using encoding specific byte order mark and termination
+            const auto terminationLength = (encoding == TagTextEncoding::Utf16BigEndian || encoding == TagTextEncoding::Utf16LittleEndian) ? 2u : 1u;
+            const auto byteOrderMark = [&] {
+                switch (encoding) {
+                case TagTextEncoding::Utf16LittleEndian:
+                    return string({ '\xFF', '\xFE' });
+                case TagTextEncoding::Utf16BigEndian:
+                    return string({ '\xFE', '\xFF' });
+                default:
+                    return string();
+                }
+            }();
+            const auto concatenatedSubstrings = joinStrings(substrings, string(), false, byteOrderMark, string(terminationLength, '\0'));
+
+            // write text encoding byte and concatenated strings to data buffer
+            m_data = make_unique<char[]>(m_decompressedSize = static_cast<uint32>(1 + concatenatedSubstrings.size()));
+            m_data[0] = static_cast<char>(Id3v2Frame::makeTextEncodingByte(encoding));
+            concatenatedSubstrings.copy(&m_data[1], concatenatedSubstrings.size());
+
         } else if ((version >= 3 && m_frameId == Id3v2FrameIds::lCover) || (version < 3 && m_frameId == Id3v2FrameIds::sCover)) {
             // make picture frame
-            m_frame.makePicture(m_data, m_decompressedSize, value, m_frame.isTypeInfoAssigned() ? m_frame.typeInfo() : 0, version);
+            m_frame.makePicture(m_data, m_decompressedSize, *values.front(), m_frame.isTypeInfoAssigned() ? m_frame.typeInfo() : 0, version);
 
         } else if (((version >= 3 && m_frameId == Id3v2FrameIds::lComment) || (version < 3 && m_frameId == Id3v2FrameIds::sComment))
             || ((version >= 3 && m_frameId == Id3v2FrameIds::lUnsynchronizedLyrics)
                    || (version < 3 && m_frameId == Id3v2FrameIds::sUnsynchronizedLyrics))) {
             // make comment frame or the unsynchronized lyrics frame
-            m_frame.makeComment(m_data, m_decompressedSize, value, version, diag);
+            m_frame.makeComment(m_data, m_decompressedSize, *values.front(), version, diag);
 
         } else {
             // make unknown frame
+            const auto &value(*values.front());
             if (value.dataSize() > maxId3v2FrameDataSize) {
                 diag.emplace_back(DiagLevel::Critical, "Assigned value exceeds maximum size.", context);
                 throw InvalidDataException();
@@ -547,10 +620,11 @@ Id3v2FrameMaker::Id3v2FrameMaker(Id3v2Frame &frame, byte version, Diagnostics &d
         }
     } catch (const ConversionException &) {
         try {
+            const auto valuesAsString = TagValue::toStrings(values);
             diag.emplace_back(DiagLevel::Critical,
-                argsToString("Assigned value \"", value.toString(TagTextEncoding::Utf8), "\" can not be converted appropriately."), context);
+                argsToString("Assigned value(s) \"", DiagMessage::formatList(valuesAsString), "\" can not be converted appropriately."), context);
         } catch (const ConversionException &) {
-            diag.emplace_back(DiagLevel::Critical, "Assigned value can not be converted appropriately.", context);
+            diag.emplace_back(DiagLevel::Critical, "Assigned value(s) can not be converted appropriately.", context);
         }
         throw InvalidDataException();
     }

id3/id3v2frame.h

@@ -31,6 +31,8 @@ public:
 
 private:
     Id3v2FrameMaker(Id3v2Frame &frame, byte version, Diagnostics &diag);
+    void makeSubstring(const TagValue &value, Diagnostics &diag, const std::string &context);
+
     Id3v2Frame &m_frame;
     uint32 m_frameId;
     const byte m_version;
@@ -83,6 +85,7 @@ public:
 
 class TAG_PARSER_EXPORT Id3v2Frame : public TagField<Id3v2Frame> {
     friend class TagField<Id3v2Frame>;
+    friend class Id3v2FrameMaker;
 
 public:
     Id3v2Frame();
@@ -94,6 +97,8 @@ public:
     void make(IoUtilities::BinaryWriter &writer, byte version, Diagnostics &diag);
 
     // member access
+    const std::vector<TagValue> &additionalValues() const;
+    std::vector<TagValue> &additionalValues();
     bool isAdditionalTypeInfoUsed() const;
     bool isValid() const;
     bool hasPaddingReached() const;
@@ -127,19 +132,23 @@ public:
     void parseBom(const char *buffer, std::size_t maxSize, TagTextEncoding &encoding, Diagnostics &diag);
 
     // making helper
-    byte makeTextEncodingByte(TagTextEncoding textEncoding);
+    static byte makeTextEncodingByte(TagTextEncoding textEncoding);
-    std::size_t makeBom(char *buffer, TagTextEncoding encoding);
+    static std::size_t makeBom(char *buffer, TagTextEncoding encoding);
-    void makeString(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const std::string &value, TagTextEncoding encoding);
+    static void makeString(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const std::string &value, TagTextEncoding encoding);
-    void makeEncodingAndData(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, TagTextEncoding encoding, const char *data, std::size_t m_dataSize);
+    static void makeEncodingAndData(
-    void makeLegacyPicture(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo);
+        std::unique_ptr<char[]> &buffer, uint32 &bufferSize, TagTextEncoding encoding, const char *data, std::size_t m_dataSize);
-    void makePicture(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo, byte version);
+    static void makeLegacyPicture(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo);
-    void makeComment(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const TagValue &comment, byte version, Diagnostics &diag);
+    static void makePicture(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo, byte version);
+    static void makeComment(std::unique_ptr<char[]> &buffer, uint32 &bufferSize, const TagValue &comment, byte version, Diagnostics &diag);
 
     static IdentifierType fieldIdFromString(const char *idString, std::size_t idStringSize = std::string::npos);
     static std::string fieldIdToString(IdentifierType id);
 
 private:
     void reset();
+    std::string ignoreAdditionalValuesDiagMsg() const;
+
+    std::vector<TagValue> m_additionalValues;
     uint32 m_parsedVersion;
     uint32 m_dataSize;
     uint32 m_totalSize;
@@ -148,6 +157,24 @@ private:
     bool m_padding;
 };
 
+/*!
+ * \brief Returns additional values.
+ * \remarks Frames might allow to store multiple values, eg. ID3v2.4 text frames allow to store multiple strings.
+ */
+inline const std::vector<TagValue> &Id3v2Frame::additionalValues() const
+{
+    return m_additionalValues;
+}
+
+/*!
+ * \brief Returns additional values.
+ * \remarks Frames might allow to store multiple values, eg. ID3v2.4 text frames allow to store multiple strings.
+ */
+inline std::vector<TagValue> &Id3v2Frame::additionalValues()
+{
+    return m_additionalValues;
+}
+
 /*!
  * \brief Returns whether the instance uses the additional type info.
  */

tests/overall.h

@@ -105,7 +105,8 @@ private:
 
     void setMkvTestMetaData();
     void setMp4TestMetaData();
-    void setMp3TestMetaData();
+    void setMp3TestMetaData1();
+    void setMp3TestMetaData2();
     void setOggTestMetaData();
     void removeAllTags();
     void noop();

tests/overallmp3.cpp

@@ -6,6 +6,8 @@
 #include "../id3/id3v2tag.h"
 #include "../mpegaudio/mpegaudioframe.h"
 
+#include <regex>
+
 namespace Mp3TestFlags {
 enum TestFlag {
     ForceRewring = 0x1,
@@ -99,47 +101,73 @@ void OverallTests::checkMp3Testfile2()
         CPPUNIT_ASSERT_EQUAL(20, track->duration().seconds());
     }
     const auto tags = m_fileInfo.tags();
+    const bool expectId3v24 = m_tagStatus == TagStatus::Original || m_mode & Mp3TestFlags::UseId3v24;
     switch (m_tagStatus) {
     case TagStatus::Original:
+    case TagStatus::TestMetaDataPresent:
         CPPUNIT_ASSERT(!m_fileInfo.id3v1Tag());
         CPPUNIT_ASSERT_EQUAL(1_st, m_fileInfo.id3v2Tags().size());
         CPPUNIT_ASSERT_EQUAL(1_st, tags.size());
         for (const auto &tag : tags) {
-            switch (tag->type()) {
+            if (tag->type() != TagType::Id3v2Tag) {
-            case TagType::Id3v1Tag:
+                CPPUNIT_FAIL(argsToString("no ", tag->typeName(), " tag expected"));
-                CPPUNIT_FAIL("no ID3v1 tag expected");
-            case TagType::Id3v2Tag:
-                CPPUNIT_ASSERT_EQUAL(TagTextEncoding::Utf8, tag->value(KnownField::Title).dataEncoding());
-                CPPUNIT_ASSERT_EQUAL("Infinite (Original Mix)"s, tag->value(KnownField::Title).toString(TagTextEncoding::Utf8));
-                CPPUNIT_ASSERT_EQUAL("B-Front"s, tag->value(KnownField::Artist).toString(TagTextEncoding::Utf8));
-                CPPUNIT_ASSERT_EQUAL("Infinite"s, tag->value(KnownField::Album).toString(TagTextEncoding::Utf8));
-                CPPUNIT_ASSERT_EQUAL("Hardstyle"s, tag->value(KnownField::Genre).toString(TagTextEncoding::Utf8));
-                CPPUNIT_ASSERT_EQUAL("Lavf57.83.100"s, tag->value(KnownField::EncoderSettings).toString(TagTextEncoding::Utf8));
-                CPPUNIT_ASSERT_EQUAL("Roughstate"s, tag->value(KnownField::RecordLabel).toString(TagTextEncoding::Utf8));
-                CPPUNIT_ASSERT_EQUAL("2017"s, tag->value(KnownField::RecordDate).toString(TagTextEncoding::Utf8));
-                CPPUNIT_ASSERT_EQUAL(1, tag->value(KnownField::TrackPosition).toPositionInSet().position());
-                CPPUNIT_ASSERT(tag->value(KnownField::Length).toTimeSpan().isNull());
-                CPPUNIT_ASSERT(tag->value(KnownField::Lyricist).isEmpty());
-                break;
-            default:;
             }
+            const auto *const id3v2Tag = static_cast<const Id3v2Tag *>(tag);
+
+            // check values as usual
+            CPPUNIT_ASSERT_EQUAL(expectId3v24 ? 4 : 3, static_cast<int>(id3v2Tag->majorVersion()));
+            CPPUNIT_ASSERT_EQUAL(
+                expectId3v24 ? TagTextEncoding::Utf8 : TagTextEncoding::Utf16LittleEndian, tag->value(KnownField::Title).dataEncoding());
+            CPPUNIT_ASSERT_EQUAL("Infinite (Original Mix)"s, tag->value(KnownField::Title).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("B-Front"s, tag->value(KnownField::Artist).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("Infinite"s, tag->value(KnownField::Album).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("Hardstyle"s, tag->value(KnownField::Genre).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("Lavf57.83.100"s, tag->value(KnownField::EncoderSettings).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("Roughstate"s, tag->value(KnownField::RecordLabel).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("2017"s, tag->value(KnownField::RecordDate).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL(1, tag->value(KnownField::TrackPosition).toPositionInSet().position());
+            CPPUNIT_ASSERT(tag->value(KnownField::Length).toTimeSpan().isNull());
+            CPPUNIT_ASSERT(tag->value(KnownField::Lyricist).isEmpty());
+
+            // check multiple values
+            const auto &fields = id3v2Tag->fields();
+            auto genreFields = fields.equal_range(Id3v2FrameIds::lGenre);
+            CPPUNIT_ASSERT_MESSAGE("genre field present"s, genreFields.first != genreFields.second);
+            const auto &genreField = genreFields.first->second;
+            const auto &additionalValues = genreField.additionalValues();
+            CPPUNIT_ASSERT_EQUAL("Hardstyle"s, tag->value(KnownField::Genre).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL(3_st, additionalValues.size());
+            CPPUNIT_ASSERT_EQUAL("Test"s, additionalValues[0].toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("Example"s, additionalValues[1].toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("Hard Dance"s, additionalValues[2].toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_EQUAL("Hardstyle"s, tag->value(KnownField::Genre).toString(TagTextEncoding::Utf8));
+            CPPUNIT_ASSERT_MESSAGE("exactly one genre field present"s, ++genreFields.first == genreFields.second);
         }
-        CPPUNIT_ASSERT_GREATEREQUAL(2_st, m_diag.size());
-        CPPUNIT_ASSERT_EQUAL(DiagLevel::Warning, m_diag[0].level());
-        CPPUNIT_ASSERT_EQUAL(DiagLevel::Warning, m_diag[1].level());
-        CPPUNIT_ASSERT_EQUAL("parsing TCON frame"s, m_diag[1].context());
-        CPPUNIT_ASSERT_EQUAL(
-            "Multiple strings found. This is not supported so far. Hence the additional values \"Test\", \"Example\" and \"Hard Dance\" are ignored."s,
-            m_diag[1].message());
-        break;
-    case TagStatus::TestMetaDataPresent:
-        checkMp3TestMetaData();
         break;
     case TagStatus::Removed:
         CPPUNIT_ASSERT_EQUAL(0_st, tracks.size());
     }
 
+    if (expectId3v24) {
+        CPPUNIT_ASSERT(m_diag.level() <= DiagLevel::Information);
+        return;
+    }
     CPPUNIT_ASSERT(m_diag.level() <= DiagLevel::Warning);
+    int warningCount = 0;
+    for (const auto &msg : m_diag) {
+        if (msg.level() != DiagLevel::Warning) {
+            continue;
+        }
+        ++warningCount;
+        TESTUTILS_ASSERT_LIKE("context", "(parsing|making) (TPE1|TCON)( frame)?", msg.context());
+        TESTUTILS_ASSERT_LIKE("message",
+            "Multiple strings (found|assigned) .*"
+            "Additional (value \"Second Artist Example\" is|"
+            "values \"Test\", \"Example\" and \"Hard Dance\" are) "
+            "supposed to be ignored.",
+            msg.message());
+    }
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("exactly 4 warnings present", 4, warningCount);
 }
 
 /*!
@@ -231,7 +259,10 @@ void OverallTests::checkMp3PaddingConstraints()
     // TODO: check rewriting behaviour
 }
 
-void OverallTests::setMp3TestMetaData()
+/*!
+ * \brief Sets meta-data for "mtx-test-data/mp3/id3-tag-and-xing-header.mp3".
+ */
+void OverallTests::setMp3TestMetaData1()
 {
     using namespace Mp3TestFlags;
 
@@ -253,19 +284,31 @@ void OverallTests::setMp3TestMetaData()
     }
 
     // assign some test meta data
-    for (Tag *tag : initializer_list<Tag *>{ id3v1Tag, id3v2Tag }) {
+    for (Tag *const tag : initializer_list<Tag *>{ id3v1Tag, id3v2Tag }) {
-        if (tag) {
+        if (!tag) {
-            tag->setValue(KnownField::Title, m_testTitle);
+            continue;
-            tag->setValue(KnownField::Comment, m_testComment);
-            tag->setValue(KnownField::Album, m_testAlbum);
-            m_preservedMetaData.push(tag->value(KnownField::Artist));
-            tag->setValue(KnownField::TrackPosition, m_testPosition);
-            tag->setValue(KnownField::DiskPosition, m_testPosition);
-            // TODO: set more fields
         }
+        tag->setValue(KnownField::Title, m_testTitle);
+        tag->setValue(KnownField::Comment, m_testComment);
+        tag->setValue(KnownField::Album, m_testAlbum);
+        m_preservedMetaData.push(tag->value(KnownField::Artist));
+        tag->setValue(KnownField::TrackPosition, m_testPosition);
+        tag->setValue(KnownField::DiskPosition, m_testPosition);
+        // TODO: set more fields
     }
 }
 
+/*!
+ * \brief Sets meta-data for "misc/multiple_id3v2_4_values.mp3".
+ */
+void OverallTests::setMp3TestMetaData2()
+{
+    using namespace Mp3TestFlags;
+
+    CPPUNIT_ASSERT_EQUAL(1_st, m_fileInfo.id3v2Tags().size());
+    m_fileInfo.id3v2Tags().front()->setVersion((m_mode & UseId3v24) ? 4 : 3, 0);
+}
+
 /*!
  * \brief Tests the MP3 parser via MediaFileInfo.
  */
@@ -333,8 +376,10 @@ void OverallTests::testMp3Making()
 
         // do actual tests
         m_tagStatus = (m_mode & RemoveTag) ? TagStatus::Removed : TagStatus::TestMetaDataPresent;
-        void (OverallTests::*modifyRoutine)(void) = (m_mode & RemoveTag) ? &OverallTests::removeAllTags : &OverallTests::setMp3TestMetaData;
+        makeFile(TestUtilities::workingCopyPath("mtx-test-data/mp3/id3-tag-and-xing-header.mp3"),
-        makeFile(TestUtilities::workingCopyPath("mtx-test-data/mp3/id3-tag-and-xing-header.mp3"), modifyRoutine, &OverallTests::checkMp3Testfile1);
+            (m_mode & RemoveTag) ? &OverallTests::removeAllTags : &OverallTests::setMp3TestMetaData1, &OverallTests::checkMp3Testfile1);
+        makeFile(TestUtilities::workingCopyPath("misc/multiple_id3v2_4_values.mp3"),
+            (m_mode & RemoveTag) ? &OverallTests::removeAllTags : &OverallTests::setMp3TestMetaData2, &OverallTests::checkMp3Testfile2);
     }
 }
 #endif