From fe94f48cf4f8fd72f5b4ff6c055202a08fd7ec7d Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 12 Aug 2026 12:41:13 +0600 Subject: [PATCH 1/2] rewrite the slideshow advancement around a single always armed timer --- .../pages/drift_slideshow.page.dart | 144 +++++++--- .../pages/drift_slideshow_page_test.dart | 254 ++++++++++++++++++ 2 files changed, 355 insertions(+), 43 deletions(-) create mode 100644 mobile/test/presentation/pages/drift_slideshow_page_test.dart diff --git a/mobile/lib/presentation/pages/drift_slideshow.page.dart b/mobile/lib/presentation/pages/drift_slideshow.page.dart index 824d7d7c6b..c1eeadca2d 100644 --- a/mobile/lib/presentation/pages/drift_slideshow.page.dart +++ b/mobile/lib/presentation/pages/drift_slideshow.page.dart @@ -43,6 +43,8 @@ class _DriftSlideshowPageState extends ConsumerState with Si late Timer _timer; late int _index; late int _nextIndex; + Duration _lastPosition = Duration.zero; + ProviderSubscription? _videoSubscription; bool _paused = false; bool _showAppBar = false; @@ -64,6 +66,7 @@ class _DriftSlideshowPageState extends ConsumerState with Si _crossfadeOpacity = Tween(begin: 1.0, end: 0.0).animate(_crossfadeController); _stopwatch = Stopwatch(); _createTimer(); + _watchVideo(); _updateNextIndex(); ref.listenManual(appConfigProvider.select((s) => s.slideshow), _onConfigChanged); @@ -89,21 +92,21 @@ class _DriftSlideshowPageState extends ConsumerState with Si } void _play() { - final asset = widget.timeline.getAssetSafe(_index)!; - - if (asset.isImage) { - _createTimer(); - } else if (ref.read(videoPlayerProvider(asset.id)).status == VideoPlaybackStatus.paused) { - unawaited(ref.read(videoPlayerProvider(asset.id).notifier).play()); - } else { - unawaited(_nextPage()); - } - - _updateNextIndex(); - setState(() { _paused = false; }); + + final asset = widget.timeline.getAssetSafe(_index)!; + + if (!asset.isImage) { + if (ref.read(videoPlayerProvider(asset.id)).status == VideoPlaybackStatus.completed) { + unawaited(_nextPage()); + return; + } + unawaited(ref.read(videoPlayerProvider(asset.id).notifier).play()); + } + + _createTimer(); } void _pause() { @@ -130,8 +133,7 @@ class _DriftSlideshowPageState extends ConsumerState with Si _config = next; _updateNextIndex(); - final asset = widget.timeline.getAssetSafe(_index); - if (durationChanged && !_paused && asset?.isImage == true) { + if (durationChanged && !_paused) { _timer.cancel(); _createTimer(); } @@ -152,24 +154,42 @@ class _DriftSlideshowPageState extends ConsumerState with Si } Future _nextPage() async { - if (_nextIndex < 0 || _nextIndex >= widget.timeline.totalAssets) { - if (_config.repeat) { - final wrapped = _config.direction == SlideshowDirection.forward ? 0 : widget.timeline.totalAssets - 1; - await widget.timeline.preloadAssets(wrapped); - _pageController.jumpToPage(wrapped); - } else { - setState(() { - _paused = true; - }); - } + _timer.cancel(); + + if (widget.timeline.totalAssets == 0) { return; } - if (!widget.timeline.hasRange(_nextIndex, 1)) { - await widget.timeline.preloadAssets(_nextIndex); + var target = _nextIndex; + if (target < 0 || target >= widget.timeline.totalAssets) { + if (!_config.repeat) { + setState(() { + _paused = true; + }); + return; + } + target = _config.direction == SlideshowDirection.forward ? 0 : widget.timeline.totalAssets - 1; } - _crossFadeToPage(_nextIndex); + if (!widget.timeline.hasRange(target, 1)) { + final from = _index; + await widget.timeline.preloadAssets(target); + if (!mounted || from != _index) { + return; + } + } + + if (target == _index) { + final asset = widget.timeline.getAssetSafe(target)!; + if (!asset.isImage && ref.read(videoPlayerProvider(asset.id)).status == VideoPlaybackStatus.completed) { + unawaited(ref.read(videoPlayerProvider(asset.id).notifier).restart()); + } + // jumpToPage to the current page fires no onPageChanged + _pageChanged(target); + return; + } + + _crossFadeToPage(target); } void _crossFadeToPage(int page) { @@ -237,15 +257,60 @@ class _DriftSlideshowPageState extends ConsumerState with Si } void _createTimer() { - _timer = Timer(Duration(milliseconds: _config.duration * 1000 - _stopwatch.elapsedMilliseconds), () { - _stopwatch.stop(); - _stopwatch.reset(); - unawaited(_nextPage()); - }); - + _timer = Timer(Duration(milliseconds: _config.duration * 1000 - _stopwatch.elapsedMilliseconds), _onTimerElapsed); _stopwatch.start(); } + void _onTimerElapsed() { + _stopwatch.stop(); + _stopwatch.reset(); + + final asset = widget.timeline.getAssetSafe(_index); + if (asset != null && !asset.isImage) { + final position = ref.read(videoPlayerProvider(asset.id)).position; + if (position != _lastPosition) { + _lastPosition = position; + _createTimer(); + return; + } + } + + unawaited(_nextPage()); + } + + void _watchVideo() { + _videoSubscription?.close(); + _videoSubscription = null; + + final asset = widget.timeline.getAssetSafe(_index); + if (asset == null || asset.isImage) { + return; + } + + // a video already playing here (e.g. opened from the viewer) never gets a + // playing transition, so its loop has to be turned off up front + if (ref.read(videoPlayerProvider(asset.id)).status == VideoPlaybackStatus.playing) { + unawaited(ref.read(videoPlayerProvider(asset.id).notifier).setLoop(false)); + } + + _videoSubscription = ref.listenManual( + videoPlayerProvider(asset.id), + (previous, next) => _onVideoChanged(asset.id, previous, next), + ); + } + + void _onVideoChanged(String assetId, VideoPlayerState? previous, VideoPlayerState next) { + if (_paused || next.status == previous?.status) { + return; + } + + if (next.status == VideoPlaybackStatus.completed && next.position > Duration.zero) { + unawaited(_nextPage()); + } else if (next.status == VideoPlaybackStatus.playing) { + unawaited(ref.read(videoPlayerProvider(assetId).notifier).setLoop(false)); + } + } + void _pageChanged(int page) { final asset = widget.timeline.getAssetSafe(page)!; @@ -261,8 +326,10 @@ class _DriftSlideshowPageState extends ConsumerState with Si _timer.cancel(); _stopwatch.stop(); _stopwatch.reset(); + _lastPosition = Duration.zero; + _watchVideo(); - if (!_paused && asset.isImage) { + if (!_paused) { _createTimer(); } @@ -374,15 +441,6 @@ class _DriftSlideshowPageState extends ConsumerState with Si builder: (context, value, _) => buildPhotoView(scale * (1.0 + value * _kenBurnsZoom)), ); } else { - 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.id).notifier).setLoop(false)); - } - return PhotoView.customChild( onTapUp: (_, _, _) => _onTapUp(), disableScaleGestures: true, diff --git a/mobile/test/presentation/pages/drift_slideshow_page_test.dart b/mobile/test/presentation/pages/drift_slideshow_page_test.dart new file mode 100644 index 0000000000..91d22bcee2 --- /dev/null +++ b/mobile/test/presentation/pages/drift_slideshow_page_test.dart @@ -0,0 +1,254 @@ +import 'package:drift/drift.dart'; +import 'package:drift/native.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/constants/enums.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/domain/models/config/app_config.dart'; +import 'package:immich_mobile/domain/models/config/slideshow_config.dart'; +import 'package:immich_mobile/domain/models/store.model.dart'; +import 'package:immich_mobile/domain/services/store.service.dart'; +import 'package:immich_mobile/domain/services/timeline.service.dart'; +import 'package:immich_mobile/infrastructure/repositories/db.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; +import 'package:immich_mobile/presentation/pages/drift_slideshow.page.dart'; +import 'package:immich_mobile/providers/asset_viewer/video_player_provider.dart'; +import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/settings.provider.dart'; +import 'package:immich_mobile/services/gcast.service.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../../service.mocks.dart'; +import '../../unit/factories/remote_asset_factory.dart'; +import '../../widget_tester_extensions.dart'; + +class MockTimelineService extends Mock implements TimelineService {} + +class FakeVideoPlayerNotifier extends VideoPlayerNotifier { + final loopCalls = []; + var restartCalls = 0; + + void emit(VideoPlayerState next) => state = next; + + @override + Future setLoop(bool loop) async => loopCalls.add(loop); + + @override + Future restart() async => restartCalls++; +} + +void main() { + const config = AppConfig(slideshow: SlideshowConfig(duration: 3, look: SlideshowLook.contain)); + const duration = Duration(seconds: 3); + + late MockTimelineService timeline; + late MockAssetService assetService; + late FakeVideoPlayerNotifier player; + + setUpAll(() async { + registerFallbackValue(RemoteAssetFactory.create()); + final db = Drift(DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true)); + await StoreService.init(storeRepository: DriftStoreRepository(db), listenUpdates: false); + await StoreService.I.put(StoreKey.serverEndpoint, 'http://localhost:3000'); + }); + + setUp(() { + timeline = MockTimelineService(); + assetService = MockAssetService(); + player = FakeVideoPlayerNotifier(); + when(() => assetService.getAsset(any())).thenAnswer((_) async => null); + + // Pigeon success replies with a null payload: images and wakelock toggles resolve to nothing. + Future nullReply(ByteData? message) async => const StandardMessageCodec().encodeMessage([null]); + final messenger = TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMessageHandler( + 'dev.flutter.pigeon.wakelock_plus_platform_interface.WakelockPlusApi.toggle', + nullReply, + ); + messenger.setMockMessageHandler('dev.flutter.pigeon.immich_mobile.RemoteImageApi.requestImage', nullReply); + messenger.setMockMessageHandler('dev.flutter.pigeon.immich_mobile.RemoteImageApi.cancelRequest', nullReply); + messenger.setMockMethodCallHandler( + SystemChannels.platform_views, + (call) async => switch (call.method) { + 'create' => 1, + 'resize' => {'width': 1080.0, 'height': 1920.0}, + _ => null, + }, + ); + }); + + void stubTimeline(List assets) { + when(() => timeline.totalAssets).thenReturn(assets.length); + when(() => timeline.hasRange(any(), any())).thenReturn(true); + when(() => timeline.preloadAssets(any())).thenAnswer((_) async {}); + when(() => timeline.getAssetSafe(any())).thenAnswer((invocation) { + final index = invocation.positionalArguments.first as int; + return index >= 0 && index < assets.length ? assets[index] : null; + }); + } + + // The page never settles while running (progress bar / spinner animations), so + // mount it with a plain pump after the localized shell settled around a placeholder. + Future pumpSlideshow(WidgetTester tester, {required List overrides}) async { + final show = ValueNotifier(false); + await tester.pumpConsumerWidget( + ValueListenableBuilder( + valueListenable: show, + builder: (context, value, _) => !value + ? const SizedBox.shrink() + : MediaQuery( + data: MediaQuery.of(context).copyWith(disableAnimations: true), + child: DriftSlideshowPage(timeline: timeline), + ), + ), + overrides: overrides, + ); + show.value = true; + await tester.pump(); + } + + double currentPage(WidgetTester tester) => tester.widget(find.byType(PageView)).controller!.page!; + + List overridesFor(RemoteAsset video) => [ + appConfigProvider.overrideWithValue(config), + assetServiceProvider.overrideWithValue(assetService), + gCastServiceProvider.overrideWithValue(MockGCastService()), + videoPlayerProvider(video.id).overrideWith((ref) => player), + ]; + + testWidgets('video that never starts advances after one duration', (tester) async { + final video = RemoteAssetFactory.create(type: .video); + stubTimeline([RemoteAssetFactory.create(), video, RemoteAssetFactory.create()]); + + await pumpSlideshow(tester, overrides: overridesFor(video)); + await tester.pump(duration); + expect(currentPage(tester), 1); + + player.emit(const VideoPlayerState(position: Duration.zero, duration: Duration.zero, status: .buffering)); + await tester.pump(duration); + expect(currentPage(tester), 2); + + await tester.pumpWidget(const SizedBox.shrink()); + }); + + testWidgets('playing video is not cut short and advances when it ends', (tester) async { + final video = RemoteAssetFactory.create(type: .video); + stubTimeline([RemoteAssetFactory.create(), video, RemoteAssetFactory.create()]); + + await pumpSlideshow(tester, overrides: overridesFor(video)); + await tester.pump(duration); + expect(currentPage(tester), 1); + + const length = Duration(seconds: 60); + player.emit(const VideoPlayerState(position: Duration(milliseconds: 500), duration: length, status: .playing)); + await tester.pump(duration); + expect(currentPage(tester), 1); + + player.emit(const VideoPlayerState(position: Duration(seconds: 2), duration: length, status: .playing)); + await tester.pump(duration); + expect(currentPage(tester), 1); + + player.emit(const VideoPlayerState(position: length, duration: length, status: .completed)); + await tester.pump(); + expect(currentPage(tester), 2); + + await tester.pumpWidget(const SizedBox.shrink()); + }); + + testWidgets('disables video looping once playback starts', (tester) async { + final video = RemoteAssetFactory.create(type: .video); + stubTimeline([RemoteAssetFactory.create(), video]); + + await pumpSlideshow(tester, overrides: overridesFor(video)); + await tester.pump(duration); + expect(currentPage(tester), 1); + + player.emit(const VideoPlayerState(position: Duration.zero, duration: Duration(seconds: 60), status: .playing)); + await tester.pump(); + expect(player.loopCalls, [false]); + + await tester.pumpWidget(const SizedBox.shrink()); + }); + + testWidgets('disables looping for a video already playing at entry', (tester) async { + final video = RemoteAssetFactory.create(type: .video); + stubTimeline([RemoteAssetFactory.create(), video]); + + player.emit(const VideoPlayerState(position: Duration.zero, duration: Duration(seconds: 60), status: .playing)); + await pumpSlideshow(tester, overrides: overridesFor(video)); + await tester.pump(duration); + expect(currentPage(tester), 1); + expect(player.loopCalls, [false]); + + await tester.pumpWidget(const SizedBox.shrink()); + }); + + testWidgets('stops on the last slide when repeat is off', (tester) async { + const noRepeat = AppConfig(slideshow: SlideshowConfig(duration: 3, look: SlideshowLook.contain, repeat: false)); + stubTimeline([RemoteAssetFactory.create(), RemoteAssetFactory.create()]); + + await pumpSlideshow( + tester, + overrides: [ + appConfigProvider.overrideWithValue(noRepeat), + assetServiceProvider.overrideWithValue(assetService), + gCastServiceProvider.overrideWithValue(MockGCastService()), + ], + ); + await tester.pump(duration); + expect(currentPage(tester), 1); + + await tester.pump(duration); + await tester.pump(duration); + expect(currentPage(tester), 1); + + await tester.pumpWidget(const SizedBox.shrink()); + }); + + testWidgets('wraps to the first slide when repeat is on', (tester) async { + stubTimeline([RemoteAssetFactory.create(), RemoteAssetFactory.create()]); + + await pumpSlideshow( + tester, + overrides: [ + appConfigProvider.overrideWithValue(config), + assetServiceProvider.overrideWithValue(assetService), + gCastServiceProvider.overrideWithValue(MockGCastService()), + ], + ); + await tester.pump(duration); + expect(currentPage(tester), 1); + + await tester.pump(duration); + expect(currentPage(tester), 0); + + await tester.pumpWidget(const SizedBox.shrink()); + }); + + testWidgets('single ended video on repeat restarts in place', (tester) async { + final video = RemoteAssetFactory.create(type: .video); + stubTimeline([video]); + + await pumpSlideshow(tester, overrides: overridesFor(video)); + + const length = Duration(seconds: 60); + player.emit(const VideoPlayerState(position: Duration(seconds: 1), duration: length, status: .playing)); + await tester.pump(); + player.emit(const VideoPlayerState(position: length, duration: length, status: .completed)); + await tester.pump(); + expect(player.restartCalls, 1); + expect(currentPage(tester), 0); + + player.emit(const VideoPlayerState(position: Duration.zero, duration: length, status: .playing)); + await tester.pump(); + player.emit(const VideoPlayerState(position: length, duration: length, status: .completed)); + await tester.pump(); + expect(player.restartCalls, 2); + expect(currentPage(tester), 0); + + await tester.pumpWidget(const SizedBox.shrink()); + }); +} From d874c67b12f53462f2860926a9bf2efae26bd08f Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 12 Aug 2026 15:38:54 +0600 Subject: [PATCH 2/2] treat a completed video as ended only when it actually played --- .../pages/drift_slideshow.page.dart | 10 +++++++--- .../pages/drift_slideshow_page_test.dart | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/mobile/lib/presentation/pages/drift_slideshow.page.dart b/mobile/lib/presentation/pages/drift_slideshow.page.dart index c1eeadca2d..5dc0f1490d 100644 --- a/mobile/lib/presentation/pages/drift_slideshow.page.dart +++ b/mobile/lib/presentation/pages/drift_slideshow.page.dart @@ -99,7 +99,8 @@ class _DriftSlideshowPageState extends ConsumerState with Si final asset = widget.timeline.getAssetSafe(_index)!; if (!asset.isImage) { - if (ref.read(videoPlayerProvider(asset.id)).status == VideoPlaybackStatus.completed) { + final player = ref.read(videoPlayerProvider(asset.id)); + if (player.status == VideoPlaybackStatus.completed && player.position > Duration.zero) { unawaited(_nextPage()); return; } @@ -181,8 +182,11 @@ class _DriftSlideshowPageState extends ConsumerState with Si if (target == _index) { final asset = widget.timeline.getAssetSafe(target)!; - if (!asset.isImage && ref.read(videoPlayerProvider(asset.id)).status == VideoPlaybackStatus.completed) { - unawaited(ref.read(videoPlayerProvider(asset.id).notifier).restart()); + if (!asset.isImage) { + final player = ref.read(videoPlayerProvider(asset.id)); + if (player.status == VideoPlaybackStatus.completed && player.position > Duration.zero) { + unawaited(ref.read(videoPlayerProvider(asset.id).notifier).restart()); + } } // jumpToPage to the current page fires no onPageChanged _pageChanged(target); diff --git a/mobile/test/presentation/pages/drift_slideshow_page_test.dart b/mobile/test/presentation/pages/drift_slideshow_page_test.dart index 91d22bcee2..de7faf5d40 100644 --- a/mobile/test/presentation/pages/drift_slideshow_page_test.dart +++ b/mobile/test/presentation/pages/drift_slideshow_page_test.dart @@ -134,6 +134,25 @@ void main() { await tester.pumpWidget(const SizedBox.shrink()); }); + testWidgets('completed at position zero does not skip the video', (tester) async { + final video = RemoteAssetFactory.create(type: .video); + stubTimeline([RemoteAssetFactory.create(), video, RemoteAssetFactory.create()]); + + await pumpSlideshow(tester, overrides: overridesFor(video)); + await tester.pump(duration); + expect(currentPage(tester), 1); + + player.emit(const VideoPlayerState(position: Duration.zero, duration: Duration(seconds: 60), status: .completed)); + await tester.pump(); + expect(currentPage(tester), 1); + expect(player.restartCalls, 0); + + await tester.pump(duration); + expect(currentPage(tester), 2); + + await tester.pumpWidget(const SizedBox.shrink()); + }); + testWidgets('playing video is not cut short and advances when it ends', (tester) async { final video = RemoteAssetFactory.create(type: .video); stubTimeline([RemoteAssetFactory.create(), video, RemoteAssetFactory.create()]);