Seven defects in the code the test suite executes least — 913 lines of
disc/mod.rs alone are run by no test at all, which is why this round scoped
from coverage rather than from what previous rounds said they had read.
Disc::scan_image kept its own copy of the crack's extent ordering and fed
crack_key_outcome largest-cell-first. That is the fifth instance in this
audit of a local reimplementation drifting from the canonical one, and the
cost here is a key that does not descramble the feature: picking by sector
count bypasses the capacity gate and can select a different VTS entirely.
The copy is gone — which title comes from the canonical order the scan
already applied, and the extents are handed over in playback order,
exactly as decrypt_keys_for_title does. Its doc records why the duplicate
existed so it cannot grow back.
Resolution::pixels returned 1920x1080 for Unknown. That is the FOURTH
instance of one trap and the other three were in this same file, two of
them fixed hours earlier — without sweeping for siblings, which is the
whole reason this one survived. It now returns (0, 0), and the sweep was
done properly this time: every remaining Unknown arm across the crate is
honest, and the two ColorSpace sites that look like fabrication are
emitting H.273 code point 2, which is the spec's own "unspecified". Two
callers carried local Unknown-to-zero workarounds — precisely the cost of
making callers responsible for a lie — and one is now redundant.
BD-ROM Part 3 code 0xA2 is the lossy secondary DTS stream, not lossless
Master Audio. A test asserted the wrong mapping as intended behaviour, so
correcting the code failed it; the test is deleted with a note pointing at
its replacement. That is a NEW failure mode for this audit: not a test
that cannot fail, but one that locks the defect in. There is no
DtsExpress variant to map to, so it takes the lossy DTS-HD member and the
approximation is documented.
Also: DiscSession::identify could panic through drive_mut once the public
API allows an absent drive — two siblings were converted in an earlier
round and this one was missed; an extent end that added without saturating
where the rest of the crate saturates; a diag reason string restating the
comparator's sort keys and drifting from them, now derived from them; and
a short read that advanced the offset by the full request, silently
skipping the gap. That last one existed twice, in two reads with the same
shape, now merged so they cannot drift apart.
The short-read policy is a judgement call I could not derive from a spec:
no skip_errors is a hard error, with skip_errors zero-fills and charges
the loss. It deliberately does not retry mid-unit, because resuming inside
an AACS aligned unit would trade a silent gap for a silent decrypt
desync — the worse of the two.
Three copies of the same two mappings existed. The canonical accessors
returned 6 channels and 48000 Hz for Unknown; a third copy in diag.rs
returned 0. The honest one was the copy.
A plausible wrong answer is worse than an obvious one. Six channels at
48 kHz is indistinguishable from a real 5.1 track, so every caller became
responsible for remembering to check the variant first — and this crate
walked into exactly that: the json:// sink reported a confident 5.1 for
audio whose neighbouring fields said "unknown". That was fixed at the call
site earlier in this audit; this fixes it at the source.
The accessors now return 0, which is what both in-crate call sites already
coerced Unknown to by hand, so their guards are gone and the behaviour is
unchanged. Zero is also obviously wrong if it ever reaches output, where
six is not.
The diag.rs duplicates are deleted rather than corrected — a fourth copy
would have drifted too. Their only caller was a trace line in the same
file, now on the canonical accessors. Their tests moved across and gained
the Unknown case, which is the point: restoring either fabricated value
fails both.
Found by the round-7 correctness agent while fixing the json:// sink; it
flagged the third copy as out of its scope rather than touching it.
Same shape as the mkv:// conflation fixed earlier in this round, found by
looking for it deliberately. E7023 carried two conditions with opposite
correct responses: one title on a multi-VTS DVD failing its own re-crack,
where skipping it and finishing the rest is right, and the main feature's
crack failing outright, which is disc-wide and dooms every title
identically. Because both raised the same code and that code is in
is_skippable_title_stub, an uncrackable disc walked all N titles printing
"title skipped, it was empty" and exited 0.
The disc-wide condition gets E7027 CssNoDiscKey, mirroring the AACS-side
E7022 NoDiscKey it is the analogue of, and joins is_disc_level_no_key.
The per-title raise keeps E7023 and stays skippable. Because the engine's
classifier already tests is_disc_level_no_key before the skippable
branch, this reaches the right outcome downstream with no change there:
such a disc now stops on the first title and reports no-key instead of
returning success with nothing written.
Disc::css_error deliberately still stores CssKeyMissing — autorip matches
that variant on the field to pick the CSS rather than AACS message, and
what consumers classify on is the gate's returned verdict, which is the
only thing that changed.
Two neighbouring CSS raises were examined and deliberately left alone:
the no-key branch in the same function is genuinely unreachable via
ensure_decryptable and documented as defensive, and resolve_dvd_title_key
is per-title on both of its call paths.
Verified by removing the new code from is_disc_level_no_key, which fails
both new tests; each pins both directions so neither can silently flip.
Not proven end to end against a real uncrackable disc — none available.
Two defects in fixes landed the same day, both found by round 4 auditing round 3's
work rather than trusting it.
**The E-AC-3 grouping ignored substreamid, so the timeline still doubled.**
Confirmed independently by the correctness and conformance lenses and verified by
hand: substreamid appeared only in test helpers, never in the production path. Per
ETSI TS 102 366 (A/52) Annex E a frame set is independent substream 0 — mandatory,
always first — with its dependents, then the OPTIONAL additional independent
substreams 1..7 with theirs, all covering the same time period. Treating an
additional independent substream as a new access unit advanced the clock a second
time for the same 32 ms, which is exactly the doubling the grouping fix existed to
prevent. No fixture caught it because every fixture used substreamid 0.
is_dependent_substream becomes substream_role -> Starts | Extends: strmtyp 1
extends; strmtyp 0/2 with substreamid != 0 now extends (this was the bug); strmtyp
0/2 with substreamid == 0, legacy AC-3, and reserved strmtyp 3 start. Reserved 3
starts regardless of its id bits, because its BSI layout is undefined so those bits
cannot be trusted — an unknown frame is neither merged into an unrelated programme
nor silently discarded.
The frame set stays ONE sample rather than being split into a separate track for
the associated service, and the reasoning is in the module doc: a substream
numbered 1..7 with no substream 0 is not conforming, so extracting one would mean
renumbering ids and rebuilding frame sets — a transcode, not a remux. Programme
selection is the player's job.
A stream joined mid-frame-set (first sync is substreamid 3) is skipped with a debug
and resyncs at the next id-0, mirroring the orphan-dependent rule: its mandatory
id-0 substream was never seen, so it is neither decodable alone nor timeable.
MAX_AC3_BUF 128 KiB -> 1 MiB, because an AU is now a whole frame set: worst case 8
independent x 9 substreams x 8192 B = 576 KiB, which the old cap could have dropped
mid-hold.
**The forced probe's two round-3 fixes cancelled each other.** CHUNK_SECTORS = 1023
exists (with a const assert) so every read starts on a 3-sector AACS aligned-unit
boundary; the short-read fix advanced by actual bytes, making the advance a
non-multiple of 3. Every later read was then misaligned, DecryptingSectorSource
refused it before reading, the stop became ReadFailed, and no verdict was asserted
— so content-based forced detection silently fell back to the vendor label on
exactly the encrypted discs the 1023 change was written for.
A partially-satisfied read now advances only by whole aligned units and re-reads
the <=2 residue sectors from the next boundary, feeding only the aligned prefix so
nothing is double-fed and no partial unit reaches the parsers. A read that fully
satisfies its request still advances by all of it. When less than one aligned unit
comes back the bytes are fed and the same LBA is retried twice before stopping, so
a starved source cannot spin — verified by raising the retry limit and watching the
test hang.
Verified red independently here: reverting substream_role to strmtyp-only fails
eac3_additional_independent_substream_stays_in_the_frame_set (6 access units where
3 are correct — the doubling, literally) and the mid-frame-set resync test.
Reported, not fixed: dec3_box still hardcodes num_ind_sub - 1 = 0 and
num_dep_sub = 0, so it under-declares any stream carrying additional independent
or dependent substreams now that frame sets arrive whole. DolbyConfig has no
fields for either; a real fix needs the parser to surface observed substream
counts. That file is another lens's this round.
Unverified: no real multi-programme DD+ stream exists here, so defect 1 rests on
synthetic Annex-E fixtures. The retail DD+ check (No Time to Die, all substreamid
0) confirms single-programme discs are unaffected.
The Windows UI needs current winsafe, whose real minimum is 1.89 (its manifest
under-declares 1.87 while it uses NonNull::from_ref). Rather than stop at the
minimum, this goes to current stable and fixes what that costs.
The counter-intuitive result: 1.97 is CHEAPER than 1.89. libfreemkv had 54
clippy errors at 1.89 and 6 at 1.97, because clippy tightened the noisy
collapsible_if lint in between. Stopping at the minimum would have been the most
expensive choice available.
Roughly 47 lints across the eight repos, the large majority auto-fixed:
libfreemkv 6, freemkv-engine 14, bdemu 8, freemkv-keysources 7, autorip 6,
freemkv-unlock 3, freemkv-i18n 3. The hand-fixed ones are a descending sort to
sort_by_key(Reverse), four manual checked-division sites, a loop counter replaced
by enumerate, and a loop whose first let-else became a while-let.
Worth recording for whoever bumps next: clippy is MSRV-AWARE. Those 54 lints only
appear once the crate DECLARES 1.89 or later, because let-chains become
available. A bare `cargo +1.89 clippy` against a manifest still pinned at 1.87
reports clean and is meaningless — gate with the real precommit script, which is
also the only thing that covers build scripts.
The pin still sits below the Mac default, so it keeps doing its job: catching
lint drift locally before CI sees it.
All 21 findings held up under verification; 15 fixed here, 6 deferred to files
another agent held this round, 0 rejected.
**A defect in my own round-2 probe fix.** CHUNK_SECTORS was 1024, and
1024 % 3 == 1 — verified — so every chunk after the first was misaligned against
the 6144-byte AACS aligned unit and would be REJECTED by
DecryptingSectorSource's alignment gate. On an encrypted disc the forced-subtitle
probe I added last round would have read almost nothing past its first chunk.
Now 1023 sectors (341 aligned units) with a const assertion that fails the build
if it stops dividing, plus set_unit_base per extent so the source's gate is
anchored where the extent actually starts.
**The same probe skipped sectors on a short read**, advancing by the REQUESTED
count rather than the bytes actually returned, so a partial read silently left a
gap in the middle of the evidence. It now advances by n/SECTOR_BYTES and clamps n
to the buffer.
**Its cache key omitted the PGS PID set**, so a playlist declaring an extra
subtitle PID got another playlist's verdict for a track that had never been
probed. And the key was the whole extent list, so partial clip sharing missed
entirely. Both fixed by keying (start_lba, sector_count, pid) — and per-extent
keying was shown SOUND rather than assumed: ForcedTracker is two monotone
booleans, so per-extent evidence composes by field-wise OR, order- and
grouping-independently. Making that honest required per-extent demux state, so an
extent's evidence comes only from its own bytes, and memoising only extents whose
read reached a designed stop.
**A reachable panic in the timeline.** mkvstream::parse_block accepts a
TimestampScale up to i64::MAX, so a video frame can set high_ns = i64::MAX and the
next passive frame panicked adding the backstep. In release it wrapped negative
instead, firing the straggler clamp for essentially every passive frame — audio
and subtitles rewritten onto the wrong point of the output timeline. All four
sites saturate.
**A public constructor divided by zero**: PrefetchedSectorSource::new_with_events
with unit_align == 0. Now InvalidInput, matching its batch_sectors sibling.
**Two Debug impls printed key material.** DiscInputs (volume_id, mkb, unit_key_ro,
samples) and UnitKeyFile both derived Debug. Nothing logs them today — fixed as
prevention, because the next tracing::debug! someone adds is the leak. A doc claim
that DiscInputs "contains no secrets" was false and is corrected.
**An env-var multiply could overflow** in file_sector_source; now bounded at 64 GiB
like its writeback sibling, with the parse split out so the bound is testable
without touching process env.
**The mp4 demuxer allowed one sample per file byte** — ~64x RAM amplification.
Now file_len/16, since only vide/soun tracks are indexed and the shortest legal
AC-3 frame is 128 bytes.
**Two pipeline concurrency defects**: a consumer apply() error was invisible to the
producer, and abandon/finalise had a TOCTOU where a caller could report an
unfinalised output. Both fixed with compare-exchange state rather than a bool.
**Two per-frame copies removed**, both MEASURED rather than reasoned: the AU
assembler now hands its allocation to the frame (same pointer, unchanged capacity,
proven by asserting the pointer) and tsmux reuses one Annex-B buffer across
frames. Both keep capacity deliberately — a naive split_off would have cost more
than it saved.
**A comment pointed at the wrong file** for a mirrored constant; the mirror is now
compiler-enforced with a const assertion converting 90 kHz ticks to ns, so drift
fails the build.
Deferred to another agent's files, all confirmed: detect_rate's fractional-twin
snap, the mp4 reserve's u32 truncation, round_up_grain's overflow, the quadratic
base-key gap fill, and MkvStream's frame cap counting frames rather than bytes.
Every fix verified red by reverting it. Also noted for later:
DecodeSampleSet still derives Debug over multi-MB of on-disc ciphertext.
resolve_content_key_map loops every title into resolve_mux_key_map, which called
resolve_fmts_key_map FIRST — before the CpsUnitCache — and on an FMTS disc
returned immediately. So every playlist re-derived facts that belong to the DISC:
a full UDF walk plus /AACS/IndividualSegment.tbl, and on an FMTS disc the anchor
probe, the 32-index phase probe, and a fetch.fmts_indexes round trip.
On a 60-playlist disc, measured on a synthetic fixture: 840 -> 14 metadata reads,
2,400 -> 40 probe reads, and 60 -> 1 key-service calls. Worst case before was up
to 256 probe reads and 32 key-service calls per title. The 60 redundant
key-service round trips are a strong candidate for the keyserver storm seen in
the field.
Two memos behind a pub(crate) DiscKeyCache. The table memo (UDF walk + tbl parse)
is disc-invariant outright — nothing in that path mentions the title — and runs on
EVERY disc, so a plain BD benefits too. Only the deterministic negatives are
memoised as "not FMTS"; a DiscRead fault propagates uncached so a later title
retries.
A blind once-per-disc hoist of the PROBES was rejected as unsafe, and this is the
load-bearing reasoning: the title enters through clip_byte_to_lba, which decides
which segments are addressable and which LBA every probed clip byte reads from, so
two titles with different extent lists probe different physical bytes. A hoist
would serve title B an answer derived from title A's media and could silently turn
a per-title FmtsKeyMissing into a success. The extent list is the ONLY per-title
input, so keying on it is exactly sufficient — matching the ForcedProbeCache
precedent.
Result-identity was proved, not assumed: NEITHER probe reads the key pool.
Verified here independently — probe_fmts_index_keys takes no keys parameter at
all; index keys come from `fetch`, and the anchor's reply feeds the phase probe.
So the pool's growth across titles, the one thing that does change between calls,
cannot move a memoised value, and the result is order-independent. A test resolves
three titles through a shared memo and through fresh memos and asserts both the
per-title ranges and the final key pool (keys, slots, order) are identical.
Not memoised, deliberately: fail-loud FmtsKeyMissing, and any run where an index
hit a read fault — that is a property of a transient drive fault, not of the
extents, and caching it would spread one bad read across 59 playlists.
A fully-memoised title now does zero I/O, which made the old in-loop halt polls
unreachable for it, so a check_halt on entry was added with a test that cancels
after warming the memos.
Also corrects my own overstatement from last round: the CpsUnitCache doc now says
plainly that on an FMTS disc it removes NO reads, because this function returns
before the extent loop ever runs.
Five mutations, all verified red. Pre-existing bug flagged but not fixed:
filter_addressable_segments only checks that a segment's START byte maps to some
LBA in the title, so a play-all playlist can pass the filter while mapping segment
bytes into the wrong clip, whose anchor then returns empty and aborts the sweep.
resolve_content_key_map calls resolve_mux_key_map once per title, and on a
multi-CPS disc that path issues 8 random single-unit reads per extent. A disc's
playlists overwhelmingly reference the same few clips — main feature, play-all,
per-chapter and seamless-branch variants — so the same physical extents were
re-sampled from the drive once per playlist. On a 60-playlist / 15-clip disc
that is ~2,400 non-sequential 6144-byte reads, roughly 8 minutes of pure seeking
at 200 ms per seek, before the mux starts. Now ~600 reads.
Keyed per EXTENT — (format, start_lba, sector_count) — rather than per title's
whole extent list, which is finer-grained than the forced-subtitle probe's cache
and strictly better here: a play-all playlist sharing 4 of 5 extents with the
main feature still hits on those 4.
Why a cached pool index is provably identical to a recomputed one, verified
rather than assumed:
* `pick` iterates the pool IN ORDER and returns the FIRST index whose key
decrypts a sample to clean.
* The pool is APPEND-ONLY. Checked across the whole crate: only `push`, with no
insert/remove/clear/retain/sort/dedup/truncate/drain/swap/reverse anywhere.
So appended keys can only land AFTER a matched index, and the first match for
the same samples cannot shift.
* The samples are a pure function of the three values in the key, read from
read-only optical media.
Two outcomes are deliberately NOT cached, which is what makes this safe rather
than merely faster:
* the inherited index (`None if samples.is_empty() => last_idx`) is per-TITLE
state, not a property of the extent — caching it would let one title's
carry-in index leak into another title's clear extent, i.e. a WRONG key;
* the fail-loud DecryptFailed verdict, so a retry after a key source banks the
missing key re-samples instead of inheriting a stale answer.
Halt is still polled before the cache lookup, so cancellation is unchanged.
resolve_mux_key_map keeps its exact signature and delegates with a fresh cache,
so there is no public API change. ContentFormat gains Eq + Hash (additive).
Four tests, and the two mutants that matter both verified red: disabling the
cache short-circuit fails the hit and recompute-equivalence tests, and wrongly
caching the inherited index fails multi_cps_inherited_index_is_not_cached.
probe_and_set_forced broke out of its read loop on a read error but still
applied whatever partial observation it had accumulated as an authoritative
verdict, overwriting the vendor-label-derived forced flag. A disc that faults
early could have a correct flag replaced by a guess from a fraction of the data.
The file already got the zero-observation case right — it deliberately leaves
the vendor flag alone rather than "assert not-forced from having seen nothing".
The defect was that a PARTIAL observation cut short by a fault was treated as
complete.
The fix rests on the two verdicts not being symmetric evidence.
settled_not_forced() is POSITIVE evidence — a non-forced display set was
actually seen, and no unread data can retract it. is_forced() is an ABSENCE
claim — display sets were seen and none was non-forced — which is only sound if
the read got far enough for the absence to mean something.
So every loop exit now yields a named StopReason, and absence claims are
asserted only for a designed stop:
* Exhausted / Budget → conclusive. The budget is deliberately conclusive: the
natural exit is "every track settled not-forced", which a genuinely forced
track never satisfies, so the budget is the ONLY path by which a real forced
verdict is ever reached. Treating it as inconclusive would disable forced
detection entirely.
* Halted / ReadFailed → inconclusive. A cancelled probe's cut-off point is as
arbitrary as a faulted one, so its absence claim is worth no more.
Evaluated per track, matching the existing observed() gate: a track that already
saw a non-forced set keeps its sound verdict even on a truncated run, while a
forced-so-far sibling keeps the vendor flag.
An inconclusive run is also NOT memoised. The cache key is the extent list and a
disc's playlists share clips, so caching a truncated run would replay one read
fault onto every playlist referencing those extents and deny any later title the
chance to re-read them.
Four tests; three of them verified red by forcing absence_is_conclusive() back
to always-true (the old semantics), while the budget test correctly stays green
either way — confirming the guard was not over-corrected.
Also moves the function doc comment back onto probe_and_set_forced; the earlier
probe commit left it attached to the ForcedProbeCache type alias.
Two defects in the encrypt_unit promoted to public API last round, both found by
round 2 auditing that new code.
It returned silently without encrypting when the slice was shorter than
ALIGNED_UNIT_LEN. Its own contract requires the caller to set the container's
encrypted flag BEFORE calling — the header is the key seed — so a silent no-op
leaves a unit advertised as encrypted while still carrying plaintext, with
nothing for an authoring caller to check. It now returns bool and is
#[must_use], so ignoring the refusal is a compile-time warning; every call site
was updated to assert on it.
bool rather than Result deliberately: a wrong buffer length is a programming
error at a library boundary, not a disc condition, and a new Error variant would
mean a new numeric code plus its rendering in another repo.
It also drove CBC from the single-block aes_ecb_encrypt, rebuilding the AES key
schedule for each of the 383 blocks in a unit — an order of magnitude slower
than its inverse, which expands the key once via aes_cbc_decrypt. The missing
counterpart aes_cbc_encrypt now exists alongside it, and encrypt_unit calls it,
so the two directions are symmetric in structure as well as in result. For an
authoring caller encrypting a 90 GB image that removes ~5.6 billion redundant
key expansions.
New test pins the boundary: ALIGNED_UNIT_LEN - 1 returns false and leaves the
buffer byte-identical, ALIGNED_UNIT_LEN succeeds. The existing round-trip and
padding-asymmetry tests still pass, so the CBC rewrite is provably the same
transform.
The probe's only natural exit was "every PGS track has shown a non-forced
display set". A genuinely FORCED track never satisfies that, so on the common
authoring — a forced-narrative track for foreign dialogue — the loop read the
title's entire extent set at 2 MiB per call with no byte cap, no time cap and
no halt check. It was also invoked once per title rather than once per distinct
clip, and a disc's playlists overwhelmingly reference the same few clips (main
feature, play-all, seamless-branch variants), so the same physical extents were
re-read 30-150 times. The two defects multiplied: tens of GB, times the
playlist count, off an optical drive.
Reached via ScanOptions::probe_forced_subtitles, whose only consumer is
`freemkv info -v` (freemkv/src/disc_info.rs). The rip path leaves it off. So
the symptom is `info -v` never returning on an ordinary UHD, not a corrupt rip.
Three changes:
* PROBE_BUDGET_SECTORS caps a probe at 256 MiB. A forced track's display sets
appear throughout the title, so a bounded prefix classifies it; the budget
only decides how long we keep looking for a non-forced set before accepting
the forced verdict.
* ScanOptions::halt is now plumbed in and checked per chunk, so `info -v` is
cancellable. The probe previously took no halt at all.
* A ForcedProbeCache memoises verdicts against the title's exact extent list.
Keying on byte-identical input means a hit cannot change any result — it
only removes the re-read.
Verdict application is factored into apply_verdicts so the cached and freshly
probed paths cannot diverge.
Three tests pin the behaviour, each against a reader that counts sectors and
never ends: the budget stops at exactly PROBE_BUDGET_SECTORS, a cancelled halt
reads zero sectors, and a second title with identical extents costs no further
reads while a different extent list still misses the cache.
Also worth recording: the CLI acceptance harness never exercised `info -v`
against an optical drive — it reads ISOs from local SSD, where a full-extent
read is fast enough to hide both defects.
encrypt_unit becomes public library API rather than a #[cfg(test)] helper.
Authoring an encrypted disc image is a legitimate use of this crate, and
the capability was already written four times over: a pub(crate) test-only
copy in aacs/content.rs plus three hand-rolled duplicates in decrypt.rs,
sector/decrypting.rs and disc/extract.rs. All four now call one function,
removing ~110 lines of duplicated cipher code that could drift from
decrypt_unit independently.
It mirrors decrypt_unit's purity contract: crypto only, no encrypted-flag
handling, because where that flag lives is container-specific (CPI bits in
byte 0 for BD-TS, elsewhere for HD-DVD-PS). Callers set the flag BEFORE
encrypting — bytes 0..16 are the key seed left in plaintext, so touching a
header byte afterwards changes the key a decryptor derives. That footgun is
documented at the function and at every call site.
Two tests pin it: an exact round trip through both directions, and the one
place the pair is deliberately asymmetric — decrypt_unit restores
all-zero-on-disc packets to zero, and the test proves an all-zero plaintext
packet enciphers to non-zero bytes so it is never mistaken for padding.
That asymmetry was previously only prose.
Two decrypt tests were also weaker than their own names:
* aacs_clear_trailing_partial_passes_through asserted only is_ok(), so a
mutant corrupting the clear partial while returning Ok passed. It now
snapshots the buffer and asserts byte equality, matching the
none_keys_is_noop pattern already in the file.
* aacs_decorator_decrypts_encrypted_unit_via_map checked only that 0x47
reappeared at the 192-byte stride, leaving corruption in the other 6112
bytes undetected. The plaintext is fully known, so it now asserts
byte-exact recovery against it.
Both were verified red first by mutating the production path.
The bisect ReadAction regression tests moved to freemkv-engine with the
recovery strategy, but their explanatory block stayed behind — seventeen lines
describing a `let _ = handle_read_error(..)` bug and asserting "the tests below
prove the required ReadAction values are produced". There are no tests below it;
handle_read_error is not even resolvable here any more. Anyone auditing whether
that bug is still guarded would read this and conclude yes. The block moved to
the engine alongside the tests it describes.
The sweep/patch recovery strategy, mapfile, retry-decision state machine,
section-recover, and damage classification move out of libfreemkv into the
new freemkv-engine crate. libfreemkv keeps the raw single-shot read and
SCSI-fact translation (SenseFamily stays in scsi).
- Delete disc/{sweep,patch,mapfile,read_error,section_recover}.rs, the
Disc::copy/sweep/patch methods, the Copy/Sweep/Patch option+result types,
classify_damage/DamageSeverity, progress_snapshot_from_mapfile, and the
three recovery integration tests.
- Trim public surface the recovery deletion orphaned: delete the dead
READ_PIPELINE_DEPTH const, the write-side SectorSink/FileSectorSink (no
consumer), and the DriveSpeed enum (its one live use — set max drive
speed — becomes Drive::SPEED_MAX_KBPS). Make mapfile_path_for,
decrypt_sectors_mapped pub(crate); gate NoopEvents to test.
- Version 1.6.0.
Typed iterators over the audio/subtitle/video streams, cleaner than matching on
the Stream enum for the common iterate-the-tracks case (stream selection, the
desktop UI info panel, disc-info). Additive; all lib tests pass on 1.86.
Moved SenseFamily::from_sense_key + is_wedge_family from disc/read_error.rs
into scsi/mod.rs (with its own tests) and re-exported at the crate root.
This is pure SCSI sense-code classification -- objective hardware fact, zero
recovery-policy opinion -- so it belongs in the library primitives, unlike
the retry-DECISION state machine (ReadCtx/PassSummary/ReadAction/
handle_read_error) built on top of it, which is freemkv's specific recovery
strategy and is moving to freemkv-engine next.
disc/read_error.rs and disc/section_recover.rs now import SenseFamily from
crate::scsi instead of defining/re-exporting their own copy. No behavior
change. Precommit green on Rust 1.86 (fmt+clippy+test).
The upcoming freemkv-engine crate needs both to build the multipass
sweep/patch recovery strategy externally over Disc's public API. Everything
else sweep/patch touch on Disc was already pub; these were the only two
gaps. No behavior change -- visibility only.
Round-2 follow-ups to 6d6e60f (inline base-map resolve on the live
single-pass Session/Live mux arms).
Fix 1 (halt threading) — the inline resolve chain sampled ciphertext off
the LIVE drive with no cancel token, so an operator /api/stop during key
resolution was not honored (the FMTS probe can issue hundreds of reads,
each able to stall to the 60s SCSI recovery timeout — violating the
"don't hammer a struggling live drive" rule). Add an optional
`halt: Option<&Halt>` to `resolve_mux_key_map`, `resolve_fmts_key_map`,
`resolve_inline_base_map`, and `Disc::resolve_content_key_map`, and poll
it at each loop boundary (FMTS anchor + per-index probe loops, multi-CPS
extent loop) — returning Err(Halted) promptly. Live/Session arms pass the
driver's halt; sweep/patch pass their own token (via Halt::from_arc);
file-backed probe/ISO callers pass None. Tested with a pre-cancelled halt
(Err Halted, no extent sampling) and a None-halt no-abort case;
mutation-verified (dropping the extent-loop check → Ok, not Err).
Fix 2 (Session-arm coverage) — the MuxInput::Session arm ran the same
resolve→install→decrypt sequence as Live but had NO end-to-end test
(DiscSession only exposed open(), which needs live hardware). Add a
#[cfg(test)] DiscSession::from_parts_for_test (injected reader + scanned
disc, no Drive), an end-to-end AACS decrypt test through the Session arm
(mutation-verified: dropping with_key_map → mux aborts), and a
missing-reader clean-error (not panic) test.
Fix 3 (cleanups) — io_error_code: remove the unreachable typed-Error
downcast branch (From<Error> for io::Error stringifies; no path builds an
io::Error holding a typed Error), keeping the stringify parse is_halt /
is_skippable_title_stub rely on. Add a resolve_keys_for test covering the
largest-title sampling branch. Document the patch wedge-exit coverage gap
(TODO) in passn_handler_ab.rs.
- dts: accept all 16 legal AMODE channel-arrangement codes (0-15), not just
0-9. Per ETSI TS 102 114 the 6-bit AMODE field has 16 defined arrangements;
only 16-63 are reserved. a reference decoder the spec per-AMODE channel table confirms 10-15 are
decodable 6/7/8-channel layouts. The old bound of 10 dropped spec-legal
multichannel core frames as undecodable, silencing recoverable audio. Add a
regression test (literal 0..16 range) that fails if the bound reverts to 10.
- keysource: only memoize a NEGATIVE (empty) key-fetch result when every source
genuinely ran and none held the key — never when a source Err'd (network down,
unreachable). A transient outage was being cached as a permanent "no key" for
the fingerprint, permanently dropping a unit that could be recovered once the
source came back. Thread an `errored` flag out of the drivers and gate the
cache insert on it. Tests cover both the recover-after-outage case and that a
genuine absence is still memoized.
- pgs_forced_probe: add happy-path coverage feeding real synthetic BD-TS PGS
display sets through the full demux -> parse -> observe -> apply path, both a
forced verdict landing and a non-forced verdict clearing a vendor flag.
- mp4: correct fit_report doc (audio carried is AC-3/E-AC-3 AND DTS/DTS-HD).
- scan_iso test: add independent fixture expectations (volume id) so the parity
test is no longer purely tautological against a re-run of the same composition.
Multi-round audit of the decrypt/AACS/mux-codec refactor. Fixes, in
descending severity:
- mux/mp4/read.rs: bound untrusted-input allocations. `sample_budget`
now also capped by file_len (a fixed-size stsz claiming count=u32::MAX
can't inflate the Vec<SampleRef> past the file's own size); trak scan
capped at MAX_TRACKS matches; find_box() takes only the first match
(cap=1) instead of materializing every match. Removes dead find_boxes
wrapper.
- disc/mod.rs: merge_content_key_ranges now UNIONS same-key overlapping
ranges (coverage-preserving) instead of dropping the non-overlapping
tail, which silently left encrypted LBAs uncovered -> ciphertext
passthrough in the whole-disc sweep/patch map. Different-key overlap
(malformed) still dropped to keep the set disjoint.
- sector/decrypting.rs: remove dead unit_key_idx field + with_unit_key_idx
setter (vestigial from the pre-keymap trial-decrypt design; AACS is
map-only now). Fix stale docs.
- decrypt.rs / resolve.rs / error.rs / extract.rs: doc/comment drift from
the refactor (AacsKeyMap positive-map semantics, resolve_mux_key_map doc
reattachment, decrypt_sectors_in_content legacy-alias, E_MP4_INVALID
meaning, multi-CPS orphan by-design note).
Test coverage (all mutation-verified real):
- DTS NeedMore force-flush buffer bound; FLAC/MPEG-audio PTS carry-forward;
mp4 mdhd timescale=0 divide-by-zero guard, MAX_TRACKS cap, sample-count
file_len bound, MAX_ALLOC_BYTES cap under inflated file_len.
- resolve_fmts_key_map: extracted filter_addressable_segments,
resolve_tie_phase, fill_base_key_gaps as pure behavior-preserving
helpers, each unit-tested (segment filter, phase-tie arms, gap-fill
gaplessness over every extent).
- AacsKeyMap now derives its distinct key-index set once at construction
(from_ranges_phased) instead of re-allocating/sorting it on every
decrypt batch; key_indices() returns the cached slice.
- Extract the whole-disc content-map range merge out of
resolve_content_key_map into merge_content_key_ranges and cover it:
sort/disjoint, shared-clip dedup, overlap drop, adjacent-kept.
- resolve_fmts_key_map: filter segments to those addressable within THIS
title's extents; a title with no forensic content (menu/extras playlist,
or a different clip) returns Ok(None) and takes the base Unit-Key/CPS
path instead of hard-failing FmtsKeyMissing. Previously the first
non-forensic title aborted the entire whole-disc sweep
(resolve_content_key_map iterates every title) and blocked muxing any
non-main title.
- FMTS phase probe: an even/odd is_clean tie now only fails loud when
BOTH halves are 0 (no clean decrypt). A both-clean tie is source-zero
padding (is_clean is true for any key on all-zero content) — the key is
valid, default Even, never abort the rip on a padding-heavy sample.
- extract_tree: multi-CPS discs now build the exact per-CPS content map
(resolve_content_key_map) instead of a blanket key-0 map that silently
mis-decrypted every secondary-CPS file into garbage. Single-CPS keeps
the blanket key-0 map (one key opens every unit, incl. orphan clips).
- decrypt_sectors_mapped: a trailing partial unit that is inside a mapped
range AND flagged encrypted in its clear seed now fails loud (a CBC
fragment split across a boundary can't be decrypted) instead of being
emitted as clear. New aacs_unit_seed_encrypted reads the flag on a
partial.
- Correct the stale decrypt_sectors doc (AACS arm now always errors;
AACS decrypts only via decrypt_sectors_mapped).
Every AACS decrypt now goes through the resolved key map (decrypt_sectors_
mapped): the map keys each content unit up front and a missing key fails at
resolve time. The old trial-decrypt path — try each held key per unit, keep
the first-tried plaintext on a miss — is gone; decrypt_sectors_impl's AACS
arm now fails loud (reaching it means a reader was built without its map,
which would silently apply a wrong key). CSS (self-descramble) and the clear
no-op path are unchanged.
Disc::sweep and Disc::patch resolve a whole-disc key map up front for a
decrypting pass (the fetch secures any missing CPS-unit key, fail-loud) and
decrypt via the map — clear nav/filesystem sectors are in no range and pass
through, so the separate content-range gate and the reactive per-unit
key-fetch recovery are no longer needed. extract_tree keys every unit with
the base Unit Key through the map (its encrypted-flag gate skips clear
files). Multipass sweeps stay --raw.
Removes the obsolete non-mapped-AACS trial/gate/recovery tests (the mapped
path and resolve fail-loud are tested directly).
mp4 demuxer (untrusted input): bound every allocation sized from a box
field (stsz/stco/stsc counts, stts/ctts run-lengths, per-sample and moov
sizes, plus an absolute cap so a sparse file can't inflate file_len);
guard the parse_stsd slice and a zero mdhd timescale; cap track count so
the per-track PID can't overflow; rewrite read_moov to handle size==0 /
size<8 / 64-bit largesize; parse esds/AudioSpecificConfig for AAC; write
tkhd duration in the movie timescale.
decrypt: resolve_mux_key_map now fails loud on an extent no key can
classify instead of inheriting the previous extent's key, so a keymap
never silently carries a wrong key; the sweep/patch key-fetch recovery
fails loud when a unit is still unresolved after the retry.
AACS: reject inverted forensic segments in both range builders; compare
the forensic index in u16 space so an out-of-range value can't truncate
onto a valid u8 index. RECOVERED_ERROR no longer latches the damage zone,
preserving the 30s wedge cooldown for a following hard error.
audio: AAC/MP2/MP3/FLAC carry the last PTS across a PES with no timestamp;
the DTS-HD extension-sync search is bounded to after the core; the MP4
16.16 sample-rate field saturates. demux_sink records the video reference
before the kind filter so audio:// / sub:// keep multi-clip PTS continuity
and the DELAY tag.
Remove a dead error variant and the AACS-unsupported-video code; codec
comments cite the primary format specs; assorted doc/naming fixes and
regression tests throughout.
Every DVD read path — the file-backed mux highway (build_iso_pipeline) and
the live-drive single-pass DiscStream — now resolves the per-VTS CSS title
key through one shared step, css::resolve_dvd_title_key, cracked keylessly in
playback order from the title's own extents. Removes the earlier design that
reused a single scan-time key (meaningless for a per-VTS scheme) and muxed a
detection-miss disc's scrambled sectors as garbage.
- Disc::scan no longer cracks a key up front; it does only the CSS bus-auth
read-unlock, hoisted before the UDF prefetch so scrambled small/menu VOBs
no longer cost a rejected read each (CSS-DVD scan ~25s -> ~6s).
- An uncrackable title hard-fails (E7023) instead of passing ciphertext as
plaintext; --raw skips the crack entirely; a Stop mid-crack surfaces Halted.
- DiscStream::new is now fallible and threads raw + halt.
- Fix a stale codec-parser doc claim (TrueHD/FLAC/MP2/AAC do gate via DropTally).
A CSS DVD whose main title was mis-detected as unencrypted (the up-front
crack scanned the largest cell first and starved its budget in that cell's
clear prefix) muxed scrambled sectors as plaintext at exit 0. CSS leaves
the pack/PES header clear, so an un-descrambled sector muxes as a
structurally-valid but corrupt PES packet with zero loss reported.
decrypt_keys_for_title resolves a DVD title's CSS key two ways:
- Fast path: reuse the scan's cracked key when its crack_span covers this
title's VTS (no re-read; on a live drive no second bus-auth).
- Crack: on a detection miss or a different VTS, crack from the title's OWN
extents in a SINGLE scan in natural PLAYBACK ORDER (never largest-first).
One scan = one CSS-locked early-bail, so a locked title is not re-hammered
per cell against a live drive (hard rule #2); the 50k-sector budget is the
same accepted bound the disc-wide scan uses. Cracked -> key; Unencrypted
-> clear; ScrambledUncracked -> hard-fail.
ensure_title_decryptable hard-fails an uncrackable DVD title even when
detection missed, and passes a title that resolved its OWN valid key
regardless of the disc-wide css_error. descramble_region is unchanged from
v1.5.1 (validated-key seed).
Also rename the unlocker report's DVD entry CSS -> DVD. Bump 1.5.2.
The HD DVD AACS directory name and title-key filename are chosen by the
authoring house, but the resolver hardcoded a single spelling
(/ANY!/VTKF000.AACS, /ANY!/MKBROM.AACS, /ANY!/CONTENT_CERT.AACS). Real discs
diverge: Freedom (Memory-Tech) names its AACS dir AAC! and ships VTKF090.AACS
+ VTKF100.AACS; Harry Potter carries VTKF000/001/002/099. On such a disc the
hardcoded path finds nothing, so no MKB/title-key/cert is read and decryption
silently can't engage.
Replace the fixed HD DVD path constants with structural discovery:
- find_hddvd_aacs_dir() locates the AACS dir as the root child dir ending in
'!' that contains MKBROM.AACS (so the ..._BAK mirror is skipped; the dozens
of decoy advanced-content '!' dirs are excluded by the MKBROM.AACS guard).
- role_paths(udf, role) builds the ordered candidate list per role: the static
BD/UHD /AACS/ paths first, then the discovered HD DVD files — MKBROM.AACS,
CONTENT_CERT.AACS, and every VTKF*.AACS (sorted), not just VTKF000.
- read_first() is now generic over &str / String so it takes the Vec<String>.
BD/UHD unaffected (no '!' dir → discovery returns None, list is the /AACS/
constants exactly as before). Verified on real Freedom (AAC!/VTKF090+100) and
Dukes (ANY!/VTKF000) ISOs; unit tests cover both shapes.
Open item (TODO(hddvd-encrypted)): when a disc has multiple VTKF variants the
correct one must be chosen by validating its VUK-derived key against a real
encrypted unit rather than first-that-reads. Blocked on obtaining a genuinely
encrypted HD DVD image — all HD DVD ISOs on hand are already-decrypted rips.
A read was only ever judged "good" by the drive's SCSI GOOD status — with no
integrity check on the bytes. On dirty/marginal media a drive can silently
return best-effort ECC data (occasionally mis-corrected) with GOOD status, so a
rip could "pass clean" yet decode with errors, pushing corruption downstream to
the mux instead of catching it at the read.
Enable recovered-error REPORTING at drive-prep: MODE SELECT the Read-Write
Error Recovery page with PER=1 (TB on / DTE off so the data still comes back),
preserving the drive's own retry count. Marginal reads now surface as
CHECK CONDITION / RECOVERED ERROR. Best-effort — a drive that doesn't honor it
keeps its defaults (no regression), and on a clean disc nothing changes.
A recovered error is distrusted: it marks just that ECC block NonTrimmed for a
Pass N re-read (a clean re-read wins; a persistent marginal becomes an honest
concealed gap), and is counted in the pass summary. Crucially it takes a
per-block SkipBlock, NOT the damage-jump path — a single marginal sector must
not trigger a 64 MB skip that would discard good data on a lightly-smudged disc.
Give `info` the same forced-subtitle verdict the muxer derives during a
rip, so the two agree. A shared classifier (mux::codec::pgs::ForcedTracker)
folds a PGS track's display sets — forced iff every one carries the
forced_on_flag — and is used by BOTH the MKV writer and a new scan-time
probe that reads the title's PGS streams (reusing the TS demuxer and PGS
parser). The probe only overrides a track it actually observed content
for, so an undecrypted/unread stream keeps its vendor-derived flag. Gated
behind ScanOptions::probe_forced_subtitles (off for the rip path, which
detects forced while muxing without a second read).
An FMTS forensic segment interleaves our device group's variant with a
foreign group's at the aligned-unit level. The mux was decrypting only
our phase but leaving the alternate (foreign) units in the buffer as
ciphertext, trusting the demux to 'drop untouched ciphertext cleanly'.
It doesn't: random 0x47 bytes at the 192-byte stride hit tracked PIDs,
mis-parse, and trip the demux's concealed-gap keyframe-resync — which
drops GOOD frames of ours around every segment (349 resyncs / ~6391
packets on Stand by Me, visible as playback flaws).
The map already knows which unit each LBA is and, for a forensic
segment, which phase is ours. AacsKeyMap::read_plan turns that into the
title's read plan: every default/CPS unit, plus inside a segment ONLY
our-phase units. The alternate units are never fetched, decrypted, or
handed to the demux — the demux sees one gapless our-variant stream.
- read_plan is general (single-CPS, multi-CPS, FMTS): a map with no
Even/Odd range returns the extents unchanged, so DVD/CSS, single-CPS
UHD and multi-CPS Blu-ray read byte-for-byte as before.
- Wired into build_iso_pipeline (the file-backed highway that muxes
resumed ISOs). Producer re-anchors unit_base per extent, so per-unit
segment reads stay unit-aligned and decrypt correctly.
- Extent gains PartialEq/Eq for the read_plan tests.
Tests: read_plan non-forensic unchanged; forensic omits exactly the
alternate units, kept units match the decrypt gate unit-for-unit. All
2314 lib tests pass; precommit green on 1.86.
Pending: end-to-end ISO re-mux validation (concealed gaps 349 -> ~0).
Verified zero callers across all consumer crates AND libfreemkv integration tests:
MUX_APP, Disc::{aacs_disc_hash,encrypted_content_ranges,inject_unit_keys},
locate_ranges, mapfile::{MapEntry,entries}, diag::dump_mkv_track,
DiscStream::{errors,lost_bytes} (read via accessors). Removed the unused mux
DemuxSink/FviSink crate-root re-exports (constructed internally by output() via
the direct module path). Staged demux option variants marked allow(dead_code).
Removed the test-only unchecked wrapper and renamed decrypt_keys_for_title_checked
-> decrypt_keys_for_title (one method returning (keys, title_is_clear)). Production
already used the checked form; the _checked suffix had no counterpart. Callers +
docs updated; two tests take .0.
Adds parse_hex_u16/u32/u8 and exposes strip_hex_prefix so callers stop hand-rolling
from_str_radix(trim_start_matches("0x")) — a case-sensitive strip that this module
exists to prevent. disc::aacs_disc_hash now uses strip_hex_prefix.
AacsState (public via Disc.aacs) and Key (the decrypt_with key-transport enum)
are crate-root re-exported and carried VUK/unit/read-data keys + volume id on
#[derive(Debug)]; HandshakeResult carried the VID + AACS 2.0 bus key. Manual
Debug impls print shape only, guarded by red->green tests.
FMTS (AACS 2.1) now decodes per (LBA, phase): Phase enum + AacsKeyMap::
from_ranges_phased, decrypt only the variant's parity half. resolve_fmts_key_map
does a 2-phase index-1 anchor then per-index phase probe, and sizes the forensic
set to whatever the source returns (no hardcoded 32). KeyFetch is now two explicit
operations (unit_keys / fmts_indexes) and KeySource splits get_uk into
get_unit_keys + get_fmts_indexes. BYPASS_FMTS_KEY gate removed (first-class format).
Teed up for 1.4.5. Local WIP baseline.
The keyserver protocol now returns all 32 index keys as an array for a
forensic content sample (and a single-element array for plain content).
resolve_fmts_key_map sends one forensic batch and maps array element i to
segment index i+1, replacing the per-index blind-probe collection loop
that repeatedly hit the key service. Segment/index parsing and the
aligned-unit content classification are reworked to support this:
- rename variant_select -> index_select (per-index, not per-variant)
- content classification moves to is_clean(buf, ContentFormat) so the
unit selector emits only units the key service accepts
- segment.rs: parse IndividualSegment.tbl index tags + SPN ranges,
build contiguous LBA key ranges from the resolved 32-key array
- decrypt/decorator plumbing for the resolved per-index keys
Fail loud (FmtsKeyMissing) when the forensic query returns < 32 keys or
any segment index stays unresolved.
decrypt:
- decrypt_sectors is now a pure decrypt (apply key, leave plaintext, report
unverified bytes); TS-structure is a separate primitive (is_clean_ts/ps) used
only for key selection and read-verify. The mux passes decrypted bytes through
(the demuxer drops non-conforming packets), ending the NULL-TS conceal loop and
the per-unit key-server refetch storm. Key-proof floor replaces the 75%
supermajority.
recovery:
- Removed the post-read decrypt-verify gate (verify.rs) that mis-aligned the
disc-absolute unit grid against clip-anchored AACS units and false-failed good
clips (e.g. Dunkirk's orphan-CPS clip). Bad sectors are marked by physical read
result; decryptability is proven at scan + mux time.
HD DVD (first-class AACS):
- Role-based candidate-list file sourcing so an HD DVD's /ANY!/ files
(MKBROM.AACS, VTKF000.AACS, CONTENT_CERT.AACS) are found with no disc-type
branch. parse_vtkf parses VTKF000.AACS into the same UnitKeyFile as a BD
Unit_Key_RO.inf, so the shared VUK unwrap applies unchanged. set_unit_base
clip-anchoring. Two decrypt-axis assumptions remain UNVERIFIED-HDDVD-DECRYPT
(no encrypted disc to test).
mux:
- MVC (Blu-ray 3D) track signals unified into one MVCDecoderConfigurationRecord;
release-safe track_vint (3-byte VINT) and pid_index (i32) guards.
hardening:
- Container-aware is_clean / encryption detection; bytes_bad_in_title fail-safe
on a corrupt mapfile; CSS crack gated on DiscFormat::Dvd (HD DVD excluded);
non-vacuous CSS tests; patch NOT_READY/HARDWARE/ILLEGAL_REQUEST/ABORTED
sense-path tests.
AACS content decryption rejected a whole 6144-byte aligned unit unless
EVERY content packet was conformant MPEG-TS. One authored-bad packet (a
pressing/encoding defect or an AACS 2.1 forensic-variant frame) made the
mux conceal the entire unit as NULL TS — destroying up to 31/32 good
packets and tallying them as loss, surfacing as false "corruption" on
otherwise-clean discs (observed across two UHD titles).
decrypt_unit now asks only "did a key OPEN this unit?" — a padding-aware
>=75% supermajority of content packets restoring their 0x47 sync, a gate
no wrong key can reach (uniform-AES noise floor) yet one that tolerates a
minority of authored-bad packets. Opened units pass through VERBATIM; a
non-conforming packet is left for the demuxer to drop on sync-loss and
resync past. TS-sync conformance is a muxer concern, never a decryption
verdict. The post-read verify/sweep gate now shares the same primitive so
it can never disagree with the mux decrypt.
Also unify the MVC (Blu-ray 3D) track signals: the mvcC CodecPrivate
extension, the BlockAdditionMapping, and each per-frame BlockAdditional
all derive from one MVCDecoderConfigurationRecord built once per track, so
a malformed dependent-view parameter set can no longer orphan a BlockAddID.
Second audit round converged (severity collapsed 6 HIGH -> 1; the one
HIGH was a bounded 32-element scan, not a defect; the sole spec MEDIUM
was the same false-positive re-raised — 0xBF matches ISO/IEC 14496-15
§7.6.2 verbatim). One genuine robustness fix plus coverage:
- extract_mvc_params: skip a zero-length NAL instead of abandoning the
scan, so a stray length prefix before the subset SPS/PPS no longer
silently drops 3D signalling. Test proves params after a zero-length
NAL are still found.
- Tests: parser_for_mvc_dependent routes H.264 to a passthrough parser;
passthrough with an IDR does not re-assert param sets (the keyframe &&
!mvc branch).
- Document the per-playlist (not per-clip) is_3d latching as a known
limitation (real main-feature playlists are uniformly 3D).
Triage of a 10-lens code audit of the 3D branch. Fixes for real defects;
rejected three spec false-positives that matched the ISO/IEC 14496-15
§7.6.2 record verbatim.
Robustness / correctness:
- Never panic when a title's only video is the MVC dependent view: the
base is now the first NON-dependent video, so a dependent-only title
sets up no merge (muxed as an ordinary track) instead of hitting an
`expect` on the skipped track slot.
- Drop a per-frame BlockAdditional (BlockAddID=2) when the track declared
no mvcC mapping (dependent params not captured before the header) — a
plain block keeps the file conforming instead of an orphaned add.
- A non-keyframe MVC base frame always carries a ReferenceBlock (fall back
to a 0 offset in the pre-first-keyframe corner) so it is never mistaken
for a seek point.
- Reference the last keyframe on the PRIMARY video track only, so a
secondary video track's keyframe can't become a cross-track reference.
- dep_by_pts overflow: bound BEFORE inserting so the just-arrived
dependent survives the drift-clear; count a displaced duplicate-PTS
dependent as an orphan instead of losing it silently.
API / docs:
- Fold write_frame_with_additional into write_frame(..., Option<&[u8]>)
per the "no foo_with_X" convention.
- Fix mvc_params doc (StereoMode is intentionally not emitted); remove a
stale PAT/PMT comment describing an approach that was never taken.
Tests: MVCDecoderConfigurationRecord over-length guards; write_int minimal
two's-complement widths; BlockGroup/BlockAdditions/BlockAdditional +
ReferenceBlock emission; additional dropped without a mapping; h264 MVC
passthrough keeps param sets in-band; extract_mvc_params no-panic on
truncated/empty input; pairing window + dep-overflow edges; no-panic on a
dependent-only title.
Fold the MVC dependent (right-eye) view into the base H.264 track as a
per-frame BlockAdditional under an mvcC BlockAdditionMapping, so a 3D
title produces one MVC video track instead of two independent H.264
tracks.
- h264: MVC-passthrough parser mode keeps the dependent view's subset
SPS/PPS in-band, so each emitted frame is a self-contained dependent
access unit for a BlockAdditional
- resolve: route the dependent stream through the passthrough parser
- mkvstream: detect the dependent view, pair it to the base frame by
PTS (bounded FIFO), attach it as a BlockAdditional (BlockAddID=2),
and skip building its own track; build the mvcC
MVCDecoderConfigurationRecord from the captured subset SPS/PPS and set
it on the base track at activation
- mkv: emit the mvcC BlockAdditionMapping and BlockGroup/BlockAdditions,
with a ReferenceBlock on non-keyframe base frames
- ebml: add BlockAdditions/BlockMore/BlockAdditional/BlockAddID/
BlockAddIDValue/ReferenceBlock elements and a signed-int writer
Verified against a Blu-ray 3D ISO: ffprobe shows a single MVC track,
the mvcC mapping is present, ~144k BlockAdditionals carry the dependent
view (8.7 GB), and the base view decodes cleanly with no regression.
MVCDecoderConfigurationRecord follows ISO/IEC 14496-15 7.6.2; StereoMode
is intentionally omitted (no enum value describes MVC-in-BlockAdditional;
the mvcC mapping is the primary 3D signal per RFC 9559).
Blu-ray 3D as a mux-path variant:
- Detect 3D (a clip has STREAM/SSIF/<clip>.ssif — note the SSIF/ subdir, which
the old .ssif fallback path got wrong) and use the SSIF extents for the mux:
one transport stream carrying both the base (left) and MVC dependent (right)
eyes on distinct PIDs, so muxing it captures the full 3D.
- Add the dependent-view video stream. The base STN table lists only the left
eye; the on-disc PAT/PMT are AACS-encrypted (unreadable pre-key) and the base
STN omits the dependent view (it lives in the MPLS STN_table_SS), so use the
BD-3D PID convention: dependent = base video PID + 1 (0x1011 -> 0x1012),
coding 0x20 -> H.264. The demux routes its packets from the SSIF.
info now reports two video streams for a 3D title; the mux writes both eyes.
from_coding_type maps 0x20 (MVC dependent view) to Codec::H264, so the existing
PAT/PMT scan surfaces the SSIF right-eye substream as a second video stream on
its own PID instead of dropping it as Unknown. No new parser: the dependent eye
rides the same demux path as any other TS video stream. Removes the throwaway
3D-structure probes (findings captured in prior commit messages).
Relicensed from AGPL-3.0 to MIT, effective 1.3.1 (<=1.3.0 remain AGPL). The CSS
content cipher and Stevenson title-key attack are attributed to their published
cryptanalysis (not libdvdcss); all libaacs/libbluray/libdvdread/libdvdnav name
references were dropped from comments while keeping the standard format/spec
descriptions. Also bumps to 1.3.1.
Parse ADV_OBJ/VPLST000.XPL (the real HD-DVD player playlist) with roxmltree
into one DiscTitle per <Title>: its PrimaryAudioVideoClip clips in order (EVO
via the .MAP sidecar), the titleDuration, the displayName, and the ChapterList.
A layer-break split (FEATURE_1+FEATURE_2, feature/feature_Divide) composes into
ONE title with the two parts as clips, each carrying its title-time in/out
points (45kHz ticks) for seamless-join splicing. Falls back to the clip-name
heuristic when no playlist is present.
Fixes unknown runtimes (real durations), poor names (FEATURE -> 'Main Movie'),
and gives authoritative composition + chapters. Validated on ANCHORMAN (97m),
SHAUN (99m), HARRY_POTTER (152m).
Round-11 findings from the 10-phase release audit (no real HIGH):
- When a no-declared-duration source (HD-DVD) muxes a degenerate single
frame at tick 0 with no per-frame duration, max_block_ticks stays 0 and
the reserved DURATION placeholder was left as a literal 0.0 (players read
that as a zero-length file). Void the element instead, so the Segment
omits DURATION as an unknown-duration source did before the back-patch.
- Add a regression test for the sniff_video_codec overlap fix (a
picture_start_code whose payload begins 00 00 followed by a real start
code) so the i+=4 marker skip can't silently regress to i+=3.
Round-10 findings from the 10-phase release audit:
- A finder claimed the DTS SFREQ→rate table was wrong at 11/12; verified
it against ffmpeg's avpriv_dca_sample_rates (12k/24k/48k/96k/192k at
11-15) — the table is CORRECT. Added a test that locks the full table so
it can't be mis-"fixed".
- sniff_video_codec advanced 3 bytes after a matched start code, re-reading
the code byte as an overlapping start code; skip the full 4-byte marker.
- Guard the HD-DVD next_id title counter with saturating_add so a crafted
disc with >65536 clips can't overflow (panic in debug).
- Add a test that an undecryptable unit (DecryptFailed) is zero-filled and
counted as loss through ExtractResult (complete=false, bytes_lost>0) —
the recovery-seam consolidation folded that bucket into bytes_unreadable.
Round-9 findings from the 10-phase release audit (no HIGH):
- Remove the MAX_PTS_MARKS backstop and its tautological test: an empty
DTS PES returns before recording a mark, and a non-empty run is already
bounded by the MAX_AU_BYTES buffer clear (which clears pts_marks) — so
the deque cannot grow unbounded and the cap was dead code.
- AuAssembler::for_codec no longer reserves 256 KiB for a Passthrough
stream (audio/subtitle, and every TS/BD stream) whose buf is never
written; only the reassembling modes reserve.
- Correct the scan comment that claimed region is computed (it is a
Region-free stub until region detection lands) and drop a public-repo
reference to internal "private refactor notes" in the mkb module doc.
Matthew Jackson