From 1a97d91b27f3a953600d8bca0f4d8da8f1f880be Mon Sep 17 00:00:00 2001 From: Martchus Date: Sun, 1 Jul 2018 23:06:36 +0200 Subject: [PATCH] Preserve multiple strings in ID3v2 text frames --- CMakeLists.txt | 4 +- id3/id3v2frame.cpp | 184 ++++++++++++++++++++++++++++++------------- id3/id3v2frame.h | 41 ++++++++-- tests/overall.h | 3 +- tests/overallmp3.cpp | 123 ++++++++++++++++++++--------- 5 files changed, 251 insertions(+), 104 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1c9bc79..e2278ca 100644 --- a/CMakeLists.txt +++ b/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_MINOR 2) +set(META_VERSION_MAJOR 8) +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) diff --git a/id3/id3v2frame.cpp b/id3/id3v2frame.cpp index f6105f3..644e9a5 100644 --- a/id3/id3v2frame.cpp +++ b/id3/id3v2frame.cpp @@ -250,7 +250,6 @@ void Id3v2Frame::parse(BinaryReader &reader, uint32 version, uint32 maximalSize, TagTextEncoding dataEncoding = parseTextEncodingByte(static_cast(*buffer.get()), diag); // parse string values (since ID3v2.4 a text frame may contain multiple strings) - std::vector 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(); - return &additionalValues.back(); + m_additionalValues.emplace_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()) { - // format list of values - const auto valuesString = [&]() -> string { - 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); - } + if (version < 4 && !m_additionalValues.empty()) { + diag.emplace_back( + DiagLevel::Warning, "Multiple strings found though the tag is pre-ID3v2.4. " + ignoreAdditionalValuesDiagMsg(), 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 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 (value.isEmpty()) { + if (values.empty()) { 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)) { - // make text frames + if (isTextFrame) { + // make text frame + vector 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 - const auto positionStr(value.toString(TagTextEncoding::Latin1)); - // -> warn if value is no valid position (although we just store a string after all) - if (value.type() != TagDataType::PositionInSet) { + encoding = version >= 4 ? TagTextEncoding::Utf8 : TagTextEncoding::Latin1; + for (const auto *const value : values) { + // convert the position to string + 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()); - if (duration.isNegative()) { - diag.emplace_back(DiagLevel::Critical, argsToString("Assigned duration \"", duration.toString(), "\" is negative."), context); - throw InvalidDataException(); + encoding = TagTextEncoding::Latin1; + for (const auto *const value : values) { + const auto duration(value->toTimeSpan()); + 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(duration.totalMilliseconds()))); } - m_frame.makeString(m_data, m_decompressedSize, ConversionUtilities::numberToString(static_cast(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 - if (version <= 3 && value.dataEncoding() == TagTextEncoding::Utf8) { - // UTF-8 is only supported by ID3v2.4, so convert back to UTF-16 - m_frame.makeString( - m_data, m_decompressedSize, value.toString(TagTextEncoding::Utf16LittleEndian), TagTextEncoding::Utf16LittleEndian); - } else { - // just keep encoding of the assigned value - m_frame.makeString(m_data, m_decompressedSize, value.toString(), value.dataEncoding()); + // make standard genre index and other text frames + // -> find text encoding suitable for all assigned values + for (const auto *const value : values) { + switch (encoding) { + case TagTextEncoding::Unspecified: + switch (value->type()) { + case TagDataType::StandardGenreIndex: + 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(m_decompressedSize = static_cast(1 + concatenatedSubstrings.size())); + m_data[0] = static_cast(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(); } diff --git a/id3/id3v2frame.h b/id3/id3v2frame.h index 74a66ad..1a5c224 100644 --- a/id3/id3v2frame.h +++ b/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 { friend class TagField; + friend class Id3v2FrameMaker; public: Id3v2Frame(); @@ -94,6 +97,8 @@ public: void make(IoUtilities::BinaryWriter &writer, byte version, Diagnostics &diag); // member access + const std::vector &additionalValues() const; + std::vector &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); - std::size_t makeBom(char *buffer, TagTextEncoding encoding); - void makeString(std::unique_ptr &buffer, uint32 &bufferSize, const std::string &value, TagTextEncoding encoding); - void makeEncodingAndData(std::unique_ptr &buffer, uint32 &bufferSize, TagTextEncoding encoding, const char *data, std::size_t m_dataSize); - void makeLegacyPicture(std::unique_ptr &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo); - void makePicture(std::unique_ptr &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo, byte version); - void makeComment(std::unique_ptr &buffer, uint32 &bufferSize, const TagValue &comment, byte version, Diagnostics &diag); + static byte makeTextEncodingByte(TagTextEncoding textEncoding); + static std::size_t makeBom(char *buffer, TagTextEncoding encoding); + static void makeString(std::unique_ptr &buffer, uint32 &bufferSize, const std::string &value, TagTextEncoding encoding); + static void makeEncodingAndData( + std::unique_ptr &buffer, uint32 &bufferSize, TagTextEncoding encoding, const char *data, std::size_t m_dataSize); + static void makeLegacyPicture(std::unique_ptr &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo); + static void makePicture(std::unique_ptr &buffer, uint32 &bufferSize, const TagValue &picture, byte typeInfo, byte version); + static void makeComment(std::unique_ptr &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 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 &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 &Id3v2Frame::additionalValues() +{ + return m_additionalValues; +} + /*! * \brief Returns whether the instance uses the additional type info. */ diff --git a/tests/overall.h b/tests/overall.h index 6a0174e..a943b09 100644 --- a/tests/overall.h +++ b/tests/overall.h @@ -105,7 +105,8 @@ private: void setMkvTestMetaData(); void setMp4TestMetaData(); - void setMp3TestMetaData(); + void setMp3TestMetaData1(); + void setMp3TestMetaData2(); void setOggTestMetaData(); void removeAllTags(); void noop(); diff --git a/tests/overallmp3.cpp b/tests/overallmp3.cpp index 4e946ad..ee6367a 100644 --- a/tests/overallmp3.cpp +++ b/tests/overallmp3.cpp @@ -6,6 +6,8 @@ #include "../id3/id3v2tag.h" #include "../mpegaudio/mpegaudioframe.h" +#include + 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()) { - case TagType::Id3v1Tag: - 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:; + if (tag->type() != TagType::Id3v2Tag) { + CPPUNIT_FAIL(argsToString("no ", tag->typeName(), " tag expected")); } + const auto *const id3v2Tag = static_cast(tag); + + // check values as usual + CPPUNIT_ASSERT_EQUAL(expectId3v24 ? 4 : 3, static_cast(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{ id3v1Tag, id3v2Tag }) { - if (tag) { - 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 + for (Tag *const tag : initializer_list{ id3v1Tag, id3v2Tag }) { + if (!tag) { + continue; } + 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"), modifyRoutine, &OverallTests::checkMp3Testfile1); + makeFile(TestUtilities::workingCopyPath("mtx-test-data/mp3/id3-tag-and-xing-header.mp3"), + (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