Handle TRACKTOTAL/DISCTOTAL/PARTTOTAL fields in Vorbis Comments

* Move those fields into their corresponding
  TRACKNUMBER/DISCNUMBER/PARTNUMBER fields after parsing so they are
  accessible via just one field as PositionInSet which is in line with
  other tag formats and also how other software like VLC expect the total
  to be specified
* NOT implemented yet: Move those fields optionally back into separate
  fields when serializing
This commit is contained in:
Martchus 2024-01-04 17:24:36 +01:00
parent 351e953b83
commit acfb9ef219
8 changed files with 140 additions and 5 deletions

View File

@ -115,7 +115,11 @@ void FlacStream::internalParseHeader(Diagnostics &diag, AbortableProgressFeedbac
m_vorbisComment = make_unique<VorbisComment>();
}
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
}

View File

@ -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

View File

@ -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 &params = 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");

View File

@ -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;

View File

@ -4,6 +4,8 @@
#include "../abstracttrack.h"
#include "../tag.h"
#include "../vorbis/vorbiscomment.h"
#include "../vorbis/vorbiscommentfield.h"
#include "../vorbis/vorbiscommentids.h"
#include <c++utilities/io/misc.h>
@ -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());
}

View File

@ -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<std::int32_t>();
auto fieldsIter = fields().equal_range(std::string(totalField));
auto fieldsDist = std::distance(fieldsIter.first, fieldsIter.second);
if (!fieldsDist) {
return;
}
totalValues.reserve(static_cast<std::size_t>(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 <class StreamType> 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);
}
}
/*!

View File

@ -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<VorbisComment> {
friend class FieldMapBasedTag<VorbisComment>;
friend class ::OverallTests;
public:
VorbisComment();
@ -52,6 +55,8 @@ protected:
private:
template <class StreamType> 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;

View File

@ -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