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

SDL_emscriptenaudio.c only supports one default audio output device #5485

Closed
connorjclark opened this issue Apr 2, 2022 · 13 comments
Closed
Assignees

Comments

@connorjclark
Copy link
Contributor

connorjclark commented Apr 2, 2022

I have an application that opens an output device for music (via SDL_Mixer) and for sfx/voices. With the current emscripten audio implementation, only the first opened device will work, and the subsequent one fails.

A cursory inspection of the emscripten audio showed me that it explicitly only supports one output device. In a moment of sheer hubris I thought simply changing OnlyHasDefaultOutputDevice from 1 to 0 might fix my problem ... and it did. I have no idea if this is just happened to work for me (now SFX and music work in my application) because I got lucky, or if this really is all that is necessary to support multiple output devices.

I don't know what would happen if I were to attempt closing these opened device handles (no need to do that in my application), so perhaps that is the only place where the support is lacking for multiple devices.

In the meantime, I've been patching my copy of SDL to fix this in my build script:

sed -i -e 's/impl->OnlyHasDefaultOutputDevice = 1/impl->OnlyHasDefaultOutputDevice = 0/' "$EMCC_CACHE_DIR/ports/sdl2/SDL-release-2.0.20/src/audio/emscripten/SDL_emscriptenaudio.c"
@slouken slouken added this to the 2.0.22 milestone Apr 3, 2022
@icculus
Copy link
Collaborator

icculus commented Apr 4, 2022

Just to be clear: you're calling SDL_OpenAudioDevice() twice to get two audio callbacks that mix together into one output stream, not trying to get two unique pieces of audio hardware to play different audio streams (like the computer's speakers for game sounds and some USB headphones for voice chat), right?

@connorjclark
Copy link
Contributor Author

That's correct. Well, in one case SDL_mixer is the one opening the audio callback (for music) and in the other SDL_OpenAudioDevice is for SFX. And under the hood emscripten audio opens two different audio nodes in JS, so perhaps that does fit SDL's abstraction of two different audio devices in a way?

I can provide a small example code if my use case isn't clear.

@icculus
Copy link
Collaborator

icculus commented Apr 4, 2022

I think the problem is this is going to break on other platforms where this assumption isn't true; it would be better to use Mix_SetPostMix so you can write your sound effects into the same buffer that SDL_mixer is using for music.

It would only be a few lines of code, I think; I'll write up an example here today.

@flibitijibibo
Copy link
Collaborator

I'd still like to know if it's intentional to be able to have multiple connections in Emscripten though; some applications have multiple connections for various reasons (in FNA for example we can have more than 3 sometimes!) so if nothing in the platform blocks this it'd be great to have here too.

@icculus
Copy link
Collaborator

icculus commented Apr 5, 2022

I'd still like to know if it's intentional to be able to have multiple connections in Emscripten though

In the sense that Emscripten doesn't expose hardware devices, just an object that contributes to the mix that goes to the real hardware, then yes, at the Emscripten/DOM layer this is intentional. But SDL intends devices to be opened once, even if it happens to work on some platforms to open a device more than once.

As such, SDL flags Emscripten as only allowing a default device (which is accurate), and the higher level code sees this and refuses to allow it to be opened multiple times.

@flibitijibibo
Copy link
Collaborator

This might be something to update for 3.0... all the major audio systems support multiple server connections for one device (including consoles even!) so the other cases (ALSA, dsp) should eventually be considered the odd ones out. Even disk could support multiple streams if we adjusted the filename to include the SDL device ID or something.

@connorjclark
Copy link
Contributor Author

BTW the web platform only has experimental support for selecting audio devices other than the default system audio device. Here are the details:

https://w3c.github.io/mediacapture-output/

https://caniuse.com/?search=setSinkId

https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/setSinkId

@icculus
Copy link
Collaborator

icculus commented Apr 5, 2022

More likely outcome for SDL3: permitting multiple callbacks and mixing it ourselves. This is actually what y'all are asking for here anyhow, I think.

@icculus
Copy link
Collaborator

icculus commented Apr 5, 2022

(Although perhaps it is desirable to pass this on to the system, where PulseAudio and Windows 10, etc, can show a system UI for volume controls, and a user could theoretically use this to adjust the music vs in-game effects separately.)

@flibitijibibo
Copy link
Collaborator

For accuracy's sake we would absolutely want the OS to do this - this allows FAudio to replicate how XAudio2 connects to the system, and is in fact required for some games to work at all (for instance, games will make a context with a high-quality format, then make a second context with a vastly different format that makes hand-mixing really hard and annoying). That said, a fallback for the cases like ALSA would be great so I don't have to see "resource busy" messages anymore!

@icculus
Copy link
Collaborator

icculus commented Apr 5, 2022

I'm going to kick this out of 2.0.22 in any case, but for now, it happens to be safe to open Emscripten audio devices multiple times, as far as I can tell, for those that want to patch out the check that prevents that.

@icculus icculus removed this from the 2.0.22 milestone Apr 5, 2022
@icculus
Copy link
Collaborator

icculus commented Jul 12, 2023

(This will be fixed in SDL3 once #7704 lands.)

@slouken
Copy link
Collaborator

slouken commented Nov 7, 2023

It landed!

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

No branches or pull requests

4 participants