diff --git a/crates/ui-egui/src/app/playback.rs b/crates/ui-egui/src/app/playback.rs index d40bc69ff..7cda624e9 100644 --- a/crates/ui-egui/src/app/playback.rs +++ b/crates/ui-egui/src/app/playback.rs @@ -44,6 +44,7 @@ impl EffectcraftApp { starting_since: now, first_cached: None, shown_at: Default::default(), + stalls: 0, }; if !self.playback.caching { if plan.video { diff --git a/crates/ui-egui/src/app/prefetch.rs b/crates/ui-egui/src/app/prefetch.rs index 4af91782f..c8d5cd938 100644 --- a/crates/ui-egui/src/app/prefetch.rs +++ b/crates/ui-egui/src/app/prefetch.rs @@ -27,6 +27,26 @@ fn startup_frames(ahead: i64, window: i64, budget: usize, frame_bytes: usize) -> ahead.min(window).min(fit).max(1) } +/// How many cached frames ahead a held sound waits for before it starts again while the silent +/// preview keeps real time: the prefetch's `ahead`, doubled for each of the preview's `stalls` +/// that stopped the sound, never more than `cap` ([`held_audio_cap`]) nor less than one. Without +/// the doubling, a preview that renders just below real time started its sound as soon as +/// `ahead` frames were cached, ran out of them, stopped it again, and so on (#2473, #3260). +fn held_audio_frames(ahead: i64, cap: i64, stalls: u32) -> i64 { + ahead.saturating_mul(1i64 << stalls.min(32)).min(cap).max(1) +} + +/// The most cached frames ahead a held sound waits for, with a RAM preview of `fit` frames. +/// `prefetch_playback` requests offsets below `fit - 1` (what `advance_playback` gives it), and +/// while the silent preview moves on, the frames it requested last are still rendering, so a +/// run of `fit` (or even `fit - 1`) frames ahead is never all cached until the end of the +/// preview cuts it short. Half of what is requested can be, and never fewer than the prefetch's +/// `ahead` (the wait before any stall) when that many are requested. +fn held_audio_cap(ahead: i64, fit: i64) -> i64 { + let reach = (fit - 1).max(1); + ahead.max(reach / 2).min(reach) +} + impl EffectcraftApp { /// RAM preview: advance at the preview's frame rate when frames are cached; otherwise hold /// and keep rendering ahead (the green cache bar fills, then playback runs in real time). @@ -248,15 +268,17 @@ impl EffectcraftApp { } /// Held sound starts once the frames ahead are cached again after a stall (see - /// `Playback::audio_held`): the prefetch window when the silent preview keeps real time, else - /// the rest of the preview. + /// `Playback::audio_held`): the prefetch window when the silent preview keeps real time + /// (twice as many frames after each stall, see [`held_audio_frames`]), else the rest of the + /// preview. fn start_held_audio(&mut self, pass: &PlaybackPass<'_>, fit: i64) { let PlaybackPass { now, plan, series, cur, ahead, frame_rate: fr, .. } = *pass; if !self.playback.audio_held { return; } let Some(p) = plan else { return }; - if (self.playback.keeps_up(now) && self.cached_ahead(&p, &series, cur, ahead.min(fit))) || self.cached_ahead(&p, &series, cur, fit) { + let need = held_audio_frames(ahead, held_audio_cap(ahead, fit), self.playback.stalls); + if (self.playback.keeps_up(now) && self.cached_ahead(&p, &series, cur, need)) || self.cached_ahead(&p, &series, cur, fit) { self.playback.audio_held = false; self.playback.start_wall = now; self.playback.start_frame = cur; @@ -294,6 +316,7 @@ impl EffectcraftApp { // frame shows as it renders until the frames ahead are cached again. self.audio = None; self.playback.audio_held = true; + self.playback.stalls = self.playback.stalls.saturating_add(1); self.playback.waiting = true; // The rate measured before the stall no longer says it keeps up. self.playback.shown_at.clear(); @@ -464,7 +487,32 @@ impl EffectcraftApp { #[cfg(test)] mod startup_tests { - use super::startup_frames; + use super::{held_audio_cap, held_audio_frames, startup_frames}; + + /// After each stall the held sound waits for twice as many frames ahead, up to its cap + /// (#2473, #3260). + #[test] + fn held_sound_waits_longer_after_each_stall() { + assert_eq!(held_audio_frames(8, 1000, 0), 8, "before any stall: the prefetch's frames ahead"); + assert_eq!(held_audio_frames(8, 1000, 1), 16); + assert_eq!(held_audio_frames(8, 1000, 3), 64); + assert_eq!(held_audio_frames(8, 100, 4), 100, "never more than the cap"); + assert_eq!(held_audio_frames(8, 100, u32::MAX), 100, "no overflow however many stalls"); + assert_eq!(held_audio_frames(8, 5, 0), 5, "no more than the cap"); + assert_eq!(held_audio_frames(0, 0, 2), 1, "at least the current frame"); + } + + /// The cap is what the prefetch can keep cached ahead of a moving preview: never `fit`, which + /// includes a frame it never requests, nor `fit - 1`, whose last frames are still rendering. + #[test] + fn a_held_sound_waits_only_for_frames_that_can_be_cached() { + assert_eq!(held_audio_cap(8, 1000), 499, "half of the 999 frames requested ahead"); + assert_eq!(held_audio_cap(8, 10), 8, "never below the wait before any stall"); + assert_eq!(held_audio_cap(8, 6), 5, "nor above the frames requested"); + assert_eq!(held_audio_cap(8, 1), 1, "a one-frame RAM preview: the current frame"); + assert_eq!(held_audio_cap(8, 0), 1); + assert_eq!(held_audio_frames(8, held_audio_cap(8, 10), 5), 8, "the backoff stops short of fit"); + } /// The frames a preview with sound waits for are always requested and fit together (#947). #[test] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 795a01e1e..e641da3d4 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -215,6 +215,11 @@ pub struct Playback { pub first_cached: Option, /// When the last frames were shown (seconds, the last second's worth): the achieved rate. pub shown_at: std::collections::VecDeque, + /// How many stalls have stopped this preview's sound. After each one the held sound waits + /// for twice as many cached frames ahead before it starts again (up to what fits in the RAM + /// preview), so a preview that renders just below real time settles instead of starting and + /// stopping its sound every few frames, which plays as distortion (#2473, #3260). + pub stalls: u32, } impl Playback { diff --git a/crates/ui-egui/tests/it/ui_audio_scrub.rs b/crates/ui-egui/tests/it/ui_audio_scrub.rs index 81e55817e..6ea74ef35 100644 --- a/crates/ui-egui/tests/it/ui_audio_scrub.rs +++ b/crates/ui-egui/tests/it/ui_audio_scrub.rs @@ -74,6 +74,16 @@ impl GatedFrames { } } + /// Cache every frame the app has asked for since the last call, and nothing else: a renderer + /// that keeps up with the real prefetch requests. + fn release_requested(&self, h: &mut Harness<'_, EffectcraftApp>) { + while h.state().frames.dispatch_remote() != 0 {} + for (_, width, height, done) in std::mem::take(&mut *self.pending.lock().unwrap()) { + let rgba = vec![0; (width as usize).saturating_mul(height as usize).saturating_mul(4)]; + done(Ok(RemoteFrame { width, height, rgba, ms: 1.0 })); + } + } + /// Drop everything queued without caching it (the warm-up passes request viewer frames). fn discard(&self, h: &mut Harness<'_, EffectcraftApp>) { while h.state().frames.dispatch_remote() != 0 {} @@ -269,6 +279,133 @@ fn preview_with_audio_sounds_without_caching_the_work_area() { assert!((1..900).contains(&cached), "it started with {cached} of the 900 frames cached, not the whole work area"); } +/// Pretend the silent preview showed its last ten frames in real time, up to now: the next step +/// then finds it keeping up (`Playback::keeps_up`), however long steps take on the test host. +fn kept_up(h: &mut Harness<'_, EffectcraftApp>) { + let now = h.state().now(); + h.state_mut().playback.shown_at = (0..10i32).map(|i| now - f64::from(9 - i) / 30.0).collect(); +} + +/// Frames `from..from + n`. +fn run(from: i64, n: i64) -> Vec { + (from..from + n).collect() +} + +/// A preview that renders just below real time (#2473, #3260): once a stall stopped the sound, +/// `ahead` cached frames were enough to start it again while the silent preview kept up, so it +/// ran out of them, stopped and started again every few frames, which played as distortion. +/// After each stall the sound waits for twice as many cached frames ahead. +#[test] +fn each_stall_doubles_the_frames_ahead_the_sound_waits_for() { + let fake = Fake::default(); + let (mut app, cid) = setup_long(true, &fake, 30.0); + let gate = GatedFrames::default(); + let renderer: Arc = Arc::new(gate.clone()); + app.frames.set_remote(Some(renderer)); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 30.0).build_eframe(|_| app); + h.state_mut().use_manual_clock(); + h.run_steps(2); + gate.discard(&mut h); + let ahead = (h.state().frames.parallelism() * 2).max(4) as i64; + let now = h.state().now(); + h.state_mut().play(now); + gate.render(&mut h, cid, &run(0, ahead)); + step(&mut h); + assert!(h.state().audio.is_some(), "the sound started"); + assert_eq!(h.state().playback.stalls, 0); + + // A stall stops the sound. + gate.discard(&mut h); + h.state().frames.clear(); + h.step(); + assert!(h.state().audio.is_none() && h.state().playback.audio_held, "a stall stops the sound"); + assert_eq!(h.state().playback.stalls, 1); + // One and a half times `ahead` cached while it keeps up: that used to start the sound again. + let at = frame(&h, cid); + gate.render(&mut h, cid, &run(at, ahead + ahead / 2)); + kept_up(&mut h); + step(&mut h); + assert!(h.state().audio.is_none(), "after one stall, {} frames ahead are not enough", ahead + ahead / 2); + // Twice `ahead` are. + let at = frame(&h, cid); + gate.render(&mut h, cid, &run(at, 3 * ahead)); + kept_up(&mut h); + step(&mut h); + assert!(h.state().audio.is_some(), "after one stall, {} frames ahead start the sound", 3 * ahead); + + // A second stall: now three times `ahead` are not enough, and five times are. + gate.discard(&mut h); + h.state().frames.clear(); + h.step(); + assert!(h.state().audio.is_none() && h.state().playback.audio_held); + assert_eq!(h.state().playback.stalls, 2); + let at = frame(&h, cid); + gate.render(&mut h, cid, &run(at, 3 * ahead)); + kept_up(&mut h); + step(&mut h); + assert!(h.state().audio.is_none(), "after two stalls, {} frames ahead are not enough", 3 * ahead); + let at = frame(&h, cid); + gate.render(&mut h, cid, &run(at, 5 * ahead)); + kept_up(&mut h); + step(&mut h); + assert!(h.state().audio.is_some(), "after two stalls, {} frames ahead start the sound", 5 * ahead); + + // A new preview starts counting again. + h.state_mut().stop(); + let now = h.state().now(); + h.state_mut().play(now); + assert_eq!(h.state().playback.stalls, 0); +} + +/// Once the backoff reaches its cap, the held sound waits only for frames the prefetch really +/// requests and can keep cached while the silent preview moves on. It used to wait for `fit` +/// frames, one of which was never asked for, so a stall kept the preview silent almost to its +/// end even after rendering recovered. +#[test] +fn a_held_sound_at_the_ram_limit_starts_on_the_frames_prefetch_requests() { + let fake = Fake::default(); + let (mut app, cid) = setup_long(true, &fake, 30.0); + let gate = GatedFrames::default(); + let renderer: Arc = Arc::new(gate.clone()); + app.frames.set_remote(Some(renderer)); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 30.0).build_eframe(|_| app); + h.state_mut().use_manual_clock(); + h.run_steps(2); + gate.discard(&mut h); + let ahead = (h.state().frames.parallelism() * 2).max(4) as i64; + // A RAM preview of `ahead + 2` frames (a 64x36 frame is 9216 bytes; 90 % of the budget counts). + let fit = ahead + 2; + h.state().frames.set_budget(((fit as f64 + 0.5) * 9216.0 / 0.9) as usize); + h.state().frames.clear(); + let now = h.state().now(); + h.state_mut().play(now); + for _ in 0..90 { + gate.release_requested(&mut h); + step(&mut h); + if h.state().audio.is_some() { + break; + } + } + assert!(h.state().audio.is_some(), "the sound started"); + + // A stall, and enough stalls before it that the backoff is past the RAM preview's limit. + gate.discard(&mut h); + h.state().frames.clear(); + h.step(); + assert!(h.state().audio.is_none() && h.state().playback.audio_held, "a stall stops the sound"); + let stalled = frame(&h, cid); + h.state_mut().playback.stalls = 8; + // Rendering keeps up again, caching only what playback itself requests. + let started = (0..4 * fit).position(|_| { + gate.release_requested(&mut h); + kept_up(&mut h); + step(&mut h); + h.state().audio.is_some() + }); + assert!(started.is_some(), "the sound starts again on the frames the prefetch requests (RAM preview of {fit})"); + assert!(frame(&h, cid) < stalled + 4 * fit, "near where it stalled ({stalled}), not at the end of the preview"); +} + /// Pull `frames` stereo frames from the output, waiting for the feeder thread to mix them. fn pull(feed: &AudioFeed, frames: usize) -> Vec { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20);