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

Improve coexistence of internal and external decoders #434

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

Conversation

ccawley2011
Copy link
Contributor

This is potentially useful to allow building with libvorbis, libFLAC and libmpg123 for the reasons described in #427 (comment), while still having the internal libraries as fallbacks if dynamic loading fails.

TODO:

  • Additional CMake changes to allow enabling both libvorbis and stb_vorbis
  • Should libvorbis, libFLAC and libmpg123 be enabled by default?

@slouken
Copy link
Collaborator

slouken commented Jul 31, 2022

If both are enabled and the external library is loaded dynamically, does it fall back to the internal codec if the external one fails?

If we do this, we probably want similar logic for SDL_image

@slouken
Copy link
Collaborator

slouken commented Jul 31, 2022

I'm kind of inclined not to do this, FYI. It's the worst of both worlds where you might or might not be using the internal or external codec, and it'll be harder to track down bugs. I'm inclined to have distributions use external codecs, and standalone builds use internal codecs and call it good.

@smcv
Copy link
Contributor

smcv commented Sep 19, 2023

I'm kind of inclined not to do this, FYI. It's the worst of both worlds where you might or might not be using the internal or external codec, and it'll be harder to track down bugs.

With my distribution hat on, it's also the worst of both worlds for security fixes: for each format, SDL_mixer would be vulnerable to CVEs in the internal codec and the external codec.

I'm inclined to have distributions use external codecs, and standalone builds use internal codecs and call it good.

I think that's a good policy.

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.

3 participants