Skip to content

Commit ce5f5bb

Browse files
authored
SDL_mixer 1.2: fix Mix_PlayChannel audio 'leaking'; fix onended callback to correctly update audio status (#22581)
### Mix_PlayChannel audio 'leaking' Hi, I found a bug while using builtin SDL_mixer 1.2. You can play many sounds on same channel in case if you do not manuall halt it before. Reagrding to [documentation](https://wiki.libsdl.org/SDL2_mixer/Mix_PlayChannel) of SDL: > If a specific channel was requested, and there is a chunk already playing there, that chunk will be halted and the new chunk will take its place. So, when you call ``` Mix_PlayChannel(channel, sound, loops); ``` It shoud internally halt the channel. Current emscripten implemntation is wrong, the previously runned chunk on that channel continue to play. You can easily reporoduce this with code: ```c++ /* compile with: emcc main.cpp -o main.html --preload-file "./@" */ #include <SDL/SDL_mixer.h> #include <chrono> #include <emscripten.h> Mix_Chunk* chunk = nullptr; uint64_t lastRun = 0; void loop() { using namespace std::chrono; uint64_t now = duration_cast<milliseconds>(system_clock::now().time_since_epoch()).count(); if (now - lastRun > 5000) { Mix_PlayChannel(0, chunk, -1); lastRun = now; } } int main(int argc, char** argv) { if ((Mix_Init(MIX_INIT_OGG) & MIX_INIT_OGG) != MIX_INIT_OGG) { printf("ERR! Mix_Init: Failed to init with required flags support!\n"); printf("ERR! Mix_Init: %s\n", Mix_GetError()); return -1; } if (Mix_OpenAudio(MIX_DEFAULT_FREQUENCY, MIX_DEFAULT_FORMAT, 1, 1024) == -1) { printf("ERR! Mix_OpenAudio: %s\n", Mix_GetError()); return -1; } Mix_AllocateChannels(1); chunk = Mix_LoadWAV("/music.ogg"); emscripten_set_main_loop(loop, 0, false); return 0; } ``` If you compile and run it then you will hear infinite looping of music.ogg, and every 5 sec new one is started. I fixed this by calling Mix_HaltChannel if channel is playing audio. ### fix onended callback to correctly update audio status Found another issue in SDL_mixer implementation. Current implementation does not detect playing status of sounds correctly (`Mix_Playing`, `Mix_PlayingMusic`). This happens because in on `onended` callback comparsion with `this` works only for case when audio created by cloning audio node, otherwise comparsion should be against audio.webAudioNode.
1 parent 91629e7 commit ce5f5bb

File tree

4 files changed

+106
-2
lines changed

4 files changed

+106
-2
lines changed

src/library_sdl.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2821,6 +2821,7 @@ var LibrarySDL = {
28212821
Mix_ReserveChannels: (num) => {
28222822
SDL.channelMinimumNumber = num;
28232823
},
2824+
Mix_PlayChannelTimed__deps: ['Mix_HaltChannel'],
28242825
Mix_PlayChannelTimed__proxy: 'sync',
28252826
Mix_PlayChannelTimed: (channel, id, loops, ticks) => {
28262827
// TODO: handle fixed amount of N loops. Currently loops either 0 or infinite times.
@@ -2864,9 +2865,14 @@ var LibrarySDL = {
28642865
audio.frequency = info.audio.frequency;
28652866
}
28662867
audio['onended'] = function() { // TODO: cache these
2867-
if (channelInfo.audio == this) { channelInfo.audio.paused = true; channelInfo.audio = null; }
2868+
if (channelInfo.audio === this || channelInfo.audio.webAudioNode === this) {
2869+
channelInfo.audio.paused = true; channelInfo.audio = null;
2870+
}
28682871
if (SDL.channelFinished) {{{ makeDynCall('vi', 'SDL.channelFinished') }}}(channel);
28692872
}
2873+
if (channelInfo.audio) {
2874+
_Mix_HaltChannel(channel);
2875+
}
28702876
channelInfo.audio = audio;
28712877
// TODO: handle N loops. Behavior matches Mix_PlayMusic
28722878
audio.loop = loops != 0;
@@ -2946,7 +2952,11 @@ var LibrarySDL = {
29462952
} else if (info.audio) { // Play via the <audio> element
29472953
audio = info.audio;
29482954
}
2949-
audio['onended'] = function() { if (SDL.music.audio == this) _Mix_HaltMusic(); } // will send callback
2955+
audio['onended'] = function() {
2956+
if (SDL.music.audio === this || SDL.music.audio?.webAudioNode === this) {
2957+
_Mix_HaltMusic(); // will send callback
2958+
}
2959+
}
29502960
audio.loop = loops != 0 && loops != 1; // TODO: handle N loops for finite N
29512961
audio.volume = SDL.music.volume;
29522962
SDL.music.audio = audio;
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#include <SDL/SDL_mixer.h>
2+
#include <emscripten.h>
3+
4+
Mix_Chunk* chunk = 0;
5+
uint64_t started = 0;
6+
uint64_t lastRun = 0;
7+
8+
void loop() {
9+
uint64_t now = emscripten_get_now();
10+
if (now - lastRun > 300) {
11+
Mix_PlayChannel(0, chunk, -1);
12+
lastRun = now;
13+
}
14+
if (now - started > 3000) {
15+
emscripten_force_exit(0);
16+
}
17+
}
18+
19+
int main(int argc, char** argv) {
20+
if ((Mix_Init(MIX_INIT_OGG) & MIX_INIT_OGG) != MIX_INIT_OGG) {
21+
printf("ERR! Mix_Init: Failed to init with required flags support!\n");
22+
printf("ERR! Mix_Init: %s\n", Mix_GetError());
23+
return 1;
24+
}
25+
26+
if (Mix_OpenAudio(MIX_DEFAULT_FREQUENCY, MIX_DEFAULT_FORMAT, 1, 1024) == -1) {
27+
printf("ERR! Mix_OpenAudio: %s\n", Mix_GetError());
28+
return 1;
29+
}
30+
31+
Mix_AllocateChannels(1);
32+
chunk = Mix_LoadWAV("/the_entertainer.ogg");
33+
if (!chunk) {
34+
printf("ERR! Mix_LoadWAV: %s\n", Mix_GetError());
35+
return 1;
36+
}
37+
38+
printf("The sound should reset to start every 300ms, if you hear overapplied music then the test has failed.\n");
39+
started = emscripten_get_now();
40+
emscripten_set_main_loop(loop, 0, 0);
41+
return 0;
42+
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#include <SDL/SDL_mixer.h>
2+
#include <emscripten.h>
3+
4+
Mix_Chunk* chunk = 0;
5+
uint64_t started = 0;
6+
7+
void loop() {
8+
uint64_t now = emscripten_get_now();
9+
if (now - started > 2000) {
10+
// length of sound is 1sec
11+
printf("isPlaying? %s\n", Mix_Playing(0) ? "yes" : "no");
12+
emscripten_force_exit(Mix_Playing(0) ? 1 : 0);
13+
}
14+
}
15+
16+
int main(int argc, char** argv) {
17+
if ((Mix_Init(MIX_INIT_OGG) & MIX_INIT_OGG) != MIX_INIT_OGG) {
18+
printf("ERR! Mix_Init: Failed to init with required flags support!\n");
19+
printf("ERR! Mix_Init: %s\n", Mix_GetError());
20+
return 1;
21+
}
22+
23+
if (Mix_OpenAudio(MIX_DEFAULT_FREQUENCY, MIX_DEFAULT_FORMAT, 1, 1024) == -1) {
24+
printf("ERR! Mix_OpenAudio: %s\n", Mix_GetError());
25+
return 1;
26+
}
27+
28+
Mix_AllocateChannels(1);
29+
chunk = Mix_LoadWAV("/noise.ogg");
30+
if (!chunk) {
31+
printf("ERR! Mix_LoadWAV: %s\n", Mix_GetError());
32+
return 1;
33+
}
34+
Mix_PlayChannel(0, chunk, 0);
35+
if (!Mix_Playing(0)) {
36+
printf("ERR! Channel is not playing\n");
37+
return 1;
38+
}
39+
started = emscripten_get_now();
40+
emscripten_set_main_loop(loop, 0, 0);
41+
return 0;
42+
}

test/test_interactive.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,16 @@ def test_sdl_audio_mix_channels(self, args):
8282

8383
self.btest_exit('test_sdl_audio_mix_channels.c', args=['-O2', '--minify=0', '--preload-file', 'sound.ogg'] + args)
8484

85+
def test_sdl_audio_mix_channels_halt(self):
86+
shutil.copy(test_file('sounds/the_entertainer.ogg'), '.')
87+
88+
self.btest_exit('test_sdl_audio_mix_channels_halt.c', args=['-O2', '--minify=0', '--preload-file', 'the_entertainer.ogg'])
89+
90+
def test_sdl_audio_mix_playing(self):
91+
shutil.copy(test_file('sounds/noise.ogg'), '.')
92+
93+
self.btest_exit('test_sdl_audio_mix_playing.c', args=['-O2', '--minify=0', '--preload-file', 'noise.ogg'])
94+
8595
@parameterized({
8696
'': ([],),
8797
'wasmfs': (['-sWASMFS'],),

0 commit comments

Comments
 (0)