diff --git a/android/app/src/main/kotlin/com/musly/musly/MusicService.kt b/android/app/src/main/kotlin/com/musly/musly/MusicService.kt index 517e387..7bbc424 100644 --- a/android/app/src/main/kotlin/com/musly/musly/MusicService.kt +++ b/android/app/src/main/kotlin/com/musly/musly/MusicService.kt @@ -697,7 +697,13 @@ class MusicService : MediaBrowserServiceCompat() { } override fun onAdjustVolume(direction: Int) { - val newVolume = (currentVolume + direction * 5).coerceIn(0, 100) + // Use getCurrentVolume() (the live VolumeProviderCompat + // property) not the captured constructor param. The param + // is a val fixed at connection time, so every relative + // adjustment would always compute from the same baseline, + // causing the volume to snap back to initialVolume±5 no + // matter how many times the user presses the button. + val newVolume = (getCurrentVolume() + direction * 5).coerceIn(0, 100) setCurrentVolume(newVolume) AndroidAutoPlugin.sendCommand("setVolume", mapOf("volume" to newVolume)) } diff --git a/lib/providers/player_provider.dart b/lib/providers/player_provider.dart index c9dd970..2cb7524 100644 --- a/lib/providers/player_provider.dart +++ b/lib/providers/player_provider.dart @@ -56,6 +56,11 @@ class PlayerProvider extends ChangeNotifier { Song? _currentSong; double _volume = 1.0; + /// True only while audio is actually being rendered on a remote device. + /// Distinct from isConnected: if the user plays a radio station while a + /// UPnP renderer is connected, the audio is still local, so this stays false. + bool _isRenderingRemotely = false; + String? _resolvedArtworkUrl; RadioStation? _currentRadioStation; @@ -149,19 +154,28 @@ class PlayerProvider extends ChangeNotifier { _windowsService.onSkipPrevious = skipPrevious; _windowsService.onSeekTo = seek; + // Audio focus and noisy callbacks must be no-ops in remote-playback mode. + // The audio is playing on the renderer device, not on this phone, so + // Android reassigning audio focus at screen-off (or a noisy event) must + // not pause the renderer. _androidSystemService.onAudioFocusLoss = () { + if (isRemotePlayback) return; pause(); }; _androidSystemService.onAudioFocusLossTransient = () { + if (isRemotePlayback) return; pause(); }; _androidSystemService.onAudioFocusLossTransientCanDuck = () { + if (isRemotePlayback) return; _audioPlayer.setVolume(0.3); }; _androidSystemService.onAudioFocusGain = () { + if (isRemotePlayback) return; _audioPlayer.setVolume(_volume); }; _androidSystemService.onBecomingNoisy = () { + if (isRemotePlayback) return; pause(); }; @@ -671,6 +685,11 @@ class PlayerProvider extends ChangeNotifier { int get currentIndex => _currentIndex; bool get isPlaying => _isPlaying; bool get isLoading => _isLoading; + /// True when audio is playing on a remote renderer (UPnP or Cast) rather + /// than locally. Used to suppress audio-focus and noisy-event handling that + /// would incorrectly pause the remote device, and to route UI volume changes + /// to the renderer instead of the Android system volume. + bool get isRemotePlayback => _isRenderingRemotely; bool get shuffleEnabled => _shuffleEnabled; RepeatMode get repeatMode => _repeatMode; Duration get position => _position; @@ -683,7 +702,19 @@ class PlayerProvider extends ChangeNotifier { RadioStation? get currentRadioStation => _currentRadioStation; bool get isPlayingRadio => _isPlayingRadio; - Stream get positionStream => _audioPlayer.positionStream; + // Unified position stream: fed by the local audio player in normal mode, or + // by UPnP/Cast polling in remote-playback mode. The UI subscribes to this + // instead of directly to _audioPlayer.positionStream so that the progress + // bar animates correctly regardless of which playback path is active. + final _positionController = StreamController.broadcast(); + Stream get positionStream => _positionController.stream; + + // Subscriptions stored so they can be cancelled before dispose closes the + // StreamController, preventing a late just_audio tick from calling add() on + // a closed controller. + StreamSubscription? _playerStateSub; + StreamSubscription? _positionSub; + StreamSubscription? _durationSub; double get progress { if (_duration.inMilliseconds == 0) return 0; @@ -777,8 +808,12 @@ class PlayerProvider extends ChangeNotifier { notifyListeners(); }); - _audioPlayer.playerStateStream.listen( + _playerStateSub = _audioPlayer.playerStateStream.listen( (state) { + // In remote-playback mode the local player is stopped/paused; ignore + // its state so it doesn't overwrite the UPnP/Cast-managed values. + if (_isRenderingRemotely) return; + final wasPlaying = _isPlaying; _isPlaying = state.playing; @@ -808,14 +843,18 @@ class PlayerProvider extends ChangeNotifier { Duration? lastNotified; Duration? lastSystemUpdate; - _audioPlayer.positionStream.listen( + _positionSub = _audioPlayer.positionStream.listen( (position) { - + // In remote-playback mode the local player sits idle at position zero; + // ignore its ticks so they don't overwrite the UPnP/Cast position. + if (_isRenderingRemotely) return; + final positionJumpedBack = _position.inMilliseconds > 0 && position.inMilliseconds < _position.inMilliseconds - 1000; _position = position; + _positionController.add(position); if (positionJumpedBack || lastNotified == null || @@ -836,11 +875,15 @@ class PlayerProvider extends ChangeNotifier { }, ); - _audioPlayer.durationStream.listen( + _durationSub = _audioPlayer.durationStream.listen( (duration) { + // In remote-playback mode the local player has no loaded track; ignore + // its duration so it doesn't zero out the UPnP/Cast duration. + if (_isRenderingRemotely) return; + _duration = duration ?? Duration.zero; notifyListeners(); - _updateAndroidAuto(); + _updateAndroidAuto(); }, onError: (error) { debugPrint('Duration stream error (can be ignored): $error'); @@ -963,9 +1006,12 @@ class PlayerProvider extends ChangeNotifier { : null, autoPlay: true, ); + _isRenderingRemotely = true; _isPlaying = true; } else if (_upnpService.isConnected) { - + // Reset before sending Stop so a poll that fires mid-load can't + // mistake the STOPPED state for a natural track end and advance twice. + _upnpWasPlaying = false; debugPrint( 'UPnP: playSong() taking UPnP branch, isConnected=${_upnpService.isConnected}', ); @@ -976,6 +1022,10 @@ class PlayerProvider extends ChangeNotifier { : _subsonicService.getStreamUrl(song.id); try { + // Resolve the MIME type so strict UPnP renderers (e.g. moode / + // upmpdcli with "check metadata" on) can validate protocolInfo. + final mimeType = song.contentType ?? + UpnpService.mimeTypeFromSuffix(song.suffix); final success = await _upnpService.loadAndPlay( url: playUrl, title: song.title, @@ -985,6 +1035,7 @@ class PlayerProvider extends ChangeNotifier { ? _subsonicService.getCoverArtUrl(song.coverArt, size: 0) : null, durationSecs: song.duration, + contentType: mimeType, ); if (!success) { _upnpService.disconnect(); @@ -997,9 +1048,10 @@ class PlayerProvider extends ChangeNotifier { debugPrint('UPnP playback failed, disconnected: $e'); rethrow; } + _isRenderingRemotely = true; _isPlaying = true; } else { - + _isRenderingRemotely = false; final String playUrl; if (song.isLocal == true && song.path != null) { playUrl = Uri.file(song.path!).toString(); @@ -1077,6 +1129,7 @@ class PlayerProvider extends ChangeNotifier { _queue = []; _currentIndex = -1; _isPlayingRadio = true; + _isRenderingRemotely = false; // radio always plays locally _currentRadioStation = station; _position = Duration.zero; _duration = Duration.zero; @@ -1180,11 +1233,12 @@ class PlayerProvider extends ChangeNotifier { if (_castService.isConnected) { await _castService.stop(); } else if (_upnpService.isConnected) { + _upnpWasPlaying = false; // prevent poll from misreading the STOPPED state await _upnpService.stop(); } else { await _audioPlayer.stop(); } - + _isPlaying = false; _position = Duration.zero; notifyListeners(); @@ -1506,6 +1560,10 @@ class PlayerProvider extends ChangeNotifier { _samsungService.dispose(); _discordRpcService.shutdown(); + _playerStateSub?.cancel(); + _positionSub?.cancel(); + _durationSub?.cancel(); + _positionController.close(); super.dispose(); } @@ -1584,7 +1642,7 @@ class PlayerProvider extends ChangeNotifier { playSong(song); } } else { - + _isRenderingRemotely = false; _androidSystemService.setRemotePlayback(isRemote: false); _isPlaying = false; notifyListeners(); @@ -1621,6 +1679,7 @@ class PlayerProvider extends ChangeNotifier { if (!connected && _upnpWasConnected) { _upnpWasConnected = false; _upnpWasPlaying = false; + _isRenderingRemotely = false; _isPlaying = false; _position = Duration.zero; _duration = Duration.zero; @@ -1637,10 +1696,12 @@ class PlayerProvider extends ChangeNotifier { final playing = _upnpService.isRendererPlaying; final rendererState = _upnpService.rendererState; - if (_upnpWasPlaying && - rendererState == 'STOPPED' && - dur > Duration.zero && - pos.inSeconds >= dur.inSeconds - 1) { + if (_upnpWasPlaying && rendererState == 'STOPPED') { + // _upnpWasPlaying is reset to false in playSong() and stop() before + // any Stop command is sent, so this only fires for a *natural* track + // end. We don't check duration > 0 here because many renderers + // (including moode/upmpdcli) return 0:00:00 from GetPositionInfo once + // the transport is stopped, which would cause the check to silently fail. debugPrint('UPnP: Track ended (pos=${pos.inSeconds}s, dur=${dur.inSeconds}s) — advancing'); _upnpWasPlaying = false; _onSongComplete(); @@ -1670,11 +1731,17 @@ class PlayerProvider extends ChangeNotifier { if ((_volume - normalized).abs() > 0.005) { _volume = normalized; changed = true; + // Only push the new volume to the Android MediaSession when it has + // actually changed. Calling updateRemoteVolume unconditionally on + // every state tick would overwrite any in-flight physical volume + // adjustment with the stale cached value before the next poll cycle + // had a chance to read it back, causing the audible snap-back. + _androidSystemService.updateRemoteVolume(vol); } - _androidSystemService.updateRemoteVolume(vol); } if (changed) { + _positionController.add(_position); notifyListeners(); _updateAndroidAuto(); } diff --git a/lib/screens/now_playing_screen.dart b/lib/screens/now_playing_screen.dart index af7f0fd..647e973 100644 --- a/lib/screens/now_playing_screen.dart +++ b/lib/screens/now_playing_screen.dart @@ -2598,10 +2598,11 @@ class _VolumeSliderState extends State<_VolumeSlider> { double _systemVolume = 0.5; StreamSubscription? _volumeSubscription; - @override - void didChangeDependencies() { - super.didChangeDependencies(); - } + // Serialise remote volume commits: store the latest desired value and run + // one async write loop at a time so out-of-order responses can't snap the + // renderer back to a stale level during a fast drag. + double? _pendingRemoteVolume; + bool _remoteCommitInProgress = false; @override void initState() { @@ -2630,149 +2631,183 @@ class _VolumeSliderState extends State<_VolumeSlider> { super.dispose(); } - void _updateVolumeFromPosition(Offset localPosition, double width) { - final newVolume = (localPosition.dx / width).clamp(0.0, 1.0); + void _applyVolume(double newVolume, PlayerProvider provider, bool isRemote) { setState(() { _dragValue = newVolume; - _systemVolume = newVolume; + if (!isRemote) _systemVolume = newVolume; }); - VolumeController.instance.setVolume(newVolume); + if (isRemote) { + // Queue the value and let a single async loop drain the queue so rapid + // drag ticks never result in out-of-order writes reaching the renderer. + _pendingRemoteVolume = newVolume; + _drainRemoteVolume(provider); + } else { + VolumeController.instance.setVolume(newVolume); + } + } + + Future _drainRemoteVolume(PlayerProvider provider) async { + if (_remoteCommitInProgress) return; + _remoteCommitInProgress = true; + try { + while (_pendingRemoteVolume != null) { + final value = _pendingRemoteVolume!; + _pendingRemoteVolume = null; + await provider.setVolume(value); + } + } finally { + _remoteCommitInProgress = false; + } + } + + void _updateVolumeFromPosition( + Offset localPosition, + double width, + PlayerProvider provider, + bool isRemote, + ) { + final newVolume = (localPosition.dx / width).clamp(0.0, 1.0); + _applyVolume(newVolume, provider, isRemote); } @override Widget build(BuildContext context) { - final displayVolume = _isDragging ? _dragValue : _systemVolume; + // Consumer so the slider re-reads provider.volume on each UPnP poll tick. + return Consumer( + builder: (context, provider, _) { + final isRemote = provider.isRemotePlayback; + final currentVolume = isRemote ? provider.volume : _systemVolume; + final displayVolume = _isDragging ? _dragValue : currentVolume; - return Row( - children: [ - GestureDetector( - onTap: () { - setState(() => _systemVolume = 0.0); - VolumeController.instance.setVolume(0.0); - }, - child: Icon( - displayVolume <= 0.01 - ? CupertinoIcons.speaker_slash_fill - : CupertinoIcons.speaker_1_fill, - color: Colors.white.withValues(alpha: 0.7), - size: 20, - ), - ), - const SizedBox(width: 12), - Expanded( - child: LayoutBuilder( - builder: (context, constraints) { - final trackWidth = constraints.maxWidth; - - return GestureDetector( - behavior: HitTestBehavior.opaque, - onHorizontalDragStart: (details) { - setState(() { - _isDragging = true; - _dragValue = _systemVolume; - }); - _updateVolumeFromPosition(details.localPosition, trackWidth); - }, - onHorizontalDragUpdate: (details) { - _updateVolumeFromPosition(details.localPosition, trackWidth); - }, - onHorizontalDragEnd: (details) { - setState(() => _isDragging = false); - }, - onTapDown: (details) { - _updateVolumeFromPosition(details.localPosition, trackWidth); - }, - child: SizedBox( - height: 40, - child: Center( - child: Stack( - alignment: Alignment.centerLeft, - children: [ - AnimatedContainer( - duration: const Duration(milliseconds: 120), - curve: Curves.easeOut, - height: _isDragging ? 6 : 4, - decoration: BoxDecoration( - color: Colors.white.withValues(alpha: 0.2), - borderRadius: BorderRadius.circular( - _isDragging ? 3 : 2, + return Row( + children: [ + GestureDetector( + onTap: () => _applyVolume(0.0, provider, isRemote), + child: Icon( + displayVolume <= 0.01 + ? CupertinoIcons.speaker_slash_fill + : CupertinoIcons.speaker_1_fill, + color: Colors.white.withValues(alpha: 0.7), + size: 20, + ), + ), + const SizedBox(width: 12), + Expanded( + child: LayoutBuilder( + builder: (context, constraints) { + final trackWidth = constraints.maxWidth; + + return GestureDetector( + behavior: HitTestBehavior.opaque, + onHorizontalDragStart: (details) { + setState(() { + _isDragging = true; + _dragValue = currentVolume; + }); + _updateVolumeFromPosition( + details.localPosition, trackWidth, provider, isRemote); + }, + onHorizontalDragUpdate: (details) { + _updateVolumeFromPosition( + details.localPosition, trackWidth, provider, isRemote); + }, + onHorizontalDragEnd: (details) { + setState(() => _isDragging = false); + }, + onTapDown: (details) { + _updateVolumeFromPosition( + details.localPosition, trackWidth, provider, isRemote); + }, + child: SizedBox( + height: 40, + child: Center( + child: Stack( + alignment: Alignment.centerLeft, + children: [ + AnimatedContainer( + duration: const Duration(milliseconds: 120), + curve: Curves.easeOut, + height: _isDragging ? 6 : 4, + decoration: BoxDecoration( + color: Colors.white.withValues(alpha: 0.2), + borderRadius: BorderRadius.circular( + _isDragging ? 3 : 2, + ), + ), ), - ), - ), - FractionallySizedBox( - widthFactor: displayVolume.clamp(0.0, 1.0), - child: AnimatedContainer( - duration: const Duration(milliseconds: 120), - curve: Curves.easeOut, - height: _isDragging ? 6 : 4, - decoration: BoxDecoration( - color: Colors.white.withValues(alpha: 0.9), - borderRadius: BorderRadius.circular( - _isDragging ? 3 : 2, + FractionallySizedBox( + widthFactor: displayVolume.clamp(0.0, 1.0), + child: AnimatedContainer( + duration: const Duration(milliseconds: 120), + curve: Curves.easeOut, + height: _isDragging ? 6 : 4, + decoration: BoxDecoration( + color: Colors.white.withValues(alpha: 0.9), + borderRadius: BorderRadius.circular( + _isDragging ? 3 : 2, + ), + ), ), ), - ), - ), - Positioned( - left: - ((trackWidth * displayVolume.clamp(0.0, 1.0)) - - (_isDragging ? 10 : 6)) - .clamp( - 0.0, - trackWidth - (_isDragging ? 20 : 12), - ), - child: AnimatedContainer( - duration: const Duration(milliseconds: 120), - curve: Curves.easeOut, - width: _isDragging ? 20 : 12, - height: _isDragging ? 20 : 12, - decoration: BoxDecoration( - color: Colors.white, - shape: BoxShape.circle, - boxShadow: _isDragging - ? [ - BoxShadow( - color: Colors.black.withValues( - alpha: 0.4, - ), - blurRadius: 8, - spreadRadius: 1, + Positioned( + left: + ((trackWidth * displayVolume.clamp(0.0, 1.0)) - + (_isDragging ? 10 : 6)) + .clamp( + 0.0, + trackWidth - (_isDragging ? 20 : 12), ), - ] - : [ - BoxShadow( - color: Colors.black.withValues( - alpha: 0.2, - ), - blurRadius: 3, - ), - ], + child: AnimatedContainer( + duration: const Duration(milliseconds: 120), + curve: Curves.easeOut, + width: _isDragging ? 20 : 12, + height: _isDragging ? 20 : 12, + decoration: BoxDecoration( + color: Colors.white, + shape: BoxShape.circle, + boxShadow: _isDragging + ? [ + BoxShadow( + color: Colors.black.withValues( + alpha: 0.4, + ), + blurRadius: 8, + spreadRadius: 1, + ), + ] + : [ + BoxShadow( + color: Colors.black.withValues( + alpha: 0.2, + ), + blurRadius: 3, + ), + ], + ), + ), ), - ), + ], ), - ], + ), ), - ), - ), - ); - }, - ), - ), - const SizedBox(width: 12), - GestureDetector( - onTap: () { - setState(() => _systemVolume = 1.0); - VolumeController.instance.setVolume(1.0); - }, - child: Icon( - CupertinoIcons.speaker_3_fill, - color: Colors.white.withValues(alpha: 0.7), - size: 20, - ), - ), - ], + ); + }, + ), + ), + const SizedBox(width: 12), + GestureDetector( + onTap: () => _applyVolume(1.0, provider, isRemote), + child: Icon( + CupertinoIcons.speaker_3_fill, + color: Colors.white.withValues(alpha: 0.7), + size: 20, + ), + ), + ], + ); + }, ); } } diff --git a/lib/services/upnp_service.dart b/lib/services/upnp_service.dart index 7981fc7..757f6b2 100644 --- a/lib/services/upnp_service.dart +++ b/lib/services/upnp_service.dart @@ -58,7 +58,8 @@ class UpnpService extends ChangeNotifier { Duration get rendererDuration => _rendererDuration; String get rendererState => _rendererState; bool get isRendererPlaying => _rendererState == 'PLAYING'; - int get volume => _volume; + int get volume => _volume; + int get consecutivePollErrors => _consecutivePollErrors; List get devices => List.unmodifiable(_devices); UpnpDevice? get connectedDevice => _connectedDevice; @@ -71,8 +72,15 @@ class UpnpService extends ChangeNotifier { final _dio = Dio( BaseOptions( - connectTimeout: const Duration(seconds: 5), - receiveTimeout: const Duration(seconds: 5), + connectTimeout: const Duration(seconds: 3), + receiveTimeout: const Duration(seconds: 3), + // Force a fresh TCP connection for every SOAP request. Many UPnP + // renderer HTTP servers (upmpdcli, BubbleUPnP Server, etc.) close + // their end of the TCP socket after a short keepalive timeout (~5–10 s). + // Without this header Dio's connection pool reuses the dead socket, the + // next write throws a SocketException, getPlaybackState() swallows it + // silently, and the progress bar freezes for the rest of the song. + headers: {'Connection': 'close'}, ), ); @@ -276,9 +284,10 @@ class UpnpService extends ChangeNotifier { bool _isPolling = false; int _pollCount = 0; + int _consecutivePollErrors = 0; Future _poll() async { - if (_isPolling) return; + if (_isPolling) return; final device = _connectedDevice; if (device == null) return; _isPolling = true; @@ -286,7 +295,24 @@ class UpnpService extends ChangeNotifier { try { final state = await getPlaybackState(); - if (state == null) return; + if (state == null) { + // getPlaybackState() caught an exception internally and returned null. + // Count consecutive failures so callers can detect a dead renderer. + _consecutivePollErrors++; + notifyListeners(); + if (_consecutivePollErrors == 1 || _consecutivePollErrors % 5 == 0) { + debugPrint( + 'UPnP: poll failed $_consecutivePollErrors time(s) in a row ' + '— renderer may be unreachable', + ); + } + return; + } + + if (_consecutivePollErrors != 0) { + _consecutivePollErrors = 0; + notifyListeners(); + } bool changed = false; if (state.transportState != _rendererState) { @@ -314,6 +340,8 @@ class UpnpService extends ChangeNotifier { notifyListeners(); } } catch (e) { + _consecutivePollErrors++; + notifyListeners(); debugPrint('UPnP: poll error: $e'); } finally { _isPolling = false; @@ -360,6 +388,7 @@ class UpnpService extends ChangeNotifier { String? album, String? albumArtUrl, int? durationSecs, + String? contentType, }) async { final device = _connectedDevice; if (device == null) { @@ -385,6 +414,7 @@ class UpnpService extends ChangeNotifier { album: album, albumArtUrl: albumArtUrl, durationSecs: durationSecs, + contentType: contentType, ); debugPrint('UPnP: SetAVTransportURI…'); await _soap( @@ -676,6 +706,35 @@ class UpnpService extends ChangeNotifier { } } + /// Returns a MIME type string for [suffix] (e.g. 'mp3' → 'audio/mpeg'). + /// Returns null when the suffix is unknown so callers can fall back to '*'. + static String? mimeTypeFromSuffix(String? suffix) { + switch (suffix?.toLowerCase()) { + case 'mp3': + return 'audio/mpeg'; + case 'flac': + return 'audio/flac'; + case 'ogg': + case 'oga': + return 'audio/ogg'; + case 'opus': + return 'audio/opus'; + case 'aac': + return 'audio/aac'; + case 'm4a': + return 'audio/mp4'; + case 'wav': + return 'audio/wav'; + case 'wma': + return 'audio/x-ms-wma'; + case 'aiff': + case 'aif': + return 'audio/aiff'; + default: + return null; + } + } + static String _didl({ required String title, required String artist, @@ -683,6 +742,7 @@ class UpnpService extends ChangeNotifier { String? album, String? albumArtUrl, int? durationSecs, + String? contentType, }) { String esc(String s) => s .replaceAll('&', '&') @@ -690,7 +750,10 @@ class UpnpService extends ChangeNotifier { .replaceAll('>', '>') .replaceAll('"', '"'); - const protocol = 'http-get:*:*:*'; + // Use the provided MIME type; fall back to wildcard so strict renderers + // (e.g. moode / upmpdcli with "check metadata" enabled) can validate. + final mimeType = contentType ?? '*'; + final protocol = 'http-get:*:$mimeType:*'; final durationAttr = durationSecs != null ? ' duration="${_formatTimeSecs(durationSecs)}"'