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

cmake: allow building SDL_mixer with not all dependencies present + disable libflac by default #555

Merged
merged 3 commits into from
Sep 5, 2023

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Sep 5, 2023

  • When CMake is configured with -DSDL3MIXER_STRICT=OFF , you can build SDL3_mixer with some dependencies not present (that feature will be disabled). The default is -DSDL3MIXER_STRICT=OFF.
  • Make drflac and libflac mutually exclusive. By default, drflac is used. Disable libflac by default, keep drflac enabled.

fixes #553

This allows to allow building SDL_mixer without all dependencies
present.
@madebr madebr changed the title Cmake extra cmake: allow building SDL_mixer with not all dependencies present + make libflac/drflac mutually exclusive Sep 5, 2023
@sezero
Copy link
Contributor

sezero commented Sep 5, 2023

Make drflac and libflac mutually exclusive. By default, drflac is used.

Note that, as far as I can see, the issue doesn't happen with the drmp3 / mpg123 pair or with the libxmp / libmodplug pair: only happens with the drflac / libflac.

@madebr
Copy link
Contributor Author

madebr commented Sep 5, 2023

You can still configure with -DSDL3MIXER_MP3_DRMP3=ON -DSDL3MIXER_MP3_MPG123=ON.
This pr also added a small post-configuration summary of enabled/disabled backends.

@sezero
Copy link
Contributor

sezero commented Sep 5, 2023

You can still configure with -DSDL3MIXER_MP3_DRMP3=ON -DSDL3MIXER_MP3_MPG123=ON.

Yes, but now that you made drflac and libflac mutually exclusive, the same won't be true for them: That's why I added my note.

@madebr
Copy link
Contributor Author

madebr commented Sep 5, 2023

You can still configure with -DSDL3MIXER_MP3_DRMP3=ON -DSDL3MIXER_MP3_MPG123=ON.

Yes, but now that you made drflac and libflac mutually exclusive, the same won't be true for them: That's why I added my note.

Sorry for me being so verbose, your use of the word "issue" confused me.

@sezero
Copy link
Contributor

sezero commented Sep 5, 2023

FWIW, the following 'remedies' the original issue I reported

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d344b77..c28ff08 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -101,7 +101,7 @@ option(SDL3MIXER_SNDFILE_SHARED "Dynamically load libsndfile" "${SDL3MIXER_DEPS_
 
 option(SDL3MIXER_FLAC "Enable FLAC music" ON)
 
-cmake_dependent_option(SDL3MIXER_FLAC_LIBFLAC "Enable FLAC music using libFLAC" ON SDL3MIXER_FLAC OFF)
+cmake_dependent_option(SDL3MIXER_FLAC_LIBFLAC "Enable FLAC music using libFLAC" OFF SDL3MIXER_FLAC OFF)
 cmake_dependent_option(SDL3MIXER_FLAC_LIBFLAC_SHARED "Dynamically load LIBFLAC" "${SDL3MIXER_DEPS_SHARED}" SDL3MIXER_FLAC_LIBFLAC OFF)
 
 cmake_dependent_option(SDL3MIXER_FLAC_DRFLAC "Enable FLAC music using drflac" ON SDL3MIXER_FLAC OFF)

@madebr madebr changed the title cmake: allow building SDL_mixer with not all dependencies present + make libflac/drflac mutually exclusive cmake: allow building SDL_mixer with not all dependencies present + disable libflac by default Sep 5, 2023
Copy link
Contributor

@sezero sezero left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@sezero
Copy link
Contributor

sezero commented Sep 5, 2023

P.S.: Having an equivalent of it for SDL2 branch would be good (or at least a bare minimum fix for drflac/libfac thing).

P.S./2: Having an equivalent of this for SDL_image would be good.

@madebr madebr merged commit 8b525a9 into libsdl-org:main Sep 5, 2023
5 checks passed
@madebr madebr deleted the cmake-extra branch September 5, 2023 20:03
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.

both of drflac and libflac versions are built
2 participants