Skip to content

Commit a5ebe5d

Browse files
authored
Address reviewer feedback: extract t_ratio helpers, add FixedAsync::Output and loop-level tests
- Extract `compute_t_ratio_increment(r1, r2, n)` from the inline computation in `process_into_buffer`; use it there. This makes the linear-in-step-size property explicit and testable. - Extract `advance_index(start, t_ratio, increment, n)` that replicates the exact `t_ratio += inc; idx += t_ratio` loop body shared by every inner resampler. Drives the bound-safety proof in tests without touching the hot paths. - Add `FixedAsync::Output` regression tests for both polynomial and sinc resamplers (large ratio changes, ramp=true), exercising the `calculate_input_size` fix symmetrically with the existing Input-mode tests. - Add `t_ratio_increment_reaches_target` and `t_ratio_increment_equal_ratios_is_zero` unit tests that verify `compute_t_ratio_increment` is linear in step-size space. - Add `advance_index_matches_analytical_formula`: verifies the closed-form estimate (`n * avg_t_ratio + ramp_overshoot`) matches the exact loop output to within floating-point noise (1e-6), closing the loop between estimate and arithmetic. - Add `advance_index_stays_within_buffer_bounds`: for a grid of ratio pairs, last_index values, and a representative interpolator_len, computes the frame count from `calculate_output_size` then drives `advance_index` and asserts floor(final_idx) <= chunk_size - interpolator_len - 1. This is the direct, loop-level proof that the estimate is never an overestimate.
1 parent 3910592 commit a5ebe5d

1 file changed

Lines changed: 284 additions & 3 deletions

File tree

src/asynchro.rs

Lines changed: 284 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,52 @@ where
383383
0.5 * (1.0 / resample_ratio + 1.0 / target_ratio)
384384
}
385385

386+
/// Compute the per-frame step-size increment used during a ramped
387+
/// transition from `resample_ratio` to `target_ratio` over `nbr_frames`
388+
/// output frames.
389+
///
390+
/// The step size ramps linearly in step-size space:
391+
/// ```text
392+
/// t_ratio[k] = 1/resample_ratio + k * t_ratio_increment
393+
/// t_ratio[nbr_frames] == 1/target_ratio
394+
/// ```
395+
/// This is the value used in the inner loop:
396+
/// `t_ratio += t_ratio_increment; idx += t_ratio;`
397+
#[inline(always)]
398+
fn compute_t_ratio_increment(
399+
resample_ratio: f64,
400+
target_ratio: f64,
401+
nbr_frames: usize,
402+
) -> f64 {
403+
(1.0 / target_ratio - 1.0 / resample_ratio) / nbr_frames as f64
404+
}
405+
406+
/// Simulate `nbr_frames` steps of the inner index-advance loop:
407+
/// ```text
408+
/// t_ratio += t_ratio_increment
409+
/// idx += t_ratio
410+
/// ```
411+
/// Returns the final value of `idx`.
412+
///
413+
/// This function is the reference implementation of the loop body shared
414+
/// by all inner resamplers (polynomial and sinc). It can be used in tests
415+
/// to verify that `calculate_output_size` and `calculate_input_size` never
416+
/// let the index exceed the input buffer boundary.
417+
fn advance_index(
418+
start_idx: f64,
419+
start_t_ratio: f64,
420+
t_ratio_increment: f64,
421+
nbr_frames: usize,
422+
) -> f64 {
423+
let mut idx = start_idx;
424+
let mut t_ratio = start_t_ratio;
425+
for _ in 0..nbr_frames {
426+
t_ratio += t_ratio_increment;
427+
idx += t_ratio;
428+
}
429+
idx
430+
}
431+
386432
fn calculate_input_size(
387433
chunk_size: usize,
388434
resample_ratio: f64,
@@ -539,9 +585,12 @@ where
539585
let interpolator_len = self.inner_resampler.nbr_points();
540586

541587
let t_ratio = 1.0 / self.resample_ratio;
542-
let t_ratio_end = 1.0 / self.target_ratio;
543588

544-
let t_ratio_increment = (t_ratio_end - t_ratio) / self.needed_output_size as f64;
589+
let t_ratio_increment = Self::compute_t_ratio_increment(
590+
self.resample_ratio,
591+
self.target_ratio,
592+
self.needed_output_size,
593+
);
545594

546595
// Update buffer with new data.
547596
for buf in self.buffer.iter_mut() {
@@ -1178,7 +1227,7 @@ mod tests {
11781227
(consumed, produced)
11791228
}
11801229

1181-
/// Regression test for issue #136.
1230+
/// Regression test for issue #136 — `FixedAsync::Input` mode.
11821231
///
11831232
/// Before the fix, `calculate_output_size` used the arithmetic mean of the
11841233
/// *ratios* to estimate how many output frames fit inside the input buffer.
@@ -1286,4 +1335,236 @@ mod tests {
12861335
"second block produced {produced2} frames, expected {frames_out2}",
12871336
);
12881337
}
1338+
1339+
/// Regression test for issue #136 — `FixedAsync::Output` mode.
1340+
///
1341+
/// In `FixedAsync::Output` mode the output chunk size is fixed and the
1342+
/// input size is variable (computed by `calculate_input_size`). The same
1343+
/// averaging error that caused the `FixedAsync::Input` panic would have
1344+
/// caused `calculate_input_size` to underestimate how many input frames
1345+
/// are needed, making the resampler read past the allocated buffer.
1346+
#[test_log::test(test_matrix(
1347+
[PolynomialDegree::Cubic, PolynomialDegree::Linear],
1348+
[0.2f64, 5.0f64]
1349+
))]
1350+
fn poly_output_fixed_ramp_large_ratio_change_does_not_panic(
1351+
degree: PolynomialDegree,
1352+
target_rel: f64,
1353+
) {
1354+
let chunk_size = 1024;
1355+
let channels = 1;
1356+
let mut resampler =
1357+
Async::<f64>::new_poly(1.0, 6.0, degree, chunk_size, channels, FixedAsync::Output)
1358+
.unwrap();
1359+
1360+
// First block at the nominal ratio.
1361+
let (_, frames_out) = process_one_block(&mut resampler, channels);
1362+
assert!(
1363+
frames_out == chunk_size,
1364+
"first block: expected {chunk_size} output frames, got {frames_out}",
1365+
);
1366+
1367+
// Change ratio dramatically with ramp=true.
1368+
resampler
1369+
.set_resample_ratio_relative(target_rel, true)
1370+
.unwrap();
1371+
1372+
let frames_in2 = resampler.input_frames_next();
1373+
let frames_out2 = resampler.output_frames_next();
1374+
assert!(
1375+
frames_in2 > 0,
1376+
"after ratio change: expected nonzero input frames, got {frames_in2}",
1377+
);
1378+
assert_eq!(
1379+
frames_out2, chunk_size,
1380+
"after ratio change: expected {chunk_size} output frames, got {frames_out2}",
1381+
);
1382+
1383+
// Second block must complete without panicking.
1384+
let (consumed2, produced2) = process_one_block(&mut resampler, channels);
1385+
assert_eq!(
1386+
consumed2, frames_in2,
1387+
"second block consumed {consumed2} frames, expected {frames_in2}",
1388+
);
1389+
assert_eq!(
1390+
produced2, frames_out2,
1391+
"second block produced {produced2} frames, expected {frames_out2}",
1392+
);
1393+
}
1394+
1395+
#[test_log::test(test_matrix(
1396+
[0.2f64, 5.0f64]
1397+
))]
1398+
fn sinc_output_fixed_ramp_large_ratio_change_does_not_panic(target_rel: f64) {
1399+
let chunk_size = 1024;
1400+
let channels = 1;
1401+
let params = basic_params();
1402+
let mut resampler =
1403+
Async::<f64>::new_sinc(1.0, 6.0, &params, chunk_size, channels, FixedAsync::Output)
1404+
.unwrap();
1405+
1406+
// First block at the nominal ratio.
1407+
let (_, frames_out) = process_one_block(&mut resampler, channels);
1408+
assert!(
1409+
frames_out == chunk_size,
1410+
"first block: expected {chunk_size} output frames, got {frames_out}",
1411+
);
1412+
1413+
// Change ratio dramatically with ramp=true.
1414+
resampler
1415+
.set_resample_ratio_relative(target_rel, true)
1416+
.unwrap();
1417+
1418+
let frames_in2 = resampler.input_frames_next();
1419+
let frames_out2 = resampler.output_frames_next();
1420+
assert!(
1421+
frames_in2 > 0,
1422+
"after ratio change: expected nonzero input frames, got {frames_in2}",
1423+
);
1424+
assert_eq!(
1425+
frames_out2, chunk_size,
1426+
"after ratio change: expected {chunk_size} output frames, got {frames_out2}",
1427+
);
1428+
1429+
// Second block must complete without panicking.
1430+
let (consumed2, produced2) = process_one_block(&mut resampler, channels);
1431+
assert_eq!(
1432+
consumed2, frames_in2,
1433+
"second block consumed {consumed2} frames, expected {frames_in2}",
1434+
);
1435+
assert_eq!(
1436+
produced2, frames_out2,
1437+
"second block produced {produced2} frames, expected {frames_out2}",
1438+
);
1439+
}
1440+
1441+
// --- compute_t_ratio_increment unit tests ---
1442+
1443+
/// `compute_t_ratio_increment` must produce a ramp that reaches exactly
1444+
/// `1/target_ratio` after `n` increments from `1/resample_ratio`.
1445+
#[test_log::test]
1446+
fn t_ratio_increment_reaches_target() {
1447+
for (r1, r2, n) in [
1448+
(1.0f64, 0.2f64, 100usize),
1449+
(1.0, 5.0, 1024),
1450+
(2.0, 3.0, 512),
1451+
(0.5, 0.5, 256),
1452+
(0.125, 8.0, 64),
1453+
] {
1454+
let inc = Async::<f64>::compute_t_ratio_increment(r1, r2, n);
1455+
let t_ratio_end = 1.0 / r1 + n as f64 * inc;
1456+
let expected_end = 1.0 / r2;
1457+
assert!(
1458+
(t_ratio_end - expected_end).abs() < 1e-10,
1459+
"r1={r1}, r2={r2}, n={n}: t_ratio after {n} increments = {t_ratio_end}, expected {expected_end}",
1460+
);
1461+
}
1462+
}
1463+
1464+
/// At equal ratios `compute_t_ratio_increment` must return zero (no ramp).
1465+
#[test_log::test]
1466+
fn t_ratio_increment_equal_ratios_is_zero() {
1467+
for r in [0.1f64, 0.5, 1.0, 2.0, 10.0] {
1468+
for n in [1usize, 100, 1024] {
1469+
let inc = Async::<f64>::compute_t_ratio_increment(r, r, n);
1470+
assert!(
1471+
inc.abs() < 1e-15,
1472+
"compute_t_ratio_increment({r}, {r}, {n}) = {inc}, expected 0.0",
1473+
);
1474+
}
1475+
}
1476+
}
1477+
1478+
// --- advance_index unit tests ---
1479+
1480+
/// The index advance produced by the exact loop must equal the closed-form
1481+
/// prediction `n * avg_t_ratio + ramp_overshoot` to within floating-point
1482+
/// rounding error.
1483+
///
1484+
/// This closes the loop between the analytical estimate used in
1485+
/// `calculate_output_size`/`calculate_input_size` and the real arithmetic
1486+
/// performed in every inner resampler loop.
1487+
#[test_log::test]
1488+
fn advance_index_matches_analytical_formula() {
1489+
for (r1, r2, n) in [
1490+
(1.0f64, 0.2f64, 100usize),
1491+
(1.0, 5.0, 1024),
1492+
(2.0, 3.0, 512),
1493+
(0.5, 0.8, 256),
1494+
(0.125, 8.0, 64),
1495+
(1.0, 1.0, 200),
1496+
] {
1497+
let start_idx = 0.0f64;
1498+
let inc = Async::<f64>::compute_t_ratio_increment(r1, r2, n);
1499+
let final_idx =
1500+
Async::<f64>::advance_index(start_idx, 1.0 / r1, inc, n);
1501+
1502+
let avg = Async::<f64>::avg_t_ratio(r1, r2);
1503+
let ramp_overshoot = 0.5 * (1.0 / r2 - 1.0 / r1);
1504+
let analytical = start_idx + n as f64 * avg + ramp_overshoot;
1505+
1506+
// Tolerate small floating-point accumulation over n steps.
1507+
let tol = 1e-6;
1508+
assert!(
1509+
(final_idx - analytical).abs() <= tol,
1510+
"r1={r1}, r2={r2}, n={n}: advance_index={final_idx}, analytical={analytical}, diff={}",
1511+
(final_idx - analytical).abs(),
1512+
);
1513+
}
1514+
}
1515+
1516+
/// For every combination of ratio-change direction and magnitude, the
1517+
/// index produced by `advance_index` (using the output-frame count from
1518+
/// `calculate_output_size`) must stay within the input buffer bounds.
1519+
///
1520+
/// This is the direct, loop-level proof that the fix in
1521+
/// `calculate_output_size` is tight: the analytical estimate is never an
1522+
/// overestimate.
1523+
#[test_log::test]
1524+
fn advance_index_stays_within_buffer_bounds() {
1525+
let chunk_size = 1024usize;
1526+
let interpolator_len = 4usize; // representative polynomial kernel half-width
1527+
1528+
for last_index in [0.0f64, 0.5, 2.0] {
1529+
for (r1, r2) in [
1530+
(1.0f64, 0.2f64),
1531+
(1.0, 5.0),
1532+
(0.5, 2.0),
1533+
(2.0, 0.5),
1534+
(1.0, 1.0),
1535+
(0.3, 0.3),
1536+
(0.125, 8.0),
1537+
(8.0, 0.125),
1538+
] {
1539+
let n = Async::<f64>::calculate_output_size(
1540+
chunk_size,
1541+
r1,
1542+
r2,
1543+
last_index,
1544+
interpolator_len,
1545+
&FixedAsync::Input,
1546+
);
1547+
1548+
if n == 0 {
1549+
// No output frames fit in this configuration; nothing to check.
1550+
continue;
1551+
}
1552+
1553+
let inc = Async::<f64>::compute_t_ratio_increment(r1, r2, n);
1554+
let final_idx =
1555+
Async::<f64>::advance_index(last_index, 1.0 / r1, inc, n);
1556+
1557+
// The inner loop uses floor(idx) as the array start index,
1558+
// so we check the integer part rather than the raw float to
1559+
// avoid false failures from sub-ULP floating-point noise.
1560+
let bound = chunk_size - interpolator_len - 1;
1561+
assert!(
1562+
final_idx.floor() as usize <= bound,
1563+
"r1={r1}, r2={r2}, last_index={last_index}, n={n}: \
1564+
advance_index floor(final_idx)={} exceeds buffer bound={bound}",
1565+
final_idx.floor() as usize,
1566+
);
1567+
}
1568+
}
1569+
}
12891570
}

0 commit comments

Comments
 (0)