From be9197eda6f34824fa3ddf30344f09e4ac8101e2 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:46:33 +0200 Subject: [PATCH] fix(playback): serve playback metadata from fresh cache instead of refetching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Videos take noticeably long to start. On Plex, tapping Play refetched /library/metadata/{id} network-first even though the detail screen wrote a strict superset of that exact payload under the same cache key seconds earlier. On Jellyfin/Emby, playback start issued a full-detail item GET before the PlaybackInfo negotiation, and the video controls issued the same heavy GET again while the stream was opening (#1784) — the most expensive queries a small home server serves, paid two to three times per start. Add ApiCache.getIfFresh over the existing cachedAt column and serve rows younger than playbackMetadataCacheFreshness (5 min) without a round trip. Plex guards with the strict stream-detail check (_plexMetadataHasStreamDetail) so a thin row written by getPlaybackExtras' lean fetch still goes to the network; Jellyfin's row has a single full-shape writer, so fetchPlaybackBundle and fetchPlaybackExtras share the new fetchItemFreshCacheFirst. Any miss, stale, thin, or malformed row falls through to the unchanged network-first path, and offline behavior is untouched. --- lib/media/media_server_client.dart | 6 + lib/services/api_cache.dart | 11 ++ .../jellyfin_client/parts/browse.dart | 27 ++++ .../jellyfin_client/parts/playback.dart | 8 +- lib/services/plex_client.dart | 28 ++++ .../jellyfin_playback_bundle_test.dart | 95 +++++++++++++ test/services/plex_api_cache_test.dart | 36 +++++ .../plex_playback_data_request_test.dart | 126 ++++++++++++++++++ 8 files changed, 335 insertions(+), 2 deletions(-) diff --git a/lib/media/media_server_client.dart b/lib/media/media_server_client.dart index dff5384f4..95781e15e 100644 --- a/lib/media/media_server_client.dart +++ b/lib/media/media_server_client.dart @@ -894,6 +894,12 @@ abstract interface class MediaDeletionPermissionClient { Future fetchDeletePermission(MediaItem item); } +/// Bounds how old a cached metadata row may be to be served without a network +/// round trip on playback start. Sized to cover the detail-screen-visit → +/// play-tap gap while keeping stale-file risk (replaced/deleted media parts) +/// negligible. +const Duration playbackMetadataCacheFreshness = Duration(minutes: 5); + /// Cache-aware fetch helpers shared by both backends so the offline-first / /// network-then-cache pattern lives in one place. /// diff --git a/lib/services/api_cache.dart b/lib/services/api_cache.dart index 721cef9ef..bd52f55e4 100644 --- a/lib/services/api_cache.dart +++ b/lib/services/api_cache.dart @@ -127,6 +127,17 @@ abstract class ApiCache { return null; } + /// Like [get], but serves the row only when it was written within [maxAge] + /// of now; missing or older rows return null. Freshness gate for callers + /// that use the cache as a latency optimization (skip a redundant network + /// round trip) rather than as an offline fallback. + Future?> getIfFresh(ServerId serverId, String endpoint, {required Duration maxAge}) async { + final key = _buildKey(serverId, endpoint); + final result = await (_db.select(_db.apiCache)..where((t) => t.cacheKey.equals(key))).getSingleOrNull(); + if (result == null || DateTime.now().difference(result.cachedAt) > maxAge) return null; + return await tryIsolateRun(() => jsonDecode(result.data) as Map); + } + Future put(ServerId serverId, String endpoint, Map data) async { final key = _buildKey(serverId, endpoint); final encoded = await tryIsolateRun(() => jsonEncode(data)); diff --git a/lib/services/jellyfin_client/parts/browse.dart b/lib/services/jellyfin_client/parts/browse.dart index c93610a17..00fe494a7 100644 --- a/lib/services/jellyfin_client/parts/browse.dart +++ b/lib/services/jellyfin_client/parts/browse.dart @@ -728,6 +728,33 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals { return request; } + /// [fetchItem], but a fresh cached row (≤ [playbackMetadataCacheFreshness] + /// old) short-circuits the network round trip. + /// + /// The single writer for this endpoint's row is [_fetchItemOnce] with the + /// full [_detailFields] shape, so a fresh row always carries `MediaSources`, + /// `Chapters` and `Trickplay`, and the raw DTO survives [_mapItem] — the + /// offline path below already relies on that. Playback start and the + /// controls' extras loader both re-request this exact payload seconds after + /// the detail screen fetched it (#1784 documents the duplicate-fetch cost), + /// which is what serving the fresh row removes. Purely an optimization + /// layer: any miss, staleness, cache error or mapping failure falls through + /// to [fetchItem] unchanged — including offline mode, where [fetchItem] + /// reads the cache without a freshness bound. + Future fetchItemFreshCacheFirst(String id) async { + final endpoint = '/Users/${_segment(connection.userId)}/Items/${_segment(id)}'; + try { + final cached = await cache.getIfFresh(ServerId(cacheServerId), endpoint, maxAge: playbackMetadataCacheFreshness); + if (cached != null) { + final item = _mapItem(cached); + if (item != null) return item; + } + } catch (e, st) { + appLogger.w('JellyfinClient.fetchItemFreshCacheFirst cache read failed', error: e, stackTrace: st); + } + return fetchItem(id); + } + Future _fetchItemOnce(String id) async { final endpoint = '/Users/${_segment(connection.userId)}/Items/${_segment(id)}'; // Contract: diff --git a/lib/services/jellyfin_client/parts/playback.dart b/lib/services/jellyfin_client/parts/playback.dart index 9e77f83f2..bcc2a36e0 100644 --- a/lib/services/jellyfin_client/parts/playback.dart +++ b/lib/services/jellyfin_client/parts/playback.dart @@ -10,6 +10,10 @@ bool _canUseJellyfinStaticStreamFallback(Object error) { } mixin _JellyfinPlaybackMethods on _JellyfinClientInternals { + // Implemented by _JellyfinBrowseMethods (cross-part call, same pattern as + // _JellyfinImageDownloadMethods' redeclarations). + Future fetchItemFreshCacheFirst(String id); + /// Backend-neutral [PlaybackExtras] for [itemId]. Both dialects expose /// chapters at the item level (`raw['Chapters']`), while only Jellyfin exposes /// native skip segments through `/MediaSegments/{itemId}`. Segment loading is @@ -22,7 +26,7 @@ mixin _JellyfinPlaybackMethods on _JellyfinClientInternals { bool forceChapterFallback = false, bool forceRefresh = false, }) async { - final item = await fetchItem(itemId); + final item = await fetchItemFreshCacheFirst(itemId); final markers = item == null ? const [] : await _fetchMediaSegmentMarkers(itemId); return jellyfinPlaybackExtrasFromRaw( item?.raw, @@ -567,7 +571,7 @@ mixin _JellyfinPlaybackMethods on _JellyfinClientInternals { String? sourceId, String? preferredSignature, }) async { - final item = await fetchItem(itemId); + final item = await fetchItemFreshCacheFirst(itemId); final raw = item?.raw; if (raw is! Map) return null; final sources = raw['MediaSources']; diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index 5e216fe50..e90f7d564 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -1864,6 +1864,34 @@ class PlexClient String? selectedMediaSourceId, String? preferredVersionSignature, }) async { + // Fresh-cache-first: the detail screen writes a strict superset of this + // query shape (includeChapters+includeMarkers+includeOnDeck+checkFiles+ + // includeStreams, [getMetadataWithImagesAndOnDeck]) under the same key + // seconds before a typical play tap, so a fresh stream-rich row makes the + // network round trip redundant on the tap-to-first-frame path. Any miss, + // staleness, thin row (getPlaybackExtras' lean fetch overwrites the shared + // row without includeStreams/checkFiles), or shape failure falls through + // to the network-first fetch below unchanged. + final freshRow = await cache.getIfFresh( + ServerId(cacheServerId), + '/library/metadata/$ratingKey', + maxAge: playbackMetadataCacheFreshness, + ); + if (freshRow != null) { + try { + final cachedMetadataJson = _validatedPlaybackMetadataJson(freshRow); + if (cachedMetadataJson != null && _plexMetadataHasStreamDetail(cachedMetadataJson)) { + return parseVideoPlaybackDataFromJson( + cachedMetadataJson, + mediaIndex: mediaIndex, + selectedMediaSourceId: selectedMediaSourceId, + preferredVersionSignature: preferredVersionSignature, + ); + } + } on FormatException { + // Malformed cached row: the network fetch below overwrites it. + } + } final data = await fetchWithCacheFallback>( cacheKey: '/library/metadata/$ratingKey', // checkFiles=1 populates Part.accessible/exists so we can skip diff --git a/test/services/jellyfin_playback_bundle_test.dart b/test/services/jellyfin_playback_bundle_test.dart index 2e0d95f56..db6af54a1 100644 --- a/test/services/jellyfin_playback_bundle_test.dart +++ b/test/services/jellyfin_playback_bundle_test.dart @@ -1,11 +1,13 @@ import 'dart:convert'; +import 'package:drift/drift.dart' show Value; import 'package:drift/native.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:plezy/connection/connection.dart'; import 'package:plezy/database/app_database.dart'; +import 'package:plezy/media/media_server_client.dart'; import 'package:plezy/services/jellyfin_api_cache.dart'; import 'package:plezy/services/jellyfin_client.dart'; import 'package:plezy/services/plex_api_cache.dart'; @@ -19,6 +21,15 @@ JellyfinConnection _conn() => testJellyfinConnection( createdAt: DateTime.fromMillisecondsSinceEpoch(0), ); +/// Rewind every cached row's `cachedAt` past the fresh-cache horizon so +/// `fetchItemFreshCacheFirst` misses while the row itself stays readable for +/// the offline/transport fallbacks. +Future _expireCachedRows(AppDatabase db) async { + await db + .update(db.apiCache) + .write(ApiCacheCompanion(cachedAt: Value(DateTime.now().subtract(playbackMetadataCacheFreshness * 2)))); +} + /// Pin the playback bundle accessor for [PlaybackInitializationService]. /// The bundle replaces the previous pattern of reaching into /// `MediaItem.raw['MediaSources']` / `raw['Chapters']` from outside the @@ -249,6 +260,11 @@ void main() { // Adjacency discovery primes the per-item row. expect(await client.fetchItem('item-9'), isNotNull); + // Age the row past the fresh-cache horizon so the bundle actually + // attempts the network — the fresh-cache-first layer would otherwise + // serve it outright and never reach the fallback under test. + await _expireCachedRows(db); + // A pure transport failure (wrapped by the HTTP layer into a // status-less MediaServerHttpException) must fall back to that row // instead of failing the transition. @@ -258,4 +274,83 @@ void main() { expect(bundle!.selectedSourceId, 'src-9'); }); }); + + // Pin the fresh-cache-first optimization: playback start and the controls' + // extras loader re-request the exact full-detail payload the detail screen + // fetched seconds earlier (#1784), so a fresh cached row must serve them + // without an item GET — while stale rows keep the network path untouched. + group('fresh-cache-first playback metadata', () { + String itemBody({required String container}) => jsonEncode({ + 'Id': 'item-fresh', + 'Type': 'Movie', + 'MediaSources': [ + {'Id': 'src-fresh', 'Container': container}, + ], + 'Chapters': [ + {'Name': 'Intro', 'StartPositionTicks': 0}, + {'Name': 'Act 1', 'StartPositionTicks': 6000000000}, + ], + }); + + // Counts full-detail item GETs and answers `{}` to everything else (the + // best-effort `/MediaSegments/{id}` probe tolerates an empty body). + (JellyfinClient, int Function()) countingClient(String Function() body) { + var itemGets = 0; + final client = testJellyfinClient( + connection: _conn(), + handler: (request) async { + if (request.url.path.contains('/Users/user-1/Items/')) { + itemGets++; + return http.Response(body(), 200, headers: {'content-type': 'application/json'}); + } + return http.Response('{}', 200, headers: {'content-type': 'application/json'}); + }, + ); + return (client, () => itemGets); + } + + test('fetchPlaybackBundle rides the row a fresh fetchItem wrote instead of re-fetching', () async { + final (client, itemGets) = countingClient(() => itemBody(container: 'mkv')); + addTearDown(client.close); + + // Detail-screen visit primes the row (fetchItem is the single writer, + // full _detailFields shape). + expect(await client.fetchItem('item-fresh'), isNotNull); + expect(itemGets(), 1); + + final bundle = await client.fetchPlaybackBundle('item-fresh'); + expect(bundle, isNotNull); + expect(bundle!.selectedSourceId, 'src-fresh'); + expect(bundle.container, 'mkv'); + expect(itemGets(), 1, reason: 'a fresh cached row must serve playback start without a second full-detail GET'); + }); + + test('stale row falls through to the network item GET exactly as before', () async { + var container = 'mkv'; + final (client, itemGets) = countingClient(() => itemBody(container: container)); + addTearDown(client.close); + + expect(await client.fetchItem('item-fresh'), isNotNull); + expect(itemGets(), 1); + + await _expireCachedRows(db); + container = 'mp4'; + + final bundle = await client.fetchPlaybackBundle('item-fresh'); + expect(itemGets(), 2, reason: 'a stale row must not short-circuit the network fetch'); + expect(bundle!.container, 'mp4', reason: 'the bundle must reflect the network payload, not the stale row'); + }); + + test('fetchPlaybackExtras served from a fresh row issues no item GET', () async { + final (client, itemGets) = countingClient(() => itemBody(container: 'mkv')); + addTearDown(client.close); + + expect(await client.fetchItem('item-fresh'), isNotNull); + expect(itemGets(), 1); + + final extras = await client.fetchPlaybackExtras('item-fresh'); + expect(extras.chapters, hasLength(2)); + expect(itemGets(), 1, reason: "the controls' extras load must be served by the row playback start just used"); + }); + }); } diff --git a/test/services/plex_api_cache_test.dart b/test/services/plex_api_cache_test.dart index 46d673b51..0e8925600 100644 --- a/test/services/plex_api_cache_test.dart +++ b/test/services/plex_api_cache_test.dart @@ -188,6 +188,42 @@ void main() { }); }); + group('getIfFresh', () { + Future backdate(String cacheKey, Duration age) async { + await (db.update(db.apiCache)..where((t) => t.cacheKey.equals(cacheKey))).write( + ApiCacheCompanion(cachedAt: Value(DateTime.now().subtract(age))), + ); + } + + test('returns null for an unknown key', () async { + expect( + await cache.getIfFresh(ServerId('srv'), '/library/metadata/1', maxAge: const Duration(minutes: 5)), + isNull, + ); + }); + + test('decodes a row written within maxAge', () async { + final payload = mediaContainer(ratingKey: '1', title: 'Fresh'); + await cache.put(ServerId('srv'), '/library/metadata/1', payload); + + final hit = await cache.getIfFresh(ServerId('srv'), '/library/metadata/1', maxAge: const Duration(minutes: 5)); + expect(hit, equals(payload)); + }); + + test('returns null for a row older than maxAge while get still serves it', () async { + await cache.put(ServerId('srv'), '/library/metadata/1', mediaContainer(ratingKey: '1', title: 'Old')); + await backdate('srv:/library/metadata/1', const Duration(minutes: 10)); + + expect( + await cache.getIfFresh(ServerId('srv'), '/library/metadata/1', maxAge: const Duration(minutes: 5)), + isNull, + ); + // Staleness only gates the freshness-checked read; the offline-fallback + // read must keep serving the row. + expect(await cache.get(ServerId('srv'), '/library/metadata/1'), isNotNull); + }); + }); + group('deletion', () { test('deleteForServer wipes only the targeted serverId', () async { await cache.put(ServerId('srv-a'), '/library/metadata/1', mediaContainer(ratingKey: '1')); diff --git a/test/services/plex_playback_data_request_test.dart b/test/services/plex_playback_data_request_test.dart index 05460faa2..084bf33cb 100644 --- a/test/services/plex_playback_data_request_test.dart +++ b/test/services/plex_playback_data_request_test.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import 'package:plezy/media/ids.dart'; +import 'package:drift/drift.dart' show Value; import 'package:drift/native.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; @@ -9,6 +10,7 @@ import 'package:plezy/exceptions/media_server_exceptions.dart'; import 'package:plezy/media/media_backend.dart'; import 'package:plezy/media/media_kind.dart'; +import 'package:plezy/media/media_server_client.dart'; import 'package:plezy/media/media_source_info.dart'; import 'package:plezy/mpv/mpv.dart'; import 'package:plezy/models/transcode_quality_preset.dart'; @@ -255,6 +257,130 @@ void main() { expect(data.mediaInfo?.subtitleTracks.single.selected, isTrue); }); + group('fresh-cache-first playback metadata', () { + // Same scope [PlexClient.getVideoPlaybackData] resolves via + // `ServerId(cacheServerId)` — the fixture's default profile scope. + final cacheScope = buildPlexProfileScopeId( + serverId: ServerId('server-id'), + profileId: 'test-profile', + ).cacheServerId; + const endpoint = '/library/metadata/42'; + + // The shape the detail screen caches: includeStreams + checkFiles keys + // (`Stream`/`exists`/`accessible`) present on the part. + Map richPlaybackPayload() => { + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': '42', + 'type': 'movie', + 'title': 'Movie', + 'Media': [ + { + 'id': 7, + 'container': 'mkv', + 'Part': [ + { + 'id': 99, + 'key': '/library/parts/99/file.mkv', + 'exists': true, + 'accessible': true, + 'Stream': [ + {'streamType': 1, 'id': 300, 'codec': 'h264'}, + {'streamType': 3, 'id': 401, 'index': 1, 'codec': 'ass', 'languageCode': 'eng', 'selected': true}, + ], + }, + ], + }, + ], + }, + ], + }, + }; + + PlexClient makeCountingClient(List requests) => makeClient((request) async { + requests.add(request.url); + if (request.url.path != endpoint) return http.Response('not found', 404); + return http.Response(jsonEncode(richPlaybackPayload()), 200, headers: {'content-type': 'application/json'}); + }); + + test('fresh stream-rich cached row is served with zero network requests', () async { + await PlexApiCache.instance.put(cacheScope, endpoint, richPlaybackPayload()); + final requests = []; + final client = makeCountingClient(requests); + addTearDown(client.close); + + final data = await client.getVideoPlaybackData('42'); + + expect(requests, isEmpty); + expect(data.hasValidVideoUrl, isTrue); + expect(data.videoUrl, contains('/library/parts/99/file.mkv')); + expect(data.mediaInfo?.subtitleTracks.single.id, 401); + }); + + test('fresh but stream-less cached row still fetches from the network', () async { + // getPlaybackExtras' lean fetch overwrites the shared row without + // includeStreams/checkFiles; that shape must never satisfy playback. + await PlexApiCache.instance.put(cacheScope, endpoint, { + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': '42', + 'type': 'movie', + 'title': 'Movie', + 'Media': [ + { + 'id': 7, + 'container': 'mkv', + 'Part': [ + {'id': 99, 'key': '/library/parts/99/file.mkv'}, + ], + }, + ], + }, + ], + }, + }); + final requests = []; + final client = makeCountingClient(requests); + addTearDown(client.close); + + final data = await client.getVideoPlaybackData('42'); + + expect(requests, hasLength(1)); + expect(requests.single.queryParameters['includeStreams'], '1'); + expect(data.mediaInfo?.subtitleTracks.single.id, 401); + }); + + test('cached row older than the freshness window fetches from the network', () async { + await PlexApiCache.instance.put(cacheScope, endpoint, richPlaybackPayload()); + await (db.update(db.apiCache)..where((t) => t.cacheKey.equals('$cacheScope:$endpoint'))).write( + ApiCacheCompanion( + cachedAt: Value(DateTime.now().subtract(playbackMetadataCacheFreshness + const Duration(seconds: 1))), + ), + ); + final requests = []; + final client = makeCountingClient(requests); + addTearDown(client.close); + + final data = await client.getVideoPlaybackData('42'); + + expect(requests, hasLength(1)); + expect(data.hasValidVideoUrl, isTrue); + }); + + test('cache miss fetches from the network', () async { + final requests = []; + final client = makeCountingClient(requests); + addTearDown(client.close); + + final data = await client.getVideoPlaybackData('42'); + + expect(requests, hasLength(1)); + expect(data.hasValidVideoUrl, isTrue); + }); + }); + test('transcode initialization burns the selected embedded stream and sidecars only the external file', () async { final requests = []; final client = makeClient((request) async {