FIX(client): Fix using the keyboad to change local volume adjustment

Previously, when using keyboard arrows, the new slider widget action
applied the new local volume but did not save it. That was because it
used the sliderReleased event to save the value, which is not triggered
by keyboard updates.

We could simply save the local volume on every sliderChange instead of
on sliderRelease, but that would cause many writes to the disk
in a short period of time and possibly lag the client.

This commit introduces an event filter in addition to the sliderChanged event,
which is observing key releases. If the slider has focus and left/right is released,
it is then calling the save slot of the slider.

Fixes mumble-voip#6211

src/mumble/CMakeLists.txt

@@ -291,6 +291,8 @@ set(MUMBLE_SOURCES
 
 	"widgets/CompletablePage.cpp"
 	"widgets/CompletablePage.h"
+	"widgets/EventFilters.cpp"
+	"widgets/EventFilters.h"
 	"widgets/MUComboBox.cpp"
 	"widgets/MUComboBox.h"
 	"widgets/MultiStyleWidgetWrapper.cpp"

src/mumble/ListenerVolumeSlider.cpp

@@ -10,7 +10,14 @@
 #include "VolumeAdjustment.h"
 #include "Global.h"
 
-ListenerVolumeSlider::ListenerVolumeSlider(QWidget *parent) : VolumeSliderWidgetAction(parent) {
+#include <QDebug>
+
+ListenerVolumeSlider::ListenerVolumeSlider(QWidget *parent) : VolumeSliderWidgetAction(parent), m_burst(0) {
+	connect(&m_messageTimer, &QTimer::timeout, this, &ListenerVolumeSlider::saveChangesServer);
+	connect(&m_burstTimer, &QTimer::timeout, this, &ListenerVolumeSlider::resetBurst);
+
+	m_messageTimer.setSingleShot(true);
+	m_burstTimer.setSingleShot(true);
 }
 
 void ListenerVolumeSlider::setListenedChannel(const Channel &channel) {
@@ -28,29 +35,68 @@ void ListenerVolumeSlider::on_VolumeSlider_valueChanged(int value) {
 	displayTooltip(value);
 }
 
-void ListenerVolumeSlider::on_VolumeSlider_sliderReleased() {
+void ListenerVolumeSlider::on_VolumeSlider_changeCompleted() {
 	ServerHandlerPtr handler = Global::get().sh;
 
-	if (!handler || !m_channel || !m_volumeSlider) {
+	if (!handler) {
 		return;
 	}
 
-	VolumeAdjustment adjustment = VolumeAdjustment::fromDBAdjustment(m_volumeSlider->value());
-
 	if (handler->m_version >= Mumble::Protocol::PROTOBUF_INTRODUCTION_VERSION) {
 		// With the new audio protocol, volume adjustments for listeners are handled on the server and thus we want
-		// to avoid spamming updates to the adjustments, which is why we only update them once the slider is released.
-		MumbleProto::UserState mpus;
-		mpus.set_session(Global::get().uiSession);
+		// to avoid spamming updates to the adjustments, which is why we only update them once the timer is completed.
+
+		m_burst++;
+		m_burstTimer.start(1000);
 
-		MumbleProto::UserState::VolumeAdjustment *adjustmentMsg = mpus.add_listening_volume_adjustment();
-		adjustmentMsg->set_listening_channel(m_channel->iId);
-		adjustmentMsg->set_volume_adjustment(adjustment.factor);
+		// We allow short burts of a few messages
+		if (m_burst <= MESSAGE_BURST_LIMIT) {
+			m_messageTimer.start(50);
+		} else {
+			m_messageTimer.start(500);
+		}
 
-		handler->sendMessage(mpus);
 	} else {
 		// Before the new audio protocol, volume adjustments for listeners are handled locally
-		Global::get().channelListenerManager->setListenerVolumeAdjustment(Global::get().uiSession, m_channel->iId,
-																		  adjustment);
+		saveChangesLocal();
 	}
 }
+
+void ListenerVolumeSlider::saveChangesLocal() {
+	ServerHandlerPtr handler = Global::get().sh;
+
+	if (!handler || !m_channel || !m_volumeSlider) {
+		return;
+	}
+
+	VolumeAdjustment adjustment = VolumeAdjustment::fromDBAdjustment(m_volumeSlider->value());
+	Global::get().channelListenerManager->setListenerVolumeAdjustment(Global::get().uiSession, m_channel->iId,
+																	  adjustment);
+	qDebug() << "local: " << adjustment.factor;
+}
+
+void ListenerVolumeSlider::saveChangesServer() {
+	ServerHandlerPtr handler = Global::get().sh;
+
+	if (!handler || !m_channel || !m_volumeSlider) {
+		return;
+	}
+
+	VolumeAdjustment adjustment = VolumeAdjustment::fromDBAdjustment(m_volumeSlider->value());
+
+	// With the new audio protocol, volume adjustments for listeners are handled on the server and thus we want
+	// to avoid spamming updates to the adjustments, which is why we only update them once the slider is released.
+	MumbleProto::UserState mpus;
+	mpus.set_session(Global::get().uiSession);
+
+	MumbleProto::UserState::VolumeAdjustment *adjustmentMsg = mpus.add_listening_volume_adjustment();
+	adjustmentMsg->set_listening_channel(m_channel->iId);
+	adjustmentMsg->set_volume_adjustment(adjustment.factor);
+	qDebug() << "sent: " << adjustment.factor;
+
+	handler->sendMessage(mpus);
+}
+
+void ListenerVolumeSlider::resetBurst() {
+	m_burst = 0;
+}

src/mumble/ListenerVolumeSlider.h

@@ -8,12 +8,16 @@
 
 #include "VolumeSliderWidgetAction.h"
 
+#include <QTimer>
+
 class Channel;
 
 class ListenerVolumeSlider : public VolumeSliderWidgetAction {
 	Q_OBJECT
 
 public:
+	static constexpr int MESSAGE_BURST_LIMIT = 3;
+
 	ListenerVolumeSlider(QWidget *parent = nullptr);
 
 	/// Must be called before adding this object as an action
@@ -23,9 +27,17 @@ class ListenerVolumeSlider : public VolumeSliderWidgetAction {
 	/// The channel of the listener proxy this dialog is operating on
 	const Channel *m_channel;
 
+	void saveChangesLocal();
+	void saveChangesServer();
+	void resetBurst();
+
+	QTimer m_messageTimer;
+	QTimer m_burstTimer;
+	unsigned int m_burst;
+
 private slots:
 	void on_VolumeSlider_valueChanged(int value) override;
-	void on_VolumeSlider_sliderReleased() override;
+	void on_VolumeSlider_changeCompleted() override;
 };
 
 #endif

src/mumble/Messages.cpp

@@ -39,6 +39,7 @@
 #include "Global.h"
 
 #include <QTextDocumentFragment>
+#include <QDebug>
 
 #define ACTOR_INIT                           \
 	ClientUser *pSrc = nullptr;              \
@@ -506,6 +507,7 @@ void MainWindow::msgUserState(const MumbleProto::UserState &msg) {
 
 		const Channel *channel = Channel::get(channelID);
 		if (channel && pSelf && pSelf->uiSession == pDst->uiSession) {
+			qDebug() << "Received: chan: " << channel->iId << " " << adjustment;
 			Global::get().channelListenerManager->setListenerVolumeAdjustment(pDst->uiSession, channel->iId,
 																			  VolumeAdjustment::fromFactor(adjustment));
 		} else if (!channel) {

src/mumble/UserLocalVolumeSlider.cpp

@@ -35,7 +35,7 @@ void UserLocalVolumeSlider::on_VolumeSlider_valueChanged(int value) {
 	}
 }
 
-void UserLocalVolumeSlider::on_VolumeSlider_sliderReleased() {
+void UserLocalVolumeSlider::on_VolumeSlider_changeCompleted() {
 	ClientUser *user = ClientUser::get(m_clientSession);
 	if (user) {
 		if (!user->qsHash.isEmpty()) {

src/mumble/UserLocalVolumeSlider.h

@@ -24,7 +24,7 @@ class UserLocalVolumeSlider : public VolumeSliderWidgetAction {
 
 private slots:
 	void on_VolumeSlider_valueChanged(int value);
-	void on_VolumeSlider_sliderReleased();
+	void on_VolumeSlider_changeCompleted();
 };
 
 #endif

src/mumble/VolumeSliderWidgetAction.cpp

@@ -5,6 +5,7 @@
 
 #include "VolumeSliderWidgetAction.h"
 #include "VolumeAdjustment.h"
+#include "widgets/EventFilters.h"
 
 #include <QSlider>
 #include <QToolTip>
@@ -16,10 +17,16 @@ VolumeSliderWidgetAction::VolumeSliderWidgetAction(QObject *parent)
 	m_volumeSlider->setAccessibleName(tr("Slider for volume adjustment"));
 	m_volumeSlider->setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::MinimumExpanding);
 
+	KeyEventObserver *eventFilter = new KeyEventObserver(this, QEvent::KeyRelease, false,
+														 { Qt::Key_Left, Qt::Key_Right, Qt::Key_Up, Qt::Key_Down });
+	m_volumeSlider->installEventFilter(eventFilter);
+
 	connect(m_volumeSlider.get(), &QSlider::valueChanged, this,
 			&VolumeSliderWidgetAction::on_VolumeSlider_valueChanged);
 	connect(m_volumeSlider.get(), &QSlider::sliderReleased, this,
-			&VolumeSliderWidgetAction::on_VolumeSlider_sliderReleased);
+			&VolumeSliderWidgetAction::on_VolumeSlider_changeCompleted);
+	connect(eventFilter, &KeyEventObserver::keyEventObserved, this,
+			&VolumeSliderWidgetAction::on_VolumeSlider_changeCompleted);
 
 	setDefaultWidget(m_volumeSlider.get());
 

src/mumble/VolumeSliderWidgetAction.h

@@ -27,7 +27,7 @@ class VolumeSliderWidgetAction : public QWidgetAction {
 
 protected slots:
 	virtual void on_VolumeSlider_valueChanged(int){};
-	virtual void on_VolumeSlider_sliderReleased(){};
+	virtual void on_VolumeSlider_changeCompleted(){};
 };
 
 #endif

src/mumble/widgets/EventFilters.cpp

@@ -0,0 +1,40 @@
+// Copyright 2023 The Mumble Developers. All rights reserved.
+// Use of this source code is governed by a BSD-style license
+// that can be found in the LICENSE file at the root of the
+// Mumble source tree or at <https://www.mumble.info/LICENSE>.
+
+#include "EventFilters.h"
+
+#include <algorithm>
+
+#include <QKeyEvent>
+#include <QWidget>
+
+KeyEventObserver::KeyEventObserver(QObject *parent, QEvent::Type eventType, bool consume,
+								   std::initializer_list< Qt::Key > keys)
+	: QObject(parent), m_eventType(eventType), m_consume(consume), m_keys(keys) {
+}
+
+bool KeyEventObserver::eventFilter(QObject *obj, QEvent *event) {
+	QWidget *widget = static_cast< QWidget * >(obj);
+
+	if (!widget || !widget->hasFocus()) {
+		return false;
+	}
+
+	QKeyEvent *keyEvent = static_cast< QKeyEvent * >(event);
+
+	if (!keyEvent || keyEvent->type() != m_eventType) {
+		return false;
+	}
+
+	Qt::Key key = static_cast< Qt::Key >(keyEvent->key());
+
+	if (std::find(m_keys.begin(), m_keys.end(), key) == m_keys.end()) {
+		return false;
+	}
+
+	emit keyEventObserved();
+
+	return m_consume;
+}

src/mumble/widgets/EventFilters.h

@@ -0,0 +1,33 @@
+// Copyright 2023 The Mumble Developers. All rights reserved.
+// Use of this source code is governed by a BSD-style license
+// that can be found in the LICENSE file at the root of the
+// Mumble source tree or at <https://www.mumble.info/LICENSE>.
+
+#ifndef MUMBLE_MUMBLE_WIDGETS_EVENTFILTERS_H_
+#define MUMBLE_MUMBLE_WIDGETS_EVENTFILTERS_H_
+
+#include <initializer_list>
+#include <vector>
+
+#include <QEvent>
+#include <QObject>
+
+class KeyEventObserver : public QObject {
+	Q_OBJECT
+
+private:
+	QEvent::Type m_eventType;
+	bool m_consume;
+	std::vector< Qt::Key > m_keys;
+
+public:
+	KeyEventObserver(QObject *parent, QEvent::Type eventType, bool consume, std::initializer_list< Qt::Key > keys);
+
+protected:
+	bool eventFilter(QObject *obj, QEvent *event) override;
+
+signals:
+	void keyEventObserved();
+};
+
+#endif