diff --git a/crates/ui-egui/src/app/playback.rs b/crates/ui-egui/src/app/playback.rs index 7cda624e9..d40bc69ff 100644 --- a/crates/ui-egui/src/app/playback.rs +++ b/crates/ui-egui/src/app/playback.rs @@ -44,7 +44,6 @@ 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 c8d5cd938..50638b708 100644 --- a/crates/ui-egui/src/app/prefetch.rs +++ b/crates/ui-egui/src/app/prefetch.rs @@ -27,24 +27,12 @@ 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) +/// Whether frame `f` comes after the current frame `cur` and before the sound's frame `target` in +/// play order. A looping preview whose sound is at a lower frame than the picture has wrapped +/// to the loop's start, so the frames up to the end and then from the start count; otherwise +/// (and always without a loop) only the frames strictly between them do. +fn between_picture_and_sound(cur: i64, f: i64, target: i64, looping: bool) -> bool { + if target >= cur || !looping { cur < f && f < target } else { f > cur || f < target } } impl EffectcraftApp { @@ -62,11 +50,15 @@ impl EffectcraftApp { let video = plan.is_none_or(|p| p.video); // Prefetch ahead. let cur = fr.frame_at(self.session.time()); + // While the sound drives a video preview, frames are rendered and kept ahead of what it + // plays rather than of the picture, which drops frames behind it when rendering can't + // keep up (#2473). + let lead = if video { self.sound_frame(fr, wa, wb, plan.map_or(1, |p| p.step)).unwrap_or(cur) } else { cur }; let ahead = self.prefetch_ahead(ctx, now); let queued = self.frames.inflight(); let series = self.frame_series(cid, scale); // The preview's position orders prefetch and eviction (#595). - let playhead = plan.filter(|p| p.video).map(|p| frames::Playhead { series, cur, start: p.start, end: p.end, step: p.step, looping: p.looping }); + let playhead = plan.filter(|p| p.video).map(|p| frames::Playhead { series, cur: lead, start: p.start, end: p.end, step: p.step, looping: p.looping }); self.frames.set_playhead(playhead); let pass = PlaybackPass { ctx, @@ -89,7 +81,7 @@ impl EffectcraftApp { } let step = plan.map_or(1, |p| p.step); let fit_ahead = self.frames_that_fit(&c, scale) - 1; - self.prefetch_playback(&pass, fit_ahead); + self.prefetch_playback(&PlaybackPass { cur: lead, ..pass }, fit_ahead); self.pump_meters(ctx, now); if !self.playback.playing { self.cache_when_idle(ctx, &c, &series, now, input_active, (wa, wb)); @@ -258,8 +250,8 @@ impl EffectcraftApp { } self.playback.starting = false; self.playback.waiting = false; - // Not on a frame that isn't cached: the stall below would stop the sound at once. The - // preview goes on as a stalled one does, and the sound starts once frames are cached. + // Not on a frame that isn't cached: the picture couldn't show where the sound starts. The + // preview goes on silently, and the sound starts once frames are cached. if need.is_some() { self.playback.audio_held = false; self.start_audio(fr.tick_of(cur)); @@ -267,17 +259,16 @@ impl EffectcraftApp { false } - /// 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 - /// (twice as many frames after each stall, see [`held_audio_frames`]), else the rest of the - /// preview. + /// Held sound starts once the frames ahead are cached (see `Playback::audio_held`): the + /// prefetch's `ahead` when the silent preview keeps real time, 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 }; - let need = held_audio_frames(ahead, held_audio_cap(ahead, fit), self.playback.stalls); + // No more than the prefetch requests: offsets below `fit - 1` (`advance_playback`). + let need = ahead.min(fit - 1).max(1); 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; @@ -312,21 +303,48 @@ impl EffectcraftApp { self.playback.frame_shown(now); } } else { - // Rendering can't keep up: rather than skip frames, the sound stops and every - // 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); + // Rendering can't keep up: the sound plays on and the picture drops frames, showing + // the latest one rendered up to the sound, while `advance_playback` renders ahead of + // the sound (#2473). Stopping the sound instead and starting it again once frames were + // cached chopped it into bursts that played as distortion (#3260). self.playback.waiting = true; - // The rate measured before the stall no longer says it keeps up. - self.playback.shown_at.clear(); - self.playback.start_wall = now; - self.playback.start_frame = cur; + if let Some(f) = self.latest_cached_before(pass, target).filter(|&f| f != cur) { + self.session.set_time(fr.tick_of(f)); + self.playback.frame_shown(now); + } } ctx.request_repaint(); true } + /// The frame under the audible sample while the sound drives the preview, kept in the + /// preview's range and on its frame step; `None` without sound. + fn sound_frame(&self, fr: effectcraft_engine::time::FrameRate, wa: i64, wb: i64, step: i64) -> Option { + let a = self.audio.as_ref()?; + let step = step.max(1); + Some(wa + (audio::frame_of_sample(a.clock(), a.rate, fr).clamp(wa, wb) - wa).div_euclid(step) * step) + } + + /// The last cached frame after the current one and before `target`, in play order: what a + /// preview behind its sound shows (`None` when no such frame is cached). The walk stops at + /// the first frame that isn't before the sound's, which it can step over without meeting + /// (a picture off the plan's frame step) or never reach (a sound behind the picture). + fn latest_cached_before(&self, pass: &PlaybackPass<'_>, target: i64) -> Option { + let PlaybackPass { plan, series, cur, window, looping, .. } = *pass; + let cached = self.frames.cached_frames(&series); + let mut best = None; + for k in 1..=(window.last - window.first + 1).max(1) { + let Some(f) = plan.map_or(Some(cur + k), |p| p.frame_after(cur, k)) else { break }; + if !between_picture_and_sound(cur, f, target, looping) { + break; + } + if cached.binary_search(&f).is_ok() { + best = Some(f); + } + } + best + } + /// The wall clock advances the preview when no audio device drives it, and shows the target /// frame (or holds when it is not cached yet). fn advance_frame_clock(&mut self, pass: &PlaybackPass<'_>, step: i64) { @@ -487,31 +505,27 @@ impl EffectcraftApp { #[cfg(test)] mod startup_tests { - use super::{held_audio_cap, held_audio_frames, startup_frames}; + use super::{between_picture_and_sound, startup_frames}; - /// After each stall the held sound waits for twice as many frames ahead, up to its cap - /// (#2473, #3260). + /// The frames a preview behind its sound may show are only those before the sound's frame in + /// play order, so the picture is never ahead of the sound (#2473). #[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"); + fn a_dropped_frame_preview_never_shows_past_the_sound() { + // Sound ahead of the picture: only the frames between them. + assert!(between_picture_and_sound(10, 12, 14, false)); + assert!(!between_picture_and_sound(10, 14, 14, false), "not the sound's own frame"); + assert!(!between_picture_and_sound(10, 15, 14, true), "nor one past it, stepped over off the grid"); + assert!(!between_picture_and_sound(10, 10, 14, true), "nor the current frame"); + // A looping preview whose sound wrapped to the start: the end of the loop, then its start. + assert!(between_picture_and_sound(90, 95, 3, true)); + assert!(between_picture_and_sound(90, 1, 3, true)); + assert!(!between_picture_and_sound(90, 3, 3, true)); + assert!(!between_picture_and_sound(90, 50, 3, true), "the wrapped walk stops before reaching the picture again"); + // Without a loop, a sound behind the picture allows nothing ahead of it. + assert!(!between_picture_and_sound(90, 95, 3, false)); + assert!(!between_picture_and_sound(90, 1, 3, false)); + // The sound on the picture's frame: nothing. + assert!(!between_picture_and_sound(10, 11, 10, true)); } /// The frames a preview with sound waits for are always requested and fit together (#947). diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e641da3d4..e84ef79dd 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -197,16 +197,18 @@ pub struct Playback { pub restore_max: Option>, /// Audio-only previews: the frame under the audible sample (the current time stays put). pub audio_frame: Option, - /// The preview's sound waits for frames: while the frames ahead aren't cached, every frame - /// shows as it renders, silently; the sound starts once the rest of the preview (or what - /// fits in the RAM preview) is cached, so it never plays over skipped frames (#103), or - /// sooner once the silent preview keeps real time with the frames ahead cached (#208). + /// The preview's sound waits for frames: a preview whose start gave up on frames that weren't + /// cached shows every frame as it renders, silently, and starts the sound once the frames + /// ahead are cached while it keeps real time (#208), or the rest of the preview is. Once the + /// sound plays it goes on through frames that aren't cached, and the picture drops frames to + /// follow it (#2473). pub audio_held: bool, /// Starting a preview with sound (#947): the playhead stays on the first frame while the /// frames ahead render (the prefetch window, at most what fits in the RAM preview), then /// sound and picture start together from it. Without this the picture played on silently /// while they rendered and the sound started at the later frame, so the opening was lost. - /// A stall later on still falls back to [`Playback::audio_held`]. + /// Once the sound plays, a stall no longer stops it: the picture drops frames to follow it + /// (#2473). pub starting: bool, /// When the preview started (seconds): [`Playback::starting`] gives up after `STARTUP_LIMIT`. pub starting_since: f64, @@ -215,11 +217,6 @@ 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 6ea74ef35..80936042a 100644 --- a/crates/ui-egui/tests/it/ui_audio_scrub.rs +++ b/crates/ui-egui/tests/it/ui_audio_scrub.rs @@ -74,16 +74,6 @@ 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 {} @@ -215,7 +205,7 @@ fn step(h: &mut Harness<'_, EffectcraftApp>) { /// Starting a preview with sound before its frames are cached (#947): the playhead holds the /// first frame while the frames ahead render, then sound and picture start together from it (the /// sound used to start at a later frame, losing the opening). A frame that isn't cached later on -/// still stops the sound rather than being skipped (#103). +/// no longer stops the sound (#2473). #[test] fn preview_with_audio_holds_the_first_frame_and_sounds_from_it() { let fake = Fake::default(); @@ -247,11 +237,12 @@ fn preview_with_audio_holds_the_first_frame_and_sounds_from_it() { assert!(h.state().audio.is_some() && fake.0.lock().unwrap().0.is_some(), "the sound started once the frames ahead were cached"); assert_eq!(audio_frame(&h, cid), Some(0), "from the first frame, not a later one"); assert!(!h.state().playback.starting); - // A frame that isn't cached stops the sound rather than being skipped. + // A frame that isn't cached keeps the sound playing; the picture waits for frames (#2473). gate.discard(&mut h); h.state().frames.clear(); h.step(); - assert!(h.state().audio.is_none() && h.state().playback.audio_held && h.state().playback.playing); + assert!(h.state().audio.is_some() && !h.state().playback.audio_held && h.state().playback.playing, "the sound plays on"); + assert!(h.state().playback.waiting, "the picture waits for its frame"); } /// A long (30 s) preview sounds once the frames ahead of its first frame are cached, not the @@ -279,24 +270,12 @@ 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. +/// A preview whose rendering falls behind its sound keeps the sound playing (#2473, #3260): the +/// picture shows the latest frame rendered up to the sound, dropping the others, and frames are +/// rendered ahead of the sound rather than of the picture. Stopping the sound at every frame that +/// wasn't cached chopped it into bursts. #[test] -fn each_stall_doubles_the_frames_ahead_the_sound_waits_for() { +fn a_preview_behind_its_sound_drops_frames_and_keeps_the_sound() { let fake = Fake::default(); let (mut app, cid) = setup_long(true, &fake, 30.0); let gate = GatedFrames::default(); @@ -309,101 +288,37 @@ fn each_stall_doubles_the_frames_ahead_the_sound_waits_for() { 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)); + gate.render(&mut h, cid, &(0..ahead).collect::>()); step(&mut h); assert!(h.state().audio.is_some(), "the sound started"); - assert_eq!(h.state().playback.stalls, 0); + let feed = fake.0.lock().unwrap().0.clone().expect("the output has the preview's feed"); - // 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() { + // Frames 0..ahead and `behind` are cached; the sound plays on to `sound`, which isn't. + let (behind, sound) = (ahead + 2, ahead + 4); + gate.render(&mut h, cid, &[behind]); + // 48 kHz at 30 fps: 1600 samples a frame. Halfway into `sound`. + let samples = sound as u64 * 1600 + 800; + for _ in 0..10_000 { + if feed.consumed() >= samples { break; } + pull(&feed, (samples - feed.consumed()).min(1600) as usize); } - assert!(h.state().audio.is_some(), "the sound started"); + assert!(feed.consumed() >= samples, "the device took the sound up to frame {sound}"); + step(&mut h); + assert!(h.state().audio.is_some() && !h.state().playback.audio_held, "the sound plays on"); + assert_eq!(frame(&h, cid), behind, "the picture shows the latest frame rendered up to the sound"); + assert!(h.state().playback.waiting); + while h.state().frames.dispatch_remote() != 0 {} + let requested: Vec = gate.pending.lock().unwrap().iter().map(|p| p.0).collect(); + assert!(requested.contains(&sound), "frames are rendered from the sound's frame on: {requested:?}"); + assert!(!requested.contains(&ahead), "not the frames the picture dropped: {requested:?}"); - // 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"); + // The sound's frame renders: the picture catches up with it. + gate.render(&mut h, cid, &[sound]); + step(&mut h); + assert_eq!(frame(&h, cid), sound, "the picture is back on the sound's frame"); + assert!(!h.state().playback.waiting && h.state().audio.is_some()); } /// Pull `frames` stereo frames from the output, waiting for the feeder thread to mix them. @@ -721,7 +636,7 @@ impl AudioDevice for Playing { } /// Once started, the sound's clock moves the picture on from the first frame, and a frame that -/// isn't cached still stops the sound rather than being skipped. +/// isn't cached doesn't stop the sound (#2473). #[test] fn a_started_preview_follows_its_sound() { let fake = Fake::default(); @@ -744,7 +659,7 @@ fn a_started_preview_follows_its_sound() { h.state().frames.set_budget(0); h.state().frames.clear(); step(&mut h); - assert!(h.state().audio.is_none() && h.state().playback.audio_held && h.state().playback.playing, "a stall stops the sound"); + assert!(h.state().audio.is_some() && !h.state().playback.audio_held && h.state().playback.playing, "a stall doesn't stop the sound"); *device.0.lock().unwrap() = true; }