fix(player): keep progress reporting and live sessions alive across failed retries, re-arm the sleep timer on prompt dismissal
A Jellyfin live retry that failed before open stop-reported the still-current session (recover returns the receiver), killing /Sessions/Playing for the rest of the session; the discard now skips a recovered session identical to the current one. An in-place reload failing before the open boundary left the progress tracker disposed and nulled, so the resumed stream never reported again and the exit flush was skipped; the re-wire now also runs on the rollback path against the restored metadata. Dismissing the Still Watching prompt via back or next also never re-armed the sleep timer; it now counts as the same acknowledgement as Continue.
This commit is contained in:
@@ -8,6 +8,12 @@ enum LiveStreamRetryResult { succeeded, failed, stale }
|
||||
/// Every asynchronous stage is followed by a current-attempt check. Failures
|
||||
/// from stale attempts are intentionally ignored because a newer operation
|
||||
/// owns the player and its error UI.
|
||||
///
|
||||
/// A recovery may hand back the very session it was asked to recover
|
||||
/// (Jellyfin's `recover` returns the receiver), so a failed attempt must not
|
||||
/// discard — terminally stop-report — the session that is still current and
|
||||
/// will be re-adopted by the next retry. [currentSession] guards the discard
|
||||
/// by identity.
|
||||
Future<LiveStreamRetryResult> runLiveStreamRetry<Session>({
|
||||
required LiveStreamRecovery<Session> recover,
|
||||
required LiveStreamUrlLookup<Session> lookupStreamUrl,
|
||||
@@ -15,6 +21,7 @@ Future<LiveStreamRetryResult> runLiveStreamRetry<Session>({
|
||||
required Future<void> Function(String streamUrl) open,
|
||||
required bool Function() isCurrent,
|
||||
required void Function(Session session) adoptSession,
|
||||
required Session? Function() currentSession,
|
||||
required void Function(Object error, StackTrace stackTrace) reportFailure,
|
||||
required void Function(Session session) discardSession,
|
||||
required void Function() onFinished,
|
||||
@@ -44,7 +51,9 @@ Future<LiveStreamRetryResult> runLiveStreamRetry<Session>({
|
||||
reportFailure(error, stackTrace);
|
||||
return LiveStreamRetryResult.failed;
|
||||
} finally {
|
||||
if (recovered != null && !adopted) discardSession(recovered);
|
||||
if (recovered != null && !adopted && !identical(recovered, currentSession())) {
|
||||
discardSession(recovered);
|
||||
}
|
||||
onFinished();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -196,6 +196,10 @@ extension _VideoPlayerLiveTvMethods on VideoPlayerScreenState {
|
||||
_live.selectedSubtitle = recoveredSubtitle;
|
||||
_live.markStreamRestartedAtLiveEdge();
|
||||
},
|
||||
// Jellyfin's recover() returns the receiver, so the recovered object can
|
||||
// be the still-current session; the retry helper skips the discard by
|
||||
// identity so a failed retry cannot terminally stop-report it.
|
||||
currentSession: () => _live.session,
|
||||
discardSession: _abandonLiveSession,
|
||||
reportFailure: (error, stackTrace) {
|
||||
appLogger.e('Failed to recover live stream', error: error, stackTrace: stackTrace);
|
||||
|
||||
@@ -287,6 +287,11 @@ extension _VideoPlayerPlaybackPromptMethods on VideoPlayerScreenState {
|
||||
_stillWatchingTimer?.cancel();
|
||||
if (_showStillWatchingPrompt) {
|
||||
_unfocusStillWatchingPrompt();
|
||||
// Back or a next-episode action on the visible prompt is the same
|
||||
// "still watching" acknowledgement as Continue: re-arm the sleep timer.
|
||||
// Guarded on prompt visibility — ordinary navigation also dismisses
|
||||
// here and must leave an armed timer untouched.
|
||||
SleepTimerService().restartTimer();
|
||||
_setPlayerState(() {
|
||||
_showStillWatchingPrompt = false;
|
||||
});
|
||||
|
||||
@@ -741,13 +741,18 @@ extension _VideoPlayerReloadMethods on VideoPlayerScreenState {
|
||||
if (wasPlayingBeforeReload && mounted && player == currentPlayer) {
|
||||
unawaited(_playWithPlaybackIntent(currentPlayer));
|
||||
}
|
||||
} else if (_progressTracker == null && player == currentPlayer) {
|
||||
// The new file is playing and its session is committed — keep the
|
||||
// new identity and make sure progress reporting is wired to the
|
||||
// item actually on screen (the failure may have hit before
|
||||
// _wirePerItemPlaybackServices ran).
|
||||
}
|
||||
if (_progressTracker == null && player == currentPlayer) {
|
||||
// Progress reporting must survive both exits: beforeArm disposed the
|
||||
// tracker before the open boundary, so rebuild it for the item
|
||||
// actually on screen (the failure may have hit before
|
||||
// _wirePerItemPlaybackServices ran). On the rollback path
|
||||
// _currentMetadata was just restored to the previous item; after a
|
||||
// committed open it is the replacement. _playbackSession describes
|
||||
// that same item on both paths, so its client and session fields are
|
||||
// the right wiring either way.
|
||||
_wirePerItemPlaybackServices(
|
||||
metadata: metadata,
|
||||
metadata: _currentMetadata,
|
||||
mediaClient: _playbackSession?.reportingClient,
|
||||
offlineWatchService: offlineWatchService,
|
||||
playSessionId: _playbackPlaySessionId,
|
||||
|
||||
@@ -863,6 +863,11 @@ class VideoPlayerScreenState extends State<VideoPlayerScreen> with WidgetsBindin
|
||||
@visibleForTesting
|
||||
PlayerChromeController get chromeController => _chromeController;
|
||||
|
||||
/// Lets reload-failure coverage assert the progress tracker was rebuilt and
|
||||
/// which item it is bound to; the tracker itself is private screen state.
|
||||
@visibleForTesting
|
||||
PlaybackProgressTracker? get debugProgressTrackerForTesting => _progressTracker;
|
||||
|
||||
late final PlayerNavigationCoordinator _playerNavigationCoordinator;
|
||||
|
||||
@override
|
||||
|
||||
@@ -70,6 +70,37 @@ void main() {
|
||||
expect(harness.adopted, isFalse);
|
||||
expect(harness.discarded, isTrue);
|
||||
});
|
||||
|
||||
test('a failed retry does not discard the recover-returned current session', () async {
|
||||
// Jellyfin's recover() returns the receiver: the recovered object IS the
|
||||
// session the screen still holds. Discarding it would stop-report (and
|
||||
// terminally latch) the session playback keeps using and re-adopts on
|
||||
// the next retry.
|
||||
final harness = _RetryHarness(failAt: _Stage.open);
|
||||
final session = Object();
|
||||
harness.activeSession = session;
|
||||
harness.recoveredSession = session;
|
||||
|
||||
expect(await harness.run(), LiveStreamRetryResult.failed);
|
||||
|
||||
expect(harness.failures, hasLength(1));
|
||||
expect(harness.finished, isTrue);
|
||||
expect(harness.adopted, isFalse);
|
||||
expect(harness.discarded, isFalse);
|
||||
});
|
||||
|
||||
test('a failed retry still discards a recovered session distinct from the current one', () async {
|
||||
final harness = _RetryHarness(failAt: _Stage.open);
|
||||
harness.activeSession = Object();
|
||||
harness.recoveredSession = Object();
|
||||
|
||||
expect(await harness.run(), LiveStreamRetryResult.failed);
|
||||
|
||||
expect(harness.failures, hasLength(1));
|
||||
expect(harness.finished, isTrue);
|
||||
expect(harness.adopted, isFalse);
|
||||
expect(harness.discarded, isTrue);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -87,13 +118,19 @@ class _RetryHarness {
|
||||
bool adopted = false;
|
||||
bool discarded = false;
|
||||
|
||||
/// What the caller's `_live.session` currently holds; recover hands back
|
||||
/// [recoveredSession] when set, otherwise a fresh object.
|
||||
Object? activeSession;
|
||||
Object? recoveredSession;
|
||||
|
||||
Future<LiveStreamRetryResult> run() => runLiveStreamRetry<Object>(
|
||||
recover: () => _stage(_Stage.recover, Object.new),
|
||||
recover: () => _stage(_Stage.recover, () => recoveredSession ?? Object()),
|
||||
lookupStreamUrl: (_) => _stage(_Stage.lookup, () => 'https://example.com/live'),
|
||||
applyPlayerOptions: () => _stage<void>(_Stage.options, () {}),
|
||||
open: (_) => _stage<void>(_Stage.open, () {}),
|
||||
isCurrent: () => current,
|
||||
adoptSession: (_) => adopted = true,
|
||||
currentSession: () => activeSession,
|
||||
reportFailure: (error, _) => failures.add(error),
|
||||
discardSession: (_) => discarded = true,
|
||||
onFinished: () => finished = true,
|
||||
|
||||
@@ -0,0 +1,201 @@
|
||||
import 'dart:async';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:drift/drift.dart' show Value;
|
||||
import 'package:drift/native.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:path_provider_platform_interface/path_provider_platform_interface.dart';
|
||||
import 'package:plezy/database/app_database.dart';
|
||||
import 'package:plezy/media/ids.dart';
|
||||
import 'package:plezy/media/media_display_criteria.dart';
|
||||
import 'package:plezy/models/download_models.dart';
|
||||
import 'package:plezy/mpv/mpv.dart';
|
||||
import 'package:plezy/providers/multi_server_provider.dart';
|
||||
import 'package:plezy/providers/playback_state_provider.dart';
|
||||
import 'package:plezy/providers/user_profile_provider.dart';
|
||||
import 'package:plezy/screens/video_player_screen.dart';
|
||||
import 'package:plezy/services/download_storage_service.dart';
|
||||
import 'package:plezy/services/offline_watch_sync_service.dart';
|
||||
import 'package:plezy/services/settings_service.dart';
|
||||
import 'package:provider/provider.dart';
|
||||
|
||||
import '../../test_helpers/io_fakes.dart';
|
||||
import '../../test_helpers/media_items.dart';
|
||||
import '../../test_helpers/mock_player_channels.dart';
|
||||
import '../../test_helpers/multi_server_fixtures.dart';
|
||||
import '../../test_helpers/prefs.dart';
|
||||
import '../../test_helpers/watch_together_fakes.dart';
|
||||
|
||||
/// Regression coverage for the in-place reload rollback: a reload that fails
|
||||
/// AFTER `beforeArm` disposed the progress tracker but BEFORE the open
|
||||
/// boundary must rebuild the tracker and bind it to the restored previous
|
||||
/// item — otherwise the resumed session plays on with no progress reporting
|
||||
/// and no exit flush.
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
late Directory tmpRoot;
|
||||
late PathProviderPlatform previousPathProvider;
|
||||
late AppDatabase db;
|
||||
|
||||
setUp(() async {
|
||||
resetSharedPreferencesForTest();
|
||||
SettingsService.resetForTesting();
|
||||
DownloadStorageService.resetForTesting();
|
||||
await SettingsService.getInstance();
|
||||
tmpRoot = await Directory.systemTemp.createTemp('playback_reload_failure_test_');
|
||||
previousPathProvider = PathProviderPlatform.instance;
|
||||
PathProviderPlatform.instance = FakePathProvider(tmpRoot);
|
||||
await DownloadStorageService.instance.initialize(SettingsService.instance);
|
||||
db = AppDatabase.forTesting(NativeDatabase.memory());
|
||||
});
|
||||
|
||||
tearDown(() async {
|
||||
await db.close();
|
||||
DownloadStorageService.resetForTesting();
|
||||
SettingsService.resetForTesting();
|
||||
PathProviderPlatform.instance = previousPathProvider;
|
||||
if (await tmpRoot.exists()) {
|
||||
await tmpRoot.delete(recursive: true);
|
||||
}
|
||||
});
|
||||
|
||||
testWidgets('reload failing at the open boundary rebuilds the tracker for the prior item', (tester) async {
|
||||
final priorItem = testMediaItem(id: 'movie-prior', serverId: 'srv-1', title: 'Prior movie');
|
||||
final targetItem = testMediaItem(id: 'movie-next', serverId: 'srv-1', title: 'Next movie');
|
||||
|
||||
// The reload target resolves offline from this row; the content:// path is
|
||||
// treated as directly playable (no filesystem check), so the flow reaches
|
||||
// player.open deterministically.
|
||||
await _insertCompletedDownload(db, serverId: ServerId('srv-1'), ratingKey: 'movie-next');
|
||||
|
||||
final multi = testMultiServer();
|
||||
final offlineWatch = OfflineWatchSyncService(database: db, serverManager: multi.manager);
|
||||
final userProfiles = UserProfileProvider();
|
||||
addTearDown(() {
|
||||
offlineWatch.dispose();
|
||||
userProfiles.dispose();
|
||||
});
|
||||
|
||||
final nativeInitialize = Completer<bool>();
|
||||
await withMockPlayerChannels(
|
||||
methodChannelName: 'com.plezy/mpv_player',
|
||||
eventChannelName: 'com.plezy/mpv_player/events',
|
||||
methodHandler: (call) {
|
||||
if (call.method == 'initialize') return nativeInitialize.future;
|
||||
return Future<Object?>.value(null);
|
||||
},
|
||||
eventHandler: (_) async => null,
|
||||
testBody: () async {
|
||||
final key = GlobalKey<VideoPlayerScreenState>();
|
||||
await tester.pumpWidget(
|
||||
MultiProvider(
|
||||
providers: [
|
||||
ChangeNotifierProvider(create: (_) => PlaybackStateProvider()),
|
||||
ChangeNotifierProvider<MultiServerProvider>.value(value: multi.provider),
|
||||
ChangeNotifierProvider<OfflineWatchSyncService>.value(value: offlineWatch),
|
||||
ChangeNotifierProvider<UserProfileProvider>.value(value: userProfiles),
|
||||
Provider<AppDatabase>.value(value: db),
|
||||
],
|
||||
child: MaterialApp(
|
||||
home: VideoPlayerScreen(key: key, metadata: priorItem, isOffline: true),
|
||||
),
|
||||
),
|
||||
);
|
||||
expect(key.currentState, isNotNull);
|
||||
|
||||
final fakePlayer = _OpenThrowingPlayer();
|
||||
addTearDown(fakePlayer.dispose);
|
||||
key.currentState!.player = fakePlayer;
|
||||
expect(key.currentState!.debugProgressTrackerForTesting, isNull);
|
||||
|
||||
var navDone = false;
|
||||
final nav = key.currentState!.navigateToQueueItem(targetItem).whenComplete(() => navDone = true);
|
||||
// Drive the reload to completion: the offline profile-settings wait
|
||||
// bounds itself at 2s of test clock, and drift/database work needs
|
||||
// real-event-loop yields.
|
||||
for (var i = 0; i < 400 && !navDone; i++) {
|
||||
await tester.pump(const Duration(milliseconds: 50));
|
||||
if (!navDone) {
|
||||
await tester.runAsync(() => Future<void>.delayed(const Duration(milliseconds: 2)));
|
||||
}
|
||||
}
|
||||
expect(navDone, isTrue, reason: 'the in-place reload must settle');
|
||||
await nav;
|
||||
|
||||
expect(fakePlayer.openCalls, 1, reason: 'the reload must fail at player.open — after beforeArm ran');
|
||||
|
||||
// The rollback must rebuild the tracker beforeArm disposed and bind it
|
||||
// to the restored previous item, not the failed replacement.
|
||||
final tracker = key.currentState!.debugProgressTrackerForTesting;
|
||||
expect(tracker, isNotNull, reason: 'a failed pre-open reload must re-wire progress reporting');
|
||||
expect(tracker!.metadata.globalKey, priorItem.globalKey);
|
||||
expect(tracker.metadata.globalKey, isNot(targetItem.globalKey));
|
||||
|
||||
// Let the rollback's failure snackbar run its display timer down so
|
||||
// nothing is pending when the tree unmounts.
|
||||
await tester.pump(const Duration(seconds: 5));
|
||||
await tester.pump(const Duration(seconds: 1));
|
||||
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
nativeInitialize.complete(true);
|
||||
await tester.pump();
|
||||
},
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/// Minimal player for the reload flow whose `open` always throws — the exact
|
||||
/// "failure before the open boundary" the rollback path recovers from.
|
||||
class _OpenThrowingPlayer extends FakeSyncPlayer {
|
||||
_OpenThrowingPlayer() : super(playing: true, duration: const Duration(minutes: 40));
|
||||
|
||||
int openCalls = 0;
|
||||
|
||||
@override
|
||||
String get playerType => 'mpv';
|
||||
|
||||
@override
|
||||
bool get attachesExternalSubtitlesAtOpen => true;
|
||||
|
||||
@override
|
||||
bool get needsDecoderRefreshAfterDisplaySwitch => false;
|
||||
|
||||
@override
|
||||
Future<void> setProperty(String name, String value) async {}
|
||||
|
||||
@override
|
||||
Future<void> setDisplayCriteria(MediaDisplayCriteria? criteria, {int extraDelayMs = 0}) async {}
|
||||
|
||||
@override
|
||||
Future<bool> requestAudioFocus() async => true;
|
||||
|
||||
@override
|
||||
Future<void> open(
|
||||
Media media, {
|
||||
bool play = true,
|
||||
bool isLive = false,
|
||||
List<SubtitleTrack>? externalSubtitles,
|
||||
Duration? timelineDuration,
|
||||
}) async {
|
||||
openCalls++;
|
||||
throw StateError('open failed before the open boundary');
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _insertCompletedDownload(AppDatabase db, {required ServerId serverId, required String ratingKey}) async {
|
||||
await db
|
||||
.into(db.downloadedMedia)
|
||||
.insert(
|
||||
DownloadedMediaCompanion.insert(
|
||||
serverId: serverId,
|
||||
ratingKey: ratingKey,
|
||||
globalKey: '$serverId:$ratingKey',
|
||||
type: 'movie',
|
||||
status: DownloadStatus.completed.index,
|
||||
videoFilePath: Value('content://offline/$ratingKey'),
|
||||
mediaIndex: const Value(0),
|
||||
),
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,131 @@
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:plezy/providers/playback_state_provider.dart';
|
||||
import 'package:plezy/screens/video_player_screen.dart';
|
||||
import 'package:plezy/services/settings_service.dart';
|
||||
import 'package:plezy/services/sleep_timer_service.dart';
|
||||
import 'package:provider/provider.dart';
|
||||
|
||||
import '../../test_helpers/media_items.dart';
|
||||
import '../../test_helpers/mock_player_channels.dart';
|
||||
import '../../test_helpers/prefs.dart';
|
||||
|
||||
/// Regression coverage for the sleep timer's "Still watching?" acknowledgement
|
||||
/// semantics: dismissing the visible prompt via system back must re-arm the
|
||||
/// timer exactly like Continue, while a back with no prompt on screen must
|
||||
/// leave the timer untouched.
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
setUp(() async {
|
||||
resetSharedPreferencesForTest();
|
||||
SettingsService.resetForTesting();
|
||||
await SettingsService.getInstance();
|
||||
SleepTimerService().cancelTimer();
|
||||
});
|
||||
|
||||
tearDown(() {
|
||||
SleepTimerService().cancelTimer();
|
||||
});
|
||||
|
||||
testWidgets('back on the visible still-watching prompt re-arms the sleep timer with its original duration', (
|
||||
tester,
|
||||
) async {
|
||||
final nativeInitialize = Completer<bool>();
|
||||
|
||||
await withMockPlayerChannels(
|
||||
methodChannelName: 'com.plezy/mpv_player',
|
||||
eventChannelName: 'com.plezy/mpv_player/events',
|
||||
methodHandler: (call) {
|
||||
if (call.method == 'initialize') return nativeInitialize.future;
|
||||
return Future<Object?>.value(null);
|
||||
},
|
||||
eventHandler: (_) async => null,
|
||||
testBody: () async {
|
||||
final key = GlobalKey<VideoPlayerScreenState>();
|
||||
await tester.pumpWidget(_screen(key));
|
||||
expect(key.currentState, isNotNull);
|
||||
|
||||
final sleepTimer = SleepTimerService();
|
||||
sleepTimer.startTimer(const Duration(minutes: 30), () {});
|
||||
// Move the deadline into the past; the next 1s heartbeat fires the
|
||||
// still-watching prompt while _originalDuration stays 30 minutes.
|
||||
sleepTimer.extendTimer(const Duration(minutes: -31));
|
||||
await tester.pump(const Duration(seconds: 1));
|
||||
// Let the onPrompt stream event reach the screen's listener.
|
||||
await tester.pump();
|
||||
|
||||
// The prompt consumed the countdown: the timer parks awaiting an answer.
|
||||
expect(sleepTimer.isActive, isFalse);
|
||||
expect(sleepTimer.originalDuration, const Duration(minutes: 30));
|
||||
|
||||
// System back dismisses the visible prompt — the same acknowledgement
|
||||
// as Continue, so the timer must restart with its original duration.
|
||||
await tester.binding.handlePopRoute();
|
||||
await tester.pump();
|
||||
|
||||
expect(sleepTimer.isActive, isTrue);
|
||||
expect(sleepTimer.duration, const Duration(minutes: 30));
|
||||
expect(sleepTimer.originalDuration, const Duration(minutes: 30));
|
||||
|
||||
sleepTimer.cancelTimer();
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
nativeInitialize.complete(true);
|
||||
await tester.pump();
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('back with no prompt visible leaves the sleep timer untouched', (tester) async {
|
||||
final nativeInitialize = Completer<bool>();
|
||||
|
||||
await withMockPlayerChannels(
|
||||
methodChannelName: 'com.plezy/mpv_player',
|
||||
eventChannelName: 'com.plezy/mpv_player/events',
|
||||
methodHandler: (call) {
|
||||
if (call.method == 'initialize') return nativeInitialize.future;
|
||||
return Future<Object?>.value(null);
|
||||
},
|
||||
eventHandler: (_) async => null,
|
||||
testBody: () async {
|
||||
final key = GlobalKey<VideoPlayerScreenState>();
|
||||
await tester.pumpWidget(_screen(key));
|
||||
expect(key.currentState, isNotNull);
|
||||
|
||||
final sleepTimer = SleepTimerService();
|
||||
sleepTimer.startTimer(const Duration(minutes: 30), () {});
|
||||
final endTimeBefore = sleepTimer.endTime;
|
||||
expect(endTimeBefore, isNotNull);
|
||||
|
||||
// No prompt is on screen: back walks the ordinary exit pipeline and
|
||||
// must not reset (or restart) the armed timer.
|
||||
await tester.binding.handlePopRoute();
|
||||
await tester.pump();
|
||||
|
||||
expect(sleepTimer.isActive, isTrue);
|
||||
expect(sleepTimer.endTime, endTimeBefore);
|
||||
expect(sleepTimer.duration, const Duration(minutes: 30));
|
||||
|
||||
sleepTimer.cancelTimer();
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
nativeInitialize.complete(true);
|
||||
await tester.pump();
|
||||
},
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
Widget _screen(GlobalKey<VideoPlayerScreenState> key) {
|
||||
return ChangeNotifierProvider(
|
||||
create: (_) => PlaybackStateProvider(),
|
||||
child: MaterialApp(
|
||||
home: VideoPlayerScreen(
|
||||
key: key,
|
||||
metadata: testMediaItem(title: 'Still watching sleep timer test'),
|
||||
isOffline: true,
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user