fix(server): handle orientation of imported face regions (#18021)

* Transform imported face RegionInfo according to Exif Orientation

* Add unit tests for re-orienting metadata face regions

* Make code DRY using ImmichTagsWithFaces instead of NonNullable

* Add e2e test for importing metadata face regions when orientation is RotateCW90

* Disable new e2e test until its asset is added to the test-assets project

* Simplify unit tests by using the same face tag definition

* Combine similar e2e tests

* Disable new e2e test until portrait-orientation-6.jpg is added to test-assets

* Fix lint error: Expected property shorthand

* Update test-assets ref to latest

* Enable new e2e test after updating test-assets

e2e/src/api/specs/asset.e2e-spec.ts

@@ -24,7 +24,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest';
 
 const locationAssetFilepath = `${testAssetDir}/metadata/gps-position/thompson-springs.jpg`;
 const ratingAssetFilepath = `${testAssetDir}/metadata/rating/mongolels.jpg`;
-const facesAssetFilepath = `${testAssetDir}/metadata/faces/portrait.jpg`;
+const facesAssetDir = `${testAssetDir}/metadata/faces`;
 
 const readTags = async (bytes: Buffer, filename: string) => {
   const filepath = join(tempDir, filename);
@@ -185,27 +185,19 @@ describe('/asset', () => {
       });
     });
 
-    it('should get the asset faces', async () => {
-      const config = await utils.getSystemConfig(admin.accessToken);
-      config.metadata.faces.import = true;
-      await updateConfig({ systemConfigDto: config }, { headers: asBearerAuth(admin.accessToken) });
-
-      // asset faces
-      const facesAsset = await utils.createAsset(admin.accessToken, {
-        assetData: {
+    describe('faces', () => {
+      const metadataFaceTests = [
+        {
+          description: 'without orientation',
           filename: 'portrait.jpg',
-          bytes: await readFile(facesAssetFilepath),
         },
-      });
-
-      await utils.waitForWebsocketEvent({ event: 'assetUpload', id: facesAsset.id });
-
-      const { status, body } = await request(app)
-        .get(`/assets/${facesAsset.id}`)
-        .set('Authorization', `Bearer ${admin.accessToken}`);
-      expect(status).toBe(200);
-      expect(body.id).toEqual(facesAsset.id);
-      expect(body.people).toMatchObject([
+        {
+          description: 'adjusting face regions to orientation',
+          filename: 'portrait-orientation-6.jpg',
+        },
+      ];
+      // should produce same resulting face region coordinates for any orientation
+      const expectedFaces = [
         {
           name: 'Marie Curie',
           birthDate: null,
@@ -240,7 +232,30 @@ describe('/asset', () => {
             },
           ],
         },
-      ]);
+      ];
+
+      it.each(metadataFaceTests)('should get the asset faces from $filename $description', async ({ filename }) => {
+        const config = await utils.getSystemConfig(admin.accessToken);
+        config.metadata.faces.import = true;
+        await updateConfig({ systemConfigDto: config }, { headers: asBearerAuth(admin.accessToken) });
+
+        const facesAsset = await utils.createAsset(admin.accessToken, {
+          assetData: {
+            filename,
+            bytes: await readFile(`${facesAssetDir}/${filename}`),
+          },
+        });
+
+        await utils.waitForWebsocketEvent({ event: 'assetUpload', id: facesAsset.id });
+
+        const { status, body } = await request(app)
+          .get(`/assets/${facesAsset.id}`)
+          .set('Authorization', `Bearer ${admin.accessToken}`);
+
+        expect(status).toBe(200);
+        expect(body.id).toEqual(facesAsset.id);
+        expect(body.people).toMatchObject(expectedFaces);
+      });
     });
 
     it('should work with a shared link', async () => {

e2e/test-assets

@@ -1 +1 @@
-Subproject commit 9e3b964b080dca6f035b29b86e66454ae8aeda78
+Subproject commit 8885d6d01c12242785b6ea68f4a277334f60bc90

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

@@ -15,21 +15,18 @@ import { tagStub } from 'test/fixtures/tag.stub';
 import { factory } from 'test/small.factory';
 import { makeStream, newTestService, ServiceMocks } from 'test/utils';
 
-const makeFaceTags = (face: Partial<{ Name: string }> = {}) => ({
+const makeFaceTags = (face: Partial<{ Name: string }> = {}, orientation?: ImmichTags['Orientation']) => ({
+  Orientation: orientation,
   RegionInfo: {
-    AppliedToDimensions: {
-      W: 100,
-      H: 100,
-      Unit: 'normalized',
-    },
+    AppliedToDimensions: { W: 1000, H: 100, Unit: 'pixel' },
     RegionList: [
       {
         Type: 'face',
         Area: {
-          X: 0.05,
-          Y: 0.05,
-          W: 0.1,
-          H: 0.1,
+          X: 0.1,
+          Y: 0.4,
+          W: 0.2,
+          H: 0.4,
           Unit: 'normalized',
         },
         ...face,
@@ -1098,11 +1095,11 @@ describe(MetadataService.name, () => {
             assetId: assetStub.primaryImage.id,
             personId: 'random-uuid',
             imageHeight: 100,
-            imageWidth: 100,
+            imageWidth: 1000,
             boundingBoxX1: 0,
-            boundingBoxX2: 10,
-            boundingBoxY1: 0,
-            boundingBoxY2: 10,
+            boundingBoxX2: 200,
+            boundingBoxY1: 20,
+            boundingBoxY2: 60,
             sourceType: SourceType.EXIF,
           },
         ],
@@ -1137,11 +1134,11 @@ describe(MetadataService.name, () => {
             assetId: assetStub.primaryImage.id,
             personId: personStub.withName.id,
             imageHeight: 100,
-            imageWidth: 100,
+            imageWidth: 1000,
             boundingBoxX1: 0,
-            boundingBoxX2: 10,
-            boundingBoxY1: 0,
-            boundingBoxY2: 10,
+            boundingBoxX2: 200,
+            boundingBoxY1: 20,
+            boundingBoxY2: 60,
             sourceType: SourceType.EXIF,
           },
         ],
@@ -1151,6 +1148,104 @@ describe(MetadataService.name, () => {
       expect(mocks.job.queueAll).not.toHaveBeenCalledWith();
     });
 
+    describe('handleFaceTagOrientation', () => {
+      const orientationTests = [
+        {
+          description: 'undefined',
+          orientation: undefined,
+          expected: { imgW: 1000, imgH: 100, x1: 0, x2: 200, y1: 20, y2: 60 },
+        },
+        {
+          description: 'Horizontal = 1',
+          orientation: ExifOrientation.Horizontal,
+          expected: { imgW: 1000, imgH: 100, x1: 0, x2: 200, y1: 20, y2: 60 },
+        },
+        {
+          description: 'MirrorHorizontal = 2',
+          orientation: ExifOrientation.MirrorHorizontal,
+          expected: { imgW: 1000, imgH: 100, x1: 800, x2: 1000, y1: 20, y2: 60 },
+        },
+        {
+          description: 'Rotate180 = 3',
+          orientation: ExifOrientation.Rotate180,
+          expected: { imgW: 1000, imgH: 100, x1: 800, x2: 1000, y1: 40, y2: 80 },
+        },
+        {
+          description: 'MirrorVertical = 4',
+          orientation: ExifOrientation.MirrorVertical,
+          expected: { imgW: 1000, imgH: 100, x1: 0, x2: 200, y1: 40, y2: 80 },
+        },
+        {
+          description: 'MirrorHorizontalRotate270CW = 5',
+          orientation: ExifOrientation.MirrorHorizontalRotate270CW,
+          expected: { imgW: 100, imgH: 1000, x1: 20, x2: 60, y1: 0, y2: 200 },
+        },
+        {
+          description: 'Rotate90CW = 6',
+          orientation: ExifOrientation.Rotate90CW,
+          expected: { imgW: 100, imgH: 1000, x1: 40, x2: 80, y1: 0, y2: 200 },
+        },
+        {
+          description: 'MirrorHorizontalRotate90CW = 7',
+          orientation: ExifOrientation.MirrorHorizontalRotate90CW,
+          expected: { imgW: 100, imgH: 1000, x1: 40, x2: 80, y1: 800, y2: 1000 },
+        },
+        {
+          description: 'Rotate270CW = 8',
+          orientation: ExifOrientation.Rotate270CW,
+          expected: { imgW: 100, imgH: 1000, x1: 20, x2: 60, y1: 800, y2: 1000 },
+        },
+      ];
+
+      it.each(orientationTests)(
+        'should transform RegionInfo geometry according to exif orientation $description',
+        async ({ orientation, expected }) => {
+          const { imgW, imgH, x1, x2, y1, y2 } = expected;
+
+          mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.primaryImage);
+          mocks.systemMetadata.get.mockResolvedValue({ metadata: { faces: { import: true } } });
+          mockReadTags(makeFaceTags({ Name: personStub.withName.name }, orientation));
+          mocks.person.getDistinctNames.mockResolvedValue([]);
+          mocks.person.createAll.mockResolvedValue([personStub.withName.id]);
+          mocks.person.update.mockResolvedValue(personStub.withName);
+          await sut.handleMetadataExtraction({ id: assetStub.primaryImage.id });
+          expect(mocks.assetJob.getForMetadataExtraction).toHaveBeenCalledWith(assetStub.primaryImage.id);
+          expect(mocks.person.getDistinctNames).toHaveBeenCalledWith(assetStub.primaryImage.ownerId, {
+            withHidden: true,
+          });
+          expect(mocks.person.createAll).toHaveBeenCalledWith([
+            expect.objectContaining({ name: personStub.withName.name }),
+          ]);
+          expect(mocks.person.refreshFaces).toHaveBeenCalledWith(
+            [
+              {
+                id: 'random-uuid',
+                assetId: assetStub.primaryImage.id,
+                personId: 'random-uuid',
+                imageWidth: imgW,
+                imageHeight: imgH,
+                boundingBoxX1: x1,
+                boundingBoxX2: x2,
+                boundingBoxY1: y1,
+                boundingBoxY2: y2,
+                sourceType: SourceType.EXIF,
+              },
+            ],
+            [],
+          );
+          expect(mocks.person.updateAll).toHaveBeenCalledWith([
+            { id: 'random-uuid', ownerId: 'admin-id', faceAssetId: 'random-uuid' },
+          ]);
+          expect(mocks.job.queueAll).toHaveBeenCalledWith([
+            {
+              name: JobName.GENERATE_PERSON_THUMBNAIL,
+              data: { id: personStub.withName.id },
+            },
+          ]);
+        },
+      );
+    });
+
     it('should handle invalid modify date', async () => {
       mocks.assetJob.getForMetadataExtraction.mockResolvedValue(assetStub.image);
       mockReadTags({ ModifyDate: '00:00:00.000' });

server/src/services/metadata.service.ts

@@ -596,6 +596,80 @@ export class MetadataService extends BaseService {
     );
   }
 
+  private orientRegionInfo(
+    regionInfo: ImmichTagsWithFaces['RegionInfo'],
+    orientation: ExifOrientation | undefined,
+  ): ImmichTagsWithFaces['RegionInfo'] {
+    // skip default Orientation
+    if (orientation === undefined || orientation === ExifOrientation.Horizontal) {
+      return regionInfo;
+    }
+
+    const isSidewards = [
+      ExifOrientation.MirrorHorizontalRotate270CW,
+      ExifOrientation.Rotate90CW,
+      ExifOrientation.MirrorHorizontalRotate90CW,
+      ExifOrientation.Rotate270CW,
+    ].includes(orientation);
+
+    // swap image dimensions in AppliedToDimensions if orientation is sidewards
+    const adjustedAppliedToDimensions = isSidewards
+      ? {
+          ...regionInfo.AppliedToDimensions,
+          W: regionInfo.AppliedToDimensions.H,
+          H: regionInfo.AppliedToDimensions.W,
+        }
+      : regionInfo.AppliedToDimensions;
+
+    // update area coordinates and dimensions in RegionList assuming "normalized" unit as per MWG guidelines
+    const adjustedRegionList = regionInfo.RegionList.map((region) => {
+      let { X, Y, W, H } = region.Area;
+      switch (orientation) {
+        case ExifOrientation.MirrorHorizontal: {
+          X = 1 - X;
+          break;
+        }
+        case ExifOrientation.Rotate180: {
+          [X, Y] = [1 - X, 1 - Y];
+          break;
+        }
+        case ExifOrientation.MirrorVertical: {
+          Y = 1 - Y;
+          break;
+        }
+        case ExifOrientation.MirrorHorizontalRotate270CW: {
+          [X, Y] = [Y, X];
+          break;
+        }
+        case ExifOrientation.Rotate90CW: {
+          [X, Y] = [1 - Y, X];
+          break;
+        }
+        case ExifOrientation.MirrorHorizontalRotate90CW: {
+          [X, Y] = [1 - Y, 1 - X];
+          break;
+        }
+        case ExifOrientation.Rotate270CW: {
+          [X, Y] = [Y, 1 - X];
+          break;
+        }
+      }
+      if (isSidewards) {
+        [W, H] = [H, W];
+      }
+      return {
+        ...region,
+        Area: { ...region.Area, X, Y, W, H },
+      };
+    });
+
+    return {
+      ...regionInfo,
+      AppliedToDimensions: adjustedAppliedToDimensions,
+      RegionList: adjustedRegionList,
+    };
+  }
+
   private async applyTaggedFaces(
     asset: { id: string; ownerId: string; faces: AssetFace[]; originalPath: string },
     tags: ImmichTags,
@@ -609,13 +683,16 @@ export class MetadataService extends BaseService {
     const existingNameMap = new Map(existingNames.map(({ id, name }) => [name.toLowerCase(), id]));
     const missing: (Insertable<Person> & { ownerId: string })[] = [];
     const missingWithFaceAsset: { id: string; ownerId: string; faceAssetId: string }[] = [];
-    for (const region of tags.RegionInfo.RegionList) {
+
+    const adjustedRegionInfo = this.orientRegionInfo(tags.RegionInfo, tags.Orientation);
+    const imageWidth = adjustedRegionInfo.AppliedToDimensions.W;
+    const imageHeight = adjustedRegionInfo.AppliedToDimensions.H;
+
+    for (const region of adjustedRegionInfo.RegionList) {
       if (!region.Name) {
         continue;
       }
 
-      const imageWidth = tags.RegionInfo.AppliedToDimensions.W;
-      const imageHeight = tags.RegionInfo.AppliedToDimensions.H;
       const loweredName = region.Name.toLowerCase();
       const personId = existingNameMap.get(loweredName) || this.cryptoRepository.randomUUID();