Skip to content

fix: remove waiting and redisign the convolver approach - #1357

Open
mdydek wants to merge 3 commits into
mainfrom
fix/convolver-thread-pool-wait
Open

mdydek wants to merge 3 commits into
mainfrom
fix/convolver-thread-pool-wait

Conversation

@mdydek

@mdydek mdydek commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Closes #1339

⚠️ Breaking changes ⚠️

Introduced changes

  • fixed the bug that could break the invariant of thread pool wait function some tasks could accidentally change its internal state
  • even with the fix mentioned above, the thread pool waiting design was wrong, because if the thread that did a convolution was suspended, we were also waiting for this result, thus here the redisign is presented. We can utilize the fact that if we remember data from the past on the current pass we can calculate only some small parts on the audio thread and then delegate the rest to the underlying thread, which gives them N (8 in this case) x RQ time of time to finish the calculations. By using this approach the underlying threads could be suspended, but still make the deadline which audio thread can utilize. If the result is not ready, unfortunately heavy convolution is done on audio thread.
  • while testing there were some false positives in tsan builds, because the audioapi library which was linked against those builds, was not build with the according flag, also fixed that

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added/Conducted relevant tests
  • Performed self-review of the code
  • Updated Web Audio API coverage
  • Added support for web
  • Updated old arch android spec file

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

WPT non-regression comparison

PASS — no regressions · 1 improved section(s) · overall 3427 → 3428 (+1)

Spec section Base pass Head pass Delta
ConvolverNode 178 179 +1
Unchanged sections (27)
Spec section Base pass Head pass Delta
Processing model 1 1 0
Other 56 56 0
AnalyserNode 138 138 0
AudioBuffer 140 140 0
AudioBufferSourceNode 220 220 0
AudioContext 58 58 0
AudioNode 261 261 0
AudioParam 680 680 0
BiquadFilterNode 275 275 0
ChannelMergerNode 30 30 0
ChannelSplitterNode 7 7 0
ConstantSourceNode 64 64 0
DelayNode 127 127 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 727 727 0
PeriodicWave 33 33 0
ScriptProcessorNode 0 0 0
StereoPannerNode 104 104 0
WaveShaperNode 82 82 0

Baseline: e39c1826e187a100d8d7ee6855e2a92ab32e8c32 · Candidate: 53768bca6fc8f3d234d218f52274467c5f8dab58

Workflow run · this comment is updated on every push.

@mdydek mdydek added the fix Code changes specifically addressing and resolving a bug label Oct 20, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ConvolverNode: SIGSEGV in Convolver::process caused by a race in ThreadPool::wait()

1 participant