-
Notifications
You must be signed in to change notification settings - Fork 539
Revert fix(wasapi): do not emit DeviceChanged when not rerouted
#1350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable-0.18
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,12 +38,12 @@ fn get_current_default(flow: Audio::EDataFlow) -> Option<Audio::IMMDevice> { | |
| super::device::current_default_endpoint(flow) | ||
| } | ||
|
|
||
| /// Fires a Windows auto-reset event when the system default audio device changes. | ||
| /// Fires a Windows auto-reset event when the system default audio device changes, allowing | ||
| /// the stream run loop to deliver `ErrorKind::DeviceChanged` to the caller. | ||
| pub(crate) struct DefaultDeviceMonitor { | ||
| enumerator: Audio::IMMDeviceEnumerator, | ||
| client: Audio::IMMNotificationClient, | ||
| event: Foundation::HANDLE, | ||
| pub(crate) flow: Audio::EDataFlow, | ||
| pub(crate) pending_device_changed: Arc<AtomicBool>, | ||
| } | ||
|
|
||
|
|
@@ -79,7 +79,6 @@ impl DefaultDeviceMonitor { | |
| enumerator, | ||
| client, | ||
| event, | ||
| flow, | ||
| pending_device_changed, | ||
| }) | ||
| } | ||
|
|
@@ -211,8 +210,9 @@ pub struct Stream { | |
| // QueryPerformanceFrequency result, cached at construction (constant for the system lifetime). | ||
| qpc_frequency: u64, | ||
|
|
||
| // Present for default-device streams. Dropped after the run thread joins, ensuring the | ||
| // HANDLE is not waited on when it is closed. | ||
| // Present for default-device streams; fires `ErrorKind::DeviceChanged` when the system | ||
| // default changes. Dropped after the run thread joins, ensuring the HANDLE is not | ||
| // waited on when it is closed. | ||
| _default_device_monitor: Option<DefaultDeviceMonitor>, | ||
|
|
||
| // Latch that ensures no callbacks fire before the caller receives the `Stream` handle. | ||
|
|
@@ -252,12 +252,10 @@ struct RunContext { | |
|
|
||
| commands: Receiver<Command>, | ||
|
|
||
| // Set by a device-change notification callback when SetEvent fails. | ||
| // Set by a device-change notification callback when SetEvent fails. The audio loop delivers | ||
| // DeviceChanged on its next iteration. | ||
| pending_device_changed: Option<Arc<AtomicBool>>, | ||
|
|
||
| // Set when this stream tracks the default device, rather than a pinned one. | ||
| default_device_flow: Option<Audio::EDataFlow>, | ||
|
|
||
| // Owned here so the worker thread closes it on exit in a self-join case. | ||
| pending_scheduled_event: Foundation::HANDLE, | ||
| } | ||
|
|
@@ -342,13 +340,11 @@ impl Stream { | |
| let pending_device_changed = default_device_monitor | ||
| .as_ref() | ||
| .map(|m| m.pending_device_changed.clone()); | ||
| let default_device_flow = default_device_monitor.as_ref().map(|m| m.flow); | ||
| let run_context = RunContext { | ||
| handles, | ||
| stream: stream_inner, | ||
| commands: rx, | ||
| pending_device_changed, | ||
| default_device_flow, | ||
| pending_scheduled_event, | ||
| }; | ||
|
|
||
|
|
@@ -414,13 +410,11 @@ impl Stream { | |
| let pending_device_changed = default_device_monitor | ||
| .as_ref() | ||
| .map(|m| m.pending_device_changed.clone()); | ||
| let default_device_flow = default_device_monitor.as_ref().map(|m| m.flow); | ||
| let run_context = RunContext { | ||
| handles, | ||
| stream: stream_inner, | ||
| commands: rx, | ||
| pending_device_changed, | ||
| default_device_flow, | ||
| pending_scheduled_event, | ||
| }; | ||
|
|
||
|
|
@@ -727,14 +721,6 @@ fn boost_current_thread_priority( | |
| } | ||
| } | ||
|
|
||
| // WASAPI never rebinds the IAudioClient, so report what's actually true instead of DeviceChanged. | ||
| fn default_device_change_error(flow: Option<Audio::EDataFlow>) -> Error { | ||
| match flow.and_then(get_current_default) { | ||
| None => ErrorKind::DeviceNotAvailable.into(), | ||
| Some(_) => ErrorKind::StreamInvalidated.into(), | ||
| } | ||
| } | ||
|
|
||
| fn process_commands_and_await_signal( | ||
| run_context: &mut RunContext, | ||
| error_callback: &ErrorCallbackArc, | ||
|
|
@@ -753,7 +739,7 @@ fn process_commands_and_await_signal( | |
| if flag.swap(false, Ordering::Relaxed) { | ||
| emit_error( | ||
| error_callback, | ||
| default_device_change_error(run_context.default_device_flow), | ||
| Error::with_message(ErrorKind::DeviceChanged, "Default audio device changed"), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to throw out more than #1339 needs.
I think the fix should be smaller than a full revert: keep fn default_device_change_error(flow: Option<Audio::EDataFlow>) -> Error {
match flow.and_then(get_current_default) {
None => ErrorKind::DeviceNotAvailable.into(),
Some(_) => ErrorKind::DeviceChanged.into(), // was StreamInvalidated
}
} |
||
| ); | ||
| } | ||
| } | ||
|
|
@@ -774,7 +760,7 @@ fn process_commands_and_await_signal( | |
| if handle_idx >= 2 { | ||
| emit_error( | ||
| error_callback, | ||
| default_device_change_error(run_context.default_device_flow), | ||
| Error::with_message(ErrorKind::DeviceChanged, "Default audio device changed"), | ||
| ); | ||
| return ControlFlow::Continue(false); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be a bit more succinct: just document observable behavior, no need to pony up about being wrong or right.