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

Change the WinMM backend so that it doesn't call the user callback during initialization, and other fixes #750

Merged
merged 12 commits into from
Apr 26, 2023

Conversation

padenot
Copy link
Collaborator

@padenot padenot commented Apr 24, 2023

No description provided.

@padenot
Copy link
Collaborator Author

padenot commented Apr 24, 2023

Lots of test pass, but I'm stuck on:

D:\a\cubeb\cubeb\src\cubeb_wasapi.cpp:2381:Unable to initialize audio client for capture: 80070005

which seems to be a permission problem, only on the capture side. Probably some configuration we need.

@ChunMinChang
Copy link
Member

ChunMinChang commented Apr 25, 2023

Lots of test pass, but I'm stuck on:

D:\a\cubeb\cubeb\src\cubeb_wasapi.cpp:2381:Unable to initialize audio client for capture: 80070005

I don't understand why the cubeb_wasapi failed while its code remains the same.

@padenot
Copy link
Collaborator Author

padenot commented Apr 25, 2023

I don't understand why the cubeb_wasapi failed while its code remains the same.

This PR enables tests on Windows. Until now, we were only building. Windows tests for cubeb are run on treeherder on the mozilla infra, but I'm trying to enable them here. I'm trying to run the tests twice: once with the default backend (this means WASAPI, because we're only testing on recent Windows), and once by forcing winmm, to get some coverage.

The problem is that the VMs on GitHub action don't have audio devices, I'm trying to address this.

@padenot padenot force-pushed the winmm-prefill branch 9 times, most recently from 4281c4b to 9b05c26 Compare April 25, 2023 14:49
@padenot
Copy link
Collaborator Author

padenot commented Apr 25, 2023

which seems to be a permission problem, only on the capture side. Probably some configuration we need.

I've been able to repro locally on Windows 11 by toggling this "Microphone access" setting:

Screenshot 2023-04-25 164652

Turns out it's possible to toggle this using the registry, this is what the second step in f8fd4bf is doing.

@padenot padenot requested a review from kinetiknz April 25, 2023 14:51
@padenot
Copy link
Collaborator Author

padenot commented Apr 25, 2023

@kinetiknz, @ChunMinChang, it took some digging and a few tries, but this is now green -- tests are now run using the winmm and wasapi backends on github actions. The winmm backend's behaviour is now aligned a bit more with the other backends.

When running on a backend that doesn't implement audio input (it's really just the winmm backend when used as fallback at this point), tests that need to do anything related to audio input are skipped, but all others still run and now succeed on Windows.

If we change the runner type, we'll have to do an adjustment: actions/runner-images#2528 (comment). If we do it now (on the current version, 2019), the tests fail, because the action errors out, saying the audio service is already running.

Nothing else has been changed (I hope!).

Copy link
Collaborator

@kinetiknz kinetiknz left a comment

Choose a reason for hiding this comment

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

That's awesome, thanks!

src/cubeb_winmm.c Outdated Show resolved Hide resolved
@padenot padenot merged commit 1ba9237 into master Apr 26, 2023
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