Commit: https://github.com/Alexays/Waybar/commit/4e8ccf36b54cacf5281726d23ea14312a133f977 Commit: https://github.com/Alexays/Waybar/commit/8da5425189d52ebbaab063c9289c5de5adbe0ca5 Issue: https://github.com/Alexays/Waybar/issues/1852 From 4e8ccf36b54cacf5281726d23ea14312a133f977 Mon Sep 17 00:00:00 2001 From: Sasha Moak Date: Thu, 12 Jan 2023 16:17:11 -0800 Subject: [PATCH] fix(wireplumber): waybar crashes when default node changes In order to fix the issue, the default node name is cached rather than the default node id. This is due to ids being unstable. So now when the object manager is installed (ie ready), the default node name is retrieved and stored for later. Now when the mixer changed signal is emitted, the id of the changed node is used to get the node from the object manager. The nodes name is grabbed off that node and compared against the default node name, if they match the volume is updated. Some safeguarding has been added such that if the node cannot be found off the object manager, it's ignored. Additionally, the "changed" signal on the default nodes api is now utilized to update the default node name if it has changed. This way if the default node changes, the module will be updated with the correct volume and node.nick. This adds additional debug logging for helping diagnose wireplumber issues. This also adds the wireplumber man page entry to the main waybar supported section. --- include/modules/wireplumber.hpp | 10 +- man/waybar.5.scd.in | 1 + src/modules/wireplumber.cpp | 184 ++++++++++++++++++++++++++------ 3 files changed, 157 insertions(+), 38 deletions(-) diff --git a/include/modules/wireplumber.hpp b/include/modules/wireplumber.hpp index c0ee7f0be..fa988fcf3 100644 --- a/include/modules/wireplumber.hpp +++ b/include/modules/wireplumber.hpp @@ -20,15 +20,19 @@ class Wireplumber : public ALabel { void loadRequiredApiModules(); void prepare(); void activatePlugins(); - static void updateVolume(waybar::modules::Wireplumber* self); - static void updateNodeName(waybar::modules::Wireplumber* self); - static uint32_t getDefaultNodeId(waybar::modules::Wireplumber* self); + static void updateVolume(waybar::modules::Wireplumber* self, uint32_t id); + static void updateNodeName(waybar::modules::Wireplumber* self, uint32_t id); static void onPluginActivated(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self); static void onObjectManagerInstalled(waybar::modules::Wireplumber* self); + static void onMixerChanged(waybar::modules::Wireplumber* self, uint32_t id); + static void onDefaultNodesApiChanged(waybar::modules::Wireplumber* self); WpCore* wp_core_; GPtrArray* apis_; WpObjectManager* om_; + WpPlugin* mixer_api_; + WpPlugin* def_nodes_api_; + gchar* default_node_name_; uint32_t pending_plugins_; bool muted_; double volume_; diff --git a/man/waybar.5.scd.in b/man/waybar.5.scd.in index b1ed4c527..7566dd000 100644 --- a/man/waybar.5.scd.in +++ b/man/waybar.5.scd.in @@ -277,6 +277,7 @@ A module group is defined by specifying a module named "group/some-group-name". - *waybar-sway-scratchpad(5)* - *waybar-sway-window(5)* - *waybar-sway-workspaces(5)* +- *waybar-wireplumber(5)* - *waybar-wlr-taskbar(5)* - *waybar-wlr-workspaces(5)* - *waybar-temperature(5)* diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index 9a12a9b5b..9652e1e2b 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -1,15 +1,22 @@ #include "modules/wireplumber.hpp" +#include + +bool isValidNodeId(uint32_t id) { return id > 0 && id < G_MAXUINT32; } + waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Value& config) : ALabel(config, "wireplumber", id, "{volume}%"), wp_core_(nullptr), apis_(nullptr), om_(nullptr), + mixer_api_(nullptr), + def_nodes_api_(nullptr), + default_node_name_(nullptr), pending_plugins_(0), muted_(false), volume_(0.0), node_id_(0) { - wp_init(WP_INIT_ALL); + wp_init(WP_INIT_PIPEWIRE); wp_core_ = wp_core_new(NULL, NULL); apis_ = g_ptr_array_new_with_free_func(g_object_unref); om_ = wp_object_manager_new(); @@ -18,10 +25,15 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val loadRequiredApiModules(); + spdlog::debug("[{}]: connecting to pipewire...", this->name_); + if (!wp_core_connect(wp_core_)) { + spdlog::error("[{}]: Could not connect to PipeWire", this->name_); throw std::runtime_error("Could not connect to PipeWire\n"); } + spdlog::debug("[{}]: connected!", this->name_); + g_signal_connect_swapped(om_, "installed", (GCallback)onObjectManagerInstalled, this); activatePlugins(); @@ -33,33 +45,26 @@ waybar::modules::Wireplumber::~Wireplumber() { g_clear_pointer(&apis_, g_ptr_array_unref); g_clear_object(&om_); g_clear_object(&wp_core_); + g_clear_object(&mixer_api_); + g_clear_object(&def_nodes_api_); + g_free(&default_node_name_); } -uint32_t waybar::modules::Wireplumber::getDefaultNodeId(waybar::modules::Wireplumber* self) { - uint32_t id; - g_autoptr(WpPlugin) def_nodes_api = wp_plugin_find(self->wp_core_, "default-nodes-api"); +void waybar::modules::Wireplumber::updateNodeName(waybar::modules::Wireplumber* self, uint32_t id) { + spdlog::debug("[{}]: updating node name with node.id {}", self->name_, id); - if (!def_nodes_api) { - throw std::runtime_error("Default nodes API is not loaded\n"); + if (!isValidNodeId(id)) { + spdlog::warn("[{}]: '{}' is not a valid node ID. Ignoring node name update.", self->name_, id); + return; } - g_signal_emit_by_name(def_nodes_api, "get-default-node", "Audio/Sink", &id); - - if (id <= 0 || id >= G_MAXUINT32) { - auto err = fmt::format("'{}' is not a valid ID (returned by default-nodes-api)\n", id); - throw std::runtime_error(err); - } - - return id; -} - -void waybar::modules::Wireplumber::updateNodeName(waybar::modules::Wireplumber* self) { - auto proxy = static_cast( - wp_object_manager_lookup(self->om_, WP_TYPE_GLOBAL_PROXY, WP_CONSTRAINT_TYPE_G_PROPERTY, - "bound-id", "=u", self->node_id_, NULL)); + auto proxy = static_cast(wp_object_manager_lookup( + self->om_, WP_TYPE_GLOBAL_PROXY, WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", "=u", id, NULL)); if (!proxy) { - throw std::runtime_error(fmt::format("Object '{}' not found\n", self->node_id_)); + auto err = fmt::format("Object '{}' not found\n", id); + spdlog::error("[{}]: {}", self->name_, err); + throw std::runtime_error(err); } g_autoptr(WpProperties) properties = @@ -73,15 +78,24 @@ void waybar::modules::Wireplumber::updateNodeName(waybar::modules::Wireplumber* auto description = wp_properties_get(properties, "node.description"); self->node_name_ = nick ? nick : description; + spdlog::debug("[{}]: Updating node name to: {}", self->name_, self->node_name_); } -void waybar::modules::Wireplumber::updateVolume(waybar::modules::Wireplumber* self) { +void waybar::modules::Wireplumber::updateVolume(waybar::modules::Wireplumber* self, uint32_t id) { + spdlog::debug("[{}]: updating volume", self->name_); double vol; GVariant* variant = NULL; - g_autoptr(WpPlugin) mixer_api = wp_plugin_find(self->wp_core_, "mixer-api"); - g_signal_emit_by_name(mixer_api, "get-volume", self->node_id_, &variant); + + if (!isValidNodeId(id)) { + spdlog::error("[{}]: '{}' is not a valid node ID. Ignoring volume update.", self->name_, id); + return; + } + + g_signal_emit_by_name(self->mixer_api_, "get-volume", id, &variant); + if (!variant) { - auto err = fmt::format("Node {} does not support volume\n", self->node_id_); + auto err = fmt::format("Node {} does not support volume\n", id); + spdlog::error("[{}]: {}", self->name_, err); throw std::runtime_error(err); } @@ -93,22 +107,121 @@ void waybar::modules::Wireplumber::updateVolume(waybar::modules::Wireplumber* se self->dp.emit(); } +void waybar::modules::Wireplumber::onMixerChanged(waybar::modules::Wireplumber* self, uint32_t id) { + spdlog::debug("[{}]: (onMixerChanged) - id: {}", self->name_, id); + + g_autoptr(WpNode) node = static_cast(wp_object_manager_lookup( + self->om_, WP_TYPE_NODE, WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", "=u", id, NULL)); + + if (!node) { + spdlog::warn("[{}]: (onMixerChanged) - Object with id {} not found", self->name_, id); + return; + } + + const gchar* name = wp_pipewire_object_get_property(WP_PIPEWIRE_OBJECT(node), "node.name"); + + if (g_strcmp0(self->default_node_name_, name) != 0) { + spdlog::debug( + "[{}]: (onMixerChanged) - ignoring mixer update for node: id: {}, name: {} as it is not " + "the default node: {}", + self->name_, id, name, self->default_node_name_); + return; + } + + spdlog::debug("[{}]: (onMixerChanged) - Need to update volume for node with id {} and name {}", + self->name_, id, name); + updateVolume(self, id); +} + +void waybar::modules::Wireplumber::onDefaultNodesApiChanged(waybar::modules::Wireplumber* self) { + spdlog::debug("[{}]: (onDefaultNodesApiChanged)", self->name_); + + uint32_t default_node_id; + g_signal_emit_by_name(self->def_nodes_api_, "get-default-node", "Audio/Sink", &default_node_id); + + if (!isValidNodeId(default_node_id)) { + spdlog::warn("[{}]: '{}' is not a valid node ID. Ignoring node change.", self->name_, + default_node_id); + return; + } + + g_autoptr(WpNode) node = static_cast( + wp_object_manager_lookup(self->om_, WP_TYPE_NODE, WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", + "=u", default_node_id, NULL)); + + if (!node) { + spdlog::warn("[{}]: (onDefaultNodesApiChanged) - Object with id {} not found", self->name_, + default_node_id); + return; + } + + const gchar* default_node_name = + wp_pipewire_object_get_property(WP_PIPEWIRE_OBJECT(node), "node.name"); + + spdlog::debug( + "[{}]: (onDefaultNodesApiChanged) - got the following default node: Node(name: {}, id: {})", + self->name_, default_node_name, default_node_id); + + if (g_strcmp0(self->default_node_name_, default_node_name) == 0) { + spdlog::debug( + "[{}]: (onDefaultNodesApiChanged) - Default node has not changed. Node(name: {}, id: {}). " + "Ignoring.", + self->name_, self->default_node_name_, default_node_id); + return; + } + + spdlog::debug( + "[{}]: (onDefaultNodesApiChanged) - Default node changed to -> Node(name: {}, id: {})", + self->name_, default_node_name, default_node_id); + + self->default_node_name_ = g_strdup(default_node_name); + updateVolume(self, default_node_id); + updateNodeName(self, default_node_id); +} + void waybar::modules::Wireplumber::onObjectManagerInstalled(waybar::modules::Wireplumber* self) { - self->node_id_ = - self->config_["node-id"].isInt() ? self->config_["node-id"].asInt() : getDefaultNodeId(self); + spdlog::debug("[{}]: onObjectManagerInstalled", self->name_); + + self->def_nodes_api_ = wp_plugin_find(self->wp_core_, "default-nodes-api"); + + if (!self->def_nodes_api_) { + spdlog::error("[{}]: default nodes api is not loaded.", self->name_); + throw std::runtime_error("Default nodes API is not loaded\n"); + } + + self->mixer_api_ = wp_plugin_find(self->wp_core_, "mixer-api"); + + if (!self->mixer_api_) { + spdlog::error("[{}]: mixer api is not loaded.", self->name_); + throw std::runtime_error("Mixer api is not loaded\n"); + } + + uint32_t default_node_id; + g_signal_emit_by_name(self->def_nodes_api_, "get-default-configured-node-name", "Audio/Sink", + &self->default_node_name_); + g_signal_emit_by_name(self->def_nodes_api_, "get-default-node", "Audio/Sink", &default_node_id); + + if (self->default_node_name_) { + spdlog::debug("[{}]: (onObjectManagerInstalled) - default configured node name: {} and id: {}", + self->name_, self->default_node_name_, default_node_id); + } - g_autoptr(WpPlugin) mixer_api = wp_plugin_find(self->wp_core_, "mixer-api"); + updateVolume(self, default_node_id); + updateNodeName(self, default_node_id); - updateVolume(self); - updateNodeName(self); - g_signal_connect_swapped(mixer_api, "changed", (GCallback)updateVolume, self); + g_signal_connect_swapped(self->mixer_api_, "changed", (GCallback)onMixerChanged, self); + g_signal_connect_swapped(self->def_nodes_api_, "changed", (GCallback)onDefaultNodesApiChanged, + self); } void waybar::modules::Wireplumber::onPluginActivated(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self) { + auto plugin_name = wp_plugin_get_name(WP_PLUGIN(p)); + spdlog::debug("[{}]: onPluginActivated: {}", self->name_, plugin_name); g_autoptr(GError) error = NULL; if (!wp_object_activate_finish(p, res, &error)) { + spdlog::error("[{}]: error activating plugin: {}", self->name_, error->message); throw std::runtime_error(error->message); } @@ -118,6 +231,7 @@ void waybar::modules::Wireplumber::onPluginActivated(WpObject* p, GAsyncResult* } void waybar::modules::Wireplumber::activatePlugins() { + spdlog::debug("[{}]: activating plugins", name_); for (uint16_t i = 0; i < apis_->len; i++) { WpPlugin* plugin = static_cast(g_ptr_array_index(apis_, i)); pending_plugins_++; @@ -127,13 +241,13 @@ void waybar::modules::Wireplumber::activatePlugins() { } void waybar::modules::Wireplumber::prepare() { - wp_object_manager_add_interest(om_, WP_TYPE_NODE, NULL); - wp_object_manager_add_interest(om_, WP_TYPE_GLOBAL_PROXY, NULL); - wp_object_manager_request_object_features(om_, WP_TYPE_GLOBAL_PROXY, - WP_PIPEWIRE_OBJECT_FEATURES_MINIMAL); + spdlog::debug("[{}]: preparing object manager", name_); + wp_object_manager_add_interest(om_, WP_TYPE_NODE, WP_CONSTRAINT_TYPE_PW_PROPERTY, "media.class", + "=s", "Audio/Sink", NULL); } void waybar::modules::Wireplumber::loadRequiredApiModules() { + spdlog::debug("[{}]: loading required modules", name_); g_autoptr(GError) error = NULL; if (!wp_core_load_component(wp_core_, "libwireplumber-module-default-nodes-api", "module", NULL, From a9c9f1d705991c7f6ff9de7eac3430a219011978 Mon Sep 17 00:00:00 2001 From: Sasha Moak Date: Tue, 31 Jan 2023 17:56:58 -0800 Subject: [PATCH] fix(wireplumber): free(): invalid pointer When freeing the `default_node_name_` pointer using `free`, the `&` operator was used to try to free the reference rather than the pointer. This caused a core dump. In order to fix this, the pointer is freed instead (ie the `&` operator is no longer used). --- src/modules/wireplumber.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index fd1a0d389..4c7a2d0b1 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -47,7 +47,7 @@ waybar::modules::Wireplumber::~Wireplumber() { g_clear_object(&wp_core_); g_clear_object(&mixer_api_); g_clear_object(&def_nodes_api_); - g_free(&default_node_name_); + g_free(default_node_name_); } void waybar::modules::Wireplumber::updateNodeName(waybar::modules::Wireplumber* self, uint32_t id) {