Skip to content
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

FIX(client,mac): Add support for input/output device switching on macOS #5273

Merged
merged 1 commit into from Oct 1, 2021

Conversation

jlsalmon
Copy link
Contributor

@jlsalmon jlsalmon commented Sep 29, 2021

Previously, when switching input/output devices at the system level, Mumble would ignore the switch and continue to use the previous device. This patch adds support for proper device switching, allowing Mumble to correctly follow the system input/output device.

Fixes #1013

Checks

Copy link
Member

@Krzmbrzl Krzmbrzl left a comment

Choose a reason for hiding this comment

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

Please run clang-format on the changed files (at least the header file - not sure if it even works on the .mm one) or alternatively run scripts/runClangFormat.sh which should invoke clang-format on all files.

src/mumble/CoreAudio.mm Outdated Show resolved Hide resolved
@Krzmbrzl Krzmbrzl added audio client feature-request This issue or PR deals with a new feature labels Sep 30, 2021
@Krzmbrzl
Copy link
Member

Thanks for tackling this! I am wondering though whether it might be more appropriate to label this as a fix rather than a new feature. I guess it would be the expected behavior for such changes to be picked up and thus it not doing so seems more like a bug to me... What do you think? 🤔

Note that this does not cover input device switching, but in theory the solution for that should be similar to this. I'd be happy to add input device switching support as well, if requested.

Absolutely! I think this would even be well suited to be done in one and the same commit 👍

@jlsalmon
Copy link
Contributor Author

I agree, it does seem like this should be something that "just works" so a fix is probably more appropriate. I'll rework this as a fix, and implement the input device switching at the same time later today 👍

@jlsalmon jlsalmon changed the title FEAT(client,mac): Add support for output device switching on macOS FIX(client,mac): Add support for output device switching on macOS Sep 30, 2021
Previously, when switching input/output devices at the system level, Mumble would ignore the switch and continue to use the previous device. This patch adds support for proper device switching, allowing Mumble to correctly follow the system input/output device.

Fixes mumble-voip#1013
@jlsalmon
Copy link
Contributor Author

Ok, input switching implemented, tested and working great with a bunch of different device switches (bluetooth headphones, AirPods, USB headset, internal microphone).

Had to manually fiddle with the .mm file formatting, but I'm fairly sure it's OK now.

Question about versioning: I see that there's a 1.4.x maintenance branch but not a 1.3.x branch - does that mean that master is tracking 1.3.x? If so, does that mean that this would make it into the next 1.3 release if accepted?

@Krzmbrzl Krzmbrzl added bug A bug (error) in the software and removed feature-request This issue or PR deals with a new feature labels Sep 30, 2021
@Krzmbrzl
Copy link
Member

Nice! 👍

Question about versioning: I see that there's a 1.4.x maintenance branch but not a 1.3.x branch - does that mean that master is tracking 1.3.x? If so, does that mean that this would make it into the next 1.3 release if accepted?

Current master is tracking 1.5.0. However this fix will then be backported to the 1.4.x branch to still make it into the upcoming release of 1.4.0. There will be no further 1.3.x releases since 1.4.0 is very close to being finished ☝️

@jlsalmon jlsalmon changed the title FIX(client,mac): Add support for output device switching on macOS FIX(client,mac): Add support for input/output device switching on macOS Sep 30, 2021
@jlsalmon
Copy link
Contributor Author

Awesome, thanks @Krzmbrzl, looking forward to seeing this in 1.4.0 :)

@Krzmbrzl Krzmbrzl merged commit c453a8b into mumble-voip:master Oct 1, 2021
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 1, 2021

Thanks for fixing this! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audio bug A bug (error) in the software client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mumble does not detect changes in default audio devices while running
2 participants