From 2f47ed23cd56dcce8de159f3028f9e149e09df93 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Thu, 20 Aug 2026 02:01:12 +0200 Subject: [PATCH] chore: dedupe resolution display labels and fold single-use helpers Two private formatters mapped the canonical server resolution label to display text (media_quality_labels, media_version) with edge-case drift, and several single-consumer helpers added indirection without callers: - add shared resolutionDisplayLabel in resolution_label.dart and migrate MediaVersion.displayLabel and the quality-label builder to it - drop the resolutionLabelFromHeight compat re-export from jellyfin_mappers (no remaining consumers) and its stale doc note - delete TraktCatalogSource.membershipKeysFor, byte-identical to the CatalogWatchlistMachinery default - derive the rating sheet backend label from MediaBrowserDialect.productName instead of a hardcoded switch - inline EndpointFailoverManager into failover_http_client.dart, its only importer, and remove endpoint_failover_interceptor.dart --- lib/media/media_version.dart | 11 +-- .../catalog/trakt_catalog_source.dart | 5 -- lib/services/jellyfin_mappers.dart | 4 -- lib/utils/endpoint_failover_interceptor.dart | 69 ------------------- lib/utils/failover_http_client.dart | 69 ++++++++++++++++++- lib/utils/media_quality_labels.dart | 19 +---- lib/utils/resolution_label.dart | 25 ++++++- lib/widgets/rating_bottom_sheet.dart | 6 +- 8 files changed, 96 insertions(+), 112 deletions(-) delete mode 100644 lib/utils/endpoint_failover_interceptor.dart diff --git a/lib/media/media_version.dart b/lib/media/media_version.dart index c6503b5bd..677a58303 100644 --- a/lib/media/media_version.dart +++ b/lib/media/media_version.dart @@ -4,6 +4,7 @@ import '../i18n/strings.g.dart'; import '../utils/codec_utils.dart'; import '../utils/formatters.dart'; import '../utils/json_utils.dart'; +import '../utils/resolution_label.dart'; import 'media_part.dart'; part 'media_version.g.dart'; @@ -14,14 +15,6 @@ int? bitrateKbpsFromBps(int? bps) { return (bps / 1000).round(); } -final _numericVideoResolution = RegExp(r'^\d+$'); - -/// Plex may return numeric heights (`1080`) or named resolution labels (`sd`, `4k`). -String _videoResolutionDisplayLabel(String resolution) { - final value = resolution.trim(); - return _numericVideoResolution.hasMatch(value) ? '${value}p' : value.toUpperCase(); -} - /// A single media variant available for an item — represents one quality level /// or transcode profile of the underlying file. An item with multiple versions /// (e.g. 4K + 1080p re-encode) exposes one [MediaVersion] per option. @@ -95,7 +88,7 @@ class MediaVersion { final parts = []; if (videoResolution != null && videoResolution!.isNotEmpty) { - parts.add(_videoResolutionDisplayLabel(videoResolution!)); + parts.add(resolutionDisplayLabel(videoResolution!)); } else if (height != null) { parts.add('${height}p'); } diff --git a/lib/services/catalog/trakt_catalog_source.dart b/lib/services/catalog/trakt_catalog_source.dart index 765bd22fb..8fc93afc3 100644 --- a/lib/services/catalog/trakt_catalog_source.dart +++ b/lib/services/catalog/trakt_catalog_source.dart @@ -242,11 +242,6 @@ class TraktCatalogSource with CatalogWatchlistMachinery implements CatalogSource add ? await _client.addToWatchlist(body) : await _client.removeFromWatchlist(body); } - @override - List membershipKeysFor(MediaKind kind, CatalogItemIds ids) => [ - for (final key in ids.allKeys) '${kind.id}/$key', - ]; - List _fromEntries(List entries, {MediaKind? kind}) => [ for (final entry in entries) if (entry.media != null && _entryKind(entry, kind) != null && entry.media!.ids.hasAny) diff --git a/lib/services/jellyfin_mappers.dart b/lib/services/jellyfin_mappers.dart index 7a2754d72..cddee9d66 100644 --- a/lib/services/jellyfin_mappers.dart +++ b/lib/services/jellyfin_mappers.dart @@ -16,10 +16,6 @@ import '../utils/resolution_label.dart'; import 'file_info_parser.dart'; import 'jellyfin_display_metadata.dart'; -// Re-export so existing callers that pulled `resolutionLabelFromHeight` -// from this file keep compiling without a bulk import rewrite. -export '../utils/resolution_label.dart' show resolutionLabelFromHeight; - Map? jellyfinFirstVideoStream(Object? streams) { if (streams is! List) return null; for (final stream in streams) { diff --git a/lib/utils/endpoint_failover_interceptor.dart b/lib/utils/endpoint_failover_interceptor.dart deleted file mode 100644 index d61bef826..000000000 --- a/lib/utils/endpoint_failover_interceptor.dart +++ /dev/null @@ -1,69 +0,0 @@ -import '../utils/app_logger.dart'; - -/// Maintains the list of endpoints we can cycle through when one fails. -class EndpointFailoverManager { - EndpointFailoverManager(List urls) { - _setEndpoints(urls); - } - - late List _endpoints; - int _currentIndex = 0; - - /// Incremented every time the active endpoint changes. Requests stamped with - /// an older generation should not trigger additional failover cascades. - int _generation = 0; - int get generation => _generation; - - List get endpoints => List.unmodifiable(_endpoints); - - String get current => _endpoints[_currentIndex]; - - bool get hasFallback => _currentIndex < _endpoints.length - 1; - - /// Move to the next endpoint, returning its URL or null if exhausted. - String? moveToNext() { - if (!hasFallback) return null; - _currentIndex++; - _generation++; - return _endpoints[_currentIndex]; - } - - /// Reset back to the first (preferred) endpoint. Called when all endpoints - /// are exhausted so the next failure cycle starts from the best candidate. - String? resetToFirst() { - if (_currentIndex != 0) { - _currentIndex = 0; - _generation++; - appLogger.d('Failover endpoint list reset to first candidate'); - return _endpoints[_currentIndex]; - } - return null; - } - - /// Replace the endpoint list and optionally set the active endpoint. - void reset(List urls, {String? currentBaseUrl}) { - _setEndpoints(urls); - if (currentBaseUrl != null) { - final index = _endpoints.indexOf(currentBaseUrl); - _currentIndex = index >= 0 ? index : 0; - } else { - _currentIndex = 0; - } - _generation++; - } - - void _setEndpoints(List urls) { - final sanitized = []; - final seen = {}; - for (final url in urls) { - if (url.isEmpty || seen.contains(url)) continue; - seen.add(url); - sanitized.add(url); - } - if (sanitized.isEmpty) { - throw ArgumentError('At least one endpoint is required'); - } - _endpoints = sanitized; - _currentIndex = _currentIndex.clamp(0, _endpoints.length - 1); - } -} diff --git a/lib/utils/failover_http_client.dart b/lib/utils/failover_http_client.dart index e24de2227..40abb2ae5 100644 --- a/lib/utils/failover_http_client.dart +++ b/lib/utils/failover_http_client.dart @@ -1,4 +1,3 @@ -import 'endpoint_failover_interceptor.dart'; import 'app_logger.dart'; import 'media_server_http_client.dart'; import '../exceptions/media_server_exceptions.dart'; @@ -246,3 +245,71 @@ class FailoverHttpClient extends MediaServerHttpClient { } } } + +/// Maintains the list of endpoints we can cycle through when one fails. +class EndpointFailoverManager { + EndpointFailoverManager(List urls) { + _setEndpoints(urls); + } + + late List _endpoints; + int _currentIndex = 0; + + /// Incremented every time the active endpoint changes. Requests stamped with + /// an older generation should not trigger additional failover cascades. + int _generation = 0; + int get generation => _generation; + + List get endpoints => List.unmodifiable(_endpoints); + + String get current => _endpoints[_currentIndex]; + + bool get hasFallback => _currentIndex < _endpoints.length - 1; + + /// Move to the next endpoint, returning its URL or null if exhausted. + String? moveToNext() { + if (!hasFallback) return null; + _currentIndex++; + _generation++; + return _endpoints[_currentIndex]; + } + + /// Reset back to the first (preferred) endpoint. Called when all endpoints + /// are exhausted so the next failure cycle starts from the best candidate. + String? resetToFirst() { + if (_currentIndex != 0) { + _currentIndex = 0; + _generation++; + appLogger.d('Failover endpoint list reset to first candidate'); + return _endpoints[_currentIndex]; + } + return null; + } + + /// Replace the endpoint list and optionally set the active endpoint. + void reset(List urls, {String? currentBaseUrl}) { + _setEndpoints(urls); + if (currentBaseUrl != null) { + final index = _endpoints.indexOf(currentBaseUrl); + _currentIndex = index >= 0 ? index : 0; + } else { + _currentIndex = 0; + } + _generation++; + } + + void _setEndpoints(List urls) { + final sanitized = []; + final seen = {}; + for (final url in urls) { + if (url.isEmpty || seen.contains(url)) continue; + seen.add(url); + sanitized.add(url); + } + if (sanitized.isEmpty) { + throw ArgumentError('At least one endpoint is required'); + } + _endpoints = sanitized; + _currentIndex = _currentIndex.clamp(0, _endpoints.length - 1); + } +} diff --git a/lib/utils/media_quality_labels.dart b/lib/utils/media_quality_labels.dart index b31984d20..94acdf908 100644 --- a/lib/utils/media_quality_labels.dart +++ b/lib/utils/media_quality_labels.dart @@ -56,25 +56,10 @@ MediaVersion? _selectedVersion(List? versions, int versionIndex) { String? _formatResolution(MediaVersion version) { final raw = version.videoResolution?.trim(); - if (raw != null && raw.isNotEmpty) return _formatResolutionValue(raw); + if (raw != null && raw.isNotEmpty) return resolutionDisplayLabel(raw); final fallback = resolutionLabelFromDimensions(version.width, version.height); - return fallback == null ? null : _formatResolutionValue(fallback); -} - -String _formatResolutionValue(String value) { - final normalized = value.trim().toLowerCase(); - if (normalized == '4k' || normalized == 'uhd') return '4K'; - if (normalized == 'sd') return 'SD'; - - final numeric = RegExp(r'^(\d+)(?:p)?$').firstMatch(normalized); - if (numeric != null) { - final height = int.tryParse(numeric.group(1)!); - if (height != null && height >= 2160) return '4K'; - return '${numeric.group(1)}p'; - } - - return value.toUpperCase(); + return fallback == null ? null : resolutionDisplayLabel(fallback); } MediaStream? _firstStreamOfKind(MediaVersion version, MediaStreamKind kind) { diff --git a/lib/utils/resolution_label.dart b/lib/utils/resolution_label.dart index f510da625..aceb39f96 100644 --- a/lib/utils/resolution_label.dart +++ b/lib/utils/resolution_label.dart @@ -3,8 +3,8 @@ /// height for non-standard sizes). Returns `null` when [height] is null. /// /// Plex hands the label back already in its `Media.videoResolution` field; -/// Jellyfin only gives raw pixel dimensions, so the Jellyfin mapper and -/// playback path both call this to produce the same shape. +/// Jellyfin only gives raw pixel dimensions, which the Jellyfin mapper feeds +/// through [resolutionLabelFromDimensions] to produce the same shape. String? resolutionLabelFromHeight(int? height) { if (height == null) return null; if (height >= 2160) return '4k'; @@ -23,3 +23,24 @@ String? resolutionLabelFromDimensions(int? width, int? height) { if ((width != null && width >= 854) || (height != null && height >= 480)) return '480'; return resolutionLabelFromHeight(height); } + +final _numericResolutionValue = RegExp(r'^(\d+)(?:p)?$'); + +/// Format a canonical resolution label — or a raw numeric height, with or +/// without a trailing `p` — for display: `'1080'`/`'1080p'` → `'1080p'`, +/// `'4k'`/`'uhd'`/heights ≥ 2160 → `'4K'`, `'sd'` → `'SD'`; anything else is +/// uppercased verbatim. +String resolutionDisplayLabel(String value) { + final normalized = value.trim().toLowerCase(); + if (normalized == '4k' || normalized == 'uhd') return '4K'; + if (normalized == 'sd') return 'SD'; + + final numeric = _numericResolutionValue.firstMatch(normalized); + if (numeric != null) { + final height = int.tryParse(numeric.group(1)!); + if (height != null && height >= 2160) return '4K'; + return '${numeric.group(1)}p'; + } + + return value.trim().toUpperCase(); +} diff --git a/lib/widgets/rating_bottom_sheet.dart b/lib/widgets/rating_bottom_sheet.dart index c632a852e..cd8ddccaa 100644 --- a/lib/widgets/rating_bottom_sheet.dart +++ b/lib/widgets/rating_bottom_sheet.dart @@ -542,11 +542,7 @@ class _RatingBottomSheetState extends State { return () => nodes[index].requestFocus(); } - String _backendLabel(MediaBackend backend) => switch (backend) { - MediaBackend.plex => 'Plex', - MediaBackend.jellyfin => 'Jellyfin', - MediaBackend.emby => 'Emby', - }; + String _backendLabel(MediaBackend backend) => backend.dialect?.productName ?? 'Plex'; } const _serverKey = 'server';