From 151e3c1ce1b0583e89f0791d0edbf1f10595e69f Mon Sep 17 00:00:00 2001 From: badaix Date: Sun, 31 Mar 2024 14:53:55 +0200 Subject: [PATCH] Fix script volume zombie processes --- client/controller.cpp | 18 ++++--- client/player/alsa_player.cpp | 39 +++++++-------- client/player/alsa_player.hpp | 6 +-- client/player/player.cpp | 86 +++++++++++++++++++--------------- client/player/player.hpp | 43 +++++++++++------ client/player/pulse_player.cpp | 19 ++++---- client/player/pulse_player.hpp | 6 +-- 7 files changed, 118 insertions(+), 99 deletions(-) diff --git a/client/controller.cpp b/client/controller.cpp index ca913a92..09dbdec2 100644 --- a/client/controller.cpp +++ b/client/controller.cpp @@ -1,6 +1,6 @@ /*** This file is part of snapcast - Copyright (C) 2014-2023 Johannes Pohl + Copyright (C) 2014-2024 Johannes Pohl This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -161,7 +161,7 @@ void Controller::getNextMessage() << ", volume: " << serverSettings_->getVolume() << ", muted: " << serverSettings_->isMuted() << "\n"; if (stream_ && player_) { - player_->setVolume(serverSettings_->getVolume() / 100., serverSettings_->isMuted()); + player_->setVolume({serverSettings_->getVolume() / 100., serverSettings_->isMuted()}); stream_->setBufferLen(std::max(0, serverSettings_->getBufferMs() - serverSettings_->getLatency() - settings_.player.latency)); } } @@ -228,17 +228,15 @@ void Controller::getNextMessage() throw SnapException("No audio player support" + (settings_.player.player_name.empty() ? "" : " for: " + settings_.player.player_name)); player_->setVolumeCallback( - [this](double volume, bool muted) + [this](const Player::Volume& volume) { - static double last_volume(-1); - static bool last_muted(true); - if ((volume != last_volume) || (last_muted != muted)) + static Player::Volume last_volume{-1, true}; + if (volume != last_volume) { last_volume = volume; - last_muted = muted; auto info = std::make_shared(); - info->setVolume(static_cast(volume * 100.)); - info->setMuted(muted); + info->setVolume(static_cast(volume.volume * 100.)); + info->setMuted(volume.mute); clientConnection_->send(info, [this](const boost::system::error_code& ec) { @@ -256,7 +254,7 @@ void Controller::getNextMessage() // The player class will send the device's volume to the server instead // if (settings_.player.mixer.mode != ClientSettings::Mixer::Mode::hardware) // { - player_->setVolume(serverSettings_->getVolume() / 100., serverSettings_->isMuted()); + player_->setVolume({serverSettings_->getVolume() / 100., serverSettings_->isMuted()}); // } } else diff --git a/client/player/alsa_player.cpp b/client/player/alsa_player.cpp index ca08ab3a..8344606f 100644 --- a/client/player/alsa_player.cpp +++ b/client/player/alsa_player.cpp @@ -1,6 +1,6 @@ /*** This file is part of snapcast - Copyright (C) 2014-2022 Johannes Pohl + Copyright (C) 2014-2024 Johannes Pohl This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -89,7 +89,7 @@ AlsaPlayer::AlsaPlayer(boost::asio::io_context& io_context, const ClientSettings } -void AlsaPlayer::setHardwareVolume(double volume, bool muted) +void AlsaPlayer::setHardwareVolume(const Volume& volume) { std::lock_guard lock(mutex_); if (elem_ == nullptr) @@ -98,7 +98,7 @@ void AlsaPlayer::setHardwareVolume(double volume, bool muted) last_change_ = std::chrono::steady_clock::now(); try { - int val = muted ? 0 : 1; + int val = volume.mute ? 0 : 1; int err = snd_mixer_selem_set_playback_switch_all(elem_, val); if (err < 0) LOG(ERROR, LOG_TAG) << "Failed to mute, error: " << snd_strerror(err) << "\n"; @@ -107,10 +107,10 @@ void AlsaPlayer::setHardwareVolume(double volume, bool muted) if ((err = snd_mixer_selem_get_playback_dB_range(elem_, &minv, &maxv)) == 0) { double min_norm = exp10((minv - maxv) / 6000.0); - volume = volume * (1 - min_norm) + min_norm; - double mixer_volume = 6000.0 * log10(volume) + maxv; + double vol = volume.volume * (1 - min_norm) + min_norm; + double mixer_volume = 6000.0 * log10(vol) + maxv; - LOG(DEBUG, LOG_TAG) << "Mixer playback dB range [" << minv << ", " << maxv << "], volume: " << volume << ", mixer volume: " << mixer_volume << "\n"; + LOG(DEBUG, LOG_TAG) << "Mixer playback dB range [" << minv << ", " << maxv << "], volume: " << vol << ", mixer volume: " << mixer_volume << "\n"; if ((err = snd_mixer_selem_set_playback_dB_all(elem_, mixer_volume, 0)) < 0) throw SnapException(std::string("Failed to set playback volume, error: ") + snd_strerror(err)); } @@ -119,9 +119,9 @@ void AlsaPlayer::setHardwareVolume(double volume, bool muted) if ((err = snd_mixer_selem_get_playback_volume_range(elem_, &minv, &maxv)) < 0) throw SnapException(std::string("Failed to get playback volume range, error: ") + snd_strerror(err)); - auto mixer_volume = volume * (maxv - minv) + minv; - LOG(DEBUG, LOG_TAG) << "Mixer playback volume range [" << minv << ", " << maxv << "], volume: " << volume << ", mixer volume: " << mixer_volume - << "\n"; + auto mixer_volume = volume.volume * (maxv - minv) + minv; + LOG(DEBUG, LOG_TAG) << "Mixer playback volume range [" << minv << ", " << maxv << "], volume: " << volume.volume + << ", mixer volume: " << mixer_volume << "\n"; if ((err = snd_mixer_selem_set_playback_volume_all(elem_, mixer_volume)) < 0) throw SnapException(std::string("Failed to set playback volume, error: ") + snd_strerror(err)); } @@ -134,7 +134,7 @@ void AlsaPlayer::setHardwareVolume(double volume, bool muted) } -bool AlsaPlayer::getHardwareVolume(double& volume, bool& muted) +bool AlsaPlayer::getHardwareVolume(Volume& volume) { try { @@ -152,11 +152,11 @@ bool AlsaPlayer::getHardwareVolume(double& volume, bool& muted) if ((err = snd_mixer_selem_get_playback_dB(elem_, SND_MIXER_SCHN_MONO, &vol)) < 0) throw SnapException(std::string("Failed to get playback volume, error: ") + snd_strerror(err)); - volume = pow(10, (vol - maxv) / 6000.0); + volume.volume = pow(10, (vol - maxv) / 6000.0); if (minv != SND_CTL_TLV_DB_GAIN_MUTE) { double min_norm = pow(10, (minv - maxv) / 6000.0); - volume = (volume - min_norm) / (1 - min_norm); + volume.volume = (volume.volume - min_norm) / (1 - min_norm); } } else @@ -168,13 +168,14 @@ bool AlsaPlayer::getHardwareVolume(double& volume, bool& muted) vol -= minv; maxv = maxv - minv; - volume = static_cast(vol) / static_cast(maxv); + volume.volume = static_cast(vol) / static_cast(maxv); } int val; if ((err = snd_mixer_selem_get_playback_switch(elem_, SND_MIXER_SCHN_MONO, &val)) < 0) throw SnapException(std::string("Failed to get mute state, error: ") + snd_strerror(err)); - muted = (val == 0); - LOG(DEBUG, LOG_TAG) << "Get volume, mixer volume range [" << minv << ", " << maxv << "], volume: " << volume << ", muted: " << muted << "\n"; + volume.mute = (val == 0); + LOG(DEBUG, LOG_TAG) << "Get volume, mixer volume range [" << minv << ", " << maxv << "], volume: " << volume.volume << ", muted: " << volume.mute + << "\n"; snd_mixer_handle_events(mixer_); return true; } @@ -228,10 +229,10 @@ void AlsaPlayer::waitForEvent() { if (!ec) { - if (getHardwareVolume(volume_, muted_)) + if (getHardwareVolume(volume_)) { - LOG(DEBUG, LOG_TAG) << "Volume changed: " << volume_ << ", muted: " << muted_ << "\n"; - notifyVolumeChange(volume_, muted_); + LOG(DEBUG, LOG_TAG) << "Volume changed: " << volume_.volume << ", muted: " << volume_.mute << "\n"; + notifyVolumeChange(volume_); } } }); @@ -580,7 +581,7 @@ void AlsaPlayer::worker() initAlsa(); // set the hardware volume. It might have changed when we were not initialized if (settings_.mixer.mode == ClientSettings::Mixer::Mode::hardware) - setHardwareVolume(volume_, muted_); + setHardwareVolume(volume_); } catch (const std::exception& e) { diff --git a/client/player/alsa_player.hpp b/client/player/alsa_player.hpp index f043457e..77018f75 100644 --- a/client/player/alsa_player.hpp +++ b/client/player/alsa_player.hpp @@ -1,6 +1,6 @@ /*** This file is part of snapcast - Copyright (C) 2014-2022 Johannes Pohl + Copyright (C) 2014-2024 Johannes Pohl This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -68,8 +68,8 @@ private: void initMixer(); void uninitMixer(); - bool getHardwareVolume(double& volume, bool& muted) override; - void setHardwareVolume(double volume, bool muted) override; + bool getHardwareVolume(Volume& volume) override; + void setHardwareVolume(const Volume& volume) override; void waitForEvent(); diff --git a/client/player/player.cpp b/client/player/player.cpp index f2b8eb73..4893e3b0 100644 --- a/client/player/player.cpp +++ b/client/player/player.cpp @@ -1,6 +1,6 @@ /*** This file is part of snapcast - Copyright (C) 2014-2022 Johannes Pohl + Copyright (C) 2014-2024 Johannes Pohl This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -26,25 +26,16 @@ #include "common/utils/string_utils.hpp" // 3rd party headers -#ifdef WINDOWS -#include -#else #pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wpragmas" -#pragma GCC diagnostic ignored "-Wunused-result" #pragma GCC diagnostic ignored "-Wunused-parameter" -#pragma GCC diagnostic ignored "-Wmissing-braces" -#pragma GCC diagnostic ignored "-Wnarrowing" -#pragma GCC diagnostic ignored "-Wc++11-narrowing" -#include -#include -#include +#include #pragma GCC diagnostic pop -#endif // standard headers #include #include +#include +#include using namespace std; @@ -55,7 +46,7 @@ namespace player static constexpr auto LOG_TAG = "Player"; Player::Player(boost::asio::io_context& io_context, const ClientSettings::Player& settings, std::shared_ptr stream) - : io_context_(io_context), active_(false), stream_(stream), settings_(settings), volume_(1.0), muted_(false), volCorrection_(1.0) + : io_context_(io_context), active_(false), stream_(stream), settings_(settings), volCorrection_(1.0) { string sharing_mode; switch (settings_.sharing_mode) @@ -146,18 +137,16 @@ void Player::worker() } -void Player::setHardwareVolume(double volume, bool muted) +void Player::setHardwareVolume(const Volume& volume) { std::ignore = volume; - std::ignore = muted; throw SnapException("Failed to set hardware mixer volume: not supported"); } -bool Player::getHardwareVolume(double& volume, bool& muted) +bool Player::getHardwareVolume(Volume& volume) { std::ignore = volume; - std::ignore = muted; throw SnapException("Failed to get hardware mixer volume: not supported"); } @@ -170,7 +159,7 @@ void Player::adjustVolume(char* buffer, size_t frames) // for any other mixer, we might still have to apply the volCorrection_ if (settings_.mixer.mode == ClientSettings::Mixer::Mode::software) { - volume = muted_ ? 0. : volume_; + volume = volume_.mute ? 0. : volume_.volume; volume *= volCorrection_; } @@ -192,8 +181,8 @@ void Player::adjustVolume(char* buffer, size_t frames) // https://lists.linuxaudio.org/pipermail/linux-audio-dev/2009-May/thread.html#22198 void Player::setVolume_poly(double volume, double exp) { - volume_ = std::pow(volume, exp); - LOG(DEBUG, LOG_TAG) << "setVolume poly with exp " << exp << ": " << volume << " => " << volume_ << "\n"; + volume_.volume = std::pow(volume, exp); + LOG(DEBUG, LOG_TAG) << "setVolume poly with exp " << exp << ": " << volume << " => " << volume_.volume << "\n"; } @@ -202,19 +191,18 @@ void Player::setVolume_exp(double volume, double base) { // double base = M_E; // double base = 10.; - volume_ = (pow(base, volume) - 1) / (base - 1); - LOG(DEBUG, LOG_TAG) << "setVolume exp with base " << base << ": " << volume << " => " << volume_ << "\n"; + volume_.volume = (pow(base, volume) - 1) / (base - 1); + LOG(DEBUG, LOG_TAG) << "setVolume exp with base " << base << ": " << volume << " => " << volume_.volume << "\n"; } -void Player::setVolume(double volume, bool mute) +void Player::setVolume(const Volume& volume) { std::lock_guard lock(mutex_); volume_ = volume; - muted_ = mute; if (settings_.mixer.mode == ClientSettings::Mixer::Mode::hardware) { - setHardwareVolume(volume, mute); + setHardwareVolume(volume); } else if (settings_.mixer.mode == ClientSettings::Mixer::Mode::software) { @@ -235,26 +223,46 @@ void Player::setVolume(double volume, bool mute) } } if (mode == "poly") - setVolume_poly(volume, (dparam < 0) ? 3. : dparam); + setVolume_poly(volume.volume, (dparam < 0) ? 3. : dparam); else - setVolume_exp(volume, (dparam < 0) ? 10. : dparam); + setVolume_exp(volume.volume, (dparam < 0) ? 10. : dparam); } else if (settings_.mixer.mode == ClientSettings::Mixer::Mode::script) { - try + static std::optional pending_volume_setting; + static bool script_running = false; + if (script_running) { -#ifdef WINDOWS - string cmd = settings_.mixer.parameter + " --volume " + cpt::to_string(volume) + " --mute " + (mute ? "true" : "false"); - std::system(cmd.c_str()); -#else - using namespace boost::process; - child c(exe = settings_.mixer.parameter, args = {"--volume", cpt::to_string(volume), "--mute", mute ? "true" : "false"}); - c.detach(); -#endif + pending_volume_setting = volume; + LOG(DEBUG, LOG_TAG) << "Volume script still running, deferring this volume setting\n"; } - catch (const std::exception& e) + else { - LOG(ERROR, LOG_TAG) << "Failed to run script '" + settings_.mixer.parameter + "', error: " << e.what() << "\n"; + try + { + namespace procv2 = boost::process::v2; + script_running = true; + procv2::async_execute(procv2::process(io_context_, settings_.mixer.parameter, + {"--volume", cpt::to_string(volume.volume), "--mute", volume.mute ? "true" : "false"}), + [&](boost::system::error_code ec, int ret_val) + { + std::unique_lock lock(mutex_); + LOG(DEBUG, LOG_TAG) << "Error code: " << ec.message() << ", i: " << ret_val << "\n"; + script_running = false; + if (pending_volume_setting.has_value()) + { + Volume v = pending_volume_setting.value(); + pending_volume_setting = std::nullopt; + lock.unlock(); + setVolume(v); + } + }); + } + catch (const std::exception& e) + { + script_running = false; + LOG(ERROR, LOG_TAG) << "Failed to run script '" + settings_.mixer.parameter + "', error: " << e.what() << "\n"; + } } } } diff --git a/client/player/player.hpp b/client/player/player.hpp index e437b304..e793826f 100644 --- a/client/player/player.hpp +++ b/client/player/player.hpp @@ -1,6 +1,6 @@ /*** This file is part of snapcast - Copyright (C) 2014-2022 Johannes Pohl + Copyright (C) 2014-2024 Johannes Pohl This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -45,16 +45,21 @@ namespace player */ class Player { - using volume_callback = std::function; - public: + struct Volume + { + double volume{1.0}; + bool mute{false}; + }; + + using volume_callback = std::function; + Player(boost::asio::io_context& io_context, const ClientSettings::Player& settings, std::shared_ptr stream); virtual ~Player(); /// Set audio volume in range [0..1] - /// @param volume the volume on range [0..1] - /// @param muted muted or not - virtual void setVolume(double volume, bool mute); + /// @param volume the volume on range [0..1], muted or not + virtual void setVolume(const Volume& volume); /// Called on start, before the first audio sample is sent or any other function is called. /// In case of hardware mixer, it will call getVolume and notify the server about the current volume @@ -74,15 +79,14 @@ protected: virtual bool needsThread() const = 0; /// get the hardware mixer volume - /// @param[out] volume the volume on range [0..1] - /// @param[out] muted muted or not + /// @param[out] volume the volume on range [0..1], muted or not /// @return success or not - virtual bool getHardwareVolume(double& volume, bool& muted); + virtual bool getHardwareVolume(Volume& volume); /// set the hardware mixer volume - /// @param volume the volume on range [0..1] + /// @param volume the volume on range [0..1], muted or not /// @param muted muted or not - virtual void setHardwareVolume(double volume, bool muted); + virtual void setHardwareVolume(const Volume& volume); void setVolume_poly(double volume, double exp); void setVolume_exp(double volume, double base); @@ -92,10 +96,10 @@ protected: /// Notify the server about hardware volume changes /// @param volume the volume in range [0..1] /// @param muted if muted or not - void notifyVolumeChange(double volume, bool muted) const + void notifyVolumeChange(const Volume& volume) const { if (onVolumeChanged_) - onVolumeChanged_(volume, muted); + onVolumeChanged_(volume); } boost::asio::io_context& io_context_; @@ -103,8 +107,7 @@ protected: std::shared_ptr stream_; std::thread playerThread_; ClientSettings::Player settings_; - double volume_; - bool muted_; + Player::Volume volume_; double volCorrection_; volume_callback onVolumeChanged_; mutable std::mutex mutex_; @@ -119,6 +122,16 @@ private: } }; +inline bool operator==(const Player::Volume& lhs, const Player::Volume& rhs) +{ + return ((lhs.volume == rhs.volume) && (lhs.mute == rhs.mute)); +} + +inline bool operator!=(const Player::Volume& lhs, const Player::Volume& rhs) +{ + return !(lhs == rhs); +} + } // namespace player #endif diff --git a/client/player/pulse_player.cpp b/client/player/pulse_player.cpp index 1405e59c..06f36452 100644 --- a/client/player/pulse_player.cpp +++ b/client/player/pulse_player.cpp @@ -1,6 +1,6 @@ /*** This file is part of snapcast - Copyright (C) 2014-2021 Johannes Pohl + Copyright (C) 2014-2024 Johannes Pohl This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -219,7 +219,7 @@ void PulsePlayer::worker() } } -void PulsePlayer::setHardwareVolume(double volume, bool muted) +void PulsePlayer::setHardwareVolume(const Volume& volume) { // if we're not connected to pulse, ignore the hardware volume change as the volume will be set upon reconnect if (playstream_ == nullptr) @@ -227,21 +227,20 @@ void PulsePlayer::setHardwareVolume(double volume, bool muted) last_change_ = std::chrono::steady_clock::now(); pa_cvolume cvolume; - if (muted) + if (volume.mute) pa_cvolume_set(&cvolume, stream_->getFormat().channels(), PA_VOLUME_MUTED); else - pa_cvolume_set(&cvolume, stream_->getFormat().channels(), volume * PA_VOLUME_NORM); + pa_cvolume_set(&cvolume, stream_->getFormat().channels(), volume.volume * PA_VOLUME_NORM); pa_context_set_sink_input_volume(pa_ctx_, pa_stream_get_index(playstream_), &cvolume, nullptr, nullptr); } -bool PulsePlayer::getHardwareVolume(double& volume, bool& muted) +bool PulsePlayer::getHardwareVolume(Volume& volume) { // This is called during start to send the initial volume to the server // Because getting the volume works async, we return false here // and instead trigger volume notification in pa_context_subscribe std::ignore = volume; - std::ignore = muted; return false; } @@ -257,9 +256,9 @@ void PulsePlayer::triggerVolumeUpdate() if (info != nullptr) { auto* self = static_cast(userdata); - self->volume_ = static_cast(pa_cvolume_avg(&(info->volume))) / static_cast(PA_VOLUME_NORM); - self->muted_ = (info->mute != 0); - LOG(DEBUG, LOG_TAG) << "Volume changed: " << self->volume_ << ", muted: " << self->muted_ << "\n"; + self->volume_.volume = static_cast(pa_cvolume_avg(&(info->volume))) / static_cast(PA_VOLUME_NORM); + self->volume_.mute = (info->mute != 0); + LOG(DEBUG, LOG_TAG) << "Volume changed: " << self->volume_.volume << ", muted: " << self->volume_.mute << "\n"; auto now = std::chrono::steady_clock::now(); if (now - self->last_change_ < 1s) @@ -269,7 +268,7 @@ void PulsePlayer::triggerVolumeUpdate() << " ms => ignoring volume change\n"; return; } - self->notifyVolumeChange(self->volume_, self->muted_); + self->notifyVolumeChange(self->volume_); } }, this); diff --git a/client/player/pulse_player.hpp b/client/player/pulse_player.hpp index a16033ba..ad2e4031 100644 --- a/client/player/pulse_player.hpp +++ b/client/player/pulse_player.hpp @@ -1,6 +1,6 @@ /*** This file is part of snapcast - Copyright (C) 2014-2022 Johannes Pohl + Copyright (C) 2014-2024 Johannes Pohl This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -59,8 +59,8 @@ protected: void connect(); void disconnect(); - bool getHardwareVolume(double& volume, bool& muted) override; - void setHardwareVolume(double volume, bool muted) override; + bool getHardwareVolume(Volume& volume) override; + void setHardwareVolume(const Volume& volume) override; void triggerVolumeUpdate();