Skip to content

feat(mixer): per-channel voice isolation - #1036

Open
wagenet wants to merge 4 commits into
Eyevinn:mainfrom
wagenet:wagenet/mixer-voice-isolation
Open

wagenet wants to merge 4 commits into
Eyevinn:mainfrom
wagenet:wagenet/mixer-voice-isolation

Conversation

@wagenet

@wagenet wagenet commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

No issue.

Problem

A remote guest sends whatever is around them to air: music, a TV, fans, kitchen noise. The HPF and gate can't remove it, because that sound overlaps speech in level and frequency.

Change

A per-channel Voice isolation switch in the mixer, between the HPF and the gate. The properties are chN_voice_isolation and chN_voice_isolation_limit (maximum cut in dB); both are live and appear in the channel detail panel.

The new element, stromvoiceisolation, runs DPDFNet (dpdfnet2_48khz_hr, Ceva, Apache-2.0 code and weights) with tract, a pure-Rust ONNX runtime. Off, it passes audio through untouched. On, it runs the model on the channel's mono downmix and writes the result to every channel. Switching it on or off crossfades over 10 ms. The model loads off the streaming thread on first enable; if loading fails, the element posts a warning and stays dry. The model runs only at 48 kHz, so a mixer at another rate gets audioresample + capsfilter around the element.

Latency. The element reports none. An enabled channel's audio is 60 ms late inside unchanged timestamps instead: 50 ms for the model plus one hop of buffering, so buffers that are not whole hops are always answered in full. Reporting it is possible (Strom already recalculates latency when a Time Offset changes live), but audiomixer reports the largest latency among its inputs, and the pipeline applies the highest latency of any sink to every sink, video included. One enabled channel would then delay every output of the flow by 60 ms, the conversation return included, and each toggle would re-sync every sink. This way only the enabled channel pays, and its audio trails its video by 60 ms, which is at the EBU R37 limit.

Build. build.rs downloads the 10.6 MB model, checks its sha256, and embeds it in the binary; STROM_VOICE_ISOLATION_MODEL points at a local copy for offline builds. Everything sits behind the default-on voice-isolation feature. Debug builds compile the tract and FFT crates at opt-level = 3, because unoptimised inference can't keep up with live audio.

Rejected: DeepFilterNet3 (dormant, unclear weight licence); RNNoise (about 3 dB off the music); onnxruntime via ort (about half the CPU, but its prebuilt needs glibc 2.38 and libstdc++, so the zigbuild Linux release builds against glibc 2.31 cannot link it).

Evidence

Model, offline on a real 47-minute, four-guest recording, through the Python reference that tract matches to −66 dB: about 40 dB off an in-room instrument, with that guest's voice changed by −0.15 dB; clean speech changed 0.06–0.15 dB (median); voiced laughter kept, breaths and knocks reduced 40–95 dB. Judged good enough by ear. Long or overlapping laughter is untested.

Element. Each test below fails when the behaviour it guards is removed; each removal was run on this head:

  • enabled_suppresses_noise_and_switches_back_cleanly: on cuts noise by at least 20 dB with sizes and timestamps unchanged, and off returns the exact dry signal. Without the wet mix: noise only reduced 0.0 dB. Without the fade back: buffer 0 after disabling is not the dry signal.
  • odd_buffer_sizes_come_back_whole_and_60_ms_late: at a 0 dB limit the wet signal is the input itself, so every output sample must equal the input exactly 2880 samples earlier, for buffers of 1 to 1024 frames. Without the padding hop: output never became the delayed input.
  • disabled_passes_audio_through_unchanged: bit-exact while off, until an enabled element fed the same audio has been running for 1 s, so the window outlasts a model load however slow the host. With enabled ignored, under background priority (taskpolicy -b): buffer 40 changed while disabled.
  • element_is_finalized_after_running: with a leaked reference: voice isolation element still alive after drop.
  • test_voice_isolation_converts_around_a_non_48k_mixer: without the conversion: link mx:hpf_0 -> mx:voiceiso_0: Noformat.
  • test_voice_isolation_sits_between_hpf_and_gate: with the element left unlinked, the placement assertion fails.

Tests

Ran, on macOS arm64 at eb76ed7, before the rebase onto main: cargo test --workspace voice_isolation (6 passed), mixer (84 passed, 2 already ignored), pipeline_lifecycle_test, clippy -D warnings with and without default features, fmt --check. After the rebase, at 3f66f15: cargo check --all-targets and cargo test --workspace voice_isolation (6 passed).

CI: at eb76ed7, before the rebase, Check (Linux) ran and passed all 6 voice_isolation tests, and both Linux zigbuild release builds passed. Check (Linux) then failed on static_underlays_are_not_reuploaded_gpu, a GPU vision mixer test this PR does not touch; a fork run of the same commit passed it. Windows tests passed in a fork dispatch (wagenet/strom run 37671643619; the upstream job needs the ci:windows label), but that run's release build hit the job's 45-minute limit; see For the reviewer.

Not run: the Docker build and macOS CI (label-gated); the GUI section in a running Strom; CPU inside Strom.

For the reviewer

  • Binary size: +34.5 MB on macOS arm64 release (98.2 → 132.7 MB, same commit with and without the feature).
  • CPU: about 2 ms per 10 ms of audio per enabled channel, measured in a standalone program on an M4, not inside Strom. Disabled channels cost a downmix copy.
  • Memory: 46 MB resident per enabled channel (four channels in one process); each channel loads its own copy of the model.
  • Windows CI time: compiling tract at opt-level = 3 adds about 5 minutes to tests and 2–3 to the release build on runs without sccache. Fork dispatches have no cache and include the release build, so they reach the job's 45-minute limit (two of three were cut off there, after tests passed; the release build passed on 66c8913 in 41 minutes). Upstream PR runs skip the release build; main runs use sccache, so after the first one tract should come from the cache (unmeasured).
  • Build-time download of the model from the sherpa-onnx GitHub release unless STROM_VOICE_ISOLATION_MODEL is set.

Known limits

  • 44.1 kHz mixers lose the top of the band even with voice isolation off. The conversion around the element is always present, so audio above about 20 kHz drops by 12 dB, and every channel runs two extra resamplers. 48 kHz mixers, the default, are unaffected.
  • Switching on costs one buffer about 7× the steady cost (six model passes to prime the engine, on the streaming thread). That's about 14 ms on an M4 against the mixer's 30 ms latency; a much slower host could miss the deadline at the moment of the switch.
  • After a feed gap, the last 60 ms before the gap play when the feed resumes. That audio was still in the delay when the gap began.
  • No option to delay video to match an enabled channel.
  • GPU inference: not tried. Each call is one 10 ms frame for one stream, so dispatch overhead would likely exceed the CPU cost (unmeasured).

🤖 Generated with Claude Code

@wagenet
wagenet marked this pull request as ready for review October 7, 2026 22:10

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: Request changes. The element is well built and its guard tests ran in CI. But one premise of the latency design is wrong, and the default-on build cost needs a maintainer's call.

Requested changes

  1. Fix the latency premise in the PR body, or argue it again. The body says "Strom never recalculates pipeline latency". Strom already does that for a live property change, in backend/src/gst/pipeline/properties.rs:210 — let _ = self.pipeline.recalculate_latency(); (the Time Offset path). Voice isolation could report 60 ms only while enabled and use the same path. The remaining argument against that is narrower: audiomixer would then hold back every channel of the mix while one channel is on. Say whether that is the reason, because it decides whether the design is right.
  2. Re-run the failed jobs (gh run rerun 37671616522 --failed). Check (Linux) went red on static_underlays_are_not_reuploaded_gpu (underlay pads hold a frame with no zone configured), in a file this diff does not touch. Because that step failed, volume_ramp_test, the whip_* tests, and the strom-types and strom-frontend steps never ran. This diff changes types/src/mixer.rs and frontend/src/mixer/.

For the maintainer — default-on cost

  • Every build downloads a model from another project's release. backend/Cargo.toml:10 — default = ["nvidia", "voice-isolation"]. The source is backend/build.rs:392. The sha256 pin makes the file safe, but not available: if that asset moves, every default build breaks, offline builds included unless STROM_VOICE_ISOLATION_MODEL is set. The binary grows by 34.5 MB.
  • The weights ship inside the binary under Apache-2.0 (Ceva). This PR adds no attribution or NOTICE for them. EXTERNAL: what redistribution requires here.
  • 44.1 kHz mixers lose quality even with the switch off. The resample pair is always built at a non-48 kHz rate, from backend/src/blocks/builtin/mixer/voice_isolation.rs:93 — let (in_first, in_last) = convert("in", voice_isolation::SAMPLE_RATE)?;. Per the PR body that costs −12 dB above 20 kHz on every 44.1 kHz flow. One option: build the conversion only when the flow starts with the switch on.

Claims

Claim Verdict Evidence
Strom never recalculates latency CONTRADICTED backend/src/gst/pipeline/properties.rs:210, above
Off is bit-exact passthrough CONFIRMED passthrough only reads the buffer (downmix into history). disabled_passes_audio_through_unchanged printed ok in job 112964538701
On writes a mono result to every channel CONFIRMED backend/src/gst/voice_isolation/imp.rs:422 — *s = *s * (1.0 - g) + w * g;, with one wet sample per frame. A stereo source collapses to mono while the switch is on
The element is finalized after running CONFIRMED element_is_finalized_after_running ran and passed in the same job

Radius. GLOBAL. The block chain is local to the mixer, but the default feature changes every build: a network fetch in build.rs, binary size, and dev-profile opt-level overrides in the workspace Cargo.toml. Lock order is consistent (state, then settings).

Overlaps.

  • #898 and #930 change the same per-channel chain in mixer/builder.rs. Land #898 (standing Approve) first.
  • #1002 and #1034 both rewrite Cargo.lock. Regenerate the lock against whichever lands first.

Tests & CI. All 6 voice_isolation tests executed and passed in Check (Linux), and both Linux builds are green. macOS and Windows were skipped here. The Windows evidence is the author's fork dispatch, so ask for the ci:windows label before merge, for the build-time curl and compile time.

Confidence: HIGH

wagenet and others added 4 commits October 8, 2026 09:57
Adds stromvoiceisolation, a native element that runs the DPDFNet speech
enhancement model (DPDFNet-2, 48 kHz, Apache-2.0) through onnxruntime, and
puts it in every mixer channel between the HPF and the gate. Off by default;
switching it on or off while the flow runs crossfades over 10 ms.

The element reports no latency: Strom never recalculates pipeline latency,
so reporting the model's delay would hold back every channel. Audio on an
enabled channel is instead 60 ms late within unchanged timestamps.

The model is downloaded and checksummed at build time and compiled into
the binary; onnxruntime is linked statically from the ort prebuilts. Both
sit behind the default `voice-isolation` feature.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ort prebuilt onnxruntime needs libstdc++ and glibc 2.38 symbols, so
the zigbuild Linux release builds (glibc 2.31, libc++) cannot link it.
tract is pure Rust and builds on every target. Debug builds optimise the
tract and FFT crates, since unoptimised inference cannot keep up with
live audio.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The disabled test now runs for 3 s, long enough for a model load that
should never start. The odd-buffer test runs at a 0 dB limit, where the
wet signal is the input itself, and checks every output sample against
the input exactly CONTENT_DELAY_SAMPLES earlier, so running short of wet
samples fails it. The element and engine use the shared
VOICE_ISOLATION_NO_LIMIT_DB instead of a literal 100.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The disabled test ran for a fixed 3 s, and a slow CI host can take longer
than that to load the model, so an element ignoring `enabled` could pass.
An enabled element fed the same audio now marks when a load completes on
this host, and the check runs until one second after that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wagenet
wagenet force-pushed the wagenet/mixer-voice-isolation branch from eb76ed7 to 3f66f15 Compare October 8, 2026 17:01
@wagenet

wagenet commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Claude here, replying for @wagenet.

Thanks for the review. I rebased onto main (3f66f159), which started a new CI run.

1. Latency premise. You're right: properties.rs:210 already recalculates latency for a live Time Offset change. I've corrected the body. The reason not to report the 60 ms is the one you named, and it goes past audiomixer. The mixer would take on the 60 ms, and the pipeline would then apply it to every sink, video included. One enabled channel would delay every output of the flow, including the conversation return, and each toggle would re-sync every sink. Keeping the delay inside the channel means only that guest pays, at 60 ms audio-behind-video, which is the EBU R37 limit.

2. CI. Re-running on the rebased head.

3. 44.1 kHz with the switch off. Agreed that a disabled feature shouldn't cost quality. Two ways to fix it:

  • Build the conversion only when the flow starts with the switch on (your suggestion). It's the simplest. On a non-48 kHz mixer, though, the switch can no longer be turned on live.
  • Resample inside the element, only while it's enabled. Off would be bit-exact passthrough at any rate, and the live switch would keep working. The cost is more element code and a pure-Rust resampler dependency, likely rubato.

I have no strong preference. Is losing the live switch at 44.1 kHz acceptable, or worth the extra code to keep?

Attribution. I can add a NOTICE crediting the DPDFNet weights (Ceva, Apache-2.0). Whether shipping the weights inside the binary needs anything more is for the maintainer.

Build-time download and default-on feature. Also the maintainer's call. If an external asset in the default build is a blocker, the options are to make voice-isolation off by default or to host the model somewhere Strom controls.

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: Comment. This replaces my review at eb76ed7 and moves it from Request changes to Comment. Both requested changes are dealt with: the body now gives the right latency argument, and CI ran again. The diff itself is unchanged since eb76ed7. Comparing the two heads over this PR's files shows only lines that came from main in the rebase. Two things still keep this from an Approve: a red Check (Linux) that this diff did not cause, and the maintainer's decisions below.

Requested changes

  1. Re-run the failed job (gh run rerun 37813364018 --failed). Check (Linux) failed in stinger_prores_test (a_prores_clip_after_another_format_still_airs, left: 29 right: 30 frames). That is a stinger timing test this diff does not touch, from the same family as #1055. The cost is that Run tests on shared types and Run tests on frontend were skipped, and this diff changes types/src/mixer.rs and frontend/src/mixer/. For two runs now, those steps have not executed against this change.

For the maintainer, still open, and the author asked you directly:

  • 44.1 kHz with the switch off. backend/src/blocks/builtin/mixer/voice_isolation.rs:93 — let (in_first, in_last) = convert("in", voice_isolation::SAMPLE_RATE)?; is always built at a non-48 kHz rate. The author offers two fixes: build the conversion only when the flow starts with the switch on, which loses the live switch at 44.1 kHz, or resample inside the element only while it is enabled, which needs more code and a resampler crate.
  • Default-on build-time download from another project's release: backend/build.rs:392 — "https://github.com/k2-fsa/sherpa-onnx/releases/download/speech-enhancement-models/dpdfnet2_48khz_hr.onnx"; with backend/Cargo.toml:10 — default = ["nvidia", "voice-isolation"]. The options are to keep it, make the feature off by default, or host the model somewhere Strom controls.
  • Attribution for the embedded weights (Ceva, Apache-2.0). There is still no NOTICE in the tree, and the author has offered to add one. EXTERNAL: what redistribution requires.

Claims

Claim Verdict Evidence
Strom already recalculates latency on a live change (corrected body) CONFIRMED backend/src/gst/pipeline/properties.rs:210 — let _ = self.pipeline.recalculate_latency();
Reporting 60 ms would delay every sink, video included EXTERNAL Assumes standard audiomixer and pipeline latency aggregation. This is the reason the body now gives for keeping the delay inside the channel
The 6 voice_isolation tests ran in CI at this head CONFIRMED All six print ... ok in job 113435663380 at 3f66f15. pipeline_lifecycle_test also ran before the stinger failure

Radius. GLOBAL, unchanged from my previous review. The feature is on by default, so it changes every build: a network fetch, 34.5 MB more binary, and dev-profile opt-level overrides.

Overlaps. #898 and #930 change the same per-channel chain in mixer/builder.rs. #1002 and #1017 rewrite Cargo.lock. Whichever of these lands second needs a rebase. None of them owns the decisions above.

Tests & CI. At 3f66f15: Build (Linux x86_64/ARM64), Check & Build (WASM) and API Contract Check are green. Check (Linux) is red, as described above. macOS and Windows were skipped. The Windows evidence is still the author's fork dispatch, so add the ci:windows label before merge to check the build-time download and the compile time on that runner.

Confidence: HIGH

@srperens
srperens dismissed their stale review October 9, 2026 08:08

Superseded by my review at 3f66f15: both requested changes are addressed, and the verdict is now Comment.

wagenet added a commit to wagenet/strom that referenced this pull request Oct 9, 2026
# Conflicts:
#	backend/src/blocks/builtin/mixer/tests.rs
#	backend/src/gst/mod.rs
#	backend/src/main.rs
wagenet added a commit to wagenet/strom that referenced this pull request Oct 9, 2026
Eyevinn#1036 puts the element on every mixer channel; Eyevinn#898's tests build the
mixer without registering it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.

2 participants