From a27dca557e85db1e23de33d2118cde87c8f32770 Mon Sep 17 00:00:00 2001 From: HomerJau Date: Tue, 19 May 2026 15:57:43 +1000 Subject: [PATCH] [Matroska] Allow Chapters Without a ChapterUID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix: Don't silently drop ChapterAtom elements that omit MkChapterUID parseChapterAtom() returned chapterUid = 0 when the source file omitted the MkChapterUID element, and the existing call sites in MkChapters::parse() (ebmlmkchapters.cpp lines 106 and 127) use a C++17 init-if `chapter.uid()` predicate that silently dropped such chapters. The caller saw an empty ChapterEditionList for files that mkvinfo, MediaInfo and FFmpeg all handle gracefully — produced by some audiobook generators and older muxers that omit the per-chapter UID. The companion orphan-EditionEntry fix in PR #1311 / commit e07b956f doesn't cover this case because the ChapterAtoms there ARE wrapped in an EditionEntry, just without a ChapterUID inside each atom. This change: - Synthesises a process-unique ChapterUID inside parseChapterAtom() when the source file lacks MkChapterUID. The synthetic value sets the high bit (1ULL << 63) and increments via a static std::atomic counter; real ChapterUIDs are random 64-bit values from muxers, so collision with a generated one is practically impossible while keeping the distinction local to TagLib. - The existing chapter.uid() filters at the call sites then always evaluate truthy and the chapter is exposed through the public ChapterEditionList API as if it were spec-compliant. No existing behavior is changed — files that already conform to the spec (ChapterAtoms with a ChapterUID element) parse identically; only previously-dropped chapters are now surfaced. Reported and verified against real-world chaptered Matroska audiobook files where mkvinfo / MediaInfo see all chapters but TagLib 2.3 returned an empty ChapterEditionList. --- taglib/matroska/ebml/ebmlmkchapters.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/taglib/matroska/ebml/ebmlmkchapters.cpp b/taglib/matroska/ebml/ebmlmkchapters.cpp index 2271ab1b..1544f863 100644 --- a/taglib/matroska/ebml/ebmlmkchapters.cpp +++ b/taglib/matroska/ebml/ebmlmkchapters.cpp @@ -25,6 +25,7 @@ #include "ebmlmkchapters.h" #include "ebmlstringelement.h" +#include #include "ebmluintelement.h" #include "matroskachapters.h" #include "matroskachapteredition.h" @@ -33,6 +34,20 @@ using namespace TagLib; namespace { +// Counter for synthesising a unique ChapterUID when the source file omits +// the MkChapterUID element (out-of-spec but produced by some muxers, e.g. +// older FFmpeg or audiobook generators). MKVToolNix / MediaInfo / FFmpeg +// all tolerate UID-less chapters; without this synth, the call sites in +// MkChapters::parse() filter such chapters out via the `chapter.uid()` +// check and they are silently lost. +// +// High bit set as a marker; counter starts at 1 and increments per atom +// so each parsed UID-less chapter gets a distinct value within the +// process lifetime. Real ChapterUIDs are random 64-bit values from +// muxers; the (1ULL << 63) | n encoding makes collision practically +// impossible while keeping the distinction local to TagLib. +static std::atomic synthesisedChapterUidCounter{1}; + Matroska::Chapter parseChapterAtom( const std::unique_ptr &atomElement) { @@ -67,6 +82,13 @@ Matroska::Chapter parseChapterAtom( } } } + // Spec-noncompliant: ChapterAtom missing ChapterUID. Synthesise a + // process-unique UID so the chapter survives downstream `chapter.uid()` + // filters. See synthesisedChapterUidCounter comment above. + if(chapterUid == 0) + chapterUid = (1ULL << 63) | + synthesisedChapterUidCounter.fetch_add(1, std::memory_order_relaxed); + return Matroska::Chapter(chapterTimeStart, chapterTimeEnd, chapterDisplays, chapterUid, chapterHidden); }