expose id on BaseAsset instead of a separate playerKey

This commit is contained in:
Santo Shakil 2026-08-09 23:42:37 +06:00
parent 9720af93cb
commit b58cf32187
11 changed files with 26 additions and 89 deletions

View file

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

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

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

View file

@ -93,8 +93,8 @@ class _DriftSlideshowPageState extends ConsumerState<DriftSlideshowPage> 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<DriftSlideshowPage> 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<DriftSlideshowPage> 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<DriftSlideshowPage> 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(

View file

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

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.playerKey).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.playerKey).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 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();
}
}

View file

@ -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<Shadow> _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<VideoControls> createState() => _VideoControlsState();
@ -27,7 +26,7 @@ class _VideoControlsState extends ConsumerState<VideoControls> {
late final RestartableTimer _hideTimer;
AutoDisposeStateNotifierProvider<VideoPlayerNotifier, VideoPlayerState> get _provider =>
videoPlayerProvider(widget.asset.playerKey);
videoPlayerProvider(widget.videoPlayerName);
@override
void initState() {
@ -38,7 +37,7 @@ class _VideoControlsState extends ConsumerState<VideoControls> {
@override
void didUpdateWidget(covariant VideoControls oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.asset.playerKey != widget.asset.playerKey) {
if (oldWidget.videoPlayerName != widget.videoPlayerName) {
_hideTimer.reset();
}
}

View file

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

View file

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

View file

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