Skip to content

fix: auto-recover audio pipeline on device disconnect - #529

Open
Sanslayerbe wants to merge 1 commit into
fxsound2:develop/windowsfrom
Sanslayerbe:fix/device-disconnect-auto-recovery
Open

fix: auto-recover audio pipeline on device disconnect#529
Sanslayerbe wants to merge 1 commit into
fxsound2:develop/windowsfrom
Sanslayerbe:fix/device-disconnect-auto-recovery

Conversation

@Sanslayerbe

Copy link
Copy Markdown

Problem

When an active audio device is disconnected (Bluetooth, USB DAC, etc.),
FxSound often enters a hung state where audio stops and the only
recovery is restarting the app.

Relates to #40, #269, #220, #367, #468

Root Cause

Three bugs working together:

  1. updateOutputs() (line 1027) doesn't check isActive, so
    disconnected devices remain in the active device list. initOutputs()
    does this check correctly but updateOutputs() was missing it.
  2. onSoundDeviceChange() doesn't re-power the audio pipeline
    after selecting a new device — powerOn(true) and unmute are
    never called after a device switch during removal.
  3. When all devices disconnect, an empty device ID is passed to
    setOutput(), permanently muting the app with no recovery path.

Changes

  • updateOutputs(): Added missing isActive check to match
    the filtering in initOutputs() (one-line fix)
  • Off-state onSoundDeviceChange(): Same isActive fix
  • On-state onSoundDeviceChange(): Rewritten to properly
    re-enumerate devices, select the best available output, and
    explicitly re-power the audio pipeline if the user had it on
  • reloadDevices(): New public method for manual recovery as
    a fallback for edge cases (driver glitches, rapid connect/disconnect)

Testing

  • Bluetooth headphones disconnect during playback → auto-switches to speakers
  • USB DAC disconnect → recovers to default output
  • All devices removed → reconnect → audio resumes
  • Normal operation with no device changes → no regression

- Add isActive check to updateOutputs() and off-state onSoundDeviceChange()
  to prevent inactive devices from being included in active device list
- Rewrite on-state onSoundDeviceChange() to properly re-power the audio
  pipeline when switching to a new device after disconnection
- Add reloadDevices() as manual fallback for edge cases

Fixes fxsound2#40, fxsound2#269, fxsound2#220, fxsound2#367, fxsound2#468
@bvijay74

Copy link
Copy Markdown
Member

Thanks @Sanslayerbe for this fix. I will review it.

@bvijay74

Copy link
Copy Markdown
Member

Please retarget this PR to develop/windows branch

@bvijay74
bvijay74 changed the base branch from main to develop/windows July 21, 2026 09:24
@bvijay74

Copy link
Copy Markdown
Member

@claude review this PR and see if it fixes anything that is missed in the previous fixes for device detection

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @bvijay74's task in 6m 53s —— View job


Review: does this fix what earlier device-detection fixes missed?

  • Gather context (PR diff, related files, prior device-detection fixes)
  • Read FxController.cpp / FxController.h changes in detail
  • Trace how sound_devices vectors are populated (native interop layer)
  • Assess whether this PR covers cases missed by previous fixes
  • Post review feedback

Short answer: partially. The core "re-power after switching devices" fix (bug #2) is real and is the part earlier fixes were missing. However, two of the three claimed root causes don't actually do what the PR description says, one edge case (all-devices-removed while FxSound is off) is still unfixed, and the new rewritten path introduces a small regression. Details below.

1. updateOutputs()'s isActive fix is dead code

onSoundDeviceChange(std::vector<SoundDevice>) used to call updateOutputs() (fxsound/Source/GUI/FxController.cpp:1112) when the device count changed. This PR replaces that call with new inline logic (lines 1478-1576) and never calls updateOutputs() again — grep confirms it now has zero callers in the whole codebase. So the "one-line isActive fix" to updateOutputs() touted in the PR description never executes. Either restore the call or delete the now-unreachable function to avoid confusion later.

2. The isActive checks added to the two live handlers are very likely no-ops

Both places that feed device vectors into FxController already filter to active devices before FxController ever sees them:

  • ON-state: s_callback_->onSoundDeviceChange(getSoundDevices()) (audiopassthru/src/AudioPassthru/AudioPassthruPrivate.cpp:421) uses the default arg active_devices = true (audiopassthru/include/u_AudioPassthru.h:43).
  • OFF-state: audio_passthru_->getSoundDevices(true) (FxController.cpp:1610).

Inside sndDeviceHandleToSoundDevices() (audiopassthru/src/AudioPassthru/AudioPassthruPrivate.cpp:169-172), when active_devices is true, any device whose native state isn't DEVICE_STATE_ACTIVE is continued past and never added to the returned vector at all. So every SoundDevice that reaches FxController::onSoundDeviceChange(...) (line 1510) and FxController::onSoundDeviceChange() (line 1616) already has isActive == true — the newly added && sd.isActive checks can't filter anything, because the disconnected device was already stripped out upstream. (Note: initOutputs() and the new reloadDevices() call getSoundDevices(false), so the isActive check is meaningful there — that part of the diagnosis is correct.)

This is worth double-checking with the reporter/author — if this analysis is right, the "missing isActive check" wasn't actually the mechanism causing the hang in bugs #1/#2; the real fix is section 3 below (repowering), and the isActive lines are harmless but not doing what the PR claims.

3. Root cause #3 (empty device ID) is only fixed for the ON-state path

In onSoundDeviceChange(std::vector<SoundDevice>), the new if (active_output_devices_.size() == 0) { ...; return; } guard (line 1521) correctly avoids ever calling setOutput() with an empty ID when all devices vanish while FxSound is on.

But the OFF-state onSoundDeviceChange() (lines 1604-1660) wasn't given the same guard. If default_device_found stays false because active_output_devices_ is empty, it still falls through to setOutput(getPreferredOutput().pwszID.c_str()) (line 1658), and getPreferredOutput() returns a default-constructed SoundDevice{} (empty pwszID) when active_output_devices_ is empty (FxController.cpp:2159-2164) — the exact bug described in the PR body, just in the branch that wasn't touched.
Fix this →

4. New regression: unconditional reinit/notification on any device-count change

In the rewritten on-state handler (lines 1549-1573), once inside the "device count changed" branch, setAsPlaybackDevice(), output_changed_ = true, powerOn(true)/mute(false), and an "Output: " toast fire unconditionally — even when previous_found == true, i.e. the currently selected output device is untouched and the count only changed because of an unrelated device (a USB microphone, webcam mic, etc. — sound_devices includes capture devices too). The old setOutput() helper (line 632) has a guard (if (isTimerRunning()) { if (sound_device.isTargetedRealPlaybackDevice) break; }) specifically to skip redundant reinitialization/notification when nothing actually changed; that guard wasn't carried into the new inline logic. Net effect: plugging in an unrelated recording device while music is playing will now cause an audible reinit blip and a spurious "Output: X" notification.
Fix this →

5. reloadDevices() is currently unreachable

It's declared in FxController.h:159 and implemented in FxController.cpp:733-825, but nothing calls it — no menu item, hotkey, or button anywhere in the codebase. As shipped it can't actually serve as the "manual recovery fallback" the PR description promises; it needs to be wired to some UI entry point (e.g. a tray-menu "Reload Devices" item) to have any effect for the driver-glitch/rapid-reconnect edge cases it's meant to cover.

What this PR does get right

Note on branch target

Per @bvijay74's earlier comment, this PR still targets develop/windows — I couldn't verify the retarget status since git merge-base against origin/develop/windows fails in this sandboxed checkout (shallow clone, unrelated history available locally), so that should be confirmed manually before merge.

@bvijay74

Copy link
Copy Markdown
Member

@Sanslayerbe Can you please address the review comments?

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