Skip to content

perf(audio): async decode + PCM/waveform caching (Slice 1, #151)#159

Merged
thewrz merged 8 commits into
mainfrom
perf/issue-151-decode-caching
Jun 24, 2026
Merged

perf(audio): async decode + PCM/waveform caching (Slice 1, #151)#159
thewrz merged 8 commits into
mainfrom
perf/issue-151-decode-caching

Conversation

@thewrz

@thewrz thewrz commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Why

Firing playback had a subtle, length-dependent lag: play_sound_entry decoded
the entire file to PCM synchronously inside Iced's update() loop on every
press, so longer files hitched the UI. The same hot path also recomputed the
waveform envelope, read the playhead length off a full decode, and copied the
whole PCM to apply per-sound volume.

What (Slice 1 of #151)

Move the decode off the UI thread and cache aggressively, in a memory-conscious way:

  • audio::store::AudioStore — a byte-capped LRU of decoded Arc<CachedPcm>
    (default 256 MB; a hot re-fired sound stays resident, cold ones evict; a lone
    oversized entry is kept) plus the in-memory waveform-envelope map.
  • Async decode — a press returns an iced::Task that decodes on a blocking
    thread and yields Message::Decoded; a warm cache hit fires synchronously
    (instant replay). The decode is keyed by the play_generation token so a
    superseded press is dropped, and a play cancelled mid-decode (StopAll) is
    dropped via the playing check.
  • Per-sound volume → engine gainPlaybackState applies master * gain,
    so the canonical pre-volume Arc is shared with the engine with no per-play copy.
  • app.rsapp/mod.rs + app/playback.rs — the new play-coordination
    logic lives in a focused module rather than growing the Application file
    (honors the CLAUDE.md "don't grow app.rs" override).

Result: fast clicks, instant warm replays, waveforms computed once then cached.

Reviews

Two adversarial passes (an in-workflow reviewer + Codex GPT-5.5 xhigh). Fixed a
CRITICAL (StopAll resurrecting a cold decode), a HIGH (per-sound boost >1.0
clamped to unity), and a HIGH regression the first StopAll fix introduced
(stale PlaybackStarted after StopAll) — each pinned by a regression test.
Remaining narrow async-decode edge cases are tracked in #152 (decision recorded:
keep the snappy optimistic highlight; the real guard is the background pipeline
#158).

Testing

  • cargo test — 487 lib + 24 integration, incl. new AudioStore LRU,
    engine-gain, async-ordering, and StopAll-resurrection regression tests
  • cargo clippy --all-targets -- -D warnings clean (default and
    --features pipewire-test); cargo fmt --all --check clean
  • Manual: UI fast, replay fast, waveforms generate on first fire then cache
  • CI green

Design + plan: docs/superpowers/specs/2026-06-23-playback-decode-and-waveform-caching-design.md,
docs/superpowers/plans/2026-06-23-async-decode-pcm-cache.md. Slice 2 (persisted
envelope) is a follow-up.

🤖 Co-authored by Claude Opus 4.8 (1M context). Closes #151.

thewrz and others added 8 commits June 24, 2026 10:29
Design for eliminating the length-dependent click-to-fire lag: move the
synchronous full decode off the Iced update loop into a generation-keyed
async Task, add a byte-capped LRU decoded-PCM cache, move per-sound volume
into the engine (share one canonical Arc), and persist the waveform envelope
to $XDG_DATA_HOME keyed by a (size, mtime) fingerprint.

Two sequenced slices; builds on the #149/#150 generation token. Refs #151.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phased TDD plan for Slice 1: AudioStore (byte-capped PCM LRU + envelope map),
per-sound volume as engine gain (shared canonical Arc), and async decode off
the UI thread via a generation-keyed Task with stale-drop. Slice 2 (persisted
envelope) gets its own plan once this lands. Refs #151.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two real issues the review caught in the async-decode work:

- CRITICAL: StopAll did not invalidate an in-flight cold-cache decode, so a
  sound the user explicitly stopped was resurrected when its Message::Decoded
  landed (the stale generation still matched). Bump play_generation in the
  StopAll handler so the existing generation guard drops the stale decode. The
  bump is scoped to StopAll — the genuine-end and decode-error teardowns have
  no in-flight decode for the current generation, and bumping there would break
  the #111 multi-event drain reconciliation. Pinned by
  stopall_mid_decode_does_not_resurrect_playback.

- HIGH: per-sound volume boost above unity was silently lost — PlaybackState
  clamped the new gain to [0,1] while the slider/sound_meta domain is [0,2], so
  boosted sounds got quieter than the pre-#151 sample-scaling path. Clamp gain
  to MAX_PER_SOUND_GAIN (2.0). Pinned by
  fill_buffer_preserves_per_sound_boost_above_unity.

Also: note the O(n) eviction scan in AudioStore as an accepted cold-path cost,
and correct the play_generation doc to its #149+#151 roles.

Refs #151

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The async-decode slice added the play-coordination logic (request_play,
start_playback, the Message::Decoded landing) to app.rs, which the project
override forbids growing ("app.rs delegates to module APIs; split first").

Convert app.rs -> app/mod.rs and move that logic into a focused
app/playback.rs child module (a child module still reaches HonkHonk's private
fields, so the methods stay methods on HonkHonk). The Decoded arm in update()
now delegates to self.handle_decoded(...). No behavior change.

mod.rs shrinks ~95 lines; the new coordination logic lives in a 120-line
module instead of inflating the Application file.

Refs #151

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n bump (#151)

The first review fix bumped play_generation in StopAll to drop an in-flight
cold decode. A second adversarial pass found that bump regressed StopAll: a
stale PlaybackStarted draining just after StopAll re-set `playing`, and the
genuine generation-stamped Finished could no longer match to clear it (bumped
generation), so the highlight stuck on a stopped sound.

Replace the StopAll bump with a `playing` check in handle_decoded: apply a
decode only when `generation == play_generation` AND `playing == Some(id)`.
`playing` is the real "is this play still wanted" signal — it is None after
StopAll — so a cancelled cold decode is dropped without disturbing the #149
Started/Finished reconciliation. Fixes the original resurrection and the
regression; stopall_mid_decode_does_not_resurrect_playback now passes via the
playing check, and all #111/#149 suites stay green.

Refs #151

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@thewrz, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 39 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c1a06720-1528-4301-93f4-3da4b2d56683

📥 Commits

Reviewing files that changed from the base of the PR and between 2ecacc1 and 99a92f3.

📒 Files selected for processing (9)
  • docs/superpowers/plans/2026-06-23-async-decode-pcm-cache.md
  • docs/superpowers/specs/2026-06-23-playback-decode-and-waveform-caching-design.md
  • src/app/mod.rs
  • src/app/playback.rs
  • src/audio/engine.rs
  • src/audio/mod.rs
  • src/audio/playback.rs
  • src/audio/store.rs
  • tests/pipewire_integration.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/issue-151-decode-caching

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thewrz

thewrz commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

🔍 Codex (GPT-5.5, xhigh) review — backup gate (CodeRabbit hit its org review rate-limit this window, so its green check is a no-op, not a real review; Codex served as the review gate per the workflow).

Codex surfaced 2 × [P2] findings against src/app/playback.rs. Both verified against the code — and both are already-tracked, deliberately-deferred edge cases of the Slice-1 async-decode model, not new regressions:

No new in-scope findings; nothing under-emphasized on re-scan. The PR body already references #152/#158, so the deferrals are linked. No code change needed.

@thewrz

thewrz commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

🧹 Backlog grooming (post-#159 pickup) — proposal only; no board moves made.

Ready already holds 4 vetted items (#13, #141, #144, #147 — above the ~3 healthy-queue bar), so the queue does not strictly need topping up. With #151 about to leave In-progress, two strong Backlog candidates are worth surfacing for the next pickup:

PROMOTE (vetted Ready-bar pass)

HOLD (needs a human call / prerequisite first)

On confirmation I can gh-project-move 142 "Ready" / gh-project-move 145 "Ready". No destructive changes proposed.

@thewrz thewrz merged commit 59cc623 into main Jun 24, 2026
7 checks passed
@thewrz thewrz deleted the perf/issue-151-decode-caching branch June 24, 2026 18:05
@thewrz

thewrz commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Codex (GPT-5.5, xhigh) adversarial review — second-reviewer gate

CodeRabbit hit its org fair-usage review limit on this PR (its green check is a rate-limit ack, not a real review), so per the review-remote-pr workflow Codex ran as the review gate. It surfaced two [P2] findings; both are real but already tracked and deliberately deferred out of Slice 1 — neither is a new regression introduced here, and neither blocks this PR.

[P2] Cold decode failure desyncs UI from a still-playing sound — declined (tracked)

A stop_before=false tile press commits playing = Some(B) optimistically (#111 instant highlight) before the async decode resolves; if B is corrupt/undecodable, handle_decoded clears UI state while A still plays in the engine.

Declining — deliberate design choice, already tracked as the HIGH item in #152. The owner decision (this session) is to keep the snappy optimistic highlight; the real guard is the background pre-processing pipeline in #158, which validates/decodes ahead of the click so a cold press of a broken file is caught before playback. No code change in this slice.

[P2] Cache not invalidated when a file changes at the same path — declined (tracked)

sound.id is a hash of the path only (path_to_id); the PCM and envelope caches key on it with no size/mtime/content fingerprint, so replacing/editing a file at the same path mid-session serves stale audio + waveform until eviction or restart.

Declining — deliberate Slice-1 scope. Content-fingerprint invalidation is exactly what #158 (background clean/fix/hash/waveform pipeline) provides; the in-session staleness is captured in #152 (MEDIUM rescan-invalidation item, now annotated with this sharper same-path-replacement framing). No code change in this slice.

Net: 0 blocking findings. All CI green; CodeRabbit rate-limited (self-resolving throttle), so Codex was the review gate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(audio): async decode + PCM/waveform caching to kill click-to-fire lag

1 participant