b64a96042e588f9103c18c6b1ded4d847579884f
1220
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Matthew Jackson
|
4fcd28b487 |
Parse MKV lacing, route by real TrackNumber, honour NAL length size and edit lists
Four conformance defects in the read paths, two of them silent corruption. **Lacing was ignored entirely.** RFC 9559 §10.2 defines Xiph, EBML and fixed-size lacing, where one Block carries several frames; the reader took the Block payload verbatim, so a laced Block became a single "frame" consisting of a lacing header followed by concatenated frames — garbage to the codec parser, no error. Audio tracks from other muxers commonly use lacing, so an ordinary foreign MKV was silently mangled. All three modes are now parsed: Xiph 255-run sizes including the trailing-zero rule for exact multiples of 255, EBML unsigned first size plus SIGNED VINT deltas with the 2^((7*n)-1)-1 bias of §10.3.3, and fixed-size even division, with the last frame's size deduced from the remainder. Laced timestamps follow §10.3.5: the first frame takes the Block timestamp and the rest are spaced by the track's DefaultDuration, else BlockDuration/count, else shared with a warn. Parsing was chosen over refusing because refusal would leave freemkv unable to remux common foreign audio at all, and each mode is about fifteen lines. A malformed lacing header now raises a NEW code, E_MKV_LACING_INVALID = 9052, deliberately NOT MkvInvalid — because is_skippable_title_stub classifies MkvInvalid as a skippable nav stub, so reusing it would have recreated the exact conflation that is still open as a separate finding. A test asserts the new code is not skippable. **TrackNumber was assumed to be 1..N in TrackEntry order.** RFC 9559 §5.1.4.1.1 only requires it to be non-zero and unique, so sparse or unordered numbers are legal. Block routing and codec_private both computed track + 1. A real TrackNumber map is now built, recorded only for TrackEntries that yield a stream so dropped track types no longer shift the mapping. Verified red here independently, and the failure mode is worse than mis-routing: with track + 1 restored, a buttons track's payload was attributed to the AUDIO stream — wrong payload into the wrong codec parser. **The NAL length prefix was hardcoded to 4 bytes.** lengthSizeMinusOne lives in avcC byte 4 and hvcC byte 21 (ISO/IEC 14496-15 §5.3.3.1.2, §8.3.3.1.2) and was never read, so a source declaring 1- or 2-byte prefixes had its raw prefixed bytes emitted verbatim with no start codes. All four conversion sites now derive the width from the track's own configuration record. **Edit lists were ignored.** No edts/elst was parsed, so the presentation timeline an edit list defines (ISO/IEC 14496-12 §8.6.5/§8.6.6) was dropped — which is how encoder delay is normally expressed. Leading empty edits and the first media edit's media_time are now applied to both dts and pts, with the movie vs media timescale distinction respected. A list needing more than a constant shift applies the leading edit and warns rather than presenting the result as faithful. 17 tests. I reproduced the lacing mutant independently: returning the body whole kills five of them, including the exact-payload and malformed-header cases. Still open and deliberately untouched: the MkvInvalid / is_skippable_title_stub conflation across ~20 reader raise sites. It is a cross-cutting error.rs change and E_MKV_LACING_INVALID is the template for it. |
||
|
Matthew Jackson
|
9527bc1e13 |
Anchor forensic segments to the forensic clip, and stop CPS branching on resolve order
Two correctness defects in AACS 2.1 / FMTS key-map resolution, both order- or anchor-dependent, and both able to abort a whole disc or silently garble it. **The single-CPS short-circuit depended on which title resolved first.** `pool_len` counted the WHOLE unit-key pool, and resolve_fmts_key_map appends the disc's forensic index keys to that same caller-owned pool. The count was captured before THIS call's FMTS branch but not before earlier titles', so once any forensic title resolved, every later title saw a pool larger than one and fell into multi-CPS sampling — 8 random reads per extent, and a whole-disc DecryptFailed if no pooled key opened a menu extent's samples. A disc that ripped fine when a non-forensic playlist sorted first failed when a forensic one did. Forensic keys are now tagged FMTS_POOL_TAG_BASE = 1 << 24 and the short-circuit asks single_base_key_slot(), which excludes them. The old 1000 tag was NOT kept, and the reasoning is worth recording: base CPS ids are Unit_Key_RO.inf position + 1 and that count is a BE16, so 1000 sits inside a genuinely reachable id space. 1<<24 cannot collide. The id field is cosmetic — decrypt.rs indexes the pool by slot and reads only the key — so widening the tag is safe. **Forensic segment SPNs were anchored to the wrong clip.** They live in the forensic feature clip's byte space, but were mapped through clip_byte_to_lba(&title.extents, ..), which treats byte 0 as the start of the title's FIRST extent. Any playlist not beginning with the forensic clip mapped every segment to the wrong LBA: either the anchor probe sampled the wrong clip and the whole-disc resolve aborted with FmtsKeyMissing, or — worse — a forensic index key was applied to non-forensic sectors while the real forensic units kept the base key, giving silently garbled output with no error at all. The correct anchor turns out to be a DISC fact, not title data: an AACS 2.1 disc names its forensic feature BDMV/STREAM/<clip>.fmts, and carries one IndividualSegment.tbl, so the SPNs are in that one clip's byte space. A new forensic_clip_extents() finds the unique .fmts in the already-walked UDF tree, and those extents now drive the segment arithmetic, the addressability filter, and the index probe — whose title parameter is gone, since its reads were mis-anchored too. "Does this title carry forensic content" is now "does it read the forensic clip's sectors" rather than "do the segment bytes land somewhere in the concatenation". Where the clip is NOT identifiable — no .fmts, or several, making the SPN space ambiguous — on a disc that does carry a non-empty table, the resolve now fails loud with FmtsKeyMissing rather than guessing an anchor. That is a deliberate behaviour change: a hypothetical disc with two .fmts clips hard-fails where it previously produced a possibly-wrong map. Failing loud beats silently garbled output, and inventing an anchor was not acceptable. This site had been flagged independently three times — by the agent that added the FMTS per-disc memo, by the round-4 correctness lens with a concrete scenario, and by the round-4 conformance pass. Verified red here independently: reverting single_base_key_slot to count the whole pool fails both new tests. The agent's own evidence was probe_reads 48 vs 40 (the 8 extra sampling reads) and an E7013 DecryptFailed whole-disc abort, and for the anchor an E7026 FmtsKeyMissing on a [trailer, forensic] extent list. All six pre-existing FMTS tests pass unchanged through the new anchor, including the exact-cost assertions (40 probe reads, one key-service call per disc, one UDF walk for 60 titles), so the round-3 memoisation wins are intact. |
||
|
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
|
62450e19bd |
Make two public-API panics return errors, and drop two shipped citations
**The session panics are reachable, and my earlier triage of them was wrong.**
`DiscSession::scan` and `resolve_keys` both did
`self.drive.as_mut().expect(..)`. I previously downgraded these to LOW on the
grounds that no shipped consumer calls them after the drive has been staged into
the reader slot. That is the wrong test: `stage_drive_as_reader` is a PUBLIC
method that empties the drive slot, so the public surface permits the sequence,
and a library must not panic from public API regardless of what current callers
happen to do. Both now return Error::DeviceNotReady.
**A shipped doc comment cited a third-party source FILE** as the authority for
the CLPI ProgramInfo layout ("Layout per the BD CLPI spec clpi_parse.c"). Now
cites the Blu-ray Disc Read-Only Format Part 3 CLIPINF specification.
**The CHANGELOG justified a muxer decision by naming a commercial competitor**
("MakeMKV's rip of the same disc omits it", "matching MakeMKV"). Reworded to
stand on its own terms: the element is optional in RFC 9559, nothing requires it
for interlaced SD, and the 40 ms DefaultDuration is the frame rate the source
actually carries.
The leak gate is extended for both new classes — third-party `*_parse.c` /
`*_dec.c` / `*_demux.c` style filenames, and a competitor named as authority.
Narrowing that rule took two attempts, which is worth recording. A bare
`\.(c|cpp|cc)` pattern produced eight false positives: this repo has its own C
shim (`macos_shim.c`) that build.rs and the docs legitimately reference, and the
pattern also matched the Rust field access `p.cc`. It now matches only the
suffixes typical of third-party media-library sources. This is the second
false-positive round on this rule — the first flagged ffmpeg INVOCATIONS in the
test harness — so the lesson is that a hygiene pattern needs testing in both
directions before it lands, exactly like any other code.
|
||
|
Matthew Jackson
|
013881ac06 |
Restore the MP4 conformance fixes I clobbered while landing another agent's work
detect_rate's nearest-match fix, the colr HLG/BT.470 fix and their four tests were silently reverted. Cause: the r3fix-silent worktree was cut BEFORE the conformance commit landed, and I landed its work by copying whole files into the main tree. mp4/mod.rs was in both agents' file sets, so silent's copy — built on the older base — overwrote the conformance changes wholesale. The gate stayed green throughout, because reverting a fix and its tests together is perfectly consistent. Re-applied the conformance commit's diff for that file with a three-way merge; both agents' changes to mp4/mod.rs now coexist (final_report, UndescribableAudio and the max(track_id) id fix are all still present alongside RATE_TOLERANCE_FPS and the colr resolver delegation). Only mp4/mod.rs was affected. mp4/audio.rs and mkv.rs were in no other agent's file set and were intact. Process lesson, recorded because I would otherwise repeat it: NEVER land a parallel agent's work by copying whole files, when its worktree was cut at an older base than HEAD. Apply its DIFF (git apply -3), or rebase its worktree first. Copying files silently discards anything committed to those files in the interim, and no test can catch it because the tests disappear with the code. |
||
|
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
|
3efa6211f3 |
Make six silent mux failures observable
All six confirmed against the code. The governing rule this cluster serves: a
lossy or degraded outcome is never silent, because a corrupt rip the user does
not know about is the worst failure available.
**A 3D MKV re-mux silently lost one eye.** The BlockGroup read path had arms for
BLOCK / BLOCK_DURATION / REFERENCE_BLOCK only, so BLOCK_ADDITIONS fell into the
skip arm — while the writer does emit BlockAdditions > BlockMore > BlockAdditional
for the MVC dependent view. Reconstruction was judged out of scope and the
reasoning is recorded: PesFrame has no side-payload field and the header parser
never reads BlockAdditionMapping, so there is no dependent-view track to route the
AU to. Instead the loss is now LOUD — counted in bytes and events, warned once,
and surfaced through MkvStream's errors()/lost_bytes(), which the driver already
samples into MuxOutcome. One detail in the finding was wrong and is corrected: the
re-mux does NOT still advertise the mvcC mapping, because the header parser
ignores that element, so the output is a plain 2D H.264 track.
**An all-titles rip silently skipped real titles.** The header-buffer-cap
overflow returned Error::MkvInvalid, and is_skippable_title_stub matches exactly
E_MKV_INVALID | E_CSS_KEY_MISSING — verified here — so a 512 MiB-of-frames title
was classified as an empty nav/menu PGC stub and dropped. It now has its own
E9051 / MuxHeaderBufferExceeded { bytes }, outside the skippable set.
**The public pre-mux report contradicted the file.** Mp4Sink::finish() drops an
audio track it cannot describe, which I chose last round over failing an export
whose video is fine — but mp4_fit_report still listed that stream as included, so
the application's plan and the actual output disagreed. Fixed at both levels:
Mp4SkipReason is now non_exhaustive with NoSamples and UndescribableAudio,
Mp4Sink::final_report() describes the FILE rather than the plan, and for the
boxed dyn Stream path a defaulted Stream::undelivered_streams() carries the
information out to MuxOutcome::undelivered_streams with a driver-side warn.
**MP4 track ids could collide.** ids were assigned before the retain that drops
sample-less tracks, while next_id came from the post-retain count, so [1,3]
yielded next_id 3. Now max(track_id) + 1, saturating.
**Stream selection silently skipped its codec_privates prune** when the lists were
not the same length — but codec_privates is consumed POSITIONALLY and trailing
extras are documented as benign, so the length-equality guard was itself the bug.
The prune now runs unconditionally by index.
**The m2ts_mux scaffolding armed params_written on both the absent and the
unparseable codec_private arms** — the same defect already fixed in tsmux.rs.
Split into params_attempted (a latch, since retrying identical bytes cannot help)
and params_emitted, with a warn on each failure arm and an accessor so the
eventual wiring and its test can observe it.
Each fix verified red by mutating back to the prior behaviour: errors() 0 vs 1,
E6008 vs E9051, final_report [0,1] vs [0], next_track_id 3 vs [1,3], and the
selection prune resolving index 1 to the wrong track's record.
API surface deliberately widened: MuxOutcome gains a public field and Mp4Sink
becomes public. Nothing in-repo breaks. Note a behaviour change on the mkv://
input path — a 3D re-mux now reports non-zero loss, so a consumer treating
errors > 0 as disc damage will trip on it. That is intended: the outcome IS
degraded.
|
||
|
Matthew Jackson
|
9ad68dd092 |
Fix four MP4 conformance defects against the standards
**dec3 declared a 0 kbit/s AC-3 substream.** parse_dolby routes bsid < 11 to parse_ac3, which leaves data_rate_kbps = 0 and keeps the AC-3 bsid, yet dolby_sample_entry wrapped that config in ec-3/dec3 for any Codec::Ac3Plus track. ETSI TS 102 366 Annex F.4 assigns ac-3/dac3 to an AC-3 bitstream and F.6 assigns ec-3/dec3 to an Enhanced AC-3 one, so the entry now follows the SYNCFRAME that was actually parsed, not the playlist's codec label. Computing an AC-3 data rate and keeping ec-3 was rejected: it fixes one field while bit_stream_identification and num_dep_sub keep misdescribing the stream. The bsid threshold is hoisted into one constant so parser and entry-chooser cannot drift. Adjacent defect fixed in the same box: data_rate is 13 bits from a u16 source, so push's mask WRAPPED anything above 8191 (9000 became 808); it now saturates. **colr tagged HLG as PQ, and PAL as BT.601.** video_colr carried a second, drifted copy of the ColorSpace-to-CICP map: transfer 16 (PQ) for every BT.2020 stream with no HdrFormat override, and 6 (BT.601) for Bt470bg. Per ITU-T H.273 Table 3, HLG is 18 and BT.470-6 System B/G is 5 — and mkv::cicp_for_video already got both right. video_colr now delegates to that shared resolver, so the duplicated table is gone and cannot drift again. It keeps only its own decision about WHETHER to emit the box, since an absent colr and an all-unspecified colr mean the same thing per ISO/IEC 14496-12. **Exact 24.000 / 30.000 / 60.000 fps was declared 23.976 / 29.97 / 59.94.** detect_rate took the FIRST STD_RATES entry within 0.5 fps, and every 1000/1001 entry precedes its integer twin 0.024 fps away — a 0.1% error across the whole track's mdhd and stts. Fixed as nearest-wins rather than by reordering the table: reordering fixes today's table and re-breaks the moment someone appends a rate, while nearest-wins is order-independent. Verified red here independently by reverting to first-match, which fails exactly the two timing tests. **ddts MultiAssetFlag was set from has_extension.** In the DTSSpecificBox (ETSI TS 102 114) that flag signals more than one audio ASSET. A DTS-HD MA/HRA track is one asset whose extension substream carries the XLL/XBR component, so setting it from "an EXSS sync follows the core" sent a parser looking for a second asset descriptor while StreamConstruction simultaneously said there was no extension — the box contradicting itself. This module parses the core header only and never reads the EXSS asset table, so 0 is the only honest declaration. A StreamConstruction index for core+EXSS was deliberately NOT invented: that field is a table lookup that could not be confirmed against the standard, and a wrong index is worse than an under-declaration. DTS_AMODE_LAYOUT's masks were independently re-derived against ETSI TS 102 114 §5.3.1 and all 16 are CORRECT — only three adjacent comments were wrong (the AMODE 2/3/4 annotations were rotated by one, and AMODE 9's said "5.1 with LFE" when 0x0007 is the 5.0 mask and LFE is OR'd in separately). Comments corrected. Every one of the eight new tests decodes the field back OUT of the emitted bytes — data_rate from the dec3 body's leading 13 bits, MultiAssetFlag from bit 48 of the ddts tail, colr from the nclx payload inside a real stsd, and the frame rate from mdhd.timescale plus stts.sample_delta of a fully muxed MP4 — rather than restating arithmetic. This audit has already caught one of my own tests doing the latter. NOT fixed, root-caused and recorded at the site instead: an A_PCM/INT/BIG track ships with no BitDepth, which the Matroska Codec Specifications make a MUST. The width exists on disc (BD LPCM signals it in the ES header byte 3, DVD in the IFO audio attribute byte 1) but neither source reaches MkvTrack::audio, and the fix needs a new AudioStream member plus a deferred setter in files another agent held this round. Guessing 16 was rejected — it would confidently misdecode every 24-bit disc — as was refusing the track, which would regress the 16-bit majority that currently plays by accident. |
||
|
Matthew Jackson
|
13897e14f0 |
Resolve the forensic key map once per disc, not once per playlist
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. |
||
|
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
|
f338552969 |
Pin the toolchain to Rust 1.87
The Windows UI needs winsafe, whose current release requires rustc 1.87. The alternative was pinning winsafe back to an older release, which would bake a stale API surface into a brand-new UI permanently to dodge one minor version. The pin's purpose is to sit BELOW the Mac default so clippy drift is caught locally before CI, not to stay on 1.86 specifically, so 1.87 preserves the discipline exactly. Verified before moving anything, not after: `cargo +1.87 clippy -- -D warnings` and `cargo +1.87 fmt --check` are clean across all eight repos, and the full precommit gate (fmt + clippy + tests) passes on libfreemkv, autorip, bdemu, freemkv-engine and freemkv-keysources. Zero new lints, zero formatting drift. |
||
|
Matthew Jackson
|
38aa895038 |
Memoise multi-CPS key-map sampling per extent, not per title
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.
|
||
|
Matthew Jackson
|
e3676e7cdf |
Build BlockGroups in memory so the hot path never seeks
Every BlockGroup frame back-patched its element size via ebml::end_master, which
does two stream_position() calls and two real seeks. BufWriter does not override
Seek::stream_position, so each position query is seek(Current(0)) = flush_buf +
lseek — and each of those flushed the 4 MiB BufWriter while every position-moving
seek reset WritebackPipeline::last_flush_pos. The buffer never got to do its job.
An MPEG-2 title takes this path for EVERY frame (the parser stamps a per-frame
duration, so I, P and B all become BlockGroups) — roughly 350,000 per feature.
A BlockGroup's size is knowable before writing, so there is no need to back-patch
at all. New seek-free twins start_master_buf / end_master_buf patch a placeholder
by buffer INDEX instead of file offset, and build_block_group assembles the whole
element into a persistent buffer that write_block_group and its MVC sibling take,
fill, write once, and hand back — including on the error path — so the allocation
is made once rather than per frame.
Measured with a counting writer that, like BufWriter, does not override
stream_position, over 200 frames:
seek calls position-moving
plain before 912 451
plain after 112 51
MVC before 2516 1253
MVC after 116 53
Per-frame cost is now zero; the residual is the header, the per-cluster
back-patch and Cues. For 350k BlockGroups that is 1.4M seek calls removed on the
plain path, 4.2M on an MVC title.
end_master is deliberately NOT changed for its other callers. The Cluster master
genuinely streams — frames are appended to an open cluster over time, so its body
cannot be buffered without holding a whole cluster in memory — and the rest
(EBML header, Tracks, Info, Chapters, Cues) run once, not per frame.
Byte-identity is the safety property, and it is structural: end_master always
patches a FIXED-width 8-byte VINT, and the buffered pair writes and patches
exactly that same placeholder, so the encodings cannot differ. Verified by
capture-then-compare over 200 frames (17 keyframes / 183 non-keyframes, multiple
clusters, BlockDuration present and absent, both reference branches) — output
byte-for-byte identical.
Three tests now pin it permanently: buffered vs seeking output byte-for-byte for
empty/tiny/multi-byte bodies, the same for NESTED masters (the MVC path nests
BlockAdditions > BlockMore, where an index-arithmetic slip would surface), and
end_master_buf erroring rather than panicking on a position outside the buffer.
All three verified red against a deliberately divergent placeholder width.
A note on verifying this kind of change: comparing emitted MKV across two
worktrees at different commits shows a spurious 14-byte difference, because
MuxingApp/WritingApp embed the build's git SHA twice. Compare at the same base.
|
||
|
Matthew Jackson
|
807eb053ca |
Only assert a forced-subtitle verdict the read actually supports
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.
|
||
|
Matthew Jackson
|
7322f4dd8a |
Record that the failed-vs-absent key source arm is unreachable
The `Ok(_) | Err(_)` arm in resolve_and_apply_traced conflates "this source had no entry for the disc" with "this source failed", and reports both as KeyNode::NoEntry. An operator whose key server is returning 502s is therefore told their disc is not in the database. The conflation is real but LATENT, and fixing it here would change nothing an operator can see, because no shipped KeySource ever returns Err: KeydbSource::get_unit_keys maps a load/parse failure to Ok(Vec::new()), OnlineSource::get_unit_keys is Ok(self.query(ctx)) where query returns empty on transport error, HTTP status, oversize body and bad JSON alike, and MultiSource discards inner Errs. Only test doubles return Err. FetchOutcome::errored in drive_unit_keys / drive_fmts_indexes is dead for the same reason — the right contract, honoured by no source. autorip already works around the missing signal by re-probing the service over HTTP (probe_online_reachability / key_service_transient_status), and its own comment names the incident: "the online keysource swallows every failure (transport error, 502, timeout)". So the fix belongs at the source boundary in freemkv-keysources, with Disc::aacs_error as the channel the operator actually reads — not in this trace. Documented here so the next reader does not assume the arm works, and does not "fix" a dead path as I nearly did twice. |
||
|
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
|
99c5fd3500 |
Reference keyframes per track, size the DTS reserve, correct two claims
The ReferenceBlock offset was computed for ANY video track, but the keyframe tick it measures against was recorded in a single global slot gated to the PRIMARY video track. On a title with two video tracks — an MVC base plus secondary view, or a multi-angle disc — a secondary track's non-keyframe therefore referenced a keyframe on a different track, or 0 (a self-reference) when the primary had not produced one yet. The tick is now recorded per track, so a non-keyframe can only reference a keyframe on its own track. The faststart moov-hole estimate modelled every audio track as (E-)AC-3 at 1536 samples per frame. 1.6.0 added DTS to the writer's carried set, and a DTS core AU is commonly 512 samples — a third of that — so a DTS track's sample table was under-reserved threefold and the mux fell back to moov-at-end, losing faststart on exactly the files 1.6.0 newly supports. mvc_frame_emits_blockgroup_additional_and_reference asserted only that the non-keyframe's ReferenceBlock was Some(_). Its non-MVC sibling, added in the same commit, pins the exact offset; this one now does too, so a mutant emitting a constant or wrong-signed offset no longer passes. The comment above the mp4 sample budget claimed file_len stops a crafted file inflating allocations "past the file's own size". Each indexed sample costs ~52 bytes, so the real ceiling is ~52x file_len (still capped by MAX_SAMPLE_COUNT). The bound is real; the comment overstated how tight it is. |
||
|
Matthew Jackson
|
d4c913e0d3 |
Validate stream-selection PIDs per class, not across both
StreamSelection::apply validated a listed PID by scanning ALL streams, so a PID named in the wrong class's filter passed validation — an audio filter listing a subtitle PID, say. `keeps` then matched it against the audio streams only, so the requested track was silently absent from the output. That is precisely the outcome this validation documents itself as preventing: "fail loud rather than silently emit an MKV missing a requested track". Each filter is now checked against its own stream class. The `listed_pids` helper existed only for the cross-class scan and is removed rather than left behind as dead code. Test covers both directions plus the sanity case, and asserts a rejected selection leaves the title unpruned. |
||
|
Matthew Jackson
|
0e23a6b291 |
Halve the per-frame allocation and copy on the m2ts NAL video path
The NAL path called length_prefixed_to_annex_b, which allocates a whole-frame Vec of its own, then copied the result into a second whole-frame Vec — two full-frame allocations and two full-frame copies per video frame. The crate already has append_length_prefixed_as_annex_b, which writes the conversion straight into a destination buffer; it is the same code path with the intermediate removed. The destination is also sized once up front instead of starting from Vec::new(), which re-grew from zero capacity inside every conversion. On a UHD HEVC title muxed to m2ts:// — ~200k video frames averaging ~310 KB of ES at 60 Mb/s — that removes roughly 62 GB of allocation and 62 GB of memcpy. Behaviour is unchanged: the existing tsmux conversion tests, including the non-NAL passthrough and Annex-B default pair added last round, all still pass. A first attempt reused a persistent scratch buffer across frames, which does not work: the buffer is handed out as Cow::Owned and so can never be returned. A right-sized single allocation gets most of the win without restructuring the function around the borrow. |
||
|
Matthew Jackson
|
bcf47cc4ca |
Fix ddts numeric truncation and the decrypt pool's poison asymmetry
ddts CoreSize wrapped to zero on a maximum-size core. core_size is FSIZE + 1 and FSIZE is itself 14 bits, so the maximum is 16384 — one past what the 14-bit CoreSize field holds — and push()'s mask turned that into 0, declaring an empty core frame. Clamped to 16383 instead: one byte short beats telling a decoder there is no core. Proven red first (the field read back as 0). ddts avg/max bitrate under-declared every non-integral frame rate. It computed sample_rate / frame_samples first, so a 512-sample core at 48 kHz truncated 93.75 frames/s to 93. Multiplying before dividing, with round-to-nearest, keeps the precision. set_decrypt_threads skipped the pool swap on a poisoned lock while DECRYPT_THREADS had already been updated, so the new thread count was reported as taking effect while the stale pool kept serving. decrypt_pool() deliberately recovers from poisoning for exactly this reason; the setter now does the same. The pool Arc is immutable once stored, so a prior panic cannot have left it half-written. The CoreSize test decodes the value back out of the emitted box rather than restating the clamp — a first draft asserted the clamp arithmetic against itself, which would have passed against the unfixed writer. |
||
|
Matthew Jackson
|
a9dc3d7244 |
Make encrypt_unit report a refused slice, and expand its key once
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. |
||
|
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
|
7030de4ec9 |
Apply stream selection on the live path, and treat a halt as a clean stop
Three defects around MuxOptions in the mux driver. MuxInput::Live never applied MuxOptions.selection, so a caller's audio/subtitle selection was silently ignored on the live-drive path while the field's own documentation said it was applied before the demux pipeline is built. The Iso and Session arms both apply it; Live now does the same, in the same place — before resolve_inline_base_map, which is keyed on extents and so unaffected by pruning the stream list. The header gate returned Error::MkvInvalid whenever headers had not resolved. On the prefetch-highway path a halt landing while the pump is blocked in a read can end the stream as Ok(None) rather than Err(Halted), so the loop breaks with headers unresolved through no fault of the data. Reporting that as MkvInvalid tells the consumer its disc is malformed and skips the stop-preserves-staging path that a clean completed=false triggers. The gate now re-checks halt first. MuxOptions.selection's doc claimed it was applied without naming the one input it is not applied to. That exception lived only in an internal comment at the Url match arm, where a caller reading the public field docs would never see it. It is now on the field, pointing at InputOptions::selection instead. |
||
|
Matthew Jackson
|
c0434e87de |
Fix the DVD MPEG-audio codec mapping and the fabricated AACS docs
parse_audio_attr mapped DVD audio_coding_mode 2 to Codec::Mpeg1 — the MPEG-1
VIDEO variant. Codec::kind() reports Video for it, so a DVD MPEG-audio stream
was classified and handled as video everywhere downstream. Modes 2 and 3 are
both MPEG audio Layer II (3 adds the MPEG-2 multichannel extension), so both
map to Codec::Mp2. A test now walks every coding mode and asserts each result's
kind() is Audio, so no mode can map to a non-audio codec again.
docs/aacs.md documented an entire keydb-resolving API that does not exist:
ScanOptions::with_keydb, Disc::open_title, reader.read_unit(). None of those
symbols appear anywhere in the crate, and ScanOptions has no keydb field — its
own doc comment says "libfreemkv is lookup-free — it resolves no keys". A
reader following that page would conclude the library reads keydb.cfg, which
inverts the actual design: the caller resolves keys out-of-band through a
KeySource and applies them with Disc::decrypt_with.
The section is rewritten against the real API, and the AacsState table's
`key_source` type corrected from KeySource to KeyOrigin.
Worth recording: the first replacement example I wrote was itself wrong. It
used `input("disc://...")`, which resolve.rs explicitly rejects with
Error::DiscUrlNotDirect — live disc must go through Drive::open + Disc::scan +
DiscStream::new. Every symbol and signature in the committed example was
checked against the source rather than assumed.
|
||
|
Matthew Jackson
|
ec5cd31ae1 |
Stop Mp4Sink losing audio frames and writing an empty sample entry
Mp4Sink::write returned Ok(()) without recording the sample whenever an audio track's frame would not parse into a sample entry. Two consequences, both silent: leading audio frames were lost until one frame parsed, and a track whose frames never parsed disappeared from the output entirely — finish()'s retain() removed the sample-less trak and the run reported success. That contradicts this crate's stated policy that a skipped track is never silently dropped. The drop was never necessary. audio_entry is read in exactly one place, build_trak, reached only from build_moov inside finish() — nothing on the write path consumes it. So write() now records every sample and derives the entry opportunistically from whichever frame parses first. That makes build_trak's `audio_entry.unwrap_or_default()` reachable, which would emit an stsd declaring entry_count=1 around an EMPTY sample entry: a structurally invalid mp4 returned as success. finish() therefore drops any audio track it cannot describe, with a tracing::warn! naming the codec and sample count. Dropping rather than erroring is deliberate. It matches finish()'s existing treatment of sample-less tracks, keeps an export whose video is fine from failing outright, and needs no new error code — a new code would mean a new i18n key across 29 locale files in another repo, which is not this change's scope. The track's bytes stay unreferenced in mdat: wasted space in a valid file, which is the cheaper failure. Test pins both halves — moov describes only the video track, and the unparseable audio bytes still reach mdat rather than being discarded at write time. |
||
|
Matthew Jackson
|
a1304f9e78 |
Stop the mp4 demuxer dropping tracks silently or inventing sample offsets
Three defects in the mp4:// read path, all of the same family: a damaged source was remuxed minus a track, or with fabricated data, and the run reported success. Silent drops. Eight paths dropped a whole track on malformed input with no report of any kind, so an mp4:// source missing its audio looked like a clean run. Each now emits a tracing::warn! naming the track and the missing or inconsistent table (tracing English is permitted in this crate; the numeric error codes are unchanged). The non-A/V handler case is debug!, since skipping a timecode or hint track is normal. Fabricated offsets. sample_offsets ended with a `while offsets.len() < sizes.len()` loop that packed unplaced samples after the last known offset. Those samples have no known location, so the invented offsets made the reader pull frame data from arbitrary file bytes — the exact "emit garbage" outcome the stco/stsc presence guards refuse. It now returns the short list and the caller drops the track. Short stts. `durations.get(i).unwrap_or(0)` gave every sample past the end of a short stts a duration of 0, collapsing the whole tail onto one timestamp. That is the same degenerate timing the `durations.is_empty()` guard was written to refuse, so the guard now refuses both cases. Two shared test fixtures were internally inconsistent and only passed because the reader was lenient: stsz declared 3 samples while stsc placed 1, and the hostile-stsz fixture's stsc/stts covered a single sample. Both are now consistent. The hostile fixture keeps its lying stsz count — that lie is what it tests — but its stsc and stts now cover whatever count survives the file_len bound, so it exercises the allocation bound rather than the inconsistency guards. Two new tests pin the new refusals by mutating the consistent fixture: an stsc that places 1 of 3 samples, and an stts that covers 1 of 3. |
||
|
Matthew Jackson
|
a39045adf1 |
Bound the forced-subtitle probe, make it cancellable, and stop re-reading clips
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.
|
||
|
Matthew Jackson
|
ea72e6df5f |
Give every audio and video codec its own registered Matroska CodecID
MkvTrack::audio's catch-all was `_ => CODEC_AC3`, and ebml.rs defined no A_AAC, A_MPEG/L2, A_MPEG/L3, A_FLAC or A_OPUS constant at all. A CodecID names the payload, so any of those codecs was written into the MKV declaring AC-3 while carrying something else — a player either refuses the track or decodes noise. Reachable through two ordinary paths, both verified: ifo.rs:577 maps DVD audio_coding_mode 3 to Codec::Mp2, so a DVD with MPEG audio muxed to mkv:// produced a track declaring A_AC3 over MP2 bytes; and mp4/read.rs:479 maps the `mp4a` sample entry to Codec::Aac for an mp4:// source. codec/mod.rs already has working parsers for MP2, MP3, AAC, FLAC and Opus, so the pipeline carried these codecs end to end and only the container label was wrong. MkvTrack::video had the same shape: `_ => CODEC_MPEG2` announced Codec::Mpeg1 and Codec::Av1 as MPEG-2 video. V_MPEG1 and V_AV1 added. Both catch-alls stay, because MkvTrack::audio/video return Self and have no error channel, but they are now reachable only by a non-audio/non-video or Unknown codec routed there in error. Two tests enumerate every real codec of each kind and cross-check each against Codec::kind(), so a codec added to the enum later cannot silently inherit another codec's ID. Both proven red first: Aac declared A_AC3 before the fix. |
||
|
Matthew Jackson
|
b2c7490b5b |
Parse H.264 parameter sets with the avcC parser, not the hvcC one
TsMuxer::write_frame handed every NAL video track's codec_private to hvcc_to_annex_b. An H.264 track carries an avcC record, whose box layout is different, so the parser returned None and no parameter sets were emitted — and params_written was set unconditionally, so it never retried. H.264 muxed to m2ts:// reached the player with no SPS/PPS and was undecodable, silently: frame_count still advanced and the mux reported success. AVC is the dominant Blu-ray video codec, so this was not an edge case. The correct dispatch already existed and was already used by demux_sink::annexb_param_sets. tsmux simply never got the codec: it knew only PIDs and a NAL-or-not bool, so it could not tell hvcC from avcC. Rather than add a second setter, set_nal_video(track, bool) becomes set_video_codec(track, Codec). One fact decides both the ES framing and the parameter-set parser, so the two can no longer disagree — and that disagreement is precisely this defect. The default stays Codec::Hevc, which is the behaviour the bool's `true` default encoded, so a caller that never calls it is unaffected. Proven red first, end-to-end through M2tsStream::create with a real avcC record: before the fix neither the SPS nor the PPS reached the transport stream. |
||
|
Matthew Jackson
|
3db4106253 |
Stop documenting the recovery API that 1.6.0 deleted
Disc::sweep, Disc::patch, Disc::copy, SweepOptions and PatchOptions have zero occurrences in src/ — recovery moved to freemkv-engine — but they were still documented in 30 places across README.md, TROUBLESHOOTING.md, six files under docs/, seven src/ doc comments and a Cargo.toml comment. README.md is the crate's GitHub front page and carried a full multi-pass code example that cannot compile. Two of the src/ references were intra-doc LINKS to deleted items ([`disc::Disc::copy`], [`disc::Disc::patch`] in scsi/mod.rs). They produced no warning on a normal `cargo doc` only because they sit on pub(crate) items; `--document-private-items` reports both, and they are gone now. The README example is deleted rather than rewritten against the engine's API: libfreemkv documenting a downstream crate's API on its own front page is the drift that produced this, and it cannot even depend on it. The src/ references become plain code spans naming freemkv_engine::recovery::* — deliberately not links, for the same reason. docs/rip-recovery.md was 202 lines about relocated code. It now documents only what this crate owns — Drive::read, SenseFamily, DiscStream's adaptive batch halving — plus the read-path design constraints, which belong with the code that enforces them, and points at freemkv-engine/src/recovery/ for the strategy. api-design.md's module tree is regenerated from the real src/disc/ and src/drive/ layouts instead of hand-patched; it had listed sweep.rs, patch.rs, mapfile.rs and read_error.rs, none of which exist. Three stale facts surfaced while rewriting and are corrected: the read timeouts are 10 s / 60 s, not the documented 1.5 s / 30 s; Drive::reset and SgIoTransport::reset no longer exist at all, so "no SCSI reset from any read path" is now stated as the stronger fact it has become; and verify_title, listed as a progress-emitting operation, was removed entirely. CHANGELOG.md keeps its references — those are the historical record of the releases that shipped the API. |
||
|
Matthew Jackson
|
d34979ac57 |
Assert ReferenceBlock per block instead of scanning for the byte 0xFB
Two tests checked only find_id(&data, ebml::REFERENCE_BLOCK).is_some(). ReferenceBlock's EBML ID is the single byte 0xFB, so that asks whether one exists ANYWHERE in the output — not which blocks carry one. Inside a BlockGroup, keyframe-ness is signalled by the ABSENCE of ReferenceBlock, so "which" is the entire question. Measured rather than assumed. A mutant emitting a ReferenceBlock on every BlockGroup, keyframes included — which reintroduces the exact 1.6.0 defect these tests were added for, every frame reading as a non-keyframe — passed mvc_frame_emits_blockgroup_additional_and_reference untouched. The duration-bearing roundtrip test did catch it, via its MkvStream read-back rather than via the presence check. A mutant corrupting the offset VALUE was invisible to both. all_block_groups() parses every BlockGroup in emission order with its Block flags, relative timestamp, duration and decoded signed ReferenceBlock. first_block_group() now delegates to it, so there is one walker rather than two. Both tests assert per block: the keyframe carries NO ReferenceBlock, each non-keyframe carries one, the offset equals the distance back to the keyframe, and the SimpleBlock-only 0x80 flag is clear throughout. Both tests previously took muxer.writer.into_inner() WITHOUT calling finish(), leaving cluster sizes unwritten — an unparseable file, which is why they could only byte-scan in the first place. They now write through SharedWriter and finish(), so the assertions run against a well-formed Matroska file. Verified: with the fixes in place, the every-block mutant and the off-by-one-offset mutant both fail. |
||
|
Matthew Jackson
|
bf2d15f39c |
Cover the non-NAL video path, including the wiring that selects it
set_nal_video had exactly one production caller and zero test callers. The
branch it gates decides whether a video track's ES goes through
length_prefixed_to_annex_b, and MPEG-2 and VC-1 must not: they are already
start-code ES. Getting it wrong is silent — frame_count still increments,
so the mux reports success while emitting a video-less file.
Five tests, each verified against a real mutant:
* non-NAL ES passes through byte-for-byte, and the default path still
converts. Both use a deliberately length-prefix-SHAPED payload so a
wrongly-applied conversion rewrites the leading four bytes into a start
code — a payload the converter happened to leave alone would let a
mutant pass.
* a non-NAL keyframe arms params_written, so following non-keyframes are
not dropped by the pre-keyframe guard.
* set_nal_video rejects an out-of-range track instead of panicking.
* M2tsStream::create wires a VC-1 track to the non-NAL path.
That last one matters more than it looks. The four TsMuxer-level tests set
the flag themselves, so deleting the set_nal_video loop from
M2tsStream::create left all 2366 tests passing — the exact mutant the
finding named went undetected until a test drove the real wiring. It now
also catches the subtler mutant of widening the matches! arm to include
Vc1.
|
||
|
Matthew Jackson
|
d09ed76e07 |
Promote AACS unit encryption to real API, and assert decrypt byte-exactly
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.
|
||
|
Matthew Jackson
|
f76688a0dc |
Make the ddts speaker mask agree with its own channel count
The ddts box declares both a channel count and a 16-bit speaker mask, and a decoder may trust either. dts_channel_layout ended in a `_ => 0x0007` catch-all describing five speakers (C + L/R + Ls/Rs), so for AMODE 6, 7, and 10 through 15 the box contradicted the count DTS_AMODE_CH declared alongside it — provoking a downmix or an outright decode error. AMODE 6 (L + R + S) is the reachable case: its S is a single centre-surround, not the Ls/Rs pair, so it is three channels and 0x0012, not four and 0x0006. AMODE 13/14/15 do not occur on retail media, which ships a 5.1 AMODE-9 core plus an extension substream. Replace the catch-all with the full 16-entry ETSI TS 102 114 mask table. Every entry is cross-checked against DTS_AMODE_CH by counting the speakers its bits imply: sixteen independent constraints, all satisfied, and that table is itself pinned to the per-AMODE channel counts in ETSI TS 102 114. AMODE is a 6-bit field, so the reserved 16..=63 are reachable from a malformed stream. The old `unwrap_or(6)` invented a channel count for them that no mask could match; parse_dts now refuses those frames rather than guessing a layout it cannot name. One existing fixture placed the ext-sync pattern at f[4..8], which incidentally set f[7]=0x25 → AMODE 20. That test is about where the pattern sits, not about AMODE, so the frame is now spec-legal (AMODE 9, 48 kHz) with the pattern moved into the payload proper. |
||
|
Matthew Jackson
|
840cb9aef0 |
Drop the comment that promised tests this crate no longer has
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. |
||
|
Matthew Jackson
|
f3e80c8499 |
Route conduct reports through GitHub instead of a private address
The enforcement contact was an address on a domain used for internal infrastructure, which does not belong in a public repository. GitHub needs no mailbox to exist and is reachable for anyone who can read this file. |
||
|
Matthew Jackson
|
c812a32f3d |
mux: fix keyframe signalling for BlockGroup frames
Inside a Matroska BlockGroup the SimpleBlock 0x80 keyframe bit is
reserved and is always written as 0; keyframe-ness is carried only by
the presence or absence of a ReferenceBlock child. Both halves of the
round-trip got this wrong:
- the reader skipped past ReferenceBlock and read the reserved bit,
so every BlockGroup frame came back as a non-keyframe;
- write_block_group discarded its `keyframe` argument and never
emitted a ReferenceBlock, on the assumption that only intra frames
(PGS subtitles) reached that path.
The MPEG-2 parser stamps a per-frame duration on I, P and B pictures
alike, so all MPEG-2 video is written as a BlockGroup. That makes the
assumption false and left no video frame looking like a keyframe on
read-back. Downstream, mkv:// -> m2ts:// dropped every video frame (the
TS muxer discards non-key video until the first keyframe) while still
reporting success, and mkv:// -> mkv:// and the stdio round-trip failed
E6008, because the MKV muxer opens a cluster only on a track-0 video
keyframe and so wrote nothing at all. HEVC was unaffected: it carries no
per-frame duration, so it takes the SimpleBlock path where the flag bit
is authoritative.
Verified on a real CSS DVD: 841 keyframes out of 11440 video packets
survive a re-mux, matching the I-picture count in the source bitstream.
Also stop running non-NAL video through the Annex-B converter. MPEG-2
and VC-1 elementary streams are already start-code framed, so
length-prefix conversion corrupts them. TsMuxer takes a per-track
nal_video flag, defaulting to the previous behaviour, which M2tsStream
sets from each video stream's codec.
|
||
|
Matthew Jackson
|
eedd27e352 |
mux: document that the Url arm selects via InputOptions.selection
The Url mux path prunes streams inside input() via InputOptions.selection; MuxOptions.selection only applies on the File/Session arms. A Url-source caller must set InputOptions.selection — noting it so a future caller does not put the selection on MuxOptions and silently keep every track (the bug the GUI hit). |
||
|
Matthew Jackson
|
d8e5b97c86 |
1.6.0: remove recovery strategy (moved to freemkv-engine) + trim dead surface
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.
|
||
|
MattJackson
|
0151e199ef | changelog: 1.6.0 section (layering API, mux fixes, engine relocation, stream selection); date 1.5.2 | ||
|
MattJackson
|
ea99c82e32 |
mux: end-to-end test that stream selection drops excluded-PID frames
A title declaring two audio PIDs, pruned to one via StreamSelection::apply before build_iso_pipeline, must never surface a frame from the excluded PID. Proves the declaration-driven seam end-to-end through the full highway (read -> demux -> codec parse): the demuxer is built from the pruned title.streams, so the excluded PID is untracked and skipped, and both track headers and frames follow the pruned list. Builds on the existing synthetic-TS harness. |
||
|
MattJackson
|
8822c29905 |
disc: add DiscTitle audio_streams/subtitle_streams/video_streams accessors
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. |
||
|
MattJackson
|
0ee8341aea |
mux: StreamSelection primitive + apply sites for per-title stream selection
The demux pipeline is declaration-driven off DiscTitle.streams (build_demux_state,
DiscStream::new, and the MKV writer all key off that list), so 'which streams to
keep' is already a pipeline capability with no public knob. This adds the knob:
- mux/select.rs: StreamSelection { audio, subtitle: PidFilter::All | Only(Vec<u16>) }
+ apply(&mut DiscTitle): keep Video always, keep Audio/Subtitle whose PID the
filter lists, prune the rest (and the parallel codec_privates in lockstep);
error SelectionPidUnknown on a listed PID absent from the title (fail loud, not
a silently-missing track). Pure; 6 unit tests. Re-exported at crate root.
- Error::SelectionPidUnknown (E6014).
- MuxOptions gains (+ derives Default now) applied in mux_stream's
Iso/Session arms before the highway/DiscStream builds demux state (and before
probe_and_remap's DVD AC-3 PID rewrite). InputOptions gains applied
in input()'s iso arm right after the title-index bounds check.
PIDs not languages -- language->PID is caller/engine policy. Default All/All is a
no-op (apply gated on !is_all()), so the no-selection path is byte-identical:
nothing below the title-finalization line changes (ts/ps/demux_thread/
pipelined_stream/mkv/disc untouched). All 2488 lib tests pass on 1.86.
|
||
|
MattJackson
|
d22d09c898 |
error: add is_disc_level_no_key classifier (re-exported)
Distinguishes a WHOLE-DISC key failure (E_NO_DISC_KEY / E_KEYDB_LOAD / E_AACS_NO_KEYS -- every title fails identically) from a per-title skippable stub. The engine's multi-title loop uses it to fail-fast on the first no-key title instead of iterating all N. Additive. |
||
|
MattJackson
|
43564d5752 |
error: re-export is_halt + is_skippable_title_stub at crate root
The engine's multi-title rip loop classifies per-title mux failures (halt vs skippable stub vs hard) using these typed classifiers instead of E-code string matching. Additive; no behavior change. |
||
|
MattJackson
|
508a2c3376 |
disc: promote locate_ranges to pub (engine multipass reads it)
Small pub promotion + fmt one-lining. The relocated multipass progress reporting in freemkv-engine needs locate_ranges externally. No behavior change. |