diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 16630ed23..b41011178 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -52,12 +52,18 @@ const SCREENSHOT_TIMEOUT_S: f64 = 4.0; /// [`EffectcraftApp::editing`]). const EDIT_QUIET: f64 = 0.3; -/// Longest a preview with sound holds its first frame waiting for frames (seconds, #947). A frame -/// that never gets cached (larger than the RAM preview, which the size estimate can't always -/// tell) mustn't hold it for ever: the sound starts from the first frame anyway, and the preview -/// goes on as a stalled one does. +/// Longest a preview with sound holds its first frame, once that frame is cached, waiting for the +/// frames ahead (seconds, #947). A frame ahead that never gets cached mustn't hold it for ever: +/// the sound starts from the first frame anyway, and the preview goes on as a stalled one does. const STARTUP_WAIT: f64 = 2.0; +/// Longest a preview with sound holds its first frame in all (seconds, #947). The first frame +/// itself can take a while (a heavy comp, a busy machine), and [`STARTUP_WAIT`] counts only from +/// when it is cached; but one that never gets cached (larger than the RAM preview, which the size +/// estimate can't always tell) mustn't hold it for ever. The preview then goes on as a stalled +/// one does: its sound starts once the frames are cached. +const STARTUP_LIMIT: f64 = 10.0; + /// How many frames from the first a preview with sound waits for before it starts (#947): the /// prefetch's `ahead`, but no more than its `window` requests from the current frame (offsets /// `0..window`), and no more than fit in the RAM preview `budget` (90 %, as for the prefetch) at @@ -240,8 +246,11 @@ pub struct Playback { /// 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`]. pub starting: bool, - /// When the preview started (seconds): [`Playback::starting`] gives up after [`STARTUP_WAIT`]. + /// When the preview started (seconds): [`Playback::starting`] gives up after [`STARTUP_LIMIT`]. pub starting_since: f64, + /// When the first frame was cached while [`Playback::starting`] (seconds): the wait for the + /// frames ahead gives up [`STARTUP_WAIT`] after it. + 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, } @@ -1031,6 +1040,7 @@ impl EffectcraftApp { audio_held: false, starting: false, starting_since: now, + first_cached: None, shown_at: Default::default(), }; if !self.playback.caching { @@ -1307,18 +1317,28 @@ impl EffectcraftApp { let [w, h] = img.size(); startup_frames(ahead, window, self.frames.budget(), w.saturating_mul(h).saturating_mul(4)) }); - // Don't wait for frames the RAM preview can't hold (not even one frame fits), nor for - // longer than STARTUP_WAIT (a frame that never gets cached). - let give_up = (self.frames.budget() as f64) < self.frame_bytes(&c, scale) || now - self.playback.starting_since >= STARTUP_WAIT; + if need.is_some() && self.playback.first_cached.is_none() { + self.playback.first_cached = Some(now); + } + // Don't wait for frames the RAM preview can't hold (not even one frame fits), for the + // frames ahead longer than STARTUP_WAIT once the first is in, nor longer than + // STARTUP_LIMIT in all (a frame that never gets cached). + let give_up = (self.frames.budget() as f64) < self.frame_bytes(&c, scale) + || self.playback.first_cached.is_some_and(|t| now - t >= STARTUP_WAIT) + || now - self.playback.starting_since >= STARTUP_LIMIT; if !give_up && !need.is_some_and(|n| self.cached_ahead(&p, &series, cur, n)) { self.playback.waiting = true; ctx.request_repaint(); return; } self.playback.starting = false; - self.playback.audio_held = false; self.playback.waiting = false; - self.start_audio(fr.tick_of(cur)); + // 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. + if need.is_some() { + self.playback.audio_held = false; + self.start_audio(fr.tick_of(cur)); + } } // 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, diff --git a/crates/ui-egui/tests/ui_audio_scrub.rs b/crates/ui-egui/tests/ui_audio_scrub.rs index c01b6bb2d..a9b400662 100644 --- a/crates/ui-egui/tests/ui_audio_scrub.rs +++ b/crates/ui-egui/tests/ui_audio_scrub.rs @@ -377,6 +377,41 @@ fn startup_finishes_when_extended_viewer_frames_are_larger() { assert_eq!(audio_frame(&h, cid), Some(0), "from the first frame"); } +/// A first frame that takes longer than STARTUP_WAIT to be cached (a heavy comp, a busy machine; +/// here it can't be cached until the RAM preview grows): the start keeps holding it, then the +/// sound starts from it and keeps playing. It used to give up two seconds after Play, start the +/// sound on the uncached frame and stop it at once, so the opening played silently. +#[test] +fn a_slow_first_frame_keeps_its_opening_sound() { + let fake = Fake::default(); + let (mut h, cid, size) = extended_viewer(&fake); + let (comp_bytes, real_bytes) = (160 * 120 * 4, size[0] * size[1] * 4); + assert!(real_bytes > comp_bytes * 2, "Extended Viewer frames ({size:?}) are larger than the comp"); + h.state().frames.set_budget((comp_bytes + real_bytes) / 2); + h.state().frames.clear(); + let start = h.ctx.input(|i| i.time); + h.state_mut().play(start); + while h.ctx.input(|i| i.time) - start < 3.0 { + h.step(); + assert!(h.state().playback.starting, "it still holds the first frame"); + assert_eq!(frame(&h, cid), 0); + } + assert!(fake.0.lock().unwrap().0.is_none(), "no sound yet"); + h.state().frames.set_budget(1 << 30); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(60); + while h.state().playback.starting && std::time::Instant::now() < deadline { + h.step(); + std::thread::sleep(std::time::Duration::from_millis(1)); + } + assert!(!h.state().playback.starting, "the preview left its startup"); + assert_eq!(audio_frame(&h, cid), Some(0), "with its sound from the first frame"); + let (started, stopped) = { + let d = fake.0.lock().unwrap(); + (d.0.is_some(), d.1) + }; + assert!(started && !stopped, "the device started and kept playing"); +} + /// An output device whose start fails. #[derive(Clone, Default)] struct Broken; @@ -460,7 +495,8 @@ fn a_started_preview_follows_its_sound() { } /// A RAM preview that can't hold even one frame (none, or half of one): the start doesn't wait -/// for frames that can never be cached; the sound starts at once from the first frame. +/// for frames that can never be cached. The sound isn't started on a frame that can't be shown +/// (a stall would stop it at once): the preview goes on as a stalled one does. #[test] fn startup_does_not_wait_when_the_ram_preview_holds_no_frame() { for budget in [0, 9216 / 2] { @@ -474,16 +510,16 @@ fn startup_does_not_wait_when_the_ram_preview_holds_no_frame() { h.state_mut().play(now); h.step(); assert!(!h.state().playback.starting, "budget {budget}: no wait"); - assert!(fake.0.lock().unwrap().0.is_some(), "budget {budget}: the sound started"); - assert!(h.state().playback.playing); + assert!(fake.0.lock().unwrap().0.is_none(), "budget {budget}: no sound started and dropped"); + assert!(h.state().playback.playing && h.state().playback.audio_held, "budget {budget}: it goes on as a stalled preview"); } } /// A frame that never gets cached although the estimate says one fits (an Extended Viewer frame -/// larger than the whole budget): the start gives up after a short wait instead of holding the -/// first frame for ever, and the sound starts from it. +/// larger than the whole budget): the start gives up after STARTUP_LIMIT instead of holding the +/// first frame for ever, without starting the sound on a frame it can't show. #[test] -fn startup_gives_up_after_a_short_wait() { +fn startup_gives_up_on_a_first_frame_that_never_caches() { let fake = Fake::default(); let (mut h, cid, size) = extended_viewer(&fake); let (comp_bytes, real_bytes) = (160 * 120 * 4, size[0] * size[1] * 4); @@ -501,6 +537,7 @@ fn startup_gives_up_after_a_short_wait() { } let waited = h.ctx.input(|i| i.time) - start; assert!(!h.state().playback.starting, "it gave up waiting"); - assert!((1.9..4.0).contains(&waited), "after about two seconds ({waited:.2} s)"); - assert!(fake.0.lock().unwrap().0.is_some(), "and the sound started"); + assert!((9.9..12.0).contains(&waited), "after about ten seconds ({waited:.2} s)"); + assert!(fake.0.lock().unwrap().0.is_none(), "no sound started and dropped"); + assert!(h.state().playback.playing && h.state().playback.audio_held, "it goes on as a stalled preview"); }