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

[Feature Request] Add SDL_GetAudioDeviceSpec #2911

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 2 comments
Closed

[Feature Request] Add SDL_GetAudioDeviceSpec #2911

SDLBugzilla opened this issue Feb 11, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2018-05-29 21:40:59 +0000, Ethan Lee wrote:

The current SDL audio API does not provide a way to get a device's current default format.

For example, there is no way to ask how many channels/speakers an audio device has. At the moment the best chance you have is to hope that the backend sets device->spec.channels after opening, which is not always the case, so you may set want.channels to 0 and get have.channels == 2 even on a 5.1 system since that's the hardcoded fallback number...

https://hg.libsdl.org/SDL/file/757d81897470/src/audio/SDL_audio.c#l1113

... and even if it wasn't hardcoded, it's still hard for applications to present audio device options to users without knowing what's available first.

Right now I'm thinking this could be fixed with a single call that uses the SDL_AudioSpec structure, for consistency:

/* return SDL_SetError if unimplemented */
int SDL_GetAudioDeviceSpec(int index, SDL_AudioSpec *spec, int iscapture);

The numbers that matter to us for FAudio are freq and channels, though it may be possible for format and even samples to matter on certain platforms (for example, for some strange reason certain WASAPI devices like having samples being a multiple of 528?!). I'd be interested in hearing feedback/experiences on this.

For full context, here is where we'd be plugging this into FAudio:

https://github.com/flibitijibibo/FACT/blob/259716d335970a83b9052a593dc3ee05947c642f/src/FAudio_platform_sdl2.c#L417

TL;DR: We need this to fully reimplement IXAudio2::GetDeviceDetails.

On 2018-06-19 16:39:30 +0000, Ryan C. Gordon wrote:

This is something we've been running into too, I'm going to think on this for a bit and come up with a solution.

--ryan.

On 2018-08-02 19:40:30 +0000, Ethan Lee wrote:

This just went from "FAudio kind of needs this" to "how on Earth is any Windows application shipping without this" real quick...

WASAPI, as it turns out, does not work at all whatsoever if we do not have a way to detect the samples value that it asks for via IAudioClient_GetBufferSize. If your period size is too small it introduces skipping (or worse if the rest of the unused buffer is uninitialized), if your period size is too big it will cause a buffer overflow (!!!).

The REALLY serious problem with WASAPI is that we don't get this value until after the IAudioClient is made, which for WinRT appears to be asynchronous and only happens after RunAudio has started. It would probably take a lot of work to do this with ALLOW_SAMPLES_CHANGE (though I do still think that's important to have), but this could be a way to avoid having to depend on that.

For now FAudio has a hack since we're locked at 48KHz anyhow, but this is still extremely dangerous and may potentially be both a health/safety concern and a security issue when we're wrong:

FNA-XNA/FAudio@59e88cb

On 2018-09-05 19:35:23 +0000, Ethan Lee wrote:

Throwing some basic notes in here for later:

ALSA:

  • snd_pcm_hw_params_get_channels
  • snd_pcm_hw_params_get_rate

Pulse: pa_sample_spec, requires connecting to server first?

CoreAudio: AudioDeviceGetProperty

  • kAudioDevicePropertyStreamConfiguration (AudioBufferList)
  • kAudioDevicePropertyNominalSampleRate

WASAPI: IAudioClient_GetMixFormat, requires initializing the client first

DirectSound:

  • IDirectSound_GetSpeakerConfig, channel mask -> channel count
  • IDirectSound_GetCaps, dwMaxSecondarySampleRate?

On 2019-09-06 17:35:22 +0000, Ethan Lee wrote:

Created attachment 3956
GetAudioDeviceSpec stub

Wow, almost exactly a year... anyway:

I've had this stub laying around but I never got into the details of the implementation. It's basically a ripoff of GetAudioDeviceName, including the part where spec is stored in the device item list. This makes the function convenient to implement, but adds the issue of detecting the spec at device add time, so we'd have to modify AddAudioDevice and DetectDevices (including the default fallback) and pray that the query on each device doesn't add to init time.

The alternative is that we add a new function to AudioDriverImpl, but it looks like no other function of this type does this, so the only thing that would change in GetAudioDeviceSpec is passing the opaque handle to the implementation instead of doing that memcpy, if that makes any sense.

Any opinions? Depending on the preference I can probably throw together a prototype for WASAPI and Pulse, dunno about the others.

On 2020-03-10 18:22:52 +0000, Ethan Lee wrote:

Been occasionally reading audio APIs and how others get device info... I'm starting to wonder if we should just make the API honest and treat it more like OpenAudioDevice? It's kind of amazing how absolutely nobody lets you get basic device info without making a whole sound server connection first.

One of the main reasons we don't just do OpenAudioDevice with 0s for all the spec info is because SDL specifically gets itself in the way between the application and the backend:

https://hg.libsdl.org/SDL/file/e42055c6d8b5/src/audio/SDL_audio.c#l1152

For XAudio2/FAudio (and other middlewares like FMOD) the API is actually the same on paper; to get the system default you just pass 0 for channels/samplerate (and sometimes buffer size). This should be the same for SDL, but instead we get hardcoded values.

I can see two possible ways of dealing with this:

  1. Fix OpenAudioDevice to actually send 0 to the backends, and allow the backend to provide the real device mix format. This would totally solve it for FAudio and friends and I'd have no problem opening/closing just for format queries, but I don't know how many programs out there depend on those hardcoded defaults. If the system doesn't match SDL's numbers it could get ugly.

  2. Add the new SDL_GetAudioDeviceSpec func, but give it the same API as OpenAudioDevice (rather than something like GetAudioDeviceName) and document it as being effectively the same as opening and closing a device manually. This would allow us to do # 1 without having to worry about prepare_audiospec compatibility, but it's a whole new function and code path on the inside and people on the outside may be surprised by how slow the function may end up being (particularly on platforms like WinRT), unlike Open/Close where you know exactly what you've gotten yourself into.

Obviously this can wait until after 2.0.12, but I'd like to start thinking about this for 2.0.14 so I can finally get rid of the environment variable hack in FAudio and FMOD_SDL.

@Kontrabant
Copy link
Contributor

Kontrabant commented Feb 17, 2021

In Pipewire, you just query the property in the registry enumeration callback

const char *prop_val = spa_dict_lookup(props, PW_KEY_AUDIO_CHANNELS);
if(prop_val)
  channels = pw_properties_parse_int(prop_val);

Repeat with PW_KEY_AUDIO_RATE for the sample rate and PW_KEY_AUDIO_FORMAT for the device sample format if needed, although Pipewire processes everything as floats so feeding or requesting anything else will have conversion overhead.

@flibitijibibo
Copy link
Collaborator

Resolved by #4109

@icculus icculus closed this as completed Mar 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants