widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

SirVer has proposed merging lp:~widelands-dev/widelands/cleanup_mixer_init into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1226137 in widelands: "Check for ogg support in SDL_mixer and show warning"
  https://bugs.launchpad.net/widelands/+bug/1226137

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/cleanup_mixer_init/+merge/245266

Cleanup SoundHandler init code a bit and report an error if Ogg is not available.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/cleanup_mixer_init into lp:widelands.

=== modified file 'src/sound/sound_handler.cc'
--- src/sound/sound_handler.cc	2014-12-03 07:15:40 +0000
+++ src/sound/sound_handler.cc	2014-12-20 18:20:27 +0000
@@ -44,8 +44,17 @@
 #include "wui/mapview.h"
 #include "wui/mapviewpixelfunctions.h"
 
-#define DEFAULT_MUSIC_VOLUME  64
-#define DEFAULT_FX_VOLUME    128
+namespace  {
+
+constexpr int kDefaultMusicVolume = 64;
+constexpr int kDefaultFxVolume = 128;
+
+void report_initalization_error(const char* msg) {
+	log("WARNING: Failed to initialize sound system: %s\n", msg);
+	return;
+}
+
+}  // namespace
 
 /** The global \ref SoundHandler object
  * The sound handler is a static object because otherwise it'd be quite
@@ -99,23 +108,27 @@
 	const uint16_t bufsize = 1024;
 #endif
 
-	if (nosound_)
-	{
-		set_disable_music(true);
-		set_disable_fx(true);
-		lock_audio_disabling_ = true;
-		return;
-	}
-
-	if (SDL_InitSubSystem(SDL_INIT_AUDIO) == -1 ||
-	    Mix_OpenAudio(MIX_DEFAULT_FREQUENCY, MIX_DEFAULT_FORMAT, 2, bufsize) == -1 ||
-	    Mix_Init(MIX_INIT_OGG) == -1) {
-		SDL_QuitSubSystem(SDL_INIT_AUDIO);
-		log("WARNING: Failed to initialize sound system: %s\n", Mix_GetError());
-
-		set_disable_music(true);
-		set_disable_fx(true);
-		lock_audio_disabling_ = true;
+	if (nosound_) {
+		set_disable_music(true);
+		set_disable_fx(true);
+		lock_audio_disabling_ = true;
+		return;
+	}
+
+	if (SDL_InitSubSystem(SDL_INIT_AUDIO) != 0) {
+		report_initalization_error(SDL_GetError());
+		return;
+	}
+
+	if (Mix_OpenAudio(MIX_DEFAULT_FREQUENCY, MIX_DEFAULT_FORMAT, 2, bufsize) != 0) {
+		initialization_error(Mix_GetError());
+		return;
+	}
+
+	constexpr int kMixInitFlags = MIX_INIT_OGG;
+	int initted = Mix_Init(kMixInitFlags);
+	if ((initted & kMixInitFlags) != kMixInitFlags) {
+		initialization_error("No Ogg support in SDL_Mixer.");
 		return;
 	}
 
@@ -128,6 +141,17 @@
 		fx_lock_ = SDL_CreateMutex();
 }
 
+void SoundHandler::initialization_error(const std::string& msg) {
+	log("WARNING: Failed to initialize sound system: %s\n", msg.c_str());
+
+	SDL_QuitSubSystem(SDL_INIT_AUDIO);
+
+	set_disable_music(true);
+	set_disable_fx(true);
+	lock_audio_disabling_ = true;
+	return;
+}
+
 void SoundHandler::shutdown()
 {
 	Mix_ChannelFinished(nullptr);
@@ -184,8 +208,8 @@
 	} else {
 		set_disable_music(s.get_bool("disable_music",      false));
 		set_disable_fx   (s.get_bool("disable_fx",         false));
-		music_volume_ =  s.get_int ("music_volume",       DEFAULT_MUSIC_VOLUME);
-		fx_volume_    =  s.get_int ("fx_volume",          DEFAULT_FX_VOLUME);
+		music_volume_ =  s.get_int ("music_volume",       kDefaultMusicVolume);
+		fx_volume_    =  s.get_int ("fx_volume",          kDefaultFxVolume);
 	}
 
 	random_order_    =  s.get_bool("sound_random_order", true);

=== modified file 'src/sound/sound_handler.h'
--- src/sound/sound_handler.h	2014-12-03 07:15:40 +0000
+++ src/sound/sound_handler.h	2014-12-20 18:20:27 +0000
@@ -243,6 +243,9 @@
 	bool lock_audio_disabling_;
 
 protected:
+	// Prints an error and disables the sound system.
+	void initialization_error(const std::string& msg);
+
 	void load_one_fx(const char * filename, const std::string & fx_name);
 	int32_t stereo_position(Widelands::Coords position);
 	bool play_or_not