SDL 2.0.16 programs go to unrequested frequency with Mix_OpenAudio

[Starbuck5]

Hi,
I'm one of the pygame contributors, and when I built pygame with SDL 2.0.16 I noticed a few unit test failures in our mixer module.

I made a minimum reproducible example in C:

#include <stdio.h>

#define SDL_MAIN_HANDLED
#include "SDL2/SDL.h"
#include "SDL2/SDL_mixer.h"

int main() {
  printf("hello world\n");

  SDL_Init(SDL_INIT_EVERYTHING);

  SDL_version ver;
  SDL_GetVersion(&ver);
  printf("version patch = %i\n", ver.patch);

  int freq = 44100;
  Uint16 fmt = AUDIO_U8;
  int channels = 2;
  int chunk = 512;

  Mix_OpenAudio(freq, fmt, channels, chunk);

  Mix_QuerySpec(&freq, &fmt, &channels);
  printf("freq = %i, fmt = %i, channels = %i\n", freq, fmt, channels);
  
  return 0;
}

Compiled with 2.0.14, it outputs:

hello world
version patch = 14
freq = 44100, fmt = 8, channels = 2

But with 2.0.16, it outputs:

hello world
version patch = 16
freq = 48000, fmt = 8, channels = 2

I'm running this on Windows 10.

[cgutman]

This seems to be expected behavior.Mix_OpenAudio() specifies SDL_AUDIO_ALLOW_FREQUENCY_CHANGE, so you can get back a frequency that's a better match for the hardware than you asked for.

See https://github.com/libsdl-org/SDL_mixer/blob/d8b31d2cde96ea7ca5e25895bc1b5743c6041fdd/src/mixer.c#L454-L459

This behavior doesn't seem to be documented in the Mix_OpenAudio() docs, but it is present in the Mix_QuerySpec() docs:
https://www.libsdl.org/projects/SDL_mixer/docs/SDL_mixer_15.html

Get the actual audio format in use by the opened audio device. This may or may not match the parameters you passed to Mix_OpenAudio.

If you need the frequency not to change, you can simply call Mix_OpenAudioDevice() directly and omit SDL_AUDIO_ALLOW_FREQUENCY_CHANGE

[Starbuck5]

Thanks for the reply!

I'm aware of how to get around this, pygame actually uses Mix_OpenAudioDevice() as well, and exposes SDL_AUDIO_ALLOW_FREQUENCY_CHANGE to its users.

I guess I made this report out of an abundance of caution, in response to the pre release window being the time to report regressions / problems.

It's strange to me that this behavior changed without any note in the release notes, and between SDL versions rather than SDL_Mixer versions.

Pygame has assumed that this will return with a frequency of 44100 for a while now, and I've never had the unittest fail, on multiple machines, and I've never heard any other contributor report it failing.

Even though it is technically within the documentation, this is still a change in behavior that people might be relying on, so I wanted to report it just to make sure everything is square before the full release.

[cgutman]

This seems like a likely culprit - b98b5ad

@icculus any thoughts on the risk of shipping this?

AUDCLNT_STREAMFLAGS_RATEADJUST is indeed only meant for minor adjustments to the frequency. It totally breaks when asked to resample when there's a significant difference between input and output frequency. I fixed a similar issue in libsoundio in andrewrk/libsoundio#194.

WASAPI does actually have a proper resampler available via AUDCLNT_STREAMFLAGS_AUTOCONVERTPCM and AUDCLNT_STREAMFLAGS_SRC_DEFAULT_QUALITY documented here. I've confirmed it works at least back to Windows 7 (haven't tried Vista). That would probably work as a drop-in replacement for AUDCLNT_STREAMFLAGS_RATEADJUST if we want to preserve the old behavior and also fix the resampling bug.

[icculus]

Pygame has assumed that this will return with a frequency of 44100 for a while now, and I've never had the unittest fail, on multiple machines, and I've never heard any other contributor report it failing.

That's because most targets can accept 44.1KHz so you haven't seen this fail recently. But there was a time in SDL2, before WASAPI used that flag we just removed, where it would have produced this result.

The unit test might often give the same result, but the unit test is asserting an incorrect truth.

[icculus]

(Also, having gotten burnt by adding that WASAPI flag in the first place, I'm extremely hesitant to add new ones while 2.0.16 is locked down unless we're extremely confident it's both necessary and safe.)

[Starbuck5]

Ok, this looks pretty cut and dry then.

On pygame's end this might be an SDL1 holdover, but it'll be easy enough to fix there.

If there aren't any loose ends I guess this issue should be closed.