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
