fix(coreaudio): honour the full timeout when waiting for a sample rate change

[dylanpulver]

set_sample_rate waits for RateListener to report the new rate, and re-arms when a notification carries some other rate. The re-arm subtracted start.elapsed() — the total elapsed time — from the already-reduced remaining, so the budget shrank super-linearly and hit zero well before the caller's timeout:

remaining = remaining.checked_sub(start.elapsed())   // total elapsed, every iteration

With a 500 ms timeout and a listener emitting a non-matching rate every few ms, the loop gave up after 82 ms and returned Sample rate update timed out on a device that was still settling. Perversely, the chattier the device, the shorter the effective timeout. Subtracting from the original timeout instead makes it a single deadline; the same measurement then runs the full 500 ms.

Non-matching notifications are reachable: RateListener's callback re-reads the property and sends whatever it gets, including 0.0 when that read fails — which matches no target rate.

The loop is extracted into wait_for_rate so it can be tested without a device. Two tests: one asserts the whole timeout is spent across repeated non-matching events (fails at 82 ms on the current code, and also on a saturating_sub variant of it, so it pins the accounting rather than the helper); one asserts it returns as soon as the target rate arrives. cargo test on macOS goes 18 → 20 passing, including the existing device tests. fmt, clippy --all --all-targets -- -D warnings and cargo doc are clean.

The other backends call recv_timeout(dur) once and never re-arm, so this is CoreAudio-only.

Honest limit: I could not reproduce the intermediate-rate notification on real hardware — the built-in mic and speakers here settle in a single notification — so that half is unverified. The evidence is the extracted unit test and the arithmetic.

[roderickvd]

Thanks for your PR for what indeed is a real bug. I propose another solution though, consistent with what we do in ALSA.

[roderickvd]

Shorter & sweeter would be:

CoreAudio: Fix sample rate changes timing out early when the device reports other rates first.

[roderickvd]

Please don't wait 500 ms in every test run.

[roderickvd]

This still manually implements the deadline by subtracting every loop iteration. In ALSA we've got something prettier by computing one deadline up front and checking saturating_duration_since each iteration:

fn wait_for_rate(
    receiver: &Receiver<f64>,
    target_sample_rate: SampleRate,
    timeout: Duration,
) -> Result<(), Error> {
    let deadline = Instant::now() + timeout;

    loop {
        let remaining = deadline.saturating_duration_since(Instant::now());
        if remaining.is_zero() {
            return Err(Error::with_message(
                ErrorKind::DeviceNotAvailable,
                "Sample rate update timed out",
            ));
        }

        match receiver.recv_timeout(remaining) {
            Ok(reported_rate) => {
                if (reported_rate - target_sample_rate as f64).abs() < 1.0 {
                    return Ok(());
                }
            }
            Err(RecvTimeoutError::Timeout) => {
                return Err(Error::with_message(
                    ErrorKind::DeviceNotAvailable,
                    "Sample rate update timed out",
                ));
            }
            Err(RecvTimeoutError::Disconnected) => {
                return Err(Error::with_message(
                    ErrorKind::StreamInvalidated,
                    "Sample rate listener disconnected unexpectedly",
                ));
            }
        }
    }
}

As bonus, this also gets rid of a blocking recv_timeout(ZERO) edge case.

[roderickvd]

@dylanpulver friendly nudge when you'd have the bandwidth to take this on?

[dylanpulver]

Thanks for the nudge, and sorry for the wait. All three taken, and rebased on master (the only conflict was the changelog).

• Deadline. Switched to your version — one deadline up front, saturating_duration_since each iteration, and the zero check before the recv_timeout. It reads better than the subtraction, and matching ALSA is the right call.
• Changelog. Used your wording.
• 500 ms per test run. Dropped to a 50 ms timeout with the feeder sending every 1 ms, so several events still land inside the window. The two tests now finish in 0.05 s together.

Still pinned to the bug rather than passing either way: restoring the old per-iteration subtraction fails the timeout test with gave up after 11.814041ms, well before the 50ms timeout. cargo test --lib is 20 passed / 0 failed on macOS, cargo fmt --check and cargo clippy --lib --all-features clean.

[roderickvd]

Thanks, almost right, see below.

[roderickvd]

This panics on overflow. Consider:

fn wait_for_rate(
    receiver: &Receiver<f64>,
    target_sample_rate: SampleRate,
    timeout: Option<Duration>,
) -> Result<(), Error> {
    let deadline = timeout.and_then(|t| Instant::now().checked_add(t));

    loop {
        let received = match deadline {
            Some(deadline) => {
                let remaining = deadline.saturating_duration_since(Instant::now());
                if remaining.is_zero() {
                    return Err(Error::with_message(
                        ErrorKind::DeviceNotAvailable,
                        "Sample rate update timed out",
                    ));
                }
                receiver.recv_timeout(remaining)
            }
            None => receiver.recv().map_err(|_| RecvTimeoutError::Disconnected),
        };

        match received {
            // ...
        }
    }
}

[roderickvd]

Pass on the Option; the contract is that a timeout of None is to wait indefinitely.