Skip to content

Commit

Permalink
Fix script volume zombie processes
Browse files Browse the repository at this point in the history
  • Loading branch information
badaix committed Mar 31, 2024
1 parent 3c39196 commit 151e3c1
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 99 deletions.
18 changes: 8 additions & 10 deletions client/controller.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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<msg::ClientInfo>();
info->setVolume(static_cast<uint16_t>(volume * 100.));
info->setMuted(muted);
info->setVolume(static_cast<uint16_t>(volume.volume * 100.));
info->setMuted(volume.mute);
clientConnection_->send(info,
[this](const boost::system::error_code& ec)
{
Expand All @@ -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
Expand Down
39 changes: 20 additions & 19 deletions client/player/alsa_player.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<std::recursive_mutex> lock(mutex_);
if (elem_ == nullptr)
Expand All @@ -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";
Expand All @@ -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));
}
Expand All @@ -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));
}
Expand All @@ -134,7 +134,7 @@ void AlsaPlayer::setHardwareVolume(double volume, bool muted)
}


bool AlsaPlayer::getHardwareVolume(double& volume, bool& muted)
bool AlsaPlayer::getHardwareVolume(Volume& volume)
{
try
{
Expand All @@ -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
Expand All @@ -168,13 +168,14 @@ bool AlsaPlayer::getHardwareVolume(double& volume, bool& muted)

vol -= minv;
maxv = maxv - minv;
volume = static_cast<double>(vol) / static_cast<double>(maxv);
volume.volume = static_cast<double>(vol) / static_cast<double>(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;
}
Expand Down Expand Up @@ -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_);
}
}
});
Expand Down Expand Up @@ -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)
{
Expand Down
6 changes: 3 additions & 3 deletions client/player/alsa_player.hpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -68,8 +68,8 @@ class AlsaPlayer : public Player
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();

Expand Down
86 changes: 47 additions & 39 deletions client/player/player.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -26,25 +26,16 @@
#include "common/utils/string_utils.hpp"

// 3rd party headers
#ifdef WINDOWS
#include <cstdlib>
#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 <boost/process/args.hpp>
#include <boost/process/child.hpp>
#include <boost/process/exe.hpp>
#include <boost/process/v2.hpp>
#pragma GCC diagnostic pop
#endif

// standard headers
#include <cmath>
#include <iostream>
#include <memory>
#include <optional>


using namespace std;
Expand All @@ -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> 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)
Expand Down Expand Up @@ -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");
}

Expand All @@ -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_;
}

Expand All @@ -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";
}


Expand All @@ -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<std::mutex> 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)
{
Expand All @@ -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<Volume> 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<std::mutex> 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";
}
}
}
}
Expand Down
Loading

0 comments on commit 151e3c1

Please sign in to comment.