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

WASAPI: Set sample rate via IAudioClockAdjustment #5540

Closed
wants to merge 1 commit into from

Conversation

glebm
Copy link
Contributor

@glebm glebm commented Apr 15, 2022

Potential fix for #5538

@cgutman
Copy link
Collaborator

cgutman commented Apr 15, 2022

This might reintroduce the quality issues fixed by b98b5ad and may cause resampling failures in other cases.

I've not tested with both AUDCLNT_STREAMFLAGS_RATEADJUST and AUDCLNT_STREAMFLAGS_AUTOCONVERTPCM together, but I know from previous experience that using AUDCLNT_STREAMFLAGS_RATEADJUST alone will fail in the 48000 Hz -> 192000 Hz resampling case. In that case, SetSampleRate() fails with E_INVALIDARG.

Previous discussions for context:
#4604
#4608

Repro steps for SetSampleRate() issue with 192KHz mix format:
andrewrk/libsoundio#194

Other comments mentioning limitations of IAudioClockAdjustment resampling:
https://bugzilla.mozilla.org/show_bug.cgi?id=894801#c21

@glebm
Copy link
Contributor Author

glebm commented Apr 15, 2022

@cgutman b98b5ad has already been reverted in ac32c52

The system (WASAPI) resampler is what's used currently and it does have quality issues

This PR may or may not fix these quality issues -- we're currently testing it

@cgutman
Copy link
Collaborator

cgutman commented Apr 15, 2022

@cgutman b98b5ad has already been reverted in ac32c52

I know. It was my suggestion to use AUDCLNT_STREAMFLAGS_AUTOCONVERTPCM in #4604 :)

The system (WASAPI) resampler is what's used currently and it does have quality issues

Does SDL's resampler work okay for this case?

@glebm
Copy link
Contributor Author

glebm commented Apr 15, 2022

We might have to revert to using SDL for resampling if this does not fix quality issues or if SetSampleRate has a limited range even with AUDCLNT_STREAMFLAGS_AUTOCONVERTPCM.

@glebm
Copy link
Contributor Author

glebm commented Apr 15, 2022

This does not fix the problem

Can we go back to SDL's resampling?

@glebm glebm closed this Apr 15, 2022
@cgutman
Copy link
Collaborator

cgutman commented Apr 15, 2022

@icculus do you think reverting ac32c52 is safe enough for 2.0.22?

@icculus
Copy link
Collaborator

icculus commented Apr 15, 2022

I don't mind changing back, but I'm extremely hesitant to do it right before 2.0.22 ships.

I'm okay with it if @slouken gives the greenlight, otherwise we can flip this as soon as 2.0.22 ships.

@slouken
Copy link
Collaborator

slouken commented Apr 18, 2022

Yep, I flipped it back in 9e264b9

@glebm
Copy link
Contributor Author

glebm commented Apr 18, 2022

Thanks!

@slouken
Copy link
Collaborator

slouken commented Apr 18, 2022

You're welcome!

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.

4 participants