chore(player): report marker counts when loading playback extras

The extras loader logged only the chapter count, so a user report of
"auto skip never fires" could not be told apart from "the server has no
intro marker for this item" — the two need opposite fixes. Log the
marker count and types on all three load paths, including the cache-only
one that previously logged nothing at all.

Drop PlexVideoPlaybackData.markers while here: the playback-start parse
filled it on every item and no caller ever read it, because the player
controls fetch their own PlaybackExtras.

Document why getPlaybackExtras may serve the shared metadata cache row
without a freshness check: getPlaybackInitialization refreshes that row
network-first before the controls mount. That ordering is what makes
cache-first correct, and nothing said so.
This commit is contained in:
edde746
2026-08-02 04:55:54 +02:00
parent 957711a650
commit f78f65faf5
4 changed files with 35 additions and 10 deletions
@@ -10,8 +10,6 @@ class PlexVideoPlaybackData {
final List<MediaVersion> availableVersions;
final List<MediaMarker> markers;
final int selectedMediaIndex;
final int selectedPartIndex;
@@ -20,7 +18,6 @@ class PlexVideoPlaybackData {
required this.videoUrl,
required this.mediaInfo,
required this.availableVersions,
this.markers = const [],
this.selectedMediaIndex = 0,
this.selectedPartIndex = 0,
});
+17 -2
View File
@@ -1665,8 +1665,23 @@ class PlexClient
}
}
/// Get chapters and markers from cached metadata or fetch if needed
/// Uses same cache key as other metadata methods for consistency
/// Chapters and markers for [ratingKey], from the shared
/// `/library/metadata/{id}` cache row.
///
/// Cache-first is safe here because of an ordering contract, not because
/// markers are static: in the normal online player flow
/// [getPlaybackInitialization] runs [getVideoPlaybackData] — a
/// network-first read of this same cache key with a superset of the query
/// params — before the controls mount and load extras, so the row this
/// serves was refreshed seconds earlier. Offline, and when that read fell
/// back to cache, the row is as old as the cache; a caller that needs the
/// current server state (e.g. after a PMS intro-detection pass finished)
/// must pass [forceRefresh].
///
/// The network call here deliberately stays lean (no `checkFiles` /
/// `includeStreams`) and runs only on a cache miss or [forceRefresh], so it
/// rarely overwrites the shared row with a payload thin enough to force the
/// re-fetch in [_fetchFileInfo].
Future<PlaybackExtras> getPlaybackExtras(
String ratingKey, {
String? introPattern,
-2
View File
@@ -74,7 +74,6 @@ PlexVideoPlaybackData parsePlexVideoPlaybackDataFromJson(
List<MediaVersion> availableVersions = [];
var selectedMediaIndex = 0;
var selectedPartIndex = 0;
final markers = plexMarkersFromCacheJson(metadataJson);
if (metadataJson != null) {
final mediaList = _mapList(metadataJson['Media']);
@@ -148,7 +147,6 @@ PlexVideoPlaybackData parsePlexVideoPlaybackDataFromJson(
videoUrl: videoUrl,
mediaInfo: mediaInfo,
availableVersions: availableVersions,
markers: markers,
selectedMediaIndex: selectedMediaIndex,
selectedPartIndex: selectedPartIndex,
);
@@ -30,7 +30,7 @@ class VideoControlsPlaybackExtrasLoader {
forceChapterFallback: settings.read(SettingsService.forceSkipMarkerFallback),
forceRefresh: forceRefresh,
);
appLogger.d('_loadPlaybackExtras: got ${extras.chapters.length} chapters');
appLogger.d('_loadPlaybackExtras: got ${_describe(extras)}');
return extras;
} catch (e, stack) {
appLogger.d('_loadPlaybackExtras: network path failed, trying cache fallback');
@@ -43,7 +43,7 @@ class VideoControlsPlaybackExtrasLoader {
forceChapterFallback: settings.read(SettingsService.forceSkipMarkerFallback),
);
if (extras != null) {
appLogger.d('_loadPlaybackExtras: loaded ${extras.chapters.length} chapters from cache');
appLogger.d('_loadPlaybackExtras: loaded ${_describe(extras)} from cache');
return extras;
}
} catch (cacheError) {
@@ -61,7 +61,7 @@ class VideoControlsPlaybackExtrasLoader {
}
try {
final settings = await SettingsService.getInstance();
return CachedPlaybackMetadataService.fetchPlaybackExtras(
final extras = await CachedPlaybackMetadataService.fetchPlaybackExtras(
backend: metadata.backend,
cacheServerId: cacheServerId,
itemId: metadata.id,
@@ -69,12 +69,27 @@ class VideoControlsPlaybackExtrasLoader {
creditsPattern: settings.read(SettingsService.creditsPattern),
forceChapterFallback: settings.read(SettingsService.forceSkipMarkerFallback),
);
appLogger.d(
extras == null
? '_loadPlaybackExtras: no cached extras for ${metadata.id}'
: '_loadPlaybackExtras: cache-only ${_describe(extras)}',
);
return extras;
} catch (e) {
appLogger.d('_loadPlaybackExtras: cache-only path failed', error: e);
return null;
}
}
/// Marker counts are the difference between "the server has no intro data"
/// and "auto-skip never fired", which is otherwise indistinguishable in a
/// user-supplied log.
static String _describe(PlaybackExtras extras) {
final markerTypes = extras.markers.map((m) => m.type).join(',');
return '${extras.chapters.length} chapters, ${extras.markers.length} markers'
'${markerTypes.isEmpty ? '' : ' ($markerTypes)'}';
}
Future<String?> _resolveCacheServerId() async {
final serverId = metadata.serverId;
if (serverId == null) return null;