SDL
SDL copied to clipboard
libsdl-org
Rename SDL_MixAudioFormat and use float volume
The original SDL_MixAudio was removed, and SDL_MIX_MAXVOLUME was added in SDL1 before AUDIO_F32 existed. Doesn't seem much point being limited to 128 volume levels, given how easy it is to scale the float back to the 128 levels when necessary.
Also: A few audio functions take Uint8* ptrs / Uint32 lengths, rather than void* ptrs and int length. Is it worth changing these?
Also: A few audio functions take
Uint8*ptrs /Uint32lengths, rather thanvoid*ptrs andintlength. Is it worth changing these?
@icculus, thoughts? If we stick with void * then we should probably use size_t for consistency with other APIs.
The removal of SDL_MIX_MAXVOLUME should also be mentioned in docs/README-migration.md and added to build-scripts/SDL_migration.cocci and include/SDL3/SDL_oldnames.h: SDL_MIX_MAXVOLUME -> 1.0f
The removal of
SDL_MIX_MAXVOLUMEshould also be mentioned indocs/README-migration.mdand added tobuild-scripts/SDL_migration.cocciandinclude/SDL3/SDL_oldnames.h:SDL_MIX_MAXVOLUME->1.0f
I'll tweak the docs/cocci, but I'm not sure it should go in oldnames, since I don't think there's any other symbols redefined as constants in there.
The thinking on uint8, in the past, is that it makes it easy to step in bytes regardless of the actual type.
But honestly, apps can just set up a temporary Uint8 * variable if they want.
I'm fine with changing this.
Hey, @0x1F9F1, I just sent you a GitHub invite for write access to the repo, so feel free to click the "Rebase and merge" button on this one yourself. :)
(And future things, if they need feedback, can go in a PR, but otherwise we trust you to just push directly to the repo.)
Is the following OK for sdl2-compat?
diff --git a/src/sdl2_compat.c b/src/sdl2_compat.c
index 16c6d20..3805f63 100644
--- a/src/sdl2_compat.c
+++ b/src/sdl2_compat.c
@@ -5356,9 +5356,12 @@ SDL_AudioStreamFlush(SDL2_AudioStream *stream2)
return SDL3_FlushAudioStream(stream2 ? stream2->stream3 : NULL);
}
+#define SDL3_MIX_MAXVOLUME 128.0f
DECLSPEC void SDLCALL
SDL_MixAudioFormat(Uint8 *dst, const Uint8 *src, SDL_AudioFormat format, Uint32 len, int volume)
{
+ const float fvolume = volume / SDL3_MIX_MAXVOLUME;
+
/* SDL3 removed U16 audio formats. Convert to S16SYS. */
if ((format == SDL2_AUDIO_U16LSB) || (format == SDL2_AUDIO_U16MSB)) {
const Uint32 tmpsamples = len / sizeof (Uint16);
@@ -5369,11 +5372,11 @@ SDL_MixAudioFormat(Uint8 *dst, const Uint8 *src, SDL_AudioFormat format, Uint32
} else if (format == SDL2_AUDIO_U16MSB) {
AudioUi16MSBToSi16Sys(tmpbuf, (const Uint16 *) src, tmpsamples);
}
- SDL3_MixAudioFormat(dst, (const Uint8 *) tmpbuf, SDL_AUDIO_S16, tmpsamples * sizeof (Sint16), volume);
+ SDL3_MixAudio(dst, (const Uint8 *) tmpbuf, SDL_AUDIO_S16, tmpsamples * sizeof (Sint16), fvolume);
SDL3_free(tmpbuf);
}
} else {
- SDL3_MixAudioFormat(dst, src, format, len, volume);
+ SDL3_MixAudio(dst, src, format, len, fvolume);
}
}
diff --git a/src/sdl3_include_wrapper.h b/src/sdl3_include_wrapper.h
index 1cbf6df..bc94875 100644
--- a/src/sdl3_include_wrapper.h
+++ b/src/sdl3_include_wrapper.h
@@ -596,7 +596,7 @@
#define SDL_Metal_DestroyView IGNORE_THIS_VERSION_OF_SDL_Metal_DestroyView
#define SDL_Metal_GetLayer IGNORE_THIS_VERSION_OF_SDL_Metal_GetLayer
#define SDL_MinimizeWindow IGNORE_THIS_VERSION_OF_SDL_MinimizeWindow
-#define SDL_MixAudioFormat IGNORE_THIS_VERSION_OF_SDL_MixAudioFormat
+#define SDL_MixAudio IGNORE_THIS_VERSION_OF_SDL_MixAudio
#define SDL_OnApplicationDidBecomeActive IGNORE_THIS_VERSION_OF_SDL_OnApplicationDidBecomeActive
#define SDL_OnApplicationDidChangeStatusBarOrientation IGNORE_THIS_VERSION_OF_SDL_OnApplicationDidChangeStatusBarOrientation
#define SDL_OnApplicationDidEnterBackground IGNORE_THIS_VERSION_OF_SDL_OnApplicationDidEnterBackground
@@ -3318,8 +3318,8 @@
#undef SDL_MinimizeWindow
#endif
-#ifdef SDL_MixAudioFormat
-#undef SDL_MixAudioFormat
+#ifdef SDL_MixAudio
+#undef SDL_MixAudio
#endif
#ifdef SDL_OnApplicationDidBecomeActive
diff --git a/src/sdl3_syms.h b/src/sdl3_syms.h
index 6ae1e2a..dff5b49 100644
--- a/src/sdl3_syms.h
+++ b/src/sdl3_syms.h
@@ -821,7 +821,7 @@ SDL3_SYM(int,SetAudioStreamGetCallback,(SDL_AudioStream *a, SDL_AudioStreamCallb
SDL3_SYM(int,SetAudioStreamPutCallback,(SDL_AudioStream *a, SDL_AudioStreamCallback b, void *c),(a,b,c),return)
SDL3_SYM(void,DestroyAudioStream,(SDL_AudioStream *a),(a),)
SDL3_SYM(int,LoadWAV_IO,(SDL_IOStream *a, SDL_bool b, SDL_AudioSpec *c, Uint8 **d, Uint32 *e),(a,b,c,d,e),return)
-SDL3_SYM(int,MixAudioFormat,(Uint8 *a, const Uint8 *b, SDL_AudioFormat c, Uint32 d, int e),(a,b,c,d,e),return)
+SDL3_SYM(int,MixAudio,(Uint8 *a, const Uint8 *b, SDL_AudioFormat c, Uint32 d, float e),(a,b,c,d,e),return)
SDL3_SYM(int,GetSilenceValueForFormat,(SDL_AudioFormat a),(a),return)
SDL3_SYM(int,PauseAudioDevice,(SDL_AudioDeviceID a),(a),return)
SDL3_SYM(int,ResumeAudioDevice,(SDL_AudioDeviceID a),(a),return)
SDL_mixer uses SDL_MIX_MAXVOLUME a lot. Maybe Mix_VolumeMusic, Mix_MasterVolume, struct Mix_Chunk, struct _Mix_Channel and all others should be converted to use floats? Possibly a lot of work, but either way SDL_mixer is unusable at the moment.
Is the following OK for sdl2-compat?
Yeah I think that looks fine.
SDL_mixer uses SDL_MIX_MAXVOLUME a lot. Maybe Mix_VolumeMusic, Mix_MasterVolume, struct Mix_Chunk, struct _Mix_Channel and all others should be converted to use floats? Possibly a lot of work, but either way SDL_mixer is unusable at the moment.
Switching to floats is probably preferable, though it has its own MIX_MAX_VOLUME define, so it should be ok for now.
Either way, unfortunately I've run out of time and need to get back to uni work for now 🙃.
Is the following OK for sdl2-compat?
I would call it SDL2_MIX_MAXVOLUME, not SDL3, since technically this is the SDL2 value, but otherwise looks okay to me.
As for SDL_mixer, let's just throw in the right conversion when making calls into SDL3 and otherwise leave it alone for now. We haven't really had time to rethink its public API.
Is the following OK for sdl2-compat?
I would call it SDL2_MIX_MAXVOLUME, not SDL3, since technically this is the SDL2 value, but otherwise looks okay to me.
Changed, applied and pushed
As for SDL_mixer, let's just throw in the right conversion when making calls into SDL3 and otherwise leave it alone for now. We haven't really had time to rethink its public API.
Something like this, then? (Not tested)
diff --git a/include/SDL3_mixer/SDL_mixer.h b/include/SDL3_mixer/SDL_mixer.h
index 90acfa0..c371b92 100644
--- a/include/SDL3_mixer/SDL_mixer.h
+++ b/include/SDL3_mixer/SDL_mixer.h
@@ -187,7 +187,7 @@ extern DECLSPEC void SDLCALL Mix_Quit(void);
#define MIX_DEFAULT_FREQUENCY 44100
#define MIX_DEFAULT_FORMAT SDL_AUDIO_S16
#define MIX_DEFAULT_CHANNELS 2
-#define MIX_MAX_VOLUME SDL_MIX_MAXVOLUME /* Volume of a chunk */
+#define MIX_MAX_VOLUME 128 /* Volume of a chunk */
/**
* The internal format for an audio chunk
diff --git a/src/music.c b/src/music.c
index 90a82e3..1299763 100644
--- a/src/music.c
+++ b/src/music.c
@@ -317,7 +317,7 @@ int music_pcm_getaudio(void *context, void *data, int bytes, int volume,
if (volume == MIX_MAX_VOLUME) {
dst += consumed;
} else {
- SDL_MixAudioFormat(snd, dst, music_spec.format, (Uint32)consumed, volume);
+ SDL_MixAudio(snd, dst, music_spec.format, (Uint32)consumed, volume/(float)MIX_MAX_VOLUME);
snd += consumed;
}
len -= consumed;
@@ -1193,8 +1193,8 @@ int Mix_VolumeMusic(int volume)
if (volume < 0) {
return prev_volume;
}
- if (volume > SDL_MIX_MAXVOLUME) {
- volume = SDL_MIX_MAXVOLUME;
+ if (volume > MIX_MAX_VOLUME) {
+ volume = MIX_MAX_VOLUME;
}
music_volume = volume;
Mix_LockAudio();
diff --git a/src/mixer.c b/src/mixer.c
index 4a61ae5..4d2704c 100644
--- a/src/mixer.c
+++ b/src/mixer.c
@@ -396,6 +396,7 @@ mix_channels(void *udata, SDL_AudioStream *astream, int len, int total)
}
if (mix_channel[i].playing > 0) {
int volume = (master_vol * (mix_channel[i].volume * mix_channel[i].chunk->volume)) / (MIX_MAX_VOLUME * MIX_MAX_VOLUME);
+ float fvolume = (float)volume / (float)MIX_MAX_VOLUME;
int index = 0;
int remaining = len;
while (mix_channel[i].playing > 0 && index < len) {
@@ -406,7 +407,7 @@ mix_channels(void *udata, SDL_AudioStream *astream, int len, int total)
}
mix_input = Mix_DoEffects(i, mix_channel[i].samples, mixable);
- SDL_MixAudioFormat(stream+index, mix_input, mixer.format, mixable, volume);
+ SDL_MixAudio(stream+index, mix_input, mixer.format, mixable, fvolume);
if (mix_input != mix_channel[i].samples)
SDL_free(mix_input);
@@ -422,6 +423,7 @@ mix_channels(void *udata, SDL_AudioStream *astream, int len, int total)
/* Update the volume after the application callback */
volume = (master_vol * (mix_channel[i].volume * mix_channel[i].chunk->volume)) / (MIX_MAX_VOLUME * MIX_MAX_VOLUME);
+ fvolume = (float)volume / (float)MIX_MAX_VOLUME;
}
}
@@ -435,7 +437,7 @@ mix_channels(void *udata, SDL_AudioStream *astream, int len, int total)
}
mix_input = Mix_DoEffects(i, mix_channel[i].chunk->abuf, remaining);
- SDL_MixAudioFormat(stream+index, mix_input, mixer.format, remaining, volume);
+ SDL_MixAudio(stream+index, mix_input, mixer.format, remaining, fvolume);
if (mix_input != mix_channel[i].chunk->abuf)
SDL_free(mix_input);
@@ -530,16 +532,16 @@ int Mix_OpenAudio(SDL_AudioDeviceID devid, const SDL_AudioSpec *spec)
mix_channel[i].chunk = NULL;
mix_channel[i].playing = 0;
mix_channel[i].looping = 0;
- mix_channel[i].volume = SDL_MIX_MAXVOLUME;
- mix_channel[i].fade_volume = SDL_MIX_MAXVOLUME;
- mix_channel[i].fade_volume_reset = SDL_MIX_MAXVOLUME;
+ mix_channel[i].volume = MIX_MAX_VOLUME;
+ mix_channel[i].fade_volume = MIX_MAX_VOLUME;
+ mix_channel[i].fade_volume_reset = MIX_MAX_VOLUME;
mix_channel[i].fading = MIX_NO_FADING;
mix_channel[i].tag = -1;
mix_channel[i].expire = 0;
mix_channel[i].effects = NULL;
mix_channel[i].paused = 0;
}
- Mix_VolumeMusic(SDL_MIX_MAXVOLUME);
+ Mix_VolumeMusic(MIX_MAX_VOLUME);
_Mix_InitEffects();
@@ -1769,8 +1771,8 @@ int Mix_MasterVolume(int volume)
if (volume < 0) {
return prev_volume;
}
- if (volume > SDL_MIX_MAXVOLUME) {
- volume = SDL_MIX_MAXVOLUME;
+ if (volume > MIX_MAX_VOLUME) {
+ volume = MIX_MAX_VOLUME;
}
SDL_AtomicSet(&master_volume, volume);
return prev_volume;
As for SDL_mixer, let's just throw in the right conversion when making calls into SDL3 and otherwise leave it alone for now. We haven't really had time to rethink its public API.
Something like this, then? (Not tested)
https://github.com/libsdl-org/SDL_mixer/pull/612
Is anyone else getting an error when using the patch file after this commit?
plus: parse error:
File "/path/to/SDL-git/build-scripts/SDL_migration.cocci", line 3195, column 5, charpos = 50532
around = 'f',
whole content = + 1.0f
my spatch version:
spatch version 1.2-00001-gc94edf65 compiled with OCaml version 5.1.1
Flags passed to the configure script: --prefix=/usr --enable-release=yes --with-python=/usr/bin/python3
OCaml scripting support: yes
Python scripting support: yes
Syntax of regular expressions: PCRE
Is anyone else getting an error when using the patch file after this commit?
plus: parse error: File "/path/to/SDL-git/build-scripts/SDL_migration.cocci", line 3195, column 5, charpos = 50532 around = 'f', whole content = + 1.0fmy spatch version:
spatch version 1.2-00001-gc94edf65 compiled with OCaml version 5.1.1 Flags passed to the configure script: --prefix=/usr --enable-release=yes --with-python=/usr/bin/python3 OCaml scripting support: yes Python scripting support: yes Syntax of regular expressions: PCRE
I'm not too familiar with coccinelle, but seems like it doesn't actually support float literals? https://coccinelle.gitlabpages.inria.fr/website/docs/main_grammar011.html#const
My suggestion would be to replace the + 1.0f with one of these:
+ 1
+ 1.0
+ ((float)1)
This would at least make the patch file work again until we figure out a better way.
I would go for + ((float)1). It may not be the prettiest, but the other two can show warnings:
Implicit conversion from 'int' to 'float' may lose precisionclang(-Wimplicit-int-float-conversion)
Implicit conversion loses floating-point precision: 'double' to 'float'clang(-Wimplicit-float-conversion)