Three Silence-of-the-Lambs (R2 PAL SD-DVD) follow-ups for rc.5.2.
SUB-TASK 1 — Windows Explorer showed 12.5 fps (half) for the 576i25 track.
Root cause: the DefaultDecodedFieldDuration (20 ms field) element rc.5.1
added to "fix" Windows fps did the opposite. With FlagInterlaced=1 +
DefaultDuration=40 ms + DefaultDecodedFieldDuration=20 ms, Explorer halved
to 12.5 fps and MediaInfo flipped to VFR. MakeMKV's correct rip omits the
field-duration element, keeps FlagInterlaced=1 + FieldOrder=TFF +
full-frame DefaultDuration (40 ms), and Explorer shows 25 fps / MediaInfo
CFR. Fix: MkvTrack::video now passes field_duration_ns == 0 so the element
is no longer written; the 1/DefaultDuration = 25 fps signal (the only one
tools trust) is the full-frame value. Interlace signalling (FlagInterlaced,
FieldOrder=TFF) is retained — MediaInfo reads scan type from the MPEG-2 ES
picture coding extension, so it still reports Interlaced / Top Field First.
Tests pin the new TrackEntry elements (element present/absent + values).
SUB-TASK 2 — opening "menu"/still-frame video. Traced the MPEG-2
opening-GOP path; the wrong/last seq header and PTS-floor-to-0 hypotheses
are RULED OUT with file:line evidence: codecPrivate is the FIRST sequence
header (read once at headers-ready, mkvstream.rs:115 + pipelined_stream.rs:289),
DVD VOBU structure guarantees each title opens on seq header + I-frame (no
mid-GOP open), the parser back-anchors leading still-frames to the disc's
real timeline (mpeg2.rs:296-303), and the muxer anchors base on the opening
keyframe's real PTS so the t=0 floor (mkv.rs:963) never corrupts it.
Regression tests pin all three (parser + muxer level).
SUB-TASK 3 — make --log-level 3 self-sufficient (diag.rs + minimal hooks).
(a) dump the ACTUAL MKV TrackEntry elements written per track
(tag=mkv.track: FlagInterlaced, FieldOrder, DefaultDuration, field duration,
Display dims, codecPrivate hex) so Windows-fps-class metadata is verifiable
from a log alone. (b) capture the first ~100 coded frames per track (raw)
to <output>.opening.bin with a per-frame summary line (tag=mkv.opening.frame:
track, key/delta, size, PTS) so opening-GOP/menu issues are diagnosable from
a future log without the disc. Both gated to log-level 3; normal runs open
no side file and record nothing.
CI gate (Rust 1.86): fmt --check, clippy -D warnings, and test --tests all
green.
The DVD AC-3 sub-stream probe recorded the FIRST decodable frame of each
physical 0x8x sub-stream as its channel count. A DVD feature opens with
logos/warnings whose audio is often a thin 2.0 bed on 0x80 before the
real 5.1 main mix begins a fraction of a second later. The probe locked
onto that opening 2.0 frame and reported 0x80=2, missing the 5.1
entirely (confirmed on Greenland: 0x80's head frames are acmod=2, then
acmod=7+lfe). With no 6-channel sub-stream found, channel-match routing
fell back to the ordinal map — harmless on Greenland, but on a disc where
the 5.1 lives on a non-ordinal sub-stream the wrong-substream bug stays
unfixed.
Fix: scan EVERY 0x0B77 frame of each sub-stream in the probe window and
keep the MAXIMUM channel count (the sub-stream's real main-mix
capability), advancing frame-by-frame via ac3_frame_size so a frame body
can't be mistaken for a new sync. Also bump PROBE_SECTORS 512->1024: the
1 MiB head window saw ONLY 0x80; 2 MiB reliably contains a frame of every
physical sub-stream.
Greenland tag=dvd.substream: before 0x80=2 (only); after 0x80=6, 0x81=2,
0x82=2 — matching the IFO and the decoded output.
Adds probe_reads_max_channels_no_cross_contamination regression test.
1. HEVC CRA->BLA false-trigger on 33-bit PTS wraparound
(src/mux/codec/hevc.rs): the clip-boundary auto-detect compared the
RAW 33-bit PES PTS against the high-water mark, so a single-clip title
crossing 2^33->0 (~26.5h) false-armed pending_clip_boundary and rewrote
a legitimate in-clip CRA(21)->BLA_W_LP(16), dropping valid RASL pictures
(visible corruption) and breaking the single-clip byte-identical
guarantee. Now unwrap the PTS onto a monotonic 64-bit timeline first
(a near-full-period backstep is a wrap: add 2^33, update the watermark,
do not arm). Regression test cra_after_33bit_pts_wrap_not_rewritten;
the genuine-clip-join test still passes.
2. Single-pass recovery read bypassed the transport-failure abort
(src/mux/disc.rs): the line-442 short-circuit only inspected the 10s
read res. A transport failure (status 0xFF, wedged USB bridge) on the
60s recovery read fell into the skip_errors branch and zero-filled/
advanced, marching the disc at one bridge-recovery per probe
(run-forever, hard rule #2). Re-check the recovery error for
is_scsi_transport_failure() before the skip block and abort with
Error::DiscRead. Test transport_failure_on_recovery_read_aborts_even_with_skip_errors.
3. Recovery-read SUCCESS branch had no coverage (src/mux/disc.rs tests):
added RecoverableReader (errors when recovery=false, succeeds when
recovery=true) and test recovery_read_success_muxes_recovered_data_no_skip
driving fill_extents to the size-1 bottom-out and asserting the recovered
data is muxed (counters advance, no skip).
4. TrueHD channel-correction probe omitted set_unit_base
(src/disc/mod.rs correct_truehd_channels): the probe read via a
DecryptingSectorSource without anchoring the AACS unit-alignment gate,
so it degraded to absolute start_lba % 3 and returned DecryptFailed on a
non-3-aligned extent, silently understating Atmos/7.1 as 5.1. Now call
set_unit_base(ext.start_lba) before the probe read (no-op for CSS/None).
5. is_unit_aligned lba<unit_base latent trap (src/aacs/decrypt.rs):
wrapping_sub mis-gated when lba < unit_base (2^32 == 1 mod 3). Switched
to saturating_sub (clamps offset to 0, a unit boundary) and pinned the
contract with is_unit_aligned_lba_below_base_is_well_defined plus
is_unit_aligned_relative_to_base.
cargo +1.86 fmt --check / clippy -D warnings / test --tests all green.
Fixes the "Silence of the Lambs" R2 PAL wrong-substream rip: the feature's
IFO declares one 5.1 AC-3 stream, but the scan assigned it the on-wire
sub-stream id 0x80 purely by per-codec ordinal (ifo::assign_audio_sub_stream_ids).
On this disc the physical 0x80 carries the 2.0 down-mix and the 5.1 main mix
lives at a different 0x8x sub-stream, so the rip muxed 2.0 while labelling it
"Dolby Digital 5.1" (the acmod fixup in mkv.rs then corrected only the Channels
element, surfacing the mismatch as the "IFO claimed 6 but acmod says 2" warning
— too late to re-route).
New src/disc/dvd_audio_probe.rs probes each physical AC-3 sub-stream's real
channel count from the head of the feature (the acmod/lfeon of its first frame
after the 0x0B77 sync) and re-routes each IFO-declared AC-3 stream onto the
physical sub-stream whose actual channel count matches the declared count,
instead of trusting the ordinal. Wired into both mux demux paths
(DiscStream::new and resolve::build_iso_pipeline) over the decrypting reader,
so it works on CSS discs and the autorip ISO-remux path alike. Bounded
512-sector best-effort read; an empty/unreadable probe degrades to the original
ordinal mapping (no regression on normal discs).
The cell selection is left unchanged: the feature's cell 0 (cat=0x02, 302.4s)
is chapter 1 of the movie (matches MakeMKV's chapter map and 1h53 duration
exactly), so it must NOT be dropped — the perceived "wrong video at the start"
was the wrong 2.0 audio over the opening, the same root cause.
Diagnostics (--log-level 3): new tag=dvd.substream rows dump the ACTUAL acmod
channel count of each physical 0x8x sub-stream read from the VOB, and the
per-cell tag=dvd.cell verdict now spells out the keep/skip reason. With the
existing tag=dvd.aattr (IFO declared sub_id + channels) a bug log alone now
shows whether the ordinal 0x80 really carries the declared layout — no disc
needed to diagnose this class.
expose ac3::find_ac3_sync as pub(crate) for the probe.
The "TopGun bug" (Top Gun 1986 UHD, DV Profile 7 dual-layer): the auditor
flags the rip `corrupt` with a flood of HEVC "Could not find ref with POC N"
decode errors (POC 114/210/228/234/240/246/252/318/336/354/372/399 — one
cluster per clip join).
Root cause: the title is multiple .m2ts clips joined at non-seamless
boundaries (mpls connection_condition 0x05/0x06), read as one concatenated
stream. Each next clip opens with a CRA whose RASL leading pictures reference
pre-join frames gone after concatenation. hevc.rs already has the spec remedy
(rewrite splice CRA_NUT 21 → BLA_W_LP 16 so a decoder sets NoRaslOutput and
drops the dangling RASL), behind mark_clip_boundary() — but nothing ever
called it: connection_condition is not plumbed through the threaded mux
pipeline, so the whole CRA→BLA mechanism was dead code.
Fix: detect the boundary inside the parser from the bitstream. Each clip
carries its own PES PTS base, so a non-seamless join is a large backward
PTS reset. HevcParser::parse now tracks a PTS high-water mark and, on a
backward step beyond 3 s (270000 ticks @ 90 kHz — mirroring the mux-side
DISCONTINUITY_BACKSTEP_NS), arms the existing CRA→BLA rewrite for the new
clip's first IRAP. Self-contained: works for every mux path, no cross-thread
plumbing. The 3 s threshold sits above any HEVC B-frame reorder dip and far
below any clip duration, so it never false-triggers in-clip; single-clip /
seamless titles never arm it and stay byte-identical.
Adds regression test cra_at_auto_detected_pts_backstep_rewritten_to_bla
(in-clip dip must not trigger; splice CRA after a backward reset must become
BLA; one-shot). hevc suite 55/55, mux suite 780/780.
Implements the rc.5.2 quick-units list from the DVD coverage audit and
corrects the "passes-but-encodes-the-bug" tests that could not
distinguish correct from wrong behaviour.
New tests (each with the bug it guards):
mux/mkv.rs
- field_duration_is_direct_trackentry_child_not_in_video: depth-aware
check that DefaultDecodedFieldDuration (and DefaultDuration) are direct
TrackEntry children, NOT nested in the Video master. Replaces the flat
find_id byte-scan that passed either way. Adds master_children /
first_track_entry depth-walking helpers.
- pal_576i_emits_bt470bg_colour_codes / ntsc_480i_emits_smpte170m_colour_codes:
assert the actual CICP tuples written into the MKV Colour master —
PAL (5,5,5,1) vs NTSC (6,6,6,1) — not just stream-layer ColorSpace.
- ntsc_480i_field_order_is_tff_and_encoded: pins NTSC 480i hardcoded TFF
and its ~33.37ms/16.68ms frame/field durations, asserting the encoded
FlagInterlaced/FieldOrder bytes (480i was never exercised before).
ifo.rs
- video_attr_absolute_bytes_pin_real_layout: drives parse_video_attr with
HARDCODED real DVD-Video bytes (PAL/NTSC x 4:3/16:9, plus mpeg_version
in bits 7-6) instead of v_atr_byte, so a co-edit of the shift constants
can't re-seed the PAL-as-NTSC bug. Anchors that permitted_df bits (1-0)
are not read as the TV system.
disc/dvd.rs
- scan_dvd_titles_lpcm_routes_to_a0_pid_range: LPCM (coding 4) → sub-id
0xA0 → PID 0xBDA0, disjoint from the AC-3 0xBD8x space, channels kept.
- scan_dvd_titles_multiple_vobsub_tracks_distinct_pids: three VobSub
tracks → distinct 0x20+ordinal PIDs, per-language, shared palette.
css/mod.rs
- crack_outcome_reaches_cracked_with_span: drives the full crack scan to
CrackOutcome::Cracked via a Stevenson-crackable synthetic sector and
asserts crack_span recording (the Cracked branch was never exercised).
- recrack_succeeds_on_other_vts_extents: per-VTS re-crack SUCCESS path.
- all_locked_synthetic_iso_yields_css_key_missing_signal: all-locked
multi-extent ISO → ScrambledUncracked, the signal the scan converts to
css_error = Some(CssKeyMissing).
Corrected fixtures (passes-but-encodes-the-bug):
- scan_dvd_titles_mixed_audio_codecs_distinct_pids: real channel nibbles
(AC-3 5.1 = 6ch, DTS 2.0 = 2ch) replacing the 1ch placeholders; asserts
channel counts and exact canonical PIDs (0xBD80 / 0xBD88).
- ebml.rs FieldOrder comment: drop the stale "PAL DVD (576i) is
bottom-field-first" line that contradicted the TFF-for-all code.
The AACS unit-alignment gate measured `lba % 3` against absolute disc LBA 0,
but aligned units are anchored at each clip's encrypted-region start. A clip
whose start_lba is not 3-aligned had its readable units wrongly rejected with
"Decryption failed" (the big-title-only failure on some Blu-rays). One
canonical clip-anchored helper (`aacs::is_unit_aligned`) is now the single
source of truth for the decrypt-on-read gate; both mux read paths set the
per-extent `unit_base = start_lba` via a new `SectorSource::set_unit_base`.
Also moves key *mechanism* into the library: the encrypted sample reader
(`read_encrypted_units`) and the candidate-key resolution loop
(`resolve_and_apply`) now live here, so a key source is purely a lookup.
Regression test covers a clip based at a non-3-aligned LBA.
Single-pass disc->MKV has no Pass N, so its read bottom-out now issues one
bounded recovery read (recovery=true, ~60s ECC) before skipping or aborting,
matching the multipass patch. Fixes a transient/marginal sector surfaced as a
read failure direct-to-MKV while multipass recovered it. One read, not a loop
(hard rule #2); recovered data is used so no bogus-status hole reopens.
- CSS: unlock scrambled-sector reads on enforcing drives via bus-auth
only; classify sense 6F/03 as CSS-locked; early-bail on a fully locked
scan; gate the AACS handshake off DVD discs.
- DVD first-play menu no longer prepended to the feature: read the title
VOBS base from vtstt_vobs (0xC4), not the menu VOBS vtsm_vobs (0xC0).
- Interlaced field-duration (DefaultDecodedFieldDuration) written as a
direct TrackEntry child rather than inside Video, so Windows reports
the correct frame rate.
- Audio channel count read from the AC-3 bitstream; FieldOrder set to
TFF; per-track BPS tags.
- Structured disc diagnostics at --log-level 3; reduced per-operation
log spam.
Interlacing is detected upstream (PAL DVD -> R576i) but was dropped in
two places: the video label hardcoded a 'p' suffix, and the muxer never
wrote any scan-type flag, so MediaInfo inferred progressive and reported
576p for a 576i source.
- Add Resolution::is_interlaced() for the R*i variants.
- generate_video_label now branches i/p for the heights that can be
interlaced (1080, 576, 480) instead of always emitting 'p'.
- MkvTrack carries interlaced + field_order; the video serializer emits
FlagInterlaced (0x9A; 1=interlaced, 2=progressive) and, for interlaced
content, FieldOrder (0x9D) - bottom-field-first for PAL 576i,
top-field-first otherwise. Adds the EBML constants.
The VTS_AST_ATR byte 1 stores (channels - 1) in the low 3 bits, but we
were reading the high nibble. A normal 2.0 AC-3 track has 0x1 in the low
nibble and 0x0 in the high nibble, so we computed 0+1=1 and mislabeled
every stereo track as mono in the muxed Channels element.
Read the low 3 bits instead, and correct the two audio_attr tests that
had encoded (channels - 1) in the high nibble to match the buggy reader.
Drop rx and recycle_tx (now Option fields) before joining the producer
thread in BytePrefetcher::Drop. Without this, a non-EOF source fills the
depth-2 forward channel then spins in send_timeout(POLL_INTERVAL) forever
because rx is never drained; join() deadlocks. Adds a regression test
(drop_endless_prefetcher_joins_cleanly) that directly exercises the path.
When a keyframe AU carried an unchanged seq_header (stripped) but a
redefined entry_point (appended), the old append-then-reassert path
produced [entry_point, seq_header] — entry_point before seq_header,
violating SMPTE 421M which requires seq+entry before every RAP.
Replace the single shared prefix Vec + reassert_active() with per-type
temporaries (redefined_seq / redefined_ep) collected during the scan,
then assembled in canonical seq-then-entry order at keyframe time.
Non-keyframes still emit only genuine redefinitions, also seq-before-ep.
Removes the now-unused reassert_active() helper. Adds a regression test
covering the seq-unchanged / entry-redefined trigger case.
Comments at lines ~63 and ~102 misidentified 0x01 (first-item/seamless) as the
non-seamless trigger and labelled 0x05/0x06 as seamless — inverted vs the BD-ROM
spec and mpls.rs (which documents 1=seamless, 5/6=non-seamless). Corrected all
affected doc blocks; no logic change.
The comment at line 404 claimed "every non-Finished range" but the
immediately-following ranges_with call lists only NonTrimmed,
NonScraped, and Unreadable — deliberately omitting NonTried.
Update the comment to accurately reflect the actual status list and
explain that NonTried is excluded because it is handled by a preceding
sweep pass, not by patch.
- keydb.rs: separate default_path()/no_home_dir() doc blocks; correct the
false XDG lock-step claim (Linux write path uses $HOME, ignores
XDG_CONFIG_HOME; read-side search also checks XDG_CONFIG_HOME).
- io/pipeline.rs: use Release/Acquire on the abandoned flag so a leaked
consumer reliably skips close() on weak memory models (ARM64/POWER),
not just x86 TSO.
- mux/disc.rs: cache the decrypt-loss Arc at construction; lost_bytes()
no longer clones an Arc per frame on the mux hot path.
- disc/dvd.rs: assert display_aspect mapping for both 16:9 (PAL test) and
4:3 (NTSC test).
- mux/resolve.rs: extract css_error_aborts() helper and unit-test the
scrambled-but-uncracked CSS guard (Fix 6) incl. the --raw exemption.
- aacs/keys.rs: add unit tests for mkb_type_raw/mkb_type/mkb_is_uhd and
MkbType (Category C 2.0 UHD, prerecorded 1.0, no-0x10-record None).
- release.yml: publish job needs [verify, test] so a failing test suite
blocks crates.io publication.
Fix three DVD video-attribute bugs surfaced by a PAL disc detected as
NTSC:
- PAL/NTSC: parse video_format from VTS_V_ATR bits 5-4, not bits 1-0
(the old mask read permitted_df, so PAL 576i/25fps was mis-detected
as NTSC 480i/29.97). Named consts replace the magic bit positions.
- Anamorphic aspect: write MKV DisplayWidth/Height from the disc's
display_aspect (16:9 720x576 -> 1024x576) instead of square pixels,
so 16:9 DVDs no longer render as 4:3.
- Colour: stamp SD colorimetry (PAL=BT.470BG, NTSC=SMPTE-170M) instead
of BT.709 (HD).
Adds VideoStream.display_aspect (threaded through every muxer) plus
TvSystem/DvdAspect/ColorSpace plumbing, with regression tests. Removes
the deprecated Disc mux set_halt bridge (use with_halt).
Add an io::fsync module with a per-OS split (posix/windows) mirroring the
writeback_file convention, replacing two duplicated dir-fsync copies:
- dir(): POSIX directory fsync; a no-op on Windows, where std cannot open
a directory as a File and the failed open logged a spurious warning on
every mapfile write.
- file_durable(): opens the target read+write before sync_all so the flush
succeeds on Windows, where FlushFileBuffers rejects a read-only handle
with ERROR_ACCESS_DENIED.
Point the mapfile writer at the shared dir() helper.
Audited every #[repr(C)] struct and IOCTL/flag constant in scsi/windows.rs
against the authoritative Windows SDK headers (ntddscsi.h, winioctl.h,
devioctl.h, winnt.h, fileapi.h). All correct except the already-reverted
ScsiPassThroughDirect packing. Add the missing regression guards:
- StoragePropertyQuery layout (STORAGE_PROPERTY_QUERY: 0/4/8, size 12).
- IOCTL/flag constants, with IOCTLs asserted against an independent CTL_CODE
re-derivation (not a tautological literal) so a mistyped code is caught.
Validated compiling via cargo xwin check --target x86_64-pc-windows-msvc.
rc.4 added #[repr(C, packed(4))] to ScsiPassThroughDirect on the false premise
that ntddscsi.h wraps SCSI_PASS_THROUGH_DIRECT in #pragma pack(push, 4). It does
NOT — verified against the Windows SDK ntddscsi.h: the struct has no pragma pack
and uses natural alignment. On 64-bit Windows (LLP64) that puts DataBuffer at
offset 24 and the struct at 56 bytes, which bare #[repr(C)] produces and which
DeviceIoControl expects.
packed(4) instead imposed offset 20 / 48 bytes — the layout of the SDK's
SEPARATE 32-bit thunk struct SCSI_PASS_THROUGH_DIRECT32 (VOID* POINTER_32). Using
that on a 64-bit host malformed every IOCTL_SCSI_PASS_THROUGH_DIRECT, so the
INQUIRY in drive enumeration failed and autorip/CLI reported zero drives
('RC4 no longer detects my drive'). rc.3.1 (bare repr(C)) worked for the same
users; this restores that layout.
Replace the tautological packed-layout test (which asserted the same wrong
offsets the struct produced) with one cross-checked against the SDK header:
DataBuffer@24, SenseInfoOffset@32, Cdb@36, size 56. Verified compiling via
cargo xwin check for x86_64-pc-windows-msvc.
Expose the MKB Type field (record 0x10) as a typed MkbType enum with mkb_type()
/ mkb_type_raw() / mkb_is_uhd() helpers, so callers can distinguish AACS 1.0
(Blu-ray) from AACS 2.0/2.1 (UHD) discs without poking raw bytes.
Matthew Jackson