We’ve got way too many routines inside music.c and mixer.c that are powered by
massive, hard-to-read “switchboards” that look something like this:
static void music_internal_volume(Mix_Music *music_playing, int volume)
{
switch (music_playing->type) {
#ifdef CMD_MUSIC
case MUS_CMD:
MusicCMD_SetVolume(volume);
break;
#endif
#ifdef WAV_MUSIC
case MUS_WAV:
WAVStream_SetVolume(volume);
break;
#endif
#ifdef MODPLUG_MUSIC
case MUS_MODPLUG:
modplug_setvolume(music_playing->data.modplug, volume);
break;
#endif
#ifdef MOD_MUSIC
case MUS_MOD:
MOD_setvolume(music_playing->data.module, volume);
break;
#endif
#ifdef MID_MUSIC
case MUS_MID:
#ifdef USE_NATIVE_MIDI
if ( native_midi_ok ) {
native_midi_setvolume(volume);
} MIDI_ELSE
#endif
#ifdef USE_TIMIDITY_MIDI
if ( timidity_ok ) {
Timidity_SetVolume(volume);
}
#endif
break;
#endif
#ifdef OGG_MUSIC
case MUS_OGG:
OGG_setvolume(music_playing->data.ogg, volume);
break;
#endif
#ifdef FLAC_MUSIC
case MUS_FLAC:
FLAC_setvolume(music_playing->data.flac, volume);
break;
#endif
#ifdef MP3_MUSIC
case MUS_MP3:
smpeg.SMPEG_setvolume(music_playing->data.mp3,(int)(((float)volume/(float)MIX_MAX_VOLUME)*100.0));
break;
#endif
#ifdef MP3_MAD_MUSIC
case MUS_MP3_MAD:
mad_setVolume(music_playing->data.mp3_mad, volume);
break;
#endif
default:
/* Unknown music type?? */
break;
}
}
Not only is that ugly and hard to read, it’s also not very extensible if someone
wanted to add a new decoder. (Not that I do or anything; just speaking
hypothetically here.)
Seems to me this could be done a lot more cleanly by creating a decoder driver
struct much like the render driver structs in SDL 1.3, a table of functions that
correspond to each operation, which would be implemented in their own units.
Then the function above could read like this:
static void music_internal_volume(Mix_Music *music_playing, int volume)
{
music_playing.driver->SetVolume(music_playing.data, volume);
}
Any thoughts?
I’ve actually been toying around with this idea myself. An alternative to
the driver approach might be to structure the API like SDL_image. All of the
format-specific loaders would be exposed directly as well as being stored in
a table. If you compile without support for a given format, you would still
have a stub that simply returns NULL.
Either approach would mean more consistency and orthogonality. Good Things.On Thu, May 5, 2011 at 2:28 PM, Mason Wheeler wrote:
We’ve got way too many routines inside music.c and mixer.c that are powered
by
massive, hard-to-read “switchboards” that look something like this:
static void music_internal_volume(Mix_Music *music_playing, int volume)
{
switch (music_playing->type) {
#ifdef CMD_MUSIC
case MUS_CMD:
MusicCMD_SetVolume(volume);
break;
#endif
#ifdef WAV_MUSIC
case MUS_WAV:
WAVStream_SetVolume(volume);
break;
#endif
#ifdef MODPLUG_MUSIC
case MUS_MODPLUG:
modplug_setvolume(music_playing->data.modplug, volume);
break;
#endif
#ifdef MOD_MUSIC
case MUS_MOD:
MOD_setvolume(music_playing->data.module, volume);
break;
#endif
#ifdef MID_MUSIC
case MUS_MID:
#ifdef USE_NATIVE_MIDI
if ( native_midi_ok ) {
native_midi_setvolume(volume);
} MIDI_ELSE
#endif
#ifdef USE_TIMIDITY_MIDI
if ( timidity_ok ) {
Timidity_SetVolume(volume);
}
#endif
break;
#endif
#ifdef OGG_MUSIC
case MUS_OGG:
OGG_setvolume(music_playing->data.ogg, volume);
break;
#endif
#ifdef FLAC_MUSIC
case MUS_FLAC:
FLAC_setvolume(music_playing->data.flac, volume);
break;
#endif
#ifdef MP3_MUSIC
case MUS_MP3:
smpeg.SMPEG_setvolume(music_playing->data.mp3,(int)(((float)volume/(float)MIX_MAX_VOLUME)*100.0));
break;
#endif
#ifdef MP3_MAD_MUSIC
case MUS_MP3_MAD:
mad_setVolume(music_playing->data.mp3_mad, volume);
break;
#endif
default:
/* Unknown music type?? */
break;
}
}
Not only is that ugly and hard to read, it’s also not very extensible if
someone
wanted to add a new decoder. (Not that I do or anything; just speaking
hypothetically here.)
Seems to me this could be done a lot more cleanly by creating a decoder
driver
struct much like the render driver structs in SDL 1.3, a table of functions
that
correspond to each operation, which would be implemented in their own
units.
Then the function above could read like this:
static void music_internal_volume(Mix_Music *music_playing, int volume)
{
music_playing.driver->SetVolume(music_playing.data, volume);
}
Any thoughts?
SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org
–
Matthew Orlando
http://cogwheel.info
Seems to me this could be done a lot more cleanly by creating a decoder driver
struct much like the render driver structs in SDL 1.3, a table of functions that
correspond to each operation, which would be implemented in their own units.
Then the function above could read like this:
What you’re describing is a very good approach (it’s probably an
accident of history that SDL_mixer doesn’t do it like this already). If
someone sends me a patch to clean this up, I’ll get it into revision
control.
–ryan.
I would like to see that!
also from the development point of view it would be nice to merge
sdl_mixer, sdl_sound and al_mixer in a single sound library
_sound and openal could just be “drivers” for sdl_mixer
jm2c
VittorioOn Sun, May 8, 2011 at 9:15 PM, Ryan C. Gordon wrote:
Seems to me this could be done a lot more cleanly by creating a decoder
driver
struct much like the render driver structs in SDL 1.3, a table of
functions that
correspond to each operation, which would be implemented in their own
units.
Then the function above could read like this:
What you’re describing is a very good approach (it’s probably an accident of
history that SDL_mixer doesn’t do it like this already). If someone sends me
a patch to clean this up, I’ll get it into revision control.
–ryan.
SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org