From b4613186412412786232746a63063c418ed23b03 Mon Sep 17 00:00:00 2001 From: Mert Alev <38161441+Ulbert@users.noreply.github.com> Date: Sun, 3 Nov 2024 15:10:27 -0500 Subject: [PATCH] fixed memory leak --- .../lib/pages/common/gallery_viewer.page.dart | 90 ++++++++------- .../lib/pages/common/native_video_loader.dart | 105 +++++++++++------- mobile/lib/widgets/memories/memory_card.dart | 1 - 3 files changed, 118 insertions(+), 78 deletions(-) diff --git a/mobile/lib/pages/common/gallery_viewer.page.dart b/mobile/lib/pages/common/gallery_viewer.page.dart index bf76d396f0..fe03a922fd 100644 --- a/mobile/lib/pages/common/gallery_viewer.page.dart +++ b/mobile/lib/pages/common/gallery_viewer.page.dart @@ -47,7 +47,7 @@ class GalleryViewerPage extends HookConsumerWidget { this.initialIndex = 0, this.heroOffset = 0, this.showStack = false, - }) : controller = PageController(initialPage: initialIndex); + }) : controller = PageController(initialPage: initialIndex, keepPage: false); final PageController controller; @@ -56,16 +56,13 @@ class GalleryViewerPage extends HookConsumerWidget { final settings = ref.watch(appSettingsServiceProvider); final loadAsset = renderList.loadAsset; final totalAssets = useState(renderList.totalAssets); - final shouldLoopVideo = useState(AppSettingsEnum.loopVideo.defaultValue); + final shouldLoopVideo = + useState(settings.getSetting(AppSettingsEnum.loopVideo)); final isZoomed = useState(false); final isPlayingVideo = useState(false); - final localPosition = useState(null); - final currentIndex = useState(initialIndex); + final localPosition = useRef(null); + final currentIndex = useValueNotifier(initialIndex); final currentAsset = loadAsset(currentIndex.value); - // Update is playing motion video - ref.listen(videoPlaybackValueProvider.select((v) => v.state), (_, state) { - isPlayingVideo.value = state == VideoPlaybackState.playing; - }); final stackIndex = useState(-1); final stack = showStack && currentAsset.stackCount > 0 @@ -80,28 +77,26 @@ class GalleryViewerPage extends HookConsumerWidget { : stackElements.elementAt(stackIndex.value); final isMotionPhoto = asset.livePhotoVideoId != null; + // Update is playing motion video + if (isMotionPhoto) { + ref.listen(videoPlaybackValueProvider.select((v) => v.state), (_, state) { + isPlayingVideo.value = state == VideoPlaybackState.playing; + }); + } // Listen provider to prevent autoDispose when navigating to other routes from within the gallery page ref.listen(currentAssetProvider, (_, __) {}); useEffect( () { // Delay state update to after the execution of build method - Future.microtask( - () => ref.read(currentAssetProvider.notifier).set(asset), - ); + ref.read(currentAssetProvider.notifier).set(asset); + // Future.microtask( + // () => ref.read(currentAssetProvider.notifier).set(asset), + // ); return null; }, [asset], ); - useEffect( - () { - shouldLoopVideo.value = - settings.getSetting(AppSettingsEnum.loopVideo); - return null; - }, - [], - ); - Future precacheNextImage(int index) async { void onError(Object exception, StackTrace? stackTrace) { // swallow error silently @@ -110,6 +105,7 @@ class GalleryViewerPage extends HookConsumerWidget { try { if (index < totalAssets.value && index >= 0) { + log.info('Precaching next image at index $index'); final asset = loadAsset(index); await precacheImage( ImmichImage.imageProvider(asset: asset), @@ -189,7 +185,9 @@ class GalleryViewerPage extends HookConsumerWidget { } else { SystemChrome.setEnabledSystemUIMode(SystemUiMode.immersive); } - isPlayingVideo.value = false; + if (isMotionPhoto) { + isPlayingVideo.value = false; + } return null; }, [], @@ -275,6 +273,8 @@ class GalleryViewerPage extends HookConsumerWidget { isZoomed.value = state != PhotoViewScaleState.initial; ref.read(showControlsProvider.notifier).show = !isZoomed.value; }, + // wantKeepAlive: true, + // gaplessPlayback: true, loadingBuilder: (context, event, index) => ClipRect( child: Stack( fit: StackFit.expand, @@ -302,13 +302,19 @@ class GalleryViewerPage extends HookConsumerWidget { itemCount: totalAssets.value, scrollDirection: Axis.horizontal, onPageChanged: (value) async { + log.info('Page changed to $value'); final next = currentIndex.value < value ? value + 1 : value - 1; ref.read(hapticFeedbackProvider.notifier).selectionClick(); + log.info('Setting current index to $value'); currentIndex.value = value; - stackIndex.value = -1; - isPlayingVideo.value = false; + if (stackIndex.value != -1) { + stackIndex.value = -1; + } + if (isMotionPhoto) { + isPlayingVideo.value = false; + } // Wait for page change animation to finish await Future.delayed(const Duration(milliseconds: 400)); @@ -324,15 +330,19 @@ class GalleryViewerPage extends HookConsumerWidget { if (a.isImage && !isPlayingVideo.value) { return PhotoViewGalleryPageOptions( - onDragStart: (_, details, __) => - localPosition.value = details.localPosition, - onDragUpdate: (_, details, __) => - handleSwipeUpDown(details), + onDragStart: (_, details, __) { + log.info('Drag start'); + localPosition.value = details.localPosition; + }, + onDragUpdate: (_, details, __) { + log.info('Drag update'); + handleSwipeUpDown(details); + }, onTapDown: (_, __, ___) { ref.read(showControlsProvider.notifier).toggle(); }, onLongPressStart: (_, __, ___) { - if (asset.livePhotoVideoId != null) { + if (isMotionPhoto) { isPlayingVideo.value = true; } }, @@ -352,24 +362,28 @@ class GalleryViewerPage extends HookConsumerWidget { ), ); } else { + log.info('Loading asset ${a.id} (index $index) as video'); + ref.read(videoPlaybackValueProvider.notifier).value = + VideoPlaybackValue.uninitialized(); return PhotoViewGalleryPageOptions.customChild( - onDragStart: (_, details, __) => - localPosition.value = details.localPosition, - onDragUpdate: (_, details, __) => - handleSwipeUpDown(details), - heroAttributes: PhotoViewHeroAttributes( - tag: isFromDto - ? '${currentAsset.remoteId}-$heroOffset' - : currentAsset.id + heroOffset, - ), + // onDragStart: (_, details, __) => + // localPosition.value = details.localPosition, + // onDragUpdate: (_, details, __) => + // handleSwipeUpDown(details), + // heroAttributes: PhotoViewHeroAttributes( + // tag: isFromDto + // ? '${currentAsset.remoteId}-$heroOffset' + // : currentAsset.id + heroOffset, + // ), filterQuality: FilterQuality.high, + initialScale: 1.0, maxScale: 1.0, minScale: 1.0, basePosition: Alignment.center, child: NativeVideoLoader( key: ValueKey(a.id), asset: a, - isMotionVideo: a.livePhotoVideoId != null, + isMotionVideo: isMotionPhoto, loopVideo: shouldLoopVideo.value, placeholder: Image( image: provider, diff --git a/mobile/lib/pages/common/native_video_loader.dart b/mobile/lib/pages/common/native_video_loader.dart index d6c857a5d2..1d7bdae221 100644 --- a/mobile/lib/pages/common/native_video_loader.dart +++ b/mobile/lib/pages/common/native_video_loader.dart @@ -50,10 +50,10 @@ class NativeVideoLoader extends HookConsumerWidget { // }, // ); - final localEntity = useMemoized( - () => asset.isLocal ? AssetEntity.fromId(asset.localId!) : null, - ); - Future calculateAspectRatio() async { + // final localEntity = useMemoized( + // () => asset.isLocal ? AssetEntity.fromId(asset.localId!) : null, + // ); + Future calculateAspectRatio(AssetEntity? localEntity) async { log.info('Calculating aspect ratio'); late final double? orientatedWidth; late final double? orientatedHeight; @@ -62,9 +62,8 @@ class NativeVideoLoader extends HookConsumerWidget { orientatedWidth = asset.orientatedWidth?.toDouble(); orientatedHeight = asset.orientatedHeight?.toDouble(); } else if (localEntity != null) { - final entity = await localEntity; - orientatedWidth = entity?.orientatedWidth.toDouble(); - orientatedHeight = entity?.orientatedHeight.toDouble(); + orientatedWidth = localEntity.orientatedWidth.toDouble(); + orientatedHeight = localEntity.orientatedHeight.toDouble(); } else { final entity = await ref.read(assetServiceProvider).loadExif(asset); orientatedWidth = entity.orientatedWidth?.toDouble(); @@ -82,16 +81,15 @@ class NativeVideoLoader extends HookConsumerWidget { return 1.0; } - final aspectRatioFuture = useMemoized(() => calculateAspectRatio()); + // final aspectRatioFuture = useMemoized(() => calculateAspectRatio()); - Future createLocalSource() async { + Future createLocalSource(AssetEntity? localEntity) async { log.info('Loading video from local storage'); - final entity = await localEntity; - if (entity == null) { + if (localEntity == null) { throw Exception('No entity found for the video'); } - final file = await entity.file; + final file = await localEntity.file; if (file == null) { throw Exception('No file found for the video'); } @@ -122,24 +120,51 @@ class NativeVideoLoader extends HookConsumerWidget { return source; } - Future createSource() { - if (asset.isLocal && asset.livePhotoVideoId == null) { - return createLocalSource(); + Future createSource(AssetEntity? localEntity) { + if (localEntity != null && asset.livePhotoVideoId == null) { + return createLocalSource(localEntity); } return createRemoteSource(); } - final createSourceFuture = useMemoized(() => createSource()); + // final createSourceFuture = useMemoized(() => createSource()); final combinedFuture = useMemoized( - () async { - final aspectRatio = await aspectRatioFuture; - final source = await createSourceFuture; - return (source, aspectRatio); - }, + () => Future.delayed(Duration(milliseconds: 1), () async { + if (!context.mounted) { + return null; + } + + final entity = + asset.isLocal ? await AssetEntity.fromId(asset.localId!) : null; + return (createSource(entity), calculateAspectRatio(entity)).wait; + }), ); + final doCleanup = useState(false); + ref.listen(videoPlaybackValueProvider.select((value) => value.state), + (_, value) { + if (value == VideoPlaybackState.initializing) { + log.info('Cleaning up video'); + doCleanup.value = true; + } + }); + + // useEffect(() { + // Future.microtask(() { + // if (!context.mounted) { + // return Future.value(null); + // } + + // return (createSourceFuture, aspectRatioFuture).wait; + // }); + + // return () { + + // } + // }, [asset.id]); + final size = MediaQuery.sizeOf(context); return SizedBox( @@ -154,25 +179,27 @@ class NativeVideoLoader extends HookConsumerWidget { child: SizedBox( height: size.height, width: size.width, - child: FutureBuilder( - key: ValueKey(asset.id), - future: combinedFuture, - // initialData: initAspectRatio, - builder: (context, snapshot) { - if (!snapshot.hasData) { - return placeholder; - } + child: doCleanup.value + ? placeholder + : FutureBuilder( + key: ValueKey(asset.id), + future: combinedFuture, + // initialData: initAspectRatio, + builder: (context, snapshot) { + if (!snapshot.hasData) { + return placeholder; + } - return NativeVideoViewerPage( - videoSource: snapshot.data!.$1, - duration: asset.duration, - aspectRatio: snapshot.data!.$2, - isMotionVideo: isMotionVideo, - hideControlsTimer: hideControlsTimer, - loopVideo: loopVideo, - ); - }, - ), + return NativeVideoViewerPage( + videoSource: snapshot.data!.$1, + aspectRatio: snapshot.data!.$2, + duration: asset.duration, + isMotionVideo: isMotionVideo, + hideControlsTimer: hideControlsTimer, + loopVideo: loopVideo, + ); + }, + ), ), ), ), diff --git a/mobile/lib/widgets/memories/memory_card.dart b/mobile/lib/widgets/memories/memory_card.dart index 39b5058646..f20bf474f8 100644 --- a/mobile/lib/widgets/memories/memory_card.dart +++ b/mobile/lib/widgets/memories/memory_card.dart @@ -5,7 +5,6 @@ import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:immich_mobile/entities/asset.entity.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/pages/common/native_video_loader.dart'; -import 'package:immich_mobile/pages/common/native_video_viewer.page.dart'; import 'package:immich_mobile/utils/hooks/blurhash_hook.dart'; import 'package:immich_mobile/widgets/common/immich_image.dart';