From 2776155794ec5cfc9c0ae79e6b0dbee065c385dc Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Tue, 25 Oct 2016 08:53:55 +0200 Subject: [PATCH] fix(volume): Proper handling of module events When processing module events, a new instance for each mixer is created so that the module event loop will receive the mixer update events. Closes jaagr/lemonbuddy#116 Closes jaagr/lemonbuddy#89 --- include/adapters/alsa.hpp | 24 +++++-- include/modules/volume.hpp | 136 +++++++++++++++---------------------- 2 files changed, 74 insertions(+), 86 deletions(-) diff --git a/include/adapters/alsa.hpp b/include/adapters/alsa.hpp index c4a8d6f0..095d40fc 100644 --- a/include/adapters/alsa.hpp +++ b/include/adapters/alsa.hpp @@ -28,14 +28,14 @@ void throw_exception(string&& message, int error_code) { class alsa_ctl_interface { public: - explicit alsa_ctl_interface(int numid) { + explicit alsa_ctl_interface(int numid) : m_numid(numid) { int err = 0; snd_ctl_elem_info_alloca(&m_info); snd_ctl_elem_value_alloca(&m_value); snd_ctl_elem_id_alloca(&m_id); - snd_ctl_elem_id_set_numid(m_id, numid); + snd_ctl_elem_id_set_numid(m_id, m_numid); snd_ctl_elem_info_set_id(m_info, m_id); if ((err = snd_ctl_open(&m_ctl, ALSA_SOUNDCARD, SND_CTL_NONBLOCK | SND_CTL_READONLY)) < 0) @@ -58,8 +58,6 @@ class alsa_ctl_interface { if ((err = snd_ctl_subscribe_events(m_ctl, 1)) < 0) throw alsa_ctl_interface_error( "Could not subscribe to events: " + to_string(snd_ctl_elem_id_get_numid(m_id))); - - // log_trace("Successfully initialized control interface with ID: "+ Intstring(numid)); } ~alsa_ctl_interface() { @@ -68,6 +66,10 @@ class alsa_ctl_interface { snd_hctl_close(m_hctl); } + int get_numid() { + return m_numid; + } + bool wait(int timeout = -1) { assert(m_ctl); @@ -103,9 +105,13 @@ class alsa_ctl_interface { return snd_ctl_elem_value_get_boolean(m_value, 0); } - void process_events() {} + void process_events() { + wait(0); + } private: + int m_numid = 0; + threading_util::spin_lock m_lock; snd_hctl_t* m_hctl = nullptr; @@ -122,7 +128,7 @@ class alsa_ctl_interface { class alsa_mixer { public: - explicit alsa_mixer(string mixer_control_name) { + explicit alsa_mixer(string mixer_control_name) : m_name(mixer_control_name) { snd_mixer_selem_id_t* mixer_id; snd_mixer_selem_id_alloca(&mixer_id); @@ -154,6 +160,10 @@ class alsa_mixer { snd_mixer_close(m_hardwaremixer); } + string get_name() { + return m_name; + } + bool wait(int timeout = -1) { assert(m_hardwaremixer); @@ -239,6 +249,8 @@ class alsa_mixer { } private: + string m_name; + threading_util::spin_lock m_lock; snd_mixer_t* m_hardwaremixer = nullptr; diff --git a/include/modules/volume.hpp b/include/modules/volume.hpp index 44478125..e23ceab1 100644 --- a/include/modules/volume.hpp +++ b/include/modules/volume.hpp @@ -24,58 +24,40 @@ namespace modules { void setup() { // Load configuration values {{{ - auto master_mixer = m_conf.get(name(), "master-mixer", "Master"); - auto speaker_mixer = m_conf.get(name(), "speaker-mixer", ""); - auto headphone_mixer = m_conf.get(name(), "headphone-mixer", ""); + string master_mixer_name{"Master"}; + string speaker_mixer_name; + string headphone_mixer_name; - GET_CONFIG_VALUE(name(), m_headphoneid, "headphone-id"); + GET_CONFIG_VALUE(name(), master_mixer_name, "master-mixer"); + GET_CONFIG_VALUE(name(), speaker_mixer_name, "speaker-mixer"); + GET_CONFIG_VALUE(name(), headphone_mixer_name, "headphone-mixer"); - if (!headphone_mixer.empty() && m_headphoneid == -1) - throw module_error("Missing required property value for \"headphone-id\"..."); - else if (headphone_mixer.empty() && m_headphoneid != -1) - throw module_error("Missing required property value for \"headphone-mixer\"..."); + if (!headphone_mixer_name.empty()) + REQ_CONFIG_VALUE(name(), m_headphoneid, "headphone-id"); - if (string_util::lower(speaker_mixer) == "master") - throw module_error( - "The \"Master\" mixer is already processed internally. Specify another " - "mixer or comment out the \"speaker-mixer\" parameter..."); - if (string_util::lower(headphone_mixer) == "master") - throw module_error( - "The \"Master\" mixer is already processed internally. Specify another " - "mixer or comment out the \"headphone-mixer\" parameter..."); + if (string_util::compare(speaker_mixer_name, "master")) + throw module_error("Master mixer is already defined"); + if (string_util::compare(headphone_mixer_name, "master")) + throw module_error("Master mixer is already defined"); // }}} // Setup mixers {{{ - auto create_mixer = [this](string mixer_name) { - try { - return mixer_t{new mixer_t::element_type{mixer_name}}; - } catch (const alsa_mixer_error& e) { - m_log.err("%s: Failed to open '%s' mixer => %s", name(), mixer_name, e.what()); - return mixer_t{}; - } - }; - - m_mixers[mixer::MASTER] = create_mixer(master_mixer); - - if (!speaker_mixer.empty()) - m_mixers[mixer::SPEAKER] = create_mixer(speaker_mixer); - if (!headphone_mixer.empty()) - m_mixers[mixer::HEADPHONE] = create_mixer(headphone_mixer); - - if (m_mixers.empty()) { - m_log.err("%s: No configured mixers, stopping module...", name()); - stop(); - return; - } - - if (m_mixers[mixer::HEADPHONE] && m_headphoneid > -1) { - try { - m_controls[control::HEADPHONE] = control_t{new control_t::element_type{m_headphoneid}}; - } catch (const alsa_ctl_interface_error& e) { - m_log.err("%s: Failed to open headphone control interface => %s", name(), e.what()); - m_controls[control::HEADPHONE].reset(); - } + try { + if (!master_mixer_name.empty()) + m_mixers[mixer::MASTER].reset(new mixer_t::element_type{master_mixer_name}); + if (!speaker_mixer_name.empty()) + m_mixers[mixer::SPEAKER].reset(new mixer_t::element_type{speaker_mixer_name}); + if (!headphone_mixer_name.empty()) + m_mixers[mixer::HEADPHONE].reset(new mixer_t::element_type{headphone_mixer_name}); + if (m_mixers[mixer::HEADPHONE]) + m_controls[control::HEADPHONE].reset(new control_t::element_type{m_headphoneid}); + if (m_mixers.empty()) + throw module_error("No configured mixers"); + } catch (const alsa_mixer_error& err) { + throw module_error(err.what()); + } catch (const alsa_ctl_interface_error& err) { + throw module_error(err.what()); } // }}} @@ -86,19 +68,16 @@ namespace modules { m_formatter->add( FORMAT_MUTED, TAG_LABEL_MUTED, {TAG_RAMP_VOLUME, TAG_LABEL_MUTED, TAG_BAR_VOLUME}); - if (m_formatter->has(TAG_BAR_VOLUME)) { + if (m_formatter->has(TAG_BAR_VOLUME)) m_bar_volume = load_progressbar(m_bar, m_conf, name(), TAG_BAR_VOLUME); - } + if (m_formatter->has(TAG_LABEL_VOLUME, FORMAT_VOLUME)) + m_label_volume = load_optional_label(m_conf, name(), TAG_LABEL_VOLUME, "%percentage%"); + if (m_formatter->has(TAG_LABEL_MUTED, FORMAT_MUTED)) + m_label_muted = load_optional_label(m_conf, name(), TAG_LABEL_MUTED, "%percentage%"); if (m_formatter->has(TAG_RAMP_VOLUME)) { m_ramp_volume = load_ramp(m_conf, name(), TAG_RAMP_VOLUME); m_ramp_headphones = load_ramp(m_conf, name(), TAG_RAMP_HEADPHONES, false); } - if (m_formatter->has(TAG_LABEL_VOLUME, FORMAT_VOLUME)) { - m_label_volume = load_optional_label(m_conf, name(), TAG_LABEL_VOLUME, "%percentage%"); - } - if (m_formatter->has(TAG_LABEL_MUTED, FORMAT_MUTED)) { - m_label_muted = load_optional_label(m_conf, name(), TAG_LABEL_MUTED, "%percentage%"); - } // }}} } @@ -111,23 +90,20 @@ namespace modules { // Poll for mixer and control events {{{ try { - bool has_event = false; - - if (m_mixers[mixer::MASTER]) - has_event |= m_mixers[mixer::MASTER]->wait(25); - if (m_mixers[mixer::SPEAKER]) - has_event |= m_mixers[mixer::SPEAKER]->wait(25); - if (m_mixers[mixer::HEADPHONE]) - has_event |= m_mixers[mixer::HEADPHONE]->wait(25); - if (m_controls[control::HEADPHONE]) - has_event |= m_controls[control::HEADPHONE]->wait(25); - - return has_event; + if (m_mixers[mixer::MASTER] && m_mixers[mixer::MASTER]->wait(25)) + return true; + if (m_mixers[mixer::SPEAKER] && m_mixers[mixer::SPEAKER]->wait(25)) + return true; + if (m_mixers[mixer::HEADPHONE] && m_mixers[mixer::HEADPHONE]->wait(25)) + return true; + if (m_controls[control::HEADPHONE] && m_controls[control::HEADPHONE]->wait(25)) + return true; } catch (const alsa_exception& e) { m_log.err("%s: %s", name(), e.what()); - return false; } + return false; + // }}} } @@ -141,7 +117,7 @@ namespace modules { if (m_mixers[mixer::HEADPHONE]) m_mixers[mixer::HEADPHONE]->process_events(); if (m_controls[control::HEADPHONE]) - m_controls[control::HEADPHONE]->wait(0); + m_controls[control::HEADPHONE]->process_events(); // }}} // Get volume, mute and headphone state {{{ @@ -151,16 +127,16 @@ namespace modules { m_headphones = false; if (m_mixers[mixer::MASTER]) { - m_volume *= m_mixers[mixer::MASTER]->get_volume() / 100.0f; + m_volume = m_volume * m_mixers[mixer::MASTER]->get_volume() / 100.0f; m_muted = m_muted || m_mixers[mixer::MASTER]->is_muted(); } if (m_controls[control::HEADPHONE] && m_controls[control::HEADPHONE]->test_device_plugged()) { m_headphones = true; - m_volume *= m_mixers[mixer::HEADPHONE]->get_volume() / 100.0f; + m_volume = m_volume * m_mixers[mixer::HEADPHONE]->get_volume() / 100.0f; m_muted = m_muted || m_mixers[mixer::HEADPHONE]->is_muted(); } else if (m_mixers[mixer::SPEAKER]) { - m_volume *= m_mixers[mixer::SPEAKER]->get_volume() / 100.0f; + m_volume = m_volume * m_mixers[mixer::SPEAKER]->get_volume() / 100.0f; m_muted = m_muted || m_mixers[mixer::SPEAKER]->is_muted(); } @@ -171,6 +147,7 @@ namespace modules { m_label_volume->reset_tokens(); m_label_volume->replace_token("%percentage%", to_string(m_volume) + "%"); } + if (m_label_muted) { m_label_muted->reset_tokens(); m_label_muted->replace_token("%percentage%", to_string(m_volume) + "%"); @@ -220,12 +197,14 @@ namespace modules { if (!m_mixers[mixer::MASTER]) return false; - vector mixers{m_mixers[mixer::MASTER]}; + vector mixers; + if (m_mixers[mixer::MASTER]) + mixers.emplace_back(new mixer_t::element_type(m_mixers[mixer::MASTER]->get_name())); if (m_mixers[mixer::HEADPHONE] && m_headphones) - mixers.emplace_back(m_mixers[mixer::HEADPHONE]); - else if (m_mixers[mixer::SPEAKER]) - mixers.emplace_back(m_mixers[mixer::SPEAKER]); + mixers.emplace_back(new mixer_t::element_type(m_mixers[mixer::HEADPHONE]->get_name())); + if (m_mixers[mixer::SPEAKER] && !m_headphones) + mixers.emplace_back(new mixer_t::element_type(m_mixers[mixer::SPEAKER]->get_name())); try { if (cmd.compare(0, strlen(EVENT_TOGGLE_MUTE), EVENT_TOGGLE_MUTE) == 0) { @@ -247,10 +226,6 @@ namespace modules { m_log.err("%s: Failed to handle command (%s)", name(), err.what()); } - // Update the mute flag since we won't poll the new state when - // sending the broadcast related to this event - m_muted = !m_muted; - return true; } @@ -282,11 +257,12 @@ namespace modules { map m_mixers; map m_controls; - int m_headphoneid = -1; - int m_volume = 0; + int m_headphoneid = 0; stateflag m_muted{false}; stateflag m_headphones{false}; + + std::atomic m_volume{0}; }; }