Skip to content

Replace Haali with libmatroska - #692

Draft
CoffeeFlux wants to merge 28 commits into
TypesettingTools:masterfrom
CoffeeFlux:libmatroska-replacement
Draft

CoffeeFlux wants to merge 28 commits into
TypesettingTools:masterfrom
CoffeeFlux:libmatroska-replacement

Conversation

@CoffeeFlux

@CoffeeFlux CoffeeFlux commented Aug 27, 2026 •

Copy link
Copy Markdown
Member

Replaces the bundled Haali MatroskaParser.c with a C++ demuxer, agi::matroska::Demuxer (src/matroska.{h,cpp}), which MatroskaWrapper now uses for subtitle import and HasSubtitles.

Design

  • libebml/libmatroska (system, or WrapDB fallbacks) parse the EBML header, Info and Tracks. Everything else (the top-level walk, SeekHead, Attachments, clusters and blocks) uses a small bounded EBML reader, so only the bytes that are needed get read.
  • Opening a file stops at the first cluster and follows the SeekHead for metadata stored after the clusters, like the Haali parser did. It only walks the whole file if Info or Tracks still haven't been found. HasSubtitles runs on the UI thread whenever video is opened, so this matters on slow storage.
  • Clusters are indexed lazily and read one block at a time, so memory use doesn't grow with cluster size.
  • The demuxer returns plain values rather than pointers into parser state. It has typed errors (IoError, InvalidDataError, LimitError, ...), size limits, and cancellation.
  • Requires libebml >= 1.4.4 and libmatroska >= 1.7.1, the bundled versions, so a system libebml is always new enough for the bundled libmatroska. Older distributions such as Ubuntu 22.04 (1.4.2/1.6.3) use the bundled copies of both.

Behavior changes from master

Subtitle timing. The demuxer reports timestamps as stored in the file. StartTime() is the earliest audio or video timestamp: what FFmpeg (and so LAV/MPC-HC) reports as the start time, and what mpv rebases to for Matroska in practice. Import makes that time zero.

  • For nearly all files this is the same as master. The Haali parser subtracted the first block in the file, which is normally audio or video.
  • Files with only subtitles are no longer shifted to start at zero. Fixes Error in loading the timing (and perhaps not only that) of the subtitles in an MKS. #91.
  • A subtitle line before the first audio/video block no longer moves every other line earlier.
  • Lines starting before the start of the file are clamped to zero, and lines ending before it are dropped.
  • Aegisub's video timeline still makes the first video frame time zero (Lift assumption that video frame 0 occurs at time 0 #21). In files where video starts after audio (e.g. the 83 ms Crunchyroll delay), imported lines therefore look late by that delay inside Aegisub, as they do on master. They are exported with their original timing, and this becomes consistent if the video timeline moves to the file start (e.g. Fix timestamps #508).

Other changes

  • Attachments are only located when opening a file. Their data is read on request.
  • Tracks with encryption or an unsupported content encoding are reported as unsupported, instead of making the whole file fail to open.
  • More than 32 tracks and track numbers above 255 are supported.
  • Partially downloaded files: the complete blocks in a cut-off last cluster are read.
  • Unknown-size (live) clusters are supported.
  • ContentEncodingScope is honored, including a zlib-compressed CodecPrivate.
  • Repeated Tracks elements are tolerated.
  • The deprecated TrackTimecodeScale is applied as RFC 9559 §11.2 specifies.
  • The decompression and total-size limits from 5a53f19 still apply: 16 MiB per packet, 64 MiB of subtitle data in total.

Import UI fixes

  • Cancelling the subtitle-reading progress dialog works again, and no longer uses a dangling progress sink.
  • An error while reading track information is reported instead of crashing.
  • A malformed ASS packet gives an error instead of an uncaught bad_lexical_cast.

Testing

  • tests/matroska/fixtures holds 14 small files. Most are generated by generate.py, which builds the edge cases directly as EBML. A gtest dumps everything the demuxer exposes for each file and compares it with the hand-written .behavior file next to it.
  • The subtitle timing of the fixtures was compared against master's MatroskaParser.c. The remaining differences are the intended ones listed above.
  • On a real 3.9 GB BD release (two ASS tracks, nine fonts), the output is identical to the earlier libmatroska-backed version of this PR. Every packet's start, end and size also matches ffprobe.
  • Start times were compared with ffprobe and mpv, including MKVs remuxed from m2ts with large timestamps.
  • Opening a file for HasSubtitles on a cold cache takes about 1 ms, compared with about 250 ms when walking every cluster.

Not tested: importing through the GUI (progress dialogs, cancelling, the track chooser).

Known limitations

  • libebml allocates whatever size an element inside Info/Tracks declares, so a crafted file can make it allocate up to about 2 GB. This is libebml's behavior and is left as is.

🤖 Generated with Claude Code

@CoffeeFlux
CoffeeFlux marked this pull request as ready for review August 27, 2026 22:20
Final AI Agent and others added 18 commits October 7, 2026 10:47
agi::matroska::Demuxer was implemented on top of a reimplementation of
the Haali MatroskaParser C API, which in turn wrapped libebml and
libmatroska. Fold the parts that are actually used directly into the
demuxer and delete the C layer along with its licence notice.

Behavior changes:
- Attachment payloads are only located while opening a file rather than
  read into memory, which libebml's SCOPE_PARTIAL_DATA did not prevent.
- Tracks with encryption or an unsupported content encoding are reported
  as unsupported instead of making the whole file fail to open.
- Subtitle tracks are no longer limited to the first 32 tracks.
- Reader failures while opening are reported as IoError.
- Duration() is nullopt when the file doesn't specify one.
- A cluster which fails to parse while computing the duration from the
  end of the file is skipped rather than failing to open the file.
- Default per-packet limits are 16 MiB to match 5a53f19.

The fixture test now dumps through the public Demuxer interface as part
of the gtest suite instead of a separate executable using the C API. Its
output on the existing fixtures is unchanged from the previous
implementation other than unknown durations, and a fixture covering
header stripping, block durations and encrypted tracks is added.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The demuxer's cancellation callback captured the metadata progress
  dialog's sink, which is destroyed when that dialog finishes and was then
  used while reading subtitles. Route it to whichever dialog is running.
- Errors while reading track information were only logged to the progress
  dialog, after which the empty demuxer was dereferenced. Rethrow them.
- Restore the 64 MiB limit on total subtitle data from 5a53f19.
- Report malformed ASS packets instead of letting bad_lexical_cast escape.
- Restore the file's original licence header.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CoffeeFlux
CoffeeFlux force-pushed the libmatroska-replacement branch 2 times, most recently from 7672447 to c0c2969 Compare October 7, 2026 18:13
CoffeeFlux and others added 8 commits October 7, 2026 11:34
- Honor ContentEncodingScope: compression which doesn't cover frame
  contents is no longer applied to frames, and a zlib-compressed
  CodecPrivate is decompressed.
- Skip track entries whose UID was already seen, as Tracks may be
  repeated in live streams, rather than failing on the duplicate number.
- Make AttachmentId an index and expose the UID separately, so that
  attachments with missing or duplicate UIDs are no longer dropped, and
  skip attachments without data as the Haali parser did.
- Fix signed overflow when a block's relative timestamp is added to a
  cluster timestamp of INT64_MAX.
- Poll for cancellation per element while parsing a cluster, as clusters
  can be arbitrarily large.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Make block timestamps relative to the first block in the file, as the
  Haali parser did. Video providers normalize frame timestamps to start at
  zero, so without this subtitles in files which don't start at zero were
  imported out of sync.
- Apply the deprecated TrackTimecodeScale to block timestamps and
  durations, as the Haali parser did.
- Skip elements such as Void between the EBML header and the Segment.
  libebml's FindNextID returns whatever element comes next, so these were
  being treated as the Segment, and anything at all was being accepted as
  the EBML header.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Opening a file walked every top-level element with libebml, indexing all
clusters before returning. MatroskaWrapper::HasSubtitles does this on the
UI thread whenever video is opened, which means reads from throughout the
file. Like the Haali parser, stop at the first cluster and follow the
SeekHead for metadata stored after the clusters, only walking the entire
file if the tracks still haven't been found. Clusters are now indexed as
packets are read, and the fallback duration is computed the first time
Duration() is called rather than while opening the file.

This walks the segment with the raw EBML reader and only uses libebml to
parse Info and Tracks. That fixes a Void before a CRC-32-prefixed Info or
Tracks element leaving libebml's nesting level negative, which made it
stop reading the element's children. The existing video-only fixture,
written by ffmpeg, has this layout and was losing its duration.

Also:
- Compute block timestamps relative to the first block exactly, so only
  the relative timestamp needs to fit in 64 bits.
- Find the end of unknown-size clusters, as live muxers write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fixture test compares describe() output with the .behavior files
byte-for-byte, which failed when Git converted them to CRLF on checkout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The demuxer now reports timestamps as stored in the file rather than
relative to the first block, and exposes the earliest audio or video
timestamp as StartTime(). This is what FFmpeg (and so MPC-HC/LAV) reports
as the start time, and what mpv uses for Matroska in practice, and both
rebase playback so that it is time zero.

MatroskaWrapper subtracts it from imported subtitles. This matches the
Haali parser for nearly all files, as their first block is normally audio
or video, while fixing two cases where that differed:

- Files with only subtitles are no longer shifted to start at zero
  (TypesettingTools#91), as they are never played by themselves.
- A subtitle line before the first audio or video block no longer moves
  every other line earlier.

Lines which start before the start of the file are clamped to start at
zero, and lines which end before it are dropped.

Aegisub's own video timeline makes the first video frame time zero
instead (TypesettingTools#21), so in files where video starts
after audio, imported lines appear late by that delay, as they did before.
They are exported with their original timing, and this will be consistent
if the video timeline is changed to use the start of the file.

The deprecated TrackTimecodeScale is now applied to the stored timestamp
as the specification describes, rather than to the offset from the first
block.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A cluster cut off by the end of the file was rejected entirely, which
  failed to open the file if it was the first cluster and otherwise
  dropped its subtitles. Read the blocks in it which are complete.
- Walk the whole file when Info, not only Tracks, wasn't found before the
  first cluster or via a SeekHead, as otherwise timestamps are scaled
  with the default TimestampScale.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Read clusters a block at a time rather than collecting every frame in a
  cluster before returning the first. Clusters have no size limit and
  lacing turns each block into up to 256 frames, so a crafted file could
  use an unbounded amount of memory before any packet limit was checked.
- Choose libebml based on which libmatroska is used. With an older system
  libebml and no system libmatroska, the bundled libmatroska requires a
  newer libebml than the one already found, which failed to configure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RFC 9559 section 11.2 gives a block's timestamp as
(Cluster\Timestamp + block timestamp * TrackTimestampScale) * TimestampScale,
so the track scale doesn't apply to the cluster's timestamp. The previous
commit wrongly scaled the sum.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CoffeeFlux
CoffeeFlux marked this pull request as draft October 7, 2026 22:53
CoffeeFlux and others added 2 commits October 7, 2026 19:03
Rather than choosing libebml based on which libmatroska was found, raise
the minimums to the bundled versions so a system libebml is always new
enough for the bundled libmatroska. Older distributions such as Ubuntu
22.04 now use the bundled copies of both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It was kept separate so the matroska-behavior executable could link it
without everything else, but that has since been folded into the gtest
suite. This also restores the executable's original dependencies line,
which had lost a space of indentation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error in loading the timing (and perhaps not only that) of the subtitles in an MKS.

1 participant