From 0cb79102fdbacf16eb659fd26aa102272a5b9fb8 Mon Sep 17 00:00:00 2001 From: Simon Dubrulle Date: Tue, 10 Dec 2024 13:25:00 +0100 Subject: [PATCH 1/4] Creation of PhotoManagerSafe + lock in LocalSyncService + improved download UI + missing notify call --- mobile/lib/services/local_sync_service.dart | 91 +++++++------ .../ui/tools/editor/image_editor_page.dart | 5 +- .../ui/tools/editor/video_editor_page.dart | 5 +- .../file_selection_actions_widget.dart | 12 +- mobile/lib/utils/file_download_util.dart | 79 ++++++----- mobile/lib/utils/photo_manager_util.dart | 128 ++++++++++++++++++ 6 files changed, 239 insertions(+), 81 deletions(-) diff --git a/mobile/lib/services/local_sync_service.dart b/mobile/lib/services/local_sync_service.dart index 1915ac30c2..ceb070011b 100644 --- a/mobile/lib/services/local_sync_service.dart +++ b/mobile/lib/services/local_sync_service.dart @@ -23,6 +23,7 @@ import "package:photos/utils/debouncer.dart"; import "package:photos/utils/photo_manager_util.dart"; import "package:photos/utils/sqlite_util.dart"; import 'package:shared_preferences/shared_preferences.dart'; +import 'package:synchronized/synchronized.dart'; import 'package:tuple/tuple.dart'; class LocalSyncService { @@ -31,6 +32,7 @@ class LocalSyncService { late SharedPreferences _prefs; Completer? _existingSync; late Debouncer _changeCallbackDebouncer; + final Lock _lock = Lock(); static const kDbUpdationTimeKey = "db_updation_time"; static const kHasCompletedFirstImportKey = "has_completed_firstImport"; @@ -77,50 +79,57 @@ class LocalSyncService { } _existingSync = Completer(); final int ownerID = Configuration.instance.getUserID()!; - final existingLocalFileIDs = await _db.getExistingLocalFileIDs(ownerID); - _logger.info("${existingLocalFileIDs.length} localIDs were discovered"); + + // We use a lock to prevent synchronisation to occur while it is downloading + // as this introduces wrong entry in FilesDB due to race condition + // This is a fix for https://github.com/ente-io/ente/issues/4296 + await _lock.synchronized(() async { + final existingLocalFileIDs = await _db.getExistingLocalFileIDs(ownerID); + _logger.info("${existingLocalFileIDs.length} localIDs were discovered"); - final syncStartTime = DateTime.now().microsecondsSinceEpoch; - final lastDBUpdationTime = _prefs.getInt(kDbUpdationTimeKey) ?? 0; - final startTime = DateTime.now().microsecondsSinceEpoch; - if (lastDBUpdationTime != 0) { - await _loadAndStoreDiff( - existingLocalFileIDs, - fromTime: lastDBUpdationTime, - toTime: syncStartTime, - ); - } else { - // Load from 0 - 01.01.2010 - Bus.instance.fire(SyncStatusUpdate(SyncStatus.startedFirstGalleryImport)); - var startTime = 0; - var toYear = 2010; - var toTime = DateTime(toYear).microsecondsSinceEpoch; - while (toTime < syncStartTime) { + final syncStartTime = DateTime.now().microsecondsSinceEpoch; + final lastDBUpdationTime = _prefs.getInt(kDbUpdationTimeKey) ?? 0; + final startTime = DateTime.now().microsecondsSinceEpoch; + if (lastDBUpdationTime != 0) { + await _loadAndStoreDiff( + existingLocalFileIDs, + fromTime: lastDBUpdationTime, + toTime: syncStartTime, + ); + } else { + // Load from 0 - 01.01.2010 + Bus.instance.fire(SyncStatusUpdate(SyncStatus.startedFirstGalleryImport)); + var startTime = 0; + var toYear = 2010; + var toTime = DateTime(toYear).microsecondsSinceEpoch; + while (toTime < syncStartTime) { + await _loadAndStoreDiff( + existingLocalFileIDs, + fromTime: startTime, + toTime: toTime, + ); + startTime = toTime; + toYear++; + toTime = DateTime(toYear).microsecondsSinceEpoch; + } await _loadAndStoreDiff( existingLocalFileIDs, fromTime: startTime, - toTime: toTime, + toTime: syncStartTime, ); - startTime = toTime; - toYear++; - toTime = DateTime(toYear).microsecondsSinceEpoch; } - await _loadAndStoreDiff( - existingLocalFileIDs, - fromTime: startTime, - toTime: syncStartTime, - ); - } - if (!hasCompletedFirstImport()) { - await _prefs.setBool(kHasCompletedFirstImportKey, true); - await _refreshDeviceFolderCountAndCover(isFirstSync: true); - _logger.fine("first gallery import finished"); - Bus.instance - .fire(SyncStatusUpdate(SyncStatus.completedFirstGalleryImport)); - } - final endTime = DateTime.now().microsecondsSinceEpoch; - final duration = Duration(microseconds: endTime - startTime); - _logger.info("Load took " + duration.inMilliseconds.toString() + "ms"); + if (!hasCompletedFirstImport()) { + await _prefs.setBool(kHasCompletedFirstImportKey, true); + await _refreshDeviceFolderCountAndCover(isFirstSync: true); + _logger.fine("first gallery import finished"); + Bus.instance + .fire(SyncStatusUpdate(SyncStatus.completedFirstGalleryImport)); + } + final endTime = DateTime.now().microsecondsSinceEpoch; + final duration = Duration(microseconds: endTime - startTime); + _logger.info("Load took " + duration.inMilliseconds.toString() + "ms"); + }); + _existingSync?.complete(); _existingSync = null; } @@ -240,6 +249,10 @@ class LocalSyncService { } } + Lock getLock() { + return _lock; + } + bool hasGrantedPermissions() { return _prefs.getBool(kHasGrantedPermissionsKey) ?? false; } @@ -361,7 +374,7 @@ class LocalSyncService { unawaited(checkAndSync()); }); }); - PhotoManager.startChangeNotify(); + PhotoManagerSafe.startChangeNotify(null); } Future checkAndSync() async { diff --git a/mobile/lib/ui/tools/editor/image_editor_page.dart b/mobile/lib/ui/tools/editor/image_editor_page.dart index 2ef93601f2..a24aa8bec7 100644 --- a/mobile/lib/ui/tools/editor/image_editor_page.dart +++ b/mobile/lib/ui/tools/editor/image_editor_page.dart @@ -27,6 +27,7 @@ import 'package:photos/ui/tools/editor/filtered_image.dart'; import 'package:photos/ui/viewer/file/detail_page.dart'; import 'package:photos/utils/dialog_util.dart'; import 'package:photos/utils/navigation_util.dart'; +import "package:photos/utils/photo_manager_util.dart"; import 'package:photos/utils/toast_util.dart'; import 'package:syncfusion_flutter_core/theme.dart'; import 'package:syncfusion_flutter_sliders/sliders.dart'; @@ -358,7 +359,7 @@ class _ImageEditorPageState extends State { ".JPEG"; //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. - await PhotoManager.stopChangeNotify(); + await PhotoManagerSafe.stopChangeNotify(widget.originalFile.title!); final AssetEntity? newAsset = await (PhotoManager.editor.saveImage(result, filename: fileName)); final newFile = await ente.EnteFile.fromAsset( @@ -410,7 +411,7 @@ class _ImageEditorPageState extends State { showToast(context, S.of(context).oopsCouldNotSaveEdits); _logger.severe(e, s); } finally { - await PhotoManager.startChangeNotify(); + await PhotoManagerSafe.startChangeNotify(widget.originalFile.title!); } } diff --git a/mobile/lib/ui/tools/editor/video_editor_page.dart b/mobile/lib/ui/tools/editor/video_editor_page.dart index 392d93034c..34f0e9ba97 100644 --- a/mobile/lib/ui/tools/editor/video_editor_page.dart +++ b/mobile/lib/ui/tools/editor/video_editor_page.dart @@ -24,6 +24,7 @@ import "package:photos/ui/tools/editor/video_trim_page.dart"; import "package:photos/ui/viewer/file/detail_page.dart"; import "package:photos/utils/dialog_util.dart"; import "package:photos/utils/navigation_util.dart"; +import "package:photos/utils/photo_manager_util.dart"; import "package:photos/utils/toast_util.dart"; import "package:video_editor/video_editor.dart"; @@ -238,7 +239,7 @@ class _VideoEditorPageState extends State { ".mp4"; //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. - await PhotoManager.stopChangeNotify(); + await PhotoManagerSafe.stopChangeNotify(widget.file.title!); try { final AssetEntity? newAsset = @@ -299,6 +300,8 @@ class _VideoEditorPageState extends State { ); } catch (_) { await dialog.hide(); + } finally { + await PhotoManagerSafe.startChangeNotify(widget.file.title!); } } } diff --git a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart index 00c06bb2ef..dc97628682 100644 --- a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart +++ b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart @@ -822,9 +822,12 @@ class _FileSelectionActionsWidgetState } Future _download(List files) async { + final totalFiles = files.length; + int downloadedFiles = 0; + final dialog = createProgressDialog( context, - S.of(context).downloading, + S.of(context).downloading + "$downloadedFiles/$totalFiles", isDismissible: true, ); await dialog.show(); @@ -832,7 +835,12 @@ class _FileSelectionActionsWidgetState final futures = []; for (final file in files) { if (file.localID == null) { - futures.add(downloadToGallery(file)); + futures.add( + downloadToGallery(file).then((_) { + downloadedFiles++; + dialog.update(message: S.of(context).downloading + " ($downloadedFiles/$totalFiles)"); + }), + ); } } await Future.wait(futures); diff --git a/mobile/lib/utils/file_download_util.dart b/mobile/lib/utils/file_download_util.dart index 1014005c94..1841fcfca5 100644 --- a/mobile/lib/utils/file_download_util.dart +++ b/mobile/lib/utils/file_download_util.dart @@ -21,6 +21,7 @@ import "package:photos/utils/data_util.dart"; import "package:photos/utils/fake_progress.dart"; import "package:photos/utils/file_key.dart"; import "package:photos/utils/file_util.dart"; +import "package:photos/utils/photo_manager_util.dart"; final _logger = Logger("file_download_util"); @@ -189,50 +190,54 @@ Future downloadToGallery(EnteFile file) async { final bool downloadLivePhotoOnDroid = type == FileType.livePhoto && Platform.isAndroid; AssetEntity? savedAsset; - final File? fileToSave = await getFile(file); - //Disabling notifications for assets changing to insert the file into - //files db before triggering a sync. - await PhotoManager.stopChangeNotify(); - if (type == FileType.image) { - savedAsset = await PhotoManager.editor - .saveImageWithPath(fileToSave!.path, title: file.title!); - } else if (type == FileType.video) { - savedAsset = - await PhotoManager.editor.saveVideo(fileToSave!, title: file.title!); - } else if (type == FileType.livePhoto) { - final File? liveVideoFile = - await getFileFromServer(file, liveVideo: true); - if (liveVideoFile == null) { - throw AssertionError("Live video can not be null"); + // We use a lock to prevent synchronisation to occur while it is downloading + // as this introduces wrong entry in FilesDB due to race condition + // This is a fix for https://github.com/ente-io/ente/issues/4296 + await LocalSyncService.instance.getLock().synchronized(() async { + final File? fileToSave = await getFile(file); + //Disabling notifications for assets changing to insert the file into + //files db before triggering a sync. + await PhotoManagerSafe.stopChangeNotify(file.generatedID.toString()); + if (type == FileType.image) { + savedAsset = await PhotoManager.editor + .saveImageWithPath(fileToSave!.path, title: file.title!); + } else if (type == FileType.video) { + savedAsset = + await PhotoManager.editor.saveVideo(fileToSave!, title: file.title!); + } else if (type == FileType.livePhoto) { + final File? liveVideoFile = + await getFileFromServer(file, liveVideo: true); + if (liveVideoFile == null) { + throw AssertionError("Live video can not be null"); + } + if (downloadLivePhotoOnDroid) { + await _saveLivePhotoOnDroid(fileToSave!, liveVideoFile, file); + } else { + savedAsset = await PhotoManager.editor.darwin.saveLivePhoto( + imageFile: fileToSave!, + videoFile: liveVideoFile, + title: file.title!, + ); + } } - if (downloadLivePhotoOnDroid) { - await _saveLivePhotoOnDroid(fileToSave!, liveVideoFile, file); - } else { - savedAsset = await PhotoManager.editor.darwin.saveLivePhoto( - imageFile: fileToSave!, - videoFile: liveVideoFile, - title: file.title!, + if (savedAsset != null) { + file.localID = savedAsset!.id; + await FilesDB.instance.insert(file); + Bus.instance.fire( + LocalPhotosUpdatedEvent( + [file], + source: "download", + ), ); + } else if (!downloadLivePhotoOnDroid && savedAsset == null) { + _logger.severe('Failed to save assert of type $type'); } - } - - if (savedAsset != null) { - file.localID = savedAsset.id; - await FilesDB.instance.insert(file); - Bus.instance.fire( - LocalPhotosUpdatedEvent( - [file], - source: "download", - ), - ); - } else if (!downloadLivePhotoOnDroid && savedAsset == null) { - _logger.severe('Failed to save assert of type $type'); - } + }); } catch (e) { _logger.severe("Failed to save file", e); rethrow; } finally { - await PhotoManager.startChangeNotify(); + await PhotoManagerSafe.startChangeNotify(file.generatedID.toString()); LocalSyncService.instance.checkAndSync().ignore(); } } diff --git a/mobile/lib/utils/photo_manager_util.dart b/mobile/lib/utils/photo_manager_util.dart index 273d0b362e..948d1cf863 100644 --- a/mobile/lib/utils/photo_manager_util.dart +++ b/mobile/lib/utils/photo_manager_util.dart @@ -1,4 +1,7 @@ +import 'dart:async'; +import "package:logging/logging.dart"; import "package:photo_manager/photo_manager.dart"; +import 'package:synchronized/synchronized.dart'; Future requestPhotoMangerPermissions() { return PhotoManager.requestPermissionExtend( @@ -10,3 +13,128 @@ Future requestPhotoMangerPermissions() { ), ); } + +final _logger = Logger("PhotoManagerUtil"); +// This is a wrapper for safe handling of PhotoManager.startChangeNotify() and +// PhotoManager.stopChangeNotify(). Since PhotoManager is globally shared, we want +// to make sure no notification is sent while it should not. The logic is that it will +// only start if no other asset (or task) requested to stop changes, or if the expiration +// time for the asset (task) expired. '_processingAssets' should be seen as a queue of +// open requests. +class PhotoManagerSafe { + // Tracks processing assets with their expiration times + static final Map _processingAssets = {}; + + // Timer for monitoring asset processing + static Timer? _expirationTimer; + + // Synchronization lock + static final _lock = Lock(); + + // Estimate processing duration based on file size + static Duration _estimateProcessingDuration(int fileSize) { + final estimatedSeconds = (fileSize / (1024 * 1024)).ceil() * 2; + return Duration( + seconds: estimatedSeconds.clamp(5, 120), + ); + } + + // Manage asset processing state. Lock ensures no start/stop is performed + // at the same time. + static Future manageAssetProcessing({ + required String? assetId, + required bool isStarting, + int? fileSize, + }) async { + await _lock.synchronized(() async { + try { + if (isStarting) { + // Remove the asset from processing only if assetId is not null + if (assetId != null) { + _processingAssets.remove(assetId); + } + + // Restart change notify only if no assets are processing and no stop was requested + if (_processingAssets.isEmpty) { + await PhotoManager.startChangeNotify(); + } + + _stopExpirationMonitoringIfEmpty(); + } else { + // Stopping the asset + final duration = _estimateProcessingDuration( + fileSize ?? 10 * 1024 * 1024, // 10MB default + ); + + // First asset to request stop + if (_processingAssets.isEmpty) { + await PhotoManager.stopChangeNotify(); + } + + // Track the processing asset with expiration + _processingAssets[assetId] = DateTime.now().add(duration); + + _startOrContinueExpirationMonitoring(); + } + } catch (e, stackTrace) { + _logger.severe( + "${isStarting ? 'Start' : 'Stop'}ChangeNotify error for ID $assetId", + e, + stackTrace, + ); + rethrow; + } + }); + } + + // Start or continue the expiration monitoring timer + static void _startOrContinueExpirationMonitoring() { + if (_expirationTimer != null && _expirationTimer!.isActive) return; + + _expirationTimer = Timer.periodic(const Duration(seconds: 1), (timer) { + final now = DateTime.now(); + + // Remove expired assets + _processingAssets.removeWhere((assetId, expiresAt) { + final bool isExpired = expiresAt.isBefore(now); + if (isExpired) { + } + return isExpired; + }); + + // Handle asset processing completion + if (_processingAssets.isEmpty) { + + // Start ChangeNotify + try { + PhotoManager.startChangeNotify(); + } catch (e, stackTrace) { + _logger.severe("Error restarting change notify", e, stackTrace); + } + + _stopExpirationMonitoringIfEmpty(); + } + }); + } + + // Stop the expiration monitoring timer if no assets are being processed + static void _stopExpirationMonitoringIfEmpty() { + if (_processingAssets.isEmpty) { + _expirationTimer?.cancel(); + _expirationTimer = null; + } + } + + static Future stopChangeNotify(String? assetId, {int? fileSize}) => + manageAssetProcessing( + assetId: assetId, + isStarting: false, + fileSize: fileSize, + ); + + static Future startChangeNotify(String? assetId) => + manageAssetProcessing( + assetId: assetId, + isStarting: true, + ); +} \ No newline at end of file From 9658cde3815a038f74fd65267f84b4f33c63d892 Mon Sep 17 00:00:00 2001 From: Simon Dubrulle Date: Tue, 10 Dec 2024 14:34:56 +0100 Subject: [PATCH 2/4] Fixed incoherent message format in download dialog --- mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart index dc97628682..7410795f56 100644 --- a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart +++ b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart @@ -827,7 +827,7 @@ class _FileSelectionActionsWidgetState final dialog = createProgressDialog( context, - S.of(context).downloading + "$downloadedFiles/$totalFiles", + S.of(context).downloading + " ($downloadedFiles/$totalFiles)", isDismissible: true, ); await dialog.show(); From e9a8449a648d4b99d6e970bd526dcb0f0b7d35e8 Mon Sep 17 00:00:00 2001 From: Simon Dubrulle Date: Wed, 11 Dec 2024 10:14:52 +0100 Subject: [PATCH 3/4] Implemented first PR feedbacks: removed PhotoManagerSafe not needded with LocalSyncService lock --- mobile/lib/services/local_sync_service.dart | 4 +- .../ui/tools/editor/image_editor_page.dart | 5 +- .../ui/tools/editor/video_editor_page.dart | 5 +- mobile/lib/utils/file_download_util.dart | 6 +- mobile/lib/utils/photo_manager_util.dart | 128 ------------------ 5 files changed, 9 insertions(+), 139 deletions(-) diff --git a/mobile/lib/services/local_sync_service.dart b/mobile/lib/services/local_sync_service.dart index ceb070011b..d8237dd92a 100644 --- a/mobile/lib/services/local_sync_service.dart +++ b/mobile/lib/services/local_sync_service.dart @@ -129,7 +129,7 @@ class LocalSyncService { final duration = Duration(microseconds: endTime - startTime); _logger.info("Load took " + duration.inMilliseconds.toString() + "ms"); }); - + _existingSync?.complete(); _existingSync = null; } @@ -374,7 +374,7 @@ class LocalSyncService { unawaited(checkAndSync()); }); }); - PhotoManagerSafe.startChangeNotify(null); + PhotoManager.startChangeNotify(); } Future checkAndSync() async { diff --git a/mobile/lib/ui/tools/editor/image_editor_page.dart b/mobile/lib/ui/tools/editor/image_editor_page.dart index a24aa8bec7..2ef93601f2 100644 --- a/mobile/lib/ui/tools/editor/image_editor_page.dart +++ b/mobile/lib/ui/tools/editor/image_editor_page.dart @@ -27,7 +27,6 @@ import 'package:photos/ui/tools/editor/filtered_image.dart'; import 'package:photos/ui/viewer/file/detail_page.dart'; import 'package:photos/utils/dialog_util.dart'; import 'package:photos/utils/navigation_util.dart'; -import "package:photos/utils/photo_manager_util.dart"; import 'package:photos/utils/toast_util.dart'; import 'package:syncfusion_flutter_core/theme.dart'; import 'package:syncfusion_flutter_sliders/sliders.dart'; @@ -359,7 +358,7 @@ class _ImageEditorPageState extends State { ".JPEG"; //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. - await PhotoManagerSafe.stopChangeNotify(widget.originalFile.title!); + await PhotoManager.stopChangeNotify(); final AssetEntity? newAsset = await (PhotoManager.editor.saveImage(result, filename: fileName)); final newFile = await ente.EnteFile.fromAsset( @@ -411,7 +410,7 @@ class _ImageEditorPageState extends State { showToast(context, S.of(context).oopsCouldNotSaveEdits); _logger.severe(e, s); } finally { - await PhotoManagerSafe.startChangeNotify(widget.originalFile.title!); + await PhotoManager.startChangeNotify(); } } diff --git a/mobile/lib/ui/tools/editor/video_editor_page.dart b/mobile/lib/ui/tools/editor/video_editor_page.dart index 34f0e9ba97..d96f0008a7 100644 --- a/mobile/lib/ui/tools/editor/video_editor_page.dart +++ b/mobile/lib/ui/tools/editor/video_editor_page.dart @@ -24,7 +24,6 @@ import "package:photos/ui/tools/editor/video_trim_page.dart"; import "package:photos/ui/viewer/file/detail_page.dart"; import "package:photos/utils/dialog_util.dart"; import "package:photos/utils/navigation_util.dart"; -import "package:photos/utils/photo_manager_util.dart"; import "package:photos/utils/toast_util.dart"; import "package:video_editor/video_editor.dart"; @@ -239,7 +238,7 @@ class _VideoEditorPageState extends State { ".mp4"; //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. - await PhotoManagerSafe.stopChangeNotify(widget.file.title!); + await PhotoManager.stopChangeNotify(); try { final AssetEntity? newAsset = @@ -301,7 +300,7 @@ class _VideoEditorPageState extends State { } catch (_) { await dialog.hide(); } finally { - await PhotoManagerSafe.startChangeNotify(widget.file.title!); + await PhotoManager.startChangeNotify(); } } } diff --git a/mobile/lib/utils/file_download_util.dart b/mobile/lib/utils/file_download_util.dart index 1841fcfca5..d91b62efef 100644 --- a/mobile/lib/utils/file_download_util.dart +++ b/mobile/lib/utils/file_download_util.dart @@ -21,7 +21,6 @@ import "package:photos/utils/data_util.dart"; import "package:photos/utils/fake_progress.dart"; import "package:photos/utils/file_key.dart"; import "package:photos/utils/file_util.dart"; -import "package:photos/utils/photo_manager_util.dart"; final _logger = Logger("file_download_util"); @@ -197,7 +196,7 @@ Future downloadToGallery(EnteFile file) async { final File? fileToSave = await getFile(file); //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. - await PhotoManagerSafe.stopChangeNotify(file.generatedID.toString()); + await PhotoManager.stopChangeNotify(); if (type == FileType.image) { savedAsset = await PhotoManager.editor .saveImageWithPath(fileToSave!.path, title: file.title!); @@ -220,6 +219,7 @@ Future downloadToGallery(EnteFile file) async { ); } } + if (savedAsset != null) { file.localID = savedAsset!.id; await FilesDB.instance.insert(file); @@ -237,7 +237,7 @@ Future downloadToGallery(EnteFile file) async { _logger.severe("Failed to save file", e); rethrow; } finally { - await PhotoManagerSafe.startChangeNotify(file.generatedID.toString()); + await PhotoManager.startChangeNotify(); LocalSyncService.instance.checkAndSync().ignore(); } } diff --git a/mobile/lib/utils/photo_manager_util.dart b/mobile/lib/utils/photo_manager_util.dart index 948d1cf863..273d0b362e 100644 --- a/mobile/lib/utils/photo_manager_util.dart +++ b/mobile/lib/utils/photo_manager_util.dart @@ -1,7 +1,4 @@ -import 'dart:async'; -import "package:logging/logging.dart"; import "package:photo_manager/photo_manager.dart"; -import 'package:synchronized/synchronized.dart'; Future requestPhotoMangerPermissions() { return PhotoManager.requestPermissionExtend( @@ -13,128 +10,3 @@ Future requestPhotoMangerPermissions() { ), ); } - -final _logger = Logger("PhotoManagerUtil"); -// This is a wrapper for safe handling of PhotoManager.startChangeNotify() and -// PhotoManager.stopChangeNotify(). Since PhotoManager is globally shared, we want -// to make sure no notification is sent while it should not. The logic is that it will -// only start if no other asset (or task) requested to stop changes, or if the expiration -// time for the asset (task) expired. '_processingAssets' should be seen as a queue of -// open requests. -class PhotoManagerSafe { - // Tracks processing assets with their expiration times - static final Map _processingAssets = {}; - - // Timer for monitoring asset processing - static Timer? _expirationTimer; - - // Synchronization lock - static final _lock = Lock(); - - // Estimate processing duration based on file size - static Duration _estimateProcessingDuration(int fileSize) { - final estimatedSeconds = (fileSize / (1024 * 1024)).ceil() * 2; - return Duration( - seconds: estimatedSeconds.clamp(5, 120), - ); - } - - // Manage asset processing state. Lock ensures no start/stop is performed - // at the same time. - static Future manageAssetProcessing({ - required String? assetId, - required bool isStarting, - int? fileSize, - }) async { - await _lock.synchronized(() async { - try { - if (isStarting) { - // Remove the asset from processing only if assetId is not null - if (assetId != null) { - _processingAssets.remove(assetId); - } - - // Restart change notify only if no assets are processing and no stop was requested - if (_processingAssets.isEmpty) { - await PhotoManager.startChangeNotify(); - } - - _stopExpirationMonitoringIfEmpty(); - } else { - // Stopping the asset - final duration = _estimateProcessingDuration( - fileSize ?? 10 * 1024 * 1024, // 10MB default - ); - - // First asset to request stop - if (_processingAssets.isEmpty) { - await PhotoManager.stopChangeNotify(); - } - - // Track the processing asset with expiration - _processingAssets[assetId] = DateTime.now().add(duration); - - _startOrContinueExpirationMonitoring(); - } - } catch (e, stackTrace) { - _logger.severe( - "${isStarting ? 'Start' : 'Stop'}ChangeNotify error for ID $assetId", - e, - stackTrace, - ); - rethrow; - } - }); - } - - // Start or continue the expiration monitoring timer - static void _startOrContinueExpirationMonitoring() { - if (_expirationTimer != null && _expirationTimer!.isActive) return; - - _expirationTimer = Timer.periodic(const Duration(seconds: 1), (timer) { - final now = DateTime.now(); - - // Remove expired assets - _processingAssets.removeWhere((assetId, expiresAt) { - final bool isExpired = expiresAt.isBefore(now); - if (isExpired) { - } - return isExpired; - }); - - // Handle asset processing completion - if (_processingAssets.isEmpty) { - - // Start ChangeNotify - try { - PhotoManager.startChangeNotify(); - } catch (e, stackTrace) { - _logger.severe("Error restarting change notify", e, stackTrace); - } - - _stopExpirationMonitoringIfEmpty(); - } - }); - } - - // Stop the expiration monitoring timer if no assets are being processed - static void _stopExpirationMonitoringIfEmpty() { - if (_processingAssets.isEmpty) { - _expirationTimer?.cancel(); - _expirationTimer = null; - } - } - - static Future stopChangeNotify(String? assetId, {int? fileSize}) => - manageAssetProcessing( - assetId: assetId, - isStarting: false, - fileSize: fileSize, - ); - - static Future startChangeNotify(String? assetId) => - manageAssetProcessing( - assetId: assetId, - isStarting: true, - ); -} \ No newline at end of file From 32ed84f48da01d706f59fc70b88cadd3b9d144c9 Mon Sep 17 00:00:00 2001 From: Simon Dubrulle Date: Fri, 13 Dec 2024 08:34:52 +0100 Subject: [PATCH 4/4] Moved getFile() outside lock + Implemented simple Download queue --- .../file_selection_actions_widget.dart | 4 +- mobile/lib/utils/file_download_util.dart | 38 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart index 7410795f56..16f23e3f73 100644 --- a/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart +++ b/mobile/lib/ui/viewer/actions/file_selection_actions_widget.dart @@ -832,11 +832,13 @@ class _FileSelectionActionsWidgetState ); await dialog.show(); try { + final downloadQueue = DownloadQueue(maxConcurrent: 5); final futures = []; for (final file in files) { if (file.localID == null) { futures.add( - downloadToGallery(file).then((_) { + downloadQueue.add(() async { + await downloadToGallery(file); downloadedFiles++; dialog.update(message: S.of(context).downloading + " ($downloadedFiles/$totalFiles)"); }), diff --git a/mobile/lib/utils/file_download_util.dart b/mobile/lib/utils/file_download_util.dart index d91b62efef..bb5fbeb323 100644 --- a/mobile/lib/utils/file_download_util.dart +++ b/mobile/lib/utils/file_download_util.dart @@ -1,3 +1,5 @@ +import "dart:async"; +import "dart:collection"; import 'dart:io'; import 'package:dio/dio.dart'; @@ -189,11 +191,11 @@ Future downloadToGallery(EnteFile file) async { final bool downloadLivePhotoOnDroid = type == FileType.livePhoto && Platform.isAndroid; AssetEntity? savedAsset; + final File? fileToSave = await getFile(file); // We use a lock to prevent synchronisation to occur while it is downloading // as this introduces wrong entry in FilesDB due to race condition // This is a fix for https://github.com/ente-io/ente/issues/4296 await LocalSyncService.instance.getLock().synchronized(() async { - final File? fileToSave = await getFile(file); //Disabling notifications for assets changing to insert the file into //files db before triggering a sync. await PhotoManager.stopChangeNotify(); @@ -278,3 +280,37 @@ Future _saveLivePhotoOnDroid( ); await IgnoredFilesService.instance.cacheAndInsert([ignoreVideoFile]); } + +class DownloadQueue { + final int maxConcurrent; + final Queue Function()> _queue = Queue(); + int _runningTasks = 0; + + DownloadQueue({this.maxConcurrent = 5}); + + Future add(Future Function() task) async { + final completer = Completer(); + _queue.add(() async { + try { + await task(); + completer.complete(); + } catch (e) { + completer.completeError(e); + } finally { + _runningTasks--; + _processQueue(); + } + return completer.future; + }); + _processQueue(); + return completer.future; + } + + void _processQueue() { + while (_runningTasks < maxConcurrent && _queue.isNotEmpty) { + final task = _queue.removeFirst(); + _runningTasks++; + task(); + } + } +} \ No newline at end of file