From d8f69866ccb66f74ed326f481be4ac9a82108c9b Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Wed, 14 Dec 2016 15:04:10 +0100 Subject: [PATCH] refactor(alsa): Memory allocation --- include/adapters/alsa/control.hpp | 12 +--- include/adapters/alsa/mixer.hpp | 11 ++-- src/adapters/alsa/control.cpp | 75 +++++++++-------------- src/adapters/alsa/mixer.cpp | 98 +++++++++++++++---------------- src/modules/volume.cpp | 12 ++-- 5 files changed, 90 insertions(+), 118 deletions(-) diff --git a/include/adapters/alsa/control.hpp b/include/adapters/alsa/control.hpp index b4b23cef..4e964af0 100644 --- a/include/adapters/alsa/control.hpp +++ b/include/adapters/alsa/control.hpp @@ -5,15 +5,9 @@ #include "common.hpp" // fwd -struct _snd_ctl_elem_id; -struct _snd_ctl_elem_info; -struct _snd_ctl_elem_value; struct _snd_ctl; struct _snd_hctl_elem; struct _snd_hctl; -typedef struct _snd_ctl_elem_id snd_ctl_elem_id_t; -typedef struct _snd_ctl_elem_info snd_ctl_elem_info_t; -typedef struct _snd_ctl_elem_value snd_ctl_elem_value_t; typedef struct _snd_ctl snd_ctl_t; typedef struct _snd_hctl_elem snd_hctl_elem_t; typedef struct _snd_hctl snd_hctl_t; @@ -36,13 +30,9 @@ namespace alsa { int m_numid{0}; + snd_ctl_t* m_ctl{nullptr}; snd_hctl_t* m_hctl{nullptr}; snd_hctl_elem_t* m_elem{nullptr}; - - snd_ctl_t* m_ctl{nullptr}; - snd_ctl_elem_info_t* m_info{nullptr}; - snd_ctl_elem_value_t* m_value{nullptr}; - snd_ctl_elem_id_t* m_id{nullptr}; }; } diff --git a/include/adapters/alsa/mixer.hpp b/include/adapters/alsa/mixer.hpp index 8185be22..748036b2 100644 --- a/include/adapters/alsa/mixer.hpp +++ b/include/adapters/alsa/mixer.hpp @@ -17,10 +17,10 @@ POLYBAR_NS namespace alsa { class mixer { public: - explicit mixer(string mixer_control_name); + explicit mixer(string&& mixer_selem_name); ~mixer(); - string get_name(); + const string& get_name(); bool wait(int timeout = -1); int process_events(); @@ -36,11 +36,10 @@ namespace alsa { private: std::mutex m_lock; - string m_name; + snd_mixer_t* m_mixer{nullptr}; + snd_mixer_elem_t* m_elem{nullptr}; - snd_mixer_selem_id_t* m_mixerid{nullptr}; - snd_mixer_t* m_hardwaremixer{nullptr}; - snd_mixer_elem_t* m_mixerelement{nullptr}; + string m_name; }; } diff --git a/src/adapters/alsa/control.cpp b/src/adapters/alsa/control.cpp index 339a7c86..fe78a412 100644 --- a/src/adapters/alsa/control.cpp +++ b/src/adapters/alsa/control.cpp @@ -8,28 +8,7 @@ namespace alsa { * Construct control object */ control::control(int numid) : m_numid(numid) { - snd_ctl_elem_info_malloc(&m_info); - - if (m_info == nullptr) { - throw control_error("Failed to allocate alsa_ctl info"); - } - - snd_ctl_elem_value_malloc(&m_value); - - if (m_value == nullptr) { - throw control_error("Failed to allocate alsa_ctl value"); - } - - snd_ctl_elem_id_malloc(&m_id); - - if (m_id == nullptr) { - throw control_error("Failed to allocate alsa_ctl id"); - } - - snd_ctl_elem_id_set_numid(m_id, m_numid); - snd_ctl_elem_info_set_id(m_info, m_id); - - int err = 0; + int err{0}; if ((err = snd_ctl_open(&m_ctl, ALSA_SOUNDCARD, SND_CTL_NONBLOCK | SND_CTL_READONLY)) == -1) { throw_exception("Could not open control '" + string{ALSA_SOUNDCARD} + "'", err); @@ -37,22 +16,31 @@ namespace alsa { snd_config_update_free_global(); - if ((err = snd_ctl_elem_info(m_ctl, m_info)) == -1) { - throw_exception("Could not get control datal", err); - } - - snd_ctl_elem_info_get_id(m_info, m_id); - - if ((err = snd_hctl_open(&m_hctl, ALSA_SOUNDCARD, 0)) == -1) { + if ((err = snd_hctl_open_ctl(&m_hctl, m_ctl)) == -1) { + snd_ctl_close(m_ctl); throw_exception("Failed to open hctl", err); } snd_config_update_free_global(); - if (m_hctl == nullptr || (err = snd_hctl_load(m_hctl)) == -1) { + if ((err = snd_hctl_load(m_hctl)) == -1) { throw_exception("Failed to load hctl", err); } + snd_ctl_elem_id_t* m_id{nullptr}; + snd_ctl_elem_id_alloca(&m_id); + snd_ctl_elem_id_set_numid(m_id, m_numid); + + snd_ctl_elem_info_t* m_info{nullptr}; + snd_ctl_elem_info_alloca(&m_info); + snd_ctl_elem_info_set_id(m_info, m_id); + + if ((err = snd_ctl_elem_info(m_ctl, m_info)) == -1) { + throw_exception("Could not get control datal", err); + } + + snd_ctl_elem_info_get_id(m_info, m_id); + if ((m_elem = snd_hctl_find_elem(m_hctl, m_id)) == nullptr) { throw control_error("Could not find control with id " + to_string(snd_ctl_elem_id_get_numid(m_id))); } @@ -67,21 +55,12 @@ namespace alsa { */ control::~control() { std::lock_guard guard(m_lock); - if (m_info != nullptr) { - snd_ctl_elem_info_free(m_info); - } - if (m_value != nullptr) { - snd_ctl_elem_value_free(m_value); - } - if (m_id != nullptr) { - snd_ctl_elem_id_free(m_id); - } - if (m_ctl != nullptr) { - snd_ctl_close(m_ctl); - } + if (m_hctl != nullptr) { snd_hctl_close(m_hctl); } + + snd_config_update_free_global(); } /** @@ -103,13 +82,13 @@ namespace alsa { std::lock_guard guard(m_lock, std::adopt_lock); - int err = 0; + int err{0}; if ((err = snd_ctl_wait(m_ctl, timeout)) == -1) { throw_exception("Failed to wait for events", err); } - snd_ctl_event_t* event; + snd_ctl_event_t* event{nullptr}; snd_ctl_event_alloca(&event); if ((err = snd_ctl_read(m_ctl, event)) == -1) { @@ -128,7 +107,6 @@ namespace alsa { */ bool control::test_device_plugged() { assert(m_elem); - assert(m_value); if (!m_lock.try_lock()) { return false; @@ -136,10 +114,15 @@ namespace alsa { std::lock_guard guard(m_lock, std::adopt_lock); - int err = 0; + snd_ctl_elem_value_t* m_value{nullptr}; + snd_ctl_elem_value_alloca(&m_value); + + int err{0}; + if ((err = snd_hctl_elem_read(m_elem, m_value)) == -1) { throw_exception("Could not read control value", err); } + return snd_ctl_elem_value_get_boolean(m_value, 0); } diff --git a/src/adapters/alsa/mixer.cpp b/src/adapters/alsa/mixer.cpp index 2632dcb2..2381ae1e 100644 --- a/src/adapters/alsa/mixer.cpp +++ b/src/adapters/alsa/mixer.cpp @@ -12,38 +12,31 @@ namespace alsa { /** * Construct mixer object */ - mixer::mixer(string mixer_control_name) : m_name(move(mixer_control_name)) { - if (m_name.empty()) { - throw mixer_error("Invalid control name"); - } - - snd_mixer_selem_id_malloc(&m_mixerid); - - if (m_mixerid == nullptr) { - throw mixer_error("Failed to allocate mixer id"); - } - + mixer::mixer(string&& mixer_selem_name) : m_name(forward(mixer_selem_name)) { int err = 0; - if ((err = snd_mixer_open(&m_hardwaremixer, 1)) == -1) { + + if ((err = snd_mixer_open(&m_mixer, 1)) == -1) { throw_exception("Failed to open hardware mixer", err); } snd_config_update_free_global(); - if ((err = snd_mixer_attach(m_hardwaremixer, ALSA_SOUNDCARD)) == -1) { + if ((err = snd_mixer_attach(m_mixer, ALSA_SOUNDCARD)) == -1) { throw_exception("Failed to attach hardware mixer control", err); } - if ((err = snd_mixer_selem_register(m_hardwaremixer, nullptr, nullptr)) == -1) { + if ((err = snd_mixer_selem_register(m_mixer, nullptr, nullptr)) == -1) { throw_exception("Failed to register simple mixer element", err); } - if ((err = snd_mixer_load(m_hardwaremixer)) == -1) { + if ((err = snd_mixer_load(m_mixer)) == -1) { throw_exception("Failed to load mixer", err); } - snd_mixer_selem_id_set_index(m_mixerid, 0); - snd_mixer_selem_id_set_name(m_mixerid, m_name.c_str()); + snd_mixer_selem_id_t* sid{nullptr}; + snd_mixer_selem_id_alloca(&sid); + snd_mixer_selem_id_set_index(sid, 0); + snd_mixer_selem_id_set_name(sid, m_name.c_str()); - if ((m_mixerelement = snd_mixer_find_selem(m_hardwaremixer, m_mixerid)) == nullptr) { + if ((m_elem = snd_mixer_find_selem(m_mixer, sid)) == nullptr) { throw mixer_error("Cannot find simple element"); } } @@ -53,22 +46,16 @@ namespace alsa { */ mixer::~mixer() { std::lock_guard guard(m_lock); - if (m_mixerid != nullptr) { - snd_mixer_selem_id_free(m_mixerid); - } - if (m_mixerelement != nullptr) { - snd_mixer_elem_remove(m_mixerelement); - } - if (m_hardwaremixer != nullptr) { - snd_mixer_detach(m_hardwaremixer, ALSA_SOUNDCARD); - snd_mixer_close(m_hardwaremixer); + + if (m_mixer != nullptr) { + snd_mixer_close(m_mixer); } } /** * Get mixer name */ - string mixer::get_name() { + const string& mixer::get_name() { return m_name; } @@ -76,7 +63,7 @@ namespace alsa { * Wait for events */ bool mixer::wait(int timeout) { - assert(m_hardwaremixer); + assert(m_mixer); if (!m_lock.try_lock()) { return false; @@ -86,7 +73,7 @@ namespace alsa { int err = 0; - if ((err = snd_mixer_wait(m_hardwaremixer, timeout)) == -1) { + if ((err = snd_mixer_wait(m_mixer, timeout)) == -1) { throw_exception("Failed to wait for events", err); } @@ -105,8 +92,8 @@ namespace alsa { std::lock_guard guard(m_lock, std::adopt_lock); - int num_events = snd_mixer_handle_events(m_hardwaremixer); - if (num_events == -1) { + int num_events{0}; + if ((num_events = snd_mixer_handle_events(m_mixer)) == -1) { throw_exception("Failed to process pending events", num_events); } @@ -117,6 +104,8 @@ namespace alsa { * Get volume in percentage */ int mixer::get_volume() { + assert(m_elem != nullptr); + if (!m_lock.try_lock()) { return 0; } @@ -125,11 +114,11 @@ namespace alsa { long chan_n = 0, vol_total = 0, vol, vol_min, vol_max; - snd_mixer_selem_get_playback_volume_range(m_mixerelement, &vol_min, &vol_max); + snd_mixer_selem_get_playback_volume_range(m_elem, &vol_min, &vol_max); for (int i = 0; i <= SND_MIXER_SCHN_LAST; i++) { - if (snd_mixer_selem_has_playback_channel(m_mixerelement, static_cast(i))) { - snd_mixer_selem_get_playback_volume(m_mixerelement, static_cast(i), &vol); + if (snd_mixer_selem_has_playback_channel(m_elem, static_cast(i))) { + snd_mixer_selem_get_playback_volume(m_elem, static_cast(i), &vol); vol_total += vol; chan_n++; } @@ -142,6 +131,8 @@ namespace alsa { * Get normalized volume in percentage */ int mixer::get_normalized_volume() { + assert(m_elem != nullptr); + if (!m_lock.try_lock()) { return 0; } @@ -151,11 +142,11 @@ namespace alsa { long chan_n = 0, vol_total = 0, vol, vol_min, vol_max; double normalized, min_norm; - snd_mixer_selem_get_playback_dB_range(m_mixerelement, &vol_min, &vol_max); + snd_mixer_selem_get_playback_dB_range(m_elem, &vol_min, &vol_max); for (int i = 0; i <= SND_MIXER_SCHN_LAST; i++) { - if (snd_mixer_selem_has_playback_channel(m_mixerelement, static_cast(i))) { - snd_mixer_selem_get_playback_dB(m_mixerelement, static_cast(i), &vol); + if (snd_mixer_selem_has_playback_channel(m_elem, static_cast(i))) { + snd_mixer_selem_get_playback_dB(m_elem, static_cast(i), &vol); vol_total += vol; chan_n++; } @@ -178,6 +169,8 @@ namespace alsa { * Set volume to given percentage */ void mixer::set_volume(float percentage) { + assert(m_elem != nullptr); + if (is_muted()) { return; } @@ -189,15 +182,16 @@ namespace alsa { std::lock_guard guard(m_lock, std::adopt_lock); long vol_min, vol_max; - snd_mixer_selem_get_playback_volume_range(m_mixerelement, &vol_min, &vol_max); - snd_mixer_selem_set_playback_volume_all( - m_mixerelement, math_util::percentage_to_value(percentage, vol_min, vol_max)); + snd_mixer_selem_get_playback_volume_range(m_elem, &vol_min, &vol_max); + snd_mixer_selem_set_playback_volume_all(m_elem, math_util::percentage_to_value(percentage, vol_min, vol_max)); } /** * Set normalized volume to given percentage */ void mixer::set_normalized_volume(float percentage) { + assert(m_elem != nullptr); + if (is_muted()) { return; } @@ -212,10 +206,10 @@ namespace alsa { double min_norm; percentage = percentage / 100.0f; - snd_mixer_selem_get_playback_dB_range(m_mixerelement, &vol_min, &vol_max); + snd_mixer_selem_get_playback_dB_range(m_elem, &vol_min, &vol_max); if (vol_max - vol_min <= MAX_LINEAR_DB_SCALE * 100) { - snd_mixer_selem_set_playback_dB_all(m_mixerelement, lrint(percentage * (vol_max - vol_min)) + vol_min, 0); + snd_mixer_selem_set_playback_dB_all(m_elem, lrint(percentage * (vol_max - vol_min)) + vol_min, 0); return; } @@ -224,26 +218,30 @@ namespace alsa { percentage = percentage * (1 - min_norm) + min_norm; } - snd_mixer_selem_set_playback_dB_all(m_mixerelement, lrint(6000.0 * log10(percentage)) + vol_max, 0); + snd_mixer_selem_set_playback_dB_all(m_elem, lrint(6000.0 * log10(percentage)) + vol_max, 0); } /** * Set mute state */ void mixer::set_mute(bool mode) { + assert(m_elem != nullptr); + if (!m_lock.try_lock()) { return; } std::lock_guard guard(m_lock, std::adopt_lock); - snd_mixer_selem_set_playback_switch_all(m_mixerelement, mode); + snd_mixer_selem_set_playback_switch_all(m_elem, mode); } /** * Toggle mute state */ void mixer::toggle_mute() { + assert(m_elem != nullptr); + if (!m_lock.try_lock()) { return; } @@ -252,14 +250,16 @@ namespace alsa { int state; - snd_mixer_selem_get_playback_switch(m_mixerelement, SND_MIXER_SCHN_MONO, &state); - snd_mixer_selem_set_playback_switch_all(m_mixerelement, !state); + snd_mixer_selem_get_playback_switch(m_elem, SND_MIXER_SCHN_MONO, &state); + snd_mixer_selem_set_playback_switch_all(m_elem, !state); } /** * Get current mute state */ bool mixer::is_muted() { + assert(m_elem != nullptr); + if (!m_lock.try_lock()) { return false; } @@ -269,9 +269,9 @@ namespace alsa { int state = 0; for (int i = 0; i <= SND_MIXER_SCHN_LAST; i++) { - if (snd_mixer_selem_has_playback_channel(m_mixerelement, static_cast(i))) { + if (snd_mixer_selem_has_playback_channel(m_elem, static_cast(i))) { int state_ = 0; - snd_mixer_selem_get_playback_switch(m_mixerelement, static_cast(i), &state_); + snd_mixer_selem_get_playback_switch(m_elem, static_cast(i), &state_); state = state || state_; } } diff --git a/src/modules/volume.cpp b/src/modules/volume.cpp index f28590bc..02efb1bc 100644 --- a/src/modules/volume.cpp +++ b/src/modules/volume.cpp @@ -43,13 +43,13 @@ namespace modules { // Setup mixers try { if (!master_mixer_name.empty()) { - m_mixer[mixer::MASTER].reset(new mixer_t::element_type{master_mixer_name}); + m_mixer[mixer::MASTER].reset(new mixer_t::element_type{move(master_mixer_name)}); } if (!speaker_mixer_name.empty()) { - m_mixer[mixer::SPEAKER].reset(new mixer_t::element_type{speaker_mixer_name}); + m_mixer[mixer::SPEAKER].reset(new mixer_t::element_type{move(speaker_mixer_name)}); } if (!headphone_mixer_name.empty()) { - m_mixer[mixer::HEADPHONE].reset(new mixer_t::element_type{headphone_mixer_name}); + m_mixer[mixer::HEADPHONE].reset(new mixer_t::element_type{move(headphone_mixer_name)}); } if (m_mixer[mixer::HEADPHONE]) { m_ctrl[control::HEADPHONE].reset(new control_t::element_type{m_headphoneid}); @@ -224,13 +224,13 @@ namespace modules { bool headphones{m_headphones}; if (m_mixer[mixer::MASTER] && !m_mixer[mixer::MASTER]->get_name().empty()) { - mixers.emplace_back(new mixer_t::element_type(m_mixer[mixer::MASTER]->get_name())); + mixers.emplace_back(new mixer_t::element_type(string{m_mixer[mixer::MASTER]->get_name()})); } if (m_mixer[mixer::HEADPHONE] && !m_mixer[mixer::HEADPHONE]->get_name().empty() && headphones) { - mixers.emplace_back(new mixer_t::element_type(m_mixer[mixer::HEADPHONE]->get_name())); + mixers.emplace_back(new mixer_t::element_type(string{m_mixer[mixer::HEADPHONE]->get_name()})); } if (m_mixer[mixer::SPEAKER] && !m_mixer[mixer::SPEAKER]->get_name().empty() && !headphones) { - mixers.emplace_back(new mixer_t::element_type(m_mixer[mixer::SPEAKER]->get_name())); + mixers.emplace_back(new mixer_t::element_type(string{m_mixer[mixer::SPEAKER]->get_name()})); } if (cmd.compare(0, strlen(EVENT_TOGGLE_MUTE), EVENT_TOGGLE_MUTE) == 0) {