From f78f65faf5a47339242b168c524f29b05cd8cd10 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 2 Aug 2026 04:55:54 +0200 Subject: [PATCH] chore(player): report marker counts when loading playback extras MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- lib/models/plex/plex_video_playback_data.dart | 3 --- lib/services/plex_client.dart | 19 +++++++++++++++-- lib/services/plex_playback_mapper.dart | 2 -- .../playback_extras_loader.dart | 21 ++++++++++++++++--- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/lib/models/plex/plex_video_playback_data.dart b/lib/models/plex/plex_video_playback_data.dart index 587fb1f3..255c6976 100644 --- a/lib/models/plex/plex_video_playback_data.dart +++ b/lib/models/plex/plex_video_playback_data.dart @@ -10,8 +10,6 @@ class PlexVideoPlaybackData { final List availableVersions; - final List 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, }); diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index a83ef337..c2375350 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -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 getPlaybackExtras( String ratingKey, { String? introPattern, diff --git a/lib/services/plex_playback_mapper.dart b/lib/services/plex_playback_mapper.dart index d68bb4f6..93951951 100644 --- a/lib/services/plex_playback_mapper.dart +++ b/lib/services/plex_playback_mapper.dart @@ -74,7 +74,6 @@ PlexVideoPlaybackData parsePlexVideoPlaybackDataFromJson( List 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, ); diff --git a/lib/widgets/video_controls/playback_extras_loader.dart b/lib/widgets/video_controls/playback_extras_loader.dart index d732305e..535f2961 100644 --- a/lib/widgets/video_controls/playback_extras_loader.dart +++ b/lib/widgets/video_controls/playback_extras_loader.dart @@ -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 _resolveCacheServerId() async { final serverId = metadata.serverId; if (serverId == null) return null;