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

[windows] List all audio devices for Windows #338

Merged
merged 2 commits into from
May 23, 2024

Conversation

Steelskin
Copy link
Contributor

@Steelskin Steelskin commented Apr 7, 2024

Previously, FAudio with PLATFORM_WIN32 would only list the default audio device. This reworks the Faudio PLATFORM_WIN32 code to enable access to every audio device on the system. In order to preserve the existing behavior (and Xaudio2 compabitility), the audio devices are re-ordered so the first device is always the default one.

In addition, this properly populates the DisplayName field in FAudioDeviceDetails. Previously, it was set to the device GUID.

Test: Local build with visualboyadvance-m

Previously, FAudio with `PLATFORM_WIN32` would only list the default
audio device. This reworks the Faudio `PLATFORM_WIN32` code to enable
access to every audio device on the system. In order to preserve the
existing behavior (and Xaudio2 compabitility), the audio devices are
re-ordered so the first device is always the default one.

In addition, this properly populates the `DisplayName` field in
`FAudioDeviceDetails`. Previously, it was set to the device GUID.

Test: Local build with visualboyadvance-m
@Steelskin
Copy link
Contributor Author

Friendly ping. Do you have any concern or issue with this PR?

@flibitijibibo
Copy link
Member

I don't personally, but this is code meant for Wine so I was hoping it would get reviewed over there... we may need to submit this as a patch to Wine first? Here's their local copy:

https://gitlab.winehq.org/wine/wine/-/tree/master/libs/faudio?ref_type=heads

@rbernon
Copy link
Collaborator

rbernon commented May 21, 2024

Sorry, I missed the ping. I'll have a look.

src/FAudio_platform_win32.c Outdated Show resolved Hide resolved
src/FAudio_platform_win32.c Outdated Show resolved Hide resolved
src/FAudio_platform_win32.c Outdated Show resolved Hide resolved
* Rework `FAudio_DefaultDeviceIndex` to return the Windows API error
  code and take the `defaultDeviceIndex` as parameter.
* Change included headers for wine compatibility.
Copy link
Contributor Author

@Steelskin Steelskin left a comment

Choose a reason for hiding this comment

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

Thanks for getting back to me! I addressed your comments.

src/FAudio_platform_win32.c Outdated Show resolved Hide resolved
@Steelskin Steelskin requested a review from rbernon May 23, 2024 03:22
Copy link
Collaborator

@rbernon rbernon left a comment

Choose a reason for hiding this comment

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

Looks good to me, with some slightly inconsistent style issues, though I don't know how picky I'm expected to be. I think squashing the changes together would also be better.

@flibitijibibo
Copy link
Member

We can be pretty picky about style, but that can always come through as style fix commits - will get this in now since it's been waiting so patiently!

@flibitijibibo flibitijibibo merged commit bae43ab into FNA-XNA:master May 23, 2024
5 checks passed
@Steelskin Steelskin deleted the win-list-all-devices branch May 26, 2024 00:01
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.

3 participants