diff --git a/mobile/lib/domain/models/asset/base_asset.model.dart b/mobile/lib/domain/models/asset/base_asset.model.dart index c8e1e26fba..ac89fe9a0a 100644 --- a/mobile/lib/domain/models/asset/base_asset.model.dart +++ b/mobile/lib/domain/models/asset/base_asset.model.dart @@ -77,11 +77,10 @@ sealed class BaseAsset { // Overridden in subclasses AssetState get storage; + String get id; String? get localId; String? get remoteId; String get heroTag; - // Key for the video player state. Unlike heroTag it survives the DB copy filling in the other side's id - String get playerKey; AssetPlaybackStyle get playbackStyle; @override diff --git a/mobile/lib/domain/models/asset/local_asset.model.dart b/mobile/lib/domain/models/asset/local_asset.model.dart index aa9beca983..15039761a8 100644 --- a/mobile/lib/domain/models/asset/local_asset.model.dart +++ b/mobile/lib/domain/models/asset/local_asset.model.dart @@ -1,6 +1,7 @@ part of 'base_asset.model.dart'; class LocalAsset extends BaseAsset { + @override final String id; final String? remoteAssetId; final String? cloudId; @@ -45,9 +46,6 @@ class LocalAsset extends BaseAsset { @override String get heroTag => '${id}_${remoteId ?? checksum}'; - @override - String get playerKey => id; - bool get hasCoordinates => latitude != null && longitude != null && latitude != 0 && longitude != 0; @override diff --git a/mobile/lib/domain/models/asset/remote_asset.model.dart b/mobile/lib/domain/models/asset/remote_asset.model.dart index 1b7f091c02..5707329ba3 100644 --- a/mobile/lib/domain/models/asset/remote_asset.model.dart +++ b/mobile/lib/domain/models/asset/remote_asset.model.dart @@ -4,6 +4,7 @@ enum AssetVisibility { timeline, hidden, archive, locked } // Model for an asset stored in the server class RemoteAsset extends BaseAsset { + @override final String id; final String? localAssetId; final String? thumbHash; @@ -65,9 +66,6 @@ class RemoteAsset extends BaseAsset { @override String get heroTag => '${localId ?? checksum}_$id'; - @override - String get playerKey => id; - @override bool get isEditable => isImage && !isMotionPhoto && !isAnimatedImage; diff --git a/mobile/lib/presentation/pages/drift_slideshow.page.dart b/mobile/lib/presentation/pages/drift_slideshow.page.dart index 44a80a7424..48b29b3499 100644 --- a/mobile/lib/presentation/pages/drift_slideshow.page.dart +++ b/mobile/lib/presentation/pages/drift_slideshow.page.dart @@ -93,8 +93,8 @@ class _DriftSlideshowPageState extends ConsumerState with Si if (asset.isImage) { _createTimer(); - } else if (ref.read(videoPlayerProvider(asset.playerKey)).status == VideoPlaybackStatus.paused) { - unawaited(ref.read(videoPlayerProvider(asset.playerKey).notifier).play()); + } else if (ref.read(videoPlayerProvider(asset.id)).status == VideoPlaybackStatus.paused) { + unawaited(ref.read(videoPlayerProvider(asset.id).notifier).play()); } else { unawaited(_nextPage()); } @@ -113,7 +113,7 @@ class _DriftSlideshowPageState extends ConsumerState with Si final asset = widget.timeline.getAssetSafe(_index)!; if (!asset.isImage) { - unawaited(ref.read(videoPlayerProvider(asset.playerKey).notifier).pause()); + unawaited(ref.read(videoPlayerProvider(asset.id).notifier).pause()); } setState(() { @@ -300,7 +300,7 @@ class _DriftSlideshowPageState extends ConsumerState with Si borderRadius: BorderRadius.zero, minHeight: 5, value: - ref.read(videoPlayerProvider(asset.playerKey).select((s) => s.position)).inMilliseconds / + ref.read(videoPlayerProvider(asset.id).select((s) => s.position)).inMilliseconds / asset.duration.inMilliseconds, ); } @@ -374,13 +374,13 @@ class _DriftSlideshowPageState extends ConsumerState with Si builder: (context, value, _) => buildPhotoView(scale * (1.0 + value * _kenBurnsZoom)), ); } else { - final status = ref.read(videoPlayerProvider(asset.playerKey).select((s) => s.status)); - final position = ref.read(videoPlayerProvider(asset.playerKey)).position; + final status = ref.read(videoPlayerProvider(asset.id).select((s) => s.status)); + final position = ref.read(videoPlayerProvider(asset.id)).position; if (status == VideoPlaybackStatus.completed && isCurrent && position.inMicroseconds > 0) { unawaited(_nextPage()); } else if (status == VideoPlaybackStatus.playing) { - unawaited(ref.read(videoPlayerProvider(asset.playerKey).notifier).setLoop(false)); + unawaited(ref.read(videoPlayerProvider(asset.id).notifier).setLoop(false)); } return PhotoView.customChild( diff --git a/mobile/lib/presentation/widgets/asset_viewer/bottom_bar.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/bottom_bar.widget.dart index 2f1fee7a69..15b90e58ea 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/bottom_bar.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/bottom_bar.widget.dart @@ -94,7 +94,7 @@ class ViewerBottomBar extends ConsumerWidget { mainAxisSize: MainAxisSize.min, children: [ if (asset.isImage) OcrToggleButton(asset: asset), - if (asset.isVideo) VideoControls(asset: asset), + if (asset.isVideo) VideoControls(videoPlayerName: asset.id), if (!isReadonlyModeEnabled) ImmichColorOverride( color: Colors.white, diff --git a/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart index 3b534abf98..9011888b54 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart @@ -47,7 +47,7 @@ class _NativeVideoViewerState extends ConsumerState with Widg bool _isVideoReady = false; bool _shouldPlayOnForeground = true; - VideoPlayerNotifier get _notifier => ref.read(videoPlayerProvider(widget.asset.playerKey).notifier); + VideoPlayerNotifier get _notifier => ref.read(videoPlayerProvider(widget.asset.id).notifier); @override void initState() { @@ -304,7 +304,7 @@ class _NativeVideoViewerState extends ConsumerState with Widg @override Widget build(BuildContext context) { final isCasting = ref.watch(castProvider.select((c) => c.isCasting)); - final status = ref.watch(videoPlayerProvider(widget.asset.playerKey).select((v) => v.status)); + final status = ref.watch(videoPlayerProvider(widget.asset.id).select((v) => v.status)); return IgnorePointer( child: Stack( diff --git a/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart b/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart index eefd58386c..ac929d2b6a 100644 --- a/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart +++ b/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart @@ -67,9 +67,9 @@ class AssetViewerStateNotifier extends Notifier { } state = state.copyWith(showingDetails: showing, showingControls: showing ? true : state.showingControls); - final playerKey = state.currentAsset?.playerKey; - if (playerKey != null) { - final notifier = ref.read(videoPlayerProvider(playerKey).notifier); + final id = state.currentAsset?.id; + if (id != null) { + final notifier = ref.read(videoPlayerProvider(id).notifier); showing ? notifier.hold() : notifier.release(); } } diff --git a/mobile/lib/widgets/asset_viewer/video_controls.dart b/mobile/lib/widgets/asset_viewer/video_controls.dart index 17abba1586..14c4daf5d8 100644 --- a/mobile/lib/widgets/asset_viewer/video_controls.dart +++ b/mobile/lib/widgets/asset_viewer/video_controls.dart @@ -4,7 +4,6 @@ import 'package:async/async.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/colors.dart'; -import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/extensions/duration_extensions.dart'; import 'package:immich_mobile/models/cast/cast_manager_state.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; @@ -13,11 +12,11 @@ import 'package:immich_mobile/providers/cast.provider.dart'; import 'package:immich_mobile/widgets/asset_viewer/animated_play_pause.dart'; class VideoControls extends ConsumerStatefulWidget { - final BaseAsset asset; + final String videoPlayerName; static const List _controlShadows = [Shadow(color: Colors.black87, blurRadius: 6, offset: Offset(0, 1))]; - const VideoControls({super.key, required this.asset}); + const VideoControls({super.key, required this.videoPlayerName}); @override ConsumerState createState() => _VideoControlsState(); @@ -27,7 +26,7 @@ class _VideoControlsState extends ConsumerState { late final RestartableTimer _hideTimer; AutoDisposeStateNotifierProvider get _provider => - videoPlayerProvider(widget.asset.playerKey); + videoPlayerProvider(widget.videoPlayerName); @override void initState() { @@ -38,7 +37,7 @@ class _VideoControlsState extends ConsumerState { @override void didUpdateWidget(covariant VideoControls oldWidget) { super.didUpdateWidget(oldWidget); - if (oldWidget.asset.playerKey != widget.asset.playerKey) { + if (oldWidget.videoPlayerName != widget.videoPlayerName) { _hideTimer.reset(); } } diff --git a/mobile/test/domain/models/base_asset_test.dart b/mobile/test/domain/models/base_asset_test.dart index a0ea9e623b..6a6c836aa7 100644 --- a/mobile/test/domain/models/base_asset_test.dart +++ b/mobile/test/domain/models/base_asset_test.dart @@ -44,13 +44,13 @@ void main() { }); }); - group('BaseAsset.playerKey', () { + group('BaseAsset.id', () { test('survives the DB copy filling localId', () { final searchCopy = RemoteAssetFactory.create(id: 'asset-1'); final mergedCopy = searchCopy.copyWith(localId: 'local-1'); expect(searchCopy.heroTag, isNot(mergedCopy.heroTag)); - expect(mergedCopy.playerKey, searchCopy.playerKey); + expect(mergedCopy.id, searchCopy.id); }); test('survives a local asset gaining a remote id', () { @@ -58,7 +58,7 @@ void main() { final uploaded = localOnly.copyWith(remoteId: 'asset-1'); expect(localOnly.heroTag, isNot(uploaded.heroTag)); - expect(uploaded.playerKey, localOnly.playerKey); + expect(uploaded.id, localOnly.id); }); test('differs between assets', () { @@ -66,8 +66,8 @@ void main() { final b = RemoteAssetFactory.create(id: 'asset-2'); final local = LocalAssetFactory.create(id: 'local-1'); - expect(a.playerKey, isNot(b.playerKey)); - expect(local.playerKey, isNot(a.playerKey)); + expect(a.id, isNot(b.id)); + expect(local.id, isNot(a.id)); }); }); } diff --git a/mobile/test/presentation/widgets/asset_viewer/bottom_bar_test.dart b/mobile/test/presentation/widgets/asset_viewer/bottom_bar_test.dart index d58111c304..f2463c090d 100644 --- a/mobile/test/presentation/widgets/asset_viewer/bottom_bar_test.dart +++ b/mobile/test/presentation/widgets/asset_viewer/bottom_bar_test.dart @@ -68,7 +68,7 @@ void main() { final viewer = ref.read(assetViewerProvider.notifier); viewer.setAsset(searchCopy); await tester.pump(); - ref.read(videoPlayerProvider(searchCopy.playerKey).notifier).attachController(controller); + ref.read(videoPlayerProvider(searchCopy.id).notifier).attachController(controller); updates.add(mergedCopy); await tester.pump(); await tester.tap(find.byType(IconButton)); diff --git a/mobile/test/widgets/asset_viewer/video_controls_test.dart b/mobile/test/widgets/asset_viewer/video_controls_test.dart deleted file mode 100644 index 8bcd9a6131..0000000000 --- a/mobile/test/widgets/asset_viewer/video_controls_test.dart +++ /dev/null @@ -1,57 +0,0 @@ -import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:immich_mobile/providers/asset_viewer/video_player_provider.dart'; -import 'package:immich_mobile/services/gcast.service.dart'; -import 'package:immich_mobile/widgets/asset_viewer/video_controls.dart'; - -import '../../service.mocks.dart'; -import '../../unit/factories/remote_asset_factory.dart'; -import '../../widget_tester_extensions.dart'; - -class _SeededPlayer extends VideoPlayerNotifier { - _SeededPlayer(VideoPlayerState seed) { - state = seed; - } -} - -void main() { - const playing = VideoPlayerState( - position: Duration(seconds: 6), - duration: Duration(seconds: 12), - status: VideoPlaybackStatus.playing, - ); - const idle = VideoPlayerState(position: Duration.zero, duration: Duration.zero, status: VideoPlaybackStatus.paused); - - testWidgets('stays on the same player when the DB copy replaces the asset', (tester) async { - // A video opened from search arrives with localId null, then the viewer watches the DB - // copy which fills it in. The controls have to keep reading the player they started on. - final searchCopy = RemoteAssetFactory.create(id: 'asset-1', type: .video); - final mergedCopy = searchCopy.copyWith(localId: 'local-1'); - final boundKey = searchCopy.playerKey; - - expect(searchCopy.heroTag, isNot(mergedCopy.heroTag)); - - var asset = searchCopy; - late StateSetter setHostState; - - await tester.pumpConsumerWidget( - StatefulBuilder( - builder: (_, setState) { - setHostState = setState; - return VideoControls(asset: asset); - }, - ), - overrides: [ - gCastServiceProvider.overrideWithValue(MockGCastService()), - videoPlayerProvider.overrideWith((ref, key) => _SeededPlayer(key == boundKey ? playing : idle)), - ], - ); - - expect(find.text('00:06 / 00:12'), findsOne); - - setHostState(() => asset = mergedCopy); - await tester.pump(); - - expect(find.text('00:06 / 00:12'), findsOne); - }); -}