From 12610e4a9f55e2cf85d6ecf8e63c642792015561 Mon Sep 17 00:00:00 2001 From: Sergey Katsubo Date: Mon, 5 May 2025 18:11:21 +0300 Subject: [PATCH] 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 | 57 +++++--- e2e/test-assets | 2 +- server/src/services/metadata.service.spec.ts | 131 ++++++++++++++++--- server/src/services/metadata.service.ts | 83 +++++++++++- 4 files changed, 230 insertions(+), 43 deletions(-) diff --git a/e2e/src/api/specs/asset.e2e-spec.ts b/e2e/src/api/specs/asset.e2e-spec.ts index f5cf78bb8a..3eb848c178 100644 --- a/e2e/src/api/specs/asset.e2e-spec.ts +++ b/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 () => { diff --git a/e2e/test-assets b/e2e/test-assets index 9e3b964b08..8885d6d01c 160000 --- a/e2e/test-assets +++ b/e2e/test-assets @@ -1 +1 @@ -Subproject commit 9e3b964b080dca6f035b29b86e66454ae8aeda78 +Subproject commit 8885d6d01c12242785b6ea68f4a277334f60bc90 diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index 969da6256d..b048923b38 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/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' }); diff --git a/server/src/services/metadata.service.ts b/server/src/services/metadata.service.ts index 3f0c353d1d..17f3325f99 100644 --- a/server/src/services/metadata.service.ts +++ b/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 & { 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();