Fix TCI DAX audio crosstalk between slices: per-channel resamplers (#1806)#1815
Conversation
There was a problem hiding this comment.
Review: TCI DAX audio crosstalk fix (#1806)
TCI resampler fix — looks correct
The core fix is sound. Replacing Resampler* resampler with QHash<int, Resampler*> resamplers keyed by DAX channel mirrors the existing rxAccumBuf pattern and correctly isolates per-channel r8brain filter state. The lazy creation, qDeleteAll cleanup on disconnect/stop/rate-change, and resamplers.clear() after rate change all look right.
Using channel key 0 as a sentinel for the non-DAX onRxAudioReady() path is a clean solution since DAX channels are 1-based.
Nice work on the PR description too — the root cause analysis and behaviour matrix are thorough.
Out-of-scope files
Six of the eight changed files (AdifParser.cpp/h, DxccColorProvider.cpp/h, DxClusterDialog.cpp, MainWindow.cpp) implement the ADIF logbook auto-reload feature, which appears to belong to PR #1801 from the same branch. These changes are unrelated to the TCI resampler crosstalk fix described in the title and body.
Please split this PR so the TCI fix contains only TciServer.cpp and TciServer.h. Bundling unrelated features makes it harder for @ten9876 to review and bisect if either change needs to be reverted independently.
One minor note (pre-existing, not introduced here)
The s.value(key, "1.0") fallback for TciRxGain at the constructor (around line 69 of the base) is already addressed by PR #1811 — just flagging for awareness since both PRs touch TCI audio paths.
Thanks for the contribution @Chaosuk97 — the crosstalk diagnosis and fix are solid. Just needs the scope cleanup before merge.
…ethersdr#1806) The r8brain CDSPResampler is stateful — it maintains internal filter history across calls. A single shared Resampler* in ClientState meant that each time onDaxAudioReady() processed audio for DAX channel 2 (Slice B), the filter state left by channel 1 (Slice A) carried over, causing both channels' audio to bleed together in the TCI output. The comment in the code already acknowledged this problem for the accumulation buffers (QHash<int, QByteArray> rxAccumBuf), but the resampler itself was never split. Fix: - Replace Resampler* resampler in ClientState with QHash<int, Resampler*> resamplers, keyed by DAX channel. - Remove eager creation in onNewConnection(); resamplers are now lazily created per-channel on first audio packet in onDaxAudioReady(). - audio_samplerate handler: qDeleteAll() all existing resamplers on rate change; they are re-created lazily at the new rate. - onClientDisconnected() and stop(): qDeleteAll() the hash instead of deleting a single pointer. - onRxAudioReady() (non-DAX, currently unused): uses channel key 0 as a sentinel (DAX channels are 1-based), preserving the same lazy pattern. No functional change when only one TCI slice is active. With two or more active DAX channels, each channel's filter history is now fully isolated, eliminating the crosstalk heard by users running two receivers simultaneously (e.g. dual-band WSJT-X / satellite full-duplex setups). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c55e0d3 to
6cdfc4c
Compare
ten9876
left a comment
There was a problem hiding this comment.
Exact same pattern as the existing per-channel rxAccumBuf — the comment on TciServer.h:102 already documents why shared state across channels is wrong, this PR applies the same reasoning to the resampler. All 6 call sites updated consistently, cleanup paths covered (qDeleteAll on disconnect/rate change/stop). LGTM.
|
Great job with this fix Ian. @Chaosuk97 |
) Cuts a community-contribution-heavy point release. Eight community PRs landed plus three AetherClaude crash fixes. Highlights: Community: - Fix RADE RX not decoding on Windows (#1820, NF0T) - Clamp stale DAX RX backlog on macOS — fixes +1.5s FT8 DT bias (#1822, jensenpat) - Fix TCI DAX resampler crosstalk between slices (#1815, Chaosuk97) - Fix RX applet pan slider with NR active (#1799, Chaosuk97) - Fix TCI RX gain default 1.0 → 0.5 to match applet (#1811, NF0T) - Fix HAVE_SERIALPORT guard (#1812, NF0T) - Seamless ADIF logbook auto-reload (#1801, Chaosuk97) - RAC Canada band-plan corrections (#1817, VE3NEM) AetherClaude crash fixes: - Applet reorder with floating containers (#1745 → #1746) - Lazy-build RadioSetupDialog tabs, dodges Wayland FFmpeg/VDPAU crash (#1776 → #1777) - PanAdapter float-freeze — show-after-reset + direct reparent (#1668 → #1669) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem
When two DAX channels are active (e.g. dual-band WSJT-X, or satellite full-duplex RX), audio from Slice A bleeds into Slice B and vice versa in the TCI output.
Root cause:
r8brain CDSPResampleris a stateful filter — it maintains internal history across calls.ClientStateheld a single sharedResampler*for all DAX channels. Each timeonDaxAudioReady()processed channel 2 (Slice B), the filter state left by channel 1 (Slice A) carried over, mixing both streams together.The code comment already acknowledged the same problem for accumulation buffers (
QHash<int, QByteArray> rxAccumBufis correctly per-channel), but the resampler was never given the same treatment.Fix
Replace
Resampler* resamplerinClientStatewithQHash<int, Resampler*> resamplerskeyed by DAX channel, matching the existingrxAccumBufpattern:onNewConnection()— removed eager single-resampler creation; instances are now lazily created per-channel on first audio packetonDaxAudioReady()— looks upcs.resamplers[channel], creating a freshResampler(24000, audioSampleRate, 4096)on first use for that channelaudio_sampleratehandler —qDeleteAll(resamplers)on rate change; new instances are lazily recreated at the new rateonClientDisconnected()/stop()—qDeleteAll(resamplers)replaces singledelete resampleronRxAudioReady()(non-DAX, currently unused) — uses channel key0as a sentinel (DAX channels are 1-based), same lazy patternBehaviour
Test plan
audio_sampleratewhile connected — confirm audio still works cleanly after rate switch🤖 Generated with Claude Code