Review: Approve reiew, compile, testplay I played some internet game with this brnahc and it was ok, felt a bit slow, though.
Completed the review, code is fine, please check some nits: * some functions should be inlined. * Once this is in r21 I would like to do a performace review * Mabye we could make more of this run in a seperate thread / cpu? This can go in in r21 anytime now. Diff comments: > > === 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() { This and the flooowing one should be inline > + 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, Should debug this to find out how often this is called, might be a bit expensive > 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) { A simple if() would be clearer > + 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. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp