Make VirtualAudioBridge thread-safe for DAX RX DirectConnection (#2486)#2762
Conversation
Mirrors the PipeWireAudioBridge thread-safety pattern (#2312) on the macOS audio bridge so PanadapterStream::daxAudioReady → feedDaxAudio can ship cross-platform under Qt::DirectConnection (unblocks #2318). Changes: * m_open, m_transmitting, m_channelGain converted to std::atomic so reads on PanadapterStream's network thread are correctly synchronised with GUI-thread writes (open/close, DaxApplet slider). * Added m_lastAudioMs atomic flag; feedDaxAudio stores wall-clock now on every RX packet — no Qt API touched on the fast path. * The silence-timer self-stop and ring-buffer readPos snap (originally done from feedDaxAudio, which would invoke QTimer::stop() across threads under DirectConnection) moved into feedSilenceToAllChannels, which runs on the GUI thread and observes m_lastAudioMs to decide when to self-stop. Matches PipeWireAudioBridge::feedSilenceToAllPipes field-for-field. * m_rxTiming left as plain POD with a comment — only touched on the audio thread today; atomic-ify if a GUI-thread inspector is ever added. Blast radius: codegraph does not index macOS-only VirtualAudioBridge (Linux-built graph), so impact was checked manually. Callers go through the `DaxBridge` typedef (`MainWindow.h:95`), and the PanadapterStream → DaxBridge::feedDaxAudio connection at MainWindow.cpp:13887 is the only invocation of the modified slot. setChannelGain / setTransmitting / isOpen all stay on the GUI thread. Scope is local to the two files touched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks for the patch — this is a faithful mirror of the PipeWireAudioBridge thread-safety pattern from #2312 applied to the macOS-side bridge. The atomicization choices and the m_lastAudioMs + GUI-thread-self-stop split look correct to me, and the comments in the header capture the cross-thread reasoning clearly.
A few small observations:
-
Consider guarding
feedDaxAudiowithm_open(matches the PipeWire reference).PipeWireAudioBridge::feedDaxAudioopens with:if (!m_open.load(std::memory_order_acquire)) return;
This PR's
feedDaxAudiorelies only onif (!block) return;, butm_blocks[channel - 1]itself is a non-atomic pointer thatclose()(GUI thread) can null out concurrently with a network-threadfeedDaxAudio. Loadingm_openfirst piggybacks the established acquire/release pairing withopen()/close()and gives the fast path the same early-exit the PipeWire side has. (The rawm_blocks[i]race is a separate concern and probably out of scope here.) -
Timing window change is worth flagging. Previously,
feedDaxAudiostopped the silence timer the instant real RX audio arrived. Now the stop happens on the next 20 ms tick offeedSilenceToAllChannels, so the worst-case extra silence after radio resumes RX is ~20 ms (the timer interval) — the readPos snap that previously fired in-line is also deferred by the same window. PipeWireAudioBridge accepts this same delay so this is presumably fine for DAX RX, but it's a behavioral change worth noting in the PR description / a release-notes entry. -
m_silenceTimer->stop()insidefeedSilenceToAllChannelsno longer has the priorm_silenceTimer && ...guard. In practice it's safe — the method only runs as a slot of that timer, so the pointer is necessarily non-null — but a one-line comment to that effect would help future readers who don't have this PR's context. -
Re: the "Recovery note" in the PR description — for the code review, the diff stands on its own and looks clean. No action needed from a review standpoint, just flagging that maintainers may want to confirm provenance separately before merge.
Nothing blocking from my read. Nice tight diff.
Summary
Fixes #2486
What was changed
Mirrors the PipeWireAudioBridge thread-safety pattern (#2312) on the macOS audio bridge so PanadapterStream::daxAudioReady → feedDaxAudio can ship cross-platform under Qt::DirectConnection (unblocks #2318).
m_open,m_transmitting,m_channelGainconverted tostd::atomicso reads on PanadapterStream's network thread are correctly synchronised with GUI-thread writes (open/close, DaxApplet slider)QTimer::stop()infeedDaxAudioreplaced with an atomicm_lastAudioMsflagfeedSilenceToAllChannelstickRecovery note
This PR is being opened manually. The aetherclaude orchestrator (trace 5a19c310) pushed the signed commit at 15:18 PDT but was SIGTERM'd by a deploy.sh race before it could create the PR. Root cause + fix shipped as a follow-up to scripts/deploy.sh; this branch's commit is the orchestrator's own work, unmodified.
Generated by AetherClaude (automated agent for AetherSDR) — PR creation completed manually after orchestrator interrupt.