fix(playback): skip redundant mark-watched

close #1287
This commit is contained in:
edde746
2026-06-09 12:19:27 +02:00
parent 726281ed44
commit 92063bd58f
9 changed files with 168 additions and 7 deletions
+28
View File
@@ -5,6 +5,7 @@ import '../services/playback_initialization_types.dart';
import '../utils/app_logger.dart';
import '../utils/media_server_http_client.dart' show AbortController, MediaServerResponse;
import '../utils/external_ids.dart';
import '../utils/watch_state_notifier.dart';
import 'download_resolution.dart';
import 'ids.dart';
import 'library_filter_result.dart';
@@ -475,6 +476,17 @@ abstract class MediaServerClient {
/// one and returns a fixed 0.9.
double get watchedThreshold;
/// Whether a playback-stopped report past [watchedThreshold] already marks
/// the item played server-side. When true, in-player auto-scrobble must NOT
/// also call [markWatched]: the server marks it played from the stop report,
/// and the extra `/UserPlayedItems` toggle double-scrobbles through
/// integrations that watch both the played-state change and the
/// playback-stop — e.g. Jellyfin's Trakt plugin fires once on `TogglePlayed`
/// and again on `PlayedToCompletion` (#1287). Plex returns false: its
/// timeline stop doesn't reliably mark watched without an active play
/// session, so the explicit call is still required.
bool get marksWatchedOnPlaybackStopped;
/// First playback signal for [itemId]. Plex sends a `/:/timeline?state=playing`
/// heartbeat; Jellyfin opens a `/Sessions/Playing` session row. Subsequent
/// ticks must call [reportPlaybackProgress] (Jellyfin distinguishes session
@@ -573,6 +585,22 @@ extension MediaServerClientScope on MediaServerClient {
ScopedMediaServerClient(:final scopedServerId) => scopedServerId,
_ => serverId,
};
/// Mark [item] watched because it crossed [watchedThreshold] during playback,
/// when a playback-stopped report is/was also sent for the same playback.
/// Backends that mark played from the stop report
/// ([marksWatchedOnPlaybackStopped]) only emit the local watch event —
/// issuing [markWatched] too would double-scrobble via the Jellyfin Trakt
/// plugin (#1287). The local event still keeps the UI and Plezy's own Trakt
/// sync (which key on `watched` events, not progress) in sync; the stop
/// report syncs the server.
Future<void> markWatchedFromPlaybackStop(MediaItem item) async {
if (marksWatchedOnPlaybackStopped) {
WatchStateNotifier().notifyWatched(item: item, isNowWatched: true, cacheServerId: cacheServerId);
} else {
await markWatched(item);
}
}
}
/// Cache-aware fetch helpers shared by both backends so the offline-first /
+5 -1
View File
@@ -208,7 +208,11 @@ class ExternalPlayerService {
if (position.inMilliseconds / duration.inMilliseconds >= client.watchedThreshold) {
try {
await client.markWatched(metadata);
// reportPlaybackStopped above marks the item played on backends that
// support it (Jellyfin); markWatchedFromPlaybackStop then only emits the
// local watch event there to avoid double-scrobbling via the Trakt
// plugin (#1287). Plex still issues the explicit server call.
await client.markWatchedFromPlaybackStop(metadata);
unawaited(TrackerCoordinator.instance.markWatched(metadata, client));
} catch (e) {
appLogger.w('Failed to mark external playback watched for ${metadata.id}', error: e);
+7
View File
@@ -242,6 +242,13 @@ class JellyfinClient
@override
double get watchedThreshold => 0.9;
/// Jellyfin marks an item played from `/Sessions/Playing/Stopped` itself
/// (server `MaxResumePct`, default 90%), so the in-player auto-scrobble must
/// not also `POST /UserPlayedItems` — that double-scrobbles via the Trakt
/// plugin (#1287). Manual mark-watched still hits `/UserPlayedItems`.
@override
bool get marksWatchedOnPlaybackStopped => true;
@override
void close() => _http.close();
+5 -2
View File
@@ -583,9 +583,12 @@ class OfflineWatchSyncService extends ChangeNotifier {
);
}
// If progress exceeded threshold, also mark as watched.
// If progress exceeded threshold, also mark as watched. On backends
// that mark played from the stopped report above (Jellyfin) this only
// emits the local watch event — an explicit markWatched would
// double-scrobble via the Trakt plugin (#1287).
if (action.shouldMarkWatched) {
await client.markWatched(item);
await client.markWatchedFromPlaybackStop(item);
await TrackerCoordinator.instance.markWatched(item, client);
}
break;
+6 -3
View File
@@ -328,9 +328,12 @@ class PlaybackProgressTracker {
if (percent >= threshold) {
_scrobbled = true;
try {
// The neutral markWatched(MediaItem) call emits the watched event
// through WatchStateNotifier itself, so no extra notify here.
await c.markWatched(metadata);
// Backends that mark the item played from the playback-stopped report
// (Jellyfin) only emit the local watch event here — an explicit
// markWatched would double-scrobble via the Trakt plugin (#1287).
// Plex still issues the server call. Either path emits the watched
// event through WatchStateNotifier, so no extra notify is needed.
await c.markWatchedFromPlaybackStop(metadata);
appLogger.d(
'Scrobbled ${metadata.id} (${(percent * 100).toStringAsFixed(0)}% >= ${(threshold * 100).toStringAsFixed(0)}%)',
);
+6
View File
@@ -4058,6 +4058,12 @@ class PlexClient
@override
double get watchedThreshold => watchedThresholdPercent / 100.0;
/// Plex's `/:/timeline?state=stopped` doesn't reliably mark watched without
/// an active play session, so the in-player auto-scrobble still issues the
/// explicit `markWatched` (`/:/scrobble`). See [marksWatchedOnPlaybackStopped].
@override
bool get marksWatchedOnPlaybackStopped => false;
@override
Map<String, String> get streamHeaders => Map.unmodifiable(config.headers);
@@ -13,20 +13,29 @@ import 'package:plezy/services/multi_server_manager.dart';
import 'package:plezy/services/offline_watch_sync_service.dart';
class _RecordingClient implements MediaServerClient {
_RecordingClient({this.backend = MediaBackend.plex});
bool failStart = false;
bool failStop = false;
final started = <({int positionMs, int? durationMs})>[];
final stopped = <({int positionMs, int? durationMs})>[];
final watched = <String>[];
@override
ServerId get serverId => ServerId('srv');
@override
MediaBackend get backend => MediaBackend.plex;
final MediaBackend backend;
@override
double get watchedThreshold => 0.9;
// Mirror the real clients: Jellyfin marks played from the stopped report, so
// the external-player completion path emits only the local watch event
// (#1287); Plex needs the explicit markWatched.
@override
bool get marksWatchedOnPlaybackStopped => backend == MediaBackend.jellyfin;
@override
Future<void> reportPlaybackStarted({
required String itemId,
@@ -55,6 +64,11 @@ class _RecordingClient implements MediaServerClient {
if (failStop) throw StateError('stop failed');
}
@override
Future<void> markWatched(MediaItem item) async {
watched.add(item.id);
}
@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
@@ -138,5 +152,24 @@ void main() {
expect(client.started, [(positionMs: 100000, durationMs: 100000)]);
expect(client.stopped, [(positionMs: 100000, durationMs: 100000)]);
expect(client.watched, ['item-1']);
});
test('Android external completion on Jellyfin marks watched via the stop report, not markWatched (#1287)', () async {
final client = _RecordingClient(backend: MediaBackend.jellyfin);
await ExternalPlayerService.reportAndroidExternalProgressForTesting(
positionMs: null,
durationMs: 100000,
playbackCompleted: true,
metadata: _item(durationMs: 100000),
client: client,
);
// The stopped report at full duration marks it played server-side…
expect(client.stopped, [(positionMs: 100000, durationMs: 100000)]);
// …so the explicit markWatched is skipped — issuing it would double-scrobble
// through the Jellyfin Trakt plugin.
expect(client.watched, isEmpty);
});
}
@@ -77,6 +77,12 @@ class _RecordingMediaClient implements MediaServerClient {
@override
double get watchedThreshold => 0.9;
// Mirror the real clients: Jellyfin marks played from the stopped report, so
// the auto-scrobble path emits only the local watch event (#1287); Plex needs
// the explicit markWatched.
@override
bool get marksWatchedOnPlaybackStopped => backend == MediaBackend.jellyfin;
@override
void close() {}
@@ -407,6 +413,29 @@ void main() {
expect(client.watched, ['42']);
expect(await svc.getPendingSyncCount(), 0);
});
test('completed Jellyfin offline progress marks watched via the stop report, not markWatched (#1287)', () async {
final (svc: svc, db: db, mgr: mgr) = _makeService();
addTearDown(() async {
svc.dispose();
mgr.dispose();
await db.close();
});
final client = _RecordingMediaClient(serverId: ServerId('srv'), backend: MediaBackend.jellyfin);
mgr.debugRegisterClientForTesting(client);
await svc.queueProgressUpdate(serverId: ServerId('srv'), itemId: '42', viewOffset: 95000, duration: 100000);
await svc.syncPendingItems();
// The stopped report at full duration marks the item played server-side…
expect(client.stopped, hasLength(1));
expect(client.stopped.single.positionMs, 100000);
// …so the offline replay must NOT also call markWatched — that would
// double-scrobble through the Jellyfin Trakt plugin.
expect(client.watched, isEmpty);
expect(await svc.getPendingSyncCount(), 0);
});
});
// ============================================================
@@ -116,6 +116,11 @@ class _FakePlexClient implements PlexClient {
@override
double get watchedThreshold => thresholdPercent / 100.0;
/// Plex relies on the explicit markWatched call (no auto-mark from the stop
/// report), so the scrobble path hits [markWatched].
@override
bool get marksWatchedOnPlaybackStopped => false;
/// (ratingKey, time, state, duration) tuples for every updateProgress call.
final List<({String ratingKey, int time, String state, int? duration})> updateProgressCalls = [];
@@ -264,6 +269,17 @@ class _DelayedStartClient extends _FakePlexClient {
}
}
/// Jellyfin-style backend: the playback-stopped report marks the item played
/// server-side, so the in-player scrobble path must emit only the local watch
/// event and skip the explicit server mark (#1287).
class _StopMarksWatchedClient extends _FakePlexClient {
@override
bool get marksWatchedOnPlaybackStopped => true;
@override
ServerId get serverId => ServerId('srv');
}
const Object _defaultServerId = Object();
MediaItem _meta({String ratingKey = '42', Object? serverId = _defaultServerId, String? type = 'movie'}) => MediaItem(
@@ -659,6 +675,35 @@ void main() {
expect(client.markWatchedCalls, ['42']);
});
test('backend that marks watched on stop skips the explicit server mark (#1287)', () async {
// Jellyfin: /Sessions/Playing/Stopped marks the item played server-side,
// so an explicit markWatched here would double-scrobble via the Trakt
// plugin. The local watch event must still fire (UI + Plezy's own Trakt
// sync, which key on `watched` events, not progress).
final client = _StopMarksWatchedClient();
final player = _FakePlayer(position: const Duration(seconds: 95), duration: const Duration(seconds: 100));
final tracker = PlaybackProgressTracker(
client: client,
metadata: _meta(ratingKey: '42'),
player: player,
isOffline: false,
);
addTearDown(tracker.dispose);
final watched = <WatchStateEvent>[];
final sub = WatchStateNotifier()
.forItem('42')
.where((e) => e.changeType == WatchStateChangeType.watched)
.listen(watched.add);
addTearDown(sub.cancel);
await tracker.sendProgress('stopped');
await Future<void>.delayed(Duration.zero);
expect(client.markWatchedCalls, isEmpty);
expect(watched, hasLength(1));
});
test('respects a custom server threshold (e.g. 80%)', () async {
// 81% >= 80%, but < 90% default.
final client = _FakePlexClient(thresholdPercent: 80);
@@ -964,6 +1009,9 @@ class _ScrobblePreciseClient implements PlexClient {
@override
double get watchedThreshold => thresholdPercent / 100.0;
@override
bool get marksWatchedOnPlaybackStopped => false;
bool failScrobbleFirstTime;
int markWatchedAttempts = 0;
int markWatchedSuccesses = 0;