diff --git a/mobile/lib/providers/asset_viewer/download.provider.dart b/mobile/lib/providers/asset_viewer/download.provider.dart index 25db76b077..37d3392d88 100644 --- a/mobile/lib/providers/asset_viewer/download.provider.dart +++ b/mobile/lib/providers/asset_viewer/download.provider.dart @@ -1,9 +1,6 @@ -import 'dart:async'; - import 'package:background_downloader/background_downloader.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/models/download/download_state.model.dart'; -import 'package:immich_mobile/models/download/livephotos_medatada.model.dart'; import 'package:immich_mobile/services/download.service.dart'; class DownloadStateNotifier extends StateNotifier { @@ -17,79 +14,9 @@ class DownloadStateNotifier extends StateNotifier { taskProgress: {}, ), ) { - _downloadService.onImageDownloadStatus = _downloadImageCallback; - _downloadService.onVideoDownloadStatus = _downloadVideoCallback; - _downloadService.onLivePhotoDownloadStatus = _downloadLivePhotoCallback; _downloadService.onTaskProgress = _taskProgressCallback; } - void _updateDownloadStatus(String taskId, TaskStatus status) { - if (status == TaskStatus.canceled) { - return; - } - - state = state.copyWith( - taskProgress: {} - ..addAll(state.taskProgress) - ..addAll({ - taskId: DownloadInfo( - progress: state.taskProgress[taskId]?.progress ?? 0, - fileName: state.taskProgress[taskId]?.fileName ?? '', - status: status, - ), - }), - ); - } - - // Download live photo callback - void _downloadLivePhotoCallback(TaskStatusUpdate update) { - _updateDownloadStatus(update.task.taskId, update.status); - - switch (update.status) { - case TaskStatus.complete: - if (update.task.metaData.isEmpty) { - return; - } - final livePhotosId = LivePhotosMetadata.fromJson(update.task.metaData).id; - _downloadService.saveLivePhotos(update.task, livePhotosId); - _onDownloadComplete(update.task.taskId); - break; - - default: - break; - } - } - - // Download image callback - void _downloadImageCallback(TaskStatusUpdate update) { - _updateDownloadStatus(update.task.taskId, update.status); - - switch (update.status) { - case TaskStatus.complete: - _downloadService.saveImageWithPath(update.task); - _onDownloadComplete(update.task.taskId); - break; - - default: - break; - } - } - - // Download video callback - void _downloadVideoCallback(TaskStatusUpdate update) { - _updateDownloadStatus(update.task.taskId, update.status); - - switch (update.status) { - case TaskStatus.complete: - _downloadService.saveVideo(update.task); - _onDownloadComplete(update.task.taskId); - break; - - default: - break; - } - } - void _taskProgressCallback(TaskProgressUpdate update) { // Ignore if the task is canceled or completed if (update.progress == -2 || update.progress == -1) { @@ -110,20 +37,6 @@ class DownloadStateNotifier extends StateNotifier { ); } - void _onDownloadComplete(String id) { - Future.delayed(const Duration(seconds: 2), () { - state = state.copyWith( - taskProgress: {} - ..addAll(state.taskProgress) - ..remove(id), - ); - - if (state.taskProgress.isEmpty) { - state = state.copyWith(showProgress: false); - } - }); - } - void cancelDownload(String id) async { final isCanceled = await _downloadService.cancelDownload(id); diff --git a/mobile/lib/providers/infrastructure/action.provider.dart b/mobile/lib/providers/infrastructure/action.provider.dart index ed62b9a0e8..d1b2c3af21 100644 --- a/mobile/lib/providers/infrastructure/action.provider.dart +++ b/mobile/lib/providers/infrastructure/action.provider.dart @@ -1,7 +1,6 @@ import 'dart:async'; import 'package:auto_route/auto_route.dart'; -import 'package:background_downloader/background_downloader.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; @@ -10,7 +9,6 @@ import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/asset_edit.model.dart'; import 'package:immich_mobile/domain/services/asset.service.dart'; import 'package:immich_mobile/domain/services/remote_album.service.dart'; -import 'package:immich_mobile/models/download/livephotos_medatada.model.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; import 'package:immich_mobile/providers/backup/asset_upload_progress.provider.dart'; import 'package:immich_mobile/providers/infrastructure/album.provider.dart'; @@ -23,7 +21,6 @@ import 'package:immich_mobile/providers/user.provider.dart'; import 'package:immich_mobile/providers/websocket.provider.dart'; import 'package:immich_mobile/routing/router.dart'; import 'package:immich_mobile/services/action.service.dart'; -import 'package:immich_mobile/services/download.service.dart'; import 'package:immich_mobile/services/foreground_upload.service.dart'; import 'package:immich_mobile/utils/semver.dart'; import 'package:immich_mobile/widgets/asset_grid/delete_dialog.dart'; @@ -48,7 +45,6 @@ class ActionNotifier extends Notifier { final Logger _logger = Logger('ActionNotifier'); late ActionService _service; late ForegroundUploadService _foregroundUploadService; - late DownloadService _downloadService; late AssetService _assetService; ActionNotifier() : super(); @@ -58,29 +54,6 @@ class ActionNotifier extends Notifier { _foregroundUploadService = ref.watch(foregroundUploadServiceProvider); _service = ref.watch(actionServiceProvider); _assetService = ref.watch(assetServiceProvider); - _downloadService = ref.watch(downloadServiceProvider); - _downloadService.onImageDownloadStatus = _downloadImageCallback; - _downloadService.onVideoDownloadStatus = _downloadVideoCallback; - _downloadService.onLivePhotoDownloadStatus = _downloadLivePhotoCallback; - } - - void _downloadImageCallback(TaskStatusUpdate update) { - if (update.status == TaskStatus.complete) { - _downloadService.saveImageWithPath(update.task); - } - } - - void _downloadVideoCallback(TaskStatusUpdate update) { - if (update.status == TaskStatus.complete) { - _downloadService.saveVideo(update.task); - } - } - - void _downloadLivePhotoCallback(TaskStatusUpdate update) async { - if (update.status == TaskStatus.complete) { - final livePhotosId = LivePhotosMetadata.fromJson(update.task.metaData).id; - unawaited(_downloadService.saveLivePhotos(update.task, livePhotosId)); - } } List _getRemoteIdsForSource(ActionSource source) { diff --git a/mobile/lib/repositories/download.repository.dart b/mobile/lib/repositories/download.repository.dart index c578746a4c..ed1ffd9e3d 100644 --- a/mobile/lib/repositories/download.repository.dart +++ b/mobile/lib/repositories/download.repository.dart @@ -27,10 +27,12 @@ class DownloadRepository { void Function(TaskStatusUpdate)? onVideoDownloadStatus; - void Function(TaskStatusUpdate)? onLivePhotoDownloadStatus; - void Function(TaskProgressUpdate)? onTaskProgress; + // #29900: `taskStatusCallback` is called before the DB has been updated, causing a race between the two Live Photo tasks + // This callback instead listens directly to DB updates + void Function(TaskRecord)? onLivePhotoRecordComplete; + DownloadRepository() { _downloader.registerCallbacks( group: kDownloadGroupImage, @@ -46,9 +48,12 @@ class DownloadRepository { _downloader.registerCallbacks( group: kDownloadGroupLivePhoto, - taskStatusCallback: (update) => onLivePhotoDownloadStatus?.call(update), taskProgressCallback: (update) => onTaskProgress?.call(update), ); + + _downloader.database.updates + .where((record) => record.group == kDownloadGroupLivePhoto && record.status == TaskStatus.complete) + .listen((record) => onLivePhotoRecordComplete?.call(record)); } Future> downloadAll(List tasks) { diff --git a/mobile/lib/services/download.service.dart b/mobile/lib/services/download.service.dart index 3f2c36fa7e..b84d6ebfe8 100644 --- a/mobile/lib/services/download.service.dart +++ b/mobile/lib/services/download.service.dart @@ -1,6 +1,8 @@ +import 'dart:async'; import 'dart:io'; import 'package:background_downloader/background_downloader.dart'; +import 'package:collection/collection.dart'; import 'package:flutter/services.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/models/download/livephotos_medatada.model.dart'; @@ -18,14 +20,27 @@ class DownloadService { final Logger _log = Logger("DownloadService"); void Function(TaskStatusUpdate)? onImageDownloadStatus; void Function(TaskStatusUpdate)? onVideoDownloadStatus; - void Function(TaskStatusUpdate)? onLivePhotoDownloadStatus; void Function(TaskProgressUpdate)? onTaskProgress; + /// Active Live Photo IDs undergoing saving + final Set _savingLivePhotoIds = {}; + DownloadService(this._fileMediaRepository, this._downloadRepository) { _downloadRepository.onImageDownloadStatus = _onImageDownloadCallback; _downloadRepository.onVideoDownloadStatus = _onVideoDownloadCallback; - _downloadRepository.onLivePhotoDownloadStatus = _onLivePhotoDownloadCallback; _downloadRepository.onTaskProgress = _onTaskProgressCallback; + _downloadRepository.onLivePhotoRecordComplete = _onLivePhotoRecordComplete; + + unawaited(_savePreviouslyCompletedLivePhotos()); + } + + Future _savePreviouslyCompletedLivePhotos() async { + // Specifically fetch Live Photo video components only, as to not double fetch assets + final records = await _downloadRepository.getLiveVideoTasks(); + final completedIds = records.map((record) => LivePhotosMetadata.fromJson(record.task.metaData).id).toSet(); + for (final id in completedIds) { + await _saveLivePhotos(id); + } } void _onTaskProgressCallback(TaskProgressUpdate update) { @@ -33,18 +48,27 @@ class DownloadService { } void _onImageDownloadCallback(TaskStatusUpdate update) { + if (update.status == TaskStatus.complete) { + unawaited(_saveImageWithPath(update.task)); + } + onImageDownloadStatus?.call(update); } void _onVideoDownloadCallback(TaskStatusUpdate update) { + if (update.status == TaskStatus.complete) { + unawaited(_saveVideo(update.task)); + } + onVideoDownloadStatus?.call(update); } - void _onLivePhotoDownloadCallback(TaskStatusUpdate update) { - onLivePhotoDownloadStatus?.call(update); + void _onLivePhotoRecordComplete(TaskRecord record) async { + final livePhotosId = LivePhotosMetadata.fromJson(record.task.metaData).id; + await _saveLivePhotos(livePhotosId); } - Future saveImageWithPath(Task task) async { + Future _saveImageWithPath(Task task) async { final filePath = await task.filePath(); final title = task.filename; final relativePath = Platform.isAndroid ? 'DCIM/Immich' : null; @@ -65,7 +89,7 @@ class DownloadService { } } - Future saveVideo(Task task) async { + Future _saveVideo(Task task) async { final filePath = await task.filePath(); final title = task.filename; final relativePath = Platform.isAndroid ? 'DCIM/Immich' : null; @@ -83,14 +107,21 @@ class DownloadService { } } - Future saveLivePhotos(Task task, String livePhotosId) async { + Future _saveLivePhotos(String livePhotosId) async { final records = await _downloadRepository.getLiveVideoTasks(); - if (records.length < 2) { + final imageRecord = _findTaskRecord(records, livePhotosId, LivePhotosPart.image); + final videoRecord = _findTaskRecord(records, livePhotosId, LivePhotosPart.video); + + if (imageRecord == null || videoRecord == null) { return false; } - final imageRecord = _findTaskRecord(records, livePhotosId, LivePhotosPart.image); - final videoRecord = _findTaskRecord(records, livePhotosId, LivePhotosPart.video); + // Write semaphore for this `livePhotoId` + if (!_savingLivePhotoIds.add(livePhotosId)) { + return false; + } + + final title = imageRecord.task.filename; final imageFilePath = await imageRecord.task.filePath(); final videoFilePath = await videoRecord.task.filePath(); @@ -98,14 +129,14 @@ class DownloadService { final result = await _fileMediaRepository.saveLivePhoto( image: File(imageFilePath), video: File(videoFilePath), - title: task.filename, + title: title, ); return result != null; } on PlatformException catch (error, stack) { // Handle saving MotionPhotos on iOS if (error.code.startsWith('PHPhotosErrorDomain')) { - final result = await _fileMediaRepository.saveImageWithFile(imageFilePath, title: task.filename); + final result = await _fileMediaRepository.saveImageWithFile(imageFilePath, title: title); return result != null; } _log.severe("Error saving live photo", error, stack); @@ -125,6 +156,7 @@ class DownloadService { } await _downloadRepository.deleteRecordsWithIds([imageRecord.task.taskId, videoRecord.task.taskId]); + _savingLivePhotoIds.remove(livePhotosId); } } @@ -133,8 +165,8 @@ class DownloadService { } } -TaskRecord _findTaskRecord(List records, String livePhotosId, LivePhotosPart part) { - return records.firstWhere((record) { +TaskRecord? _findTaskRecord(List records, String livePhotosId, LivePhotosPart part) { + return records.firstWhereOrNull((record) { final metadata = LivePhotosMetadata.fromJson(record.task.metaData); return metadata.id == livePhotosId && metadata.part == part; }); diff --git a/mobile/test/providers/infrastructure/action_provider_test.dart b/mobile/test/providers/infrastructure/action_provider_test.dart index be13e58f73..7f5c8d4ec2 100644 --- a/mobile/test/providers/infrastructure/action_provider_test.dart +++ b/mobile/test/providers/infrastructure/action_provider_test.dart @@ -12,7 +12,6 @@ import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; import 'package:immich_mobile/providers/infrastructure/asset_viewer/asset.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; import 'package:immich_mobile/services/action.service.dart'; -import 'package:immich_mobile/services/download.service.dart'; import 'package:immich_mobile/services/foreground_upload.service.dart'; import 'package:mocktail/mocktail.dart'; @@ -20,8 +19,6 @@ class MockActionService extends Mock implements ActionService {} class MockAssetService extends Mock implements AssetService {} -class MockDownloadService extends Mock implements DownloadService {} - class MockForegroundUploadService extends Mock implements ForegroundUploadService {} class MockUserService extends Mock implements UserService {} @@ -67,7 +64,6 @@ void main() { overrides: [ actionServiceProvider.overrideWithValue(actionService), assetServiceProvider.overrideWithValue(assetService), - downloadServiceProvider.overrideWithValue(MockDownloadService()), foregroundUploadServiceProvider.overrideWithValue(MockForegroundUploadService()), currentUserProvider.overrideWith((ref) => CurrentUserProvider(userService)), ],