From f1f00950aa79b9f26d15ab696a8f0610ec15ee02 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 18 Dec 2025 00:02:42 -0600 Subject: [PATCH] fix: merged video in On This Device played with incorrect dimension --- mobile/lib/domain/services/asset.service.dart | 72 +++++++++++-------- .../domain/services/asset.service_test.dart | 42 ++++++++++- 2 files changed, 81 insertions(+), 33 deletions(-) diff --git a/mobile/lib/domain/services/asset.service.dart b/mobile/lib/domain/services/asset.service.dart index 3d8fddc9b7..40621c3356 100644 --- a/mobile/lib/domain/services/asset.service.dart +++ b/mobile/lib/domain/services/asset.service.dart @@ -6,6 +6,14 @@ import 'package:immich_mobile/infrastructure/repositories/local_asset.repository import 'package:immich_mobile/infrastructure/repositories/remote_asset.repository.dart'; import 'package:immich_mobile/infrastructure/utils/exif.converter.dart'; +class AssetVideoDimension { + final double? width; + final double? height; + final bool isFlipped; + + const AssetVideoDimension(this.width, this.height, this.isFlipped); +} + class AssetService { final RemoteAssetRepository _remoteAssetRepository; final DriftLocalAssetRepository _localAssetRepository; @@ -58,44 +66,48 @@ class AssetService { } Future getAspectRatio(BaseAsset asset) async { - bool isFlipped; - double? width; - double? height; + final dimension = asset is LocalAsset + ? await _getLocalAssetDimensions(asset) + : await _getRemoteAssetDimensions(asset as RemoteAsset); - if (asset.hasRemote) { - final exif = await getExif(asset); - isFlipped = ExifDtoConverter.isOrientationFlipped(exif?.orientation); - width = asset.width?.toDouble(); - height = asset.height?.toDouble(); - } else if (asset is LocalAsset) { - isFlipped = CurrentPlatform.isAndroid && (asset.orientation == 90 || asset.orientation == 270); - width = asset.width?.toDouble(); - height = asset.height?.toDouble(); - } else { - isFlipped = false; + if (dimension.width == null || dimension.height == null || dimension.height == 0) { + return 1.0; } + return dimension.isFlipped ? dimension.height! / dimension.width! : dimension.width! / dimension.height!; + } + + Future _getLocalAssetDimensions(LocalAsset asset) async { + double? width = asset.width?.toDouble(); + double? height = asset.height?.toDouble(); + int orientation = asset.orientation; + if (width == null || height == null) { - if (asset.hasRemote) { - final id = asset is LocalAsset ? asset.remoteId! : (asset as RemoteAsset).id; - final remoteAsset = await _remoteAssetRepository.get(id); - width = remoteAsset?.width?.toDouble(); - height = remoteAsset?.height?.toDouble(); - } else { - final id = asset is LocalAsset ? asset.id : (asset as RemoteAsset).localId!; - final localAsset = await _localAssetRepository.get(id); - width = localAsset?.width?.toDouble(); - height = localAsset?.height?.toDouble(); - } + final fetched = await _localAssetRepository.get(asset.id); + width = fetched?.width?.toDouble(); + height = fetched?.height?.toDouble(); + orientation = fetched?.orientation ?? 0; } - final orientedWidth = isFlipped ? height : width; - final orientedHeight = isFlipped ? width : height; - if (orientedWidth != null && orientedHeight != null && orientedHeight > 0) { - return orientedWidth / orientedHeight; + // On Android, local assets need orientation correction for 90°/270° rotations + // On iOS, the Photos framework pre-corrects dimensions + final isFlipped = CurrentPlatform.isAndroid && (orientation == 90 || orientation == 270); + return AssetVideoDimension(width, height, isFlipped); + } + + Future _getRemoteAssetDimensions(RemoteAsset asset) async { + double? width = asset.width?.toDouble(); + double? height = asset.height?.toDouble(); + + if (width == null || height == null) { + final fetched = await _remoteAssetRepository.get(asset.id); + width = fetched?.width?.toDouble(); + height = fetched?.height?.toDouble(); } - return 1.0; + final exif = await getExif(asset); + final isFlipped = ExifDtoConverter.isOrientationFlipped(exif?.orientation); + return AssetVideoDimension(width, height, isFlipped); } Future> getPlaces(String userId) { diff --git a/mobile/test/domain/services/asset.service_test.dart b/mobile/test/domain/services/asset.service_test.dart index 5e7179ffa6..ca9defc332 100644 --- a/mobile/test/domain/services/asset.service_test.dart +++ b/mobile/test/domain/services/asset.service_test.dart @@ -87,6 +87,25 @@ void main() { verify(() => mockLocalAssetRepository.get('local-1')).called(1); }); + test('uses fetched asset orientation when dimensions are missing on Android', () async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + addTearDown(() => debugDefaultTargetPlatformOverride = null); + + // Original asset has default orientation 0, but dimensions are missing + final localAsset = TestUtils.createLocalAsset(id: 'local-1', width: null, height: null, orientation: 0); + + // Fetched asset has 90° orientation and proper dimensions + final fetchedAsset = TestUtils.createLocalAsset(id: 'local-1', width: 1920, height: 1080, orientation: 90); + + when(() => mockLocalAssetRepository.get('local-1')).thenAnswer((_) async => fetchedAsset); + + final result = await sut.getAspectRatio(localAsset); + + // Should flip dimensions since fetched asset has 90° orientation + expect(result, 1080 / 1920); + verify(() => mockLocalAssetRepository.get('local-1')).called(1); + }); + test('returns 1.0 when dimensions are still unavailable after fetching', () async { final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: null, height: null); @@ -112,7 +131,9 @@ void main() { expect(result, 1.0); }); - test('handles local asset with remoteId and uses exif from remote', () async { + test('handles local asset with remoteId using local orientation not remote exif', () async { + // When a LocalAsset has a remoteId (merged), we should use local orientation + // because the width/height come from the local asset (pre-corrected on iOS) final localAsset = TestUtils.createLocalAsset( id: 'local-1', remoteId: 'remote-1', @@ -121,9 +142,24 @@ void main() { orientation: 0, ); - final exif = const ExifInfo(orientation: '6'); + final result = await sut.getAspectRatio(localAsset); - when(() => mockRemoteAssetRepository.getExif('remote-1')).thenAnswer((_) async => exif); + expect(result, 1920 / 1080); + // Should not call remote exif for LocalAsset + verifyNever(() => mockRemoteAssetRepository.getExif(any())); + }); + + test('handles local asset with remoteId and 90 degree rotation on Android', () async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + addTearDown(() => debugDefaultTargetPlatformOverride = null); + + final localAsset = TestUtils.createLocalAsset( + id: 'local-1', + remoteId: 'remote-1', + width: 1920, + height: 1080, + orientation: 90, + ); final result = await sut.getAspectRatio(localAsset);