libsdl-org / SDL_mixer

An audio mixer that supports various file formats for Simple Directmedia Layer.
zlib License
433 stars 146 forks source link

load sounds via libsndfile #499

Closed fabiangreffrath closed 1 year ago

fabiangreffrath commented 1 year ago

This PR adds support for loading sounds via libsndfile in Mix_LoadWAV_RW() to make use of its extended format support, which by far exceeds the ability to load merely WAV/RIFF format files.

Pendant to https://github.com/libsdl-org/SDL/pull/7334

fabiangreffrath commented 1 year ago

Thanks for your feedback @madebr @sezero !

madebr commented 1 year ago
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 82413b32..9eb62ba5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -77,7 +77,7 @@ else()
 endif()
 option(SDL3MIXER_CMD "Support an external music player" ${sdl3mixer_cmd_default})

-option(SDL3MIXER_SNDFILE "Support loading sounds via libsndfile" ON)
+option(SDL3MIXER_SNDFILE "Support loading sounds via libsndfile" OFF)
 option(SDL3MIXER_SNDFILE_SHARED "Dynamically load libsndfile" "${SDL3MIXER_DEPS_SHARED}")

 option(SDL3MIXER_FLAC "Enable FLAC music" ON)
diff --git a/cmake/SDL3_mixerConfig.cmake.in b/cmake/SDL3_mixerConfig.cmake.in
index 8534e78d..f8969ac1 100644
--- a/cmake/SDL3_mixerConfig.cmake.in
+++ b/cmake/SDL3_mixerConfig.cmake.in
@@ -12,6 +12,8 @@ set(SDL3MIXER_VENDORED              @SDL3MIXER_VENDORED@)

 set(SDL3MIXER_CMD                   @SDL3MIXER_CMD@)

+set(SDL3MIXER_SNDFILE               @SDL3MIXER_SNDFILE@)
+
 set(SDL3MIXER_FLAC_LIBFLAC          @SDL3MIXER_FLAC_LIBFLAC@)
 set(SDL3MIXER_FLAC_DRFLAC           @SDL3MIXER_FLAC_DRFLAC@)

@@ -57,6 +59,10 @@ if(EXISTS "${CMAKE_CURRENT_LIST_DIR}/SDL3_mixer-static-targets.cmake")

     include(CMakeFindDependencyMacro)

+    if(SDL3MIXER_SNDFILE AND NOT SDL3MIXER_VENDORED AND NOT TARGET SndFile::sndfile)
+        find_dependency(SndFile)
+    endif()
+
     if(SDL3MIXER_FLAC_LIBFLAC AND NOT SDL3MIXER_VENDORED AND NOT TARGET FLAC::FLAC)
         find_dependency(FLAC)
     endif()
diff --git a/src/codecs/load_sndfile.c b/src/codecs/load_sndfile.c
index c8f341fa..29fe2aa7 100644
--- a/src/codecs/load_sndfile.c
+++ b/src/codecs/load_sndfile.c
@@ -25,12 +25,12 @@
   This file by Fabian Greffrath (fabian@greffrath.com).
 */

+#include "load_sndfile.h"
+
 #ifdef LOAD_SNDFILE

 #include <SDL3/SDL_loadso.h>

-#include "load_sndfile.h"
-
 #include <sndfile.h>

 static SNDFILE* (*SF_sf_open_virtual) (SF_VIRTUAL_IO *sfvirtual, int mode, SF_INFO *sfinfo, void *user_data);
@@ -198,7 +198,7 @@ done:
 #else

 SDL_AudioSpec *Mix_LoadSndFile_RW (SDL_RWops *src, int freesrc,
-        SDL_AudioSpec *spec, Uint8 **audio_buf, Uint32 *audio_len);
+        SDL_AudioSpec *spec, Uint8 **audio_buf, Uint32 *audio_len)
 {
     return NULL;
 }
fabiangreffrath commented 1 year ago
  • fix build for disabled sndfile

Ouch, sorry. Apparently I only tested building with sndfile enabled.

sezero commented 1 year ago

@slouken: What do you think?

fabiangreffrath commented 1 year ago

@slouken said "thumbs up" 😁

sezero commented 1 year ago

@slouken said "thumbs up" grin

OK then, he can apply this whenever he wants.

One question is whether we want this in the SDL2 branch too.

fabiangreffrath commented 1 year ago

Should I squash everything into one single clean commit?

sezero commented 1 year ago

Should I squash everything into one single clean commit?

That would be nice, IMO.

fabiangreffrath commented 1 year ago

Should I squash everything into one single clean commit?

That would be nice, IMO.

Done.

slouken commented 1 year ago

Looks good, thanks!

sezero commented 1 year ago

Do we want this in SDL2 branch, or do we keep it SDL3-only?

slouken commented 1 year ago

I think SDL3 only.

baidwwy commented 1 year ago

I think SDL3 only.

Please add to SDL2, I reallyreally need this https://github.com/libsdl-org/SDL_mixer/issues/332