From c86d40f8ddf2aaabffd634b7fcfa11d79e3b09a7 Mon Sep 17 00:00:00 2001 From: Ryan Ernest Date: Sat, 11 Jul 2026 06:46:58 -0400 Subject: [PATCH] fix(player): preserve volume when toggling mute (#1532) * fix(player): preserve volume when toggling mute Muting now keeps the last volume level memorized. Unmuting restores that level, including amplified values allowed by the configured maximum. * fix(player): preserve volume for companion remote mute Route the companion remote through the shared mute transition so it no longer overwrites the persisted volume or restores to 100%. Also correct the Dartdoc formatting for the transition record fields. --- .../video_player/parts/companion_remote.dart | 6 +- lib/services/keyboard_shortcuts_service.dart | 6 +- lib/services/settings_service.dart | 15 ++++ .../widgets/volume_control.dart | 6 +- .../keyboard_shortcuts_service_test.dart | 40 ++++++++- test/services/settings_service_test.dart | 44 ++++++++++ test/widgets/volume_control_test.dart | 85 +++++++++++++++++++ 7 files changed, 192 insertions(+), 10 deletions(-) create mode 100644 test/widgets/volume_control_test.dart diff --git a/lib/screens/video_player/parts/companion_remote.dart b/lib/screens/video_player/parts/companion_remote.dart index d9637ebb..762b7e7f 100644 --- a/lib/screens/video_player/parts/companion_remote.dart +++ b/lib/screens/video_player/parts/companion_remote.dart @@ -39,9 +39,9 @@ extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState { receiver.onVolumeMute = () async { if (player == null) return; final settings = await SettingsService.getInstance(); - final newVolume = player!.state.volume > 0 ? 0.0 : 100.0; - unawaited(player!.setVolume(newVolume)); - unawaited(settings.write(SettingsService.volume, newVolume)); + final transition = settings.resolveMuteToggle(player!.state.volume); + unawaited(player!.setVolume(transition.playerVolume)); + unawaited(settings.write(SettingsService.volume, transition.persistedVolume)); }; receiver.onSubtitles = _cycleSubtitleTrack; receiver.onAudioTracks = _cycleAudioTrack; diff --git a/lib/services/keyboard_shortcuts_service.dart b/lib/services/keyboard_shortcuts_service.dart index f1a4c409..2959935f 100644 --- a/lib/services/keyboard_shortcuts_service.dart +++ b/lib/services/keyboard_shortcuts_service.dart @@ -344,9 +344,9 @@ class KeyboardShortcutsService extends ChangeNotifier { onToggleFullscreen?.call(); break; case 'mute_toggle': - final newVolume = player.state.volume > 0 ? 0.0 : 100.0; - player.setVolume(newVolume); - _settingsService.write(SettingsService.volume, newVolume); + final transition = _settingsService.resolveMuteToggle(player.state.volume); + player.setVolume(transition.playerVolume); + _settingsService.write(SettingsService.volume, transition.persistedVolume); break; case 'subtitle_toggle': onToggleSubtitles?.call(); diff --git a/lib/services/settings_service.dart b/lib/services/settings_service.dart index 926100f0..1ab54e54 100644 --- a/lib/services/settings_service.dart +++ b/lib/services/settings_service.dart @@ -614,6 +614,21 @@ class SettingsService extends BaseSharedPreferencesService { _cachedInstance = null; } + /// Resolves a video mute toggle without replacing the saved volume with 0. + /// + /// `persistedVolume` is the non-zero value callers should keep in [volume], + /// while `playerVolume` is the value to apply to the active player. + ({double playerVolume, double persistedVolume}) resolveMuteToggle(double currentVolume) { + if (currentVolume.isFinite && currentVolume > 0) { + return (playerVolume: 0, persistedVolume: currentVolume); + } + + final previousVolume = read(volume); + final candidate = previousVolume.isFinite && previousVolume > 0 ? previousVolume : volume.defaultValue; + final restoredVolume = candidate.clamp(0.0, read(maxVolume).toDouble()).toDouble(); + return (playerVolume: restoredVolume, persistedVolume: restoredVolume); + } + static Map defaultKeyboardShortcuts() => _defaultKeyboardShortcuts(); static Map defaultKeyboardHotkeys() => _defaultKeyboardHotkeys(); diff --git a/lib/widgets/video_controls/widgets/volume_control.dart b/lib/widgets/video_controls/widgets/volume_control.dart index 20bda087..6ad34232 100644 --- a/lib/widgets/video_controls/widgets/volume_control.dart +++ b/lib/widgets/video_controls/widgets/volume_control.dart @@ -148,9 +148,9 @@ class _VolumeControlState extends State { color: Colors.white, ), onPressed: () async { - final newVolume = isMuted ? 100.0 : 0.0; - await widget.player.setVolume(newVolume); - await _settings.write(SettingsService.volume, newVolume); + final transition = _settings.resolveMuteToggle(widget.player.state.volume); + await widget.player.setVolume(transition.playerVolume); + await _settings.write(SettingsService.volume, transition.persistedVolume); }, ), ); diff --git a/test/services/keyboard_shortcuts_service_test.dart b/test/services/keyboard_shortcuts_service_test.dart index e1edf33e..51b6e90f 100644 --- a/test/services/keyboard_shortcuts_service_test.dart +++ b/test/services/keyboard_shortcuts_service_test.dart @@ -279,6 +279,34 @@ void main() { expect(commandCommaResult, KeyEventResult.ignored); }); + testWidgets('mute shortcut matches the button restoration behavior', (tester) async { + final service = await KeyboardShortcutsService.getInstance(); + addTearDown(service.dispose); + final settings = SettingsService.instance; + await settings.write(SettingsService.volume, 37.0); + final player = _FakePlayer(volume: 37); + const muteKey = KeyDownEvent( + physicalKey: PhysicalKeyboardKey.keyM, + logicalKey: LogicalKeyboardKey.keyM, + timeStamp: Duration.zero, + ); + + final muteResult = service.handleVideoPlayerKeyEvent(muteKey, player, null, null, null, null, null, null); + await tester.pumpAndSettle(); + + expect(muteResult, KeyEventResult.handled); + expect(player.volume, 0); + expect(settings.read(SettingsService.volume), 37); + + final unmuteResult = service.handleVideoPlayerKeyEvent(muteKey, player, null, null, null, null, null, null); + await tester.pumpAndSettle(); + + expect(unmuteResult, KeyEventResult.handled); + expect(player.volume, 37); + expect(settings.read(SettingsService.volume), 37); + expect(player.volumeChanges, [0, 37]); + }); + test('video zoom scale maps to mpv logarithmic property', () { expect(VideoFilterManager.videoZoomPropertyForScale(1.0), closeTo(0.0, 0.0001)); expect(VideoFilterManager.videoZoomPropertyForScale(2.0), closeTo(1.0, 0.0001)); @@ -287,7 +315,11 @@ void main() { } class _FakePlayer implements Player { + _FakePlayer({this.volume = 100}); + final commands = >[]; + final volumeChanges = []; + double volume; @override Future command(List args) async { @@ -295,7 +327,13 @@ class _FakePlayer implements Player { } @override - PlayerState get state => PlayerState(); + PlayerState get state => PlayerState(volume: volume); + + @override + Future setVolume(double volume) async { + this.volume = volume; + volumeChanges.add(volume); + } @override dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); diff --git a/test/services/settings_service_test.dart b/test/services/settings_service_test.dart index 50268839..063a0fec 100644 --- a/test/services/settings_service_test.dart +++ b/test/services/settings_service_test.dart @@ -79,6 +79,50 @@ void main() { }); }); + group('SettingsService mute volume restoration', () { + test('keeps 37 persisted across mute and restores it on unmute', () async { + final settings = await SettingsService.getInstance(); + await settings.write(SettingsService.volume, 37.0); + + final mute = settings.resolveMuteToggle(37); + await settings.write(SettingsService.volume, mute.persistedVolume); + + expect(mute.playerVolume, 0); + expect(settings.read(SettingsService.volume), 37); + + final unmute = settings.resolveMuteToggle(mute.playerVolume); + + expect(unmute.playerVolume, 37); + expect(unmute.persistedVolume, 37); + }); + + test('restores amplified volumes when the configured maximum permits them', () async { + final settings = await SettingsService.getInstance(); + await settings.write(SettingsService.maxVolume, 250); + await settings.write(SettingsService.volume, 175.0); + + final mute = settings.resolveMuteToggle(175); + await settings.write(SettingsService.volume, mute.persistedVolume); + final unmute = settings.resolveMuteToggle(mute.playerVolume); + + expect(mute.playerVolume, 0); + expect(mute.persistedVolume, 175); + expect(unmute.playerVolume, 175); + expect(unmute.persistedVolume, 175); + }); + + test('falls back to 100 when no previous non-zero volume exists', () async { + final settings = await SettingsService.getInstance(); + await settings.write(SettingsService.maxVolume, 200); + await settings.write(SettingsService.volume, 0.0); + + final unmute = settings.resolveMuteToggle(0); + + expect(unmute.playerVolume, 100); + expect(unmute.persistedVolume, 100); + }); + }); + group('SettingsService TV card defaults', () { test('full card layout starts disabled', () async { final settings = await SettingsService.getInstance(); diff --git a/test/widgets/volume_control_test.dart b/test/widgets/volume_control_test.dart new file mode 100644 index 00000000..0c9f241c --- /dev/null +++ b/test/widgets/volume_control_test.dart @@ -0,0 +1,85 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/i18n/strings.g.dart'; +import 'package:plezy/mpv/mpv.dart'; +import 'package:plezy/services/settings_service.dart'; +import 'package:plezy/widgets/video_controls/widgets/volume_control.dart'; + +import '../test_helpers/prefs.dart'; + +void main() { + setUp(() async { + resetSharedPreferencesForTest(); + SettingsService.resetForTesting(); + await SettingsService.getInstance(); + LocaleSettings.setLocaleSync(AppLocale.en); + }); + + testWidgets('mute button keeps and restores the exact non-zero volume', (tester) async { + final settings = SettingsService.instance; + await settings.write(SettingsService.volume, 37.0); + final player = _VolumePlayer(37); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold(body: VolumeControl(player: player)), + ), + ); + + await tester.tap(find.byType(IconButton)); + await tester.pumpAndSettle(); + + expect(player.volume, 0); + expect(settings.read(SettingsService.volume), 37); + + await tester.tap(find.byType(IconButton)); + await tester.pumpAndSettle(); + + expect(player.volume, 37); + expect(settings.read(SettingsService.volume), 37); + expect(player.volumeChanges, [0, 37]); + }); +} + +class _VolumePlayer implements Player { + _VolumePlayer(this.volume) + : _streams = PlayerStreams( + playing: const Stream.empty(), + completed: const Stream.empty(), + buffering: const Stream.empty(), + position: const Stream.empty(), + duration: const Stream.empty(), + seekable: const Stream.empty(), + buffer: const Stream.empty(), + volume: const Stream.empty(), + rate: const Stream.empty(), + tracks: const Stream.empty(), + track: const Stream.empty(), + log: const Stream.empty(), + error: const Stream.empty(), + audioDevice: const Stream.empty(), + audioDevices: const Stream>.empty(), + bufferRanges: const Stream>.empty(), + playbackRestart: const Stream.empty(), + backendSwitched: const Stream.empty(), + ); + + double volume; + final List volumeChanges = []; + final PlayerStreams _streams; + + @override + PlayerState get state => PlayerState(volume: volume); + + @override + PlayerStreams get streams => _streams; + + @override + Future setVolume(double volume) async { + this.volume = volume; + volumeChanges.add(volume); + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +}