MLP/TrueHD carries predictor + restart state across access units, so after a source transport-stream discontinuity (continuity-counter gap) the parser was splicing post-gap audio mid-stream, decoding against stale state and producing a decoder-choking seam (a burst of "restart header sync incorrect" / "Invalid blocksize" errors that could flag the whole track corrupt).
Honor the source's own gap signal: drop forward to the next CRC-valid major-sync AU (a clean re-init point) before resuming, guarded so it never arms before a validated baseline exists (no stream-head whole-track drop). Mirrors the video keyframe resync and the ResyncGate design. Corrects the "audio has no inter-frame refs" comments in truehd/dts/resync. Adds a regression test.
Adds a 'changelog' job that fails when Cargo.toml's version has no matching
'## [X.Y.Z]' heading in CHANGELOG.md, hard on main and PRs into main, warning
on dev/qa — so the unified release can't ship a crate with a blank changelog.
The B1 resync gate (mux/resync.rs) arms on a concealed-gap / clip-seam
discontinuity and drops video frames until it sees a keyframe. The H.264
parser flagged a keyframe only on an IDR slice (NAL type 5), so on a BD
open-GOP tail whose only random-access point is a non-IDR I-frame (the
recovery point) the gate never disarmed and dropped every remaining frame
to EOF — truncating the video by up to a GOP-plus tail and failing the
cli-acceptance timeline_sound strict span check for BD.
Promote an intra-coded access unit (measured coding_type == I) to a
keyframe, alongside the existing IDR-by-NAL-type flag. This is the H.264
equivalent of HEVC's open-GOP IRAP (CRA) handling, which the HEVC parser
already flags by NAL type — H.264 has no such NAL tag, so the measured
intra coding type is the signal. Keying off coding type rather than a
recovery_point SEI covers every signalling: an SEI-tagged recovery point
(which is intra by construction) and a disc that marks the recovery point
by GOP structure alone. Consistent with videomap.rs's model that a
keyframe IS the intra decode-restart point; the open-GOP anchor now also
gets the self-contained SPS/PPS re-assert, mirroring HEVC at CRA. IDR
stays flagged by NAL type, so a slice whose header fails to parse
(coding_type == None) is unaffected. HEVC, DVD/MPEG-2 unchanged.
Tests: open_gop_intra_access_unit_is_a_keyframe (red before this change),
inter_coded_access_unit_is_not_a_keyframe (precision guard).
find_drives and resolve_device each carried a three-arm cfg(target_os) block in
their bodies. Alias the current platform's module once (`use linux as platform`
under a single cfg) and let both dispatch through `platform::…`, so the function
bodies are cfg-free and a new entry point cannot forget an arm.
Two audit fixes plus two consistency cleanups, all in the changed 1.6.5 surface:
- hddvd: the VTI clip-order read used `.ok()`, flattening an unreadable
authored order (a scratched sector under the .vti — the name came from the
directory, so it is never "absent") into "no order" with no diagnostic. The
sibling clip-extent arms log every read failure with its code; this one now
does too, then falls back to the per-clip heuristic exactly as an unauthored
disc would. Behaviour is otherwise unchanged; the loud line is the point.
- extract: the progress sink hardcoded bytes_unreadable_total: 0 and counted
every zero-filled hole as good, so a progress-only consumer saw a holed
extraction climb to a clean 100%. Thread the running unreadable total through
and report the real good/unreadable split. The authoritative ExtractResult was
already truthful; only the live channel lied.
- labels: correct a stale comment that described testlog's old lock-based
capture; it now installs one global subscriber and routes to a thread-local
sink.
- ps: import SYSTEM_HEADER from consts instead of re-declaring 0xBB, matching
its sibling stream-id constants and the file's single-source rule.
testlog: capture through a single global subscriber that routes each
event to a thread-local sink and returns Interest::sometimes, so
tracing's global per-callsite interest cache can never short-circuit a
live capture when another thread rebuilds it. The old scoped
with_default lost events at random in the full suite (an empty capture
failed the log-accounting assertions ~1 run in 15).
labels: capture the escaping test through testlog instead of installing
its own process-wide subscriber, which hard-cached every foreign
callsite "off" and poisoned the captures above.
dirimage: name scratch directories from a process-monotonic counter
rather than SystemTime, which resolves to only a microsecond — two of
the parallel tests sharing a tag drew the same value, collided on one
directory, and the first to finish removed it out from under the
other's reads.
Commit b02c402 ("key the DVD crack on the disc, not on the container") fixed a
real bug — a decrypted HD-DVD hit E7023 (CssKeyMissing) because the per-title
CSS crack keyed on the MPEG-PS container, which DVD and HD-DVD share — but did
it by adding a required `disc_format: DiscFormat` parameter to the PUBLIC
`DiscStream::new` and `build_iso_pipeline`, a `disc_format` field to
`MuxInput::Iso`/`Live`, and `DiscFormat::may_have_css`, threading the axis down
through mux/driver and mux/resolve. That changed the public API and broke every
downstream caller's compilation (freemkv-engine's integration test now needed 8
args, autorip's MuxInput arms a new field). 1.6.4 shipped and worked with these
exact signatures; a bug fix must not reshape them, and needing a whole
disc-format plumb for HD-DVD was a code smell.
Revert all of that plumbing (public signatures restored to their pre-b02c402
form; no `disc_format` parameter or field, no `may_have_css`, anywhere), and fix
the ACTUAL bug where it lives: the scramble-detection heuristic.
Root cause: `is_scrambled_pack` counted a sector as CSS scramble evidence on
pack-start (00 00 01 BA) + bits 4-5 of byte 0x14. Offset 0x14 is only the PES
scrambling-control field when the sector is a genuine elementary-stream pack. An
HD-DVD `.evo` RDI navigation pack is private_stream_2 (stream_id 0xBF), an
MPEG-PS pack exactly like a DVD VOB, whose byte 0x14 is raw nav payload that
routinely has bits 4-5 set. On a decrypted HD-DVD (None keys, MPEG-PS, so it
reaches the crack) those nav packs flipped the scan's `saw_scrambled` flag; the
crack then found no key — there is no CSS on an HD-DVD — and the scan returned
ScrambledUncracked, hard-failing a good disc with E7023.
Fix: exclude the MPEG-PS structural stream_ids CSS never scrambles — system
header (0xBB), padding (0xBE), private_stream_2 (0xBF) — by the stream_id at
offset 0x11. This is the refinement the DVD design notes already called for
("matches CrackTitleKey"). It needs no format plumbing because byte 0x11 lives
in the CSS-clear header (0x00-0x7F, untouched by scrambling), so it is the true
stream_id even on ciphertext. A decrypted HD-DVD now scans to Unencrypted and
muxes cleanly.
The DVD CSS crack is preserved and proven: a genuinely CSS-scrambled DVD sector
is always video (0xE0-0xEF) or private_stream_1 (0xBD), never an excluded id, so
its scrambled packs still set saw_scrambled and still hard-fail an uncrackable
disc — the "ciphertext muxed as plaintext at rc=0" catastrophe cannot slip
through. Red-before-green both directions: dropping the 0x11 exclusion turns the
decrypted-HD-DVD case back into E7023; inverting it (only nav ids count) turns a
real uncrackable DVD into Unencrypted and strands a crackable one. Both mutations
are caught by tests.
Gate (cargo +1.97): fmt, clippy --all-targets -D warnings, 3539 tests green;
freemkv-engine and autorip both compile against this tree again; precommit.sh
libfreemkv clean.
The same hole, two disc families, two answers — and the asymmetry is now a
decision written into both files instead of an oversight in one.
`file_extents` can return `Ok` and still yield no usable extent: an empty
allocation-descriptor list, or one every entry of which the `sectors > 0 &&
lba > 0` filter discards. An ordinary zero-byte file reaches it; no crafted
disc is needed.
On HD-DVD that was the flagship failure shape. The clip entered neither
`clip_extents` nor `unusable`, and nothing was logged, so the composer's
`any(|n| unusable.contains(..))` guard missed it while the
`filter(|n| clip_extents.contains_key(..))` beside it quietly deleted the
part: a `FEATURE_2.EVO` of size 0 next to a healthy `FEATURE_1.EVO` composed
a FEATURE title out of part one alone, still advertising the whole runtime,
at rc=0, in silence. Half a movie presented as a whole one. Round 1 accounted
for every `Err` from the resolver and left this route open. It now marks the
clip unusable and logs it under its own new code, E6019
(`E_UDF_NO_USABLE_EXTENT`) — deliberately not the neighbouring E6017, which
would file a zero-length file as an authoring hole and send whoever triages
it at the wrong population.
On Blu-ray the identical hole stays open, as previously decided, and the
reasons are now recorded on both sides. BD has no `unusable` set, so closing
it there means inventing a post-loop "every clip_id must appear in `spans`"
invariant that DROPS the title, and it is not settled that an empty-but-Ok
resolve is always a defect; dropping healthy titles is worse than the gap.
The consequence is milder too: on BD the clip is one PlayItem of an otherwise
whole title, on HD-DVD the feature is COMPOSED from parts. Same hole,
different price.
Also in this change:
* bluray: a non-absence SSIF failure that the `.m2ts` fallback papers over is
logged. `unresolved` had exactly one reader, `if let (None, Some(code))`, so
when `/BDMV/STREAM/SSIF/<clip>.ssif` failed with DiscRead /
UdfAdChainTooLong / UdfEmbeddedData and the base view then resolved, the
code was recorded and thrown away: the title shipped base-view 2D off a 3D
disc at rc=0 with no log at all. The site's own doctrine is "ABSENCE is the
only benign failure". Logged, not refused — the base view is a real rip.
* drive: `wait_ready` polled TEST UNIT READY through a bare `execute` and its
60 x 500 ms loop never read `self.halt`, so a Stop during spin-up did
nothing for ~30 s while every other drive path returns Halted at the next
command boundary. `spin_cycle` issued both START STOP UNIT commands outside
`checked_exec` and slept `SPIN_DOWN_IDLE_SECS` + `SPIN_UP_SETTLE_SECS`
blind — ~15 s deaf to Stop, from the recovery path, exactly when the
operator is most likely to press it. Both now use `checked_exec` and
`sleep_until_halted`, which already lived in this file with four tests and
was `#[cfg(test)]`, called from nowhere. It is production code again.
* drive: a READ(10) that returns GOOD status with a residual underrun was
correctly refused and logged NOWHERE, while the sibling `Err` arm warns with
lba/count/status. A residual-underrunning drive was indistinguishable from a
scratched disc — two populations with opposite remedies. It now warns with
transferred vs expected, which is the whole signal.
* error: `all_error_code_constants_are_unique` was a hand-maintained `vec![]`
naming 109 of the 127 declared codes while its doc claimed to pin them all,
and an earlier audit trusted that claim while assigning new ones. The list
is now derived from the declarations by parsing `include_str!("error.rs")`,
so a new constant is covered the moment it is written. A parser self-test
cross-checks the count and three known name/value pairs, so it cannot pass
vacuously.
* testlog: a test-only `tracing` capture (~120 lines, no new dependency) so
the logging contract is enforced rather than commented. Three sites carry
long comments insisting they log the error's OWN code; putting a literal
back broke nothing. They are pinned now, along with the two new log lines.
Captures are serialised process-wide: `tracing`'s interest cache is global
while `with_default` is thread-local, and the rebuild on the exiting
capture can land after the entering one's, leaving the cache at "never"
while a capture is live. That produced a real empty-event flake.
* disc: `scan_with`'s halt wiring for the BD and DVD enumerators had no test —
every BD/DVD cancellation test calls the scanners directly, so passing
`None` on either branch left the suite green while Stop did nothing.
* mux::network: `accept_from_rejects_stream_without_fmkv_header` half-closes
instead of `Shutdown::Both`, which raced an RST against the server's read
and returned ConnectionReset instead of InvalidInput under load. The port
was already ephemeral; that was never the cause.
Gate: fmt, clippy --all-targets -D warnings, and 3439 tests green on 1.97;
precommit.sh libfreemkv clean.
`resolve_dvd_title_key` decided whether to run the DVD CSS crack by asking
what CONTAINER it was looking at — `ContentFormat::MpegPs`. MPEG program
stream is what DVD and HD-DVD have in common, so every HD-DVD title was
run through a crack scan for a copy-protection scheme HD-DVD does not
use and cannot carry: AACS is its family, and CSS appears nowhere on the
disc.
Both outcomes of that scan were wrong. The cheap one wasted up to 50,000
sector reads per title. The expensive one returned `CssKeyMissing` —
E7023 — refusing a perfectly good HD-DVD with a CSS error, which is what
a real CI run produced on the HD-DVD fixture. Reading E7023 there sends
whoever triages it looking for a missing DVD key on a disc that never had
one, which is how a routing bug spends a day disguised as a key problem.
The right axis was already in the codebase and already used: `mux/resolve`
asks `disc.format == DiscFormat::Dvd`. This threads the disc format down
to the decision and adds `DiscFormat::may_have_css` to name the question.
The asymmetry decides the default, so it is worth stating. Running CSS on
an HD-DVD costs a wasted scan or a false refusal — visible, recoverable,
annoying. NOT running it on a real DVD muxes scrambled sectors as
plaintext and exits 0, which is the failure-that-looks-like-success class
this project has shipped once already. So `may_have_css` is false ONLY
for the families proven CSS-free, and `DiscFormat::Unknown` — a bare
reader with no scan behind it — still cracks. An `== Dvd` allow-list
would have read as tighter while silently stranding every caller that
cannot name its disc.
Both directions are pinned: an HD-DVD title must never enter the crack,
and an unknown-format title must still enter it. Removing the disc-format
clause fails the first and leaves the second and every DVD test green.
A cancelled scan was indistinguishable from a disc that simply holds
fewer titles, in all three tree enumerators. Once the operator presses
Stop, `Drive::checked_exec` fails EVERY subsequent SCSI command with
`Error::Halted` and `Drive::read` deliberately preserves the variant —
so a cancel is not one failed read, it is every remaining read failing.
* `scan_bluray_titles` returned a bare `Vec<DiscTitle>` and
`parse_playlist` an `Option`, so neither had a channel to report a
halt. During a cancel each remaining `.mpls` read merely failed and
was skipped by an `if let Ok(..)`, and the scan returned a TRUNCATED
title list at rc=0. Both now return `Result`; `Ok(None)` from
`parse_playlist` keeps its two benign meanings (unparseable MPLS,
playlist under 30 s) and `Err` means the scan is over.
* `scan_dvd_titles` was worse: `Err(_) => return Vec::new()` turned a
cancel into ZERO titles at rc=0 — a DVD reported as carrying no
video at all. `ifo::parse_vmg` compounded it, treating a halted
title set as one of the placeholder TT_SRPT entries it is designed
to tolerate and continuing to the next; both now propagate.
* HD-DVD was already correct and is the model this follows, including
its reasoning: a cancelled enumeration that returned `Ok` would be
cached, displayed and ripped from as if it were the disc.
The cancel is propagated at ALL FOUR Blu-ray read sites, not just the
obvious one — the playlist read, the `.clpi` read, and both
`file_extents` calls in the clip resolver — and the halt is polled
between playlists and again after the loop, so the final iteration
cannot slip a truncated list through. The resolver's `note` closure
previously EXEMPTED `Halted` from its disc-defect classification, which
was right (a cancel is not an authoring hole) but left it swallowed:
with no exemption and no propagation a cancel landing on `file_extents`
returned a title claiming its full runtime with that clip's bytes
silently missing. Measured with the fix reverted: `Ok(Some((768000,
[])))`.
Second defect, same function: the `.clpi` read and `clpi::parse` sat
inside `if let Ok(..) && let Ok(..)`, with extent resolution nested
inside. A scratched CLIPINF sector, a missing `.clpi` or a malformed one
skipped the whole block — no extents, nothing added to `total_size` —
but `duration_ticks` had ALREADY been summed from the play items, so the
title still advertised the movie's full runtime. The `if let` discarded
the error, so nothing was logged either: a failure that looks like
success, which is this crate's flagship defect class. It is now a
`match` that drops the title and warns with the error's OWN code, the
same classification the extent resolver below it already uses. Logging a
fixed code would account a scratched disc as an authoring hole.
Deliberately NOT added: a post-loop "every unique clip_id must appear in
`spans`" invariant. `file_extents` can also return `Ok(vec![])`, or a
vector whose every entry the `sectors > 0 && lba > 0` filter discards,
and it is not settled that an empty-but-Ok resolve is always a defect
rather than a legitimate healthy-disc state (zero-length placeholder
ADs). Dropping healthy titles is a worse failure than that residual gap,
so the route stays open, with a comment at the site recording the
decision so the next audit finds it instead of re-deriving it.
Tests: `parse_playlist_missing_clpi_yields_no_extent_no_size` asserted a
title WAS returned with `size_bytes == 0` and empty extents, and its
docstring quoted the buggy control flow as the specification — it
blessed the defect. Inverted and renamed. `parse_playlist_keeps_exactly_
30_seconds` encodes correct behaviour but used a fixture wiring no
STREAM/CLIPINF, so the `.clpi` fix would have dropped its title for a
reason unrelated to the boundary it pins; repointed at the fully wired
fixture rather than weakening the assertion. Four new halt-propagation
tests (BD playlist read, BD `.clpi` read, BD extent resolve, DVD title
set + VMG), each proven red before green.
Ten lenses over v1.6.4..HEAD, every claim read against the code before
it was believed. Seven confirmed; six are here, one is recorded for the
next round. All of these are the same family — a failure wearing the
shape of success — which is the family that once shipped 9 MB of
ciphertext inside a main-movie m2ts at rc=0.
A clip whose extents cannot be resolved is now accounted for, in both
disc readers. Only `UdfUnrecordedExtent` used to count: every other way
`file_extents` can fail — a scratched sector under the clip's ICB
(DiscRead), an allocation-descriptor chain that never terminated, a file
whose data is embedded rather than extent-mapped — fell through to the
ordinary "file absent" path. On Blu-ray that yielded a title advertising
its full runtime with a clip's bytes silently missing, because the size
and the play-item timing had already counted it. On HD-DVD it was worse:
the clip was never added to `unusable`, so a split feature still composed
from FEATURE_1 alone and offered half a movie as the whole thing. Neither
emitted a single log line. Absence is still benign — a 2D disc has no
.ssif and the extension fallback exists for exactly that.
`Halted` is excluded deliberately, and that exclusion is the whole reason
the first version of this fix was wrong. Cancellation makes EVERY drive
command return `Halted`; classifying it as a disc defect would have
dropped each remaining playlist in turn and handed back a truncated title
list at success — the same defect, wearing a cancel. `parse_playlist`
returns Option and has no channel to propagate a halt, so the existing
behaviour is preserved rather than made worse. Propagating it properly is
next round's work.
Both log sites now emit the error's OWN code instead of a hardcoded 6017.
Accounting a scratched disc (E6000) as an authoring hole would send
anyone triaging it looking for the wrong thing entirely.
AD type 3 is embedded data, not a descriptor list (ECMA-167 4/14.6.8).
`read_icb_extents` lumped it in with the reserved values and decoded the
file's own CONTENT as (length, LBA) pairs, manufacturing extents out of
arbitrary bytes and pointing the reader at unrelated sectors. This same
release already taught `read_directory` to honour type 3; this is the
file half of that decision. It is an error rather than an empty list,
because an empty list reaches the caller as a clip that contributed
nothing while its declared duration still counts it — the silent loss
pointed the other way. A legally zero-length embedded file still returns
an empty list. New code E6018: reusing DiscRead would have mislabelled a
deterministic structural property as transient I/O and fed the retry and
NonTrimmed machinery a byte that will never change.
`file_extents_addressing`, `extents_abs_at` and `AbsExtent` drop to
`pub(crate)`. The first hands back unrecorded extents UNFLAGGED, in a
shape identical to the safe call's return; its doc says callers must use
`file_extents` instead, but a doc comment is not a guard. No dependent
crate references any of the three.
Three tests close gaps the audit found, each proven red before green:
a held AC-3 access unit must not resume as a normal frame after its track
poisons; the PS resume cursor must survive a drain that rebases it (three
separate mutants caught); and AD type 3 must be refused rather than
decoded. The first attempt at the HD-DVD test passed with the fix
reverted, which made it worthless — it needed a VTI fixture before the
composition path ran at all.
Also: four error codes were missing from the uniqueness test that claims
to cover every published code, so a new variant reusing 6014, 6016 or
6017 would have passed it.
`dtolnay/rust-toolchain` is not a dependency in the sense Dependabot
means: it is versioned by the Rust release it installs, and the tag we
pin is the toolchain CI is pinned to on purpose — precommit.sh runs the
same one locally, so a lint that passes on a developer's newer default
cannot pass CI by accident.
Read as semver, those tags produced a 1.97.0 -> 1.100.0 proposal, a Rust
version that does not exist. Every such PR 404s on toolchain download
across all eight repos and regenerates weekly: eight permanently-red PRs
that promote.yml then has to special-case when it decides whether dev is
green.
freemkv already carries this ignore; this is the same block in the other
seven, so the fleet stays uniform. Bumping the toolchain stays a
deliberate, all-eight-repos change made by hand together with
precommit.sh.
Squashed from 12 commits. Every fix was proven red-before-green and killed by a
mutation; the reasoning for each is in the private audit record.
UDF and extents
Honour ICB types rather than assuming a Short AD, so an AD-type-3 directory
is no longer decoded from FID bytes into a silently empty listing. Carry the
ECMA-167 recorded flag through to the resolvers: an allocated-but-never-
written extent used to reach the read plan as ordinary content and splice
undefined sectors into the rip. file_extents now refuses such a file, and
only when the hole actually occupies byte space — a zero-length one displaces
nothing, and refusing on it dropped whole titles off discs that ripped
correctly. Type-2 sparse extents are kept alongside type-1; they were falling
into a catch-all that exited the descriptor loop and returned a truncated
list as complete. merge_ranges no longer claims a sector neither input
covered. A short skip or an over-long AD chain errors instead of truncating.
HD-DVD and Blu-ray scanning
Bound the XPL nesting depth, title count, clips and chapters per title, and
memoize the clip-name fallback probe — four separate amplification axes, each
of which alone left the worst case unbounded. The clip and title caps are 512,
~10x any retail disc, and a test pins the product of cap and probe budget.
The scan is cancellable: it returned Ok with titles carrying no streams when
halted, presenting a cancelled scan as a successful one. A clip dropped for an
unrecorded extent now says so.
Codecs and muxing
Resume a held E-AC-3 access unit rather than rescanning from its first frame,
and drop it on a discontinuity — a stale hold indexed past the end of the new
buffer. Map every ISO 639-1 code instead of collapsing fifteen languages to
und. Correct the DVD palette order. Detect a skip past EOF.
Drive and I/O
Classify dead-bus faults so the wedged-drive path can see them; a catch-all
arm had been flattening the variants before the classifier ran. A prefetch
producer that dies now reports SourceTerminated instead of Ok(0), which the
reader legitimately read as a short read and zero-filled — a whole title
could be fabricated and the pass reported complete.
Also: charge Ok(0) reads to the CSS crack budget, drop the unreachable soft
re-crack, and send disc-derived strings to logs through the debug formatter so
a crafted label cannot paint an operator's terminal.
`resolve_vts_key` called the `Option`-returning `css::crack_key`, which
collapses "no scrambled sector was seen" with "scrambled sectors were
seen and no key came out", and then fell back to the disc-wide key. A
multi-VTS CSS DVD whose second title set resists the Stevenson scan was
descrambled under the FIRST set's key: corrupt PES behind an intact
header, written out with `complete = true` at exit 0.
`CrackOutcome` exists precisely to keep those two apart, and its doc says
callers must surface the second as a hard error. Every sibling path in
the crate already does — `Disc::decrypt_keys_for_title` and the mux path
both map `ScrambledUncracked` to `CssKeyMissing`. This was the one path
that did not. The ordering comment 15 lines above describes this exact
outcome as the bug it exists to prevent; ordering makes the crack far
more likely to succeed, but it cannot make a failed crack safe.
Also, three things nothing could catch:
- The demux output filename took the stream's language raw while the
base beside it was sanitised. A language code is three raw STN bytes
through `from_utf8_lossy`, and `00 00 00` is the ordinary "undefined"
encoding on real discs — a NUL in a path fails `File::create` with
InvalidInput, taking the whole export down before one track file
opened. `sanitize` now maps control characters too; it did not.
- `css::crack_key_scan`'s short-read handling was dead code under test:
every source in the module returned the full request, so reverting
`advance` to the requested count, or dropping the `.max(1)`, left the
suite green. The `.max(1)` is load-bearing — without it a source
returning `Ok(0)` never moves the cursor and never increments the
budget, so the scan spins forever. That mutation now HANGS the test
rather than failing it, which is the honest demonstration.
- `MAX_SUBDIRS`'s const-assert carried `#[cfg(not(test))]` inside a
`#[cfg(test)] mod tests`, so it compiled in no configuration and could
never fire — the dead gate the test above it was written to replace.
Moved to module scope, and verified it now rejects a wrong constant at
compile time.
`forced_sub` was the last unbounded attacker-controlled list in
paramount.rs. `MAX_COM_INDICES` capped the two `*_com1_idx` attributes;
this one had nothing capping it at all, and unlike them it has no value
to filter — a cell is a classification of the position it sits at, so
its bound has to be positional. Extracted as `forced_subs` for the same
reason `com_indices` was extracted: through `labels_from_feature` the
bound is unobservable, because the subtitle loop cannot reach those
cells either, so the assertion could not fail.
The `MAX_COM_INDICES` doc claimed the entry-allocation argument for the
whole constant. It is the VALUE filter that caps the set (values below
the bound, so at most that many distinct entries, however long the
attribute); the `take` caps the WORK. Both are real and they are not
the same bound; the doc now says which is which.
`jar_inventory_dedup_is_not_quadratic` called itself proof by deadline.
It is a hang guard — a return to the linear scan runs for minutes and
would wedge CI rather than fail it — and no assertion in it can tell a
BTreeSet from any other sub-quadratic dedup. Renamed and documented for
what it does. Its margin was measured before keeping it: 0.14s debug /
0.07s release against 10s, ~70x, unlike the 6x that made paramount.rs's
wall-clock test flake.
Both are mine, and both pass with the bound they "cover" deleted.
`a_hostile_commentary_index_list_is_bounded_not_merely_fast` asserted
only label purposes. The out-of-range filler it feeds is unobservable at
the label level and a HashSet collapses the repeats, so removing
MAX_COM_INDICES entirely leaves it green. What it DOES catch is a bound
set too low — verified at 2, where the real indices stop resolving. Named
and documented for that, and it no longer implies it guards enforcement.
`an_index_that_cannot_address_any_cell_is_not_retained` asserted through
the labels, where retention is by definition unobservable: the loop never
queries a cell that high. It now reads the set through `com_indices`,
where the claim is checkable.
Enforcement was and remains proven by
`distinct_unaddressable_indices_are_refused_not_stored`, which was red at
50,000 retained entries and green at zero. Two tests, two properties;
neither pretends to the other's job now.
The gate caught `commentary_index_lookup_is_not_quadratic` failing, then
passing on a re-run. Measured both ways: 1.62s alone, OVER 10s against
its own 10s deadline while the suite's other 3,347 tests ran
concurrently. A 6x margin against a shared CPU is not a margin, and a
test that gets re-run until it passes is not a test.
It was also measuring the wrong thing. Replacing the linear scan with a
HashSet bounded the LOOKUP; the set was still built from every entry the
disc declared. `playlists.xml` is attacker-controlled and has no length
of its own, so a hostile disc could still force an unbounded allocation
before any lookup happened — the parse, not the query, was the exposure.
The code's own comment said "unbounded parsed input" and only fixed half
of it.
`MAX_COM_INDICES` bounds both halves, at the one value that cannot
change behaviour: an index at or beyond `u16::MAX` can never match a
cell, because the labelling loops break at `u16::try_from(i + 1)`. Real
authoring is nowhere near it — the BD STN table admits 32 streams.
The parse moved into a `com_indices` helper so the bound is OBSERVABLE.
Through `labels_from_feature` it is not: a HashSet collapses repeated
values, and an out-of-range index changes no label, so the obvious test
passes with or without the cap — an assertion that cannot fail, which is
what the first draft of this fix shipped. The test now hands in 50,000
DISTINCT unaddressable indices and asserts they are refused. Proven red
with the bound removed (50,000 kept), green with it.
Applies to both `sub_com1_idx` and `aud_com1_idx`.
NOT done here: `labels/mod.rs:1421`'s `jar_inventory_dedup_is_not_quadratic`
is the same wall-clock shape and has the same flakiness. Named so it is
not lost.
Three round-1 audit findings on v1.6.0..HEAD.
Tests: `mux::resolve::input("dir://…")` — the door the CLI rips a folder
through — was never driven with a real folder. The only dir:// input test
uses a missing path, which fails in DirImage::open long before
`image_input`, so dropping the `is_folder` argument (and with it
`session::apply_folder_encryption_verdict`) passed the whole suite. Two
tests now close it, stated as a DIFFERENTIAL between the two doors,
because that is the invariant the shared function exists to hold: a
clear folder that kept its AACS/ directory must scan the same and select
the same extents through `scan_dir` and through `dir://`, and a
scrambled one must be refused through both with E9063. Proven red
against a mutated guard (both fail with E7022, the tree-shape verdict
the probe overrides) and green with it restored.
Docs: css/mod.rs documented `is_scrambled`, a function this line renamed
to `has_scramble_flag_bits`. Eleven stale sites, two of them broken
rustdoc links. Two were not stale names but false statements — they said
the descramble loop keeps the looser raw-flag test, when
`descramble_sector` and `descramble_region` both gate on
`is_scrambled_pack`; that parenthetical is rewritten to say what the
code does and why (the measured VIDEO_TS.IFO case: 38 titles became 10,
silently, at exit 0). The one mention that must stay is the sentence
explaining why the name was rejected.
Constants: io::image_writer and dirimage::encode each re-declared the
2048-byte sector that consts::SECTOR_BYTES already exports; both now
alias it. Note `BATCH_SECTORS = 2048` in image_writer is a different
quantity (sectors per batch, not bytes) and is deliberately left alone.
Bare 2048 literals elsewhere in the crate are out of scope here.
These are eight public repos that ship one product on one version, and
they had drifted: freemkv-engine carried no LICENSE at all (GitHub
reported its licence as none), two crates had no code of conduct, four
had no contributing guide, and none had a security policy — so there was
no private route to report a vulnerability in a disc-decryption tool.
SECURITY.md names GitHub Security Advisories on each repo rather than an
address, so there is nothing to keep in sync and no inbox to go stale.
dev is where work lands and is meant to be pushed to often, so what runs
there should be the cheap answer to "did I break it": lint, tests and the
Linux build. The macOS and Windows jobs now run on qa and main instead of
on every push to dev.
Nothing is deleted and no platform goes unchecked before a release. qa.yml
already covers macOS and Windows independently, and the jobs that live only
here -- the Intel macOS build, the Windows release build -- still run, on
the branches where a cross-platform break is worth blocking on.
They are SKIPPED on dev via `if`, not left unscheduled. A queued job would
be worse than a slow one: release.sh's CI gate refuses while any run for
the commit is still in progress, so a job that never gets a runner blocks
releases silently, with no error anywhere. That is the failure the
real-media note in qa.yml describes, and it is why this is an `if` on the
job rather than a narrower set of trigger branches.
Version bump and changelog for the 1.6.3 sync. All eight crates ship the
same version, so the crates with no functional change this cycle say so
rather than carrying an empty section.
Two problems, both invisible until the whole graph is looked at together.
DEAD: num-bigint, sha2, num-traits, num-integer, cmac and cbc are
declared here and referenced nowhere -- not in src, tests or benches.
They were being compiled, audited and offered version bumps forever for
no reason. Removing beats bumping.
cbc nearly survived the sweep: a substring search for "cbc" matches 44
occurrences of ycbcr_to_rgb in the DVD subtitle decoder, so it looked
used. Only a word-boundary search exposed it.
SKEW: this crate was the outlier on every shared dependency -- aes 0.8,
rand 0.8, base64 0.22.1 and zip 2 against 0.9 / 0.10 / 0.23 / 8
elsewhere. Cargo cannot unify across a major version, so it compiled
BOTH: 32 duplicated crates in the freemkv binary's graph, including two
complete AES implementations (aes 0.8 + 0.9, cipher 0.4 + 0.5), two
digest stacks and two getrandom. Two crypto stacks in one product is
worth removing on its own.
The aes bump is an API rename -- BlockCipher-prefixed traits, Array for
GenericArray -- and the obvious translation uses Array::from_slice,
which the new version deprecates and clippy's -D warnings would reject.
These use the From<[T; N]> conversion the crate points at instead.
3441 tests pass in debug and release. The AACS crypto here is covered by
known-answer tests, so a byte-order or sizing mistake in that rename
could not have passed.
Version updates were never configured here -- only security alerts, which
report but never open a pull request. So the absence of Dependabot PRs was
not "nothing to update", it was "nothing was checking".
Updates target `dev`, never `main`. main is a release pointer that
release.sh moves to each tag, so a bot commit on it would put work there
that no tag contains -- the exact state that aborted the 1.6.2 cascade at
its last step, and pointing a scheduled bot at main would recreate it
weekly.
Minor and patch bumps are grouped into one PR rather than one per crate:
eight repos times a handful of dependencies is a volume nobody reads, and
an unread PR queue is indistinguishable from no updates at all.
The freemkv crates are excluded. They depend on each other by git tag,
re-pinned by release.sh inside the release commit, and Dependabot cannot
see that cascade -- a PR bumping one could pin a version whose tag does
not exist yet.
It fired on `release: published` and committed to main, the branch the
release points at its tag, so it could only ever leave main ahead of the
tag. release.sh now makes the same edit inside the release commit.
In this repo it was a no-op regardless: the pattern it rewrote does not
appear in this README, so it has been running and changing nothing.
A clip's span is measured over the title's full extents when the disc is
scanned, and a frame is placed by the offset it was read from. Those only
agree while the mux reads every byte the scan counted.
On a disc carrying a forensic segment it does not: the read plan omits the
units belonging to another device group, so fewer bytes are fed than the
spans describe, and the shortfall grows through the title. Every frame past
the first segment then looks earlier than it is — placed in a clip it did
not come from, or dropped at a join for failing marks it was never inside.
The spans still tile one another perfectly, so the check that asks whether
they can be trusted cannot see any of it.
The plan is now compared against the extents it was built from. When they
differ the spans are dropped and placement falls back to timestamps, which
is what that path is for and what the surrounding comment already promised
would happen when an offset stops meaning anything. Ordinary discs are
untouched: with no forensic segment the plan IS the extents.
The provenance guard knew one way to lose a frame's source offset —
writing it as absent. Omitting the field entirely does the same thing,
because the frame type fills it in by default, and the guard read straight
past that. A parser rewritten into the second spelling would have kept its
green light while its track silently went back to being placed by guesswork.
It now reports which spelling it found and where.
The title-count clamp was asserted over a fixture too small to hold more
titles than the cap allows, so the walk stopped when the buffer ran out and
the clamp was never what bounded it — the assertion held with the clamp
deleted. The fixture now carries more entries than the cap.
The subdirectory limit was checked by restating the constant's own
definition; the guard itself had never run, and deleting it changed
nothing. The limit is lowered under test so a real folder can exceed it,
and the test now walks one and requires the refusal.
One test also carried two unrelated grounding notes while the test they
described had none, so reading the note above a test told you about a
different one.
A playlist may point at the same .m2ts from several play items — a
seamless split, a looped segment, multiple angles. The file has one set of
bytes, so it has one span, and the byte a frame was read from therefore
identifies the FILE, not which play item's range it falls in. Placement
took the first of them and judged every frame against its marks, so
everything past that range's end was treated as material the playlist
excludes and dropped: half the clip missing from the rip, with the
timeline still charged for its full duration.
The offset narrows a frame to the run; only its timestamp can finish the
job, and each play item carries its own marks. A frame is now matched
against the marks of the entry it actually falls in, falling back to the
first when it falls in none, which leaves genuinely-excluded material
dropped as before.
The test for this construction asserted only that such a playlist is
trusted, and never placed a frame from the second range — so the loss it
described in prose was invisible to it. It now places one from every range
and requires them all to survive.
The deferred-mux replay of buffered frames wrote them without their source
offset, quietly sending the head of every such title down the timestamp
heuristic the rest of this work exists to retire. It passes the offset
through now, and the frame writer that omits it is compiled out of the
library entirely: no production path can discard provenance any more.
Also: the AC-3 buffer-reuse test could not see the regression it named.
Feeding equal-sized packets, a fresh allocation per call yields the same
capacity as a reused one. It now feeds a large packet then a small one,
where only a reused buffer keeps the larger capacity.
Matthew Jackson