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.
This commit is contained in:
Matthew Jackson
2026-07-29 18:56:48 -07:00
parent a9dc3d7244
commit bcf47cc4ca
2 changed files with 54 additions and 7 deletions
+8 -3
View File
@@ -74,9 +74,14 @@ pub fn set_decrypt_threads(n: usize) {
DECRYPT_THREADS.store(clamped, Ordering::Relaxed); DECRYPT_THREADS.store(clamped, Ordering::Relaxed);
// Drop the existing pool. Next decrypt_pool() call rebuilds with // Drop the existing pool. Next decrypt_pool() call rebuilds with
// the new resolved thread count. // the new resolved thread count.
if let Ok(mut guard) = DECRYPT_POOL.write() { //
*guard = None; // Recover the guard on poisoning, exactly as `decrypt_pool` does. Skipping
} // the swap on a poisoned lock silently kept the STALE pool alive while the
// atomic above already reported the new thread count, so the setting appeared
// to take effect and never did. The pool Arc is immutable once stored, so a
// prior panic cannot have left it half-written.
let mut guard = DECRYPT_POOL.write().unwrap_or_else(|e| e.into_inner());
*guard = None;
} }
/// Get (or lazily build) the active rayon thread pool. Returns an /// Get (or lazily build) the active rayon thread pool. Returns an
+46 -4
View File
@@ -361,12 +361,17 @@ fn dts_channel_layout(amode: usize, lfe: bool) -> u16 {
fn ddts_box(c: &DtsConfig) -> Vec<u8> { fn ddts_box(c: &DtsConfig) -> Vec<u8> {
// avg/max bitrate: computed from the core frame size × frame rate (the core // avg/max bitrate: computed from the core frame size × frame rate (the core
// RATE field reads "open/variable" for lossless, so it's not usable directly). // RATE field reads "open/variable" for lossless, so it's not usable directly).
let frames_per_sec = if c.frame_samples > 0 { // Rate is NOT integral in general (e.g. 48000 / 512 = 93.75 frames/s for a
c.sample_rate as u64 / c.frame_samples as u64 // 512-sample core), so dividing first truncates and under-declares the
// bitrate. Multiply before dividing to keep the full precision, rounding to
// nearest so the declared value is not systematically low.
let bitrate = if c.frame_samples > 0 {
let num = c.core_size as u64 * 8 * c.sample_rate as u64;
let den = c.frame_samples as u64;
((num + den / 2) / den).min(u32::MAX as u64) as u32
} else { } else {
0 0
}; };
let bitrate = (c.core_size as u64 * 8 * frames_per_sec) as u32;
let mut out = Vec::new(); let mut out = Vec::new();
out.extend_from_slice(&c.sample_rate.to_be_bytes()); // DTSSamplingFrequency out.extend_from_slice(&c.sample_rate.to_be_bytes()); // DTSSamplingFrequency
@@ -392,7 +397,11 @@ fn ddts_box(c: &DtsConfig) -> Vec<u8> {
push(stream_construction, 5); push(stream_construction, 5);
push(c.lfe as u128, 1); push(c.lfe as u128, 1);
push(c.amode as u128, 6); push(c.amode as u128, 6);
push(c.core_size as u128, 14); // CoreSize is 14 bits, but core_size = FSIZE + 1 and FSIZE is itself 14 bits,
// so a maximum-size core is 16384 — one past what the field can hold, and the
// `& ((1<<14)-1)` mask in `push` would wrap it to 0. Clamp: declaring 16383 is
// one byte short, declaring 0 tells a decoder the core is empty.
push((c.core_size as u128).min((1u128 << 14) - 1), 14);
push(0, 1); // StereoDownmix push(0, 1); // StereoDownmix
push(0, 3); // RepresentationType push(0, 3); // RepresentationType
push(c.channel_layout as u128, 16); push(c.channel_layout as u128, 16);
@@ -631,6 +640,39 @@ mod tests {
assert_eq!(c.channels, 8, "AMODE 15 core is 8 channels, not 6"); assert_eq!(c.channels, 8, "AMODE 15 core is 8 channels, not 6");
} }
#[test]
fn ddts_core_size_clamps_instead_of_wrapping_to_zero() {
// core_size = FSIZE + 1 with FSIZE 14 bits, so its maximum is 16384 — one
// past the 14-bit ddts CoreSize field. Masking wrapped that to 0, telling a
// decoder the core frame is empty.
let c = DtsConfig {
sample_rate: 48_000,
channels: 6,
amode: 9,
lfe: true,
core_size: 16_384,
frame_samples: 512,
has_extension: false,
channel_layout: dts_channel_layout(9, true),
};
let b = ddts_box(&c);
// Decode CoreSize back out of the emitted box rather than restating the
// clamp. Layout: 8-byte box header, then DTSSamplingFrequency(4) +
// maxBitrate(4) + avgBitrate(4) + pcmSampleDepth(1) = 13 bytes, then the
// 56-bit packed tail. Within that tail CoreSize sits after
// FrameDuration(2) + StreamConstruction(5) + CoreLFEPresent(1) +
// CoreLayout(6) = 14 bits, so it occupies bits 14..28.
let tail = &b[8 + 13..];
assert!(tail.len() >= 4, "packed tail present");
let word = u32::from_be_bytes([tail[0], tail[1], tail[2], tail[3]]);
let core_size = (word >> 4) & 0x3FFF;
assert_eq!(
core_size, 16_383,
"a 16384-byte core must be declared as the 14-bit maximum, not wrapped"
);
assert_ne!(core_size, 0, "wrapping to 0 declares an empty core frame");
}
#[test] #[test]
fn ddts_channel_layout_speaker_count_matches_declared_channels() { fn ddts_channel_layout_speaker_count_matches_declared_channels() {
// The `ddts` box carries BOTH a channel count and a 16-bit speaker mask, // The `ddts` box carries BOTH a channel count and a 16-bit speaker mask,