0bc8d7af9c3d1440bf10131b6211bd225905778f
113
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Matthew Jackson
|
0bc8d7af9c |
test: constrain the AACS key-map gap fill, the PSI walk, and MP4 field offsets
Third pass over src/mux/. 40 survivors killed, no production change. resolve.rs — the deleted-statement cluster is now fully constrained. All 14 deletable statements probed; 9 were already caught, 5 survived: c.sort_unstable() in fill_base_key_gaps. Every existing case handed it cuts already in LBA order, but IndividualSegment.tbl is a record list. Verified on HEAD: deleting the sort passes all 54 resolve tests. The mutant lays a base-key fill straight over a forensic segment. last_idx = idx (FMTS gap fill) and last_idx = hit (multi-CPS cache hit). An extent with nothing to sample must inherit its neighbour's CPS unit; the mutants fall back to the first unit's key. Exactly the shape this file's own comments name — wrong key, no error, lost_bytes == 0. Both check_halt()? polls in probe_fmts_index_keys. These cannot be killed by outcome, since a later poll returns Halted too. The tests count reads instead, which is what the don't-hammer-a-struggling- drive rule actually says: after a Stop the drive is asked for zero content sectors. The four unresolved += 1 arms each got a test, and deleting each fails exactly one — one-to-one, so no fixture passes for the wrong reason. A control test pins that the baseline table resolves, so an expect_err cannot succeed for an unrelated reason. ts.rs::scan_streams was never entered. Six killed, two of which return wrong answers that look right: reading the PAT/PMT CRC as a table entry invents a stream on PID 546 out of CRC bytes, and dropping the ES_info_length skip decodes a descriptor as an entry and loses the one after it. Every existing PMT fixture declares ES_info_length = 0; a real BD PMT carries a registration descriptor on essentially every entry. Also ISO/IEC 13818-1 2.4.4.3 program_number == 0 is the network PID, not a program. mp4/read.rs — 13. Height read as the width beside it; channelcount; the 4-byte base-128 descriptor varint (every existing esds fixture uses a single byte); all three optional ES_Descriptor fields, whose loss is silent (an AAC track just loses its CodecPrivate); first-vs-last media edit, which is A/V desync of the difference; and the version-1 mvhd timescale offset, emitted by any writer whose duration exceeds 32 bits. dts.rs — 7, from a real cargo-mutants run over the file rather than guesswork. Including a buffer that IS the syncword, which is the state a sync split across PES packets lands in the moment its last byte arrives. Equivalents proven by application, not argued: the sample_encrypted_units guard pair is mutually redundant by construction (total*p/9 < total for p <= 8), so either alone is equivalent and both together are not; the PMT section_len guard is dead code where its PAT twin panics; three of the seven EXSS_HEADER_MIN_BYTES arithmetic mutants still sum to 10. |
||
|
Matthew Jackson
|
9de88969ca |
test: constrain the DiscStream loss surface and the empty-title guards
Second mutation pass over src/mux/. 26 survivors killed, no production change. Verified on HEAD before landing: each mutation below passes all 1,237 mux tests unmutated-suite. The priority item was the honest-loss-reporting surface. Both DiscStream::errors and DiscStream::lost_bytes could return a constant with nothing failing — a rip that lost sectors would report zero loss to the caller. This project has already shipped one defect of that shape (a total decryption failure reported as an empty title, exit 0). Driven now through two short-read fills so both land on values that are neither 0 nor 1 and differ from each other; no constant and no field swap survives. MkvStream::finish -> Ok(()) also survived. MkvMuxer::finish has the zero-frame MkvInvalid guard and two tests cover it, but the Stream wrapper above it could return Ok unconditionally and bypass the guard entirely — the empty-title defence was one layer thinner than it looked. au_assembly: pinned au_opener_from behaviourally to the normative byte values for all four modes, with negative cases for codes that are explicitly not openers (MPEG-2 slice 0x01..0xAF, user data 0xB2, extension 0xB5, sequence end 0xB7 per 13818-2 Table 6-1; VC-1 0x0A/0x0B/0x0C; H.264 SPS/PPS/IDR-slice). au_assembly and codec/ hold independent copies of these constants; they agree today, and comparing constants would not catch logic drifting apart, so both sides are now pinned to the spec instead of to each other. demux_sink::sanitize: every filename component demux:// writes comes from disc-controlled text, so the path-separator arm is a traversal guard. Deleting it now fails, including an end-to-end case where base = "../evil/Title" must produce exactly one file inside the chosen directory. stts_and_ctts_expand renamed to stts_expands_runs_to_per_sample_deltas_in_order and given runs with distinct deltas AND distinct lengths. Its old name claimed ctts coverage it never had, which is why the composition-time chain went unconstrained for eight rounds; the doc comment now points at the tests that do cover ctts. Correction to the previous pass: codec/truehd.rs flush -> vec![] IS equivalent. Applied it, full mux suite green. TrueHD buffers across PES but parse emits every complete unit immediately, so a residual buffer at EOF is a truncated access unit and is correctly discarded. The vec![Default::default()] variants are genuinely different and are killed. Deliberately not constrained: mkv::set_opening_capture (diagnostics behind a process-global tracing check, flaky under the parallel runner), and the three stdio.rs header paths (StdioStream holds concrete io::Stdin/Stdout and cannot be driven without a production refactor to injectable Read/Write). |
||
|
Matthew Jackson
|
170fd0c064 |
test: constrain MP4 composition timing, MLP substream directory, and codec-private absence
Mutation testing over src/mux/. No production change — 49 survivors killed, all proven red before green. The MP4 composition-time chain was entirely unconstrained: VideoTiming::ctts, build_ctts and parse_ctts could each return a constant and the suite stayed green. Confirmed on HEAD: build_ctts -> vec![] passes all 1,220 mux tests. A demuxed B-frame title presenting in decode order would have shipped. The cause is a test whose name asserts coverage its body does not deliver — stts_and_ctts_expand builds an stts box and never touches ctts, and write_then_read_round_trip asserts sample sizes and keyframe flags but not one PTS. Same shape as the set_speed forwarding finding, different disguise. mlp_num_substreams / mlp_substr_header_size: every TrueHD fixture in the crate uses one substream and no extraword, so both could return a constant and agree with all of them. These position mlp_parity_ok's window over the AU header, so a constant mis-windows the parity check on exactly the multi-substream AUs that carry 7.1 and Atmos. CodecPrivate absent vs empty: mkv.rs writes Some(bytes) verbatim and omits the element on None (RFC 9559 5.1.4.1.24), so a zero-length Some emits a track header asserting the config IS empty. Four parsers could return Some(vec![]) before any frame. Also: mandatory ISO/IEC 14496-12 boxes (tkhd, vmhd, smhd, dinf, mdhd) could each build empty; HEVC num_extra_slice_header_bits (H.265 7.3.2.3) was never non-zero in any fixture, so the slice-type offset skip was unexercised; chapter names from the disc go straight into <ChapterString> and the & escape must run first; a stray 0x47 in a payload must not latch a TS resync. Documented as equivalent rather than killed: CodecParser::flush and the three parser flush bodies that differ from the mutant only by a tracing call, and DropTally::log_summary. |
||
|
Matthew Jackson
|
327087c70e |
Make five tests capable of failing, and stop the presence probe unmounting the disc
The worst of the five was a regression suite that never touched the code it guarded: nine batch-count tests called `safe_batch_count` and `buggy_batch_count`, both defined in the test file itself. The u16 truncation they exist to prevent could be reintroduced in sector/prefetched.rs with every one of them green. They now drive the real producer through the public API, and reinstating the truncation fails five of the nine. Worth recording that the symptom has changed since the original fix: the unit-alignment clamp below floors a zero batch at three sectors, so the bug is now a twenty-fold throughput cliff rather than the stall it once was. The MP4 reserve test's only numeric case was dominated by the floor and the buffer, so BYTES_PER_SAMPLE could be zeroed without failing it. It now has a case where the per-sample term dominates. The zero-count guard in FileSectorSource was likewise unfalsifiable — seek-past-EOF and a zero-length read both succeed — so the test now observes the file cursor. The AACS media-key ambiguity guard had no test at all; the pool scan is extracted so the verifier can be injected, because a genuine two-key collision needs one ciphertext decrypting under two AES-128 keys to plaintexts sharing a 64-bit magic, which is a 2^64 search and not a fixture. macOS implemented the documented cheap, side-effect-free presence probe by building a full exclusive transport — which force-unmounts the disc. Linux and Windows issue one TEST UNIT READY with no unmount; macOS was the outlier. It now walks the IOKit registry for the media object instead. The C shim's registry reads assumed CoreFoundation types the registry does not guarantee, so a driver publishing a CFNumber where a CFString was expected aborted the process from inside public API. Types are checked and a wrong type treated as absent. The unbounded waitpid on the unmount child is now a polled deadline, and the last-resort match gained the NULL check its two siblings already had. The empty-CDB guard existed only on Linux while a shared helper's comment claimed all three backends had it. Moved into the helper, so the comment is now true and macOS and Windows are covered. One finding was REJECTED with evidence rather than fixed. The TrueHD buffer-cap test was indeed bogus, but MAX_TRUEHD_BUF turns out to be unreachable by any input: the parser only retains data when the buffer is shorter than the declared AU, and that declaration is twelve bits, so the worst case is 8189 bytes against a 256 KiB cap. An exhaustive sweep over all 65536 AU headers confirmed it. The fixture now sits at the reachable ceiling and asserts that instead. The cap itself is left in place as defence, unreachable by construction, matching how the AC-3 resync guard was handled earlier in this audit. Two behaviour changes worth naming: Linux's empty-CDB error becomes InvalidCdbLength rather than a transport failure, and an unknown device now reports absent media rather than a not-found error, because the registry cannot tell an empty drive from a missing one. The latter is a conflation of the kind this audit has fixed three times; it is recorded for the next round rather than left silent. |
||
|
Matthew Jackson
|
fdd473d7e9 |
Remove emulation-prevention bytes before reading the H.264 slice header
The bytes after a NAL header are EBSP, not RBSP: ISO/IEC 14496-10 §7.4.1 has the encoder insert 0x03 after any 0x00 0x00, and §7.3.1 removes it before parsing. The measured-picture-type parse read the raw NAL instead, on the stated reasoning that slice_type is too early for an escape to intervene. That holds only up to a point. first_mb_in_slice is ue(v), so a value of 65535 or more needs sixteen leading zero bits and opens the payload with 0x00 0x00, which an encoder must then escape. A UHD frame is ~32,400 macroblocks, so a conforming Blu-ray never reaches it — but 8K does, and the disc is untrusted input. Such a stream decoded slice_type against a byte the encoder had inserted and reported the wrong picture type: a wrong result rather than an error, which is the class this lens exists for. The prefix is un-escaped into a 16-octet buffer rather than the whole NAL: the two ue(v) fields are at most 32 bits each, so nothing longer can be needed, and it keeps a per-frame allocation proportional to the frame off the path. The test pins both directions. It asserts the un-escaped prefix decodes to first_mb_in_slice = 65535 and slice_type = 2, AND that the raw EBSP does NOT — without that second assertion the test would pass whether or not the fix were present, which is the failure mode this audit has now found four times. The bit string was derived independently rather than by hand: my first attempt at the fixture was wrong by one nibble and the test caught it. Also covers the cases that must NOT be unescaped: a 0x03 not preceded by 00 00 is ordinary payload, and 00 00 03 03 keeps its second 0x03 because the escape resets the zero run. |
||
|
Matthew Jackson
|
399c3d2769 |
Reject an over-length CDB on every transport, splice H.264 param sets in place
Two fixes from round 5. The Linux and Windows backends truncated a CDB longer than 16 bytes (`cdb.len().min(16)`) where macOS returned InvalidCdbLength. Under SPC-4 a command's length is fixed by its opcode group code, so a shortened CDB is not a shorter form of the same command — it is a DIFFERENT command, and the drive executes it and answers GOOD with data for a request nobody made. A silently wrong result on the layer everything else sits on. Rather than mirror the guard a third time it now lives in scsi::mod as checked_cdb_len, with all three backends routed through it, so it cannot drift per platform again. That also makes it testable everywhere: each platform module is cfg-gated to its own host, so a guard inlined into linux.rs and windows.rs would have had no test coverage on any single machine. The shared helper is the only place the behaviour can be asserted on every platform's CI. The two existing macOS tests were tautological — they replicated the guard's logic inline instead of calling it, so they would have passed with the guard deleted. They now call the real helper. Separately, the H.264 keyframe parameter-set re-assert grew a few-hundred-byte prefix buffer to the full access-unit size, copied the whole frame into it, and dropped the presized buffer: one extra whole-frame allocation and copy per keyframe. A UHD title is ~200,000 frames of 150-400 KB with a keyframe every second or two, so that is thousands of avoidable multi-hundred-KB copies per title, each large enough to go through mmap. It now splices into the reserved headroom in place. This mirrors the identical fix already made in hevc.rs, which the H.264 path had drifted from. Byte-for-byte equivalence is pinned by a test whose expected literals were captured from the pre-change implementation, and which I confirmed still passes when the old build-and-copy code is restored. The no-reallocation claim is measured rather than argued: a counter over 30 bare keyframes, which reports 30 of 30 against the old path and 0 with the splice. The reallocation test initially passed even with PARAM_REASSERT_HEADROOM set to zero, because a small parameter set fits in the presize's incidental slack — it proved the fixture did not reallocate, not that the headroom prevented it. Its SPS is now large enough that the constant is load-bearing, so zeroing it fails the test. Not verified: no runtime behaviour on Linux or Windows: no drive, no ioctl. Both files were confirmed to compile for their own targets. |
||
|
Matthew Jackson
|
5c6a6d0785 |
Round 5: reject a degenerate fixed lace, bound the pending buffer by bytes
Five fixes. Three are real defects with regression tests; two are bounds that were expressible but not expressed. A fixed-size lace (RFC 9559 §10.3.4) whose body is empty declared n frames and carried none. The divisibility check passed, because 0 % n is 0, and `chunks` yields nothing on an empty slice whatever width it is given — so the clamp that existed to avoid chunks(0) returned zero frames where the Lacing Head said n. The whole lace vanished with no error raised and the caller saw a clean short block. A zero-size frame cannot be a valid frame, so it is now malformed. A disc read failure while fetching a directory entry's ICB became a file size of zero rather than an error. Zero is indistinguishable from a genuinely empty file, so an unreadable ICB on a damaged disc silently changed which titles a caller saw as present — read_directory already fails hard on its entry-budget guard, so propagating is also what the surrounding code does. read_file_size still returns Ok(0) for an ICB whose tag is neither File Entry nor Extended File Entry, which is a real zero and not a failure. The pending-frame buffer was capped at 4096 frames, which does not bound memory: frames are arbitrarily large and a UHD video frame runs to a few hundred KB, so the existing cap permitted over a gigabyte. Now bounded by bytes as well, at 64 MiB. round_up_grain overflowed for inputs within one grain of u64::MAX — div_ceil then multiply — and the wrapped product is small, turning the largest possible estimate into a negligible reserve. It saturates, and the reserve is clamped to what a `free` box's 32-bit size field can actually hold, since writing a larger one truncated the size and left mdat beyond a box claiming to be far shorter. No real title comes close; a 90 GB UHD title estimates a few MiB. The AC-3 resync guard now advances the PTS cadence like both of its sibling branches, so the three paths out of that block cannot disagree. This one is defensive and has NO test: reaching it needs input that both parses frames and leaves a megabyte of residue, and the parser's own carry rules drop pre-sync junk and cap a partial frame at 8192 bytes, so no such input was found. Stated here rather than covered by a test that would pass either way. Two findings from this round were rejected on inspection. A reported panic in the .mpls suffix check does not exist: the `.get(..)` on the line above returns None off a char boundary and `filter` never runs its closure, so the byte index is unreachable. A test written for it passed against the unfixed code, which is what surfaced the error. |
||
|
Matthew Jackson
|
0bbceed985 |
Round 4: fix 26 defects across crypto, resource use and codec paths
Twenty-six confirmed findings from the fourth audit round, landed as one cluster because they were found by agents working over disjoint file sets. The one worth calling out is a pair of AACS tests that could not fail. Both asserted CBC behaviour against a hand-rolled expectation that happened to be IV-independent, so replacing AACS_IV with sixteen zero bytes left them passing — they were pinning the code's own arithmetic, not the published constant. Replaced with a literal witness of the published IV plus the NIST SP 800-38A F.2.2 CBC-AES128 vector, and verified the other way round: zeroing AACS_IV now fails three tests. The rest are allocation and correctness work on hot paths: the Annex-B writer in demux_sink allocated and freed a whole-frame Vec per frame, which for a UHD title is ~200,000 allocations over the mmap threshold plus the page faults to first-touch each one; it now reuses a buffer on the writer, and still takes the NAL prefix width from the configuration record rather than assuming four. Six findings whose real fix lives in a consumer crate are recorded for re-filing rather than patched here. |
||
|
Matthew Jackson
|
58bdb42f8e |
Group whole E-AC-3 frame sets, and keep short reads unit-aligned
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. |
||
|
Matthew Jackson
|
5f8dc392c0 |
Sweep the pinned toolchain to Rust 1.97
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. |
||
|
Matthew Jackson
|
a32373ff40 |
Fix fifteen defects across perf, resource, panics and key hygiene
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. |
||
|
Matthew Jackson
|
b4bf0daa82 |
Group E-AC-3 dependent substreams into one access unit
The AC-3 parser's own module doc stated the assumption: "AC3 frames are self-contained and always start with syncword 0x0B77". True for legacy AC-3, false for E-AC-3 above 5.1. Per ETSI TS 102 366 (A/52) Annex E, byte 2 of an E-AC-3 syncframe is strmtyp(2) | substreamid(3) | frmsiz[10:8], and an access unit is one INDEPENDENT substream plus every DEPENDENT substream that follows it until the next independent one. The parser emitted one PES frame per syncframe, so a decoder saw each dependent substream as a standalone frame with no parent — including the AC-3-core + E-AC-3-dependent form Blu-ray uses for Dolby Digital Plus. The extra channels were lost and the timeline ran at 2x. The bit position is cross-checked against code already in the tree: the existing frmsiz parse takes byte2 & 0x07 as its high bits, which is only consistent with strmtyp occupying byte2's top two bits. Legacy AC-3 is excluded by bsid < 11, where byte 2 is crc1 and reading strmtyp there would be nonsense. Reserved strmtyp 3 is treated as INDEPENDENT so an unknown type starts a fresh AU rather than merging into an unrelated one. The AU carries the INDEPENDENT substream's PTS, and only the independent substream advances the clock — dependents cover the same time period and add zero duration. That is what removes the doubled timeline. A trailing AU that can still grow is HELD across the PES boundary, because the boundary is unknowable until the next independent sync; the whole AU is re-scanned next call, so there is no shift and no double-count in the loss tally. Plain AC-3 is never held, which keeps DVD/AC-3 latency and behaviour unchanged. A latent pre-existing bug surfaced while testing this: a new PES's PTS was re-stamping an AU that began in an earlier PES, a constant one-frame shift. Fixed with a PtsAnchor so a PES timestamp applies to the first AU that STARTS in that PES's own bytes, while a genuine PTS jump is still adopted. Nine tests. Verified red against five mutations, each killing a specific set: reverting to the pre-fix behaviour kills 8 while plain_ac3_frames_are_not_grouped_or_delayed SURVIVES as the no-regression guard — reproduced independently here. Stamping the dependent's PTS kills 6; not holding across PES kills 4; holding plain AC-3 too kills 15; neutering the PTS anchor kills exactly the 2 split-across-PES timing tests. Three sibling defects found and deliberately NOT fixed, all in mp4/audio.rs: dec3 hardcodes num_dep_sub = 0 (and a nonzero value changes the box LAYOUT, not just a field, per Annex F/G); parse_eac3 ignores strmtyp/substreamid entirely; and a 7.1 DD+ track is still labelled 5.1 because the channel count comes from the independent substream while the extra channels are described by the dependent's chanmap, which nothing parses. Not verified: no real E-AC-3-with-dependents sample exists here, so all evidence is synthetic frames plus the spec layout. The multi-independent-substream case (num_ind_sub > 1, main + associated audio in one PID) is deliberately treated as one AU per independent substream and is untested. |
||
|
Matthew Jackson
|
ef36b452ad |
Fix three defects in last round's own fixes
Round 3 audited the round-1/2 fix commits rather than trusting them, and found three defects in that new code. This is why the pin moves each round. 1. LICENCE REGRESSION, and it was mine. Reverting the "distinguish a failed key source" commit also restored a verbatim reference-decoder table citation in src/mux/codec/dts.rs, because both changes were in that one commit. The MIT licence cleanup was silently undone at HEAD and nothing caught it. The citation is replaced with ETSI TS 102 114 §5.3.1 again, and — more importantly — the rule now lives in the leak gate instead of in my memory. scan-secrets.sh gains LICENCE_RE, which flags ff_dca*, dcadec, l-smash, libav*, and bare ffmpeg/FFmpeg as REF-IMPL-CITATION. `no ffmpeg` is explicitly allowed via negative lookbehind: stating what this project does NOT depend on carries no risk and is a genuine selling point. Verified by re-introducing the citation (gate fails) and removing it (gate clean). 2. TsMuxer armed params_written even when the avcC/hvcC parser returned None, so a track whose codec_private exists but will not parse was muxed to BD-TS with no VPS/SPS/PPS ever emitted — undecodable video, reported as success, with no log line. Last round's fix corrected WHICH parser is used and left this half untouched. Arming the flag is still right (retrying identical bytes cannot succeed) but it is no longer silent: it now warns with the track, codec and codec_private length. 3. test_aes_cbc_roundtrip defined a LOCAL fn aes_cbc_encrypt that SHADOWED the production primitive, so it round-tripped a copy of the algorithm against itself and never touched crypto::aes_cbc_encrypt — the function this cycle added. Any mutation to the shipped code passed it. The shadow is deleted and the test now calls the real primitive; verified by mutating crypto::aes_cbc_encrypt, which now fails it and previously would not have. |
||
|
Matthew Jackson
|
4ed245868e |
Revert "Distinguish a failed key source from one with no entry"
This reverts commit
|
||
|
Matthew Jackson
|
50f37462db |
Remove reference-decoder citations from a public MIT-licensed repo
This crate is MIT licensed. Comments citing another decoder's internal symbols and reproducing its tables verbatim create licence risk that no engineering benefit justifies, so every such citation is replaced with the primary source: ETSI TS 102 114 §5.3.1. Eleven sites across src/mux/codec/dts.rs and src/mux/mp4/audio.rs. The technical substance is unchanged in every case — the deficit-sample-count semantics, the reserved-field skips, the invalid LFF value, and the 16 legal AMODE codes are all spec facts and are now attributed as such. Four CHANGELOG entries that named a validator are reworded; the "no a reference decoder" dependency claim stays, since stating what this project does NOT depend on carries no risk. I initially argued this was a false positive on the grounds that the project's hygiene rules name internal infrastructure and reverse-engineering material, not open-source citations, and that a channel-count table from a standard is fact rather than expression. That reasoning missed the point: the exposure is MIT distributing text derived from GPL/LGPL sources, and that is the maintainer's risk to weigh, not mine. Reversed in full. |
||
|
Matthew Jackson
|
22a3e3fd01 |
Distinguish a failed key source from one with no entry
resolve_and_apply_traced collapsed `Ok(_) | Err(_)` into a single KeyNode::NoEntry step, so a key source that FAILED — server unreachable, keydb unreadable, malformed entry — was recorded identically to one that simply had no entry for this disc. The front-end renders that trace, so it told the operator their disc is not in the database when the real cause was a fixable infrastructure problem. drive_unit_keys and drive_fmts_indexes were refactored this cycle to preserve exactly this distinction; this path had not been. KeyNode gains a SourceFailed variant and the two arms are split. freemkv's trace renderer matches KeyNode exhaustively with no catch-all, so its arm is added in the same change — otherwise the consumer would not build. Also made ETSI TS 102 114 the primary authority for DTS_AMODE_COUNT's comment rather than a reference decoder internal symbol, and pointed it at this crate's own cross-checked DTS_AMODE_LAYOUT / DTS_AMODE_CH tables. A round-2 finding asked for every a reference decoder and a reference decoder citation in the DTS parser to be stripped as a public-repo hygiene violation. Rejected: the project's rules (scan-secrets.sh, CLAUDE.md) prohibit internal infrastructure references and reverse-engineering material, and a reference-decoder citation is neither. The AMODE channel-count table is a factual table from the standard, not expression copied from an implementation. Citing the spec plus a corroborating implementation is how a decodability gate should be justified. |
||
|
Matthew Jackson
|
94a876664b |
Correct six stale comments and doc claims
All six describe code that does something different from what they say, which is the class of defect that gets a maintainer to write a bug on purpose. docs/clpi.md presented the CLPI stream-PID entry as byte-aligned 2/2/2/4/4-byte fields with a 32-bit fine-entry count. It is one 80-bit packed block — reserved(10) + EP_stream_type(4) + num_EP_coarse(16) + num_EP_fine(18) + EP_map_start_address(32) — and num_EP_fine is 18 bits. Anyone parsing to the doc's offsets would read garbage. Replaced with the real bit layout. docs/udf.md said read_directory()'s recursion cap is 3; MAX_DIR_DEPTH is 8. TROUBLESHOOTING.md called Pass 1 `recovery::copy`. The engine's `sweep` is documented as "Pass 1 of a multipass rip"; `copy` is the dispatch verb that chooses between sweep and patch. This inconsistency was mine, introduced in the 1.6.0 doc rewrite. docs/drive-access.md already said `sweep` and was right — a round-2 finding claimed the opposite on the grounds that `recovery::sweep` appears nowhere else in this crate, which it cannot, being in another crate. io/pipeline.rs cited `disc::patch` as WRITE_THROUGH_DEPTH's caller; that moved to freemkv-engine in 1.6.0 and no `patch` exists here. truehd.rs's doc on mlp_major_sync_crc_ok said the trailer is compared big-endian while the body compares u16::from_le_bytes — and a big-endian compare was the bug the function was fixed for, so the comment described the defect rather than the code. sector/decrypting.rs claimed the decorator owns "the only mutable state (its call-count cap and spent flag)". DecryptingSectorSource has no such fields and no KeyFetch field at all in this revision. |
||
|
Matthew Jackson
|
8421c227cd |
mux: fix DTS core-header false-drops + close TrueHD/mux gate coverage
DTS core decodability gate (core_header_drop_reason) — full ETSI TS 102 114
spec-conformance sweep against a reference decoder the spec core-header rules and
a reference decoder parse_frame_header:
- deficit_samples: only require ==32 for NORMAL frames (FTYPE==1). A
TERMINATION frame (FTYPE==0, the last frame of a stream) legitimately
carries fewer and is fully decodable; the old unconditional check dropped
it on every stream that ends on one — a guaranteed per-track silence gap.
Matches a reference decoder (normal_frame && deficit != DCA_PCMBLOCK_SAMPLES) and
a reference decoder (branches on normal_frame).
- reserved bit (after RATE): both reference decoders SKIP it (a reference decoder
skip_bits1, a reference decoder bits_skip1 "Reserved field") and never reject on it.
Rejecting was a false-drop that silenced any real stream whose encoder
set the bit. Relaxed to read-and-discard; DropReason::ReservedBit removed.
Swept and confirmed spec-correct as-is (no change): npcmblocks multiple-of-8,
frame_size>=96, audio_mode>=16 (a reference decoder-permissive), sample-rate validity
table (matches avpriv_dca_sample_rates incl 96k/192k at 14/15), LFE flag==3
invalid, PCMR bits table (matches a reference decoder sample_res {16,16,20,20,0,24,24,0}).
Bit-read order verified field-by-field against a reference decoder. bit_rate is left
unvalidated (lenient, never-false-drop direction) as before.
Tests: termination frame with small deficit is kept; normal frame with bad
deficit is dropped; reserved-bit-set frame is kept. make_bad_dts_core now
uses an invalid LFE flag (duration-neutral) instead of the relaxed reserved
bit.
TrueHD: add coverage for the EXTENDED major-sync header CRC path (ms[25]&1,
mshdr=28+2+2n) — previously zero-tested, the exact path a shipped endianness
bug once used to silently drop whole 7.1/Atmos tracks. Trailer is an
independently-computed oracle (separate CRC-16/0x2D, anchored to the 0x4FF7
catalogue value, NOT crc16_mlp), stored little-endian; test asserts accept,
body-corruption reject, and big-endian-trailer reject.
mux driver: extract the finish completion mapping into pure mux_run_completed
so the finalize_failed -> completed=false branch (reachable only via real
write-thread wedge timing) is unit-tested; add an out-of-range
MuxInput::Session title_index test asserting a clean Error::MuxTrackRange
(E9011) instead of a panic.
|
||
|
Matthew Jackson
|
3bd2fd23b0 |
Fix audit findings: DTS AMODE bound, key-fetch negative memoization, PGS probe coverage
- 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. |
||
|
Matthew Jackson
|
b9568242df |
libfreemkv: 10-phase release audit fixes (v1.5.2..HEAD)
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). |
||
|
Matthew Jackson
|
1eb6910bdb |
Harden mux + decrypt paths; fail-loud on unresolvable keys
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. |
||
|
Matthew Jackson
|
52fd0f733a |
CSS DVD: resolve the per-title key at read time, drop the scan-time crack
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). |
||
|
Matthew Jackson
|
da19280950 |
mux/codec/truehd: fix MLP major-sync checksum endianness (was dropping the whole TrueHD track)
Regression since the previous release, which added an MLP major-sync checksum
gate to drop genuinely-undecodable audio frames. The checksum itself was computed
with mismatched byte order: `crc16_mlp` is the correct crc_2D table (poly 0x2D,
MSB-first) but returns its two bytes in the OPPOSITE order to libavutil's
`av_crc`, and `mlp_major_sync_crc_ok` then folded in the pre-trailer word
little-endian while comparing the trailer big-endian. The net result never
matched a real major sync, so EVERY major sync was judged corrupt. That armed the
drop-forward on the first AU and, since no major sync ever validated to clear it,
collateral-dropped every following AU forever — the entire TrueHD track was
silently dropped. Its AUs then flushed only at mux end, so the track's blocks
landed physically after all the video: a decoder reading video+TrueHD had to
buffer the whole title to reach the first audio block and spiralled into an
unbounded memory runaway ("decoder ran out of memory"). Every TrueHD title
produced after the gate landed was affected; a title from the release before it
is clean. (The header-size parse — a frequent suspect for extended 7.1/Atmos
headers — is NOT the bug; it already matches ffmpeg's `mlp_get_major_sync_size`
byte-for-byte.)
Fix: compute the checksum exactly as ffmpeg's `ff_mlp_checksum16` —
`crc16_mlp(body).swap_bytes() ^ AV_RL16(word) == AV_RL16(trailer)`. Cross-verified
byte-exact against two real discs (a 7.1/Atmos title and a 5.1 title, independent
32-byte headers both validate). With the checksum correct, major syncs validate
and the drop-forward corruption protection works as intended.
Defence in depth: a major-sync checksum that STILL can't be validated (a genuinely
corrupt or as-yet-unparsed header) no longer arms the drop-forward until we hold a
validated baseline (`num_substreams` from a prior clean major sync) — so a single
bad header can never again silently drop an entire track.
Tests: the `finalize_major_sync` fixture now builds the checksum the corrected way;
a synthetic checksum-failed head major sync is kept, not dropped; the existing
baseline-then-corrupt drop-forward tests still pass. Verified end to end against a
real disc: the TrueHD track demuxes to a full, cleanly-decodable 48 kHz 8-channel
stream, interleaved with the video, instead of 0 bytes.
|
||
|
Matthew Jackson
|
3841ae2250 |
disc: detect forced PGS subtitles from stream content for info
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). |
||
|
Matthew Jackson
|
2ccb5c9d01 |
mux/codec/mpegaudio: keep free-format frames
Free-format MPEG-audio (bitrate_index 0) is a legal, decodable mode — the decoder derives the frame size from the sync spacing. Dropping it was a false positive on a clean stream, so it now passes the gate. |
||
|
Matthew Jackson
|
b2bd5f8b3e |
mux/codec/truehd: fix drop-forward poison + rate/resync validation
Three TrueHD state-machine fixes from an adversarial audit: - A corrupt access unit drops forward to the next major sync, but only the individually-verified corruption now feeds the whole-track poison verdict; the resync run is collateral. A couple of transient errors can no longer poison and discard an otherwise-good multi-hour track. The shared drop tally gains a verified/collateral split for this. - The per-AU PTS rate is refined only from a CRC-validated major sync, so a corrupt major sync whose rate nibble decodes to another rate family can no longer shift the resumed audio — a drop stays a silence gap. - The resync clears only on a CRC-validated major sync, never on a runt too short to hold and validate its header. |
||
|
Matthew Jackson
|
98f3dc513f |
mux: detect forced PGS subtitles from the stream
Flag a PGS subtitle track FlagForced when it displays subtitles and every one carries the HDMV forced_on_flag (a dedicated forced/narrative track), independent of the disc's vendor label metadata. The track header reserves a FlagForced byte up front and it is promoted at finish() from the accumulated display-set state. Only ever promotes — a track already forced from the playlist metadata is never demoted. |
||
|
Matthew Jackson
|
5ecfe7c69a |
mux/codec: drop undecodable audio frames, keep A/V sync
A damaged audio access unit is now dropped rather than muxed as a
decoder-choking glitch. Sync is preserved — a drop becomes a silence
gap, never a shift — and every drop is logged. Detection is per-codec,
each mirroring the format's authoritative integrity check:
DTS core-header validity gates
AC-3/E-AC-3 native frame CRC-16 + bitstream-id range
FLAC whole-frame CRC-16 residue
MP2/MP3 header sanity + free-format reject
AAC-ADTS header sanity (raw AAC passes through untouched)
TrueHD/MLP major-sync CRC-16 + AU parity; corrupt AUs drop forward
to the next major sync, since decode state carries
across access units
LPCM and video are excluded by design (no in-frame integrity data;
inter-frame prediction). A shared DropTally handles counting, logging,
and a whole-track fallback for a mostly-undecodable track.
|
||
|
Matthew Jackson
|
f255361683 |
dts: emit clean core alone when the extension boundary is garbage
DTS-HD MA access units are a lossy core frame followed by trailing extension substreams up to the next core sync. On source-damaged discs (observed on the Bourne UHDs) the bytes where the XLL extension belongs are neither a core sync nor an extension sync -- pure garbage -- which desyncs ffmpeg's XLL decoder and cascades into 'Read past end of XLL band data' / 'DSYNC check failed' across the whole track. next_core_boundary now distinguishes three boundary states via a new ext_clean flag on NextCore::Found: - precise/recognized extension sync -> ext_clean=true (keep full AU) - garbage at the boundary byte -> ext_clean=false (drop the ext) When ext_clean is false we emit the DTS core alone (drop the smallest junk piece, keep the frame and its PTS) and drain past the garbage to the next core. Recognized-but-unsizeable extensions still ride the heuristic scan and are kept intact, so lossless tracks are unaffected -- only genuinely corrupt extension bytes are dropped. Bourne s1.dts: 1606 damaged frames / 691620 (0.232%), 93% isolated single frames, worst run 3 in a row (~32ms lossy blip). |
||
|
Matthew Jackson
|
422f2b6bcf |
3D MVC mux: audit round 2 (converged)
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). |
||
|
Matthew Jackson
|
d4021114cd |
Harden 3D MVC mux: robustness + tests (audit round 1)
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. |
||
|
Matthew Jackson
|
fd6dfbe5b0 |
Mux Blu-ray 3D (MVC) as a single MVC video track
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). |
||
|
Matthew Jackson
|
640502d5a8 |
audit: lock DTS rate table, fix sniff overflow-scan, cover decrypt loss
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. |
||
|
Matthew Jackson
|
270f9d88b3 |
audit: drop dead DTS marks cap, lazy passthrough buf, doc corrections
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
|
6a0e61d415 |
audit: clamp BD format fallback, running GOP byte counter, O(1) DTS marks
Round-8 findings from the 10-phase release audit: - detect_disc_format's BDMV fallback passed detect_format's result through unchanged, so an SD bonus/menu title could tag a BD-tree disc as DVD (mis-sizing the ECC sweep) — violating its own "never below Blu-ray" invariant. Clamp anything but UHD up to Blu-ray. - Track the MPEG-2 GOP byte total incrementally instead of re-summing the whole gop_buf on every pushed picture (was O(pictures²) on any MPEG-2 disc, not just adversarial input). - Back the DTS pts_marks deque with a VecDeque so the over-cap prune is an O(1) pop_front, not an O(n) Vec::remove(0). - Add a test exercising parse_stream_id_extension's PTS/DTS skip branches (the real AU-opening 0xFD video PES path) — previously untested. |
||
|
Matthew Jackson
|
92e3b41468 |
audit: bound DTS marks, align disc-format tree order, doc/test cleanups
Round-7 findings from the 10-phase release audit (no HIGH; convergence): - Cap DtsParser.pts_marks (MAX_PTS_MARKS): a run of zero-length timed PES packets grew no buffer bytes, so the drain_front mark-prune never ran — the deque could accumulate without bound on hostile PS input. - detect_disc_format tested HVDVD_TS before BDMV while the title-scan dispatch tests BDMV first, so a disc with both trees would be classified HD-DVD but enumerated as Blu-ray. Align both to BDMV → HVDVD_TS → VIDEO_TS. - Document why the DTS new-PES re-base can emit a locally-decreasing PTS (the muxer's block_ts applies the strictly-monotonic audio nudge, tested in mkv.rs) — this is by design, not a mux defect. - Fix stale aacs/keys.rs comment references (functions moved to aacs/inf.rs / aacs::resolve/derive in the module split). |
||
|
Matthew Jackson
|
7d852419b5 |
audit: byte caps on GOP buffers, opener-scan resume, honest video codec
Round-6 findings from the 10-phase release audit: - Wire the documented MAX_PENDING_BYTES byte cap into the MPEG-2 GOP buffer (it was dead code) and add an equivalent MAX_GOP_BYTES cap to the sparse-PTS reorder, so a crafted stream of few-but-huge access units cannot over-allocate — both were bounded only by frame count before. - probe_evo_streams defaulted an unsniffable HD-DVD video stream to H.264, which mis-parses a VC-1 (or still-encrypted) clip into a corrupt track. Emit the video stream only when the codec is actually identified — the honest outcome, matching the audio path (a real clear clip always carries its sequence header at the head). - Resume the AU-opener search from a cursor (like the boundary search), so a long unsynced junk run is O(bytes), not O(buffer) per push. - Mark mpeg2's now-dead MAX_AU_BUFFER test-only; restore #[doc(hidden)] on the aacs probe harness module. - Add regression tests: the 0xFD video-routing guard, the FMTS-is-UHD key state, and the GOP byte caps. |
||
|
Matthew Jackson
|
9066433c29 |
audit: guard 0xFD video routing, carry frame duration, add cap tests
Round-5 findings from the 10-phase release audit: - collect_es routed EVERY extended-stream-id (0xFD) PES into the video ES buffer, so a 0xFD HD-audio sub-stream (MLP/TrueHD) could pollute the video sample and — if it preceded the video PES — stamp the video track with the audio PID, losing the video. Only the VC-1 extension (0x55) is now treated as video; routing 0xFD audio to its own track is deferred to the HD-DVD program-chain follow-up. - The sparse-PTS reorder now carries its calibrated per-frame duration onto each frame, so the muxer emits a BlockDuration and the back-patched Segment Duration covers the final frame instead of understating it. - Add regression tests for the MAX_MARKS and MAX_VTI_HITS caps (promote MAX_VTI_HITS to module scope); make the differential-test factory array a named type; drop an identity-op in a reorder test. |
||
|
Matthew Jackson
|
c81a6e05cd |
audit: fix AU mark-field loss, VTI tie determinism, and mark/perf issues
Round-4 findings from the 10-phase release audit (the first fully clean round; it dug into the new #22/#18 refactor code): - AuAssembler closed each AU from only the FRONT mark's fields, so when one PES fragment carried the source and a later fragment of the same AU carried the PTS, the second field was dropped — a regression vs the old separate pts/source mark deques. Now merge the first Some of each field across all in-range marks. - parse_vti_clip_order picked the largest residue bucket with HashMap::into_values().max_by_key(), nondeterministic on a size tie (randomized HashMap iteration) — could select a different clip table run-to-run. Break ties by smallest offset. - Bound the marks/disc_marks deques (MAX_MARKS): the buf-size cap prunes marks only when bytes accumulate, so a run of zero-length timed fragments could grow them without bound on hostile input. - Add push_owned so the PS path moves the PES payload into a passthrough AU with no copy (MPEG-2 video + all audio), removing a per-PES malloc+memcpy the refactor had introduced on the DVD path. - Back-patch the MKV duration from the block END (start + its own duration) so it covers the final frame instead of understating by one. - Add direct tests for the MKB record-framing walker; drop a stale drain_complete_aus doc comment left on process_au. |
||
|
Matthew Jackson
|
a94f78d090 |
audit: cap the sparse-PTS reorder buffer, FMTS key state, zero KCD
Round-1 findings from the 10-phase release audit: - SparsePtsReorder buffered its current GOP with no bound, draining only on a keyframe — an open-GOP or crafted program stream that never signals one could hold the whole title in RAM. Force-complete the GOP at MAX_GOP_FRAMES, matching the MPEG-2 parser's backstop. - inject_unit_keys labelled a 2.1 FMTS disc as AACS 1.0 / bus-encryption off; FMTS is UHD-family, so synthesize the UHD version + bus encryption. - The compiled Key Correction Data was a non-zero 16-byte constant fed into the Media Key derivation. Per the no-compiled-keys rule it is now all-zero; the chain still cannot complete on a real disc (documented), so this is behaviour-neutral — all variant tests pass unchanged. - Fix stale doc references (broken `super::variants` intra-doc links, and `aacs::keys` comments) left by the module rename. |
||
|
Matthew Jackson
|
3633882d6c |
mux: reassemble MPEG-2 access units via the shared AuAssembler
The MPEG-2 parser hand-rolled its own PES reassembly — a byte buffer plus parallel PTS / source / discontinuity mark queues keyed by absolute offset — duplicating what AuAssembler already does for H.264/HEVC/VC-1. Add a Mode::Mpeg2 to AuAssembler (picture 0x00 with preceding sequence 0xB3 / GOP 0xB8 headers — the same headers-precede-picture shape as the VC-1 mode) and have the MPEG-2 parser own one via AuAssembler::mpeg2(). parse() now feeds fragments to the assembler and processes each complete access unit; the buffer, base offset, and three mark queues are gone. The GOP-buffered temporal_reference reorder and PTS origin-locking are unchanged. The parser's external contract is unchanged, so all existing MPEG-2 parser tests pass as-is; new AuAssembler tests cover the MPEG-2 boundary rule directly. |
||
|
Matthew Jackson
|
5fdff5664f |
mux: reconstruct display-order PTS for sparse-PTS program streams
HD-DVD EVO (and DVD VOB) program streams timestamp video at GOP granularity: only one access unit per GOP carries a PES PTS. The H.264 / HEVC / VC-1 parsers collapsed a missing PTS to 0, so on such a source every non-anchor frame landed on the same block timestamp and a decoder reported "non monotonically increasing dts". Add a shared SparsePtsReorder that rebuilds a display-order PTS per frame from the coded picture type (I/P/B) plus the sparse anchor PTS, with a per-frame duration self-calibrated from the spacing between consecutive GOP anchors (no external frame-rate needed). Display order is derived via the classic single-anchor-delay rule (an anchor displays only after the previously-held anchor; a B displays immediately), exact for the non-hierarchical GOP structures HD-DVD H.264/VC-1 use. It mirrors the MPEG-2 parser's GOP-buffered origin-locking. Gated to the program-stream path only: the three parsers enable it via with_ps_reorder(is_dvd_ps), so the BD/UHD transport path (per-frame PTS) is byte-identical and untouched. |
||
|
Matthew Jackson
|
88c03152e2 |
mux/dts: re-base PTS per PES instead of a running clock (fix long-title drift)
The first cut used a global running clock (max(next, own-PES PTS) + advance), which fixed the same-PES collision but DRIFTED: once accumulated frame durations exceeded the PES-timestamp spacing, it never re-based, so a feature-long DVD DTS track ran minutes past its real length (2h44 for a 2h03 film) while AC-3 from the same source stayed exact. Match the AC-3 path: re-base to each PES's own container timestamp, and advance by one frame duration ONLY within a run of AUs sharing one PES. Fixes the DVD multi-frame-per-PES collision without drift; the UHD DTS-HD MA per-PES attribution (da85f56) is preserved (each AU still takes its own core PES's PTS). Adds new_pes_rebases_to_its_own_pts_no_drift; full mux suite green (905). |
||
|
Matthew Jackson
|
f122f08628 |
mux/dts: monotonic per-frame PTS + real frame duration (fixes DVD DTS)
DVD packs several DTS core frames into one PES; the parser stamped every access unit with that single PES PTS and duration_ns=None, so consecutive frames collided on one timestamp — ffmpeg rejected the output as 'non monotonically increasing dts to muxer: X >= X' (deep-decode = corrupt, e.g. The Punisher). The UHD DTS-HD MA path (one AU per PES, distinct PTS) was unaffected, which is why this only surfaced on DVD. Parse the DTS core header for samples ((NBLKS+1)*32) and sample rate (SFREQ, 48kHz fallback) to derive each AU's duration, and stamp a running monotonic PTS: max(next_clock, own-core-PES PTS), then advance by the frame duration. A later PES whose PTS is ahead of the clock still wins (preserves the UHD per-PES attribution from da85f56/c49a180); frames sharing one PES advance frame-by-frame instead of colliding. Tests: the 3 that encoded 'same PES -> same PTS' now assert monotonic advance; new dvd_many_cores_one_pes_are_strictly_monotonic reproduces the Punisher bug; duration/SFREQ-fallback unit tests added. |
||
|
Matthew Jackson
|
9e6af4a729 |
mux: harden audio discontinuity handling (audit follow-up)
Two defensive hardenings from the post-fix audit (vs FFmpeg/GStreamer): 1. Move the `pes.discontinuity` partial-drop ABOVE the empty-data guard in all three audio parsers (ac3/dts/truehd), so a discontinuity signal can never be stranded by an empty post-gap PES. The demuxer only emits non-empty PES today; this is defense-in-depth for any future caller. 2. A PES with no PTS must not reset the timeline to 0. ac3 now carries `flush_pts_ns`, dts continues from the most recent known base; truehd already kept its running cadence on a None PTS. Matches OSS behavior (PTS rebases off the next PES that actually carries a PTS). Adds an ac3 regression test (empty-payload discontinuity PES still drops the stranded partial). Loss accounting was reviewed: TS-demux CC-gaps are NOT counted toward lost_video_secs / abort (that is sector-based via DiscStream::errors / mapfile bytes_unreadable), so a source splice never inflates loss — no gating needed there. |
||
|
Matthew Jackson
|
be08e3938b |
mux: drop truncated partial audio frame on concealed gap
The AC-3, DTS and TrueHD parsers buffer access units across PES boundaries. At a concealed-loss gap the buffered unit is truncated: splicing post-gap bytes onto it manufactures a corrupt frame on top of the real loss (FFmpeg "Failed to decode block code(s)" / "Invalid data found" at the gap) and, for TrueHD, strands the PTS cadence into the non-monotonic audio-DTS band seen on multi-clip titles. The video parsers already handle this via the ResyncGate, but the discontinuity signal was only wired into video — audio parsers ignored pes.discontinuity and spliced across the gap. Now, when pes.discontinuity is set, each audio parser drops the partial (clears buf, and for DTS its PTS marks / pending base) so the post-gap PES re-bases a fresh unit. A lost gap degrades to a clean single-frame drop instead of a corrupt spliced frame. No effect on perfect rips: the branch only runs when concealment inserted a discontinuity marker. Adds a per-parser test feeding a partial frame then a discontinuity PES, asserting the truncated partial is dropped (not spliced) and the post-gap PTS is adopted. |
||
|
Matthew Jackson
|
789b699f95 |
mux: make B1 concealment decode-clean on every gap shape
Closes the three residual holes where a concealed/lost gap could still let a dangling-reference frame reach the muxer (degraded/undecryptable-disc path only; clean rips are byte-identical and untouched). Root cause: the discontinuity signal was reconstructed from the 4-bit continuity counter and applied per-PES, both of which are lossy. Three coordinated changes: 1. CC-INDEPENDENT marker. fill_null_ts_unit now tags its NULL packets with an adaptation-field discontinuity_indicator; the demuxer recognises a 0x1FFF packet carrying it as a concealed gap and forces a discontinuity on every tracked PID (the lost unit's PID is unknowable). This survives a loss that is an exact multiple of 16 packets (CC aliases to in-sequence — hole 3) and a loss at a PID's very start (no prior CC — hole 4); it also drops any open, potentially-truncated partial PES. 2. PUSI ATTRIBUTION. A gap landing on a PES boundary now flags the PES STARTING after it, not the one flushed at the boundary (hole 1) — stamping the pre-gap frame could arm-then-disarm the gate on a keyframe and admit the real post-gap inter frame. 3. PER-FRAME signal. codec::Frame gains `discontinuity`; each parser propagates it onto the first post-gap frame. MPEG-2 buffers whole GOPs asynchronously, so it associates the gap by ES OFFSET (like PTS/source), landing it on the exact post-gap picture mid-GOP (hole 2) — a per-PES flag stamped the previous picture. consume_ts (and the EOF flush drain) gate on frame.discontinuity. Tests: CC-independent marker with in-sequence CC + leading-loss; PUSI attribution flags the post-gap PES; MPEG-2 offset-mark stamps the post-gap picture through GOP reorder, not the previous one. Existing B1 gate + EOF tests still green (2270 lib tests). |
||
|
Matthew Jackson
|
d715a0943a |
mux: B1 drop-to-keyframe resync after a concealed gap
Pairs with A2 (read-path NULL-TS concealment). When the demux assembler sees a TS continuity gap it now stamps `discontinuity` on the next completed PES; the codec-parse stage carries that onto a per-track ResyncGate. After a gap on an inter-coded video track the gate drops forward to the next IRAP/IDR keyframe so no frame with a dangling reference reaches the muxer (an ffmpeg deep scan would otherwise report a missing-reference / non-existing-PPS error). Audio and subtitle tracks have no cross-frame references, so the gate is a no-op there. - ts.rs: PesPacket gains `discontinuity`; PesAssembler tracks a sticky pending_discontinuity flag set on CC gap / discontinuity_indicator and carried to the next completed/flushed PES. - resync.rs (new): ResyncGate — per-track arm-on-gap, drop non-keyframes until the next keyframe disarms and resumes. Logs the resync + drop count once at the keyframe. - pipelined_stream.rs: precompute per-track is_video, apply the gate in consume_ts. Out-of-range track index emits as-is (defensive). Tests: ResyncGate unit tests; ts.rs gap-stamps-discontinuity; end-to-end B1 video-drops-to-keyframe and audio-never-drops through PipelinedPesStream. |
||
|
Matthew Jackson
|
a7bd574c34 |
verify: post-read decrypt-verify gate + libaacs-strict verify + audit fixes
Post-read verify gate (new src/disc/verify.rs): UnitVerifier buffers/aligns the disc-absolute read stream into clip-file 6144-byte units, then makes one decryptability() decision per unit (CPI gate -> held keys -> key_fetch -> strict TS). POST_READ_VERIFY const kill-switch; fail-safe contract (only ever downgrades units it is confident are undecryptable; every doubt skips). Hooked into Disc::sweep (producer observes ciphertext -> WorkItem::MarkBad after the Good, FIFO-ordered) and Disc::patch (post-loop reverify_iso reads recovered units whole from the patched ISO). extract::clip_layouts enumerates AACS clips for the gate.
Standards-correct AACS verify: aacs::unit_is_clean_ts is a strict port of libaacs _verify_ts (all 32 TS syncs, not a majority vote); decrypt_unit accepts a key only on it; the majority verify_ts is removed. Deleted the Disc::verify_clips post-pass bolt-on (its primitive is absorbed by the read-path gate).
libaacs/DVD audit fixes: content-cert bus_encryption flag now read from bit 7 (was bit 0 - defeated the bus-key fail-loud gate); cc_id read from offset 14; title_cps_unit range-validated + 1->0 index-converted per libaacs. Corrected attack_crib ("functionally-equivalent" not "exact" port) and read_disc_key (READ DVD STRUCTURE 0xAD, not REPORT KEY) doc comments.
Also includes accumulated uncommitted work: key-fetch seam and TrueHD/DTS audio fix.
|
||
|
Matthew Jackson
|
c49a180ce7 |
mux: fix non-monotonic audio DTS (TrueHD + DTS-HD MA) and stamp builds with git hash
TrueHD: when the PES PTS lags the access-unit cadence, resync to the PTS but never snap the running timestamp backward, so the emitted DTS stays monotonic across the resync (next_pts_ns = max(next_pts_ns, pts)). DTS-HD MA: size each EXSS extension substream exactly from its header (exss_frame_size) and skip it as a unit, so a false 0x7FFE8001 core sync inside the lossless extension payload can no longer split the access unit and truncate the extension. Falls back to a bounded scan when the header is unparseable. Provenance: build.rs bakes the git short hash into GIT_SUFFIX; the muxing/ writing-application field and the FVI generator tag now record the exact build (e.g. "freemkv 1.1.0-beta.1 (g835cc99)"), so any output file is traceable to the revision that produced it. |