mux: fix DTS core-header false-drops + close TrueHD/mux gate coverage

DTS core decodability gate (core_header_drop_reason) — full ETSI TS 102 114
spec-conformance sweep against ffmpeg ff_dca_parse_core_frame_header and
dcadec parse_frame_header:

- deficit_samples: only require ==32 for NORMAL frames (FTYPE==1). A
  TERMINATION frame (FTYPE==0, the last frame of a stream) legitimately
  carries fewer and is fully decodable; the old unconditional check dropped
  it on every stream that ends on one — a guaranteed per-track silence gap.
  Matches ffmpeg (normal_frame && deficit != DCA_PCMBLOCK_SAMPLES) and
  dcadec (branches on normal_frame).
- reserved bit (after RATE): both reference decoders SKIP it (ffmpeg
  skip_bits1, dcadec bits_skip1 "Reserved field") and never reject on it.
  Rejecting was a false-drop that silenced any real stream whose encoder
  set the bit. Relaxed to read-and-discard; DropReason::ReservedBit removed.

Swept and confirmed spec-correct as-is (no change): npcmblocks multiple-of-8,
frame_size>=96, audio_mode>=16 (ffmpeg-permissive), sample-rate validity
table (matches avpriv_dca_sample_rates incl 96k/192k at 14/15), LFE flag==3
invalid, PCMR bits table (matches dcadec sample_res {16,16,20,20,0,24,24,0}).
Bit-read order verified field-by-field against dcadec. bit_rate is left
unvalidated (lenient, never-false-drop direction) as before.

Tests: termination frame with small deficit is kept; normal frame with bad
deficit is dropped; reserved-bit-set frame is kept. make_bad_dts_core now
uses an invalid LFE flag (duration-neutral) instead of the relaxed reserved
bit.

TrueHD: add coverage for the EXTENDED major-sync header CRC path (ms[25]&1,
mshdr=28+2+2n) — previously zero-tested, the exact path a shipped endianness
bug once used to silently drop whole 7.1/Atmos tracks. Trailer is an
independently-computed oracle (separate CRC-16/0x2D, anchored to the 0x4FF7
catalogue value, NOT crc16_mlp), stored little-endian; test asserts accept,
body-corruption reject, and big-endian-trailer reject.

mux driver: extract the finish completion mapping into pure mux_run_completed
so the finalize_failed -> completed=false branch (reachable only via real
write-thread wedge timing) is unit-tested; add an out-of-range
MuxInput::Session title_index test asserting a clean Error::MuxTrackRange
(E9011) instead of a panic.
This commit is contained in:
Matthew Jackson
2026-07-24 10:28:21 -07:00
parent eb0aa3556e
commit 967d0ac77e
3 changed files with 295 additions and 29 deletions
+100 -28
View File
@@ -683,9 +683,10 @@ fn dts_core_duration_ns(data: &[u8]) -> u64 {
}
/// DTS core-header validity constants (ETSI TS 102 114).
/// `deficit_samples` must equal this (`DTS_PCMBLOCK_SAMPLES`); `npcmblocks`
/// must be a multiple of `DTS_SUBBAND_SAMPLES`; `audio_mode` must be below
/// `DTS_AMODE_COUNT`; `lfe_present == DTS_LFE_FLAG_INVALID` is rejected.
/// For a NORMAL frame `deficit_samples` must equal this (`DTS_PCMBLOCK_SAMPLES`)
/// — a termination frame may carry fewer; `npcmblocks` must be a multiple of
/// `DTS_SUBBAND_SAMPLES`; `audio_mode` must be below `DTS_AMODE_COUNT`;
/// `lfe_present == DTS_LFE_FLAG_INVALID` is rejected.
const DTS_PCMBLOCK_SAMPLES: u32 = 32;
const DTS_SUBBAND_SAMPLES: u32 = 8;
/// Number of LEGAL `AMODE` (channel-arrangement) codes. The 6-bit AMODE field
@@ -723,7 +724,6 @@ enum DropReason {
FrameSize,
Amode,
SampleRate,
ReservedBit,
LfeFlag,
PcmRes,
TrackPoisoned,
@@ -738,7 +738,6 @@ impl DropReason {
DropReason::FrameSize => "frame-size",
DropReason::Amode => "audio-mode",
DropReason::SampleRate => "sample-rate",
DropReason::ReservedBit => "reserved-bit",
DropReason::LfeFlag => "lfe-flag",
DropReason::PcmRes => "pcm-resolution",
DropReason::TrackPoisoned => "track-poisoned",
@@ -762,9 +761,17 @@ impl DropReason {
fn core_header_drop_reason(au: &[u8]) -> Option<DropReason> {
let mut r = BitReader::new(au.get(SYNCWORD_BYTES..)?);
let _ftype = r.read_bit()?;
// FTYPE: 1 = NORMAL frame, 0 = TERMINATION frame (the last frame of the
// stream). Per ETSI TS 102 114 and both reference decoders — ffmpeg's
// `ff_dca_parse_core_frame_header` (`normal_frame && deficit_samples !=
// DCA_PCMBLOCK_SAMPLES`) and dcadec's `parse_frame_header` (which branches
// on `normal_frame`) — the deficit-sample field must equal 32 ONLY for a
// normal frame. A termination frame legitimately carries fewer samples and
// is fully decodable; dropping it would silence the last frame of every
// stream that ends on one (a guaranteed per-track loss on real discs).
let normal_frame = r.read_bit()? == 1;
let deficit_samples = r.read_bits(5)? + 1;
if deficit_samples != DTS_PCMBLOCK_SAMPLES {
if normal_frame && deficit_samples != DTS_PCMBLOCK_SAMPLES {
return Some(DropReason::DeficitSamples);
}
let crc_present = r.read_bit()? == 1;
@@ -785,9 +792,12 @@ fn core_header_drop_reason(au: &[u8]) -> Option<DropReason> {
return Some(DropReason::SampleRate);
}
let _br_code = r.read_bits(5)?;
if r.read_bit()? != 0 {
return Some(DropReason::ReservedBit);
}
// Reserved bit. Both reference decoders SKIP this field rather than reject
// on it — ffmpeg (`skip_bits1`) and dcadec (`bits_skip1`, comment "Reserved
// field"). A frame that sets it is still fully decodable, so rejecting it
// was a false-drop that silenced any real stream whose encoder set the bit.
// Read past it without gating (never reject a decodable frame).
let _reserved = r.read_bit()?;
// drc, ts, aux, hdcd (1 each) → ext_audio_type (3) → ext_present, aspf (1 each).
r.skip_bits(4)?;
r.skip_bits(3)?;
@@ -831,11 +841,12 @@ mod tests {
let fsize = size - 1;
let mut data = vec![0u8; size];
data[0..4].copy_from_slice(&DTS_CORE_SYNC);
// byte4: FTYPE(0) SHORT(5) CPF(0) NBLKS-high(0). SHORT = 31 makes
// deficit_samples = 32 = DTS_PCMBLOCK_SAMPLES, which the decodability
// gate (per ETSI TS 102 114) requires of a real core frame. NBLKS high
// bit (byte4 bit0) stays 0 for NBLKS = 15.
data[4] = 31u8 << 2;
// byte4: FTYPE(1) SHORT(5) CPF(0) NBLKS-high(0). FTYPE = 1 = a NORMAL
// frame (the common real-stream case); SHORT = 31 makes deficit_samples
// = 32 = DTS_PCMBLOCK_SAMPLES, which the decodability gate (per ETSI TS
// 102 114) requires of a normal frame. NBLKS high bit (byte4 bit0) stays
// 0 for NBLKS = 15. (0x80 | (31 << 2) = 0xFC.)
data[4] = 0x80 | (31u8 << 2);
// NBLKS = 15 → (15+1)*32 = 512 samples/frame (the DVD/UHD DTS-core norm).
// NBLKS is byte4 bit0 + byte5 bits7-2; here byte4 bit0 = 0, byte5 = 15<<2.
data[5] = (15u8 << 2) | ((fsize >> 12) & 0x03) as u8;
@@ -1791,22 +1802,24 @@ mod tests {
);
}
/// A structurally-framed but UNDECODABLE core: a valid `make_dts_core`
/// whose reserved header bit is set. It still sizes and syncs correctly (so
/// the framer delimits it normally), but the core-frame header validity
/// check rejects it as a set reserved bit (ETSI TS 102 114). The reserved
/// bit is byte9 bit4 in the core header (after SYNC..RATE).
/// A structurally-framed but UNDECODABLE core: a valid `make_dts_core` whose
/// LFE flag is set to the reserved value 3 (`DTS_LFE_FLAG_INVALID`). It still
/// sizes and syncs correctly (so the framer delimits it normally), but the
/// core-frame header validity check rejects it as an invalid LFE flag (ETSI
/// TS 102 114; dcadec `LFE_FLAG_INVALID`). LFE is byte10 bits2-1, and does
/// NOT feed the frame duration (NBLKS + SFREQ only), so a dropped bad core
/// still carries the same `DTS_CORE_DUR_NS` as its good peers.
fn make_bad_dts_core(size: usize) -> Vec<u8> {
let mut d = make_dts_core(size);
assert!(
core_header_drop_reason(&d).is_none(),
"base core is decodable"
);
d[9] |= 0x10; // set the reserved bit
d[10] |= 0x06; // LFE flag = 3 (invalid)
assert_eq!(
core_header_drop_reason(&d),
Some(DropReason::ReservedBit),
"reserved-bit core must be judged undecodable"
Some(DropReason::LfeFlag),
"invalid-LFE core must be judged undecodable"
);
d
}
@@ -1967,11 +1980,6 @@ mod tests {
d[8] &= !0x3C;
assert_eq!(core_header_drop_reason(&d), Some(DropReason::SampleRate));
// reserved bit set: byte9 bit4.
let mut d = good.clone();
d[9] |= 0x10;
assert_eq!(core_header_drop_reason(&d), Some(DropReason::ReservedBit));
// lfe_present == 3: LFE is byte10 bits2-1.
let mut d = good.clone();
d[10] |= 0x06;
@@ -2022,6 +2030,70 @@ mod tests {
}
}
/// Set FTYPE (byte4 bit7: 1 = normal, 0 = termination) and the 5-bit SHORT
/// field (byte4 bits6-2), leaving CPF and the NBLKS high bit (bits1-0) intact.
/// `deficit_samples = short_field + 1`.
fn set_ftype_short(core: &mut [u8], normal: bool, short_field: u8) {
core[4] = (core[4] & 0x03) | ((normal as u8) << 7) | ((short_field & 0x1F) << 2);
}
#[test]
fn termination_frame_with_small_deficit_is_kept() {
// ETSI TS 102 114 / ffmpeg (`normal_frame && deficit != 32`) / dcadec:
// a TERMINATION frame (FTYPE=0) may legally carry fewer than 32 deficit
// samples and is fully decodable. It must NOT be dropped — dropping the
// last frame of a stream silences real audio (recover-100% violation).
let mut core = make_dts_core(512);
set_ftype_short(&mut core, false, 10); // termination, deficit = 11 (< 32)
assert_eq!(
core_header_drop_reason(&core),
None,
"a termination frame with a small deficit is decodable and must be kept"
);
// End-to-end: a termination frame closed by a following core survives.
let mut term = make_dts_core(512);
set_ftype_short(&mut term, false, 5); // deficit = 6
let mut stream = term;
stream.extend_from_slice(&make_dts_core(640));
let mut parser = DtsParser::new();
let mut frames = parser.parse(&make_pes(stream, Some(90000)));
frames.extend(parser.flush());
assert_eq!(frames.len(), 2, "termination frame is emitted, not dropped");
assert_eq!(frames[0].data.len(), 512);
assert_eq!(parser.dropped_frames(), 0);
}
#[test]
fn normal_frame_with_wrong_deficit_is_dropped() {
// The other side of the FTYPE gate: a NORMAL frame (FTYPE=1) whose
// deficit-sample field is not 32 is genuinely undecodable and must be
// dropped. Guards against the fix over-relaxing into "never check deficit".
let mut core = make_dts_core(512);
set_ftype_short(&mut core, true, 10); // normal, deficit = 11 (!= 32)
assert_eq!(
core_header_drop_reason(&core),
Some(DropReason::DeficitSamples),
"a normal frame with deficit != 32 is undecodable and must be dropped"
);
}
#[test]
fn reserved_bit_set_is_not_dropped() {
// The bit after RATE is a RESERVED field that both reference decoders
// SKIP (ffmpeg `skip_bits1`, dcadec `bits_skip1` "Reserved field") — they
// never reject a frame that sets it. Rejecting was a false-drop that
// silenced any real stream whose encoder set the bit. Setting it (byte9
// bit4) on an otherwise-valid core must leave it KEPT.
let mut core = make_dts_core(512);
assert_eq!(core_header_drop_reason(&core), None, "baseline decodable");
core[9] |= 0x10; // set the reserved bit
assert_eq!(
core_header_drop_reason(&core),
None,
"a set reserved bit must NOT drop a decodable frame"
);
}
/// Real-data fixture (ignored). Re-parses a raw `.dts` elementary stream
/// through `DtsParser` and writes the emitted access units back out, so the
/// garbage-extension → core-only drop can be validated against an actual
+97
View File
@@ -835,6 +835,103 @@ mod tests {
// `corrupt_major_sync_drops_forward_to_next_valid`).
}
/// Independent bitwise CRC-16 (poly 0x002D, init 0, MSB-first) — a SEPARATE
/// oracle from `crc16_mlp`, so a fixture built with it is not tautological
/// with the validator under test. Anchored to the catalogue check value
/// (0x4FF7 for "123456789") so the oracle itself is proven correct without
/// reference to the code under test.
fn ref_crc16_2d(data: &[u8]) -> u16 {
let mut crc: u16 = 0;
for &b in data {
crc ^= (b as u16) << 8;
for _ in 0..8 {
crc = if crc & 0x8000 != 0 {
(crc << 1) ^ 0x002D
} else {
crc << 1
};
}
}
crc
}
#[test]
fn extended_major_sync_crc_validates_and_rejects() {
// COVERAGE GAP (the endianness bug that once "silently dropped the whole
// track" on real 7.1/Atmos): the EXTENDED major-sync header path
// (ms[25]&1 set, mshdr = 28 + 2 + 2*n) had ZERO test coverage — every
// other fixture builds only the basic 28-byte header. Build an extended
// header whose trailer is an INDEPENDENTLY-computed oracle (ref_crc16_2d,
// NOT crc16_mlp) stored LITTLE-ENDIAN, and assert the validator accepts
// it, rejects a body corruption, and rejects the same trailer stored
// big-endian (which is exactly the endianness-mix regression).
assert_eq!(
ref_crc16_2d(b"123456789"),
0x4FF7,
"oracle anchored to catalogue"
);
// n = 3 extension words → mshdr = 28 + 2 + 2*3 = 36.
let n = 3usize;
let mshdr = 28 + 2 + 2 * n;
assert_eq!(mshdr, 36);
let mut ms = vec![0u8; 40]; // slack past the 36-byte header
// Non-trivial, varied body so the CRC is a meaningful function of it.
for (i, b) in ms.iter_mut().enumerate().take(mshdr - 4) {
*b = (0x37u8).wrapping_add((i as u8).wrapping_mul(0x53));
}
ms[25] |= 1; // extension flag → selects the extended header size
ms[26] = (ms[26] & 0x0F) | ((n as u8) << 4); // extension word count in high nibble
// The 2-byte "penultimate" word (between the CRC-covered body and the
// trailer). Chosen non-zero and non-palindromic so the LE/BE distinction
// is observable.
ms[mshdr - 4] = 0x12;
ms[mshdr - 3] = 0x34;
// Oracle: checksum16 = crc16_2D(body).swap_bytes() ^ le16(penultimate),
// computed with the INDEPENDENT ref CRC, then stored LITTLE-ENDIAN.
let le_word = u16::from_le_bytes([ms[mshdr - 4], ms[mshdr - 3]]);
let trailer = ref_crc16_2d(&ms[..mshdr - 4]).swap_bytes() ^ le_word;
ms[mshdr - 2] = (trailer & 0xFF) as u8;
ms[mshdr - 1] = (trailer >> 8) as u8;
assert_ne!(
ms[mshdr - 2],
ms[mshdr - 1],
"trailer bytes must differ so the LE/BE swap below is a real distinction"
);
// The extended header size is computed from ms[25]/ms[26].
assert_eq!(
mlp_major_sync_header_size(&ms),
Some(mshdr),
"extended header size = 28 + 2 + 2*n"
);
// The validator accepts the independently-built extended major sync.
assert!(
mlp_major_sync_crc_ok(&ms, mshdr),
"valid extended major-sync checksum must validate"
);
// A single corrupted body byte must be rejected.
let mut corrupt = ms.clone();
corrupt[10] ^= 0xFF;
assert!(
!mlp_major_sync_crc_ok(&corrupt, mshdr),
"a corrupted extended major sync must be rejected"
);
// The endianness regression: the SAME checksum stored big-endian must be
// rejected. A validator that reads the trailer big-endian (the shipped
// bug) would instead accept this and reject the correct LE form above.
let mut swapped = ms.clone();
swapped.swap(mshdr - 2, mshdr - 1);
assert!(
!mlp_major_sync_crc_ok(&swapped, mshdr),
"a big-endian-stored trailer must be rejected (little-endian is load-bearing)"
);
}
#[test]
fn parity_failure_is_dropped() {
// A normal AU whose header parity is broken (after a major sync sets
+98 -1
View File
@@ -540,6 +540,20 @@ fn reader_event_fn(events: Arc<dyn MuxEvents>) -> crate::sector::prefetched::Eve
/// The reader-agnostic driver body: headers → gate → sink → pump → finish.
/// Split out so it can be unit-tested against a synthetic [`Stream`] (the
/// injection seam), independent of which constructor built `stream`.
/// Whether a finished mux counts as COMPLETED. A clean operator stop
/// (`interrupted`), a wedged/halted finalize (`finalize_failed` — the write
/// [`Pipeline`] returned `Halted`/`PipelineJoinTimeout` from `finish`), or a
/// halt cancellation each force `completed = false`, so the consumer runs its
/// stop-preserves-staging path instead of reporting a truncated file as done.
///
/// Extracted as a pure fn because the `finalize_failed` branch is otherwise
/// reachable only through real write-thread wedge timing (the internally-built
/// `WriteSink` offers no seam to force a `finish` timeout deterministically), so
/// the mapping is unit-tested here directly.
fn mux_run_completed(interrupted: bool, finalize_failed: bool, halt_cancelled: bool) -> bool {
!(interrupted || finalize_failed || halt_cancelled)
}
fn drive_mux(
mut stream: Box<dyn Stream>,
dest_url: &str,
@@ -738,7 +752,7 @@ fn drive_mux(
Err(e) => return Err(e.into()),
};
if interrupted || finalize_failed || halt.is_cancelled() {
if !mux_run_completed(interrupted, finalize_failed, halt.is_cancelled()) {
return Ok(MuxOutcome {
completed: false,
output_opened: true,
@@ -1692,6 +1706,89 @@ mod tests {
);
}
/// FIX 3: a mux whose read side drained cleanly (`interrupted = false`, halt
/// not cancelled) but whose write pipeline WEDGED on finish (`finish_with_halt`
/// → `Err(Halted | PipelineJoinTimeout)` → `finalize_failed = true`) must fall
/// through to `completed = false` — never surface a truncated file as a
/// finished rip. The wedge is reachable only via real write-thread timing, so
/// the completion mapping is tested via the extracted pure fn.
///
/// Mutation: dropping `finalize_failed` from `mux_run_completed`'s condition
/// makes `mux_run_completed(false, true, false)` return `true` → this fails.
#[test]
fn finalize_failed_forces_incomplete_outcome() {
// The load-bearing case: clean drain, wedged finalize → NOT completed.
assert!(
!mux_run_completed(false, true, false),
"a wedged/halted finalize must force completed = false"
);
// A fully clean finish is the only path to completed = true.
assert!(
mux_run_completed(false, false, false),
"a clean drain + clean finalize completes"
);
// The other two forcers likewise yield incomplete.
assert!(
!mux_run_completed(true, false, false),
"operator stop → incomplete"
);
assert!(
!mux_run_completed(false, false, true),
"halt cancel → incomplete"
);
}
/// FIX 4: `MuxInput::Session` with a `title_index` past the disc's title count
/// must surface a clean `Error::MuxTrackRange` (code E9011), NOT panic on the
/// out-of-range `titles.get(idx)`. Everything else is valid (disc scanned,
/// reader staged) so the range guard is the sole failure.
///
/// Mutation: replacing the `.ok_or(MuxTrackRange…)?` guard with `.unwrap()`
/// panics on the out-of-range index → this test fails.
#[test]
fn mux_input_session_out_of_range_title_is_clean_error_not_panic() {
use crate::disc::Extent;
use crate::session::DiscSession;
let unit_key = [0x5Au8; 16];
let reader = Box::new(AacsUnitReader {
unit: encrypted_audio_unit(&unit_key),
capacity: 2048,
});
let mut title = aac_audio_title(0x1100);
title.extents = vec![Extent {
start_lba: 0,
sector_count: 3,
}];
let disc = aacs_session_disc(title, unit_key);
let num_titles = disc.titles.len();
let mut session = DiscSession::from_parts_for_test(Some(disc), Some(reader), None);
let opts = MuxOptions {
skip_errors: false,
batch_sectors: 3,
raw: false,
send_deadline: Some(Duration::from_secs(60)),
};
let halt = Halt::new();
let err = mux_stream(
MuxInput::Session {
session: &mut session,
title_index: num_titles + 5, // out of range
},
"null://",
&opts,
&halt,
Arc::new(NoopEvents),
)
.expect_err("an out-of-range title index must be a clean error, not a panic");
// MuxTrackRange renders as "E9011: track/tracks".
assert!(
err.to_string().contains("E9011"),
"expected MuxTrackRange (E9011), got: {err}"
);
}
/// The shared `resolve_inline_base_map` helper's gating: an AACS key set
/// yields a map (Some); CSS/clear/None and `raw` yield None (CSS self-cracks
/// in `DiscStream::new`; raw is ciphertext passthrough). Guards the Session