Skip to content

Support formats option in sdl2 mixer #24158

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ryanking13
Copy link
Contributor

This PR adds handle_options function in the SDL2_mixer port so one can build SDL2_mixer with specific formats with embuilder, such as

embuilder build sdl2_mixer:formats:mid,ogg,mp3

I found this syntax was added in #21345, but it was only applied to SDL2_image.

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Can you add a test for this?

Perhaps another test alongside test_sdl2_mixer_wav in test_other.py


def needed(settings):
return settings.USE_SDL_MIXER == 2


def get_formats(settings):
return set(settings.SDL2_MIXER_FORMATS).union(opts['formats'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can just write this as opts['formats'].union(settings.SDL2_MIXER_FORMATS) (i.e. union can take a list)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I changed it to opts['formats'].union(settings.SDL2_MIXER_FORMATS) (and update the similar code in sdl2_image as well).

@ryanking13
Copy link
Contributor Author

I added some tests, but it seems like FROZEN_CACHE is not happy with sdl2_mixer variants, hmm...

# There are tests that using libraries
# that are not included by "./embuilder build ALL". For example the
# different variant of libSDL2_mixer is used by test_sdl2_mixer_wav
frozen_cache: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better not to do this. Instead you can add the variant you are testing the variants list in sdl2_mixer.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks I removed this and used ogg format for a test case.

@@ -2470,6 +2470,7 @@ def test_sdl_get_key_name(self):
def test_sdl2_mixer_wav(self):
self.emcc(test_file('browser/test_sdl2_mixer_wav.c'), ['-sUSE_SDL_MIXER=2'], output_filename='a.out.js')
self.emcc(test_file('browser/test_sdl2_mixer_wav.c'), ['--use-port=sdl2_mixer'], output_filename='a.out.js')
self.emcc(test_file('browser/test_sdl2_mixer_wav.c'), ['--use-port=sdl2_mixer:formats=ogg'], output_filename='a.out.js')
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does this test (which plays a wav file) work if only ogg is selected? Is this because wav is simply always available maybe?

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

Successfully merging this pull request may close these issues.

2 participants