Repository navigation
fix(audio): 5.1 voice detection, compressor timing, crosstalk limits, safe profiles, no leftover audio nodes - #307
Merged
Merged
Conversation
…e model fails The voice detector's worklet node took every channel of the video's audio and scored the first one only: the left channel. In a 5.1 film the dialogue sits in the centre channel, so the subtitle syncer and the fine-time timeline saw almost no speech (#192). The node now asks Web Audio for one channel (channelCount 1, explicit, speakers), which mixes 5.1 down as 0.707 (L + R) + C + 0.5 (SL + SR) and stereo as the mean of L and R. A start whose model did not load (a missing file, ONNX Runtime not starting) left the worklet node behind, its processor running for the rest of the audio context, one per retry (#199, part 1). The model now loads first and the node is made only once it has; a node that cannot be made releases the model. #199's second part, two inferences sharing the model state, does not happen: ONNX Runtime's wasm build runs a session synchronously (no proxy worker in the extension), so a run started by one worklet message has finished before the next message is handled. Measured on onnxruntime-web 1.30.0 in Node. Proven by tests/unit/vadModel.test.mjs: "has Web Audio mix every channel down to the one channel it scores" and "leaves no worklet node running when the model does not load"; each fails with its fix undone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eb now does
Port of ricky0123/vad aa048997 ("Give Silero v5 its context window"), the one
change between the copy's ref 7c2f259b and 43e20495 that changes what
FastStream sees (#127). Silero v5 and v6 - FastStream ships v6.2.1 - score
each 512-sample frame against the last 64 samples of the frame before it, as
silero-vad's own OnnxWrapper does; vad.mjs passed the bare frame, so the model
took every frame for the first of an utterance and scored starting and quiet
speech too low (upstream measured quiet speech going from 0.163 to 0.405 on
average). reset_state() clears the tail with the state.
The rest of the range does not apply here: the frame processor's
millisecond units, onSpeechRealStart and submitUserSpeechOnPause only touch
speech-segment events, which FastStream never raises (both thresholds are 1;
it uses the per-frame scores); the microphone, AudioContext and packaging
changes are for code the copy does not have; model release (#246) was already
here; the resampler's `>=` emits a frame one 128-sample block earlier at
48 kHz and changes nothing else.
The e2e reference scores change with it. tests/e2e/vad-reference.json is
rewritten from the new path on onnxruntime-web 1.30.0 in Node (112 of 600
frames speech, nearest score 1.6e-3 from the threshold); on the same runtime,
Node gave the old Firefox-recorded scores to within 3.4e-6 with no decision
changed, so CI's Firefox run should match the new file the same way.
tools/vendored-sources.json moves vad-web's ref to 43e20495, which closes #127.
Proven by tests/unit/vadModel.test.mjs "scores each frame with the last 64
samples of the frame before it in front", which fails with the tail left out
and with reset_state() keeping it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…able The background audio analyzer plays the video a second time at 8x, with its own downloads and audio context. Its start loads that player and only then checks it is still wanted, and the check compared sources only (#193): - Closing and reopening the tool that needs it (or play, stop, play) while the first player loaded started a second load for the same video. Both passed the check; the second took the first one's place and the first ran on with nothing referring to it, so nothing could ever stop it. - A new video while the old video's player loaded: the new one started first, then the old one finished loading, was destroyed as stale, and its end cleared the new player's place unconditionally. The new player ran on, unstoppable, and the status left at RUNNING refused every later start. Each start now has a number; a load whose start is no longer the latest is destroyed, and a player's end clears only its own place. An older load that fails no longer makes a newer start for the same video give up. Proven by three tests in tests/unit/vadLifecycle.test.mjs ("keeps one background player when it is stopped and started again for the same video while loading", "keeps the new video's background player when the old video's finishes loading after it", "keeps a newer start for the same video when an older load fails after it"); each fails with its part undone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The audio settings count the video's channels with a small worklet, and ask again for every video and every quality switch, discarding the count in progress. A recount that came while the previous counter's worklet was still loading (tens of ms on a new audio context: setup and an early first frame, or two quick level switches) could not stop it: the counter finished loading afterwards, connected itself to the audio anyway, and on its first count saw it had been replaced and returned without disconnecting. It stayed connected and running for the life of the audio context, and in the input's list of outputs, so every later connection fanned out to it too (#194). It is now destroyed as soon as it has loaded if it was replaced meanwhile. A counter that could not start (no AudioWorklet) answered 2 to its own caller and null to every later one; the 2 is now kept. Also from #201 (part 1): the channel update called the output convolver unguarded, and there is none where IndexedDB is not available; it threw into an empty catch that hid every other failure too. It is guarded now, and the catch logs anything that is not a replaced recount. ChannelUpmixer never stored a new output channel count, so it set it again on every update, and a count the node refused threw out of the whole update. Proven by tests/unit/audioChannelCount.test.mjs, on a stand-in Web Audio graph (tests/unit/fakeWebAudio.mjs) that records each connection; each of its four tests fails with its fix undone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each change to the crosstalk cancellation settings builds the new impulse response into the idle one of two convolvers and switches to it. The idle convolver was connected with an output index (1, which selected nothing here) and later disconnected without one; VirtualAudioNode matches the index, so the disconnect threw "Node not connected" into an empty catch and the retired convolver stayed connected (#197). From the second change on, both 4-channel convolvers ran for the rest of the session: the sound was right (only the output was switched) but the filter cost twice its CPU. The convolver is now connected the way it is disconnected, and the empty catches are gone: detachConvolver() takes a convolver off only where it is connected (the first switch has nothing to retire, and a pending switch has two on the input at destroy()), so a failed disconnect would now show. Proven by tests/unit/audioCrosstalk.test.mjs, which follows the real connections on a stand-in Web Audio graph through three changes and a destroy(); it fails with the index put back, and with the input disconnect left out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The crosstalk cancellation's colour gain knob ends at "20 dB", but that end turned the limit off altogether (Infinity), so every frequency got the unregularised filter (#200). With the decay near 0 dB this boosts some frequencies inside the cancellation band by 52.8 dB (measured on the filter itself: decay -0.01 dB, any microdelay), so the sound clips, and 52-69% of the impulse response's energy falls beyond the 1024 samples the convolver keeps of it, which rings. The knob's top now means 20 dB, like the rest of its range: the same settings peak at 14.4 dB. Decays of -1 dB or lower are unaffected (the limit never acted there). Proven by tests/unit/audioCrosstalk.test.mjs "boosts no frequency by more than the colour gain knob's top, 20 dB", which fails with Infinity put back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The crosstalk filter rounded its delay to whole samples, 20.8 us at 48 kHz, so the microdelay knob (30-200 us) gave about nine different filters over its whole range and most of a turn changed nothing (#200). The delay now stays fractional. A fraction breaks the symmetry a real impulse response needs, so only the positive frequencies are computed and the negative ones mirror them (the Nyquist bin made real); for a delay of whole samples this is the filter it was before, to 1.2e-12 of the peak tap. This changes what users with crosstalk cancellation on hear by a fraction of a sample (the delay they set, no longer the nearest whole sample): a separate commit, so it can be left out. Proven by tests/unit/audioCrosstalk.test.mjs: "has the response asked for with a delay that is not a whole number of samples" (the impulse response's spectrum against the formula at 4.5 samples) fails with the rounding put back and with the mirroring left out; "keeps the filter it had for a delay of whole samples" guards the old behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A DynamicsCompressorNode takes two channels at most, so on a video with more than two the master compressor sends the front pair through it and the other channels around it. But the compressor delays what it compresses by its look-ahead - Firefox's node (Blink's DynamicsCompressor) has a 6 ms pre-delay, in whole frames: 288 at 48 kHz, 264 at 44.1 kHz - and the channels around it were not delayed, so the centre, the LFE and the surrounds reached the speakers 6 ms before the front pair: smeared dialogue and transients, a shifted image (#195). Each of them now goes through a DelayNode of exactly that look-ahead. Not changed: those channels are still not compressed. Compressing them too (more compressors, each reacting on its own) changes how the master compressor sounds, so it is left to the owner. Proven by tests/unit/audioCompressor.test.mjs, which follows every channel's path on a stand-in Web Audio graph and adds up its delay, at 48 and 44.1 kHz; it fails with the delays left out and with a delay of 6 ms not rounded to whole frames. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Audio profiles come from storage and from imported .fsprofile.json files, and nothing checked them (#196). An AudioParam throws on NaN, so: - an equalizer node with "frequency": "abc", or with no gain, imported fine; choosing the profile threw in the equalizer after it had taken its old filters out, so that channel went silent until the next profile change, and the rest of the profile (other channels, master, mixer) was not applied; - a channel or master gain that was not a number threw in the mixer; - a list of six channels with an ID out of range (7) or given twice left the mixer without a channel for some ID, and it threw; - an unknown filter type was ignored by the browser, so the node stayed a lowpass at its frequency; - a crosstalk crossover frequency that was not a number made the filter throw when it was built. Profiles are now made safe as they are read: an equalizer node with no usable type or frequency is dropped, a missing gain is 0 dB and a missing Q is 1; channels are rebuilt one per ID (the first of an ID wins); gains, compressor and crosstalk values that are not numbers take their defaults, and all are kept within the ranges of their faders and knobs; the master is always the master. The equalizer also builds its new filters before taking the old ones out, so a refused value can no longer silence a channel. The eleven default profiles load exactly as before (compared old code against new). Not a bug: the crosstalk decay and microdelay save as null when they follow the distances (NaN); they load back as NaN, which means the same. Proven by tests/unit/audioProfiles.test.mjs; with each part undone in turn (the build order, the channel rebuild, the node check, the node gain, the channel gain, a compressor value, a crosstalk value, the master's ID) a test fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The output convolver (impulse responses for the output device) set each convolver's normalize flag and impulse response on every update: every quality switch (the channel recount), every checkbox and toggle. Each new buffer makes the browser rebuild the reverb and start it over, so its tail was cut off - a dropout or click per adaptive-bitrate switch - and a 16384- sample response had its FFT partitions rebuilt each time (#198). They are now set only when the response or the flag has changed. A stored impulse length that was not a number (localStorage) made the trim check false, so a whole file, minutes of it, was used as the impulse response. A stored length is now kept within what the field allows, 128 to 16384 (4096 when it is not a number). Proven by tests/unit/outputConvolver.test.mjs, which counts the buffers each stand-in convolver is given; it fails when they are always given, when a normalize change is not applied, and with the stored length taken as is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two small defects from #201 (its first part, the unguarded convolver call, went with the channel counter fix): - The mixer's mute button set its lit state from `channel.mute`, a field that does not exist (it is `muted`). classList.toggle(name, undefined) flips, so the button only showed the right state while nothing else changed `muted`; unmuted elsewhere while the strip was shown, the next press lit it the wrong way round. - An impulse response that did not decode was reported as "Could not decode impulse response: " followed by `name`, which is not in scope there and so read the window's name; it now names the file. Proven by tests/unit/audioMixer.test.mjs "lights the mute button when, and only when, the channel is muted" and tests/unit/outputConvolver.test.mjs "names the file that did not decode"; each fails with its fix undone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… every clip Two of #202's proposals that are plain wins: - The mixer's meters set the size of their canvases on every animation frame, seven of them while the audio settings are open. Setting a canvas's size, even to the size it already has, clears it and resets its context; the meters clear and redraw themselves anyway. The size is now set when it changes (compared as the whole number the canvas keeps). - The clip check read frequencyBinCount samples, half of the analyser's 256-sample window: the browser fills a short array with the oldest samples, so a clip in the newer half was never seen. It now reads the whole window. (Samples between two frames are still not looked at; that needs a peak-hold worklet, left as a proposal.) Proven by tests/unit/audioMixer.test.mjs "sizes each meter canvas once, not every frame" and "sees a clip anywhere in the analyser's window"; they fail with the size set every frame, with a fractional size compared as is, and with the half-window read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…time The owner chose this for #195. A DynamicsCompressorNode takes two channels at most, so on a video with more than two the master compressor compressed the front pair only: the centre (the dialogue), the LFE and the surrounds went around it untouched. Every channel now goes through a compressor with the same settings: a pair of speakers shares one (front, surround, side), so both sides are turned down together and the image stays put; the centre and the LFE get one each, so a bass hit does not turn the dialogue down. 5.1 uses four compressors (L+R, C, LFE, SL+SR), quad two, 5.0 three. They all delay by the same look-ahead, so the bypass delays of 6f2c033 are no longer needed. The gain-reduction graph still shows the front pair. The same 6 ms offset happened in the mixer: with the compressor on for some channels only, those lagged the others by the look-ahead. A channel compressor that is off can now wait exactly as long (setLatencyMatch, a DelayNode of the look-ahead in whole frames), and the mixer asks that of every active channel without one while any channel has one on. Stereo is built exactly as before: source, compressor, gain, out (the stereo test passes on the previous code too). Proven by tests/unit/audioCompressor.test.mjs and audioMixer.test.mjs on a stand-in Web Audio graph that follows each channel through the mergers and splitters and adds up its delay. Each of these undone in turn fails a test: compressing the front pair only, the LFE or the centre sharing a compressor, a 6 ms delay not in whole frames, no delay for an uncompressed channel, the mixer not asking for it, stereo through the splitter path, the settings on the front compressor only, the delay left in when the compressor is built, and the delay not put back when it is switched off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audio: fixes for 5.1 sound, the voice detector, crosstalk, the compressor and audio profiles
What was wrong, as you'd notice it:
What changed: each of these is fixed at its source. The default audio profiles load exactly as before.
How it's proven:
For the final try-list:
Fixes #192
Fixes #193
Fixes #194
Fixes #196
Fixes #197
Fixes #198
Fixes #199
Fixes #200
Fixes #195
Fixes #201
Fixes #127
Not fixed here (your call):
🤖 Generated with Claude Code