fix(mobile): video playback controls dead for backed up videos opened from search (#30587)

* key video player state by asset id instead of hero tag

* expose id on BaseAsset instead of a separate playerKey
This commit is contained in:
Santo Shakil 2026-08-09 23:54:55 +06:00 committed by GitHub
parent 2314330c78
commit 35c2a90fca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 128 additions and 13 deletions

View file

@ -77,6 +77,7 @@ sealed class BaseAsset {
// Overridden in subclasses
AssetState get storage;
String get id;
String? get localId;
String? get remoteId;
String get heroTag;

View file

@ -1,6 +1,7 @@
part of 'base_asset.model.dart';
class LocalAsset extends BaseAsset {
@override
final String id;
final String? remoteAssetId;
final String? cloudId;

View file

@ -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;

View file

@ -93,8 +93,8 @@ class _DriftSlideshowPageState extends ConsumerState<DriftSlideshowPage> with Si
if (asset.isImage) {
_createTimer();
} else if (ref.read(videoPlayerProvider(asset.heroTag)).status == VideoPlaybackStatus.paused) {
unawaited(ref.read(videoPlayerProvider(asset.heroTag).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<DriftSlideshowPage> with Si
final asset = widget.timeline.getAssetSafe(_index)!;
if (!asset.isImage) {
unawaited(ref.read(videoPlayerProvider(asset.heroTag).notifier).pause());
unawaited(ref.read(videoPlayerProvider(asset.id).notifier).pause());
}
setState(() {
@ -300,7 +300,7 @@ class _DriftSlideshowPageState extends ConsumerState<DriftSlideshowPage> with Si
borderRadius: BorderRadius.zero,
minHeight: 5,
value:
ref.read(videoPlayerProvider(asset.heroTag).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<DriftSlideshowPage> with Si
builder: (context, value, _) => buildPhotoView(scale * (1.0 + value * _kenBurnsZoom)),
);
} else {
final status = ref.read(videoPlayerProvider(asset.heroTag).select((s) => s.status));
final position = ref.read(videoPlayerProvider(asset.heroTag)).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.heroTag).notifier).setLoop(false));
unawaited(ref.read(videoPlayerProvider(asset.id).notifier).setLoop(false));
}
return PhotoView.customChild(

View file

@ -94,7 +94,7 @@ class ViewerBottomBar extends ConsumerWidget {
mainAxisSize: MainAxisSize.min,
children: [
if (asset.isImage) OcrToggleButton(asset: asset),
if (asset.isVideo) VideoControls(videoPlayerName: asset.heroTag),
if (asset.isVideo) VideoControls(videoPlayerName: asset.id),
if (!isReadonlyModeEnabled)
ImmichColorOverride(
color: Colors.white,

View file

@ -47,7 +47,7 @@ class _NativeVideoViewerState extends ConsumerState<NativeVideoViewer> with Widg
bool _isVideoReady = false;
bool _shouldPlayOnForeground = true;
VideoPlayerNotifier get _notifier => ref.read(videoPlayerProvider(widget.asset.heroTag).notifier);
VideoPlayerNotifier get _notifier => ref.read(videoPlayerProvider(widget.asset.id).notifier);
@override
void initState() {
@ -304,7 +304,7 @@ class _NativeVideoViewerState extends ConsumerState<NativeVideoViewer> with Widg
@override
Widget build(BuildContext context) {
final isCasting = ref.watch(castProvider.select((c) => c.isCasting));
final status = ref.watch(videoPlayerProvider(widget.asset.heroTag).select((v) => v.status));
final status = ref.watch(videoPlayerProvider(widget.asset.id).select((v) => v.status));
return IgnorePointer(
child: Stack(

View file

@ -67,9 +67,9 @@ class AssetViewerStateNotifier extends Notifier<AssetViewerState> {
}
state = state.copyWith(showingDetails: showing, showingControls: showing ? true : state.showingControls);
final heroTag = state.currentAsset?.heroTag;
if (heroTag != null) {
final notifier = ref.read(videoPlayerProvider(heroTag).notifier);
final id = state.currentAsset?.id;
if (id != null) {
final notifier = ref.read(videoPlayerProvider(id).notifier);
showing ? notifier.hold() : notifier.release();
}
}

View file

@ -43,4 +43,31 @@ void main() {
expect(localOnly.refersToSameAsset(remoteOnly), isTrue);
});
});
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.id, searchCopy.id);
});
test('survives a local asset gaining a remote id', () {
final localOnly = LocalAssetFactory.create(id: 'local-1');
final uploaded = localOnly.copyWith(remoteId: 'asset-1');
expect(localOnly.heroTag, isNot(uploaded.heroTag));
expect(uploaded.id, localOnly.id);
});
test('differs between assets', () {
final a = RemoteAssetFactory.create(id: 'asset-1');
final b = RemoteAssetFactory.create(id: 'asset-2');
final local = LocalAssetFactory.create(id: 'local-1');
expect(a.id, isNot(b.id));
expect(local.id, isNot(a.id));
});
});
}

View file

@ -0,0 +1,85 @@
import 'dart:async';
import 'package:flutter/material.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/services/timeline.service.dart';
import 'package:immich_mobile/presentation/actions/action.dart';
import 'package:immich_mobile/presentation/widgets/asset_viewer/bottom_bar.widget.dart';
import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.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/readonly_mode.provider.dart';
import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart';
import 'package:immich_mobile/providers/routes.provider.dart';
import 'package:immich_mobile/services/gcast.service.dart';
import 'package:immich_mobile/utils/asset_filter.dart';
import 'package:mocktail/mocktail.dart';
import 'package:native_video_player/native_video_player.dart';
import '../../../service.mocks.dart';
import '../../../unit/factories/remote_asset_factory.dart';
import '../../../widget_tester_extensions.dart';
class MockNativeVideoPlayerController extends Mock implements NativeVideoPlayerController {}
class MockTimelineService extends Mock implements TimelineService {}
class TestReadOnlyModeNotifier extends ReadOnlyModeNotifier {
@override
bool build() => true;
}
void main() {
testWidgets('player stays bound after local id arrives', (tester) async {
final searchCopy = RemoteAssetFactory.create(type: .video);
final mergedCopy = searchCopy.copyWith(localId: 'local-1', isFavorite: true);
final assetService = MockAssetService();
final controller = MockNativeVideoPlayerController();
final timeline = MockTimelineService();
final updates = StreamController<BaseAsset?>(sync: true);
when(() => assetService.watchAsset(searchCopy)).thenAnswer((_) => updates.stream);
when(controller.play).thenAnswer((_) async {});
when(controller.pause).thenAnswer((_) async {});
when(() => timeline.origin).thenReturn(.search);
late WidgetRef ref;
await tester.pumpConsumerWidget(
Consumer(
builder: (context, widgetRef, _) {
ref = widgetRef;
return const ViewerBottomBar();
},
),
overrides: [
assetServiceProvider.overrideWithValue(assetService),
assetsActionProvider(ActionSource.viewer).overrideWithValue(const AssetFilter<BaseAsset>({})),
gCastServiceProvider.overrideWithValue(MockGCastService()),
inLockedViewProvider.overrideWithValue(true),
ownedAssetsActionProvider(ActionSource.viewer).overrideWithValue(const AssetFilter<RemoteAsset>({})),
readonlyModeProvider.overrideWith(TestReadOnlyModeNotifier.new),
timelineServiceProvider.overrideWithValue(timeline),
],
);
final viewer = ref.read(assetViewerProvider.notifier);
viewer.setAsset(searchCopy);
await tester.pump();
ref.read(videoPlayerProvider(searchCopy.id).notifier).attachController(controller);
updates.add(mergedCopy);
await tester.pump();
await tester.tap(find.byType(IconButton));
await tester.pump();
verify(controller.play).called(1);
viewer.setShowingDetails(true);
await tester.pump();
verify(controller.pause).called(1);
await tester.pumpWidget(const SizedBox.shrink());
unawaited(updates.close());
});
}