[Matroska] Allow Chapters Without a ChapterUID

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.
This commit is contained in:
HomerJau
2026-05-19 15:57:43 +10:00
committed by Urs Fleisch
parent 8fcda2daa2
commit a27dca557e

View File

@@ -25,6 +25,7 @@
#include "ebmlmkchapters.h"
#include "ebmlstringelement.h"
#include <atomic>
#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<Matroska::Chapter::UID> synthesisedChapterUidCounter{1};
Matroska::Chapter parseChapterAtom(
const std::unique_ptr<EBML::Element> &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);
}