← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/cleanup-soundhandler into lp:widelands

 

Thanks a lot for the review!

SDL_Mixer already uses threading for playing the sounds.

The only other spot where we could use some threading would be when registering sounds, but then the threads would fight over hard disk access and probably make things slower rather than faster. I already have another branch in the works that will speed up finding the files for animations and sounds by not using regular expressions, which will greatly speed up loading the tribes and world.

Diff comments:

> 
> === modified file 'src/economy/road.h'
> --- src/economy/road.h	2019-02-27 17:19:00 +0000
> +++ src/economy/road.h	2019-04-09 04:52:18 +0000
> @@ -131,8 +131,7 @@
>  	void cleanup(EditorGameBase&) override;
>  
>  	void draw(uint32_t gametime,
> -	          TextToDraw draw_text,
> -	          const Vector2f& point_on_dst,
> +	          TextToDraw draw_text, const Vector2f&, const Coords&,

Road::draw doesn't do anything. The variables have names on those map objects that do draw themselves.

>  	          float scale,
>  	          RenderTarget* dst) override;
>  
> 
> === modified file 'src/editor/editorinteractive.cc'
> --- src/editor/editorinteractive.cc	2019-02-27 17:19:00 +0000
> +++ src/editor/editorinteractive.cc	2019-04-09 04:52:18 +0000
> @@ -405,6 +405,10 @@
>  	is_painting_ = false;
>  }
>  
> +bool EditorInteractive::player_hears_field(const Widelands::Coords&) const {

Will add one in interactive_base.h

> +	return true;
> +}
> +
>  void EditorInteractive::on_buildhelp_changed(const bool value) {
>  	toggle_buildhelp_->set_perm_pressed(value);
>  }
> 
> === modified file 'src/sound/sound_handler.cc'
> --- src/sound/sound_handler.cc	2019-02-23 11:00:49 +0000
> +++ src/sound/sound_handler.cc	2019-04-09 04:52:18 +0000
> @@ -199,476 +176,461 @@
>  		fx_lock_ = nullptr;
>  	}
>  
> -	songs_.clear();
> -	fxs_.clear();
> -
>  	Mix_Quit();
>  	SDL_QuitSubSystem(SDL_INIT_AUDIO);
>  }
>  
> -/** Read the main config file, load background music and systemwide sound fx
> - *
> +/// Prints an error and disables and shuts down the sound system.
> +void SoundHandler::initialization_error(const char* const msg, bool quit_sdl) {
> +	log("WARNING: Failed to initialize sound system: %s\n", msg);
> +	SoundHandler::disable_backend();
> +	if (quit_sdl) {
> +		SDL_QuitSubSystem(SDL_INIT_AUDIO);
> +	}
> +	return;
> +}
> +
> +/**
> + * Load the sound options from g_options. If an option is not available, use the defaults set by the constructor.
>   */
>  void SoundHandler::read_config() {
> -	Section& s = g_options.pull_section("global");
> -
> -	if (nosound_) {
> -		set_disable_music(true);
> -		set_disable_fx(true);
> -	} 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", kDefaultMusicVolume);
> -		fx_volume_ = s.get_int("fx_volume", kDefaultFxVolume);
> -	}
> -
> -	random_order_ = s.get_bool("sound_random_order", true);
> -
> -	register_song("music", "intro");
> -	register_song("music", "menu");
> -	register_song("music", "ingame");
> -}
> -
> -/** Load systemwide sound fx into memory.
> - * \note This loads only systemwide fx. Worker/building fx will be loaded by
> - * their respective conf-file parsers
> - */
> -void SoundHandler::load_system_sounds() {
> -	load_fx_if_needed("sound", "click", "click");
> -	load_fx_if_needed("sound", "create_construction_site", "create_construction_site");
> -	load_fx_if_needed("sound", "message", "message");
> -	load_fx_if_needed("sound/military", "under_attack", "military/under_attack");
> -	load_fx_if_needed("sound/military", "site_occupied", "military/site_occupied");
> -	load_fx_if_needed("sound", "lobby_chat", "lobby_chat");
> -	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
> +	// TODO(GunChleoc): Compatibility code to avoid getting bug reports about unread sections. Remove after Build 21.
> +	if (g_options.get_section("sound") == nullptr) {
> +		Section& global = g_options.pull_section("global");
> +
> +		for (auto& option : sound_options_) {
> +			switch (option.first) {
> +			case SoundType::kMusic:
> +				option.second.volume = global.get_int("music_volume", option.second.volume);
> +				option.second.enabled = !global.get_bool("disable_music", !option.second.enabled);
> +				break;
> +			case SoundType::kChat:
> +				option.second.volume = global.get_int("fx_volume", option.second.volume);
> +				option.second.enabled = global.get_bool("sound_at_message", option.second.enabled);
> +				break;
> +			default:
> +				option.second.volume = global.get_int("fx_volume", option.second.volume);
> +				option.second.enabled = !global.get_bool("disable_fx", !option.second.enabled);
> +				break;
> +			}
> +		}
> +		save_config();
> +	}
> +
> +	// This is the code that we want to keep
> +	Section& sound = g_options.pull_section("sound");
> +	for (auto& option : sound_options_) {
> +		option.second.volume = sound.get_int(("volume_" + option.second.name).c_str(), option.second.volume);
> +		option.second.enabled = sound.get_bool(("enable_" + option.second.name).c_str(), option.second.enabled);
> +	}
> +}
> +
> +/// Save the current sound options to g_options
> +void SoundHandler::save_config() {
> +	Section& sound = g_options.pull_section("sound");
> +	for (auto& option : sound_options_) {
> +		const int volume = option.second.volume;
> +		const std::string& name = option.second.name;
> +		const bool enabled = option.second.enabled;
> +
> +		const std::string enable_name = "enable_" + name;
> +		sound.set_bool(enable_name.c_str(), enabled);
> +
> +		const std::string volume_name = "volume_" + name;
> +		sound.set_int(volume_name.c_str(), volume);
> +	}
> +}
> +
> +/// Read the sound options from g_options and apply them
> +void SoundHandler::load_config() {
> +	read_config();
> +	for (auto& option : sound_options_) {
> +		set_volume(option.first, option.second.volume);
> +		set_enable_sound(option.first, option.second.enabled);
> +	}
> +}
> +
> +/**
> + * Returns 'true' if the playing of sounds is disabled due to sound driver problems, or because disable_backend() was used.
> + */
> +bool SoundHandler::is_backend_disabled() {

Will do

> +	return SoundHandler::backend_is_disabled_;
> +}
> +
> +/**
> + * Disables all sound.
> + */
> +void SoundHandler::disable_backend() {
> +	SoundHandler::backend_is_disabled_ = true;
> +}
> +
> +/** Register a sound effect. One sound effect can consist of several audio files
>   * named EFFECT_XX.ogg, where XX is between 00 and 99.
>   *
>   * Subdirectories of and files under FILENAME_XX can be named anything you want.
>   *
> - * \param dir        The relative directory where the audio files reside in data/sound
> - * \param filename   Name from which filenames will be formed
> - *                   (BASENAME_XX.ogg);
> - *                   also the name used with \ref play_fx
> - */
> -void SoundHandler::load_fx_if_needed(const std::string& dir,
> -                                     const std::string& basename,
> -                                     const std::string& fx_name) {
> -	assert(g_fs);
> -
> -	if (!g_fs->is_directory(dir)) {
> -		throw wexception("SoundHandler: Can't load files from %s, not a directory!", dir.c_str());
> -	}
> -
> -	if (nosound_ || fxs_.count(fx_name) > 0)
> -		return;
> -
> -	fxs_.insert(std::make_pair(fx_name, std::unique_ptr<FXset>(new FXset())));
> -
> -	boost::regex re(basename + "_\\d+\\.ogg");
> -	FilenameSet files = filter(g_fs->list_directory(dir), [&re](const std::string& fn) {
> -		return boost::regex_match(FileSystem::fs_filename(fn.c_str()), re);
> -	});
> -
> -	for (const std::string& path : files) {
> -		assert(!g_fs->is_directory(path));
> -		load_one_fx(path, fx_name);
> -	}
> -}
> -
> -/** Add exactly one file to the given fxset.
> - * \param path      the effect to be loaded
> - * \param fx_name   the fxset to add the file to
> - * The file format must be ogg. Otherwise this call will complain and
> - * not load the file.
> - * \note The complete audio file will be loaded into memory and stays there
> - * until the game is finished.
> - */
> -void SoundHandler::load_one_fx(const std::string& path, const std::string& fx_name) {
> -	if (nosound_ || is_backend_disabled_) {
> -		return;
> -	}
> -
> -	FileRead fr;
> -	if (!fr.try_open(*g_fs, path)) {
> -		log("WARNING: Could not open %s for reading!\n", path.c_str());
> -		return;
> -	}
> -
> -	if (Mix_Chunk* const m =
> -	       Mix_LoadWAV_RW(SDL_RWFromMem(fr.data(fr.get_size(), 0), fr.get_size()), 1)) {
> -		// Make sure that requested FXset exists
> -
> -		assert(fxs_.count(fx_name) > 0);
> -
> -		fxs_[fx_name]->add_fx(m);
> -	} else
> -		log("SoundHandler: loading sound effect \"%s\" for FXset \"%s\" "
> -		    "failed: %s\n",
> -		    path.c_str(), fx_name.c_str(), Mix_GetError());
> -}
> -
> -/** Find out whether to actually play a certain effect right now or rather not
> - * (to avoid "sonic overload").
> - */
> -// TODO(unknown): What is the selection algorithm? cf class documentation
> -bool SoundHandler::play_or_not(const std::string& fx_name,
> -                               int32_t const stereo_pos,
> + * \param type       The category of the FxSet to create
> + * \param fx_path    The relative path and base filename from which filenames will be formed
> + *                   (<datadir>/fx_path_XX.ogg). If an effect with the same 'type' and 'fx_path' already exists, we assume that it is already registered and skip it.
> + * \returns  An ID for the effect that can be used to identify it in \ref play_fx.
> + */
> +
> +FxId SoundHandler::register_fx(SoundType type, const std::string& fx_path) {
> +	if (SoundHandler::is_backend_disabled() || g_sh == nullptr) {
> +		return kNoSoundEffect;
> +	}
> +	return g_sh->do_register_fx(type, fx_path);
> +}
> +
> +/// Non-static implementation of register_fx
> +FxId SoundHandler::do_register_fx(SoundType type, const std::string& fx_path) {
> +	assert(!SoundHandler::is_backend_disabled());
> +	if (fx_ids_[type].count(fx_path) == 0) {
> +		const FxId new_id = fxs_[type].size();
> +		fx_ids_[type].insert(std::make_pair(fx_path, new_id));
> +		fxs_[type].insert(std::make_pair(new_id, std::unique_ptr<FXset>(new FXset(fx_path, rng_.rand()))));
> +		return new_id;
> +	} else {
> +		return fx_ids_[type].at(fx_path);
> +	}
> +}
> +
> +/**
> + * Find out whether to actually play a certain effect right now or rather not
> + * (to avoid "sonic overload"). Based on priority and on when it was last played.
> + * System sounds and sounds with priority "kFxPriorityAlwaysPlay" always return 'true'.
> + */
> +bool SoundHandler::play_or_not(SoundType type, const FxId fx_id,

I don't think that it's more expensive than on trunk. I did not want to mess with the statistic functions in this branch.

It will definitely be faster for non-ambient sounds, because it will skip the calculations now.

>                                 uint8_t const priority) {
> -	bool allow_multiple = false;  //  convenience for easier code reading
> -	float evaluation;             // Temporary to calculate single influences
> -	float probability;            // Weighted total of all influences
> -
> -	if (nosound_)
> -		return false;
> -
> -	// Probability that this fx gets played; initially set according to priority
> -
> -	//  float division! not integer
> -	probability = (priority % PRIO_ALLOW_MULTIPLE) / 128.0f;
> -
> -	// TODO(unknown): what to do with fx that happen offscreen?
> -	// TODO(unknown): reduce volume? reduce priority? other?
> -	if (stereo_pos == -1) {
> -		return false;
> -	}
> -
> -	// TODO(unknown): check for a free channel
> -
> -	if (priority == PRIO_ALWAYS_PLAY) {
> -		// TODO(unknown): if there is no free channel, kill a running fx and complain
> -		return true;
> -	}
> -
> -	if (priority >= PRIO_ALLOW_MULTIPLE)
> -		allow_multiple = true;
> -
> -	// Find out if an fx called fx_name is already running
> -	bool already_running = false;
> -
> -	// Access to active_fx_ is protected because it can
> -	// be accessed from callback
> -	if (fx_lock_)
> -		SDL_LockMutex(fx_lock_);
> -
> -	// starting a block, so I can define a local type for iterating
> -	{
> +	assert(!SoundHandler::is_backend_disabled() && is_sound_enabled(type));
> +	assert(priority >= kFxPriorityLowest);
> +
> +	if (fxs_[type].count(fx_id) == 0) {
> +		return false;
> +	}
> +
> +	switch (type) {

Will do

> +	case SoundType::kAmbient:
> +		break;
> +	default:
> +		// We always play UI, chat and system sounds
> +		return true;
> +	}
> +
> +	// We always play important sounds
> +	if (priority == kFxPriorityAlwaysPlay) {
> +		return true;
> +	}
> +
> +	// Do not run multiple instances of the same sound effect if the priority is too low
> +	bool too_many_playing = false;
> +	if (priority < kFxPriorityAllowMultiple) {
> +		lock_fx();
> +		// Find out if an fx called 'fx_name' is already running
>  		for (const auto& fx_pair : active_fx_) {
> -			if (fx_pair.second == fx_name) {
> -				already_running = true;
> +			if (fx_pair.second == fx_id) {
> +				too_many_playing = true;
>  				break;
>  			}
>  		}
>  	}
>  
> -	if (fx_lock_)
> -		SDL_UnlockMutex(fx_lock_);
> +	release_fx_lock();
>  
> -	if (!allow_multiple && already_running)
> +	if (too_many_playing) {
>  		return false;
> +	}
>  
>  	// TODO(unknown): long time since any play increases weighted_priority
>  	// TODO(unknown): high general frequency reduces weighted priority
>  	// TODO(unknown): deal with "coupled" effects like throw_net and retrieve_net
>  
> -	uint32_t const ticks_since_last_play = SDL_GetTicks() - fxs_[fx_name]->last_used_;
> -
> -	//  reward an fx for being silent
> -	if (ticks_since_last_play > SLIDING_WINDOW_SIZE) {
> -		evaluation = 1;  //  arbitrary value; 0 -> no change, 1 -> probability = 1
> +	uint32_t const ticks_since_last_play = fxs_[type][fx_id]->ticks_since_last_play();
> +
> +	// Weighted total probability that this fx gets played; initially set according to priority
> +	//  float division! not integer
> +	float probability = (priority % kFxPriorityAllowMultiple) / static_cast<float>(kFxPriorityAllowMultiple);
> +
> +	// How many milliseconds in the past to consider
> +	constexpr uint32_t kSlidingWindowSize = 20000;
> +
> +	if (ticks_since_last_play > kSlidingWindowSize) { //  reward an fx for being silent
> +		const float evaluation = 1.0f;  //  arbitrary value; 0 -> no change, 1 -> probability = 1
>  
>  		//  "decrease improbability"
> -		probability = 1 - ((1 - probability) * (1 - evaluation));
> +		probability = 1.0f - ((1.0f - probability) * (1.0f - evaluation));
>  	} else {  // Penalize an fx for playing in short succession
> -		evaluation = static_cast<float>(ticks_since_last_play) / SLIDING_WINDOW_SIZE;
> +		const float evaluation = static_cast<float>(ticks_since_last_play) / kSlidingWindowSize;
>  		probability *= evaluation;  //  decrease probability
>  	}
>  
>  	// finally: the decision
>  	// float division! not integer
> -	return (rng_.rand() % 255) / 255.0f <= probability;
> +	return (rng_.rand() % kFxPriorityAlwaysPlay) / static_cast<float>(kFxPriorityAlwaysPlay) <= probability;
>  }
>  
> -/** \overload
> - * \param fx_name  The identifying name of the sound effect, see \ref load_fx .
> - * \param stereo_position  position in widelands' game window, see
> - *                         \ref stereo_position
> +/**
> + * \param type             The categorization of the sound effect to be played
> + * \param fx_id            The ID of the sound effect, see \ref register_fx
>   * \param priority         How important is it that this FX actually gets
>   *                         played? (see \ref FXset::priority_)
> + * \param stereo_position  Position in widelands' game window
> + * \param distance         Distance in widelands' game window
>   */
> -void SoundHandler::play_fx(const std::string& fx_name,
> +void SoundHandler::play_fx(SoundType type, const FxId fx_id,
> +						   uint8_t const priority,
>                             int32_t const stereo_pos,
> -                           uint8_t const priority) {
> -	if (nosound_ || is_backend_disabled_)
> -		return;
> -
> -	assert(stereo_pos >= -1);
> -	assert(stereo_pos <= 254);
> -
> -	if (get_disable_fx())
> -		return;
> -
> -	if (fxs_.count(fx_name) == 0) {
> -		log("SoundHandler: sound effect \"%s\" does not exist!\n", fx_name.c_str());
> +                           int distance) {
> +	if (SoundHandler::is_backend_disabled() || !is_sound_enabled(type)) {
> +		return;
> +	}
> +
> +	assert(stereo_pos >= kStereoLeft);
> +	assert(stereo_pos <= kStereoRight);
> +
> +	if (fx_id == kNoSoundEffect) {
> +		throw wexception("SoundHandler: Trying to play sound effect that was never registered. Maybe you registered it before instantiating g_sh?\n");
> +	}
> +
> +	if (fxs_[type].count(fx_id) == 0) {
> +		log("SoundHandler: Sound effect %d does not exist!\n", fx_id);
>  		return;
>  	}
>  
>  	// See if the FX should be played
> -	if (!play_or_not(fx_name, stereo_pos, priority))
> +	if (!play_or_not(type, fx_id, priority)) {
>  		return;
> +	}
>  
>  	//  retrieve the fx and play it if it's valid
> -	if (Mix_Chunk* const m = fxs_[fx_name]->get_fx()) {
> +	if (Mix_Chunk* const m = fxs_[type][fx_id]->get_fx(rng_.rand())) {
>  		const int32_t chan = Mix_PlayChannel(-1, m, 0);
>  		if (chan == -1) {
>  			log("SoundHandler: Mix_PlayChannel failed: %s\n", Mix_GetError());
>  		} else {
> -			Mix_SetPanning(chan, 254 - stereo_pos, stereo_pos);
> -			Mix_Volume(chan, get_fx_volume());
> +			Mix_SetPanning(chan, kStereoRight - stereo_pos, stereo_pos);
> +			Mix_SetDistance(chan, distance);
> +			Mix_Volume(chan, get_volume(type));
>  
> -			// Access to active_fx_ is protected
> -			// because it can be accessed from callback
> -			if (fx_lock_)
> -				SDL_LockMutex(fx_lock_);
> -			active_fx_[chan] = fx_name;
> -			if (fx_lock_)
> -				SDL_UnlockMutex(fx_lock_);
> +			lock_fx();
> +			active_fx_[chan] = fx_id;
> +			release_fx_lock();
>  		}
> -	} else
> -		log("SoundHandler: sound effect \"%s\" exists but contains no files!\n", fx_name.c_str());
> -}
> -
> -/** Load a background song. One "song" can consist of several audio files named
> +	} else {
> +		log("SoundHandler: Sound effect %d exists but contains no files!\n", fx_id);
> +	}
> +}
> +
> +/// Removes the given FXset from memory
> +void SoundHandler::remove_fx_set(SoundType type) {
> +	fxs_.erase(type);
> +	fx_ids_.erase(type);
> +}
> +
> +/**
> + * Register a background songset. A songset can consist of several audio files named
>   * FILE_XX.ogg, where XX is between 00 and 99.
>   * \param dir        The directory where the audio files reside.
>   * \param basename   Name from which filenames will be formed
> - *                   (BASENAME_XX.ogg); also the name used with \ref play_fx .
> - * This just registers the song, actual loading takes place when
> - * \ref Songset::get_song() is called, i.e. when the song is about to be
> + *                   (BASENAME_XX.ogg); also the name used with \ref change_music .
> + * This just registers the songs, actual loading takes place when
> + * \ref Songset::get_song() is called, i.e. when a song is about to be
>   * played. The song will automatically be removed from memory when it has
>   * finished playing.
>   */
> -void SoundHandler::register_song(const std::string& dir, const std::string& basename) {
> -	if (nosound_ || is_backend_disabled_)
> +void SoundHandler::register_songs(const std::string& dir, const std::string& basename) {
> +	if (SoundHandler::is_backend_disabled()) {
>  		return;
> -	assert(g_fs);
> -
> -	FilenameSet files;
> -
> -	files = filter(g_fs->list_directory(dir), [&basename](const std::string& fn) {
> -		const std::string only_filename = FileSystem::fs_filename(fn.c_str());
> -		return boost::starts_with(only_filename, basename) && boost::ends_with(only_filename, ".ogg");
> -	});
> -
> -	for (const std::string& filename : files) {
> -		assert(!g_fs->is_directory(filename));
> -		if (songs_.count(basename) == 0) {
> -			songs_.insert(std::make_pair(basename, std::unique_ptr<Songset>(new Songset())));
> -		}
> -		songs_[basename]->add_song(filename);
> +	}
> +	if (songs_.count(basename) == 0) {
> +		songs_.insert(std::make_pair(basename, std::unique_ptr<Songset>(new Songset(dir, basename))));
>  	}
>  }
>  
> -/** Start playing a songset.
> +/**
> + * Start playing a songset.
>   * \param songset_name  The songset to play a song from.
> - * \param fadein_ms     Song will fade from 0% to 100% during fadein_ms
> - *                      milliseconds starting from now.
> - * \note When calling start_music() while music is still fading out from
> - * \ref stop_music()
> - * or \ref change_music() this function will block until the fadeout is complete
> + * \note When calling start_music() while music is still fading out from \ref stop_music() 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_ || is_backend_disabled_)
> +void SoundHandler::start_music(const std::string& songset_name) {
> +	if (SoundHandler::is_backend_disabled() || !is_sound_enabled(SoundType::kMusic)) {
>  		return;
> -
> -	if (fadein_ms == 0)
> -		fadein_ms = 250;  //  avoid clicks
> -
> -	if (Mix_PlayingMusic())
> -		change_music(songset_name, 0, fadein_ms);
> -
> -	if (songs_.count(songset_name) == 0)
> +	}
> +
> +	if (Mix_PlayingMusic()) {
> +		change_music(songset_name, kMinimumMusicFade);
> +	}
> +
> +	if (songs_.count(songset_name) == 0) {
>  		log("SoundHandler: songset \"%s\" does not exist!\n", songset_name.c_str());
> -	else {
> -		if (Mix_Music* const m = songs_[songset_name]->get_song()) {
> -			Mix_FadeInMusic(m, 1, fadein_ms);
> +	} else {
> +		if (Mix_Music* const m = songs_[songset_name]->get_song(rng_.rand())) {
> +			Mix_FadeInMusic(m, 1, kMinimumMusicFade);
>  			current_songset_ = songset_name;
> -		} else
> +		} else {
>  			log("SoundHandler: songset \"%s\" exists but contains no files!\n", songset_name.c_str());
> +		}
>  	}
> -	Mix_VolumeMusic(music_volume_);
>  }
>  
> -/** Stop playing a songset.
> +/**
> + * Stop playing a songset.
>   * \param fadeout_ms Song will fade from 100% to 0% during fadeout_ms
>   *                   milliseconds starting from now.
>   */
> -void SoundHandler::stop_music(int32_t fadeout_ms) {
> -	if (get_disable_music() || nosound_)
> +void SoundHandler::stop_music(int fadeout_ms) {
> +	if (SoundHandler::is_backend_disabled()) {
>  		return;
> -
> -	if (fadeout_ms == 0)
> -		fadeout_ms = 250;  //  avoid clicks
> -
> -	Mix_FadeOutMusic(fadeout_ms);
> +	}
> +
> +	if (Mix_PlayingMusic()) {
> +		Mix_FadeOutMusic(std::max(fadeout_ms, kMinimumMusicFade));
> +	}
>  }
>  
> -/** Play an other piece of music.
> +/**
> + * Play a new piece of music.
>   * This is a member function provided for convenience. It is a wrapper around
>   * \ref start_music and \ref stop_music.
>   * \param fadeout_ms  Old song will fade from 100% to 0% during fadeout_ms
>   *                    milliseconds starting from now.
> - * \param fadein_ms   New song will fade from 0% to 100% during fadein_ms
> - *                    milliseconds starting from now.
>   * If songset_name is empty, another song from the currently active songset will
>   * be selected
>   */
>  void SoundHandler::change_music(const std::string& songset_name,
> -                                int32_t const fadeout_ms,
> -                                int32_t const fadein_ms) {
> -	if (nosound_)
> +                                int const fadeout_ms) {
> +	if (SoundHandler::is_backend_disabled()) {
>  		return;
> -
> -	std::string s = songset_name;
> -
> -	if (s == "")
> -		s = current_songset_;
> -	else
> -		current_songset_ = s;
> -
> -	if (Mix_PlayingMusic())
> +	}
> +
> +	if (!songset_name.empty()) {
> +		current_songset_ = songset_name;
> +	}
> +
> +	if (Mix_PlayingMusic()) {
>  		stop_music(fadeout_ms);
> -	else
> -		start_music(s, fadein_ms);
> -}
> -
> -bool SoundHandler::get_disable_music() const {
> -	return disable_music_;
> -}
> -bool SoundHandler::get_disable_fx() const {
> -	return disable_fx_;
> -}
> -int32_t SoundHandler::get_music_volume() const {
> -	return music_volume_;
> -}
> -int32_t SoundHandler::get_fx_volume() const {
> -	return fx_volume_;
> -}
> -
> -/** Normal set_* function, but the music must be started/stopped accordingly
> - * Also, the new value is written back to the config file right away. It might
> - * get lost otherwise.
> - */
> -void SoundHandler::set_disable_music(bool const disable) {
> -	if (is_backend_disabled_ || disable_music_ == disable)
> -		return;
> -
> -	if (disable) {
> -		stop_music();
> -		disable_music_ = true;
>  	} else {
> -		disable_music_ = false;
>  		start_music(current_songset_);
>  	}
> -
> -	g_options.pull_section("global").set_bool("disable_music", disable);
> -}
> -
> -/** Normal set_* function
> - * Also, the new value is written back to the config file right away. It might
> - * get lost otherwise.
> - */
> -void SoundHandler::set_disable_fx(bool const disable) {
> -	if (is_backend_disabled_)
> -		return;
> -
> -	disable_fx_ = disable;
> -
> -	g_options.pull_section("global").set_bool("disable_fx", disable);
> -}
> -
> -/**
> - * Normal set_* function.
> - * Set the music volume between 0 (muted) and \ref get_max_volume().
> - * The new value is written back to the config file.
> - *
> - * \param volume The new music volume.
> - */
> -void SoundHandler::set_music_volume(int32_t volume) {
> -	if (!is_backend_disabled_ && !nosound_) {
> -		music_volume_ = volume;
> +}
> +
> +/// Returns the currently playing songset
> +const std::string SoundHandler::current_songset() const {
> +	return current_songset_;
> +}
> +
> +/// Returns whether we want to hear sounds of the given 'type'
> +bool SoundHandler::is_sound_enabled(SoundType type) const {
> +	assert(sound_options_.count(type) == 1);
> +	return sound_options_.at(type).enabled;
> +}
> +
> +/// Returns the volume that the given 'type' of sound is to be played at
> +int32_t SoundHandler::get_volume(SoundType type) const {
> +	assert(sound_options_.count(type) == 1);
> +	return sound_options_.at(type).volume;
> +}
> +
> +/**
> + * Sets that we want to / don't want to hear the given 'type' of sounds. If the type is \ref SoundType::kMusic, start/stop the music as well.
> + */
> +void SoundHandler::set_enable_sound(SoundType type, bool const enable) {
> +	if (SoundHandler::is_backend_disabled()) {
> +		return;
> +	}
> +	assert(sound_options_.count(type) == 1);
> +
> +	SoundOptions& sound_options = sound_options_.at(type);
> +	sound_options.enabled = enable;
> +
> +	// Special treatment for music
> +	switch (type) {
> +	case SoundType::kMusic:
> +		if (enable) {
> +			if (!Mix_PlayingMusic()) {
> +				start_music(current_songset_);
> +			}
> +		} else {
> +			stop_music();
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +/**
> + * Sets the music or sound 'volume' for the given 'type' between 0 (muted) and \ref get_max_volume().
> + */
> +void SoundHandler::set_volume(SoundType type, int32_t volume) {
> +	if (SoundHandler::is_backend_disabled()) {
> +		return;
> +	}
> +
> +	assert(sound_options_.count(type) == 1);
> +	assert(volume >= 0 && volume <= get_max_volume());
> +
> +	sound_options_.at(type).volume = volume;
> +
> +	// Special treatment for music
> +	switch (type) {
> +	case SoundType::kMusic:
>  		Mix_VolumeMusic(volume);
> -		g_options.pull_section("global").set_int("music_volume", volume);
> -	}
> -}
> -
> -/**
> - * Normal set_* function
> - * Set the FX sound volume between 0 (muted) and \ref get_max_volume().
> - * The new value is written back to the config file.
> - *
> - * \param volume The new music volume.
> - */
> -void SoundHandler::set_fx_volume(int32_t volume) {
> -	if (!is_backend_disabled_ && !nosound_) {
> -		fx_volume_ = volume;
> +		break;
> +	default:
>  		Mix_Volume(-1, volume);
> -		g_options.pull_section("global").set_int("fx_volume", volume);
> +		break;
>  	}
>  }
>  
> -/** Callback to notify \ref SoundHandler that a song has finished playing.
> - * Usually, another song from the same songset will be started.
> - * There is a special case for the intro screen's music: only one song will be
> - * played. If the user has not clicked the mouse or pressed escape when the song
> - * finishes, Widelands will automatically go on to the main menu.
> +/**
> + * Returns the max value for volume settings. We use a function to hide
> + * SDL_mixer constants outside of sound_handler.
> + */
> +int32_t SoundHandler::get_max_volume() const {
> +	return MIX_MAX_VOLUME;
> +}
> +
> +/**
> + * Callback to notify \ref SoundHandler that a song has finished playing.
> + * Pushes an SDL_Event with type = SDL_USEREVENT and user.code = CHANGE_MUSIC.
>   */
>  void SoundHandler::music_finished_callback() {
>  	// DO NOT CALL SDL_mixer FUNCTIONS OR SDL_LockAudio FROM HERE !!!
>  
> +	assert(!SoundHandler::is_backend_disabled());
> +	// Trigger that we want a music change and leave the specifics to the application.
>  	SDL_Event event;
> -	if (g_sound_handler.current_songset_ == "intro") {
> -		// Special case for splashscreen: there, only one song is ever played
> -		event.type = SDL_KEYDOWN;
> -		event.key.state = SDL_PRESSED;
> -		event.key.keysym.sym = SDLK_ESCAPE;
> -	} else {
> -		// Else just play the next song - see general description for
> -		// further explanation
> -		event.type = SDL_USEREVENT;
> -		event.user.code = CHANGE_MUSIC;
> -	}
> +	event.type = SDL_USEREVENT;
> +	event.user.code = CHANGE_MUSIC;
>  	SDL_PushEvent(&event);
>  }
>  
> -/** Callback to notify \ref SoundHandler that a sound effect has finished
> - * playing.
> +/**
> + * Callback to notify \ref SoundHandler that a sound effect has finished
> + * playing. Removes the finished sound fx from the list of currently playing ones.
>   */
>  void SoundHandler::fx_finished_callback(int32_t const channel) {
>  	// DO NOT CALL SDL_mixer FUNCTIONS OR SDL_LockAudio FROM HERE !!!
>  
> +	assert(!SoundHandler::is_backend_disabled());
>  	assert(0 <= channel);
> -	g_sound_handler.handle_channel_finished(static_cast<uint32_t>(channel));
> +	g_sh->lock_fx();
> +	g_sh->active_fx_.erase(static_cast<uint32_t>(channel));
> +	g_sh->release_fx_lock();
>  }
>  
> -/** Remove a finished sound fx from the list of currently playing ones
> - * This is part of \ref fx_finished_callback
> - */
> -void SoundHandler::handle_channel_finished(uint32_t channel) {
> -	// Needs locking because active_fx_ may be accessed
> -	// from this callback or from main thread
> -	if (fx_lock_)
> +/// Lock the SDL mutex. Access to 'active_fx_' is protected by mutex because it can be accessed both from callbacks or from the main thread.
> +void SoundHandler::lock_fx() {
> +	if (fx_lock_) {
>  		SDL_LockMutex(fx_lock_);
> -	active_fx_.erase(channel);
> -	if (fx_lock_)
> +	}
> +}
> +
> +/// Release the SDL mutex
> +void SoundHandler::release_fx_lock() {
> +	if (fx_lock_) {
>  		SDL_UnlockMutex(fx_lock_);
> +	}
>  }


-- 
https://code.launchpad.net/~widelands-dev/widelands/cleanup-soundhandler/+merge/365001
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/cleanup-rendertarget.


References