yorickr/plezy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(player): skip relative to the position a jump landed on

Browse Source 
A coalesced key-repeat skip pins its target so a slow backend cannot make
the next press rebase off a position the seek has not reached yet. Nothing
retired that pin when something else moved the playhead, so for the ten
seconds it survived, a skip taken after a timeline tap, a chapter jump, an
OS media control or a peer sync resumed from the superseded target and threw
the user back across their own jump.

Publish every playhead movement on the player and retire the pin whenever
the announced destination is not the accumulator's own commit. Overlapping
seeks and backend-chosen relocations arbitrate by which operation the
backend accepted, so a request that was merely asked for cannot speak for
where the playhead ended up.

close #1819
This commit is contained in:
edde746
2026-08-07 08:43:48 +02:00
parent 660e375248
commit 4816e3928f
21 changed files with 2809 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/widgets/video_controls_mobile_skip_zones_test.dart
+29
View File
@@ -1,3 +1,4 @@
import 'dart:async';
import 'package:drift/native.dart';
import 'package:flutter/gestures.dart' show kDoubleTapTimeout;
import 'package:flutter/material.dart';
@@ -299,6 +300,26 @@ void main() {
await settleFeedback(tester);
});
testWidgets('a double tap keeps its own readout while a keyboard burst is pending', (tester) async {
// Both input paths share one badge. The tap's seek is foreign to the
// keyboard accumulator, so retiring that burst must not take down the
// readout the tap just raised.
await pumpControls(tester);
await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowRight);
await tester.pump();
expect(find.byType(DoubleTapFeedback), findsOneWidget);
await doubleTap(tester, forwardZoneOf(tester));
expect(find.byType(DoubleTapFeedback), findsOneWidget, reason: 'the tap that just seeked owns the readout now');
expect(find.text('10s'), findsOneWidget, reason: 'and it counts only its own step');
await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowRight);
await tester.pump();
await settleFeedback(tester);
});
testWidgets('a lone tap in the opposite zone does not skip', (tester) async {
await pumpControls(tester);
@@ -340,6 +361,7 @@ void main() {
/// Minimal [Player] recording seek targets against a fixed 45-minute item.
class _RecordingPlayer implements Player {
final List<Duration> seeks = [];
final StreamController<Duration?> _jumpController = StreamController<Duration?>.broadcast();
bool _playing = true;
Duration _position = const Duration(minutes: 10);
@@ -353,6 +375,7 @@ class _RecordingPlayer implements Player {
@override
PlayerStreams get streams => PlayerStreams(
playheadJump: _jumpController.stream,
playing: const Stream<bool>.empty(),
completed: const Stream<bool>.empty(),
buffering: const Stream<bool>.empty(),
@@ -377,6 +400,12 @@ class _RecordingPlayer implements Player {
Future<void> seek(Duration position) async {
seeks.add(position);
_position = position;
_jumpController.add(position);
}
@override
Future<void> dispose({bool preserveDisplayMode = false}) async {
await _jumpController.close();
}
@override
test/widgets/video_controls_transient_feedback_test.dart
+248 -4
View File
@@ -1,3 +1,4 @@
import 'dart:async';
import 'dart:math' as math;
import 'package:drift/native.dart';
import 'package:flutter/material.dart';
@@ -17,6 +18,7 @@ import 'package:plezy/utils/platform_detector.dart';
import 'package:plezy/watch_together/providers/watch_together_provider.dart';
import 'package:plezy/widgets/video_controls/player_chrome_controller.dart';
import 'package:plezy/widgets/app_icon.dart';
import 'package:plezy/widgets/video_controls/desktop_video_controls.dart';
import 'package:plezy/widgets/video_controls/video_controls.dart';
import 'package:plezy/widgets/video_controls/widgets/double_tap_feedback.dart';
import 'package:plezy/widgets/video_controls/widgets/player_toast_indicator.dart';
@@ -75,6 +77,7 @@ void main() {
chrome.dispose();
toast.dispose();
await database.close();
await player.dispose();
});
Future<void> pumpControls(
@@ -83,6 +86,7 @@ void main() {
bool wireTransportCallback = false,
bool isLive = false,
ValueChanged<int>? onLiveSeekBy,
String itemId = 'transient-feedback',
}) async {
transportCommands = [];
await tester.pumpWidget(
@@ -101,7 +105,7 @@ void main() {
child: PlexVideoControls(
player: player,
volumeController: volume,
metadata: testMediaItem(id: 'transient-feedback'),
metadata: testMediaItem(id: itemId),
toastController: toast,
chromeController: chrome,
initialChapters: chapters,
@@ -266,6 +270,101 @@ void main() {
await settleFeedback(tester);
});
testWidgets('a skip after a jump elsewhere starts from where the jump landed', (tester) async {
// #1819: the timeline, a chapter jump, an OS media control and a Watch
// Together peer all land on Player.seek. Whichever of them moves the
// playhead, the coalesced target the previous skip pinned is stale, and a
// skip that resumes from it rewinds the user back across their own jump.
await pumpControls(tester);
await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
expect(player.seeks, [const Duration(minutes: 10, seconds: 10)]);
await player.seek(const Duration(minutes: 30));
await tester.pump();
expect(
find.text('10s'),
findsNothing,
reason: 'the readout promised a skip the jump just cancelled, so it must come down at once',
);
await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
expect(
player.seeks.last,
const Duration(minutes: 30, seconds: 10),
reason: 'the skip must be relative to the new position, not to the superseded 10:10 target',
);
expect(find.text('10s'), findsOneWidget, reason: 'the abandoned burst total must not keep climbing');
await settleFeedback(tester);
});
testWidgets('a skip after a stream rebuilt at a resume position starts from there', (tester) async {
// Dead-stream recovery answers a seek request by reopening the source at
// the target rather than seeking, so a fix that only watched Player.seek
// would leave the pin stale here.
await pumpControls(tester);
await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
final seeksBeforeReload = player.seeks.length;
player.reopenAt(const Duration(minutes: 3));
await tester.pump();
expect(player.seeks, hasLength(seeksBeforeReload), reason: 'a reload is not a seek');
await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
expect(player.seeks.last, const Duration(minutes: 3, seconds: 10));
await settleFeedback(tester);
});
testWidgets('a new item drops the previous item\'s pending skip and its badge total', (tester) async {
// The controls survive an in-place episode swap. A pending target is an
// offset into the outgoing item's timeline, and the badge total describes
// a burst that will never be committed.
player.freezePositionOnSeek = true;
await pumpControls(tester);
for (var i = 0; i < 2; i++) {
await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
}
expect(find.text('20s'), findsOneWidget);
expect(player.seeks.last, const Duration(minutes: 10, seconds: 20));
await pumpControls(tester, itemId: 'next-episode');
await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward);
await tester.pump();
expect(
player.seeks.last,
const Duration(minutes: 10, seconds: 10),
reason: 'the new item restarts from the live position, not from the outgoing 10:20 target',
);
expect(find.text('10s'), findsOneWidget, reason: 'the badge must not keep counting the abandoned burst');
await settleFeedback(tester);
});
testWidgets('a media fast-forward key announces the chapter it lands on', (tester) async {
await pumpControls(
tester,
@@ -713,9 +812,19 @@ void main() {
chrome.dispose();
toast.dispose();
await database.close();
await player.dispose();
});
Future<void> pumpDesktopControls(WidgetTester tester) async {
Future<void> pumpDesktopControls(
WidgetTester tester, {
_RecordingPlayer? withPlayer,
bool isLive = false,
ValueChanged<int>? onLiveSeekBy,
ValueChanged<int>? onLiveSeek,
VoidCallback? onNext,
bool canNavigateMediaItems = false,
}) async {
final active = withPlayer ?? player;
await tester.pumpWidget(
MultiProvider(
providers: [
@@ -730,12 +839,16 @@ void main() {
width: 1280,
height: 720,
child: PlexVideoControls(
player: player,
player: active,
volumeController: volume,
metadata: testMediaItem(id: 'desktop-keyboard-seek'),
toastController: toast,
chromeController: chrome,
canNavigateMediaItems: false,
canNavigateMediaItems: canNavigateMediaItems,
isLive: isLive,
onLiveSeekBy: onLiveSeekBy,
onLiveSeek: onLiveSeek,
onNext: onNext,
),
),
),
@@ -830,6 +943,119 @@ void main() {
await settleFeedback(tester);
});
testWidgets('a lagging backend still yields the pin to a jump elsewhere', (tester) async {
// The two invariants pull in opposite directions: a slow seek must not
// retire the pin (#1676), a jump from anywhere else must (#1819). Freeze
// the reported position so only the seek announcement can tell them apart.
player.freezePositionOnSeek = true;
await pumpDesktopControls(tester);
await pressKey(tester, LogicalKeyboardKey.arrowRight);
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(player.seeks.last, const Duration(minutes: 10, seconds: 20), reason: 'the burst is still pinned');
player.setPosition(const Duration(minutes: 2));
await player.seek(const Duration(minutes: 2));
await tester.pump();
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(player.seeks.last, const Duration(minutes: 2, seconds: 10));
expect(find.text('10s'), findsOneWidget);
await settleFeedback(tester);
});
testWidgets('swapping the player moves the pin listener with it', (tester) async {
// The controls survive a player replacement (didUpdateWidget accepts a new
// instance), so an accumulator left bound to the retired player would keep
// a pin from a timeline that no longer exists and take orders from a
// player nobody is watching.
final replacement = _RecordingPlayer()
..setPosition(const Duration(minutes: 4))
// Frozen, so a pinned chain stays distinguishable from a rebase.
..freezePositionOnSeek = true;
addTearDown(replacement.dispose);
await pumpDesktopControls(tester);
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(player.seeks.last, const Duration(minutes: 10, seconds: 10));
await pumpDesktopControls(tester, withPlayer: replacement);
expect(tester.takeException(), isNull);
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(
replacement.seeks.last,
const Duration(minutes: 4, seconds: 10),
reason: 'the retired pin must not survive the swap and chain to 10:20',
);
// The badge is not asserted here: pumping the replacement settles the
// feedback timer, which clears the total on its own.
// Build a fresh pin on the new player, then let the retired one shout.
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(replacement.seeks.last, const Duration(minutes: 4, seconds: 20));
await player.seek(const Duration(minutes: 30));
await tester.pump();
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(
replacement.seeks.last,
const Duration(minutes: 4, seconds: 30),
reason: 'a jump from the retired player must not retire the current pin',
);
await settleFeedback(tester);
});
testWidgets('an absolute live seek takes down the badge a live skip raised', (tester) async {
// Live relative skips go to the parent epoch accumulator, not _hiddenSeek
// (#1253), so no playhead jump can retire this badge. The absolute seek
// cancels the queued skip, so its promised total is going nowhere.
final liveOffsets = <int>[];
final absoluteSeeks = <int>[];
await pumpDesktopControls(tester, isLive: true, onLiveSeekBy: liveOffsets.add, onLiveSeek: absoluteSeeks.add);
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(liveOffsets, [10]);
expect(find.text('10s'), findsOneWidget);
chrome.show();
await tester.pump();
final live = tester.widget<DesktopVideoControls>(find.byType(DesktopVideoControls));
live.onLiveSeek!(120);
await tester.pump();
expect(absoluteSeeks, [120], reason: 'the wrapper still delegates to the screen');
expect(find.byType(DoubleTapFeedback), findsNothing, reason: 'the skip it promised was cancelled with it');
await settleFeedback(tester);
});
testWidgets('switching what is playing takes the badge with the old timeline', (tester) async {
// A live channel switch cancels the queued live offset, and an item
// change re-keys the whole timeline. Neither runs through _hiddenSeek, so
// nothing else would retire a readout that now describes nothing.
final liveOffsets = <int>[];
var nextPresses = 0;
await pumpDesktopControls(tester, isLive: true, onLiveSeekBy: liveOffsets.add, onNext: () => nextPresses++);
await pressKey(tester, LogicalKeyboardKey.arrowRight);
expect(liveOffsets, [10]);
expect(find.text('10s'), findsOneWidget);
chrome.show();
await tester.pump();
tester.widget<DesktopVideoControls>(find.byType(DesktopVideoControls)).onNext!();
await tester.pump();
expect(nextPresses, 1, reason: 'the wrapper still delegates to the screen');
expect(find.byType(DoubleTapFeedback), findsNothing);
await settleFeedback(tester);
});
});
group('formatSkipFeedbackLabel', () {
@@ -851,6 +1077,7 @@ void main() {
/// playing/position state so intent-dependent behaviour can be asserted.
class _RecordingPlayer implements Player {
final List<Duration> seeks = [];
final StreamController<Duration?> _jumpController = StreamController<Duration?>.broadcast();
int playCalls = 0;
int pauseCalls = 0;
int playOrPauseCalls = 0;
@@ -866,6 +1093,13 @@ class _RecordingPlayer implements Player {
void setPosition(Duration value) => _position = value;
/// Mirrors [PlayerBase.resetPlaybackProgress]: an in-place reload rebuilds
/// the stream at a resume position without ever calling [seek].
void reopenAt(Duration value) {
_position = value;
_jumpController.add(value);
}
@override
String get playerType => 'mpv';
@@ -879,6 +1113,7 @@ class _RecordingPlayer implements Player {
completed: const Stream<bool>.empty(),
buffering: const Stream<bool>.empty(),
position: const Stream<Duration>.empty(),
playheadJump: _jumpController.stream,
duration: const Stream<Duration>.empty(),
seekable: const Stream<bool>.empty(),
buffer: const Stream<Duration>.empty(),
@@ -895,10 +1130,19 @@ class _RecordingPlayer implements Player {
backendSwitched: const Stream<void>.empty(),
);
/// Mirrors [PlayerBase.runSeek]: the requested target is announced whatever
/// asked for it, while [freezePositionOnSeek] models a backend that has not
/// moved the reported position yet.
@override
Future<void> seek(Duration position) async {
seeks.add(position);
if (!freezePositionOnSeek) _position = position;
_jumpController.add(position);
}
@override
Future<void> dispose({bool preserveDisplayMode = false}) async {
await _jumpController.close();
}
@override