From f426b2d1f0cc256d34267b511665c0e8de6d16c8 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 18 Oct 2026 03:11:20 +0700 Subject: [PATCH] fix(player): report Android output failures without skipping tracks --- lib/services/music_playback_deck.dart | 40 ++++- lib/services/music_player_service.dart | 118 ++++++++++----- lib/widgets/playback_feedback.dart | 18 +++ test/music_player_automix_test.dart | 137 ++++++++++++++++++ test/playback_feedback_test.dart | 51 +++++++ .../audioplayers/player/MediaPlayerWrapper.kt | 12 +- .../luan/audioplayers/player/WrappedPlayer.kt | 30 ++-- 7 files changed, 348 insertions(+), 58 deletions(-) diff --git a/lib/services/music_playback_deck.dart b/lib/services/music_playback_deck.dart index f6a4581a..a2044ded 100644 --- a/lib/services/music_playback_deck.dart +++ b/lib/services/music_playback_deck.dart @@ -85,12 +85,24 @@ class MusicPlaybackDeck { _ordinary.onPlayerStateChanged.listen((value) { if (!_direct) _states.add(value); }), - _ordinary.onDurationChanged.listen((value) { - if (!_direct) _durations.add(value); - }), - _ordinary.onPlayerComplete.listen((_) { - if (!_direct) _completions.add(null); - }), + _ordinary.eventStream.listen( + (event) { + if (_direct || _disposed) return; + switch (event.eventType) { + case AudioEventType.duration: + _durations.add(event.duration!); + case AudioEventType.complete: + _completions.add(null); + default: + break; + } + }, + onError: (Object error, StackTrace stackTrace) { + if (_direct || _disposed) return; + _ordinaryError = AsyncError(error, stackTrace); + _states.addError(error, stackTrace); + }, + ), ]); } @@ -151,6 +163,7 @@ class MusicPlaybackDeck { final _positions = StreamController.broadcast(); final _completions = StreamController.broadcast(); final _subscriptions = >[]; + AsyncError? _ordinaryError; StreamSubscription? _nativeSubscription; StreamSubscription? _positionSubscription; Timer? _timer; @@ -306,6 +319,7 @@ class MusicPlaybackDeck { // /proc/self/fd number for a different SAF file, so URL equality must not // reuse the previous MediaPlayer or its pending preparation callbacks. await _ordinary.release(); + _ordinaryError = null; await _ordinary.setSource(source); await _syncEqualizer(); } @@ -341,7 +355,19 @@ class MusicPlaybackDeck { } Future resume() async { - if (!_direct) return _ordinary.resume(); + if (!_direct) { + void checkError() { + final failure = _ordinaryError; + if (failure != null) { + Error.throwWithStackTrace(failure.error, failure.stackTrace); + } + } + + checkError(); + await _ordinary.resume(); + checkError(); + return; + } if (_volumeBlocked) { usbAudioStatus.value = const UsbAudioStatus(reason: 'volume_unavailable'); throw UsbOutputUnavailable( diff --git a/lib/services/music_player_service.dart b/lib/services/music_player_service.dart index 18d3f060..839d0559 100644 --- a/lib/services/music_player_service.dart +++ b/lib/services/music_player_service.dart @@ -3,7 +3,6 @@ import 'dart:io'; import 'dart:math'; import 'package:audio_service/audio_service.dart'; -import 'package:flutter/services.dart'; import 'package:flutter/foundation.dart' show compute; import 'package:flutter/widgets.dart' show AppLifecycleState, WidgetsBinding, WidgetsBindingObserver; @@ -346,6 +345,7 @@ class MusicPlayerHandler extends BaseAudioHandler Future? _disposeFuture; bool _initialized = false; bool _sourceReady = false; + Object? _playbackError; Future? _activePlayOperation; Future _sourceChangeTail = Future.value(); Timer? _sleepTimer; @@ -637,27 +637,36 @@ class MusicPlayerHandler extends BaseAudioHandler getPosition: player.getCurrentPosition, ); _playerSubscriptions[player] = [ - player.onPlayerStateChanged.listen((state) { - if (!identical(player, _player)) return; - if (state == PlayerState.paused && player.needsSourceReload) { - _userPaused = true; - _pausedByInterruption = false; - _playbackAutomation.cancelPendingActions(); - } - if (_switchingGeneration != 0 && - (state == PlayerState.stopped || - state == PlayerState.completed || - state == PlayerState.disposed)) { - return; - } - if (state == PlayerState.completed && _shouldIgnoreComplete) { - if (_userPaused || _interruptionActive) { - _broadcastState(playerState: PlayerState.paused); + player.onPlayerStateChanged.listen( + (state) { + if (!identical(player, _player)) return; + if (state == PlayerState.paused && player.needsSourceReload) { + _userPaused = true; + _pausedByInterruption = false; + _playbackAutomation.cancelPendingActions(); } - return; - } - _broadcastState(playerState: state); - }), + if (_switchingGeneration != 0 && + (state == PlayerState.stopped || + state == PlayerState.completed || + state == PlayerState.disposed)) { + return; + } + if (state == PlayerState.completed && _shouldIgnoreComplete) { + if (_userPaused || _interruptionActive) { + _broadcastState(playerState: PlayerState.paused); + } + return; + } + _broadcastState(playerState: state); + }, + onError: (Object error, StackTrace stackTrace) { + // Preparation errors are handled by _loadIndex, including the SAF + // fallback. Runtime failures must not look like successful completion. + if (_sourceReady || _switchingGeneration == 0) { + _reportPlaybackError(player, error, stackTrace); + } + }, + ), player.onPositionChanged.listen((position) { if (identical(player, _player)) _handlePositionChanged(position); }), @@ -674,6 +683,34 @@ class MusicPlayerHandler extends BaseAudioHandler ]; } + void _reportPlaybackError( + MusicPlaybackDeck player, + Object error, + StackTrace stackTrace, + ) { + if (_disposed || !identical(player, _player) || _playbackError != null) { + return; + } + _sourceReady = false; + _playbackError = error; + _userPaused = true; + _pausedByInterruption = false; + _playbackAutomation.cancelPendingActions(); + _log.e('Playback failed for ${mediaItem.value?.title}', error, stackTrace); + _broadcastState(playerState: PlayerState.stopped); + final generation = _playRequestGeneration; + unawaited( + _serializeSourceChange(() async { + if (_disposed || generation != _playRequestGeneration) return; + await _autoMix.cancel(); + if (_disposed || generation != _playRequestGeneration) return; + await player.stop(); + }).catchError((Object error) { + _log.w('Could not stop failed playback: $error'); + }), + ); + } + Future _disposeDeck(MusicPlaybackDeck player) async { final subscriptions = _playerSubscriptions.remove(player); if (subscriptions == null) return; @@ -811,7 +848,10 @@ class MusicPlayerHandler extends BaseAudioHandler } bool get _shouldIgnoreComplete => - _switchingGeneration != 0 || _interruptionActive || _userPaused; + _switchingGeneration != 0 || + _interruptionActive || + _userPaused || + _playbackError != null; Future _pauseForFocusLoss({required String reason}) async { _log.i('Pausing internal player because of $reason'); @@ -886,7 +926,8 @@ class MusicPlayerHandler extends BaseAudioHandler void _broadcastState({PlayerState? playerState, bool? loading}) { final state = playerState ?? _player.state; - final playing = state == PlayerState.playing; + final failed = _playbackError != null; + final playing = !failed && state == PlayerState.playing; playbackState.add( playbackState.value.copyWith( @@ -903,9 +944,13 @@ class MusicPlayerHandler extends BaseAudioHandler MediaAction.skipToNext, }, androidCompactActionIndices: const [1, 2, 3], - processingState: (loading == true) + processingState: failed + ? AudioProcessingState.error + : (loading == true) ? AudioProcessingState.loading : _mapProcessingState(state), + errorCode: failed ? 1 : null, + errorMessage: _playbackError?.toString(), playing: playing, queueIndex: _index >= 0 ? _index : null, speed: _autoMix.rate, @@ -1465,6 +1510,7 @@ class MusicPlayerHandler extends BaseAudioHandler } final generation = ++_playRequestGeneration; _sourceReady = false; + _playbackError = null; // Advance the requested index synchronously so consecutive Next taps do // not all target the same song while AutoMix/source preparation awaits. _index = index; @@ -1598,8 +1644,11 @@ class MusicPlayerHandler extends BaseAudioHandler return; } if (resolved == null) { - _log.e('No playable source for ${media.title}'); - _broadcastState(playerState: PlayerState.stopped); + _reportPlaybackError( + _player, + StateError('Audio file is unavailable'), + StackTrace.current, + ); return; } @@ -1696,11 +1745,9 @@ class MusicPlayerHandler extends BaseAudioHandler // Some files do not emit onDurationChanged reliably (stuck at 0:00); // poll the engine for the real duration as a fallback. unawaited(_ensureDurationKnown(media, generation)); - } catch (e) { + } catch (e, stackTrace) { if (!_isCurrentPlayRequest(generation, media)) return; - _sourceReady = false; - _log.e('Playback failed for ${media.title}: $e'); - _broadcastState(playerState: PlayerState.stopped); + _reportPlaybackError(_player, e, stackTrace); } finally { if (playbackLease != null) { await PlatformBridge.closeContentUriPlaybackLease(playbackLease.token); @@ -1921,9 +1968,13 @@ class MusicPlayerHandler extends BaseAudioHandler if (generation != _playRequestGeneration || _disposed) return; try { await _player.resume(); - } on PlatformException { - if (!_player.needsSourceReload) rethrow; - _broadcastState(playerState: PlayerState.paused); + } catch (error, stackTrace) { + if (generation != _playRequestGeneration || _disposed) return; + if (_player.needsSourceReload) { + _broadcastState(playerState: PlayerState.paused); + } else { + _reportPlaybackError(_player, error, stackTrace); + } return; } if (generation != _playRequestGeneration || _disposed) { @@ -2045,6 +2096,7 @@ class MusicPlayerHandler extends BaseAudioHandler await _player.stop(); if (generation != _playRequestGeneration || _disposed) return; _sourceReady = false; + _playbackError = null; _activeResolvedPath = null; await _cleanupPendingResolvedPaths(); if (generation != _playRequestGeneration || _disposed) return; diff --git a/lib/widgets/playback_feedback.dart b/lib/widgets/playback_feedback.dart index 53eefcdd..53242550 100644 --- a/lib/widgets/playback_feedback.dart +++ b/lib/widgets/playback_feedback.dart @@ -23,6 +23,24 @@ class _PlaybackFeedbackState extends ConsumerState { @override void initState() { super.initState(); + ref.listenManual( + playbackStateProvider.select((state) => state.value?.errorMessage), + (previous, message) { + if (message == null || + message == previous || + !mounted || + WidgetsBinding.instance.lifecycleState != + AppLifecycleState.resumed) { + return; + } + showAppSnackBar( + context, + content: Text( + context.l10n.snackbarCannotOpenFile(context.friendlyError(message)), + ), + ); + }, + ); ref.listenManual(musicPlayerRuntimeProvider, (_, runtime) { final revision = ++_subscriptionRevision; unawaited(_subscription?.cancel()); diff --git a/test/music_player_automix_test.dart b/test/music_player_automix_test.dart index 8418d3bd..9841e710 100644 --- a/test/music_player_automix_test.dart +++ b/test/music_player_automix_test.dart @@ -69,6 +69,9 @@ class _AudioNative { final sourceContents = {}; final preparedContents = {}; final sourceGates = >{}; + final sourceFailures = {}; + final resumeFailures = {}; + bool failResumeMethod = false; final resumedSources = []; Completer? resumeGate; final resumeGates = >{}; @@ -103,6 +106,10 @@ class _AudioNative { case 'setSourceUrl': final source = args['url']! as String; await sourceGates[source]?.future; + if (sourceFailures.contains(source)) { + unawaited(error(id)); + return null; + } // Android caches equal URLs, including recycled SAF descriptors. if (sources[id] != source) { sources[id] = source; @@ -117,6 +124,13 @@ class _AudioNative { case 'resume': await resumeGate?.future; await resumeGates[id]?.future; + if (failResumeMethod) { + throw PlatformException(code: 'AndroidAudioError'); + } + if (resumeFailures.contains(id)) { + await error(id); + return null; + } playing.add(id); resumedSources.add(sources[id]!); if (id.startsWith('music-mix-') || id.startsWith('music-effect-')) { @@ -156,6 +170,18 @@ class _AudioNative { ); } + Future error(String id) async { + await messenger.handlePlatformMessage( + 'xyz.luan/audioplayers/events/$id', + const StandardMethodCodec().encodeErrorEnvelope( + code: 'AndroidAudioError', + message: 'Audio output failed', + details: 'MEDIA_ERROR_SYSTEM', + ), + (_) {}, + ); + } + String get prepared => live.singleWhere((id) => id != 'music-player'); double? lastVolume(String id) => @@ -285,6 +311,117 @@ void main() { }, ); + group('playback failures', () { + test('runtime error stays on the song and Play retries it', () async { + await handler.setQueueAndPlay(_tracks); + await native.error('music-player'); + await _until( + () => + handler.playbackState.value.processingState == + AudioProcessingState.error && + native.playing.isEmpty, + ); + expect(handler.playbackState.value.playing, isFalse); + expect( + handler.playbackState.value.errorMessage, + contains('Audio output'), + ); + expect(handler.mediaItem.value?.id, 'one'); + + // Even a late completion from a failed output must not advance the queue. + await native.event('music-player', 'audio.onComplete'); + await Future.delayed(const Duration(milliseconds: 30)); + expect(handler.playbackState.value.queueIndex, 0); + expect( + handler.playbackState.value.processingState, + AudioProcessingState.error, + ); + + await handler.play(); + expect(handler.playbackState.value.playing, isTrue); + expect(handler.playbackState.value.errorMessage, isNull); + expect(handler.playbackState.value.errorCode, isNull); + expect(native.resumedSources, ['/one.flac', '/one.flac']); + await native.event('music-player', 'audio.onComplete'); + await _until(() => handler.playbackState.value.queueIndex == 1); + }); + + test('source failure is reported and can be retried', () async { + native.sourceFailures.add('/one.flac'); + await handler.setQueueAndPlay(_tracks); + expect( + handler.playbackState.value.processingState, + AudioProcessingState.error, + ); + expect(handler.playbackState.value.playing, isFalse); + expect(handler.playbackState.value.queueIndex, 0); + expect(native.resumedSources, isEmpty); + + native.sourceFailures.clear(); + await handler.play(); + expect(handler.playbackState.value.playing, isTrue); + expect(handler.mediaItem.value?.id, 'one'); + expect(handler.playbackState.value.errorMessage, isNull); + }); + + test('error during resume cannot be overwritten by ready state', () async { + native.resumeFailures.add('music-player'); + await handler.setQueueAndPlay(_tracks); + await _until( + () => + handler.playbackState.value.processingState == + AudioProcessingState.error, + ); + expect(handler.playbackState.value.playing, isFalse); + expect(handler.playbackState.value.queueIndex, 0); + native.resumeFailures.clear(); + await handler.skipToNext(); + expect(handler.mediaItem.value?.id, 'two'); + expect(handler.playbackState.value.playing, isTrue); + expect(handler.playbackState.value.errorMessage, isNull); + }); + + test( + 'failed resume from pause reports an error without throwing', + () async { + await handler.setQueueAndPlay(_tracks); + await handler.pause(); + native.resumeFailures.add('music-player'); + await handler.play(); + await _until( + () => + handler.playbackState.value.processingState == + AudioProcessingState.error, + ); + expect( + handler.playbackState.value.processingState, + AudioProcessingState.error, + ); + expect(handler.playbackState.value.playing, isFalse); + expect(handler.mediaItem.value?.id, 'one'); + native.resumeFailures.clear(); + await handler.play(); + expect(handler.playbackState.value.playing, isTrue); + expect(handler.playbackState.value.errorMessage, isNull); + }, + ); + + test('resume method failure is reported without escaping Play', () async { + await handler.setQueueAndPlay(_tracks); + await handler.pause(); + native.failResumeMethod = true; + await handler.play(); + expect( + handler.playbackState.value.processingState, + AudioProcessingState.error, + ); + expect(handler.playbackState.value.playing, isFalse); + native.failResumeMethod = false; + await handler.play(); + expect(handler.playbackState.value.playing, isTrue); + }); + }); + group('automatic playback on system volume', () { const events = MethodChannel( 'com.kurenai7968.volume_controller.volume_listener_event', diff --git a/test/playback_feedback_test.dart b/test/playback_feedback_test.dart index 088b5ce6..a2c38324 100644 --- a/test/playback_feedback_test.dart +++ b/test/playback_feedback_test.dart @@ -1,3 +1,6 @@ +import 'dart:async'; + +import 'package:audio_service/audio_service.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -11,6 +14,54 @@ import 'package:spotiflac_android/widgets/playback_feedback.dart'; void main() { for (final mornye in [false, true]) { + testWidgets('playback errors show themed feedback (Mornye: $mornye)', ( + tester, + ) async { + tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.resumed); + final runtime = MusicPlayerRuntime(); + addTearDown(runtime.dispose); + final states = StreamController(); + addTearDown(states.close); + await tester.pumpWidget( + ProviderScope( + overrides: [ + musicPlayerRuntimeProvider.overrideWithValue(runtime), + playbackStateProvider.overrideWith((_) => states.stream), + ], + child: MaterialApp( + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + theme: mornye ? MornyeTheme.build(Brightness.dark) : ThemeData(), + builder: (_, child) => + AppScaffoldMessenger(child: PlaybackFeedback(child: child!)), + home: const Scaffold(body: SizedBox.expand()), + ), + ), + ); + await tester.pumpAndSettle(); + final failed = PlaybackState( + processingState: AudioProcessingState.error, + errorCode: 1, + errorMessage: 'Audio output failed', + ); + states.add(failed); + await tester.pumpAndSettle(); + expect( + find.text('Cannot open file: Audio output failed'), + findsOneWidget, + ); + expect( + find.byType(MornyeGlassPanel), + mornye ? findsOneWidget : findsNothing, + ); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + states.add(failed.copyWith(updatePosition: const Duration(seconds: 1))); + await tester.pumpAndSettle(); + expect(find.text('Cannot open file: Audio output failed'), findsNothing); + expect(tester.takeException(), isNull); + }); + testWidgets('muted feedback is transient and themed (Mornye: $mornye)', ( tester, ) async { diff --git a/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/MediaPlayerWrapper.kt b/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/MediaPlayerWrapper.kt index 0c780fb4..f6041f40 100644 --- a/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/MediaPlayerWrapper.kt +++ b/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/MediaPlayerWrapper.kt @@ -11,6 +11,7 @@ class MediaPlayerWrapper( ) : PlayerWrapper { private val mediaPlayer = createMediaPlayer(wrappedPlayer) private var equalizer: PlaybackEqualizer? = null + private var playbackParamsApplied = false fun setEqualizer(configuration: Map) { if (Build.VERSION.SDK_INT < 28) { @@ -56,10 +57,12 @@ class MediaPlayerWrapper( override fun setRate(rate: Float) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + // Normal playback does not need the platform's time-stretch path. + // Once changed, still apply 1.0 to restore the original speed. + if (rate == 1.0f && !playbackParamsApplied) return mediaPlayer.playbackParams = mediaPlayer.playbackParams.setSpeed(rate) - } else if (rate == 1.0f) { - mediaPlayer.start() - } else { + playbackParamsApplied = true + } else if (rate != 1.0f) { error("Changing the playback rate is only available for Android M/23+ or using LOW_LATENCY mode.") } } @@ -74,8 +77,9 @@ class MediaPlayerWrapper( } override fun start() { - // Setting playback rate instead of mediaPlayer.start(). setRate(wrappedPlayer.rate) + // Do not depend on setPlaybackParams implicitly starting the output. + mediaPlayer.start() } override fun pause() { diff --git a/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/WrappedPlayer.kt b/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/WrappedPlayer.kt index 3a5baab7..ef6947b7 100644 --- a/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/WrappedPlayer.kt +++ b/third_party/audioplayers_android/android/src/main/kotlin/xyz/luan/audioplayers/player/WrappedPlayer.kt @@ -325,6 +325,10 @@ class WrappedPlayer internal constructor( } fun onError(what: Int, extra: Int): Boolean { + val wasPrepared = prepared + prepared = false + playing = false + focusManager.handleStop() val whatMsg = if (what == MediaPlayer.MEDIA_ERROR_SERVER_DIED) { "MEDIA_ERROR_SERVER_DIED" } else { @@ -338,20 +342,18 @@ class WrappedPlayer internal constructor( MediaPlayer.MEDIA_ERROR_TIMED_OUT -> "MEDIA_ERROR_TIMED_OUT" else -> "MEDIA_ERROR_UNKNOWN {extra:$extra}" } - if (!prepared && extraMsg == "MEDIA_ERROR_SYSTEM") { - handleError( - "AndroidAudioError", - "Failed to set source. For troubleshooting, see: " + - "https://github.com/bluefireteam/audioplayers/blob/main/troubleshooting.md", - "$whatMsg, $extraMsg", - ) - } else { - // When an error occurs, reset player to not [prepared]. - // Then no functions will be called, which end up in an illegal player state. - prepared = false - handleError("AndroidAudioError", whatMsg, extraMsg) - } - return false + handleError( + "AndroidAudioError", + if (wasPrepared) { + "Android could not play this audio." + } else { + "Android could not open this audio source." + }, + "$whatMsg, $extraMsg", + ) + // An unhandled error also invokes OnCompletionListener, which would + // incorrectly advance the application's queue after a playback failure. + return true } /**