diff --git a/flac/flacstream.cpp b/flac/flacstream.cpp index e6e7b3a..d6c898d 100644 --- a/flac/flacstream.cpp +++ b/flac/flacstream.cpp @@ -115,7 +115,11 @@ void FlacStream::internalParseHeader(Diagnostics &diag, AbortableProgressFeedbac m_vorbisComment = make_unique(); } try { - m_vorbisComment->parse(*m_istream, header.dataSize(), VorbisCommentFlags::NoSignature | VorbisCommentFlags::NoFramingByte, diag); + auto flags = VorbisCommentFlags::NoSignature | VorbisCommentFlags::NoFramingByte; + if (m_mediaFileInfo.fileHandlingFlags() & MediaFileHandlingFlags::ConvertTotalFields) { + flags += VorbisCommentFlags::ConvertTotalFields; + } + m_vorbisComment->parse(*m_istream, header.dataSize(), flags, diag); } catch (const Failure &) { // error is logged via notifications, just continue with the next metadata block } diff --git a/mediafileinfo.h b/mediafileinfo.h index 8c029bb..2f50cb1 100644 --- a/mediafileinfo.h +++ b/mediafileinfo.h @@ -66,6 +66,9 @@ enum class MediaFileHandlingFlags : std::uint64_t { PreserveRawTimingValues = (1 << 8), /**< preverves raw timing values (so far only used when making MP4 tracks) */ PreserveMuxingApplication = (1 << 9), /**< preverves the muxing application (so far only used when making Matroska container) */ PreserveWritingApplication = (1 << 10), /**< preverves the writing application (so far only used when making Matroska container) */ + ConvertTotalFields = (1 << 11), /**< ensures fields usually holding PositionInSet values such as KnownField::TrackPosition are actually + stored as such (and *not* as two separate fields for the position and total values); currently only relevant for Vorbis Comments + \sa VorbisCommentFlags::ConvertTotalFields */ }; } // namespace TagParser diff --git a/ogg/oggcontainer.cpp b/ogg/oggcontainer.cpp index 5b61462..c1a5c9f 100644 --- a/ogg/oggcontainer.cpp +++ b/ogg/oggcontainer.cpp @@ -280,6 +280,10 @@ void OggContainer::internalParseTags(Diagnostics &diag, AbortableProgressFeedbac { // tracks needs to be parsed before because tags are stored at stream level parseTracks(diag, progress); + auto flags = VorbisCommentFlags::None; + if (fileInfo().fileHandlingFlags() & MediaFileHandlingFlags::ConvertTotalFields) { + flags += VorbisCommentFlags::ConvertTotalFields; + } for (auto &comment : m_tags) { OggParameter ¶ms = comment->oggParams(); m_iterator.setPageIndex(params.firstPageIndex); @@ -287,16 +291,16 @@ void OggContainer::internalParseTags(Diagnostics &diag, AbortableProgressFeedbac m_iterator.setFilter(m_iterator.currentPage().streamSerialNumber()); switch (params.streamFormat) { case GeneralMediaFormat::Vorbis: - comment->parse(m_iterator, VorbisCommentFlags::None, diag); + comment->parse(m_iterator, flags, diag); break; case GeneralMediaFormat::Opus: // skip header (has already been detected by OggStream) m_iterator.ignore(8); - comment->parse(m_iterator, VorbisCommentFlags::NoSignature | VorbisCommentFlags::NoFramingByte, diag); + comment->parse(m_iterator, flags | VorbisCommentFlags::NoSignature | VorbisCommentFlags::NoFramingByte, diag); break; case GeneralMediaFormat::Flac: m_iterator.ignore(4); - comment->parse(m_iterator, VorbisCommentFlags::NoSignature | VorbisCommentFlags::NoFramingByte, diag); + comment->parse(m_iterator, flags | VorbisCommentFlags::NoSignature | VorbisCommentFlags::NoFramingByte, diag); break; default: diag.emplace_back(DiagLevel::Critical, "Stream format not supported.", "parsing tags from OGG streams"); diff --git a/tests/overall.h b/tests/overall.h index ad3dfd1..fef12f5 100644 --- a/tests/overall.h +++ b/tests/overall.h @@ -50,6 +50,7 @@ class OverallTests : public TestFixture { CPPUNIT_TEST(testFlacMaking); CPPUNIT_TEST(testMkvMakingWithDifferentSettings); CPPUNIT_TEST(testMkvMakingNestedTags); + CPPUNIT_TEST(testVorbisCommentFieldHandling); CPPUNIT_TEST_SUITE_END(); public: @@ -122,6 +123,7 @@ public: void testMp3Making(); void testOggMaking(); void testFlacMaking(); + void testVorbisCommentFieldHandling(); private: MediaFileInfo m_fileInfo; diff --git a/tests/overallogg.cpp b/tests/overallogg.cpp index bfed567..c1acb98 100644 --- a/tests/overallogg.cpp +++ b/tests/overallogg.cpp @@ -4,6 +4,8 @@ #include "../abstracttrack.h" #include "../tag.h" #include "../vorbis/vorbiscomment.h" +#include "../vorbis/vorbiscommentfield.h" +#include "../vorbis/vorbiscommentids.h" #include @@ -250,3 +252,52 @@ void OverallTests::testOggMaking() makeFile(workingCopyPath("ogg/noise-without-cover.opus"), modifyRoutineCover, &OverallTests::checkOggTestfile3); } } + +/*! + * \brief Tests the Vorbis Comment specifc handling of certain fields done in VorbisComment::convertTotalFields(). + */ +void OverallTests::testVorbisCommentFieldHandling() +{ + const auto context = std::string(); + const auto trackNumberFieldId = std::string(VorbisCommentIds::trackNumber()); + const auto trackTotalFieldId = std::string(VorbisCommentIds::trackTotal()); + const auto diskNumberFieldId = std::string(VorbisCommentIds::diskNumber()); + const auto diskTotalFieldId = std::string(VorbisCommentIds::diskTotal()); + + auto diag = Diagnostics(); + auto vc = VorbisComment(); + auto trackNumber = VorbisCommentField(trackNumberFieldId, TagValue(5)); + auto trackTotal = VorbisCommentField(trackTotalFieldId, TagValue(20)); + auto &fields = vc.fields(); + fields.insert(std::make_pair(trackNumberFieldId, std::move(trackNumber))); + fields.insert(std::make_pair(trackTotalFieldId, std::move(trackTotal))); + vc.convertTotalFields(context, diag); + + const auto convertedValues = vc.values(trackNumberFieldId); + CPPUNIT_ASSERT_EQUAL_MESSAGE("the two fileds have been combined into one", 1_st, fields.size()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("there is exactly one track number value", 1_st, convertedValues.size()); + const auto convertedTrackNumber = convertedValues.front()->toPositionInSet(); + CPPUNIT_ASSERT_EQUAL(PositionInSet(5, 20), convertedTrackNumber); + CPPUNIT_ASSERT_EQUAL(0_st, diag.size()); + + auto diskNumber = VorbisCommentField(diskNumberFieldId, TagValue("invalid pos")); + auto diskTotal = VorbisCommentField(diskTotalFieldId, TagValue("invalid total")); + auto diskTotal2 = VorbisCommentField(diskTotalFieldId, TagValue(42)); + fields.insert(std::make_pair(diskNumberFieldId, std::move(diskNumber))); + fields.insert(std::make_pair(diskTotalFieldId, std::move(diskTotal))); + fields.insert(std::make_pair(diskTotalFieldId, std::move(diskTotal2))); + vc.convertTotalFields(context, diag); + + const auto newDiskNumberValues = vc.values(diskNumberFieldId); + const auto newDiskTotalValues = vc.values(diskTotalFieldId); + CPPUNIT_ASSERT_EQUAL_MESSAGE("invalid fields have not been combined", 4_st, fields.size()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("invalid disk position has been preserved and valid disk total converted", 2_st, newDiskNumberValues.size()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("invalid disk total has been preserved", 1_st, newDiskTotalValues.size()); + const auto preservedDiskNumber = newDiskNumberValues[0]->toString(); + const auto convertedDiskTotal = newDiskNumberValues[1]->toPositionInSet(); + const auto preservedDiskTotal = newDiskTotalValues[0]->toString(); + CPPUNIT_ASSERT_EQUAL("invalid pos"s, preservedDiskNumber); + CPPUNIT_ASSERT_EQUAL(PositionInSet(0, 42), convertedDiskTotal); + CPPUNIT_ASSERT_EQUAL("invalid total"s, preservedDiskTotal); + CPPUNIT_ASSERT_EQUAL(3_st, diag.size()); +} diff --git a/vorbis/vorbiscomment.cpp b/vorbis/vorbiscomment.cpp index 2385a54..9af63bc 100644 --- a/vorbis/vorbiscomment.cpp +++ b/vorbis/vorbiscomment.cpp @@ -158,6 +158,67 @@ KnownField VorbisComment::internallyGetKnownField(const IdentifierType &id) cons return knownField != fieldMap.cend() ? knownField->second : KnownField::Invalid; } +/// \cond +void VorbisComment::extendPositionInSetField(std::string_view field, std::string_view totalField, const std::string &diagContext, Diagnostics &diag) +{ + auto totalValues = std::vector(); + auto fieldsIter = fields().equal_range(std::string(totalField)); + auto fieldsDist = std::distance(fieldsIter.first, fieldsIter.second); + if (!fieldsDist) { + return; + } + totalValues.reserve(static_cast(fieldsDist)); + for (; fieldsIter.first != fieldsIter.second;) { + try { + totalValues.emplace_back(fieldsIter.first->second.value().toInteger()); + fields().erase(fieldsIter.first++); + } catch (const ConversionException &e) { + diag.emplace_back(DiagLevel::Warning, argsToString("Unable to parse \"", totalField, "\" as integer: ", e.what()), diagContext); + totalValues.emplace_back(0); + ++fieldsIter.first; + } + } + + auto totalIter = totalValues.begin(), totalEnd = totalValues.end(); + for (fieldsIter = fields().equal_range(std::string(field)); fieldsIter.first != fieldsIter.second && totalIter != totalEnd; + ++fieldsIter.first, ++totalIter) { + auto &v = fieldsIter.first->second.value(); + try { + auto p = v.toPositionInSet(); + if (p.total() && p.total() != *totalIter) { + diag.emplace_back(DiagLevel::Warning, + argsToString("The \"", totalField, "\" field value (", *totalIter, ") does not match \"", field, "\" field value (", p.total(), + "). Discarding the former in favor of the latter."), + diagContext); + } else { + p.setTotal(*totalIter); + v.assignPosition(p); + } + } catch (const ConversionException &e) { + diag.emplace_back(DiagLevel::Warning, + argsToString("Unable to parse \"", field, "\" as position in set for incorporating \"", totalField, "\": ", e.what()), diagContext); + } + } + if (totalIter != totalEnd) { + diag.emplace_back( + DiagLevel::Warning, argsToString("Vorbis Comment contains more \"", totalField, "\" fields than \"", field, "\" fields."), diagContext); + } + for (; totalIter != totalEnd; ++totalIter) { + fields().insert(std::make_pair(field, VorbisCommentField(std::string(field), TagValue(PositionInSet(0, *totalIter))))); + } +} +/// \endcond + +/*! + * \brief Converts TRACKTOTAL/DISCTOTAL/PARTTOTAL to be included in the TRACKNUMBER/DISCNUMBER/PARTNUMBER fields instead. + */ +void VorbisComment::convertTotalFields(const std::string &diagContext, Diagnostics &diag) +{ + extendPositionInSetField(VorbisCommentIds::trackNumber(), VorbisCommentIds::trackTotal(), diagContext, diag); + extendPositionInSetField(VorbisCommentIds::diskNumber(), VorbisCommentIds::diskTotal(), diagContext, diag); + extendPositionInSetField(VorbisCommentIds::partNumber(), VorbisCommentIds::partTotal(), diagContext, diag); +} + /*! * \brief Internal implementation for parsing. */ @@ -249,6 +310,10 @@ template void VorbisComment::internalParse(StreamType &stream diag.emplace_back(DiagLevel::Warning, argsToString(bytesRemaining, " bytes left in last segment."), context); } } + + if (flags & VorbisCommentFlags::ConvertTotalFields) { + convertTotalFields(context, diag); + } } /*! diff --git a/vorbis/vorbiscomment.h b/vorbis/vorbiscomment.h index 97765ba..f227ddb 100644 --- a/vorbis/vorbiscomment.h +++ b/vorbis/vorbiscomment.h @@ -7,6 +7,8 @@ #include "../fieldbasedtag.h" #include "../mediaformat.h" +class OverallTests; + namespace TagParser { class OggIterator; @@ -24,6 +26,7 @@ public: class TAG_PARSER_EXPORT VorbisComment : public FieldMapBasedTag { friend class FieldMapBasedTag; + friend class ::OverallTests; public: VorbisComment(); @@ -52,6 +55,8 @@ protected: private: template void internalParse(StreamType &stream, std::uint64_t maxSize, VorbisCommentFlags flags, Diagnostics &diag); + void extendPositionInSetField(std::string_view field, std::string_view totalField, const std::string &diagContext, Diagnostics &diag); + void convertTotalFields(const std::string &diagContext, Diagnostics &diag); private: TagValue m_vendor; diff --git a/vorbis/vorbiscommentfield.h b/vorbis/vorbiscommentfield.h index bdab537..a97455b 100644 --- a/vorbis/vorbiscommentfield.h +++ b/vorbis/vorbiscommentfield.h @@ -19,7 +19,8 @@ enum class VorbisCommentFlags : std::uint8_t { None = 0x0, /**< Regular parsing/making. */ NoSignature = 0x1, /**< Skips the signature when parsing and making. */ NoFramingByte = 0x2, /**< Doesn't expect the framing bit to be present when parsing; does not make the framing bit when making. */ - NoCovers = 0x4 /**< Skips all covers when making. */ + NoCovers = 0x4, /**< Skips all covers when making. */ + ConvertTotalFields = 0x8, /**< Converts TRACKTOTAL/DISCTOTAL/PARTTOTAL to be included in the TRACKNUMBER/DISCNUMBER/PARTNUMBER fields. */ }; } // namespace TagParser