Skip to content

fix(web): release retired pitch-correction sources - #1306

Open
kimchouard wants to merge 1 commit into
software-mansion:mainfrom
kimchouard:codex/web-audio-buffer-disposal
Open

kimchouard wants to merge 1 commit into
software-mansion:mainfrom
kimchouard:codex/web-audio-buffer-disposal

Conversation

@kimchouard

Copy link
Copy Markdown
Contributor

Problem

On web, a pitch-corrected buffer source keeps its Signalsmith processor running after playback stops and the source is disconnected. Repeated playback and seeking accumulate idle processors in a shared audio context. We found this while investigating backing-track playback becoming choppy after repeated pauses.

Reproduced on current main (a14f1a4b): after six playback cycles, all six processors still received audio callbacks during a 600 ms idle window.

Change

Additive API; no breaking changes.

Add AudioBufferSourceNode.dispose(): void for callers that are finished with a source:

const source = context.createBufferSource({ pitchCorrection: true });
source.buffer = buffer;
source.connect(context.destination);
source.start();

// When the source is no longer needed:
source.dispose();

Disposal is terminal and idempotent. It disconnects the source, clears its buffer and callbacks, and cancels pending web initialization. The worklet drops its PCM/WASM references and returns false from process(). Pending requests settle without waiting for an audio-thread acknowledgement.

stop(when) keeps its scheduling behavior; disconnect() remains reversible. The shared context stays open. The method is a documented library extension with native and mock counterparts; it does not promise immediate garbage collection.

Validation

  • Chromium 149, six cycles: retained processors 6 → 0; callbacks during the final idle window 226 per processor → 0. Every source produced nonzero, finite PCM before disposal; the context remained running. Output was muted downstream.
  • 21 regression tests cover pending module/WASM initialization, shared-loading cancellation, context closure, outstanding requests, repeated disposal, terminal guards, and native/web/mock API behavior.
  • yarn validate:fast passed: formatting, lint, typecheck, enum sync, build, 439 C++ smoke tests, and 134 JavaScript tests. Native device playback was not re-evaluated; native changes use existing bindings.

@github-actions

Copy link
Copy Markdown

WPT non-regression comparison

PASS — no regressions · 0 improved section(s) · overall 2717 → 2717 (0)

Unchanged sections (28)
Spec section Base pass Head pass Delta
Processing model 0 0 0
Other 52 52 0
AnalyserNode 138 138 0
AudioBuffer 140 140 0
AudioBufferSourceNode 218 218 0
AudioContext 58 58 0
AudioNode 261 261 0
AudioParam 629 629 0
BiquadFilterNode 275 275 0
ChannelMergerNode 30 30 0
ChannelSplitterNode 7 7 0
ConstantSourceNode 64 64 0
ConvolverNode 203 203 0
DelayNode 104 104 0
DestinationNode 0 0 0
DynamicsCompressorNode 4 4 0
GainNode 15 15 0
IIRFilterNode 87 87 0
MediaElementAudioSourceNode 0 0 0
MediaStreamAudioDestinationNode 1 1 0
MediaStreamAudioSourceNode 0 0 0
OfflineAudioContext 35 35 0
OscillatorNode 104 104 0
PannerNode 75 75 0
PeriodicWave 33 33 0
ScriptProcessorNode 0 0 0
StereoPannerNode 102 102 0
WaveShaperNode 82 82 0

Baseline: a14f1a4b8d9efb154fb64475abcf5b0ad541b8f0 · Candidate: 878737b97036477f0ad3982ba86152611f5d5123

Workflow run · this comment is updated on every push.

@mdydek mdydek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the idea, but I would like to see also the immediate release of the native memory tbh. Many users asked for it and I think it is a reasonable change. You can reference to AudioBufferSourceNode::setBuffer implementation. We have separate thread for deconstructing and you can easily schedule something there: context->getDisposer()->dispose(std::move(audioBuffer_));

Comment on lines +174 to +181

Use `stop(when)` to schedule a stop, or `disconnect()` when you may reconnect later.
Those methods do not dispose the source.

This is a React Native Audio API extension, not part of the Web Audio API standard.
It returns `undefined` immediately. Cleanup runs on the audio thread when it can
process the request; disposal does not guarantee immediate memory reclamation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Use `stop(when)` to schedule a stop, or `disconnect()` when you may reconnect later.
Those methods do not dispose the source.
This is a React Native Audio API extension, not part of the Web Audio API standard.
It returns `undefined` immediately. Cleanup runs on the audio thread when it can
process the request; disposal does not guarantee immediate memory reclamation.

Comment on lines +33 to +34
protected assertNotDisposed(): void {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we do not need those assertions in the AudioNode and in the AudioScheduledSourceNode. The only node that benefit out of that is AudioBufferSourceNode and it cannot be started and has buffer set after disposal. I think that is enough, we don't need to introduce this noise.

@closetcaiman closetcaiman added fix Code changes specifically addressing and resolving a bug web Web platform support, React Native Web integration, or browser-specific AudioContext behavior labels Sep 24, 2026

This branch has not been deployed

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

Labels

fix Code changes specifically addressing and resolving a bug web Web platform support, React Native Web integration, or browser-specific AudioContext behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants