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

@@ -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<Id::MkTimestampScale>(element)->getValue();
@@ -61,12 +60,19 @@ void EBML::MkInfo::parse(Matroska::Properties *properties) const
     else if(id == Id::MkDuration) {
       duration = element_cast<Id::MkDuration>(element)->getValueAsDouble();
     }
-    else if(id == Id::MkTitle) {
-      title = element_cast<Id::MkTitle>(element)->getValue();
-    }
   }
 
   properties->setLengthInMilliseconds(
     static_cast<int>(duration * static_cast<double>(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<Id::MkTitle>(element)->getValue();
+    }
+  }
+  return String();
 }

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

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();
+}

taglib/matroska/ebml/ebmlmksegment.h

@@ -60,6 +60,7 @@ namespace TagLib {
       std::unique_ptr<Matroska::Segment> parseSegment() const;
       void parseInfo(Matroska::Properties *properties) const;
       void parseTracks(Matroska::Properties *properties) const;
+      String parseSegmentTitle() const;
 
     private:
       std::unique_ptr<MkTags> tags;

taglib/matroska/matroskafile.cpp

@@ -63,6 +63,9 @@ public:
   std::unique_ptr<Cues> cues;
   std::unique_ptr<Segment> segment;
   std::unique_ptr<Properties> 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<Tag>();
-    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 &&

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");