fix(server): extraction of Samsung Motionphoto videos (#6337)

* Fix extraction of samsung motionphoto videos

* Refactor binary tag extraction to the repository to consolidate exiftool usage

* format

* fix linting and swap argument orders

* Fix tag name and conditional order

* Add unit test

* Update server test assets submodule

* Remove old motion photo video assets when a new one is extracted

* delete first, then write

* Include motion photo asset uuid's in the filename

If the filenames are not uniquified, then we can't delete old/corrupt ones

* Fix formatting and fix/add tests

* chore: only use new uuid

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

server/e2e/jobs/specs/metadata.e2e-spec.ts

@@ -36,7 +36,7 @@ describe(`${AssetController.name} (e2e)`, () => {
     await restoreTempFolder();
   });
 
-  describe.only('should strip metadata of', () => {
+  describe('should strip metadata of', () => {
     let assetWithLocation: AssetResponseDto;
 
     beforeEach(async () => {
@@ -84,4 +84,26 @@ describe(`${AssetController.name} (e2e)`, () => {
       expect(exifData).not.toHaveProperty('GPSLatitude');
     });
   });
+
+  describe.each([
+    // These hashes were created by copying the image files to a Samsung phone,
+    // exporting the video from Samsung's stock Gallery app, and hashing them locally.
+    // This ensures that immich+exiftool are extracting the videos the same way Samsung does.
+    // DO NOT assume immich+exiftool are doing things correctly and just copy whatever hash it gives
+    // into the test here.
+    ['Samsung One UI 5.jpg', 'fr14niqCq6N20HB8rJYEvpsUVtI='],
+    ['Samsung One UI 6.jpg', 'lT9Uviw/FFJYCjfIxAGPTjzAmmw='],
+    ['Samsung One UI 6.heic', '/ejgzywvgvzvVhUYVfvkLzFBAF0='],
+  ])('should extract motionphoto video', (file, checksum) => {
+    itif(runAllTests)(`with checksum ${checksum} from ${file}`, async () => {
+      const fileContent = await fs.promises.readFile(`${IMMICH_TEST_ASSET_PATH}/formats/motionphoto/${file}`);
+
+      const response = await api.assetApi.upload(server, admin.accessToken, 'test-asset-id', { content: fileContent });
+      const asset = await api.assetApi.get(server, admin.accessToken, response.id);
+      expect(asset).toHaveProperty('livePhotoVideoId');
+      const video = await api.assetApi.get(server, admin.accessToken, asset.livePhotoVideoId as string);
+
+      expect(video.checksum).toStrictEqual(checksum);
+    });
+  });
 });

server/src/domain/metadata/metadata.service.spec.ts

@@ -1,6 +1,7 @@
 import { AssetType, ExifEntity, SystemConfigKey } from '@app/infra/entities';
 import {
   assetStub,
+  fileStub,
   newAlbumRepositoryMock,
   newAssetRepositoryMock,
   newCommunicationRepositoryMock,
@@ -16,6 +17,7 @@ import {
   probeStub,
 } from '@test';
 import { randomBytes } from 'crypto';
+import { BinaryField } from 'exiftool-vendored';
 import { Stats } from 'fs';
 import { constants } from 'fs/promises';
 import { when } from 'jest-when';
@@ -343,28 +345,66 @@ describe(MetadataService.name, () => {
       );
     });
 
-    it('should apply motion photos', async () => {
+    it('should extract the MotionPhotoVideo tag from Samsung HEIC motion photos', async () => {
       assetMock.getByIds.mockResolvedValue([{ ...assetStub.livePhotoStillAsset, livePhotoVideoId: null }]);
       metadataMock.readTags.mockResolvedValue({
         Directory: 'foo/bar/',
-        MotionPhoto: 1,
-        MicroVideo: 1,
-        MicroVideoOffset: 1,
+        MotionPhotoVideo: new BinaryField(0, ''),
+        // The below two are included to ensure that the MotionPhotoVideo tag is extracted
+        // instead of the EmbeddedVideoFile, since HEIC MotionPhotos include both
+        EmbeddedVideoFile: new BinaryField(0, ''),
+        EmbeddedVideoType: 'MotionPhoto_Data',
       });
-      storageMock.readFile.mockResolvedValue(randomBytes(512));
       cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
-      assetMock.getByChecksum.mockResolvedValue(assetStub.livePhotoMotionAsset);
+      assetMock.getByChecksum.mockResolvedValue(null);
+      assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
+      cryptoRepository.randomUUID.mockReturnValue(fileStub.livePhotoMotion.uuid);
+      const video = randomBytes(512);
+      metadataMock.extractBinaryTag.mockResolvedValue(video);
 
       await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
+      expect(metadataMock.extractBinaryTag).toHaveBeenCalledWith(
+        assetStub.livePhotoStillAsset.originalPath,
+        'MotionPhotoVideo',
+      );
       expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
-      expect(storageMock.readFile).toHaveBeenCalledWith(assetStub.livePhotoStillAsset.originalPath, expect.any(Object));
-      expect(assetMock.save).toHaveBeenCalledWith({
+      expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
+      expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
+      expect(assetMock.save).toHaveBeenNthCalledWith(1, {
         id: assetStub.livePhotoStillAsset.id,
-        livePhotoVideoId: assetStub.livePhotoMotionAsset.id,
+        livePhotoVideoId: fileStub.livePhotoMotion.uuid,
       });
     });
 
-    it('should create new motion asset if not found and link it with the photo', async () => {
+    it('should extract the EmbeddedVideo tag from Samsung JPEG motion photos', async () => {
+      assetMock.getByIds.mockResolvedValue([{ ...assetStub.livePhotoStillAsset, livePhotoVideoId: null }]);
+      metadataMock.readTags.mockResolvedValue({
+        Directory: 'foo/bar/',
+        EmbeddedVideoFile: new BinaryField(0, ''),
+        EmbeddedVideoType: 'MotionPhoto_Data',
+      });
+      cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
+      assetMock.getByChecksum.mockResolvedValue(null);
+      assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
+      cryptoRepository.randomUUID.mockReturnValue(fileStub.livePhotoMotion.uuid);
+      const video = randomBytes(512);
+      metadataMock.extractBinaryTag.mockResolvedValue(video);
+
+      await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
+      expect(metadataMock.extractBinaryTag).toHaveBeenCalledWith(
+        assetStub.livePhotoStillAsset.originalPath,
+        'EmbeddedVideoFile',
+      );
+      expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
+      expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
+      expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
+      expect(assetMock.save).toHaveBeenNthCalledWith(1, {
+        id: assetStub.livePhotoStillAsset.id,
+        livePhotoVideoId: fileStub.livePhotoMotion.uuid,
+      });
+    });
+
+    it('should extract the motion photo video from the XMP directory entry ', async () => {
       assetMock.getByIds.mockResolvedValue([{ ...assetStub.livePhotoStillAsset, livePhotoVideoId: null }]);
       metadataMock.readTags.mockResolvedValue({
         Directory: 'foo/bar/',
@@ -372,32 +412,60 @@ describe(MetadataService.name, () => {
         MicroVideo: 1,
         MicroVideoOffset: 1,
       });
+      cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
+      assetMock.getByChecksum.mockResolvedValue(null);
+      assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
+      cryptoRepository.randomUUID.mockReturnValue(fileStub.livePhotoMotion.uuid);
       const video = randomBytes(512);
       storageMock.readFile.mockResolvedValue(video);
-      cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
-      assetMock.create.mockResolvedValueOnce(assetStub.livePhotoMotionAsset);
-      assetMock.save.mockResolvedValueOnce(assetStub.livePhotoMotionAsset);
 
       await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
       expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoStillAsset.id]);
       expect(storageMock.readFile).toHaveBeenCalledWith(assetStub.livePhotoStillAsset.originalPath, expect.any(Object));
-      expect(assetMock.create).toHaveBeenCalledWith(
-        expect.objectContaining({
-          type: AssetType.VIDEO,
-          originalFileName: assetStub.livePhotoStillAsset.originalFileName,
-          isVisible: false,
-          isReadOnly: false,
-        }),
-      );
-      expect(assetMock.save).toHaveBeenCalledWith({
+      expect(assetMock.create).toHaveBeenCalled(); // This could have arguments added
+      expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
+      expect(assetMock.save).toHaveBeenNthCalledWith(1, {
         id: assetStub.livePhotoStillAsset.id,
-        livePhotoVideoId: assetStub.livePhotoMotionAsset.id,
+        livePhotoVideoId: fileStub.livePhotoMotion.uuid,
       });
-      expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
-      expect(jobMock.queue).toHaveBeenCalledWith({
-        name: JobName.METADATA_EXTRACTION,
-        data: { id: assetStub.livePhotoMotionAsset.id },
+    });
+
+    it('should delete old motion photo video assets if they do not match what is extracted', async () => {
+      assetMock.getByIds.mockResolvedValue([assetStub.livePhotoStillAsset]);
+      metadataMock.readTags.mockResolvedValue({
+        Directory: 'foo/bar/',
+        MotionPhoto: 1,
+        MicroVideo: 1,
+        MicroVideoOffset: 1,
+      });
+      cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
+      assetMock.getByChecksum.mockResolvedValue(null);
+      assetMock.create.mockResolvedValue(assetStub.livePhotoMotionAsset);
+
+      await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
+      expect(jobMock.queue).toHaveBeenNthCalledWith(2, {
+        name: JobName.ASSET_DELETION,
+        data: { id: assetStub.livePhotoStillAsset.livePhotoVideoId },
+      });
+    });
+
+    it('should not create a new motionphoto video asset if the of the extracted video matches an existing asset', async () => {
+      assetMock.getByIds.mockResolvedValue([assetStub.livePhotoStillAsset]);
+      metadataMock.readTags.mockResolvedValue({
+        Directory: 'foo/bar/',
+        MotionPhoto: 1,
+        MicroVideo: 1,
+        MicroVideoOffset: 1,
       });
+      cryptoRepository.hashSha1.mockReturnValue(randomBytes(512));
+      assetMock.getByChecksum.mockResolvedValue(assetStub.livePhotoMotionAsset);
+
+      await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
+      expect(assetMock.create).toHaveBeenCalledTimes(0);
+      expect(storageMock.writeFile).toHaveBeenCalledTimes(0);
+      // The still asset gets saved by handleMetadataExtraction, but not the video
+      expect(assetMock.save).toHaveBeenCalledTimes(1);
+      expect(jobMock.queue).toHaveBeenCalledTimes(0);
     });
 
     it('should save all metadata', async () => {

server/src/domain/metadata/metadata.service.ts

@@ -354,7 +354,7 @@ export class MetadataService {
   }
 
   private async applyMotionPhotos(asset: AssetEntity, tags: ImmichTags) {
-    if (asset.type !== AssetType.IMAGE || asset.livePhotoVideoId) {
+    if (asset.type !== AssetType.IMAGE) {
       return;
     }
 
@@ -362,6 +362,8 @@ export class MetadataService {
     const isMotionPhoto = tags.MotionPhoto;
     const isMicroVideo = tags.MicroVideo;
     const videoOffset = tags.MicroVideoOffset;
+    const hasMotionPhotoVideo = tags.MotionPhotoVideo;
+    const hasEmbeddedVideoFile = tags.EmbeddedVideoType === 'MotionPhoto_Data' && tags.EmbeddedVideoFile;
     const directory = Array.isArray(rawDirectory) ? (rawDirectory as DirectoryEntry[]) : null;
 
     let length = 0;
@@ -381,7 +383,7 @@ export class MetadataService {
       length = videoOffset;
     }
 
-    if (!length) {
+    if (!length && !hasEmbeddedVideoFile && !hasMotionPhotoVideo) {
       return;
     }
 
@@ -390,20 +392,35 @@ export class MetadataService {
     try {
       const stat = await this.storageRepository.stat(asset.originalPath);
       const position = stat.size - length - padding;
-      const video = await this.storageRepository.readFile(asset.originalPath, {
-        buffer: Buffer.alloc(length),
-        position,
-        length,
-      });
+      let video: Buffer;
+      // Samsung MotionPhoto video extraction
+      //     HEIC-encoded
+      if (hasMotionPhotoVideo) {
+        video = await this.repository.extractBinaryTag(asset.originalPath, 'MotionPhotoVideo');
+      }
+      //     JPEG-encoded; HEIC also contains these tags, so this conditional must come second
+      else if (hasEmbeddedVideoFile) {
+        video = await this.repository.extractBinaryTag(asset.originalPath, 'EmbeddedVideoFile');
+      }
+      // Default video extraction
+      else {
+        video = await this.storageRepository.readFile(asset.originalPath, {
+          buffer: Buffer.alloc(length),
+          position,
+          length,
+        });
+      }
       const checksum = this.cryptoRepository.hashSha1(video);
 
-      const motionPath = StorageCore.getAndroidMotionPath(asset);
-      this.storageCore.ensureFolders(motionPath);
-
       let motionAsset = await this.assetRepository.getByChecksum(asset.ownerId, checksum);
       if (!motionAsset) {
+        // We create a UUID in advance so that each extracted video can have a unique filename
+        // (allowing us to delete old ones if necessary)
+        const motionAssetId = this.cryptoRepository.randomUUID();
+        const motionPath = StorageCore.getAndroidMotionPath(asset, motionAssetId);
         const createdAt = asset.fileCreatedAt ?? asset.createdAt;
         motionAsset = await this.assetRepository.create({
+          id: motionAssetId,
           libraryId: asset.libraryId,
           type: AssetType.VIDEO,
           fileCreatedAt: createdAt,
@@ -419,12 +436,26 @@ export class MetadataService {
           deviceId: 'NONE',
         });
 
+        this.storageCore.ensureFolders(motionPath);
         await this.storageRepository.writeFile(motionAsset.originalPath, video);
         await this.jobRepository.queue({ name: JobName.METADATA_EXTRACTION, data: { id: motionAsset.id } });
+        await this.assetRepository.save({ id: asset.id, livePhotoVideoId: motionAsset.id });
+
+        // If the asset already had an associated livePhotoVideo, delete it, because
+        // its checksum doesn't match the checksum of the motionAsset we just extracted
+        // (if it did, getByChecksum() would've returned non-null)
+        if (asset.livePhotoVideoId) {
+          await this.jobRepository.queue({ name: JobName.ASSET_DELETION, data: { id: asset.livePhotoVideoId } });
+          this.logger.log(`Removed old motion photo video asset (${asset.livePhotoVideoId})`);
+        }
+      } else {
+        this.logger.debug(
+          `Asset ${asset.id}'s motion photo video with checksum ${checksum.toString(
+            'base64',
+          )} already exists in the repository`,
+        );
       }
 
-      await this.assetRepository.save({ id: asset.id, livePhotoVideoId: motionAsset.id });
-
       this.logger.debug(`Finished motion photo video extraction (${asset.id})`);
     } catch (error: Error | any) {
       this.logger.error(`Failed to extract live photo ${asset.originalPath}: ${error}`, error?.stack);

server/src/domain/repositories/metadata.repository.ts

@@ -1,4 +1,4 @@
-import { Tags } from 'exiftool-vendored';
+import { BinaryField, Tags } from 'exiftool-vendored';
 
 export const IMetadataRepository = 'IMetadataRepository';
 
@@ -27,6 +27,9 @@ export interface ImmichTags extends Omit<Tags, 'FocalLength' | 'Duration'> {
   ImagePixelDepth?: string;
   FocalLength?: number;
   Duration?: number | ExifDuration;
+  EmbeddedVideoType?: string;
+  EmbeddedVideoFile?: BinaryField;
+  MotionPhotoVideo?: BinaryField;
 }
 
 export interface IMetadataRepository {
@@ -35,4 +38,5 @@ export interface IMetadataRepository {
   reverseGeocode(point: GeoPoint): Promise<ReverseGeocodeResult | null>;
   readTags(path: string): Promise<ImmichTags | null>;
   writeTags(path: string, tags: Partial<Tags>): Promise<void>;
+  extractBinaryTag(tagName: string, path: string): Promise<Buffer>;
 }

server/src/domain/storage/storage.core.ts

@@ -103,8 +103,8 @@ export class StorageCore {
     return StorageCore.getNestedPath(StorageFolder.ENCODED_VIDEO, asset.ownerId, `${asset.id}.mp4`);
   }
 
-  static getAndroidMotionPath(asset: AssetEntity) {
-    return StorageCore.getNestedPath(StorageFolder.ENCODED_VIDEO, asset.ownerId, `${asset.id}-MP.mp4`);
+  static getAndroidMotionPath(asset: AssetEntity, uuid: string) {
+    return StorageCore.getNestedPath(StorageFolder.ENCODED_VIDEO, asset.ownerId, `${uuid}-MP.mp4`);
   }
 
   static isAndroidMotionPath(originalPath: string) {

server/src/infra/repositories/metadata.repository.ts

@@ -201,6 +201,10 @@ export class MetadataRepository implements IMetadataRepository {
       }) as Promise<ImmichTags | null>;
   }
 
+  extractBinaryTag(path: string, tagName: string): Promise<Buffer> {
+    return exiftool.extractBinaryTagToBuffer(tagName, path);
+  }
+
   async writeTags(path: string, tags: Partial<Tags>): Promise<void> {
     try {
       await exiftool.write(path, tags, ['-overwrite_original']);

server/test/assets

@@ -1 +1 @@
-Subproject commit 948f353e3c9b66156c86c86cf078e0746ec1598e
+Subproject commit 61131e84ec91d316265aebe375b3155308baaa89

server/test/fixtures/asset.stub.ts

@@ -401,7 +401,7 @@ export const assetStub = {
   }),
 
   livePhotoMotionAsset: Object.freeze({
-    id: 'live-photo-motion-asset',
+    id: fileStub.livePhotoMotion.uuid,
     originalPath: fileStub.livePhotoMotion.originalPath,
     ownerId: authStub.user1.user.id,
     type: AssetType.VIDEO,

server/test/fixtures/file.stub.ts

@@ -7,7 +7,7 @@ export const fileStub = {
     size: 42,
   }),
   livePhotoMotion: Object.freeze({
-    uuid: 'random-uuid',
+    uuid: 'live-photo-motion-asset',
     originalPath: 'fake_path/asset_1.mp4',
     checksum: Buffer.from('live photo file hash', 'utf8'),
     originalName: 'asset_1.mp4',

server/test/repositories/metadata.repository.mock.ts

@@ -7,5 +7,6 @@ export const newMetadataRepositoryMock = (): jest.Mocked<IMetadataRepository> =>
     reverseGeocode: jest.fn(),
     readTags: jest.fn(),
     writeTags: jest.fn(),
+    extractBinaryTag: jest.fn(),
   };
 };