Preview: a slow first frame keeps the opening sound (#947)

Under load the first frame could take longer than STARTUP_WAIT to be
cached. The start then gave up, started the sound on that uncached
frame, and the stall path stopped it in the same step, so the opening
played silently.

STARTUP_WAIT (2 s) now counts from when the first frame is cached, and
only bounds the wait for the frames ahead. A new STARTUP_LIMIT (10 s
from Play) still ends the wait for a first frame that never gets cached.
The sound is never started on a frame that isn't cached: the preview
goes on as a stalled one does, and the sound starts once frames are in.

Tests (ui_audio_scrub): a first frame held uncached for 3 s still gets
its sound from frame 0, and the device keeps playing (fails on the
previous commit). The two give-up tests now check that no sound is
started and dropped.

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<f64>,
     /// When the last frames were shown (seconds, the last second's worth): the achieved rate.
     pub shown_at: std::collections::VecDeque<f64>,
 }
@@ -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,

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");
 }