output/MultipleOutputs: SetVolume() throws on error

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)

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;
 }

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;
+
+	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;
+			}
+		}
+	}
 
-	return success;
+	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

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

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

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