[SDL_mixer] Fix Mix_PlayChannel audio 'leaking' and fix onended callback to correctly update audio status

[caiiiycuk]

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 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:

/* 
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.

[sbc100]

LGTM.

I wonder if we could add test for this? Perhaps updating an existed test_interactive.py test?

[caiiiycuk]

Thanks, will look into it.

[caiiiycuk]

1. Added test for Mix_PlayChannel
2. 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.
3. Fixed (2), added test for (2)
4. Description updated

@sbc100 please review, thanks!

[caiiiycuk]

Updated, squashed.

Do you think its a bug there this can be channelInfo.audio in once case but channelInfo.audio.webAudioNode in another case? Do you know why we have both of those modes?

I don't think it's a bug. As far as I understand, the implementation has 2 ways of loading sounds, first is preloaded audio it preloads sounds into audio elements, second option is using webAudio. If webAudio is not available, the implementation will fallback to audio elements anyway. The implementation tries to mimic webAudio in an audio-like way. So it's valid to have 2 cases there. Honestly, I never used 1st case (audio elements).

--
Do you know why check_clean.py is failing and what it is?

Check for clean checkout. This is run after tests during CI to ensure we are not polluting the source checkout.

Should I do some extra steps to clean workdir?

[sbc100]

Looks like closure is complaining:

Closure compiler completed with warnings and -Werror=closure enabled, aborting!

building:ERROR: /tmp/emtest_chk86dq6/emscripten_temp_uaqtpd90/hello_world.js:23757:56: WARNING - [JSC_TYPE_MISMATCH] No properties on this expression
found   : (null|undefined)
required: Object
  23757|         if (SDL.music.audio === this || SDL.music.audio.webAudioNode === this) {

Maybe SDL.music.audio?.webAudioNode == this?

[caiiiycuk]

Done