From b2e27c3a45e58950b50c1b36502a9ef7fcf088db Mon Sep 17 00:00:00 2001 From: Ryan Francesconi <2917795+ryanfrancesconi@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:28:38 -0700 Subject: [PATCH] Matroska: read the segment title without audio properties (#1409) Matroska::Tag::title() falls back to \Segment\Info\Title when a file has no TITLE simple tag. That title was only extracted inside read()'s readProperties branch, because Info is also where the audio properties come from, so opening a file with readAudioProperties = false left the tag with an empty title: FileRef(path, true ).tag()->title(); // "handbrake" FileRef(path, false).tag()->title(); // "" Reading tags without audio properties is the documented fast path for scanning a library, so this silently emptied the title for exactly the callers who opted into it, and it made tag content depend on an audio-properties flag in a way no other format does. Store the segment title on the file itself, read regardless of readProperties, and use it from both read() and tag(). The Info element is already resident by then, so no additional I/O is involved and audioProperties() still returns null when properties were not requested. --- taglib/matroska/ebml/ebmlmkinfo.cpp | 16 +++++++++++----- taglib/matroska/ebml/ebmlmkinfo.h | 7 +++++++ taglib/matroska/ebml/ebmlmksegment.cpp | 5 +++++ taglib/matroska/ebml/ebmlmksegment.h | 1 + taglib/matroska/matroskafile.cpp | 18 ++++++++++++------ tests/test_matroska.cpp | 19 +++++++++++++++++++ 6 files changed, 55 insertions(+), 11 deletions(-) diff --git a/taglib/matroska/ebml/ebmlmkinfo.cpp b/taglib/matroska/ebml/ebmlmkinfo.cpp index a2e4e910..fd5e6f24 100644 --- a/taglib/matroska/ebml/ebmlmkinfo.cpp +++ b/taglib/matroska/ebml/ebmlmkinfo.cpp @@ -53,7 +53,6 @@ void EBML::MkInfo::parse(Matroska::Properties *properties) const unsigned long long timestampScale = 1000000; double duration = 0.0; - String title; for(const auto &element : elements) { if(const Id id = element->getId(); id == Id::MkTimestampScale) { timestampScale = element_cast(element)->getValue(); @@ -61,12 +60,19 @@ void EBML::MkInfo::parse(Matroska::Properties *properties) const else if(id == Id::MkDuration) { duration = element_cast(element)->getValueAsDouble(); } - else if(id == Id::MkTitle) { - title = element_cast(element)->getValue(); - } } properties->setLengthInMilliseconds( static_cast(duration * static_cast(timestampScale) / 1000000.0)); - properties->setTitle(title); + properties->setTitle(parseTitle()); +} + +String EBML::MkInfo::parseTitle() const +{ + for(const auto &element : elements) { + if(element->getId() == Id::MkTitle) { + return element_cast(element)->getValue(); + } + } + return String(); } diff --git a/taglib/matroska/ebml/ebmlmkinfo.h b/taglib/matroska/ebml/ebmlmkinfo.h index c381b46a..00f6c0b7 100644 --- a/taglib/matroska/ebml/ebmlmkinfo.h +++ b/taglib/matroska/ebml/ebmlmkinfo.h @@ -44,6 +44,13 @@ namespace TagLib { MkInfo(); void parse(Matroska::Properties * properties) const; + + /*! + * Returns the segment title, without requiring a Properties instance. + * Used when the file is read without audio properties, where the title + * is still needed for Matroska::Tag::title(). + */ + String parseTitle() const; }; } } diff --git a/taglib/matroska/ebml/ebmlmksegment.cpp b/taglib/matroska/ebml/ebmlmksegment.cpp index 79de5ca5..453289fd 100644 --- a/taglib/matroska/ebml/ebmlmksegment.cpp +++ b/taglib/matroska/ebml/ebmlmksegment.cpp @@ -297,3 +297,8 @@ void EBML::MkSegment::parseTracks(Matroska::Properties *properties) const tracks->parse(properties); } } + +String EBML::MkSegment::parseSegmentTitle() const +{ + return info ? info->parseTitle() : String(); +} diff --git a/taglib/matroska/ebml/ebmlmksegment.h b/taglib/matroska/ebml/ebmlmksegment.h index 3e8f84f1..8634868e 100644 --- a/taglib/matroska/ebml/ebmlmksegment.h +++ b/taglib/matroska/ebml/ebmlmksegment.h @@ -60,6 +60,7 @@ namespace TagLib { std::unique_ptr parseSegment() const; void parseInfo(Matroska::Properties *properties) const; void parseTracks(Matroska::Properties *properties) const; + String parseSegmentTitle() const; private: std::unique_ptr tags; diff --git a/taglib/matroska/matroskafile.cpp b/taglib/matroska/matroskafile.cpp index f542ddbc..c18a5d96 100644 --- a/taglib/matroska/matroskafile.cpp +++ b/taglib/matroska/matroskafile.cpp @@ -63,6 +63,9 @@ public: std::unique_ptr cues; std::unique_ptr segment; std::unique_ptr properties; + // Matroska::Tag::title() falls back to this when the file has no TITLE simple + // tag, so it is read even when the audio properties are not. + String segmentTitle; }; //////////////////////////////////////////////////////////////////////////////// @@ -121,9 +124,7 @@ Matroska::Tag *Matroska::File::tag(bool create) const { if(!d->tag && create) { d->tag = std::make_unique(); - if(d->properties) { - d->tag->setSegmentTitle(d->properties->title()); - } + d->tag->setSegmentTitle(d->segmentTitle); } return d->tag.get(); } @@ -474,9 +475,14 @@ void Matroska::File::read(bool readProperties, Properties::ReadStyle readStyle) segment->parseInfo(d->properties.get()); segment->parseTracks(d->properties.get()); - if(d->tag) { - d->tag->setSegmentTitle(d->properties->title()); - } + d->segmentTitle = d->properties->title(); + } + else { + // The Info element is already in memory, so this costs no additional I/O. + d->segmentTitle = segment->parseSegmentTitle(); + } + if(d->tag) { + d->tag->setSegmentTitle(d->segmentTitle); } if(readStyle == AudioProperties::Accurate && diff --git a/tests/test_matroska.cpp b/tests/test_matroska.cpp index 7057d52b..bf0bc875 100644 --- a/tests/test_matroska.cpp +++ b/tests/test_matroska.cpp @@ -185,6 +185,7 @@ class TestMatroska : public CppUnit::TestFixture CPPUNIT_TEST(testFastReadStyleLargeSegment); CPPUNIT_TEST(testAttachedFileDataReadOnDemand); CPPUNIT_TEST(testSaveUnrequestedAttachedFileData); + CPPUNIT_TEST(testSegmentTitleWithoutAudioProperties); CPPUNIT_TEST_SUITE_END(); public: @@ -1908,6 +1909,24 @@ public: } } + void testSegmentTitleWithoutAudioProperties() + { + // optimized.mkv keeps its title in \Segment\Info\Title and has no TITLE + // simple tag, so Tag::title() falls back to the segment title. That fallback + // must not depend on whether the audio properties were read. + for(const bool readProperties : {true, false}) { + Matroska::File f(TEST_FILE_PATH_C("optimized.mkv"), readProperties); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT_EQUAL(readProperties, f.audioProperties() != nullptr); + CPPUNIT_ASSERT_EQUAL(String("handbrake"), f.tag()->title()); + } + + // A file without a segment title still reports an empty title. + Matroska::File noTitle(TEST_FILE_PATH_C("no-tags.mka"), false); + CPPUNIT_ASSERT(noTitle.isValid()); + CPPUNIT_ASSERT_EQUAL(String(""), noTitle.tag()->title()); + } + void testUnknownSizeSegment() { ScopedFileCopy copy("no-tags", ".mka");