From fbed679819b6e72aa42ca0797eaf1f9e5bf49e37 Mon Sep 17 00:00:00 2001 From: TermeHansen Date: Wed, 21 Oct 2026 23:57:55 +0200 Subject: [PATCH 1/6] Add --alsa-mixer: hardware volume through an ALSA mixer control --alsa-mixer drives a hardware mixer for volume and mute instead of scaling the stream in software, mapped over the control's dB range the way alsamixer lays its scale out. The level the mixer is at at startup seeds the player and is shown in status until a server sends its own. A mixer that cannot be opened or set falls back to software gain; the flag is refused off the ALSA backend and alongside --fixed-volume. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- docs/wiki/Configuration.md | 1 + docs/wiki/Controlling-the-Player.md | 2 + packaging/sendspin-cli.conf.example | 7 ++ src/alsa_sink.cpp | 176 +++++++++++++++++++++++++++- src/alsa_sink.h | 30 ++++- src/audio_sink.cpp | 7 +- src/audio_sink.h | 4 +- src/cli.cpp | 36 +++++- src/cli.h | 5 + src/control_common.cpp | 2 + src/main.cpp | 42 +++++-- src/player_listener.cpp | 8 ++ src/player_listener.h | 3 + tests/cli_test.cpp | 40 +++++++ tests/config_file_test.cpp | 15 +++ 15 files changed, 364 insertions(+), 14 deletions(-) diff --git a/docs/wiki/Configuration.md b/docs/wiki/Configuration.md index f76cae2..54c68b8 100644 --- a/docs/wiki/Configuration.md +++ b/docs/wiki/Configuration.md @@ -59,6 +59,7 @@ already does that. | `output-delay` | `--output-delay` | latency this endpoint's hardware adds after the audio port, 0–5000. `static-delay` / `--static-delay` is still accepted as the old name | `0` | | `allow-unpaired` | `--allow-unpaired` | `true`/`false` — let a server that has not paired with this player play on it | `false` | | `fixed-volume` | `--fixed-volume` | `true`/`false` — play at full volume, unmuted, and offer the server neither volume nor mute, for an output whose level is set elsewhere (a line-out into an amplifier). A remembered volume is kept but ignored, and returns once the key is dropped | `false` | +| `alsa-mixer` | `--alsa-mixer` | `:` — drive this ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`. The value is mapped over the control's dB range the way alsamixer lays its scale out, and the level the mixer is at when the player starts is what it reports and keeps until a server sends its own. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `fixed-volume` | none | | `pairing-code` | `--pairing-code` | exactly 8 digits — a static code a server can pair with, confirmed each time with `sendspin-cli pair confirm`; replaces the 6-digit code the player otherwise shows. See [Pairing a Player](Pairing-a-Player#3-static-code) | none | | `no-mdns` | `--no-mdns` | `true`/`false` — do not advertise `_sendspin._tcp` | `false` | | `mdns-name` | `--mdns-name` | the instance label to advertise, when it should differ from `name` | `name` | diff --git a/docs/wiki/Controlling-the-Player.md b/docs/wiki/Controlling-the-Player.md index 1bd1cc3..fa0e6fa 100644 --- a/docs/wiki/Controlling-the-Player.md +++ b/docs/wiki/Controlling-the-Player.md @@ -113,6 +113,8 @@ above `output:` says so, and appears whenever a server is connected. reads `100 (fixed; the server cannot set it)`: the output plays at full gain, unmuted, the server is offered neither volume nor mute, and a volume remembered from an earlier run is kept but ignored until the flag is dropped. `vol` and `mute` still address the group. + With `--alsa-mixer` it starts at the level the hardware control was already at, marked + `(read from the hardware mixer at startup)`, until the server sends a volume of its own. - **`state` and `stream` are different facts.** `state` is the *group's* transport state. `stream` is whether audio is arriving at *this* endpoint — a player dropped from the group loses it while the group plays on. A `stream: receiving` line with **no format after the diff --git a/packaging/sendspin-cli.conf.example b/packaging/sendspin-cli.conf.example index 6a64d72..a221e3c 100644 --- a/packaging/sendspin-cli.conf.example +++ b/packaging/sendspin-cli.conf.example @@ -46,6 +46,13 @@ # set elsewhere, such as a line-out into an amplifier. A remembered volume returns once dropped. #fixed-volume = true +# Drive an ALSA hardware mixer for volume and mute instead of scaling the stream in software, +# as card:control, e.g. 0:Master. The value is mapped over the control's dB range the way +# alsamixer lays its scale out, and the level the mixer is at when the player starts is what +# it reports and keeps until a server sends its own. ALSA outputs only; a mixer that cannot +# be opened or set falls back to software gain. Contradicts fixed-volume. +#alsa-mixer = 0:Master + # A static pairing code: exactly 8 digits, chosen at random for this player. Every attempt # with it waits for 'sendspin-cli pair confirm'. Unset, the player shows a fresh 6-digit code. #pairing-code = 48151623 diff --git a/src/alsa_sink.cpp b/src/alsa_sink.cpp index afd34d5..f662b94 100644 --- a/src/alsa_sink.cpp +++ b/src/alsa_sink.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -193,8 +194,8 @@ void install_alsa_error_handler() { std::call_once(once, [] { snd_lib_error_set_handler(&alsa_error_handler); }); } -AlsaAudioSink::AlsaAudioSink(std::string device, uint32_t buffer_ms) - : device_(std::move(device)), buffer_ms_(buffer_ms) {} +AlsaAudioSink::AlsaAudioSink(std::string device, uint32_t buffer_ms, std::string mixer_spec) + : device_(std::move(device)), buffer_ms_(buffer_ms), mixer_spec_(std::move(mixer_spec)) {} AlsaAudioSink::~AlsaAudioSink() { // A sink destroyed without stop() must still release the device. @@ -693,12 +694,25 @@ void AlsaAudioSink::stop() { void AlsaAudioSink::set_volume(uint8_t volume) { this->volume_.store(volume > 100 ? 100 : volume); + // Hardware first: a mixer that takes the value leaves the PCM scaling at unity. + if (!this->mixer_spec_.empty() && set_mixer_volume_(this->mixer_spec_, volume)) { + this->target_multiplier_.store(Q32_ONE, std::memory_order_relaxed); + cli_log(LogLevel::DEBUG, "alsa: mixer '%s' volume now %u", this->mixer_spec_.c_str(), + this->volume_.load()); + return; + } this->update_target_multiplier_(); cli_log(LogLevel::DEBUG, "alsa: volume now %u", this->volume_.load()); } void AlsaAudioSink::set_muted(bool muted) { this->muted_.store(muted); + if (!this->mixer_spec_.empty() && set_mixer_mute_(this->mixer_spec_, muted)) { + this->target_multiplier_.store(Q32_ONE, std::memory_order_relaxed); + cli_log(LogLevel::DEBUG, "alsa: mixer '%s' %s", this->mixer_spec_.c_str(), + muted ? "muted" : "unmuted"); + return; + } this->update_target_multiplier_(); cli_log(LogLevel::DEBUG, "alsa: %s", muted ? "muted" : "unmuted"); } @@ -747,4 +761,162 @@ void AlsaAudioSink::update_target_multiplier_() { std::memory_order_relaxed); } +// ============================================================================ +// Hardware mixer +// ============================================================================ + +snd_mixer_elem_t* AlsaAudioSink::open_mixer_(const std::string& mixer_spec, snd_mixer_t** handle) { + const size_t colon = mixer_spec.find(':'); + if (colon == std::string::npos || colon == 0 || colon + 1 == mixer_spec.size()) { + return nullptr; + } + const std::string card = mixer_spec.substr(0, colon); + const std::string control = mixer_spec.substr(colon + 1); + + if (snd_mixer_open(handle, 0) < 0) { + return nullptr; + } + const std::string hw = "hw:" + card; + if (snd_mixer_attach(*handle, hw.c_str()) < 0 || + snd_mixer_selem_register(*handle, nullptr, nullptr) < 0 || snd_mixer_load(*handle) < 0) { + snd_mixer_close(*handle); + return nullptr; + } + + 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, control.c_str()); + snd_mixer_elem_t* elem = snd_mixer_find_selem(*handle, sid); + if (elem == nullptr) { + snd_mixer_close(*handle); + } + return elem; +} + +bool AlsaAudioSink::playback_dB_range_(snd_mixer_elem_t* elem, long& min, long& max) { + if (!snd_mixer_selem_has_playback_volume(elem)) { + return false; + } + return snd_mixer_selem_get_playback_dB_range(elem, &min, &max) == 0; +} + +bool AlsaAudioSink::playback_dB_(snd_mixer_elem_t* elem, long& dB) { + // Channels share one dB range, so the first channel that reports carries the control's value. + for (unsigned int channel = 0; channel <= SND_MIXER_SCHN_LAST; ++channel) { + if (snd_mixer_selem_get_playback_dB( + elem, static_cast(channel), &dB) == 0) { + return true; + } + } + return false; +} + +bool AlsaAudioSink::playback_switch_(snd_mixer_elem_t* elem) { + int value = 0; + for (unsigned int channel = 0; channel <= SND_MIXER_SCHN_LAST; ++channel) { + if (snd_mixer_selem_get_playback_switch( + elem, static_cast(channel), &value) == 0) { + return value != 0; + } + } + return false; +} + +long AlsaAudioSink::mixer_dB_for_volume_(long min, long max, uint8_t volume) { + if (volume >= 100) { + return max; + } + if (volume == 0) { + return min; + } + // alsamixer's curve: linear in perceived loudness between the control's endpoints, not in + // dB, so a control whose floor is far below -100 dB stays usable across the whole scale. + const double min_norm = std::pow(10.0, static_cast(min - max) / 6000.0); + const double normalized = static_cast(volume) / 100.0 * (1.0 - min_norm) + min_norm; + return static_cast( + std::lround(6000.0 * std::log10(normalized) + static_cast(max))); +} + +uint8_t AlsaAudioSink::volume_for_mixer_dB_(long min, long max, long dB) { + double normalized = std::pow(10.0, static_cast(dB - max) / 6000.0); + if (min != SND_CTL_TLV_DB_GAIN_MUTE) { + const double min_norm = std::pow(10.0, static_cast(min - max) / 6000.0); + normalized = (normalized - min_norm) / (1.0 - min_norm); + } + return static_cast(std::lround(normalized * 100.0)); +} + +bool AlsaAudioSink::set_mixer_volume_(const std::string& mixer_spec, uint8_t volume) { + snd_mixer_t* handle = nullptr; + snd_mixer_elem_t* elem = open_mixer_(mixer_spec, &handle); + if (elem == nullptr) { + cli_log(LogLevel::WARN, "alsa: cannot open mixer '%s' -- using software volume", + mixer_spec.c_str()); + return false; + } + + long min = 0; + long max = 0; + if (!playback_dB_range_(elem, min, max) || + snd_mixer_selem_set_playback_dB_all(elem, mixer_dB_for_volume_(min, max, volume), 0) < 0) { + cli_log(LogLevel::WARN, "alsa: mixer '%s' refused volume %u -- using software volume", + mixer_spec.c_str(), volume); + snd_mixer_close(handle); + return false; + } + snd_mixer_close(handle); + return true; +} + +bool AlsaAudioSink::set_mixer_mute_(const std::string& mixer_spec, bool muted) { + snd_mixer_t* handle = nullptr; + snd_mixer_elem_t* elem = open_mixer_(mixer_spec, &handle); + if (elem == nullptr) { + cli_log(LogLevel::WARN, "alsa: cannot open mixer '%s' -- using software mute", + mixer_spec.c_str()); + return false; + } + // A control without a switch is not a failure: there is simply nothing to mute with. + const bool took = snd_mixer_selem_has_playback_switch(elem) && + snd_mixer_selem_set_playback_switch_all(elem, muted ? 0 : 1) == 0; + snd_mixer_close(handle); + return took; +} + +bool AlsaAudioSink::probe_mixer(const std::string& mixer_spec, std::string& error) { + install_alsa_error_handler(); + + snd_mixer_t* handle = nullptr; + if (open_mixer_(mixer_spec, &handle) == nullptr) { + error = "cannot open ALSA mixer '" + mixer_spec + + "' -- the spec is card:control, e.g. '0:Master', or run -l to list this host's " + "PCMs and amixer to list the controls"; + return false; + } + snd_mixer_close(handle); + return true; +} + +int AlsaAudioSink::read_mixer_volume(const std::string& mixer_spec, bool& muted) { + install_alsa_error_handler(); + + snd_mixer_t* handle = nullptr; + snd_mixer_elem_t* elem = open_mixer_(mixer_spec, &handle); + if (elem == nullptr) { + return -1; + } + muted = !playback_switch_(elem); + long min = 0; + long max = 0; + long dB = 0; + if (!playback_dB_(elem, dB) || !playback_dB_range_(elem, min, max)) { + snd_mixer_close(handle); + return -1; + } + const uint8_t volume = volume_for_mixer_dB_(min, max, dB); + snd_mixer_close(handle); + return volume; +} + } // namespace sendspin_cli diff --git a/src/alsa_sink.h b/src/alsa_sink.h index 99b3efb..9575e2f 100644 --- a/src/alsa_sink.h +++ b/src/alsa_sink.h @@ -36,7 +36,9 @@ namespace sendspin_cli { class AlsaAudioSink final : public AudioSink { public: /// @param buffer_ms Ring size request, already range-checked; ALSA rounds it to the card. - AlsaAudioSink(std::string device, uint32_t buffer_ms); + /// @param mixer_spec `card:control` naming the hardware mixer to drive, e.g. `1:Digital`; + /// empty keeps the software volume. Only the ALSA backend takes one. + AlsaAudioSink(std::string device, uint32_t buffer_ms, std::string mixer_spec = {}); ~AlsaAudioSink() override; std::string name() const override; @@ -56,6 +58,13 @@ class AlsaAudioSink final : public AudioSink { /// Checks that `device` names a playback PCM this host can open; a busy device passes. static bool probe(const std::string& device, std::string& error); + /// Checks that `mixer_spec` names a playback mixer control this host can open. + static bool probe_mixer(const std::string& mixer_spec, std::string& error); + + /// Reads the mixer's current volume and mute; -1 volume when the mixer cannot be read. + /// @param muted Set from the control's playback switch; false without one. + static int read_mixer_volume(const std::string& mixer_spec, bool& muted); + /// Prints the host's playback PCMs, as `aplay -L` does. static void list_devices(std::FILE* out); @@ -72,9 +81,28 @@ class AlsaAudioSink final : public AudioSink { bool handle_device_loss_(); void update_target_multiplier_(); + /// Opens the mixer for `mixer_spec`; caller closes *handle even on a null element. + static snd_mixer_elem_t* open_mixer_(const std::string& mixer_spec, snd_mixer_t** handle); + /// The dB range for playback volume, or false when the control has none. + static bool playback_dB_range_(snd_mixer_elem_t* elem, long& min, long& max); + /// A control with one dB range for both channels; false when no channel reports one. + static bool playback_dB_(snd_mixer_elem_t* elem, long& dB); + /// The playback switch state of the first channel that has one; false when none does. + static bool playback_switch_(snd_mixer_elem_t* elem); + /// Maps a 0-100 volume onto a dB range, alsamixer's way: linear in perceived loudness. + static long mixer_dB_for_volume_(long min, long max, uint8_t volume); + /// The inverse of mixer_dB_for_volume_(). + static uint8_t volume_for_mixer_dB_(long min, long max, long dB); + /// @return true when the control took the volume; false leaves the caller on software gain. + static bool set_mixer_volume_(const std::string& mixer_spec, uint8_t volume); + /// @return true when the control took the mute, or has no switch for the caller to fall back. + static bool set_mixer_mute_(const std::string& mixer_spec, bool muted); + std::string device_; /// Ring size to ask ALSA for, in milliseconds; the period is this over PERIODS_PER_BUFFER. uint32_t buffer_ms_; + /// `card:control` for hardware volume, empty for software volume. + std::string mixer_spec_; /// Serialises every snd_pcm_* call and the fields describing the open stream. std::mutex device_mutex_; diff --git a/src/audio_sink.cpp b/src/audio_sink.cpp index 3ab85e9..bbcb69d 100644 --- a/src/audio_sink.cpp +++ b/src/audio_sink.cpp @@ -303,7 +303,7 @@ void warn_if_shadowing_an_alsa_pcm([[maybe_unused]] const std::string& spec, // buffer_ms is unused on a build with no device-backed backend. std::unique_ptr make_audio_sink(const std::string& device, [[maybe_unused]] uint32_t buffer_ms, - std::string& error) { + const std::string& alsa_mixer, std::string& error) { DeviceSpec spec; if (!resolve_device_spec(device, spec, error)) { return nullptr; @@ -321,7 +321,10 @@ std::unique_ptr make_audio_sink(const std::string& device, if (!AlsaAudioSink::probe(spec.device, error)) { return nullptr; } - return std::make_unique(spec.device, buffer_ms); + if (!alsa_mixer.empty() && !AlsaAudioSink::probe_mixer(alsa_mixer, error)) { + return nullptr; + } + return std::make_unique(spec.device, buffer_ms, alsa_mixer); #else break; // unreachable: resolve_device_spec() never yields Alsa without the backend #endif diff --git a/src/audio_sink.h b/src/audio_sink.h index 05acd64..cc2a90f 100644 --- a/src/audio_sink.h +++ b/src/audio_sink.h @@ -56,6 +56,7 @@ enum class VolumeSource { Restored, ///< read back from the state store at startup Server, ///< a server sent a volume or mute command this run Fixed, ///< --fixed-volume: full gain, unmuted, and no server may change it + Hardware, ///< --alsa-mixer: read from the hardware mixer at startup }; /// The parameters configure() takes, as one value. @@ -152,9 +153,10 @@ void print_sink_capabilities(std::FILE* out, const SinkCapabilities& caps, /// Builds the sink named by -o. /// @param buffer_ms Already range-checked by the parser. +/// @param alsa_mixer `card:control` naming a hardware mixer; ignored off the ALSA backend. /// @return The sink, or nullptr with `error` set. std::unique_ptr make_audio_sink(const std::string& device, uint32_t buffer_ms, - std::string& error); + const std::string& alsa_mixer, std::string& error); /// Prints the device specs make_audio_sink() accepts, for -l. void print_audio_devices(std::FILE* out); diff --git a/src/cli.cpp b/src/cli.cpp index 37a83c7..c6410e7 100644 --- a/src/cli.cpp +++ b/src/cli.cpp @@ -71,6 +71,7 @@ enum LongOnly { OPT_INPUT, OPT_PAIRING_CODE, OPT_FIXED_VOLUME, + OPT_ALSA_MIXER, }; /// An option a config file may set: its key (the long flag name) and how diagnostics name it. @@ -111,6 +112,7 @@ const std::vector& settable_options() { {Opt::AllowUnpaired, "allow-unpaired", "--allow-unpaired"}, {Opt::PairingCode, "pairing-code", "--pairing-code"}, {Opt::FixedVolume, "fixed-volume", "--fixed-volume"}, + {Opt::AlsaMixer, "alsa-mixer", "--alsa-mixer"}, }; return table; } @@ -415,6 +417,12 @@ bool apply_option(const SettableOption& option, const std::string& value, Option return false; } break; + case Opt::AlsaMixer: + if (empty_value()) { + return false; + } + out.alsa_mixer = value; + break; case Opt::PairingCode: // The value is a credential, so the message does not repeat it. if (value.size() != 8 || value.find_first_not_of("0123456789") != std::string::npos) { @@ -580,6 +588,7 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { {"allow-unpaired", no_argument, nullptr, OPT_ALLOW_UNPAIRED}, {"pairing-code", required_argument, nullptr, OPT_PAIRING_CODE}, {"fixed-volume", no_argument, nullptr, OPT_FIXED_VOLUME}, + {"alsa-mixer", required_argument, nullptr, OPT_ALSA_MIXER}, {nullptr, 0, nullptr, 0}, }; @@ -725,6 +734,9 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { case OPT_FIXED_VOLUME: apply(Opt::FixedVolume, "true"); break; + case OPT_ALSA_MIXER: + apply(Opt::AlsaMixer, optarg); + break; case OPT_MANUFACTURER: apply(Opt::Manufacturer, optarg); break; @@ -819,6 +831,21 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { "' contradict each other -- drop one"); } + // The mixer spec names an ALSA control; no other backend has one. An unresolvable -o is + // left to make_audio_sink(), which names the device problem. + if (error.empty() && out.was_given(Opt::AlsaMixer)) { + DeviceSpec spec; + std::string reason; + if (resolve_device_spec(out.device, spec, reason) && spec.backend != SinkBackend::Alsa) { + fail("--alsa-mixer '" + out.alsa_mixer + + "' needs -o alsa: -- no other backend has a hardware mixer"); + } + if (out.fixed_volume) { + fail("--alsa-mixer and --fixed-volume contradict each other -- the mixer exists to " + "let the server set the level"); + } + } + // Absolutized before the length check, since the resolved path is what must fit. if (error.empty()) { // --no-control governs only a daemon; a subcommand still needs the path. @@ -864,7 +891,7 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { Opt::NoMdns, Opt::MdnsName, Opt::NoControl, Opt::StateDir, Opt::OutputDelay, Opt::HookStart, Opt::HookStop, Opt::ClientId, Opt::Manufacturer, Opt::ProductName, Opt::AudioFormat, Opt::AllowUnpaired, - Opt::Input, Opt::PairingCode, Opt::FixedVolume, + Opt::Input, Opt::PairingCode, Opt::FixedVolume, Opt::AlsaMixer, }; for (Opt opt : DAEMON_ONLY) { if (out.was_given(opt)) { @@ -1059,6 +1086,13 @@ void print_usage(std::FILE* out, const char* prog) { std::fprintf(out, " such as a line-out into an amplifier. A remembered\n"); std::fprintf(out, " volume is kept but ignored, and returns once the flag\n"); std::fprintf(out, " is dropped. 'vol' and 'mute' still address the group\n"); + std::fprintf(out, " --alsa-mixer \n"); + std::fprintf(out, " Drive this ALSA hardware mixer for volume and mute\n"); + std::fprintf(out, " instead of scaling the stream in software, e.g.\n"); + std::fprintf(out, " 0:Master. The value is mapped over the control's dB\n"); + std::fprintf(out, " range the way alsamixer lays its scale out. ALSA\n"); + std::fprintf(out, " outputs only; a mixer that cannot be opened or set\n"); + std::fprintf(out, " falls back to software gain\n"); std::fprintf(out, " --allow-unpaired\n"); std::fprintf(out, " Let a server that has not paired with this player play\n"); std::fprintf(out, " on it. Off by default: every connection is encrypted,\n"); diff --git a/src/cli.h b/src/cli.h index f6131c3..16fe565 100644 --- a/src/cli.h +++ b/src/cli.h @@ -84,6 +84,7 @@ enum class Opt : unsigned { Input, ///< --input PairingCode, ///< --pairing-code FixedVolume, ///< --fixed-volume + AlsaMixer, ///< --alsa-mixer }; /// Everything the flag surface configures. @@ -121,6 +122,10 @@ struct Options { /// --fixed-volume: play at full gain, unmuted, and withhold volume and mute from the server. bool fixed_volume{false}; + /// --alsa-mixer : drive this hardware mixer instead of scaling the PCM. + /// ALSA only; empty scales the stream in software. + std::string alsa_mixer; + /// --no-mdns: do not advertise `_sendspin._tcp`. bool no_mdns{false}; diff --git a/src/control_common.cpp b/src/control_common.cpp index bbc0693..cf156ef 100644 --- a/src/control_common.cpp +++ b/src/control_common.cpp @@ -177,6 +177,8 @@ const char* volume_source_note(VolumeSource source) { return ""; case VolumeSource::Fixed: return " (fixed; the server cannot set it)"; + case VolumeSource::Hardware: + return " (read from the hardware mixer at startup)"; case VolumeSource::SinkDefault: break; } diff --git a/src/main.cpp b/src/main.cpp index fa2b88b..1e1e0b1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -14,6 +14,9 @@ /// sendspin-cli: a headless Sendspin player, and its own control-socket client. +#ifdef SENDSPIN_CLI_HAVE_ALSA +#include "alsa_sink.h" +#endif #include "audio_sink.h" #include "audio_source.h" #include "cli.h" @@ -703,7 +706,8 @@ int main(int argc, char* argv[]) { } std::string sink_error; - std::unique_ptr sink = make_audio_sink(opts.device, opts.buffer_ms, sink_error); + std::unique_ptr sink = + make_audio_sink(opts.device, opts.buffer_ms, opts.alsa_mixer, sink_error); if (!sink) { log_fatal(LOG_TAG_AUDIO, "%s", sink_error.c_str()); return 1; @@ -886,12 +890,36 @@ int main(int argc, char* argv[]) { player_listener.fix_volume(); volume = DEFAULT_SINK_VOLUME; muted = false; - } else if (remembered) { - // Only when something was remembered, since restore_volume() marks the pair as restored. - player_listener.restore_volume(volume, muted); - log_line(LogLevel::INFO, LOG_TAG_PLAYER, "Restored volume %u%s from %s", - static_cast(volume), muted ? " (muted)" : "", - state_store.path().c_str()); + } else { + bool seeded_from_mixer = false; +#ifdef SENDSPIN_CLI_HAVE_ALSA + // The mixer is the truth about the level: it wins over a remembered volume, and the + // hardware value is what a controller should show until the server sends its own. + bool mixer_muted = false; + if (!opts.alsa_mixer.empty()) { + const int mixer_volume = AlsaAudioSink::read_mixer_volume(opts.alsa_mixer, mixer_muted); + if (mixer_volume >= 0) { + volume = static_cast(mixer_volume); + muted = mixer_muted; + player_listener.seed_volume(volume, muted); + seeded_from_mixer = true; + log_line(LogLevel::INFO, LOG_TAG_PLAYER, "Volume %u%s from ALSA mixer '%s'", + static_cast(volume), muted ? " (muted)" : "", + opts.alsa_mixer.c_str()); + } else { + log_line(LogLevel::WARN, LOG_TAG_PLAYER, + "Could not read volume from ALSA mixer '%s'", opts.alsa_mixer.c_str()); + } + } +#endif + // Only when something was remembered and the mixer did not already seed the pair, since + // restore_volume() marks the pair as restored. + if (remembered && !seeded_from_mixer) { + player_listener.restore_volume(volume, muted); + log_line(LogLevel::INFO, LOG_TAG_PLAYER, "Restored volume %u%s from %s", + static_cast(volume), muted ? " (muted)" : "", + state_store.path().c_str()); + } } player.update_volume(volume); player.update_muted(muted); diff --git a/src/player_listener.cpp b/src/player_listener.cpp index c8d3db8..038ed00 100644 --- a/src/player_listener.cpp +++ b/src/player_listener.cpp @@ -130,6 +130,14 @@ void PlayerListener::restore_volume(uint8_t volume, bool muted) { this->volume_source_ = VolumeSource::Restored; } +void PlayerListener::seed_volume(uint8_t volume, bool muted) { + this->sink_.set_volume(volume); + this->sink_.set_muted(muted); + this->applied_volume_ = volume; + this->applied_muted_ = muted; + this->volume_source_ = VolumeSource::Hardware; +} + void PlayerListener::fix_volume() { this->sink_.set_volume(DEFAULT_SINK_VOLUME); this->sink_.set_muted(false); diff --git a/src/player_listener.h b/src/player_listener.h index c3e5dc4..d7752fa 100644 --- a/src/player_listener.h +++ b/src/player_listener.h @@ -71,6 +71,9 @@ class PlayerListener final : public sendspin::PlayerRoleListener { /// Seeds the sink from the state store without marking it server-set. Call before connecting. void restore_volume(uint8_t volume, bool muted); + /// Seeds the sink from the hardware mixer, marking the pair hardware-sourced. + void seed_volume(uint8_t volume, bool muted); + /// Pins the sink at full gain, unmuted, and ignores volume and mute from then on. void fix_volume(); diff --git a/tests/cli_test.cpp b/tests/cli_test.cpp index 5abeeea..1402c8c 100644 --- a/tests/cli_test.cpp +++ b/tests/cli_test.cpp @@ -446,6 +446,46 @@ TEST(ParseOptions, FixedVolumeIsOffUnlessAsked) { EXPECT_TRUE(on.options().fixed_volume); } +TEST(ParseOptions, AlsaMixerDefaultsToSoftwareVolume) { + Parse parse({}); + + ASSERT_TRUE(parse.ok()) << parse.diagnostics(); + EXPECT_TRUE(parse.options().alsa_mixer.empty()); + EXPECT_FALSE(parse.options().was_given(Opt::AlsaMixer)); +} + +TEST(ParseOptions, AlsaMixerIsAccepted) { + Parse parse({"-o", "alsa:default", "--alsa-mixer", "0:Master"}); + + ASSERT_TRUE(parse.ok()) << parse.diagnostics(); + EXPECT_EQ(parse.options().alsa_mixer, "0:Master"); + EXPECT_TRUE(parse.options().was_given(Opt::AlsaMixer)); +} + +TEST(ParseOptions, AlsaMixerNeedsTheAlsaBackend) { + Parse parse({"-o", "null", "--alsa-mixer", "0:Master"}); + + EXPECT_FALSE(parse.ok()); + EXPECT_NE(parse.diagnostics().find("needs -o alsa:"), std::string::npos) + << parse.diagnostics(); +} + +TEST(ParseOptions, AlsaMixerContradictsFixedVolume) { + Parse parse({"-o", "alsa:default", "--alsa-mixer", "0:Master", "--fixed-volume"}); + + EXPECT_FALSE(parse.ok()); + EXPECT_NE(parse.diagnostics().find("contradict each other"), std::string::npos) + << parse.diagnostics(); +} + +TEST(ParseOptions, AlsaMixerRefusesAnEmptyValue) { + Parse parse({"-o", "alsa:default", "--alsa-mixer", ""}); + + EXPECT_FALSE(parse.ok()); + EXPECT_NE(parse.diagnostics().find("needs a non-empty value"), std::string::npos) + << parse.diagnostics(); +} + TEST(ParseOptions, OutputDelayDefaultsToNoDelay) { Parse parse({}); diff --git a/tests/config_file_test.cpp b/tests/config_file_test.cpp index bdaf6ab..d24a4f9 100644 --- a/tests/config_file_test.cpp +++ b/tests/config_file_test.cpp @@ -293,6 +293,21 @@ TEST(ConfigPrecedence, FixedVolumeIsSettableAndOffByDefault) { EXPECT_FALSE(defaulted.options().fixed_volume); } +TEST(ConfigPrecedence, AlsaMixerIsSettableAndLosesToTheCommandLine) { + ScratchDir scratch; + ASSERT_TRUE(scratch.created()); + const std::string file = scratch.write("mixer", "alsa-mixer = 1:Digital\n"); + + Parse from_file({}, file); + ASSERT_TRUE(from_file.ok()) << from_file.diagnostics(); + EXPECT_EQ(from_file.options().alsa_mixer, "1:Digital"); + EXPECT_TRUE(from_file.options().was_given(Opt::AlsaMixer)); + + Parse override({"--alsa-mixer", "0:Master"}, file); + ASSERT_TRUE(override.ok()) << override.diagnostics(); + EXPECT_EQ(override.options().alsa_mixer, "0:Master"); +} + TEST(ConfigPrecedence, PairingCodeIsSettableAndLosesToTheCommandLine) { ScratchDir scratch; ASSERT_TRUE(scratch.created()); From 42e61399d93adb4d1dc183f3d2ac6f1ab247aae8 Mon Sep 17 00:00:00 2001 From: Rasmus Date: Tue, 27 Oct 2026 08:58:54 +0100 Subject: [PATCH 2/6] Address PR #96 review: mute/volume independence and live mixer watch Two spec MUSTs were broken. A volume change could clear a mute that fell back to software gain, because set_volume forced the PCM multiplier to unity regardless of muted_; the software gain is now derived from both fields, so mute survives it. And the mixer was read once at startup, so a change from amixer, alsamixer or a hardware knob never reached client/state; the sink now keeps the control open, watches its poll descriptors, and publishes a change through the player role. Also from the review: the volume-to-dB curve now follows the spec's 30*log10(v/100) instead of alsamixer's twice-as-steep slope (-9.0 dB at 50, not -18.1), volume 0 silences the stream even when the control's floor is not silent, mixer writes step over a short ramp, set_mixer_mute_'s doc matches its return, and a switchless control no longer reports muted. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- CMakeLists.txt | 1 + docs/wiki/Configuration.md | 2 +- docs/wiki/Controlling-the-Player.md | 4 +- packaging/sendspin-cli.conf.example | 9 +- src/alsa_sink.cpp | 281 ++++++++++++++++++++++------ src/alsa_sink.h | 60 +++++- src/audio_sink.h | 4 + src/main.cpp | 5 + src/player_listener.cpp | 15 ++ src/player_listener.h | 4 + tests/alsa_mixer_test.cpp | 68 +++++++ 11 files changed, 383 insertions(+), 70 deletions(-) create mode 100644 tests/alsa_mixer_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 2bc5393..7f75b10 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -432,6 +432,7 @@ if(SENDSPIN_CLI_BUILD_TESTS) include(GoogleTest) add_executable(sendspin-cli-tests + tests/alsa_mixer_test.cpp tests/audio_source_test.cpp tests/cli_test.cpp tests/config_file_test.cpp diff --git a/docs/wiki/Configuration.md b/docs/wiki/Configuration.md index 54c68b8..bf71e92 100644 --- a/docs/wiki/Configuration.md +++ b/docs/wiki/Configuration.md @@ -59,7 +59,7 @@ already does that. | `output-delay` | `--output-delay` | latency this endpoint's hardware adds after the audio port, 0–5000. `static-delay` / `--static-delay` is still accepted as the old name | `0` | | `allow-unpaired` | `--allow-unpaired` | `true`/`false` — let a server that has not paired with this player play on it | `false` | | `fixed-volume` | `--fixed-volume` | `true`/`false` — play at full volume, unmuted, and offer the server neither volume nor mute, for an output whose level is set elsewhere (a line-out into an amplifier). A remembered volume is kept but ignored, and returns once the key is dropped | `false` | -| `alsa-mixer` | `--alsa-mixer` | `:` — drive this ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`. The value is mapped over the control's dB range the way alsamixer lays its scale out, and the level the mixer is at when the player starts is what it reports and keeps until a server sends its own. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `fixed-volume` | none | +| `alsa-mixer` | `--alsa-mixer` | `:` — drive this ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`. The value is mapped over the control's dB range on the spec's curve, and the level the mixer is at when the player starts is what it reports and keeps until a server sends its own. A change made outside the player (alsamixer, `amixer`, a hardware knob) is reported to the server, and volume 0 silences the stream even when the control's floor is not silent. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `fixed-volume` | none | | `pairing-code` | `--pairing-code` | exactly 8 digits — a static code a server can pair with, confirmed each time with `sendspin-cli pair confirm`; replaces the 6-digit code the player otherwise shows. See [Pairing a Player](Pairing-a-Player#3-static-code) | none | | `no-mdns` | `--no-mdns` | `true`/`false` — do not advertise `_sendspin._tcp` | `false` | | `mdns-name` | `--mdns-name` | the instance label to advertise, when it should differ from `name` | `name` | diff --git a/docs/wiki/Controlling-the-Player.md b/docs/wiki/Controlling-the-Player.md index fa0e6fa..f6dfe5b 100644 --- a/docs/wiki/Controlling-the-Player.md +++ b/docs/wiki/Controlling-the-Player.md @@ -114,7 +114,9 @@ above `output:` says so, and appears whenever a server is connected. server is offered neither volume nor mute, and a volume remembered from an earlier run is kept but ignored until the flag is dropped. `vol` and `mute` still address the group. With `--alsa-mixer` it starts at the level the hardware control was already at, marked - `(read from the hardware mixer at startup)`, until the server sends a volume of its own. + `(read from the hardware mixer at startup)`, until the server sends a volume of its own. A + later change made outside the player — in alsamixer, `amixer`, or by a hardware knob — is + picked up and reported, so a controller keeps showing the real level. - **`state` and `stream` are different facts.** `state` is the *group's* transport state. `stream` is whether audio is arriving at *this* endpoint — a player dropped from the group loses it while the group plays on. A `stream: receiving` line with **no format after the diff --git a/packaging/sendspin-cli.conf.example b/packaging/sendspin-cli.conf.example index a221e3c..3844ea8 100644 --- a/packaging/sendspin-cli.conf.example +++ b/packaging/sendspin-cli.conf.example @@ -47,10 +47,11 @@ #fixed-volume = true # Drive an ALSA hardware mixer for volume and mute instead of scaling the stream in software, -# as card:control, e.g. 0:Master. The value is mapped over the control's dB range the way -# alsamixer lays its scale out, and the level the mixer is at when the player starts is what -# it reports and keeps until a server sends its own. ALSA outputs only; a mixer that cannot -# be opened or set falls back to software gain. Contradicts fixed-volume. +# as card:control, e.g. 0:Master. The value is mapped over the control's dB range on the spec's +# curve, and the level the mixer is at when the player starts is what it reports and keeps until +# a server sends its own. A change made outside the player (alsamixer, amixer, a hardware knob) +# is reported to the server, and volume 0 silences the stream. ALSA outputs only; a mixer that +# cannot be opened or set falls back to software gain. Contradicts fixed-volume. #alsa-mixer = 0:Master # A static pairing code: exactly 8 digits, chosen at random for this player. Every attempt diff --git a/src/alsa_sink.cpp b/src/alsa_sink.cpp index f662b94..3a0f636 100644 --- a/src/alsa_sink.cpp +++ b/src/alsa_sink.cpp @@ -19,6 +19,8 @@ #include +#include + #include #include #include @@ -53,6 +55,17 @@ constexpr int WAIT_SLICE_MS = 20; constexpr int RESUME_TRIES = 10; constexpr auto RESUME_PAUSE = std::chrono::milliseconds(10); +/// A hardware mixer write is stepped over this long, so the level does not jump in one click. +constexpr int MIXER_RAMP_STEPS = 4; +constexpr int MIXER_RAMP_STEP_MS = 5; + +/// Monotonic milliseconds, for spacing mixer ramp steps. +int64_t mixer_now_ms() { + return std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()) + .count(); +} + /// ALSA's spelling of PROBE_BIT_DEPTHS; 24-bit is packed S24_3LE, not padded S24_LE. constexpr std::array PROBE_FORMATS{ SND_PCM_FORMAT_S8, @@ -195,13 +208,22 @@ void install_alsa_error_handler() { } AlsaAudioSink::AlsaAudioSink(std::string device, uint32_t buffer_ms, std::string mixer_spec) - : device_(std::move(device)), buffer_ms_(buffer_ms), mixer_spec_(std::move(mixer_spec)) {} + : device_(std::move(device)), buffer_ms_(buffer_ms), mixer_spec_(std::move(mixer_spec)) { + if (!this->mixer_spec_.empty()) { + install_alsa_error_handler(); + // Held open for the run, so poll() can watch it for changes made outside this process. + this->mixer_open_(); + } +} AlsaAudioSink::~AlsaAudioSink() { // A sink destroyed without stop() must still release the device. this->stopping_.store(true); - const std::lock_guard lock(this->device_mutex_); - this->close_device_(); + { + const std::lock_guard lock(this->device_mutex_); + this->close_device_(); + } + this->mixer_close_(); } std::string AlsaAudioSink::name() const { @@ -694,12 +716,11 @@ void AlsaAudioSink::stop() { void AlsaAudioSink::set_volume(uint8_t volume) { this->volume_.store(volume > 100 ? 100 : volume); - // Hardware first: a mixer that takes the value leaves the PCM scaling at unity. - if (!this->mixer_spec_.empty() && set_mixer_volume_(this->mixer_spec_, volume)) { - this->target_multiplier_.store(Q32_ONE, std::memory_order_relaxed); - cli_log(LogLevel::DEBUG, "alsa: mixer '%s' volume now %u", this->mixer_spec_.c_str(), - this->volume_.load()); - return; + // The mixer carries the level when it has the control; the software gain only ever covers + // what the mixer cannot, so it is recomputed from both fields rather than forced to unity. + if (this->mixer_elem_ != nullptr) { + start_mixer_ramp_( + mixer_dB_for_volume(this->mixer_dB_min_, this->mixer_dB_max_, this->volume_.load())); } this->update_target_multiplier_(); cli_log(LogLevel::DEBUG, "alsa: volume now %u", this->volume_.load()); @@ -707,17 +728,18 @@ void AlsaAudioSink::set_volume(uint8_t volume) { void AlsaAudioSink::set_muted(bool muted) { this->muted_.store(muted); - if (!this->mixer_spec_.empty() && set_mixer_mute_(this->mixer_spec_, muted)) { - this->target_multiplier_.store(Q32_ONE, std::memory_order_relaxed); - cli_log(LogLevel::DEBUG, "alsa: mixer '%s' %s", this->mixer_spec_.c_str(), - muted ? "muted" : "unmuted"); - return; + // A control without a switch leaves the mute to the software gain below. + if (this->mixer_elem_ != nullptr && this->mixer_has_switch_) { + set_mixer_mute_(muted); } this->update_target_multiplier_(); cli_log(LogLevel::DEBUG, "alsa: %s", muted ? "muted" : "unmuted"); } void AlsaAudioSink::poll(int64_t now_ms) { + // Before the fast path: a ramp step and an external mixer change are both due on every tick. + this->poll_mixer_(); + // Unlocked fast path: nothing to do on almost every tick. if (!this->recovery_.pending() || this->stopping_.load()) { return; @@ -757,8 +779,21 @@ void AlsaAudioSink::poll(int64_t now_ms) { void AlsaAudioSink::update_target_multiplier_() { // Only the target moves; write() walks current_multiplier_ toward it. - this->target_multiplier_.store(q32_gain_for(this->volume_.load(), this->muted_.load()), - std::memory_order_relaxed); + this->target_multiplier_.store(this->software_gain_(), std::memory_order_relaxed); +} + +uint64_t AlsaAudioSink::software_gain_() const { + // Volume 0 is silence per the spec; a control whose floor is only -60 dB cannot reach it, so + // the PCM gain carries it. This is independent of mute, which stays on the switch. + if (this->volume_.load() == 0) { + return 0; + } + // The mixer owns the level when it has a control, so the PCM stays at unity and only a mute + // without a hardware switch falls back to gain 0. Without a mixer, volume stays in software. + if (this->mixer_elem_ != nullptr) { + return (this->muted_.load() && !this->mixer_has_switch_) ? 0 : Q32_ONE; + } + return q32_gain_for(this->volume_.load(), this->muted_.load()); } // ============================================================================ @@ -823,65 +858,198 @@ bool AlsaAudioSink::playback_switch_(snd_mixer_elem_t* elem) { return false; } -long AlsaAudioSink::mixer_dB_for_volume_(long min, long max, uint8_t volume) { +long mixer_dB_for_volume(long min, long max, uint8_t volume) { if (volume >= 100) { return max; } if (volume == 0) { - return min; + return min; // the curve is -inf here; the floor is the closest a control can get } - // alsamixer's curve: linear in perceived loudness between the control's endpoints, not in - // dB, so a control whose floor is far below -100 dB stays usable across the whole scale. - const double min_norm = std::pow(10.0, static_cast(min - max) / 6000.0); - const double normalized = static_cast(volume) / 100.0 * (1.0 - min_norm) + min_norm; - return static_cast( - std::lround(6000.0 * std::log10(normalized) + static_cast(max))); + // The spec's curve, amplitude = (v/100)^1.5, is 30*log10(v/100) dB below max, in 0.01 dB. + const double dB = + static_cast(max) + 3000.0 * std::log10(static_cast(volume) / 100.0); + return std::max(min, static_cast(std::lround(dB))); } -uint8_t AlsaAudioSink::volume_for_mixer_dB_(long min, long max, long dB) { - double normalized = std::pow(10.0, static_cast(dB - max) / 6000.0); - if (min != SND_CTL_TLV_DB_GAIN_MUTE) { - const double min_norm = std::pow(10.0, static_cast(min - max) / 6000.0); - normalized = (normalized - min_norm) / (1.0 - min_norm); +uint8_t volume_for_mixer_dB(long min, long max, long dB) { + if (dB <= min) { + return 0; } - return static_cast(std::lround(normalized * 100.0)); + if (dB >= max) { + return 100; + } + // The inverse of mixer_dB_for_volume(). + const double volume = 100.0 * std::pow(10.0, static_cast(dB - max) / 3000.0); + return static_cast(std::lround(std::clamp(volume, 0.0, 100.0))); } -bool AlsaAudioSink::set_mixer_volume_(const std::string& mixer_spec, uint8_t volume) { - snd_mixer_t* handle = nullptr; - snd_mixer_elem_t* elem = open_mixer_(mixer_spec, &handle); - if (elem == nullptr) { +bool AlsaAudioSink::mixer_open_() { + this->mixer_ = nullptr; + this->mixer_elem_ = nullptr; + this->mixer_elem_ = open_mixer_(this->mixer_spec_, &this->mixer_); + if (this->mixer_elem_ == nullptr) { cli_log(LogLevel::WARN, "alsa: cannot open mixer '%s' -- using software volume", - mixer_spec.c_str()); + this->mixer_spec_.c_str()); + return false; + } + if (!playback_dB_range_(this->mixer_elem_, this->mixer_dB_min_, this->mixer_dB_max_)) { + cli_log(LogLevel::WARN, "alsa: mixer '%s' has no playback volume -- using software volume", + this->mixer_spec_.c_str()); + this->mixer_close_(); return false; } + this->mixer_has_switch_ = snd_mixer_selem_has_playback_switch(this->mixer_elem_) != 0; + this->sync_mixer_reported_(); + return true; +} - long min = 0; - long max = 0; - if (!playback_dB_range_(elem, min, max) || - snd_mixer_selem_set_playback_dB_all(elem, mixer_dB_for_volume_(min, max, volume), 0) < 0) { - cli_log(LogLevel::WARN, "alsa: mixer '%s' refused volume %u -- using software volume", - mixer_spec.c_str(), volume); - snd_mixer_close(handle); +void AlsaAudioSink::mixer_close_() { + if (this->mixer_ != nullptr) { + snd_mixer_close(this->mixer_); + } + this->mixer_ = nullptr; + this->mixer_elem_ = nullptr; + this->mixer_ramping_ = false; +} + +void AlsaAudioSink::sync_mixer_reported_() { + long dB = 0; + if (playback_dB_(this->mixer_elem_, dB)) { + this->mixer_reported_dB_ = dB; + } + this->mixer_reported_muted_ = this->mixer_has_switch_ && !playback_switch_(this->mixer_elem_); +} + +bool AlsaAudioSink::write_mixer_dB_(long dB) { + if (this->mixer_elem_ == nullptr) { return false; } - snd_mixer_close(handle); + if (snd_mixer_selem_set_playback_dB_all(this->mixer_elem_, dB, 0) < 0) { + return false; + } + // Store the read-back, not the request: ALSA rounds to the control's step, and the rounded + // value is what the next event reports. Recording the request would make that echo look like + // a change made outside this process. + long actual = dB; + if (playback_dB_(this->mixer_elem_, actual)) { + this->mixer_reported_dB_ = actual; + } else { + this->mixer_reported_dB_ = dB; + } return true; } -bool AlsaAudioSink::set_mixer_mute_(const std::string& mixer_spec, bool muted) { - snd_mixer_t* handle = nullptr; - snd_mixer_elem_t* elem = open_mixer_(mixer_spec, &handle); - if (elem == nullptr) { - cli_log(LogLevel::WARN, "alsa: cannot open mixer '%s' -- using software mute", - mixer_spec.c_str()); +bool AlsaAudioSink::set_mixer_mute_(bool muted) { + if (this->mixer_elem_ == nullptr || !this->mixer_has_switch_) { return false; } - // A control without a switch is not a failure: there is simply nothing to mute with. - const bool took = snd_mixer_selem_has_playback_switch(elem) && - snd_mixer_selem_set_playback_switch_all(elem, muted ? 0 : 1) == 0; - snd_mixer_close(handle); - return took; + if (snd_mixer_selem_set_playback_switch_all(this->mixer_elem_, muted ? 0 : 1) < 0) { + return false; + } + this->mixer_reported_muted_ = muted; + return true; +} + +bool AlsaAudioSink::start_mixer_ramp_(long target_dB) { + if (this->mixer_elem_ == nullptr) { + return false; + } + // A move of one step or less is not worth a ramp; write it now. + const long distance = std::abs(target_dB - this->mixer_reported_dB_); + const long step = + std::max(1, std::abs(this->mixer_dB_max_ - this->mixer_dB_min_) / MIXER_RAMP_STEPS); + if (distance <= step) { + return this->write_mixer_dB_(target_dB); + } + this->mixer_ramping_ = true; + this->mixer_ramp_from_dB_ = this->mixer_reported_dB_; + this->mixer_ramp_target_dB_ = target_dB; + this->mixer_ramp_step_ = 0; + this->mixer_ramp_next_ms_ = mixer_now_ms(); + return true; +} + +bool AlsaAudioSink::step_mixer_ramp_() { + if (!this->mixer_ramping_) { + return true; + } + ++this->mixer_ramp_step_; + const double progress = static_cast(this->mixer_ramp_step_) / MIXER_RAMP_STEPS; + const long dB = static_cast(std::lround( + static_cast(this->mixer_ramp_from_dB_) + + progress * (static_cast(this->mixer_ramp_target_dB_) - this->mixer_ramp_from_dB_))); + if (this->mixer_ramp_step_ >= MIXER_RAMP_STEPS) { + this->mixer_ramping_ = false; + return this->write_mixer_dB_(this->mixer_ramp_target_dB_); + } + this->mixer_ramp_next_ms_ = mixer_now_ms() + MIXER_RAMP_STEP_MS; + return this->write_mixer_dB_(dB); +} + +void AlsaAudioSink::report_mixer_change_() { + if (this->mixer_elem_ == nullptr) { + return; + } + long dB = 0; + const bool have_dB = playback_dB_(this->mixer_elem_, dB); + const bool muted = this->mixer_has_switch_ && !playback_switch_(this->mixer_elem_); + // Our own write stores its read-back, so any other value is a change made outside this + // process. A muted control reports its mute-level dB, which is not a volume change. + const bool dB_changed = have_dB && !muted && dB != this->mixer_reported_dB_; + const bool mute_changed = this->mixer_has_switch_ && muted != this->mixer_reported_muted_; + if (!dB_changed && !mute_changed) { + return; + } + if (have_dB && !muted) { + this->mixer_reported_dB_ = dB; + } + this->mixer_reported_muted_ = muted; + + const uint8_t volume = (have_dB && !muted) + ? volume_for_mixer_dB(this->mixer_dB_min_, this->mixer_dB_max_, dB) + : this->volume_.load(); + this->volume_.store(volume); + this->muted_.store(muted); + // A control whose floor is not silent reports volume 0 here; this is what makes it silent. + this->update_target_multiplier_(); + cli_log(LogLevel::INFO, "alsa: mixer '%s' changed to %u%s", this->mixer_spec_.c_str(), + static_cast(volume), muted ? " (muted)" : ""); + if (this->on_local_volume_changed) { + this->on_local_volume_changed(volume, muted); + } +} + +void AlsaAudioSink::poll_mixer_() { + if (this->mixer_ == nullptr) { + return; + } + if (this->mixer_ramping_ && mixer_now_ms() >= this->mixer_ramp_next_ms_) { + if (!this->step_mixer_ramp_()) { + cli_log(LogLevel::WARN, "alsa: mixer '%s' stopped taking the volume ramp", + this->mixer_spec_.c_str()); + this->mixer_ramping_ = false; + } + } + + // poll() with a zero timeout, so a tick never blocks; handle_events() only runs when a + // descriptor is readable, which is what keeps it from blocking on the control handle. + const int count = snd_mixer_poll_descriptors_count(this->mixer_); + if (count <= 0) { + return; + } + std::array pfds{}; + const unsigned int space = + std::min(static_cast(count), pfds.size()); + if (snd_mixer_poll_descriptors(this->mixer_, pfds.data(), space) <= 0) { + return; + } + if (::poll(pfds.data(), space, 0) <= 0) { + return; + } + // Read the element after draining rather than trusting the event count: the callback chain + // may report no event for a change, and the read is cheap and echo-suppressed anyway. + snd_mixer_handle_events(this->mixer_); + this->report_mixer_change_(); } bool AlsaAudioSink::probe_mixer(const std::string& mixer_spec, std::string& error) { @@ -906,7 +1074,8 @@ int AlsaAudioSink::read_mixer_volume(const std::string& mixer_spec, bool& muted) if (elem == nullptr) { return -1; } - muted = !playback_switch_(elem); + // A control without a switch is not muted: there is nothing whose state could say so. + muted = snd_mixer_selem_has_playback_switch(elem) != 0 && !playback_switch_(elem); long min = 0; long max = 0; long dB = 0; @@ -914,7 +1083,7 @@ int AlsaAudioSink::read_mixer_volume(const std::string& mixer_spec, bool& muted) snd_mixer_close(handle); return -1; } - const uint8_t volume = volume_for_mixer_dB_(min, max, dB); + const uint8_t volume = volume_for_mixer_dB(min, max, dB); snd_mixer_close(handle); return volume; } diff --git a/src/alsa_sink.h b/src/alsa_sink.h index 9575e2f..fbccf58 100644 --- a/src/alsa_sink.h +++ b/src/alsa_sink.h @@ -79,6 +79,7 @@ class AlsaAudioSink final : public AudioSink { /// Retires a lost device and arms poll() to get it back. Caller holds device_mutex_. /// @return false always. bool handle_device_loss_(); + /// Sets target_multiplier_ from volume_ and muted_ for the software-gain path. void update_target_multiplier_(); /// Opens the mixer for `mixer_spec`; caller closes *handle even on a null element. @@ -89,14 +90,30 @@ class AlsaAudioSink final : public AudioSink { static bool playback_dB_(snd_mixer_elem_t* elem, long& dB); /// The playback switch state of the first channel that has one; false when none does. static bool playback_switch_(snd_mixer_elem_t* elem); - /// Maps a 0-100 volume onto a dB range, alsamixer's way: linear in perceived loudness. - static long mixer_dB_for_volume_(long min, long max, uint8_t volume); - /// The inverse of mixer_dB_for_volume_(). - static uint8_t volume_for_mixer_dB_(long min, long max, long dB); - /// @return true when the control took the volume; false leaves the caller on software gain. - static bool set_mixer_volume_(const std::string& mixer_spec, uint8_t volume); - /// @return true when the control took the mute, or has no switch for the caller to fall back. - static bool set_mixer_mute_(const std::string& mixer_spec, bool muted); + + /// Opens the persistent mixer handle and element; false when the spec has no usable control. + bool mixer_open_(); + /// Closes the persistent mixer handle. Idempotent. + void mixer_close_(); + /// Records the control's current dB and switch as our own, so a later event reads as an echo. + void sync_mixer_reported_(); + /// Writes `dB` to the persistent element, recording the read-back as our own value. + /// @return false when there is no control, or ALSA refused the write. + bool write_mixer_dB_(long dB); + /// @return true when the control took the mute; false when it has no switch, leaving the + /// caller on software mute. + bool set_mixer_mute_(bool muted); + /// Begins a short stepped write toward `target_dB`; false when the first step was refused. + bool start_mixer_ramp_(long target_dB); + /// Writes one ramp step; a no-op outside a ramp. + /// @return false when the step was refused, which abandons the ramp where it stands. + bool step_mixer_ramp_(); + /// Records the mixer's current volume and mute, unless it is an echo of our own write. + void report_mixer_change_(); + /// Steps any ramp and handles a pending control event. Main-loop only. + void poll_mixer_(); + /// Q32 software gain to pair with the mixer: unity when it carries the level, 0 when silencing. + uint64_t software_gain_() const; std::string device_; /// Ring size to ask ALSA for, in milliseconds; the period is this over PERIODS_PER_BUFFER. @@ -130,8 +147,35 @@ class AlsaAudioSink final : public AudioSink { std::atomic target_multiplier_{Q32_ONE}; /// Q32 gain being applied, walked toward target_multiplier_; guarded by device_mutex_. std::uint64_t current_multiplier_{Q32_ONE}; + + /// The persistent mixer handle and element; both null when --alsa-mixer opened nothing. + /// Touched only from the main loop after construction, so no lock guards them. + snd_mixer_t* mixer_{nullptr}; + snd_mixer_elem_t* mixer_elem_{nullptr}; + /// The control's playback dB range; valid only while mixer_elem_ is set. + long mixer_dB_min_{0}; + long mixer_dB_max_{0}; + /// True when the control has a playback switch, so mute is hardware rather than software. + bool mixer_has_switch_{false}; + /// The switch state we last set or read; an event holding it is our own echo. + bool mixer_reported_muted_{false}; + /// dB the control was last set to by us or read back; an event holding it is our own echo. + long mixer_reported_dB_{0}; + /// A stepped write in progress: where it started, where it ends, and how far along it is. + bool mixer_ramping_{false}; + long mixer_ramp_from_dB_{0}; + long mixer_ramp_target_dB_{0}; + int mixer_ramp_step_{0}; + int64_t mixer_ramp_next_ms_{0}; }; +/// Maps a 0-100 volume onto a hardware mixer's dB range on the spec's curve, amplitude = +/// (v/100)^1.5, i.e. 30*log10(v/100) dB below max. Clamped to the control's floor. +long mixer_dB_for_volume(long min, long max, uint8_t volume); + +/// The inverse of mixer_dB_for_volume(): the 0-100 volume a mixer dB reading stands for. +uint8_t volume_for_mixer_dB(long min, long max, long dB); + /// Routes libasound's diagnostics to our logger; call before any snd_* that can fail. void install_alsa_error_handler(); diff --git a/src/audio_sink.h b/src/audio_sink.h index cc2a90f..42338da 100644 --- a/src/audio_sink.h +++ b/src/audio_sink.h @@ -113,6 +113,10 @@ class AudioSink { /// Assign before start() and never after: backends read it from their audio thread. std::function on_frames_played; + /// Reports a level changed outside the sink, e.g. through a hardware mixer. Fired on the main + /// loop from poll(); a sink that has no such control never calls it. + std::function on_local_volume_changed; + protected: AudioSink() = default; }; diff --git a/src/main.cpp b/src/main.cpp index 1e1e0b1..43412ce 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -860,6 +860,11 @@ int main(int argc, char* argv[]) { PairingListener pairing_listener; HostNetworkProvider network_provider; + // A hardware mixer turned by hand reaches the server through the listener's own pair. + sink->on_local_volume_changed = [&player_listener](uint8_t volume, bool muted) { + player_listener.on_sink_volume_changed(volume, muted); + }; + client.set_listener(&pairing_listener); player.set_listener(&player_listener); diff --git a/src/player_listener.cpp b/src/player_listener.cpp index 038ed00..f169619 100644 --- a/src/player_listener.cpp +++ b/src/player_listener.cpp @@ -146,6 +146,21 @@ void PlayerListener::fix_volume() { this->volume_source_ = VolumeSource::Fixed; } +void PlayerListener::on_sink_volume_changed(uint8_t volume, bool muted) { + // A local change while fixed must not reach the state file or the server's view. + if (this->volume_source_ == VolumeSource::Fixed) { + return; + } + this->applied_volume_ = volume; + this->applied_muted_ = muted; + this->volume_source_ = VolumeSource::Hardware; + // Through the role, not the sink: the sink already applied it, and this is what sends + // client/state out so a controller sees the new level. + this->player_.update_volume(volume); + this->player_.update_muted(muted); + this->persist_volume(); +} + void PlayerListener::persist_volume() const { if (this->store_ == nullptr) { return; diff --git a/src/player_listener.h b/src/player_listener.h index d7752fa..2aaab19 100644 --- a/src/player_listener.h +++ b/src/player_listener.h @@ -77,6 +77,10 @@ class PlayerListener final : public sendspin::PlayerRoleListener { /// Pins the sink at full gain, unmuted, and ignores volume and mute from then on. void fix_volume(); + /// Records volume and mute the sink applied on its own, e.g. a hardware mixer turned by hand, + /// and publishes them so the server sees the change. Main loop only. + void on_sink_volume_changed(uint8_t volume, bool muted); + /// The format the sink was configured for; absent between streams and for a refused one. const std::optional& stream_format() const { return this->stream_format_; diff --git a/tests/alsa_mixer_test.cpp b/tests/alsa_mixer_test.cpp new file mode 100644 index 0000000..fedecce --- /dev/null +++ b/tests/alsa_mixer_test.cpp @@ -0,0 +1,68 @@ +// Copyright 2026 sendspin-cpp-cli Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// The volume-to-dB curve behind --alsa-mixer; built only with the ALSA backend. + +#ifdef SENDSPIN_CLI_HAVE_ALSA + +#include "alsa_sink.h" + +#include + +#include + +namespace sendspin_cli { +namespace { + +// A control whose range runs from -60 dB to 0 dB, in hundredths of a dB. +constexpr long MIN_DB = -6000; +constexpr long MAX_DB = 0; + +TEST(MixerVolumeCurve, FullVolumeIsTheControlMax) { + EXPECT_EQ(mixer_dB_for_volume(MIN_DB, MAX_DB, 100), MAX_DB); +} + +TEST(MixerVolumeCurve, ZeroVolumeIsTheControlFloor) { + EXPECT_EQ(mixer_dB_for_volume(MIN_DB, MAX_DB, 0), MIN_DB); +} + +TEST(MixerVolumeCurve, FollowsTheSpecThirtyLogSlope) { + // The spec's (v/100)^1.5 is 30*log10(v/100) dB; at 50 that is -9.03 dB, not -18. + EXPECT_EQ(mixer_dB_for_volume(MIN_DB, MAX_DB, 50), -903); +} + +TEST(MixerVolumeCurve, IsClampedToTheControlFloor) { + // 30*log10(0.1) is -30 dB, below a floor of -20 dB, so the floor wins. + EXPECT_EQ(mixer_dB_for_volume(-2000, MAX_DB, 10), -2000); +} + +TEST(MixerVolumeCurve, RoundTripsThroughTheInverse) { + // 1 is left out: its dB lands on the floor, which the inverse reads back as 0. + for (uint8_t volume : {2, 25, 50, 75, 99, 100}) { + const long dB = mixer_dB_for_volume(MIN_DB, MAX_DB, volume); + EXPECT_EQ(volume_for_mixer_dB(MIN_DB, MAX_DB, dB), volume) << "volume " << int{volume}; + } +} + +TEST(MixerVolumeCurve, InverseClampsAtBothEnds) { + EXPECT_EQ(volume_for_mixer_dB(MIN_DB, MAX_DB, MIN_DB), 0); + EXPECT_EQ(volume_for_mixer_dB(MIN_DB, MAX_DB, MIN_DB - 100), 0); + EXPECT_EQ(volume_for_mixer_dB(MIN_DB, MAX_DB, MAX_DB), 100); + EXPECT_EQ(volume_for_mixer_dB(MIN_DB, MAX_DB, MAX_DB + 100), 100); +} + +} // namespace +} // namespace sendspin_cli + +#endif // SENDSPIN_CLI_HAVE_ALSA From 57308365e2ea889bd93fe98c66fe172b04b8b888 Mon Sep 17 00:00:00 2001 From: Rasmus Date: Tue, 27 Oct 2026 11:12:06 +0100 Subject: [PATCH 3/6] Document --alsa-mixer in the README option table The flag shipped without a README row; add one next to --fixed-volume, matching the wiki's Configuration table. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b98945e..d8a3d47 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,7 @@ sendspin-cli -n den --input default | `--input ` | Audio to stream to the server: a microphone or line-in, e.g. `default`, `hw:1,0`, `pulse`, `pipewire:` or `portaudio`, or `coreaudio` on macOS; or `loopback` for what this host plays (on macOS without the player's own playback, on Linux with it). `-l` lists them. Off unless given. See [Stream a microphone or line-in](https://github.com/Sendspin/sendspin-cpp-cli/wiki/Advanced-Usage#stream-a-microphone-or-line-in) and [Stream what this host plays](https://github.com/Sendspin/sendspin-cpp-cli/wiki/Advanced-Usage#stream-what-this-host-plays). | | `--allow-unpaired` | Let a server play without pairing with this player first. Off by default. | | `--fixed-volume` | Play at full volume, unmuted, and let the server set neither — for a line-out into an amplifier whose level is set there. | +| `--alsa-mixer ` | Drive an ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`, mapped over the control's dB range on the spec's curve. The level the mixer is at when the player starts is what it reports and keeps until a server sends its own, and a change made outside the player (alsamixer, `amixer`, a hardware knob) is reported to the server. Volume 0 silences the stream even when the control's floor is not silent. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `--fixed-volume`. | | `--release-device` | Close the output device when a stream ends and reopen it when the next one starts, so other players can take turns on an exclusive device such as an ALSA `hw:` DAC. Off by default: between streams another app can take the device, and the next stream is then discarded. ALSA and PortAudio only. | | `--pairing-code <8 digits>` | A fixed code a server can pair with, confirmed with `sendspin-cli pair confirm`. Without it the player shows a fresh 6-digit code per attempt. | | `-s, --server ` | Connect out to a server rather than waiting to be discovered, or `mdns:[]` to discover one over mDNS. Turns off the mDNS advertisement. | From 42d8315e72d0bd2de7303267fadc422f1f219eca Mon Sep 17 00:00:00 2001 From: Rasmus Date: Fri, 30 Oct 2026 10:25:40 +0100 Subject: [PATCH 4/6] Add a selectable mixer curve and repair a mixer dB floor of silence The hardware-mixer volume curve was hardcoded to the spec's 30*log10(v/100). --alsa-mixer-curve now picks between that and the steeper cubic curve (Snapcast's), whose inverse 100*10^(dB/60) is alsamixer's slider position, so the two track each other on a control with a wide range. Default stays spec; the flag needs --alsa-mixer, is settable from a config file, and rejects anything but spec or cubic. A control that mutes at its lowest step reports that step as SND_CTL_TLV_DB_GAIN_MUTE (-9999999), so its dB range came back as -99999.99 dB .. 0 dB. As the curve floor that collapsed every volume but 100% onto it and inflated the ramp step to the whole sentinel range. playback_dB_range_ now rebuilds the floor from the raw range and the per-step dB, which is where alsamixer gets the same numbers; a range without the sentinel is untouched. Verified on an IQaudIO DAC (card 1, Digital): the floor resolves to -103.50 dB instead of the sentinel, and the read-back matches (raw 153 -> -27.00 dB -> volume 12 spec / 35 cubic). Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- README.md | 3 +- docs/wiki/Configuration.md | 3 +- src/alsa_sink.cpp | 88 ++++++++++++++++++++++++++++++++------ src/alsa_sink.h | 31 +++++++++++--- src/audio_sink.cpp | 12 ++++-- src/audio_sink.h | 4 +- src/cli.cpp | 32 ++++++++++++-- src/cli.h | 5 +++ src/main.cpp | 9 ++-- tests/alsa_mixer_test.cpp | 33 ++++++++++++++ tests/cli_test.cpp | 32 ++++++++++++++ tests/config_file_test.cpp | 16 +++++++ 12 files changed, 237 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index d8a3d47..64ca0fa 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,8 @@ sendspin-cli -n den --input default | `--input ` | Audio to stream to the server: a microphone or line-in, e.g. `default`, `hw:1,0`, `pulse`, `pipewire:` or `portaudio`, or `coreaudio` on macOS; or `loopback` for what this host plays (on macOS without the player's own playback, on Linux with it). `-l` lists them. Off unless given. See [Stream a microphone or line-in](https://github.com/Sendspin/sendspin-cpp-cli/wiki/Advanced-Usage#stream-a-microphone-or-line-in) and [Stream what this host plays](https://github.com/Sendspin/sendspin-cpp-cli/wiki/Advanced-Usage#stream-what-this-host-plays). | | `--allow-unpaired` | Let a server play without pairing with this player first. Off by default. | | `--fixed-volume` | Play at full volume, unmuted, and let the server set neither — for a line-out into an amplifier whose level is set there. | -| `--alsa-mixer ` | Drive an ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`, mapped over the control's dB range on the spec's curve. The level the mixer is at when the player starts is what it reports and keeps until a server sends its own, and a change made outside the player (alsamixer, `amixer`, a hardware knob) is reported to the server. Volume 0 silences the stream even when the control's floor is not silent. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `--fixed-volume`. | +| `--alsa-mixer ` | Drive an ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`, mapped over the control's dB range on the curve `--alsa-mixer-curve` selects. The level the mixer is at when the player starts is what it reports and keeps until a server sends its own, and a change made outside the player (alsamixer, `amixer`, a hardware knob) is reported to the server. Volume 0 silences the stream even when the control's floor is not silent. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `--fixed-volume`. | +| `--alsa-mixer-curve ` | How volume maps onto the mixer's dB range. `spec` (default) is the perceived-loudness curve; `cubic` is the steeper one whose inverse is alsamixer's slider position, so the two track each other. Needs `--alsa-mixer`. | | `--release-device` | Close the output device when a stream ends and reopen it when the next one starts, so other players can take turns on an exclusive device such as an ALSA `hw:` DAC. Off by default: between streams another app can take the device, and the next stream is then discarded. ALSA and PortAudio only. | | `--pairing-code <8 digits>` | A fixed code a server can pair with, confirmed with `sendspin-cli pair confirm`. Without it the player shows a fresh 6-digit code per attempt. | | `-s, --server ` | Connect out to a server rather than waiting to be discovered, or `mdns:[]` to discover one over mDNS. Turns off the mDNS advertisement. | diff --git a/docs/wiki/Configuration.md b/docs/wiki/Configuration.md index be329ae..7a2d7fc 100644 --- a/docs/wiki/Configuration.md +++ b/docs/wiki/Configuration.md @@ -64,7 +64,8 @@ already does that. | `output-delay` | `--output-delay` | latency this endpoint's hardware adds after the audio port, 0–5000. `static-delay` / `--static-delay` is still accepted as the old name | `0` | | `allow-unpaired` | `--allow-unpaired` | `true`/`false` — let a server that has not paired with this player play on it | `false` | | `fixed-volume` | `--fixed-volume` | `true`/`false` — play at full volume, unmuted, and offer the server neither volume nor mute, for an output whose level is set elsewhere (a line-out into an amplifier). A remembered volume is kept but ignored, and returns once the key is dropped | `false` | -| `alsa-mixer` | `--alsa-mixer` | `:` — drive this ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`. The value is mapped over the control's dB range on the spec's curve, and the level the mixer is at when the player starts is what it reports and keeps until a server sends its own. A change made outside the player (alsamixer, `amixer`, a hardware knob) is reported to the server, and volume 0 silences the stream even when the control's floor is not silent. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `fixed-volume` | none | +| `alsa-mixer` | `--alsa-mixer` | `:` — drive this ALSA hardware mixer for volume and mute instead of scaling the stream in software, e.g. `0:Master`. The value is mapped over the control's dB range on the curve `alsa-mixer-curve` selects, and the level the mixer is at when the player starts is what it reports and keeps until a server sends its own. A change made outside the player (alsamixer, `amixer`, a hardware knob) is reported to the server, and volume 0 silences the stream even when the control's floor is not silent. ALSA outputs only; a mixer that cannot be opened or set falls back to software gain. Contradicts `fixed-volume` | none | +| `alsa-mixer-curve` | `--alsa-mixer-curve` | `spec` or `cubic` — how volume maps onto the mixer's dB range. `spec` (default) is the perceived-loudness curve, amplitude `(v/100)^1.5`; `cubic` is the steeper curve Snapcast uses, whose inverse `100*10^(dB/60)` is alsamixer's slider position, so the two track each other. Needs `alsa-mixer` | `spec` | | `release-device` | `--release-device` | `true`/`false` — close the output device at each stream end and reopen it at the next stream start, for an exclusive device such as an ALSA `hw:` DAC shared with other players (Squeezelite, AirPlay, Spotify). The trade-off: another app can take the device between streams, and the next stream is then discarded; each stream also pays a fresh open. ALSA and PortAudio only; other backends ignore it | `false` | | `pairing-code` | `--pairing-code` | exactly 8 digits — a static code a server can pair with, confirmed each time with `sendspin-cli pair confirm`; replaces the 6-digit code the player otherwise shows. See [Pairing a Player](Pairing-a-Player#3-static-code) | none | | `no-mdns` | `--no-mdns` | `true`/`false` — do not advertise `_sendspin._tcp` | `false` | diff --git a/src/alsa_sink.cpp b/src/alsa_sink.cpp index da681f6..4a230f7 100644 --- a/src/alsa_sink.cpp +++ b/src/alsa_sink.cpp @@ -207,8 +207,10 @@ void install_alsa_error_handler() { std::call_once(once, [] { snd_lib_error_set_handler(&alsa_error_handler); }); } -AlsaAudioSink::AlsaAudioSink(std::string device, uint32_t buffer_ms, std::string mixer_spec) - : device_(std::move(device)), buffer_ms_(buffer_ms), mixer_spec_(std::move(mixer_spec)) { +AlsaAudioSink::AlsaAudioSink(std::string device, uint32_t buffer_ms, std::string mixer_spec, + MixerCurve curve) + : device_(std::move(device)), buffer_ms_(buffer_ms), mixer_spec_(std::move(mixer_spec)), + mixer_curve_(curve) { if (!this->mixer_spec_.empty()) { install_alsa_error_handler(); // Held open for the run, so poll() can watch it for changes made outside this process. @@ -737,8 +739,8 @@ void AlsaAudioSink::set_volume(uint8_t volume) { // The mixer carries the level when it has the control; the software gain only ever covers // what the mixer cannot, so it is recomputed from both fields rather than forced to unity. if (this->mixer_elem_ != nullptr) { - start_mixer_ramp_( - mixer_dB_for_volume(this->mixer_dB_min_, this->mixer_dB_max_, this->volume_.load())); + start_mixer_ramp_(mixer_dB_for_volume(this->mixer_dB_min_, this->mixer_dB_max_, + this->volume_.load(), this->mixer_curve_)); } this->update_target_multiplier_(); cli_log(LogLevel::DEBUG, "alsa: volume now %u", this->volume_.load()); @@ -851,7 +853,40 @@ bool AlsaAudioSink::playback_dB_range_(snd_mixer_elem_t* elem, long& min, long& if (!snd_mixer_selem_has_playback_volume(elem)) { return false; } - return snd_mixer_selem_get_playback_dB_range(elem, &min, &max) == 0; + if (snd_mixer_selem_get_playback_dB_range(elem, &min, &max) != 0) { + return false; + } + // A control that mutes at its lowest step reports that step as SND_CTL_TLV_DB_GAIN_MUTE + // (-9999999) rather than a real level, so the range comes back as -99999.99 dB .. 0 dB. As a + // curve floor that makes every volume but 100% collapse onto it, and inflates the ramp step + // to the whole sentinel range. Rebuild the floor from the raw range and the per-step dB, + // which is where alsamixer gets the same numbers. A range without the sentinel is untouched. + if (min != SND_CTL_TLV_DB_GAIN_MUTE) { + return true; + } + long raw_min = 0; + long raw_max = 0; + if (snd_mixer_selem_get_playback_volume_range(elem, &raw_min, &raw_max) < 0 || + raw_max <= raw_min) { + return true; + } + long top = 0; + long previous = 0; + if (snd_mixer_selem_ask_playback_vol_dB(elem, raw_max, &top) < 0 || + snd_mixer_selem_ask_playback_vol_dB(elem, raw_max - 1, &previous) < 0) { + return true; + } + const long step = top - previous; + if (step <= 0) { + return true; + } + const long floor = top - (raw_max - raw_min) * step; + if (floor >= top) { + return true; + } + min = floor; + max = top; + return true; } bool AlsaAudioSink::playback_dB_(snd_mixer_elem_t* elem, long& dB) { @@ -876,20 +911,45 @@ bool AlsaAudioSink::playback_switch_(snd_mixer_elem_t* elem) { return false; } -long mixer_dB_for_volume(long min, long max, uint8_t volume) { +bool parse_mixer_curve(const std::string& name, MixerCurve& curve) { + if (name == "spec") { + curve = MixerCurve::kSpec; + } else if (name == "cubic") { + curve = MixerCurve::kCubic; + } else { + return false; + } + return true; +} + +namespace { + +/// dB per volume-decade * 100 for a curve, i.e. the slope of dB = max + slope*log10(v/100). +/// kSpec is 3000 (30 dB/decade); kCubic is 6000 (60 dB/decade). +int mixer_curve_slope_centi_dB(MixerCurve curve) { + return curve == MixerCurve::kCubic ? 6000 : 3000; +} + +} // namespace + +long mixer_dB_for_volume(long min, long max, uint8_t volume, MixerCurve curve) { if (volume >= 100) { return max; } if (volume == 0) { return min; // the curve is -inf here; the floor is the closest a control can get } - // The spec's curve, amplitude = (v/100)^1.5, is 30*log10(v/100) dB below max, in 0.01 dB. + // The spec's curve, amplitude = (v/100)^1.5, is 30*log10(v/100) dB below max, in 0.01 dB; + // the cubic (Snapcast) curve is twice that, and its inverse 100*10^(dB/60) is the position + // alsamixer gives the same control. Volume 0 never reaches the floor on either, which is + // what the min clamp handles. + const int slope = mixer_curve_slope_centi_dB(curve); const double dB = - static_cast(max) + 3000.0 * std::log10(static_cast(volume) / 100.0); + static_cast(max) + slope * std::log10(static_cast(volume) / 100.0); return std::max(min, static_cast(std::lround(dB))); } -uint8_t volume_for_mixer_dB(long min, long max, long dB) { +uint8_t volume_for_mixer_dB(long min, long max, long dB, MixerCurve curve) { if (dB <= min) { return 0; } @@ -897,7 +957,8 @@ uint8_t volume_for_mixer_dB(long min, long max, long dB) { return 100; } // The inverse of mixer_dB_for_volume(). - const double volume = 100.0 * std::pow(10.0, static_cast(dB - max) / 3000.0); + const int slope = mixer_curve_slope_centi_dB(curve); + const double volume = 100.0 * std::pow(10.0, static_cast(dB - max) / slope); return static_cast(std::lround(std::clamp(volume, 0.0, 100.0))); } @@ -1024,7 +1085,8 @@ void AlsaAudioSink::report_mixer_change_() { this->mixer_reported_muted_ = muted; const uint8_t volume = (have_dB && !muted) - ? volume_for_mixer_dB(this->mixer_dB_min_, this->mixer_dB_max_, dB) + ? volume_for_mixer_dB(this->mixer_dB_min_, this->mixer_dB_max_, dB, + this->mixer_curve_) : this->volume_.load(); this->volume_.store(volume); this->muted_.store(muted); @@ -1084,7 +1146,7 @@ bool AlsaAudioSink::probe_mixer(const std::string& mixer_spec, std::string& erro return true; } -int AlsaAudioSink::read_mixer_volume(const std::string& mixer_spec, bool& muted) { +int AlsaAudioSink::read_mixer_volume(const std::string& mixer_spec, bool& muted, MixerCurve curve) { install_alsa_error_handler(); snd_mixer_t* handle = nullptr; @@ -1101,7 +1163,7 @@ int AlsaAudioSink::read_mixer_volume(const std::string& mixer_spec, bool& muted) snd_mixer_close(handle); return -1; } - const uint8_t volume = volume_for_mixer_dB(min, max, dB); + const uint8_t volume = volume_for_mixer_dB(min, max, dB, curve); snd_mixer_close(handle); return volume; } diff --git a/src/alsa_sink.h b/src/alsa_sink.h index a6f9529..de025c7 100644 --- a/src/alsa_sink.h +++ b/src/alsa_sink.h @@ -31,6 +31,17 @@ namespace sendspin_cli { +/// How a 0-100 volume maps onto a hardware mixer control's dB range. +enum class MixerCurve { + kSpec, ///< The spec's perceived-loudness curve: amplitude (v/100)^1.5, 30*log10 dB. + kCubic, ///< Amplitude (v/100)^3, the steeper curve Snapcast uses for hardware volume. + ///< Its inverse, 100*10^(dB/60), is alsamixer's slider position, so the two track + ///< each other on a control with more than 24 dB of range. +}; + +/// Parses a curve name ("spec", "cubic"). @return false when unrecognized. +bool parse_mixer_curve(const std::string& name, MixerCurve& curve); + /// An AudioSink that plays through an ALSA PCM, e.g. `default` or `hw:2,0`. /// Every snd_pcm_* call holds device_mutex_. write() never reopens a dead device; poll() does. class AlsaAudioSink final : public AudioSink { @@ -38,7 +49,9 @@ class AlsaAudioSink final : public AudioSink { /// @param buffer_ms Ring size request, already range-checked; ALSA rounds it to the card. /// @param mixer_spec `card:control` naming the hardware mixer to drive, e.g. `1:Digital`; /// empty keeps the software volume. Only the ALSA backend takes one. - AlsaAudioSink(std::string device, uint32_t buffer_ms, std::string mixer_spec = {}); + /// @param curve How volume maps onto the mixer's dB range; ignored without a mixer. + AlsaAudioSink(std::string device, uint32_t buffer_ms, std::string mixer_spec = {}, + MixerCurve curve = MixerCurve::kSpec); ~AlsaAudioSink() override; std::string name() const override; @@ -64,7 +77,9 @@ class AlsaAudioSink final : public AudioSink { /// Reads the mixer's current volume and mute; -1 volume when the mixer cannot be read. /// @param muted Set from the control's playback switch; false without one. - static int read_mixer_volume(const std::string& mixer_spec, bool& muted); + /// @param curve How volume maps onto the mixer's dB range. + static int read_mixer_volume(const std::string& mixer_spec, bool& muted, + MixerCurve curve = MixerCurve::kSpec); /// Prints the host's playback PCMs, as `aplay -L` does. static void list_devices(std::FILE* out); @@ -121,6 +136,8 @@ class AlsaAudioSink final : public AudioSink { uint32_t buffer_ms_; /// `card:control` for hardware volume, empty for software volume. std::string mixer_spec_; + /// Curve mapping volume onto the mixer's dB range; kSpec unless configured otherwise. + MixerCurve mixer_curve_{MixerCurve::kSpec}; /// Serialises every snd_pcm_* call and the fields describing the open stream. std::mutex device_mutex_; @@ -170,12 +187,14 @@ class AlsaAudioSink final : public AudioSink { int64_t mixer_ramp_next_ms_{0}; }; -/// Maps a 0-100 volume onto a hardware mixer's dB range on the spec's curve, amplitude = -/// (v/100)^1.5, i.e. 30*log10(v/100) dB below max. Clamped to the control's floor. -long mixer_dB_for_volume(long min, long max, uint8_t volume); +/// Maps a 0-100 volume onto a hardware mixer's dB range on the given curve. +/// The spec curve is amplitude (v/100)^1.5, i.e. 30*log10(v/100) dB below max; the cubic curve +/// is twice that. Clamped to the control's floor. +long mixer_dB_for_volume(long min, long max, uint8_t volume, + MixerCurve curve = MixerCurve::kSpec); /// The inverse of mixer_dB_for_volume(): the 0-100 volume a mixer dB reading stands for. -uint8_t volume_for_mixer_dB(long min, long max, long dB); +uint8_t volume_for_mixer_dB(long min, long max, long dB, MixerCurve curve = MixerCurve::kSpec); /// Routes libasound's diagnostics to our logger; call before any snd_* that can fail. void install_alsa_error_handler(); diff --git a/src/audio_sink.cpp b/src/audio_sink.cpp index bbcb69d..8c47d5d 100644 --- a/src/audio_sink.cpp +++ b/src/audio_sink.cpp @@ -303,7 +303,9 @@ void warn_if_shadowing_an_alsa_pcm([[maybe_unused]] const std::string& spec, // buffer_ms is unused on a build with no device-backed backend. std::unique_ptr make_audio_sink(const std::string& device, [[maybe_unused]] uint32_t buffer_ms, - const std::string& alsa_mixer, std::string& error) { + const std::string& alsa_mixer, + [[maybe_unused]] const std::string& alsa_mixer_curve, + std::string& error) { DeviceSpec spec; if (!resolve_device_spec(device, spec, error)) { return nullptr; @@ -315,7 +317,7 @@ std::unique_ptr make_audio_sink(const std::string& device, return std::make_unique(NullSinkOutput::Discard); case SinkBackend::Stdout: return std::make_unique(NullSinkOutput::Stdout); - case SinkBackend::Alsa: + case SinkBackend::Alsa: { #ifdef SENDSPIN_CLI_HAVE_ALSA // Probed now so a typo fails at startup, not at the first stream. if (!AlsaAudioSink::probe(spec.device, error)) { @@ -324,10 +326,14 @@ std::unique_ptr make_audio_sink(const std::string& device, if (!alsa_mixer.empty() && !AlsaAudioSink::probe_mixer(alsa_mixer, error)) { return nullptr; } - return std::make_unique(spec.device, buffer_ms, alsa_mixer); + // The parser already rejected an unknown name, so this cannot fail here. + MixerCurve curve = MixerCurve::kSpec; + parse_mixer_curve(alsa_mixer_curve, curve); + return std::make_unique(spec.device, buffer_ms, alsa_mixer, curve); #else break; // unreachable: resolve_device_spec() never yields Alsa without the backend #endif + } case SinkBackend::CoreAudio: #ifdef SENDSPIN_CLI_HAVE_COREAUDIO // Probed now so a typo fails at startup, not at the first stream. diff --git a/src/audio_sink.h b/src/audio_sink.h index 15cc8c7..4c2f2e8 100644 --- a/src/audio_sink.h +++ b/src/audio_sink.h @@ -163,9 +163,11 @@ void print_sink_capabilities(std::FILE* out, const SinkCapabilities& caps, /// Builds the sink named by -o. /// @param buffer_ms Already range-checked by the parser. /// @param alsa_mixer `card:control` naming a hardware mixer; ignored off the ALSA backend. +/// @param alsa_mixer_curve How volume maps onto the mixer's dB range; ignored without a mixer. /// @return The sink, or nullptr with `error` set. std::unique_ptr make_audio_sink(const std::string& device, uint32_t buffer_ms, - const std::string& alsa_mixer, std::string& error); + const std::string& alsa_mixer, + const std::string& alsa_mixer_curve, std::string& error); /// Prints the device specs make_audio_sink() accepts, for -l. void print_audio_devices(std::FILE* out); diff --git a/src/cli.cpp b/src/cli.cpp index 7f164db..ea2a888 100644 --- a/src/cli.cpp +++ b/src/cli.cpp @@ -80,6 +80,7 @@ enum LongOnly { OPT_PAIRING_CODE, OPT_FIXED_VOLUME, OPT_ALSA_MIXER, + OPT_ALSA_MIXER_CURVE, OPT_RELEASE_DEVICE, }; @@ -122,6 +123,7 @@ const std::vector& settable_options() { {Opt::PairingCode, "pairing-code", "--pairing-code"}, {Opt::FixedVolume, "fixed-volume", "--fixed-volume"}, {Opt::AlsaMixer, "alsa-mixer", "--alsa-mixer"}, + {Opt::AlsaMixerCurve, "alsa-mixer-curve", "--alsa-mixer-curve"}, {Opt::ReleaseDevice, "release-device", "--release-device"}, }; return table; @@ -433,6 +435,13 @@ bool apply_option(const SettableOption& option, const std::string& value, Option } out.alsa_mixer = value; break; + case Opt::AlsaMixerCurve: + if (value != "spec" && value != "cubic") { + error = "invalid --alsa-mixer-curve '" + value + "' -- expected spec or cubic"; + return false; + } + out.alsa_mixer_curve = value; + break; case Opt::ReleaseDevice: if (!parse_bool(value, out.release_device)) { error = "invalid --release-device '" + value + "' -- expected true or false"; @@ -617,6 +626,7 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { {"pairing-code", required_argument, nullptr, OPT_PAIRING_CODE}, {"fixed-volume", no_argument, nullptr, OPT_FIXED_VOLUME}, {"alsa-mixer", required_argument, nullptr, OPT_ALSA_MIXER}, + {"alsa-mixer-curve", required_argument, nullptr, OPT_ALSA_MIXER_CURVE}, {"release-device", no_argument, nullptr, OPT_RELEASE_DEVICE}, {nullptr, 0, nullptr, 0}, }; @@ -766,6 +776,9 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { case OPT_ALSA_MIXER: apply(Opt::AlsaMixer, optarg); break; + case OPT_ALSA_MIXER_CURVE: + apply(Opt::AlsaMixerCurve, optarg); + break; case OPT_RELEASE_DEVICE: apply(Opt::ReleaseDevice, "true"); break; @@ -878,6 +891,12 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { } } + // The curve only describes a mixer; without one it would silently do nothing. + if (error.empty() && out.was_given(Opt::AlsaMixerCurve) && !out.was_given(Opt::AlsaMixer)) { + fail("--alsa-mixer-curve '" + out.alsa_mixer_curve + + "' needs --alsa-mixer -- the curve only describes a hardware mixer"); + } + // Absolutized before the length check, since the resolved path is what must fit. if (error.empty()) { // --no-control governs only a daemon; a subcommand still needs the path. @@ -924,6 +943,7 @@ bool parse_options(int argc, char* argv[], Options& out, std::FILE* err) { Opt::OutputDelay, Opt::HookStart, Opt::HookStop, Opt::ClientId, Opt::Manufacturer, Opt::ProductName, Opt::AudioFormat, Opt::AllowUnpaired, Opt::Input, Opt::PairingCode, Opt::FixedVolume, Opt::AlsaMixer, + Opt::AlsaMixerCurve, Opt::ReleaseDevice, }; for (Opt opt : DAEMON_ONLY) { @@ -1131,9 +1151,15 @@ void print_usage(std::FILE* out, const char* prog) { std::fprintf(out, " Drive this ALSA hardware mixer for volume and mute\n"); std::fprintf(out, " instead of scaling the stream in software, e.g.\n"); std::fprintf(out, " 0:Master. The value is mapped over the control's dB\n"); - std::fprintf(out, " range on the spec's curve. ALSA outputs only; a mixer\n"); - std::fprintf(out, " that cannot be opened or set falls back to software\n"); - std::fprintf(out, " gain\n"); + std::fprintf(out, " range on the curve --alsa-mixer-curve selects. ALSA\n"); + std::fprintf(out, " outputs only; a mixer that cannot be opened or set\n"); + std::fprintf(out, " falls back to software gain\n"); + std::fprintf(out, " --alsa-mixer-curve \n"); + std::fprintf(out, " How volume maps onto the mixer's dB range. 'spec'\n"); + std::fprintf(out, " (default) is the perceived-loudness curve; 'cubic'\n"); + std::fprintf(out, " is the steeper one whose inverse is alsamixer's\n"); + std::fprintf(out, " slider position, so the two track each other. Needs\n"); + std::fprintf(out, " --alsa-mixer\n"); std::fprintf(out, " --release-device\n"); std::fprintf(out, " Close the output device when a stream ends and reopen\n"); std::fprintf(out, " it at the next, so other players can share an exclusive\n"); diff --git a/src/cli.h b/src/cli.h index 9812500..1cd038d 100644 --- a/src/cli.h +++ b/src/cli.h @@ -85,6 +85,7 @@ enum class Opt : unsigned { PairingCode, ///< --pairing-code FixedVolume, ///< --fixed-volume AlsaMixer, ///< --alsa-mixer + AlsaMixerCurve, ///< --alsa-mixer-curve ReleaseDevice, ///< --release-device }; @@ -127,6 +128,10 @@ struct Options { /// ALSA only; empty scales the stream in software. std::string alsa_mixer; + /// --alsa-mixer-curve : how volume maps onto the mixer's dB range. Only used + /// with --alsa-mixer. Defaults to spec. + std::string alsa_mixer_curve{"spec"}; + /// --release-device: close the output device at stream end instead of holding it. bool release_device{false}; diff --git a/src/main.cpp b/src/main.cpp index 7cd90c0..041fc22 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -749,8 +749,8 @@ int main(int argc, char* argv[]) { } std::string sink_error; - std::unique_ptr sink = - make_audio_sink(opts.device, opts.buffer_ms, opts.alsa_mixer, sink_error); + std::unique_ptr sink = make_audio_sink(opts.device, opts.buffer_ms, opts.alsa_mixer, + opts.alsa_mixer_curve, sink_error); if (!sink) { log_fatal(LOG_TAG_AUDIO, "%s", sink_error.c_str()); return 1; @@ -958,7 +958,10 @@ int main(int argc, char* argv[]) { // hardware value is what a controller should show until the server sends its own. bool mixer_muted = false; if (!opts.alsa_mixer.empty()) { - const int mixer_volume = AlsaAudioSink::read_mixer_volume(opts.alsa_mixer, mixer_muted); + MixerCurve mixer_curve = MixerCurve::kSpec; + parse_mixer_curve(opts.alsa_mixer_curve, mixer_curve); + const int mixer_volume = + AlsaAudioSink::read_mixer_volume(opts.alsa_mixer, mixer_muted, mixer_curve); if (mixer_volume >= 0) { volume = static_cast(mixer_volume); muted = mixer_muted; diff --git a/tests/alsa_mixer_test.cpp b/tests/alsa_mixer_test.cpp index fedecce..e0605b4 100644 --- a/tests/alsa_mixer_test.cpp +++ b/tests/alsa_mixer_test.cpp @@ -62,6 +62,39 @@ TEST(MixerVolumeCurve, InverseClampsAtBothEnds) { EXPECT_EQ(volume_for_mixer_dB(MIN_DB, MAX_DB, MAX_DB + 100), 100); } +TEST(MixerVolumeCurve, CubicFollowsTheSixtyLogSlope) { + // The cubic (Snapcast) curve is twice the spec slope in dB: -18.06 dB at 50 on a 60 dB range. + EXPECT_EQ(mixer_dB_for_volume(MIN_DB, MAX_DB, 50, MixerCurve::kCubic), -1806); +} + +TEST(MixerVolumeCurve, CubicLandsWhereAlsamixerPutsTheSlider) { + // alsamixer's slider position is 100*10^(dB/60) (alsa-utils volume_mapping.c), which is + // exactly the cubic curve's inverse: volume 25 stands at -36.12 dB, and 10 at -60 dB. + EXPECT_EQ(mixer_dB_for_volume(MIN_DB, MAX_DB, 25, MixerCurve::kCubic), -3612); + EXPECT_EQ(mixer_dB_for_volume(MIN_DB, MAX_DB, 10, MixerCurve::kCubic), -6000); +} + +TEST(MixerVolumeCurve, EveryCurveRoundTripsThroughItsInverse) { + // 20 is the lowest sample: below it the cubic curve clamps to a 60 dB floor. + for (MixerCurve curve : {MixerCurve::kSpec, MixerCurve::kCubic}) { + for (uint8_t volume : {20, 25, 50, 75, 99, 100}) { + const long dB = mixer_dB_for_volume(MIN_DB, MAX_DB, volume, curve); + EXPECT_EQ(volume_for_mixer_dB(MIN_DB, MAX_DB, dB, curve), volume) + << "volume " << int{volume}; + } + } +} + +TEST(MixerVolumeCurve, ParsesTheCurveNames) { + MixerCurve curve = MixerCurve::kCubic; + EXPECT_TRUE(parse_mixer_curve("spec", curve)); + EXPECT_EQ(curve, MixerCurve::kSpec); + EXPECT_TRUE(parse_mixer_curve("cubic", curve)); + EXPECT_EQ(curve, MixerCurve::kCubic); + EXPECT_FALSE(parse_mixer_curve("bogus", curve)); + EXPECT_FALSE(parse_mixer_curve("linear", curve)); +} + } // namespace } // namespace sendspin_cli diff --git a/tests/cli_test.cpp b/tests/cli_test.cpp index 4bfff60..d62df31 100644 --- a/tests/cli_test.cpp +++ b/tests/cli_test.cpp @@ -486,6 +486,38 @@ TEST(ParseOptions, AlsaMixerRefusesAnEmptyValue) { << parse.diagnostics(); } +TEST(ParseOptions, AlsaMixerCurveDefaultsToSpec) { + Parse parse({"-o", "alsa:default", "--alsa-mixer", "0:Master"}); + + ASSERT_TRUE(parse.ok()) << parse.diagnostics(); + EXPECT_EQ(parse.options().alsa_mixer_curve, "spec"); + EXPECT_FALSE(parse.options().was_given(Opt::AlsaMixerCurve)); +} + +TEST(ParseOptions, AlsaMixerCurveIsAccepted) { + Parse parse({"-o", "alsa:default", "--alsa-mixer", "0:Master", "--alsa-mixer-curve", "cubic"}); + + ASSERT_TRUE(parse.ok()) << parse.diagnostics(); + EXPECT_EQ(parse.options().alsa_mixer_curve, "cubic"); + EXPECT_TRUE(parse.options().was_given(Opt::AlsaMixerCurve)); +} + +TEST(ParseOptions, AlsaMixerCurveRefusesAnUnknownName) { + Parse parse({"-o", "alsa:default", "--alsa-mixer", "0:Master", "--alsa-mixer-curve", "linear"}); + + EXPECT_FALSE(parse.ok()); + EXPECT_NE(parse.diagnostics().find("expected spec or cubic"), std::string::npos) + << parse.diagnostics(); +} + +TEST(ParseOptions, AlsaMixerCurveNeedsAMixer) { + Parse parse({"-o", "alsa:default", "--alsa-mixer-curve", "cubic"}); + + EXPECT_FALSE(parse.ok()); + EXPECT_NE(parse.diagnostics().find("needs --alsa-mixer"), std::string::npos) + << parse.diagnostics(); +} + TEST(ParseOptions, ReleaseDeviceIsOffUnlessAsked) { Parse off({}); Parse on({"--release-device"}); diff --git a/tests/config_file_test.cpp b/tests/config_file_test.cpp index 25167b2..ecc8871 100644 --- a/tests/config_file_test.cpp +++ b/tests/config_file_test.cpp @@ -332,6 +332,22 @@ TEST(ConfigPrecedence, AlsaMixerIsSettableAndLosesToTheCommandLine) { EXPECT_EQ(override.options().alsa_mixer, "0:Master"); } +TEST(ConfigPrecedence, AlsaMixerCurveIsSettableAndLosesToTheCommandLine) { + ScratchDir scratch; + ASSERT_TRUE(scratch.created()); + const std::string file = + scratch.write("mixer", "alsa-mixer = 1:Digital\nalsa-mixer-curve = cubic\n"); + + Parse from_file({}, file); + ASSERT_TRUE(from_file.ok()) << from_file.diagnostics(); + EXPECT_EQ(from_file.options().alsa_mixer_curve, "cubic"); + EXPECT_TRUE(from_file.options().was_given(Opt::AlsaMixerCurve)); + + Parse override({"--alsa-mixer-curve", "spec"}, file); + ASSERT_TRUE(override.ok()) << override.diagnostics(); + EXPECT_EQ(override.options().alsa_mixer_curve, "spec"); +} + TEST(ConfigPrecedence, ReleaseDeviceIsSettableAndOffByDefault) { ScratchDir scratch; ASSERT_TRUE(scratch.created()); From f0d6c17da7d2a93e64db47b71b5963552c9a2d1b Mon Sep 17 00:00:00 2001 From: Rasmus Date: Fri, 30 Oct 2026 12:23:34 +0100 Subject: [PATCH 5/6] Fix the hardware mixer path where a control refuses a write Four issues on the mixer path, each reachable with a real control. A refused mixer write left the software gain at unity, so a control that rejected writes (or a switch write it would not take) kept playing at the old level. Track whether the control actually carries the level and the mute in mixer_carries_volume_/mixer_carries_mute_, cleared by a refused write and restored by one that succeeds; set from write_mixer_dB_, so a failure partway through a ramp is caught too. A newer volume request that took the immediate-write branch left any ramp in flight running, and the next poll resumed writing toward the older target. The same happened when an outside change was accepted. Cancel the pending ramp in both places. On a control with no playback switch, an outside dB change read muted as false and assigned it to muted_, audibly unmuting a stream the user had muted. Only a control with a switch can report a mute change, so leave muted_ alone when there is none, and report the preserved state. A mute-at-minimum control leaves mixer_reported_dB_ at the mute sentinel, so the next ramp started near -100,000 dB: every intermediate write sat at the mute step and then jumped. Clamp the ramp's start to the repaired range, leaving the reported value itself alone since it recognizes echoes. Verified on the IQaudIO DAC: the ramp from the mute step walks 0 -> 50 -> 101 -> 151 -> 201 instead of pinning at the mute step and jumping, and an outside change on the switchless Analogue control no longer clears the mute. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- src/alsa_sink.cpp | 75 +++++++++++++++++++++++++++++++++++++---------- src/alsa_sink.h | 7 +++++ 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/alsa_sink.cpp b/src/alsa_sink.cpp index 4a230f7..5d1f420 100644 --- a/src/alsa_sink.cpp +++ b/src/alsa_sink.cpp @@ -739,8 +739,10 @@ void AlsaAudioSink::set_volume(uint8_t volume) { // The mixer carries the level when it has the control; the software gain only ever covers // what the mixer cannot, so it is recomputed from both fields rather than forced to unity. if (this->mixer_elem_ != nullptr) { - start_mixer_ramp_(mixer_dB_for_volume(this->mixer_dB_min_, this->mixer_dB_max_, - this->volume_.load(), this->mixer_curve_)); + // A refused write hands the level back to the software gain; the ramp reports success. + this->mixer_carries_volume_ = start_mixer_ramp_( + mixer_dB_for_volume(this->mixer_dB_min_, this->mixer_dB_max_, this->volume_.load(), + this->mixer_curve_)); } this->update_target_multiplier_(); cli_log(LogLevel::DEBUG, "alsa: volume now %u", this->volume_.load()); @@ -750,7 +752,7 @@ void AlsaAudioSink::set_muted(bool muted) { this->muted_.store(muted); // A control without a switch leaves the mute to the software gain below. if (this->mixer_elem_ != nullptr && this->mixer_has_switch_) { - set_mixer_mute_(muted); + this->mixer_carries_mute_ = set_mixer_mute_(muted); } this->update_target_multiplier_(); cli_log(LogLevel::DEBUG, "alsa: %s", muted ? "muted" : "unmuted"); @@ -808,10 +810,16 @@ uint64_t AlsaAudioSink::software_gain_() const { if (this->volume_.load() == 0) { return 0; } - // The mixer owns the level when it has a control, so the PCM stays at unity and only a mute - // without a hardware switch falls back to gain 0. Without a mixer, volume stays in software. - if (this->mixer_elem_ != nullptr) { - return (this->muted_.load() && !this->mixer_has_switch_) ? 0 : Q32_ONE; + // Mute is the switch's when the mixer took it; otherwise the PCM gain silences. This covers + // a control with no switch as well as one whose switch write was refused. + if (this->muted_.load() && !this->mixer_carries_mute_) { + return 0; + } + // The mixer owns the level when it took the write, so the PCM stays at unity. When it did + // not (no control, or a refused write), the software curve carries the level instead; a + // control that recovers sets the flag again on its next successful write. + if (this->mixer_carries_volume_) { + return Q32_ONE; } return q32_gain_for(this->volume_.load(), this->muted_.load()); } @@ -978,6 +986,11 @@ bool AlsaAudioSink::mixer_open_() { return false; } this->mixer_has_switch_ = snd_mixer_selem_has_playback_switch(this->mixer_elem_) != 0; + // Assume the control takes the level until a write says otherwise: starting at unity gain + // keeps the software path from scaling a level the mixer is already carrying. A write that + // fails clears the flag, so a control that refuses everything falls back on first use. + this->mixer_carries_volume_ = true; + this->mixer_carries_mute_ = this->mixer_has_switch_; this->sync_mixer_reported_(); return true; } @@ -989,6 +1002,8 @@ void AlsaAudioSink::mixer_close_() { this->mixer_ = nullptr; this->mixer_elem_ = nullptr; this->mixer_ramping_ = false; + this->mixer_carries_volume_ = false; + this->mixer_carries_mute_ = false; } void AlsaAudioSink::sync_mixer_reported_() { @@ -1001,11 +1016,16 @@ void AlsaAudioSink::sync_mixer_reported_() { bool AlsaAudioSink::write_mixer_dB_(long dB) { if (this->mixer_elem_ == nullptr) { + this->mixer_carries_volume_ = false; return false; } if (snd_mixer_selem_set_playback_dB_all(this->mixer_elem_, dB, 0) < 0) { + // A refused write hands the level to the software gain; recording it here covers a + // failure in the middle of a ramp, not just the immediate write. + this->mixer_carries_volume_ = false; return false; } + this->mixer_carries_volume_ = true; // Store the read-back, not the request: ALSA rounds to the control's step, and the rounded // value is what the next event reports. Recording the request would make that echo look like // a change made outside this process. @@ -1033,15 +1053,23 @@ bool AlsaAudioSink::start_mixer_ramp_(long target_dB) { if (this->mixer_elem_ == nullptr) { return false; } - // A move of one step or less is not worth a ramp; write it now. + // A move of one step or less is not worth a ramp; write it now. Any ramp still in flight is + // abandoned, or the next poll would resume writing toward its older target and overwrite + // this newer level. const long distance = std::abs(target_dB - this->mixer_reported_dB_); const long step = std::max(1, std::abs(this->mixer_dB_max_ - this->mixer_dB_min_) / MIXER_RAMP_STEPS); if (distance <= step) { + this->mixer_ramping_ = false; return this->write_mixer_dB_(target_dB); } this->mixer_ramping_ = true; - this->mixer_ramp_from_dB_ = this->mixer_reported_dB_; + // Start from where the control actually stands. On a mute-at-minimum control that is the + // mute sentinel, well below the repaired floor, so an unclamped start would leave every + // intermediate step pinned at the mute level and then jump to the target. The reported value + // itself is left alone, since it is what recognizes an echoed event. + this->mixer_ramp_from_dB_ = std::clamp(this->mixer_reported_dB_, this->mixer_dB_min_, + this->mixer_dB_max_); this->mixer_ramp_target_dB_ = target_dB; this->mixer_ramp_step_ = 0; this->mixer_ramp_next_ms_ = mixer_now_ms(); @@ -1084,18 +1112,33 @@ void AlsaAudioSink::report_mixer_change_() { } this->mixer_reported_muted_ = muted; - const uint8_t volume = (have_dB && !muted) - ? volume_for_mixer_dB(this->mixer_dB_min_, this->mixer_dB_max_, dB, - this->mixer_curve_) - : this->volume_.load(); + // The level this event reports is newer than any ramp in flight; without this the next poll + // would keep writing toward the ramp's older target and undo the change. + this->mixer_ramping_ = false; + + uint8_t volume = this->volume_.load(); + if (have_dB && !muted) { + volume = volume_for_mixer_dB(this->mixer_dB_min_, this->mixer_dB_max_, dB, + this->mixer_curve_); + // An outside change read back is proof the control holds the level; a control that had + // fallen back to software gain is used again. + this->mixer_carries_volume_ = true; + } this->volume_.store(volume); - this->muted_.store(muted); + // Only a control with a switch can report a mute change. Without one, leave the mute where + // it is: a control whose floor is not silent cannot express mute, so reading it here would + // silently unmute a stream the user muted. + if (this->mixer_has_switch_) { + this->muted_.store(muted); + this->mixer_carries_mute_ = true; + } + const bool reported_muted = this->muted_.load(); // A control whose floor is not silent reports volume 0 here; this is what makes it silent. this->update_target_multiplier_(); cli_log(LogLevel::INFO, "alsa: mixer '%s' changed to %u%s", this->mixer_spec_.c_str(), - static_cast(volume), muted ? " (muted)" : ""); + static_cast(volume), reported_muted ? " (muted)" : ""); if (this->on_local_volume_changed) { - this->on_local_volume_changed(volume, muted); + this->on_local_volume_changed(volume, reported_muted); } } diff --git a/src/alsa_sink.h b/src/alsa_sink.h index de025c7..6ec8a1b 100644 --- a/src/alsa_sink.h +++ b/src/alsa_sink.h @@ -175,6 +175,13 @@ class AlsaAudioSink final : public AudioSink { long mixer_dB_max_{0}; /// True when the control has a playback switch, so mute is hardware rather than software. bool mixer_has_switch_{false}; + /// True while the control is carrying the level. A refused volume write clears it, handing the + /// level to the software gain; a write that succeeds sets it again. Tracked apart from the + /// mute so a failed switch write does not also drop the level into software, scaling it twice. + bool mixer_carries_volume_{false}; + /// True while the switch is carrying the mute. Only meaningful with a switch; a control + /// without one, or one whose switch write was refused, leaves the mute to the software gain. + bool mixer_carries_mute_{false}; /// The switch state we last set or read; an event holding it is our own echo. bool mixer_reported_muted_{false}; /// dB the control was last set to by us or read back; an event holding it is our own echo. From 1f59b78608580f0a69da1adced93ff549b932651 Mon Sep 17 00:00:00 2001 From: Rasmus Date: Fri, 30 Oct 2026 12:35:03 +0100 Subject: [PATCH 6/6] Fix three more issues on the hardware mixer path report_mixer_change_() cancelled the volume ramp on any event, including a mute-only change. Muting mid-ramp stopped the mixer at an intermediate level, and unmuting then adopted that level as volume_ and reported it to the server, losing the level that was actually asked for. Cancel the ramp and adopt the level only for an outside volume change; a mute-only change leaves the ramp to finish at its target. A ramp step that failed cleared the carry flag but left the software multiplier at the unity a running ramp uses, so the level stayed wherever the control last accepted a write while software scaled nothing. Refresh the multiplier when a ramp is abandoned. open_mixer_() closed the handle on every failure path but left the caller's pointer dangling. The persistent mixer_ member then held freed memory and the destructor's mixer_close_() freed it again, so a typo in --alsa-mixer crashed the process on exit instead of falling back to software volume. Clear the pointer wherever the handle is closed. Verified on the IQaudIO DAC: an external mute mid-ramp leaves the ramp running and adopts the mute, a failed ramp step hands the level to the software gain (multiplier 3667107365, not unity), and an unopenable control exits cleanly instead of segfaulting in snd_mixer_close. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- src/alsa_sink.cpp | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/alsa_sink.cpp b/src/alsa_sink.cpp index 5d1f420..9c95e3a 100644 --- a/src/alsa_sink.cpp +++ b/src/alsa_sink.cpp @@ -836,6 +836,9 @@ snd_mixer_elem_t* AlsaAudioSink::open_mixer_(const std::string& mixer_spec, snd_ const std::string card = mixer_spec.substr(0, colon); const std::string control = mixer_spec.substr(colon + 1); + // Every failure below closes the handle. Clear the caller's pointer as well, or it keeps a + // dangling value that a later mixer_close_() would free a second time. + *handle = nullptr; if (snd_mixer_open(handle, 0) < 0) { return nullptr; } @@ -843,6 +846,7 @@ snd_mixer_elem_t* AlsaAudioSink::open_mixer_(const std::string& mixer_spec, snd_ if (snd_mixer_attach(*handle, hw.c_str()) < 0 || snd_mixer_selem_register(*handle, nullptr, nullptr) < 0 || snd_mixer_load(*handle) < 0) { snd_mixer_close(*handle); + *handle = nullptr; return nullptr; } @@ -853,6 +857,7 @@ snd_mixer_elem_t* AlsaAudioSink::open_mixer_(const std::string& mixer_spec, snd_ snd_mixer_elem_t* elem = snd_mixer_find_selem(*handle, sid); if (elem == nullptr) { snd_mixer_close(*handle); + *handle = nullptr; } return elem; } @@ -1107,28 +1112,25 @@ void AlsaAudioSink::report_mixer_change_() { if (!dB_changed && !mute_changed) { return; } - if (have_dB && !muted) { - this->mixer_reported_dB_ = dB; - } - this->mixer_reported_muted_ = muted; - - // The level this event reports is newer than any ramp in flight; without this the next poll - // would keep writing toward the ramp's older target and undo the change. - this->mixer_ramping_ = false; - + // An outside volume change is newer than any ramp in flight; without this the next poll + // would keep writing toward the ramp's older target and undo it. A mute-only change leaves + // the ramp alone, so muting mid-ramp still finishes at the level that was asked for. uint8_t volume = this->volume_.load(); - if (have_dB && !muted) { + if (dB_changed) { + this->mixer_reported_dB_ = dB; volume = volume_for_mixer_dB(this->mixer_dB_min_, this->mixer_dB_max_, dB, this->mixer_curve_); // An outside change read back is proof the control holds the level; a control that had // fallen back to software gain is used again. this->mixer_carries_volume_ = true; + this->mixer_ramping_ = false; } this->volume_.store(volume); // Only a control with a switch can report a mute change. Without one, leave the mute where // it is: a control whose floor is not silent cannot express mute, so reading it here would // silently unmute a stream the user muted. if (this->mixer_has_switch_) { + this->mixer_reported_muted_ = muted; this->muted_.store(muted); this->mixer_carries_mute_ = true; } @@ -1151,6 +1153,10 @@ void AlsaAudioSink::poll_mixer_() { cli_log(LogLevel::WARN, "alsa: mixer '%s' stopped taking the volume ramp", this->mixer_spec_.c_str()); this->mixer_ramping_ = false; + // The failed step cleared the carry flag, so the level is the software gain's now. + // Without this the multiplier stays at the unity a running ramp uses, and the level + // sits wherever the control last accepted a write. + this->update_target_multiplier_(); } }