Skip to content

perf(editor): keep rendered playback audio across pause and play - #2521

Open
richiemcilroy wants to merge 17 commits into
mainfrom
perf/bounded-playback-audio-prerender
Open

richiemcilroy wants to merge 17 commits into
mainfrom
perf/bounded-playback-audio-prerender

Conversation

@richiemcilroy

@richiemcilroy richiemcilroy commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

On main, every Play renders the whole timeline mix again from 1 s before the playhead, then wraps to the start (crates/editor/src/audio.rs). Pausing drops the buffer, so the next Play repeats the same work. This PR replays the previous render on the next Play, but only when that provably gives the same samples a fresh render would.

flowchart TD
  Play[Play at P] --> Fresh[spawn main's render from P - 1 s<br/>exactly as on main]
  Fresh --> Check{origin independent project,<br/>key matches cache, cache already covers<br/>P - 1 s .. P + 0.25 s?}
  Check -- yes --> Replay[read the cached render,<br/>stop the fresh one]
  Check -- no --> Keep[play the fresh render]
  Keep --> Retain{origin independent<br/>and start rounds down?}
  Retain -- yes --> Cache[fresh render becomes the cache entry;<br/>old entry freed once it finishes or parks]
  Retain -- no --> Done[main behaviour]
  Pause[Pause] --> Park[cached render parks with its state]
  Idle[2 min paused, export, memory pressure,<br/>output stream dropped] --> Free[cache released]
Loading

When a replay is allowed

render_is_origin_independent (audio.rs:1972) requires every stage between the sources and the buffer to be stateless, so each sample is a pure function of its timeline position:

  • 48 kHz stereo output, so the resampler only copies samples. Other rates resample with filter state that depends on where the render started.
  • No Studio Sound: the denoiser carries state from the render origin, so main gives position-dependent samples.
  • No retimed audio (unless that clip's audio is muted), no transitions, no fullscreen text holds.
  • Segment starts not on a sample rounding tie, and a duration that matches the timeline.

Otherwise the Play runs main's code unchanged and the cache entry is dropped.

Why replayed samples match a fresh render

main's pass 1 writes into slot floor(O * 48000) while the renderer starts at source sample round(O * 48000). So a render from origin O holds v(f + d) in every slot f of pass 1, where d is 0 or 1, and v(f) in the wrap pass. The cache only keeps renders with d = 0, which hold v(f) everywhere. A replay with d = 1 reads one frame ahead from its own pass 1 start (origin_shift, audio.rs:2008):

let mut idx = self.read_position + i;
if idx >= self.shift_from {
    idx += self.shift;
}

Other details

  • First audio: the fresh render is spawned before any cache work, so a cache miss starts exactly like main. A hit needs the cached render to already cover what main would have rendered at ready time (covers, audio.rs:1548), from 1 s before the playhead to 0.25 s after it.
  • Cache key (audio_project_key, audio.rs:1901): track and music identity, output format, duration, and the serialized audio inputs (audio, clips, timeline segments, transitions, music segments, hold windows). ProjectConfiguration and TimelineConfiguration are destructured without .., so adding a field fails to compile until someone decides whether it affects audio. Visual edits (zoom, background, captions, clip names) keep the cache.
  • Memory: while paused, the cached render is held, about 22 MB per minute of timeline. It is freed after 2 minutes paused (PLAYBACK_AUDIO_IDLE_RELEASE), before exports (release_playback_audio_caches), on macOS memory pressure, and when the output stream drops. A replaced entry is freed once the next render finishes or parks, never while playback is starting.

Results

M4 Max under shared load (load average 15 to 100), runs interleaved with main and a second main run as an A/A control, 20 to 24 rounds per case. Harness: PrerenderedAudioBuffer with 512-frame real-time pulls, plus AudioOutput::new_headless for the output path.

Replay (cache hit), Studio Sound off

Project Audio ready, median main → PR p95 main → PR CPU per replay main → PR
60 s 0.43 → 0.054 ms 0.55-2.3 → 0.08 ms 16 → 1.1 ms
10 min 0.61-0.73 → 0.06 ms 2.0-10.2 → 0.1-0.3 ms 130-160 → 3 ms (2 s play)

First Play, plays after an audio edit, and Studio Sound on (cache miss). These run main's code. Across 104 interleaved timing cases (audio ready and first audible block), none is significantly slower than main (Mann-Whitney p < 0.05). The A/A control misses main's own median and p95 more often than the PR does.

Audio: 2,696 plays compared sample for sample. They covered play from start, fractional start positions, pause and play, near end, seeks during play, every edit type in the editor (split, delete, reorder, trims, clip/mic/system volume and mute, stereo mode, offsets, isolation, speed in three modes, transition, text hold, music and music volume, Studio Sound toggle, and visual edits), Studio Sound on and off, and 60 s and 10 min projects. All are identical to main, except three seeks into audio a Studio Sound render had not reached yet. There, both sides play main's code and differ only in how long the silence lasts, which varies run to run on main too (A/A: 853 vs 75 ms, 203 vs 1003 ms).

Memory while paused (Studio Sound off): 60 s 19 → 40 MB, 10 min 308 → 528 MB, back to main after the idle release. With Studio Sound on nothing is cached and footprint matches main.

RetriggerView in GreptileConfidence Score: 5/5

The latest changes appear safe to merge, with no new actionable issues found.

Summary

This PR keeps eligible rendered audio across pause and play. The latest change prevents the render worker from missing a resume notification.

  • Playback reuses rendered audio when you pause and press Play again.

Reviews (9) · Last reviewed commit: "fix(editor): never miss a replay's wake-..." · Reviewed by Greptile

@vercel

vercel Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
cap-status Ready Ready Preview Oct 20, 2026 12:29pm UTC

Comment thread crates/editor/src/audio.rs Outdated

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

View 2 other findings in Capy.

Open in Capy

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

View 1 other finding in Capy.

Open in Capy

@richiemcilroy richiemcilroy changed the title perf(editor): render playback audio a bounded window ahead of the playhead perf(editor): keep rendered playback audio across pause and play Oct 14, 2026
@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

View 3 other findings in Capy.

Open in Capy

Comment thread crates/editor/src/audio.rs Outdated
Comment thread crates/editor/src/audio.rs Outdated
Comment thread crates/editor/src/audio.rs Outdated
@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

View 2 other findings in Capy.

Open in Capy

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

Open in Capy

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

View 1 other finding in Capy.

Open in Capy

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

View 1 other finding in Capy.

Open in Capy

@richiemcilroy

Copy link
Copy Markdown
Member Author

@greptileai review

Comment thread crates/editor/src/audio.rs
Comment on lines +2135 to +2146
let sources = cache.is_some().then(|| (segments.clone(), music.clone()));
let fresh = ProgressiveAudioBuffer::new(
segments,
music,
project,
output_info,
duration_secs,
start_playhead_secs,
estimated_output_samples,
))
);
PrerenderedAudioBufferMode::Progressive(match (cache, sources) {
(Some(cache), Some((segments, music))) => fresh.reuse_or_retain(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Cached plays still allocate

Even a cache hit first allocates another full-timeline buffer and starts a render thread. reuse_or_retain() checks the cache only after ProgressiveAudioBuffer::new() returns, then drops that fresh render on a hit. Every replay still pays for the large allocation and thread startup, and the old and new allocations overlap.

Look up a usable cached render before constructing the fresh one.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/editor/src/audio.rs
Line: 2135-2146

Comment:
**Cached plays still allocate**

Even a cache hit first allocates another full-timeline buffer and starts a render thread. `reuse_or_retain()` checks the cache only after `ProgressiveAudioBuffer::new()` returns, then drops that fresh render on a hit. Every replay still pays for the large allocation and thread startup, and the old and new allocations overlap.

Look up a usable cached render before constructing the fresh one.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found no issues.

Open in Capy

…-audio-prerender

# Conflicts:
#	crates/editor/src/lib.rs

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capy found 1 potential issue (1 high).

Open in Capy

struct ProgressiveAudioBuffer<T: FromSampleBytes> {
timeline: Arc<ProgressiveTimeline>,
ready_rx: std::sync::mpsc::Receiver<()>,
render: Arc<ProgressiveRender>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged-in rendered() reads removed self.timeline field

High Severity

The defect lives in main's newly merged test-hook fn rendered() at crates/editor/src/audio.rs:2495, not at this anchor: its first line does let timeline = &self.timeline;, but this PR reworked the struct this impl targets (anchor line 2058) so ProgressiveAudioBuffer now holds render: Arc<ProgressiveRender> and has no timeline field (confirmed: no Deref or timeline() accessor exists for it). Main added rendered() and its dispatch path after the PR's base — PrerenderedAudioBuffer::wait_until_rendered (audio.rs:2223) calls buffer.rendered(samples), and OfflineAudioSink::pull (audio_output.rs:1344) calls wait_until_rendered — and the merge of origin/main (b91e378) textually interleaved the new fn into the PR's rewritten impl without adapting the field access, so cap-editor no longer compiles with the test-hooks feature ('no field timeline on type ProgressiveAudioBuffer<T>'). test-hooks is not an edge configuration: crates/verify/Cargo.toml:34 enables it for the whole verify crate, and CI's cargo clippy --workspace --exclude cap-desktop --all-features --locked -- -D warnings (.github/workflows/ci.yml:234) turns it on workspace-wide, so the head fails the CI gate and every OfflineAudioSink-based test is unbuildable. Fix: change rendered()'s first line to let timeline = &self.render.timeline; — every member it reads (samples, complete, primary_end, wrap_end, primary_start, stop) exists on the PR's ProgressiveTimeline (audio.rs:1476+), exactly as the adjacent set_playhead (audio.rs:2521) already accesses it. The second merged-in rendered() copy for StreamingAudioBuffer (audio.rs:2736) is unaffected because that struct still has a timeline field.

Open in Capy

This branch was successfully deployed

2 active deployments
Preview – cap-web — b91e3786 Deployed Oct 20, 2026 by vercel[bot]
Preview – cap-status — b91e3786 Deployed Oct 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant