Merge remote-tracking branch 'upstream/main' into aep-textdoc-3276

.github/workflows/release.yml

@@ -15,7 +15,14 @@ concurrency:
   cancel-in-progress: false
 
 jobs:
+  # Regenerates contributors/contributors.json for the release commit; every build bundles it.
+  credits:
+    permissions:
+      contents: read
+      pull-requests: read
+    uses: storytold/craft-common-actions/.github/workflows/refresh-contributors.yml@main
   release:
+    needs: credits
     permissions:
       contents: write
       packages: write
@@ -25,6 +32,7 @@ jobs:
       app-name: effectcraft
       app-title: EffectCraft
       version: ${{ inputs.version }}
+      contributors-artifact: ${{ needs.credits.outputs.artifact-name }}
     secrets:
       APPLE_CERTIFICATE: ${{ secrets.APPLE_CERTIFICATE }}
       APPLE_CERTIFICATE_PASSWORD: ${{ secrets.APPLE_CERTIFICATE_PASSWORD }}

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 {

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]

crates/ui-egui/src/lib.rs

@@ -215,6 +215,11 @@ pub struct Playback {
     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>,
+    /// 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 {

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<i64> {
+    (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<dyn RemoteFrames> = 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<dyn RemoteFrames> = 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<f32> {
     let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20);

docs/contributors.md

@@ -8,7 +8,8 @@ the AI models named in `Co-Authored-By` trailers.
 - Two files, both **compiled into the binary** by the UI crate's `build.rs` (nothing is read from
   disk or the network at run time; the web build has them too):
   - [`contributors/contributors.json`](../contributors/contributors.json): commit stats per GitHub
-    username. Never hand-edit.
+    username. Never hand-edit. Release builds regenerate it for the release commit
+    (`.github/workflows/release.yml`, `credits` job); the copy in the repository is not updated.
   - [`contributors/people.toml`](../contributors/people.toml): the names people chose for
     themselves. **Each contributor adds only their own entry**, in their own PR.
 - Each contributor gets one line: GitHub username (always), display name (opt-in, or their synced