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

Allow linking with -lSDL2_mixer to work properly #12633

Merged
merged 7 commits into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ Current Trunk
- dlopen, in conformace with the spec, now checks that one of either RTDL_LAZY
or RTDL_NOW flags ar set. Previously, it was possible set nether of these
without generating an error.
- Allow `-lSDL2_mixer` to just work. (Others like `-lSDL2` always worked, but
for `SDL2_mixer` things were broken because we build multiple variations of
that library.) That link flag is now the same as `-s USE_SDL2_MIXER=2`.
- Stack state is no longer stored in JavaScript. The following variables have
been replaced with native functions in `<emscripten/stack.h>`:
- STACK_BASE
Expand Down
2 changes: 2 additions & 0 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -3167,6 +3167,8 @@ def process_libraries(libs, lib_dirs, temp_files):
if jslibs is not None:
libraries += [(i, jslib) for jslib in jslibs]
consumed.append(i)
elif building.map_and_apply_to_settings(lib):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this just be map_to_settings ? I'm not sure either is that descriptive so I'm fine either way.

I guess if it was called map_to_settings then it would return the new settings and you would apply it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, but then returning a list of key mappings and applying them seems odd. a pretty complex interface, compared to just applying them there.

consumed.append(i)

shared.Settings.SYSTEM_JS_LIBRARIES += libraries

Expand Down
39 changes: 31 additions & 8 deletions tests/sdl2_mixer_wav.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include <assert.h>
#include <emscripten.h>
#include <SDL2/SDL.h>
#include <SDL2/SDL_mixer.h>
Expand All @@ -21,8 +22,18 @@ void sound_loop_then_quit() {
}

int main(int argc, char* argv[]){
if (SDL_Init(SDL_INIT_AUDIO) < 0)
return -1;
SDL_version version;
SDL_GetVersion(&version);
printf("Linked against SDL %d.%d.%d.\n",
version.major, version.minor, version.patch);
assert(version.major == 2);

if (SDL_Init(SDL_INIT_AUDIO) < 0) {
puts("Failed to init audio");
#ifdef REPORT_RESULT
REPORT_RESULT(100);
#endif
}
int const frequency = EM_ASM_INT_V({
var context;
try {
Expand All @@ -32,13 +43,25 @@ int main(int argc, char* argv[]){
}
return context.sampleRate;
});
if(Mix_OpenAudio(frequency, MIX_DEFAULT_FORMAT, 2, 1024) == -1)
return -1;
if(Mix_OpenAudio(frequency, MIX_DEFAULT_FORMAT, 2, 1024) == -1) {
puts("Failed to open audio");
#ifdef REPORT_RESULT
REPORT_RESULT(101);
#endif
}
wave = Mix_LoadWAV(WAV_PATH);
if (wave == NULL)
return -1;
if (Mix_PlayChannel(-1, wave, 0) == -1)
return -1;
if (wave == NULL) {
puts("Failed to load audio");
#ifdef REPORT_RESULT
REPORT_RESULT(102);
#endif
}
if (Mix_PlayChannel(-1, wave, 0) == -1) {
puts("Failed to play audio");
#ifdef REPORT_RESULT
REPORT_RESULT(103);
#endif
}
printf("Starting sound play loop\n");
emscripten_set_main_loop(sound_loop_then_quit, 0, 1);
return 0;
Expand Down
8 changes: 6 additions & 2 deletions tests/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -3121,10 +3121,14 @@ def test_sdl2_misc(self):
self.compile_btest(['test.o', '-s', 'USE_SDL=2', '-o', 'test.html'])
self.run_browser('test.html', '...', '/report_result?1')

@parameterized({
'dash_s': (['-s', 'USE_SDL=2', '-s', 'USE_SDL_MIXER=2'],),
'dash_l': (['-lSDL2', '-lSDL2_mixer'],),
})
@requires_sound_hardware
def test_sdl2_mixer_wav(self):
def test_sdl2_mixer_wav(self, flags):
shutil.copyfile(path_from_root('tests', 'sounds', 'the_entertainer.wav'), 'sound.wav')
self.btest('sdl2_mixer_wav.c', expected='1', args=['--preload-file', 'sound.wav', '-s', 'USE_SDL=2', '-s', 'USE_SDL_MIXER=2', '-s', 'INITIAL_MEMORY=33554432'])
self.btest('sdl2_mixer_wav.c', expected='1', args=['--preload-file', 'sound.wav', '-s', 'INITIAL_MEMORY=33554432'] + flags)

@parameterized({
'wav': ([], '0', 'the_entertainer.wav'),
Expand Down
20 changes: 20 additions & 0 deletions tools/building.py
Original file line number Diff line number Diff line change
Expand Up @@ -1465,6 +1465,26 @@ def map_to_js_libs(library_name):
return None


# map a linker flag to a Settings option, and apply it. this lets a user write
# -lSDL2 and it will have the same effect as -s USE_SDL=2.
def map_and_apply_to_settings(library_name):
# most libraries just work, because the -l name matches the name of the
# library we build. however, if a library has variations, which cause us to
# build multiple versions with multiple names, then we need this mechanism.
library_map = {
# SDL2_mixer's built library name contains the specific codecs built in.
'SDL2_mixer': [('USE_SDL_MIXER', 2)],
}

if library_name in library_map:
for key, value in library_map[library_name]:
logger.debug('Mapping library `%s` to settings changes: %s = %s' % (library_name, key, value))
setattr(shared.Settings, key, value)
return True

return False


def emit_wasm_source_map(wasm_file, map_file):
# source file paths must be relative to the location of the map (which is
# emitted alongside the wasm)
Expand Down