diff --git a/NEWS b/NEWS index 1144fa406..c9f55947e 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ ver 0.23.8 (not yet released) - curl: fix crash if web server does not understand WebDAV * output - pipewire: fix crash with PipeWire 0.3.53 +* mixer + - better error messages * support libfmt 9 ver 0.23.7 (2022/05/09) diff --git a/src/command/OtherCommands.cxx b/src/command/OtherCommands.cxx index 9cac216f0..43a53f7c8 100644 --- a/src/command/OtherCommands.cxx +++ b/src/command/OtherCommands.cxx @@ -333,15 +333,11 @@ handle_getvol(Client &client, Request, Response &r) } CommandResult -handle_setvol(Client &client, Request args, Response &r) +handle_setvol(Client &client, Request args, Response &) { unsigned level = args.ParseUnsigned(0, 100); - if (!volume_level_change(client.GetPartition().outputs, level)) { - r.Error(ACK_ERROR_SYSTEM, "problems setting volume"); - return CommandResult::ERROR; - } - + volume_level_change(client.GetPartition().outputs, level); return CommandResult::OK; } @@ -364,11 +360,8 @@ handle_volume(Client &client, Request args, Response &r) else if (new_volume > 100) new_volume = 100; - if (new_volume != old_volume && - !volume_level_change(outputs, new_volume)) { - r.Error(ACK_ERROR_SYSTEM, "problems setting volume"); - return CommandResult::ERROR; - } + if (new_volume != old_volume) + volume_level_change(outputs, new_volume); return CommandResult::OK; } diff --git a/src/mixer/MixerAll.cxx b/src/mixer/MixerAll.cxx index 2d9c9080e..96830e7cc 100644 --- a/src/mixer/MixerAll.cxx +++ b/src/mixer/MixerAll.cxx @@ -73,42 +73,74 @@ MultipleOutputs::GetVolume() const noexcept return total / ok; } -static bool -output_mixer_set_volume(AudioOutputControl &ao, unsigned volume) noexcept +enum class SetVolumeResult { + NO_MIXER, + DISABLED, + ERROR, + OK, +}; + +static SetVolumeResult +output_mixer_set_volume(AudioOutputControl &ao, unsigned volume) { assert(volume <= 100); auto *mixer = ao.GetMixer(); if (mixer == nullptr) - return false; + return SetVolumeResult::NO_MIXER; /* software mixers are always updated, even if they are disabled */ if (!ao.IsReallyEnabled() && !mixer->IsPlugin(software_mixer_plugin)) - return false; + return SetVolumeResult::DISABLED; try { mixer_set_volume(mixer, volume); - return true; + return SetVolumeResult::OK; } catch (...) { FmtError(mixer_domain, "Failed to set mixer for '{}': {}", ao.GetName(), std::current_exception()); - return false; + std::throw_with_nested(std::runtime_error(fmt::format("Failed to set mixer for '{}'", + ao.GetName()))); } } -bool -MultipleOutputs::SetVolume(unsigned volume) noexcept +void +MultipleOutputs::SetVolume(unsigned volume) { assert(volume <= 100); - bool success = false; - for (const auto &ao : outputs) - success = output_mixer_set_volume(*ao, volume) - || success; + SetVolumeResult result = SetVolumeResult::NO_MIXER; + std::exception_ptr error; - return success; + for (const auto &ao : outputs) { + try { + auto r = output_mixer_set_volume(*ao, volume); + if (r > result) + result = r; + } catch (...) { + /* remember the first error */ + if (!error) { + error = std::current_exception(); + result = SetVolumeResult::ERROR; + } + } + } + + switch (result) { + case SetVolumeResult::NO_MIXER: + throw std::runtime_error{"No mixer"}; + + case SetVolumeResult::DISABLED: + throw std::runtime_error{"All outputs are disabled"}; + + case SetVolumeResult::ERROR: + std::rethrow_exception(error); + + case SetVolumeResult::OK: + break; + } } static int diff --git a/src/mixer/Volume.cxx b/src/mixer/Volume.cxx index 5f37e48fc..4afb97e5d 100644 --- a/src/mixer/Volume.cxx +++ b/src/mixer/Volume.cxx @@ -71,16 +71,16 @@ software_volume_change(MultipleOutputs &outputs, unsigned volume) return true; } -static bool +static void hardware_volume_change(MultipleOutputs &outputs, unsigned volume) { /* reset the cache */ last_hardware_volume = -1; - return outputs.SetVolume(volume); + outputs.SetVolume(volume); } -bool +void volume_level_change(MultipleOutputs &outputs, unsigned volume) { assert(volume <= 100); @@ -89,7 +89,7 @@ volume_level_change(MultipleOutputs &outputs, unsigned volume) idle_add(IDLE_MIXER); - return hardware_volume_change(outputs, volume); + hardware_volume_change(outputs, volume); } bool diff --git a/src/mixer/Volume.hxx b/src/mixer/Volume.hxx index ae38ce6f0..b245fe7fe 100644 --- a/src/mixer/Volume.hxx +++ b/src/mixer/Volume.hxx @@ -30,7 +30,10 @@ InvalidateHardwareVolume() noexcept; int volume_level_get(const MultipleOutputs &outputs) noexcept; -bool +/** + * Throws on error. + */ +void volume_level_change(MultipleOutputs &outputs, unsigned volume); bool diff --git a/src/output/MultipleOutputs.hxx b/src/output/MultipleOutputs.hxx index 761111ae3..d32c2c7e9 100644 --- a/src/output/MultipleOutputs.hxx +++ b/src/output/MultipleOutputs.hxx @@ -141,10 +141,11 @@ public: /** * Sets the volume on all available mixers. * + * Throws on error. + * * @param volume the volume (range 0..100) - * @return true on success, false on failure */ - bool SetVolume(unsigned volume) noexcept; + void SetVolume(unsigned volume); /** * Similar to GetVolume(), but gets the volume only for