mirror of
https://github.com/freemkv/libfreemkv.git
synced 2026-10-07 18:13:10 +00:00
Security - Remove the unsafe Vec::set_len over recycled sector buffers — UB reachable from safe code (GHSA-j8ww-f5fg-9pmh, low). Uses Vec::resize; pooling unchanged. CI / tooling - clippy on stable with the MSRV single-sourced from Cargo.toml; cargo-deny dependency-audit gate with first-party git sources allowed (#15); audience-based comment-guard (#17). Docs - README (keydb ownership, BD-J parser count), drive-access transport/reset notes, mux timeline module-doc links.
257 lines
13 KiB
Markdown
257 lines
13 KiB
Markdown
# mkvstream internals
|
|
|
|
Relocated rationale/history for comments in `src/mux/mkvstream.rs` that
|
|
exceeded the comment-guard's internal-comment cap. Each section is pointed
|
|
to from a short `// See docs/mkvstream.md#<anchor>` comment at its call site.
|
|
|
|
## ebml-size-caps
|
|
|
|
`MAX_BLOCK_SIZE`: largest accepted SIMPLE_BLOCK payload. A block is a small
|
|
vint track header + 2-byte rel-ts + 1-byte flags + one frame of elementary
|
|
data. UHD HEVC keyframes run a few MB; 64 MiB is generously above any real
|
|
single-frame block while still bounding a hostile allocation.
|
|
|
|
## apply-coding-to-track
|
|
|
|
Set a video track's `FieldOrder` from the MEASURED coding of the first coded
|
|
picture — the parser's value, the first time, never a guess.
|
|
|
|
A progressive track — or a progressive picture on an interlaced-flagged track
|
|
— has no field order (left UNDETERMINED — expected); the latter case ALSO
|
|
clears the track's `interlaced` flag, since the declared scan type came from
|
|
the IFO/MPLS resolution and the measurement supersedes it. An INTERLACED
|
|
track that reaches here WITH a video picture but no measured field order is a
|
|
parser/source gap (MPEG-2 carries `top_field_first` on every interlaced
|
|
picture, so it should never be missing): LOG it loudly so the source can be
|
|
debugged, and leave UNDETERMINED — a muxer never fabricates a source fact.
|
|
`video_picture_seen == false` (an empty title finalized with no frames, or a
|
|
cap-triggered build that never saw the video frame) is NOT a defect — the
|
|
missing coding is expected there, so log it quietly.
|
|
|
|
## track-table
|
|
|
|
Read-side map from Matroska TrackNumber to the index of the corresponding
|
|
entry in `DiscTitle::streams`.
|
|
|
|
RFC 9559 §5.1.4.1.1 constrains TrackNumber only to be non-zero ("range: not
|
|
0"); NOTHING in the specification requires the numbers to be `1..=N`, to be
|
|
contiguous, or to appear in ascending TrackEntry order. `parse_track` also
|
|
DROPS every TrackEntry whose TrackType this crate cannot carry (anything but
|
|
1/2/17 — e.g. a TrackType 18 buttons track), so the TrackNumber space and the
|
|
stream vector diverge for perfectly legal inputs.
|
|
|
|
The reader used to derive the stream index as `TrackNumber - 1`, which routes
|
|
blocks to the WRONG stream (parsed by the wrong codec parser) or drops them
|
|
entirely. This table records the real TrackNumber for each retained stream,
|
|
in stream order, and is the only thing allowed to translate between the two.
|
|
|
|
## split-lacing
|
|
|
|
Split the body of a LACED (Simple)Block — the bytes after the flags octet,
|
|
beginning with the Lacing Head — into its individual frame payloads, per
|
|
RFC 9559 §10.3.
|
|
|
|
`lacing` is the 2-bit LACING field value (`LACING_XIPH`, `LACING_EBML` or
|
|
`LACING_FIXED`). Returns `None` when the lacing header is malformed — the
|
|
frame boundaries are then unknown, and the caller MUST reject the block
|
|
rather than hand a concatenation of frames plus lacing header downstream as
|
|
though it were one frame (which is exactly the silent corruption this
|
|
function exists to end).
|
|
|
|
The Lacing Head is "number of frames in the lace minus 1" on one octet, so
|
|
the frame count is bounded by 256 and no allocation here is attacker-scaled.
|
|
|
|
## parse-block
|
|
|
|
Parse a (Simple)Block payload into zero or more PesFrames.
|
|
|
|
Zero frames means the block was SKIPPED — too short, track 0, or a
|
|
TrackNumber this file does not (retainedly) declare. More than one frame
|
|
means the Block was LACED (RFC 9559 §10.3): one Block legitimately carries
|
|
several frames, and handing the raw payload downstream as a single frame
|
|
feeds the codec parser a concatenation of frames plus lacing header. An
|
|
`Err` means the lacing header is malformed, so the frame boundaries are
|
|
unknowable — the block is rejected rather than mangled.
|
|
|
|
`cluster_ts_ticks` is the open cluster's timestamp in TimestampScale ticks
|
|
and `ts_scale_ns` is that scale (ns per tick); the block PTS is computed as
|
|
`(cluster_ts_ticks + rel_ts) * ts_scale_ns` so foreign MKVs whose scale
|
|
isn't 1 ms are honoured (freemkv's own output uses 1_000_000 and round-trips
|
|
unchanged). `tracks` resolves the TrackNumber to a stream index; `duration_ns`
|
|
is propagated for BlockGroup blocks (None for SimpleBlock).
|
|
|
|
## block-additions-dropped-test
|
|
|
|
A BlockGroup's `BlockAdditions` subtree (BlockAddID=2 — the MVC
|
|
dependent/right-eye access unit this crate's 3D writer emits, see
|
|
`mkv.rs::build_block_group`) cannot be carried by `PesFrame`, so read-back
|
|
drops it. That is a LOSSY outcome, and this crate's rule is that a lossy
|
|
outcome is never silent.
|
|
|
|
Regression: the arm did not exist, so the subtree fell into the `_ =>`
|
|
skip arm — an `mkv://` → `mkv://` re-mux of a 3D rip lost one whole eye
|
|
with no error, no warning and `lost_bytes == 0`, i.e. the mux reported a
|
|
clean, complete, loss-free copy of a file it had halved. The base view
|
|
must still read back intact, and the dropped payload must now be counted
|
|
so it reaches `MuxOutcome.lost_bytes` / `.errors`.
|
|
|
|
## reference-block-keyframe-test
|
|
|
|
A BlockGroup carrying a ReferenceBlock is NOT a keyframe — that element's
|
|
presence is the only non-keyframe signal a BlockGroup has (the SimpleBlock
|
|
0x80 flag bit is reserved and always 0 inside one).
|
|
|
|
Regression: the reader used to `skip_bytes` past REFERENCE_BLOCK and read
|
|
the reserved bit instead, so EVERY BlockGroup frame came back as a
|
|
non-keyframe. Since the MPEG-2 parser stamps a per-frame duration, all
|
|
MPEG-2 video takes the BlockGroup path — so no video frame ever looked
|
|
like a keyframe on re-mux. That silently dropped all video on
|
|
`mkv://`→`m2ts://` and failed `mkv://`→`mkv://` with E6008.
|
|
|
|
## ebml-lacing-test
|
|
|
|
EBML lacing (RFC 9559 §10.3.3): the Lacing Head, the first frame's size as
|
|
an unsigned VINT, then each later size as a SIGNED VINT difference from
|
|
the previous one. Three frames of 3/4/5 octets must come out as THREE
|
|
frames with byte-exact payloads.
|
|
|
|
Regression (silent corruption): the reader took the Block payload verbatim
|
|
and never looked at the LACING bits, so this Block became ONE 15-byte frame
|
|
whose first three bytes are the lacing header — garbage handed to the codec
|
|
parser with no error, and one timestamp for three frames.
|
|
|
|
## track-number-gap-and-sub-44100-sampling-test
|
|
|
|
RFC 9559 §5.1.4.1.1 constrains TrackNumber only to be non-zero — nothing
|
|
requires `1..=N` in TrackEntry order. A file with a TrackType this reader
|
|
drops (18 = buttons) between two carried tracks makes the TrackNumber
|
|
space and the stream vector diverge.
|
|
|
|
Regression (silent corruption): the reader computed the stream index as
|
|
`TrackNumber - 1`, so the audio blocks of TrackNumber 3 resolved to index
|
|
2 in a 2-stream title and were DISCARDED — a remux with no audio, reported
|
|
as success.
|
|
A legal SamplingFrequency below the lowest rate this enum maps must come
|
|
back as Unknown, not silently as 48 kHz.
|
|
|
|
The ladder's final `else` was `SampleRate::S48`, so a 32000 Hz AC-3 or DTS
|
|
track — legal, and common in broadcast-sourced content — was recorded as
|
|
48 kHz and the wrong rate propagated into the reconstructed AudioStream.
|
|
The crate's canonical mapping, `SampleRate::from_hz`, returns Unknown for
|
|
32000; this ladder disagreed with it.
|
|
|
|
## finish-produces-readable-mkv-test
|
|
|
|
`finish()` is what turns a stream of frames into a FILE. It activates a
|
|
still-pending muxer (writing EBML header, Segment, Info, Tracks), then
|
|
finalizes it (Cues, SeekHead, the backpatched Segment size). A `finish`
|
|
that returned `Ok(())` without doing any of that leaves the caller with a
|
|
zero-byte or truncated `.mkv` and an exit code of 0 — a rip that reports
|
|
success and produced nothing.
|
|
|
|
Proven by reading the output back through this crate's own MKV reader:
|
|
the frames must come out in order, with their real payloads, timestamps
|
|
and keyframe flags.
|
|
|
|
## finish-refuses-zero-frame-title-test
|
|
|
|
A title that produced NO frames must NOT finish successfully. `finish()`
|
|
activates the still-pending muxer (so the header/Tracks are written) and
|
|
then hands off to `MkvMuxer::finish`, whose zero-frame guard raises
|
|
`Error::MkvInvalid` (E6008) rather than emitting a structurally valid but
|
|
clusterless MKV.
|
|
|
|
A `finish` that returned `Ok(())` would report a completed rip for a
|
|
title that muxed nothing — precisely the "empty title, exit code 0"
|
|
outcome the guard exists to prevent — and `error::is_skippable_title_stub`
|
|
would never get the code it classifies on.
|
|
|
|
## headers-ready-and-untrusted-size-caps-test
|
|
|
|
`headers_ready()` gates the CLI's wait-for-codec-private loop. For
|
|
Matroska it is unconditionally true because RFC 9559 §5.1 places the
|
|
Tracks element (carrying every CodecPrivate) in the Segment header,
|
|
ahead of the first Cluster — `MkvStream::open` has therefore already
|
|
parsed them by the time it returns. Returning `false` would hang the
|
|
mux forever on a source whose headers are, by construction, present.
|
|
|
|
Pinned as an implication rather than a bare constant: readiness is
|
|
asserted TOGETHER with the codec private actually being retrievable, on
|
|
a freshly opened stream that has read no frame yet.
|
|
The untrusted-size caps ARE the OOM guard: every EBML element size is
|
|
checked against one before it is used to allocate. Only their existence
|
|
was pinned, never their magnitude — so a cap that collapsed to a few
|
|
kilobytes would still look guarded while rejecting ordinary discs, and one
|
|
that ballooned would allocate whatever a hostile container asks for. Both
|
|
ends need a number.
|
|
|
|
## track-entry-metadata-round-trip-test
|
|
|
|
Write a real three-track title through this crate's own muxer, then read
|
|
it back through this crate's own reader and check the TrackEntry metadata
|
|
survived. Language, track name, the forced flag, pixel height and channel
|
|
count each had a dedicated arm in `parse_track` and NONE of them was
|
|
asserted — every one could have been deleted and the suite stayed green
|
|
while a re-mux quietly lost the audio language, the subtitle forced flag,
|
|
the track labels, the resolution and the channel layout.
|
|
|
|
## default-track-dedup-test
|
|
|
|
FlagDefault says "play this track unless the viewer picks another". Only
|
|
ONE video and ONE audio track may carry it; `MkvTrack::video`/`audio` set
|
|
it from `!secondary` alone, so a disc with two ordinary video angles or
|
|
two ordinary audio tracks arrives here with the flag on all of them and
|
|
this de-duplication is the only thing that fixes it. It lives here and
|
|
nowhere else — the muxer just writes what it is handed — and it had no
|
|
test at all.
|
|
|
|
## progressive-on-declared-interlaced-test
|
|
|
|
A DVD whose IFO declares 480i/576i but whose pictures are CODED
|
|
progressive — film and animation on NTSC discs routinely are — must ship
|
|
`FlagInterlaced=progressive`, because that is what the bitstream says.
|
|
|
|
`MkvTrack::video` sets `interlaced` from the DECLARED resolution, which
|
|
on a DVD is 480i essentially always. Shipping that unchecked marks
|
|
progressive content interlaced, and every player that honours the flag
|
|
then runs a deinterlacer over progressive frames — softening every frame
|
|
of an otherwise bit-exact remux. Measured against a real disc: `idet`
|
|
reports 100% progressive on titles this shipped as `FlagInterlaced=1`.
|
|
|
|
The sibling test above pins the interlaced direction, so together they
|
|
constrain both: a measurement of TFF keeps the track interlaced, and a
|
|
measurement of progressive corrects it.
|
|
|
|
## deferred-activation-end-to-end-test
|
|
|
|
THE deferred-activation contract, end to end: the field order MEASURED
|
|
from the first coded picture has to reach the FILE.
|
|
|
|
`apply_coding_to_track` was tested in isolation, which proves nothing
|
|
about whether the caller ever reaches it with a real measurement — and
|
|
the route there runs through the pending-buffer cap, the
|
|
"is this the video frame" test and the activation trigger, none of which
|
|
were observed from the outside. Any of them mis-set and the file ships
|
|
FieldOrder omitted: an interlaced DVD that players then deinterlace with
|
|
the fields in the wrong order (visible combing on motion).
|
|
|
|
## video-master-walk-bounds-test
|
|
|
|
The `Video` master's child walk must consume EXACTLY the bytes its
|
|
children declare. This crate's own writer happens to place every other
|
|
TrackEntry field ahead of `Video`, so an over-run there costs nothing —
|
|
but a foreign MKV (mkvmerge orders children differently) puts fields
|
|
after it, and an over-running walk then swallows them: the reader reports
|
|
language `und` for a track that declared one, on a file that is perfectly
|
|
well formed.
|
|
|
|
## skip-bytes
|
|
|
|
A skip that runs out of input before `n` bytes is a TRUNCATED element, and is
|
|
reported the same way `ebml::read_binary_val` reports a truncated body: as
|
|
`MkvSourceInvalid`. Discarding `io::copy`'s byte count instead made a skip
|
|
that hit EOF look like a success, so one corrupt size field mid-Clusters
|
|
drained the rest of the file, the next element header raised
|
|
`UnexpectedEof`, and `Stream::read` mapped that to `Ok(None)` — half the
|
|
title missing, `errors = 0`, `completed = true`.
|