Skip to content

fix(desktop): suspend idle poof audio context - #2907

Open
yinkev wants to merge 3 commits into
block:mainfrom
yinkev:fix/suspend-idle-poof-audio
Open

yinkev wants to merge 3 commits into
block:mainfrom
yinkev:fix/suspend-idle-poof-audio

Conversation

@yinkev

@yinkev yinkev commented Jul 25, 2026

Copy link
Copy Markdown

Summary

  • disconnect each poof sound's AudioBufferSourceNode and GainNode when playback ends, keeping the WebAudio graph flat
  • suspend the singleton AudioContext 1.5 seconds after the final sound and cancel that idle suspension when another poof starts
  • serialize replay against an already in-flight suspend() so rapid clicks resume cleanly instead of starting into a newly suspended context
  • preserve the existing HTML audio fallback when graph creation, context resume, or source start fails, while rearming idle suspension after a failed graph setup

This stays focused on the measured lifecycle leak. Sample-rate policy, offline decoding, and a native-audio rewrite remain separate work.

Related issue

Fixes #2868.

Closest existing PR: #2804 changes the poof asset codec for Linux decoding; this PR is orthogonal and fixes the context/node lifetime after playback.

Testing

  • pnpm check
  • pnpm typecheck
  • focused poof lifecycle tests — 8 passed
  • full desktop unit suite — 3,512 passed
  • desktop production build — passed

No visual changes.

Signed-off-by: kyinhub <kevinpyin@gmail.com>
@yinkev
yinkev requested a review from a team as a code owner July 25, 2026 23:13
@yinkev

yinkev commented Jul 25, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 9dc4602964

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: kyinhub <kevinpyin@gmail.com>
@yinkev

yinkev commented Jul 25, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 3675d5059c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@FabianHertwig

Copy link
Copy Markdown

Thanks for picking this up — the node-disconnect plus debounced-suspend logic reads right for the lifecycle the issue describes. Flagging one path I don't think it covers, based on macOS measurements I've just posted on #2868.

scheduleSuspend() is only reachable from play() — the graph-creation catch and cleanup() when the last playback ends. That holds if the context can only reach running via playPoofSound()resume().

On macOS it can't: when the embedder allows autoplay (the webview default Buzz takes), a freshly constructed AudioContext auto-resumes to running on its own, with no user gesture and nothing played. I isolated it in a minimal WKWebView host — A/B table and log trace are in #2868. The short version from a real launch:

13:11:25.996  RemoteAudioDestination::RemoteAudioDestination(…)   ← the AudioContext
13:11:26.004  coreaudiod  Take: …BuiltInSpeakerDevice.context.preventuseridlesleep on behalf of 79092
13:11:26.035  RemoteAudioDestination::start(…)                    ← went running by itself

Since PoofBurstProvider's mount effect calls loadPoofAudioBuffer()getPoofAudioContext() at startup just to decode the asset, that means the context is constructed on every launch, goes running moments later, play() is never called, scheduleSuspend() never runs, and the output device stays pinned for the whole session. Measured here: no idle sleep for 12.5 h and battery 38% → 1% overnight, without a single poof.

One wrinkle if you do arm the suspension at construction: a plain scheduleSuspend(context) there would race the auto-resume — the timer fires at 1.5 s, sees state === "suspended", and returns; the context then flips to running at ~2 s and stays. Keying on statechange sidesteps the ordering:

// in createPoofAudioPlayer(), alongside play()
function armIdleSuspend(context: AudioContext) {
  // WebKit returns a suspended context but auto-resumes it when the embedder
  // allows autoplay, so an idle context can reach "running" with no playback.
  // Re-arm on every transition instead of assuming play() is the only path.
  context.addEventListener("statechange", () => {
    if (context.state === "running" && activePlaybacks === 0) {
      scheduleSuspend(context);
    }
  });
  if (context.state === "running") scheduleSuspend(context);
}

return { armIdleSuspend, play };

called once where the singleton is created:

function getPoofAudioContext() {
  try {
    if (!poofAudioContext) {
      poofAudioContext = new AudioContext({ latencyHint: "interactive" });
      poofAudioPlayer.armIdleSuspend(poofAudioContext);
    }
    return poofAudioContext;
  } catch {
    return null;
  }
}

This composes with what you already have rather than duplicating it: play() calls cancelPendingSuspend() first, and activePlaybacks is incremented before resume(), so the statechange from a real poof can't schedule a suspension mid-playback; cleanup() still owns the post-playback case; and suspend() itself fires one more statechange that no-ops. The first poof after startup finds a suspended context, which resumeThenStart() already handles.

The alternative is to not construct a hardware context at startup at all — decode via OfflineAudioContext, which never binds an output device, and create the real context on the first poof. That's the offline-decoding work you scoped out of this PR, so the statechange arming above is probably the smaller move here.

Happy to test either against my setup — the assertion is directly observable via pmset -g assertions | grep -A2 BuiltInSpeakerDevice, so it's a clean pass/fail.

@yinkev

yinkev commented Jul 26, 2026

Copy link
Copy Markdown
Author

Thanks for the concrete WebKit trace. Addressed in the latest commit: the singleton now arms an idle-suspend statechange listener when the AudioContext is created, so an autoplay-enabled context that transitions to running without any poof is suspended after the debounce. Active playback still cancels the timer before resume, and the listener only arms while there are zero active playbacks. Added a focused suspended → auto-running regression; the poof lifecycle suite passes 8/8.

Signed-off-by: Kevin Yin <182213728+yinkev@users.noreply.github.com>
@yinkev
yinkev force-pushed the fix/suspend-idle-poof-audio branch from 79560c2 to 49c1627 Compare July 27, 2026 23:40
@cameronhotchkies cameronhotchkies added the triage-ready Appropriate for agentic review label Jul 29, 2026

yinkev commented Aug 5, 2026

Copy link
Copy Markdown
Author

Superseded after fresh GitHub recalculation — 2026-08-05: this PR is currently cleanly mergeable into main, and CI on head 49c16271 completed successfully. No rebase or code change is requested; the branch should remain unchanged unless a maintainer raises a new finding. The current GitHub installation cannot update the fork branch, and maintainer_can_modify remains false.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@eeSeeGee

Copy link
Copy Markdown

🤖 Fresh confirmation for this PR: a third platform report landed on #2868 — macOS 26.6.1 / arm64 / Buzz 0.5.10, with the pinned device being a Bluetooth headset, so A2DP never idles. Full capture: #2868 (comment)

Two things from that trace that are relevant to reviewing this change specifically:

  1. Your armIdleSuspend() design is the part that matters on macOS. The device is pinned 7 ms after RemoteAudioDestination is constructed and 126 ms before it start()s — i.e. from app mount, with no poof ever played. A suspend debounced only off playback completion would never fire. Calling armIdleSuspend() at construction with the statechange listener and your comment "WebKit may start an autoplay-enabled context without any playback" is exactly right for this path.
  2. PoofBurstProvider.tsx:60 is the only AudioContext in desktop/src with no teardown — the three huddle contexts (HuddleContext.tsx:790, audioWorklet.ts:56, useHuddlePttState.ts:41) all close(). That makes this PR the whole fix for the device-pinning symptom rather than one of several needed changes.

Scope note for whoever reviews: this PR keeps the HTMLAudioElement fallback, which is fine for #2868, but that element is the separate media-key/Now Playing hijack that #4916 targets. The reporter hit both symptoms and needs both PRs.

Heads-up on landing order: #4916 also edits PoofBurstProvider.tsx and deletes the HTML fallback this PR preserves. Whoever merges second has a behavioral reconcile, not just a text conflict. Worth coordinating with @nathan-thillairajah.

This has been open 18 days with no review. Would be good to get eyes on it — it's a battery/power regression, not just idle CPU.

yinkev commented Aug 13, 2026

Copy link
Copy Markdown
Author

Coordination with #4916 is resolved: this PR should land first, and I’ll keep its head stable. Afterward #4916 will rebase and retain this PR’s construction-time idle suspension, playback/suspend race handling, and node cleanup while removing the Poof HTML preload/fallback. No code change is requested here for that integration; the behavioral reconcile belongs in the second PR. #5222 is the narrower superseded notification implementation.

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

Labels

triage-ready Appropriate for agentic review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Idle desktop app burns ~4% CPU and pins the audio device awake

4 participants