From e07b956fdadc7c3a1012f5293f1843de650585bf Mon Sep 17 00:00:00 2001 From: HomerJau Date: Sun, 26 Apr 2026 12:00:48 +1000 Subject: [PATCH] [Matroska] Allow Orphaned Chapter Reading (when Chapter has no EditionID) Fix: Handle orphan ChapterAtom elements not wrapped in EditionEntry The Matroska specification requires every ChapterAtom to be inside an EditionEntry. However, some muxers (older FFmpeg versions, some streaming tools) produce files with ChapterAtom elements directly under Chapters, without an EditionEntry wrapper. MKVToolNix and FFmpeg both handle this case gracefully by treating orphan atoms as belonging to an implicit default edition. Previously, TagLib silently ignored these chapters, returning an empty ChapterEditionList. This change: - Collects orphan ChapterAtom elements encountered directly under Chapters - Wraps them in an implicit default edition (UID = 0, isDefault = true, isOrdered = false) so they are exposed through the existing chapterEditionList() API - Extracts the atom-parsing logic into a private parseChapterAtom() helper to avoid code duplication between the two call sites No existing behavior is changed - files that already conform to the spec (chapters inside an EditionEntry) parse identically. --- taglib/matroska/ebml/ebmlmkchapters.cpp | 100 ++++++++++++++++-------- 1 file changed, 66 insertions(+), 34 deletions(-) diff --git a/taglib/matroska/ebml/ebmlmkchapters.cpp b/taglib/matroska/ebml/ebmlmkchapters.cpp index d8d3c136..4f9c71ff 100644 --- a/taglib/matroska/ebml/ebmlmkchapters.cpp +++ b/taglib/matroska/ebml/ebmlmkchapters.cpp @@ -31,6 +31,48 @@ using namespace TagLib; +namespace { + +Matroska::Chapter parseChapterAtom( + const std::unique_ptr &atomElement) +{ + Matroska::Chapter::UID chapterUid = 0; + Matroska::Chapter::Time chapterTimeStart = 0; + Matroska::Chapter::Time chapterTimeEnd = 0; + List chapterDisplays; + bool chapterHidden = false; + + const auto chapterAtom = EBML::element_cast(atomElement); + for(const auto &chapterChild : *chapterAtom) { + if(const EBML::Element::Id cid = chapterChild->getId(); cid == EBML::Element::Id::MkChapterUID) + chapterUid = EBML::element_cast(chapterChild)->getValue(); + else if(cid == EBML::Element::Id::MkChapterTimeStart) + chapterTimeStart = EBML::element_cast(chapterChild)->getValue(); + else if(cid == EBML::Element::Id::MkChapterTimeEnd) + chapterTimeEnd = EBML::element_cast(chapterChild)->getValue(); + else if(cid == EBML::Element::Id::MkChapterFlagHidden) + chapterHidden = EBML::element_cast(chapterChild)->getValue() != 0; + else if (cid == EBML::Element::Id::MkChapterDisplay) { + const auto display = EBML::element_cast(chapterChild); + String displayString; + String displayLanguage; + for(const auto &displayChild : *display) { + if (const EBML::Element::Id did = displayChild->getId(); did == EBML::Element::Id::MkChapString) + displayString = EBML::element_cast(displayChild)->getValue(); + else if(did == EBML::Element::Id::MkChapLanguage) + displayLanguage = EBML::element_cast(displayChild)->getValue(); + } + if(!displayString.isEmpty()) { + chapterDisplays.append(Matroska::Chapter::Display(displayString, displayLanguage)); + } + } + } + return Matroska::Chapter(chapterTimeStart, chapterTimeEnd, chapterDisplays, + chapterUid, chapterHidden); +} + +} // namespae + EBML::MkChapters::MkChapters(int sizeLength, offset_t dataSize, offset_t offset): MasterElement(Id::MkChapters, sizeLength, dataSize, offset) { @@ -41,7 +83,7 @@ EBML::MkChapters::MkChapters(Id, int sizeLength, offset_t dataSize, offset_t off { } -EBML::MkChapters::MkChapters(): +EBML::MkChapters::MkChapters() : MasterElement(Id::MkChapters, 0, 0, 0) { } @@ -52,7 +94,20 @@ std::unique_ptr EBML::MkChapters::parse() const chapters->setOffset(offset); chapters->setSize(getSize()); + // Collect any orphan ChapterAtom elements not wrapped in an EditionEntry. + // The Matroska spec requires ChapterAtom to be inside an EditionEntry, but + // some muxers produce files with ChapterAtom directly under Chapters. + // MKVToolNix and FFmpeg handle this case by treating the orphan atoms as + // belonging to an implicit default edition. + List orphanChapters; + for(const auto &element : elements) { + if(element->getId() == Id::MkChapterAtom) { + if(auto chapter = parseChapterAtom(element); chapter.uid()) { + orphanChapters.append(chapter); + } + continue; + } if(element->getId() != Id::MkEditionEntry) continue; @@ -69,39 +124,8 @@ std::unique_ptr EBML::MkChapters::parse() const else if(id == Id::MkEditionFlagOrdered) editionIsOrdered = element_cast(editionChild)->getValue() != 0; else if(id == Id::MkChapterAtom) { - Matroska::Chapter::UID chapterUid = 0; - Matroska::Chapter::Time chapterTimeStart = 0; - Matroska::Chapter::Time chapterTimeEnd = 0; - List chapterDisplays; - bool chapterHidden = false; - const auto chapterAtom = element_cast(editionChild); - for(const auto &chapterChild : *chapterAtom) { - if(const Id cid = chapterChild->getId(); cid == Id::MkChapterUID) - chapterUid = element_cast(chapterChild)->getValue(); - else if(cid == Id::MkChapterTimeStart) - chapterTimeStart = element_cast(chapterChild)->getValue(); - else if(cid == Id::MkChapterTimeEnd) - chapterTimeEnd = element_cast(chapterChild)->getValue(); - else if(cid == Id::MkChapterFlagHidden) - chapterHidden = element_cast(chapterChild)->getValue() != 0; - else if(cid == Id::MkChapterDisplay) { - const auto display = element_cast(chapterChild); - String displayString; - String displayLanguage; - for(const auto &displayChild : *display) { - if(const Id did = displayChild->getId(); did == Id::MkChapString) - displayString = element_cast(displayChild)->getValue(); - else if(did == Id::MkChapLanguage) - displayLanguage = element_cast(displayChild)->getValue(); - } - if(!displayString.isEmpty()) { - chapterDisplays.append(Matroska::Chapter::Display(displayString, displayLanguage)); - } - } - } - if(chapterUid) { - editionChapters.append(Matroska::Chapter( - chapterTimeStart, chapterTimeEnd, chapterDisplays, chapterUid, chapterHidden)); + if(auto chapter = parseChapterAtom(editionChild); chapter.uid()) { + editionChapters.append(chapter); } } } @@ -110,5 +134,13 @@ std::unique_ptr EBML::MkChapters::parse() const editionChapters, editionIsDefault, editionIsOrdered, editionUid)); } } + + // If orphan chapters were found, wrap them in an implicit default edition + // so they are not silently lost. + if (!orphanChapters.isEmpty()) { + chapters->addChapterEdition(Matroska::ChapterEdition( + orphanChapters, true, false, 0)); + } + return chapters; }