widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Klaus Halfmann has proposed merging lp:~widelands-dev/widelands/bug-1743086-sdl-sound-bug-workaround into lp:widelands.

Commit message:
Workaround for Bug 1743086, completly disable sound in this case.

Test this on "Linux Budgie 4.13.0-38-generic #43-Ubuntu SMP Wed Mar 14 15:20:44 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux" works as expected

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1743086 in widelands: "ASAN memcopy overlap on Ubuntu Budgie from libSDL 2.0.6+dfsg1"
  https://bugs.launchpad.net/widelands/+bug/1743086

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1743086-sdl-sound-bug-workaround/+merge/343764

Actually Done by GunChleoc: 

Set a special flag that completly disables any sound, despite any user settigns, adds some log- and warning- messages to inform the User about this.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1743086-sdl-sound-bug-workaround into lp:widelands.

=== modified file 'src/sound/sound_handler.cc'
--- src/sound/sound_handler.cc	2018-04-07 16:59:00 +0000
+++ src/sound/sound_handler.cc	2018-04-22 11:58:26 +0000
@@ -66,7 +66,7 @@
 */
 SoundHandler::SoundHandler()
    : nosound_(false),
-     lock_audio_disabling_(false),
+     is_backend_disabled_(false),
      disable_music_(false),
      disable_fx_(false),
      music_volume_(MIX_MAX_VOLUME),
@@ -100,10 +100,33 @@
 	const uint16_t bufsize = 1024;
 #endif
 
+	SDL_version sdl_version;
+	SDL_GetVersion(&sdl_version);
+	log("**** SOUND REPORT ****\n");
+	log("SDL version: %d.%d.%d\n",
+		 static_cast<unsigned int>(sdl_version.major),
+		 static_cast<unsigned int>(sdl_version.minor),
+		 static_cast<unsigned int>(sdl_version.patch));
+
+	/// SDL 2.0.6 will crash due to upstream bug:
+	/// https://bugs.launchpad.net/ubuntu/+source/libsdl2/+bug/1722060
+	if (sdl_version.major == 2 && sdl_version.minor == 0 && sdl_version.patch == 6) {
+		log("Disabled sound due to a bug in SDL 2.0.6\n");
+		nosound_ = true;
+	}
+
+	SDL_MIXER_VERSION(&sdl_version);
+	log("SDL_mixer version: %d.%d.%d\n",
+		 static_cast<unsigned int>(sdl_version.major),
+		 static_cast<unsigned int>(sdl_version.minor),
+		 static_cast<unsigned int>(sdl_version.patch));
+
+	log("**** END SOUND REPORT ****\n");
+
 	if (nosound_) {
 		set_disable_music(true);
 		set_disable_fx(true);
-		lock_audio_disabling_ = true;
+		is_backend_disabled_ = true;
 		return;
 	}
 
@@ -144,7 +167,7 @@
 
 	set_disable_music(true);
 	set_disable_fx(true);
-	lock_audio_disabling_ = true;
+	is_backend_disabled_ = true;
 	return;
 }
 
@@ -225,6 +248,13 @@
 	load_fx_if_needed("sound", "lobby_freshmen", "lobby_freshmen");
 }
 
+/**
+ * Returns 'true' if the playing of sounds is disabled due to sound driver problems.
+ */
+bool SoundHandler::is_backend_disabled() const {
+	return is_backend_disabled_;
+}
+
 /** Load a sound effect. One sound effect can consist of several audio files
  * named EFFECT_XX.ogg, where XX is between 00 and 99.
  *
@@ -269,8 +299,9 @@
  * until the game is finished.
 */
 void SoundHandler::load_one_fx(const std::string& path, const std::string& fx_name) {
-	if (nosound_)
+	if (nosound_ || is_backend_disabled_) {
 		return;
+	}
 
 	FileRead fr;
 	if (!fr.try_open(*g_fs, path)) {
@@ -382,7 +413,7 @@
 void SoundHandler::play_fx(const std::string& fx_name,
                            int32_t const stereo_pos,
                            uint8_t const priority) {
-	if (nosound_)
+	if (nosound_ || is_backend_disabled_)
 		return;
 
 	assert(stereo_pos >= -1);
@@ -432,7 +463,7 @@
  * finished playing.
 */
 void SoundHandler::register_song(const std::string& dir, const std::string& basename) {
-	if (nosound_)
+	if (nosound_ || is_backend_disabled_)
 		return;
 	assert(g_fs);
 
@@ -461,7 +492,7 @@
  * or \ref change_music() this function will block until the fadeout is complete
 */
 void SoundHandler::start_music(const std::string& songset_name, int32_t fadein_ms) {
-	if (get_disable_music() || nosound_)
+	if (get_disable_music() || nosound_ || is_backend_disabled_)
 		return;
 
 	if (fadein_ms == 0)
@@ -543,7 +574,7 @@
  * get lost otherwise.
  */
 void SoundHandler::set_disable_music(bool const disable) {
-	if (lock_audio_disabling_ || disable_music_ == disable)
+	if (is_backend_disabled_ || disable_music_ == disable)
 		return;
 
 	if (disable) {
@@ -562,7 +593,7 @@
  * get lost otherwise.
 */
 void SoundHandler::set_disable_fx(bool const disable) {
-	if (lock_audio_disabling_)
+	if (is_backend_disabled_)
 		return;
 
 	disable_fx_ = disable;
@@ -578,7 +609,7 @@
  * \param volume The new music volume.
  */
 void SoundHandler::set_music_volume(int32_t volume) {
-	if (!lock_audio_disabling_ && !nosound_) {
+	if (!is_backend_disabled_ && !nosound_) {
 		music_volume_ = volume;
 		Mix_VolumeMusic(volume);
 		g_options.pull_section("global").set_int("music_volume", volume);
@@ -593,7 +624,7 @@
  * \param volume The new music volume.
  */
 void SoundHandler::set_fx_volume(int32_t volume) {
-	if (!lock_audio_disabling_ && !nosound_) {
+	if (!is_backend_disabled_ && !nosound_) {
 		fx_volume_ = volume;
 		Mix_Volume(-1, volume);
 		g_options.pull_section("global").set_int("fx_volume", volume);

=== modified file 'src/sound/sound_handler.h'
--- src/sound/sound_handler.h	2018-04-07 16:59:00 +0000
+++ src/sound/sound_handler.h	2018-04-22 11:58:26 +0000
@@ -176,6 +176,7 @@
 	void shutdown();
 	void read_config();
 	void load_system_sounds();
+	bool is_backend_disabled() const;
 
 	void load_fx_if_needed(const std::string& dir,
 	                       const std::string& basename,
@@ -220,12 +221,6 @@
 	// TODO(unknown): This is ugly. Find a better way to do it
 	bool nosound_;
 
-	/** Can disable_music_ and disable_fx_ be changed?
-	 * true = they mustn't be changed (e.g. because hardware is missing)
-	 * false = can be changed at user request
-	*/
-	bool lock_audio_disabling_;
-
 private:
 	// Prints an error and disables the sound system.
 	void initialization_error(const std::string& msg);
@@ -233,6 +228,12 @@
 	void load_one_fx(const std::string& path, const std::string& fx_name);
 	bool play_or_not(const std::string& fx_name, int32_t stereo_position, uint8_t priority);
 
+	/** Can sounds be played?
+	 * true = they mustn't be played (e.g. because hardware is missing)
+	 * false = can be played
+	*/
+	bool is_backend_disabled_;
+
 	/// Whether to disable background music
 	bool disable_music_;
 	/// Whether to disable sound effects

=== modified file 'src/ui_fsmenu/options.cc'
--- src/ui_fsmenu/options.cc	2018-04-07 16:59:00 +0000
+++ src/ui_fsmenu/options.cc	2018-04-22 11:58:26 +0000
@@ -259,6 +259,12 @@
 	box_sound_.add(&fx_);
 	box_sound_.add(&message_sound_);
 
+	if (g_sound_handler.is_backend_disabled()) {
+		UI::Textarea* sound_warning = new UI::Textarea(&box_sound_, 0, 0, _("Sound is disabled due to a problem with the sound driver"));
+		sound_warning->set_color(UI_FONT_CLR_WARNING);
+		box_sound_.add(sound_warning);
+	}
+
 	// Saving
 	box_saving_.add(&sb_autosave_);
 	box_saving_.add(&sb_rolling_autosave_);
@@ -327,10 +333,11 @@
 
 	// Sound options
 	music_.set_state(opt.music);
-	music_.set_enabled(!g_sound_handler.lock_audio_disabling_);
+	music_.set_enabled(!g_sound_handler.is_backend_disabled());
 	fx_.set_state(opt.fx);
-	fx_.set_enabled(!g_sound_handler.lock_audio_disabling_);
+	fx_.set_enabled(!g_sound_handler.is_backend_disabled());
 	message_sound_.set_state(opt.message_sound);
+	message_sound_.set_enabled(!g_sound_handler.is_backend_disabled());
 
 	// Saving options
 	zip_.set_state(opt.zip);

=== modified file 'src/wui/game_options_sound_menu.cc'
--- src/wui/game_options_sound_menu.cc	2018-04-07 16:59:00 +0000
+++ src/wui/game_options_sound_menu.cc	2018-04-22 11:58:26 +0000
@@ -21,6 +21,7 @@
 #include "base/i18n.h"
 #include "graphic/graphic.h"
 #include "sound/sound_handler.h"
+#include "ui_basic/multilinetextarea.h"
 
 GameOptionsSoundMenu::GameOptionsSoundMenu(InteractiveGameBase& gb,
                                            UI::UniqueWindow::Registry& registry)
@@ -62,11 +63,36 @@
 	ingame_music.set_state(!g_sound_handler.get_disable_music());
 	ingame_sound.set_state(!g_sound_handler.get_disable_fx());
 
-	if (g_sound_handler.lock_audio_disabling_) {  //  disabling sound options
+	uint32_t boxes_width =
+	   kStateboxSize + hspacing() + std::max(ingame_music.get_w(), ingame_sound.get_w());
+	uint32_t labels_width =
+	   std::max(ingame_music_volume_label.get_w(), ingame_sound_volume_label.get_w());
+
+	set_inner_size(std::max(static_cast<uint32_t>(get_inner_w()),
+	                        2 * hmargin() + std::max(boxes_width, labels_width)),
+	               2 * vmargin() + 2 * (kStateboxSize + vspacing()) + vbigspacing() +
+	                  3 * vspacing() + 2 * slideh() + ingame_music_volume_label.get_h() +
+	                  ingame_music_volume_label.get_h());
+
+	if (g_sound_handler.is_backend_disabled()) {  //  disabling sound options
 		ingame_music.set_enabled(false);
 		ingame_sound.set_enabled(false);
 		ingame_music_volume.set_enabled(false);
 		ingame_sound_volume.set_enabled(false);
+		UI::MultilineTextarea* sound_warning = new UI::MultilineTextarea(
+					this,
+					hmargin(),
+					ingame_sound_volume.get_y() + ingame_sound_volume.get_h() + vspacing(),
+					get_inner_w() - 2 * hmargin(),
+					0,
+					_("Sound is disabled due to a problem with the sound driver"),
+					UI::Align::kLeft,
+					g_gr->images().get("images/ui_basic/but4.png"),
+					UI::MultilineTextarea::ScrollMode::kNoScrolling);
+
+		sound_warning->set_color(UI_FONT_CLR_WARNING);
+		set_size(get_w(), get_h() + sound_warning->get_h() + vspacing());
+
 	} else {  // initial widget states
 		ingame_music.set_state(!g_sound_handler.get_disable_music());
 		ingame_sound.set_state(!g_sound_handler.get_disable_fx());
@@ -84,17 +110,6 @@
 	ingame_sound_volume.changedto.connect(
 	   boost::bind(&GameOptionsSoundMenu::sound_volume_changed, this, _1));
 
-	uint32_t boxes_width =
-	   kStateboxSize + hspacing() + std::max(ingame_music.get_w(), ingame_sound.get_w());
-	uint32_t labels_width =
-	   std::max(ingame_music_volume_label.get_w(), ingame_sound_volume_label.get_w());
-
-	set_inner_size(std::max(static_cast<uint32_t>(get_inner_w()),
-	                        2 * hmargin() + std::max(boxes_width, labels_width)),
-	               2 * vmargin() + 2 * (kStateboxSize + vspacing()) + vbigspacing() +
-	                  3 * vspacing() + 2 * slideh() + ingame_music_volume_label.get_h() +
-	                  ingame_music_volume_label.get_h());
-
 	if (get_usedefaultpos())
 		center_to_parent();
 }