From 522d126d1b17f09efbf6e63d54f1cda84e451e6e Mon Sep 17 00:00:00 2001
From: outfoxxed <outfoxxed@outfoxxed.me>
Date: Wed, 1 Oct 2025 00:29:45 -0700
Subject: [PATCH] services/pipewire: consider device volume step when sending
 updates

Previously a hardcoded 0.0001 offset was used to determine if a volume
change was significant enough to send to a device, however some
devices have a much more granular step size, which caused future
volume updates to be blocked.

This change replaces the hardcoded offset with the volumeStep device
route property which should be large enough for the device to work with.

Fixes #279
---
 changelog/next.md              |  1 +
 src/services/pipewire/node.cpp | 59 +++++++++++++++++++++-------------
 src/services/pipewire/node.hpp |  2 ++
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/changelog/next.md b/changelog/next.md
index d0e9895..fdd62d8 100644
--- a/changelog/next.md
+++ b/changelog/next.md
@@ -4,6 +4,7 @@
 
 ## Bug Fixes
 
+- Fixed volumes getting stuck on change for pipewire devices with few volume steps.
 - Fixed a crash when running out of disk space to write log files.
 - Fixed a rare crash when disconnecting a monitor.
 - Fixed build issues preventing cross compilation from working.
diff --git a/src/services/pipewire/node.cpp b/src/services/pipewire/node.cpp
index 3e68149..031a68f 100644
--- a/src/services/pipewire/node.cpp
+++ b/src/services/pipewire/node.cpp
@@ -304,6 +304,8 @@ void PwNodeBoundAudio::updateVolumeProps(const PwVolumeProps& volumeProps) {
 		return;
 	}
 
+	this->volumeStep = volumeProps.volumeStep;
+
 	// It is important that the lengths of channels and volumes stay in sync whenever you read them.
 	auto channelsChanged = false;
 	auto volumesChanged = false;
@@ -435,31 +437,35 @@ void PwNodeBoundAudio::setVolumes(const QVector<float>& volumes) {
 			                << "via device";
 			this->waitingVolumes = realVolumes;
 		} else {
-			auto significantChange = this->mServerVolumes.isEmpty();
-			for (auto i = 0; i < this->mServerVolumes.length(); i++) {
-				auto serverVolume = this->mServerVolumes.value(i);
-				auto targetVolume = realVolumes.value(i);
-				if (targetVolume == 0 || abs(targetVolume - serverVolume) >= 0.0001) {
-					significantChange = true;
-					break;
-				}
-			}
-
-			if (significantChange) {
-				qCInfo(logNode) << "Changing volumes of" << this->node << "to" << realVolumes
-				                << "via device";
-				if (!this->node->device->setVolumes(this->node->routeDevice, realVolumes)) {
-					return;
+			if (this->volumeStep != -1) {
+				auto significantChange = this->mServerVolumes.isEmpty();
+				for (auto i = 0; i < this->mServerVolumes.length(); i++) {
+					auto serverVolume = this->mServerVolumes.value(i);
+					auto targetVolume = realVolumes.value(i);
+					if (targetVolume == 0 || abs(targetVolume - serverVolume) >= this->volumeStep) {
+						significantChange = true;
+						break;
+					}
 				}
 
-				this->mDeviceVolumes = realVolumes;
-				this->node->device->waitForDevice();
-			} else {
-				// Insignificant changes won't cause an info event on the device, leaving qs hung in the
-				// "waiting for acknowledgement" state forever.
-				qCInfo(logNode) << "Ignoring volume change for" << this->node << "to" << realVolumes
-				                << "from" << this->mServerVolumes
-				                << "as it is a device node and the change is too small.";
+				if (significantChange) {
+					qCInfo(logNode) << "Changing volumes of" << this->node << "to" << realVolumes
+					                << "via device";
+					if (!this->node->device->setVolumes(this->node->routeDevice, realVolumes)) {
+						return;
+					}
+
+					this->mDeviceVolumes = realVolumes;
+					this->node->device->waitForDevice();
+				} else {
+					// Insignificant changes won't cause an info event on the device, leaving qs hung in the
+					// "waiting for acknowledgement" state forever.
+					qCInfo(logNode).nospace()
+					    << "Ignoring volume change for " << this->node << " to " << realVolumes << " from "
+					    << this->mServerVolumes
+					    << " as it is a device node and the change is too small (min step: "
+					    << this->volumeStep << ").";
+				}
 			}
 		}
 	} else {
@@ -519,6 +525,7 @@ PwVolumeProps PwVolumeProps::parseSpaPod(const spa_pod* param) {
 	const auto* volumesProp = spa_pod_find_prop(param, nullptr, SPA_PROP_channelVolumes);
 	const auto* channelsProp = spa_pod_find_prop(param, nullptr, SPA_PROP_channelMap);
 	const auto* muteProp = spa_pod_find_prop(param, nullptr, SPA_PROP_mute);
+	const auto* volumeStepProp = spa_pod_find_prop(param, nullptr, SPA_PROP_volumeStep);
 
 	const auto* volumes = reinterpret_cast<const spa_pod_array*>(&volumesProp->value);
 	const auto* channels = reinterpret_cast<const spa_pod_array*>(&channelsProp->value);
@@ -537,6 +544,12 @@ PwVolumeProps PwVolumeProps::parseSpaPod(const spa_pod* param) {
 
 	spa_pod_get_bool(&muteProp->value, &props.mute);
 
+	if (volumeStepProp) {
+		spa_pod_get_float(&volumeStepProp->value, &props.volumeStep);
+	} else {
+		props.volumeStep = -1;
+	}
+
 	return props;
 }
 
diff --git a/src/services/pipewire/node.hpp b/src/services/pipewire/node.hpp
index 0d4c92e..b53015f 100644
--- a/src/services/pipewire/node.hpp
+++ b/src/services/pipewire/node.hpp
@@ -158,6 +158,7 @@ struct PwVolumeProps {
 	QVector<PwAudioChannel::Enum> channels;
 	QVector<float> volumes;
 	bool mute = false;
+	float volumeStep = -1;
 
 	static PwVolumeProps parseSpaPod(const spa_pod* param);
 };
@@ -214,6 +215,7 @@ private:
 	QVector<float> mServerVolumes;
 	QVector<float> mDeviceVolumes;
 	QVector<float> waitingVolumes;
+	float volumeStep = -1;
 	PwNode* node;
 };