Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(web/server): Face thumbnail selection #3081

Merged
merged 16 commits into from
Jul 2, 2023
3 changes: 2 additions & 1 deletion mobile/openapi/doc/PersonUpdateDto.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 35 additions & 7 deletions mobile/openapi/lib/model/person_update_dto.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions mobile/openapi/test/person_update_dto_test.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions server/immich-openapi-specs.json
Original file line number Diff line number Diff line change
Expand Up @@ -5816,12 +5816,14 @@
"type": "object",
"properties": {
"name": {
"type": "string"
"type": "string",
"description": "Person name."
},
"featureFaceAssetId": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about primary asset id? This id is also used for matching against right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the asset to crop the face of the person from

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I am mistaken the API doesn't ever reference asset_face, AssetFaceEntity, or faces ever. That is an implementation detail. All the external client knows is that a person has a thumbnail and then can update it with an asset id. Personally, I think calling it something with "face" makes me think you are referring to the asset face table and that seems like an implementation detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does here

async getFaceById({ personId, assetId }: AssetFaceId): Promise<AssetFaceEntity | null> {
return this.assetFaceRepository.findOneBy({ assetId, personId });
}

"type": "string",
"description": "Asset is used to get the feature face thumbnail."
}
},
"required": [
"name"
]
}
},
"QueueStatusDto": {
"type": "object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ describe(FacialRecognitionService.name, () => {
personId: 'person-1',
assetId: 'asset-id',
embedding: [1, 2, 3, 4],
boundingBoxX1: 100,
boundingBoxY1: 100,
boundingBoxX2: 200,
boundingBoxY2: 200,
imageHeight: 500,
imageWidth: 400,
});
});

Expand All @@ -207,6 +213,12 @@ describe(FacialRecognitionService.name, () => {
personId: 'person-1',
assetId: 'asset-id',
embedding: [1, 2, 3, 4],
boundingBoxX1: 100,
boundingBoxY1: 100,
boundingBoxX2: 200,
boundingBoxY2: 200,
imageHeight: 500,
imageWidth: 400,
});
expect(jobMock.queue.mock.calls).toEqual([
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,16 @@ export class FacialRecognitionService {

const faceId: AssetFaceId = { assetId: asset.id, personId };

await this.faceRepository.create({ ...faceId, embedding });
await this.faceRepository.create({
...faceId,
embedding,
imageHeight: rest.imageHeight,
imageWidth: rest.imageWidth,
boundingBoxX1: rest.boundingBox.x1,
boundingBoxX2: rest.boundingBox.x2,
boundingBoxY1: rest.boundingBox.y1,
boundingBoxY2: rest.boundingBox.y2,
});
await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_FACE, data: faceId });
}

Expand Down
16 changes: 13 additions & 3 deletions server/src/domain/person/person.dto.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
import { AssetFaceEntity, PersonEntity } from '@app/infra/entities';
import { IsNotEmpty, IsString } from 'class-validator';
import { IsOptional, IsString } from 'class-validator';

export class PersonUpdateDto {
@IsNotEmpty()
/**
* Person name.
*/
@IsOptional()
@IsString()
name!: string;
name?: string;

/**
* Asset is used to get the feature face thumbnail.
*/
@IsOptional()
@IsString()
featureFaceAssetId?: string;
}

export class PersonResponseDto {
Expand Down
6 changes: 4 additions & 2 deletions server/src/domain/person/person.repository.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AssetEntity, PersonEntity } from '@app/infra/entities';

import { AssetFaceId } from '@app/domain';
import { AssetEntity, AssetFaceEntity, PersonEntity } from '@app/infra/entities';
export const IPersonRepository = 'IPersonRepository';

export interface PersonSearchOptions {
Expand All @@ -16,4 +16,6 @@ export interface IPersonRepository {
update(entity: Partial<PersonEntity>): Promise<PersonEntity>;
delete(entity: PersonEntity): Promise<PersonEntity | null>;
deleteAll(): Promise<number>;

getFaceByAssetId(payload: AssetFaceId): Promise<AssetFaceEntity>;
alextran1502 marked this conversation as resolved.
Show resolved Hide resolved
}
31 changes: 31 additions & 0 deletions server/src/domain/person/person.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { BadRequestException, NotFoundException } from '@nestjs/common';
import {
assetEntityStub,
authStub,
faceStub,
newJobRepositoryMock,
newPersonRepositoryMock,
newStorageRepositoryMock,
Expand Down Expand Up @@ -108,6 +109,36 @@ describe(PersonService.name, () => {
data: { ids: [assetEntityStub.image.id] },
});
});

it("should update a person's thumbnailPath", async () => {
personMock.getById.mockResolvedValue(personStub.withName);
personMock.getFaceByAssetId.mockResolvedValue(faceStub.face1);

await expect(
sut.update(authStub.admin, 'person-1', { featureFaceAssetId: faceStub.face1.assetId }),
).resolves.toEqual(responseDto);

expect(personMock.getById).toHaveBeenCalledWith('admin_id', 'person-1');
expect(personMock.getFaceByAssetId).toHaveBeenCalledWith({
assetId: faceStub.face1.assetId,
personId: 'person-1',
});
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.GENERATE_FACE_THUMBNAIL,
data: {
assetId: faceStub.face1.assetId,
personId: 'person-1',
boundingBox: {
x1: faceStub.face1.boundingBoxX1,
x2: faceStub.face1.boundingBoxX2,
y1: faceStub.face1.boundingBoxY1,
y2: faceStub.face1.boundingBoxY2,
},
imageHeight: faceStub.face1.imageHeight,
imageWidth: faceStub.face1.imageWidth,
},
});
});
});

describe('handlePersonCleanup', () => {
Expand Down
41 changes: 37 additions & 4 deletions server/src/domain/person/person.service.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { PersonEntity } from '@app/infra/entities';
import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common';
import { AssetResponseDto, mapAsset } from '../asset';
import { AuthUserDto } from '../auth';
Expand Down Expand Up @@ -52,18 +53,50 @@ export class PersonService {
}

async update(authUser: AuthUserDto, personId: string, dto: PersonUpdateDto): Promise<PersonResponseDto> {
const exists = await this.repository.getById(authUser.id, personId);
if (!exists) {
let person = await this.repository.getById(authUser.id, personId);
if (!person) {
throw new BadRequestException();
}

const person = await this.repository.update({ id: personId, name: dto.name });
if (dto.name) {
person = await this.updateName(authUser, personId, dto.name);
}

if (dto.featureFaceAssetId) {
await this.updateFaceThumbnail(personId, dto.featureFaceAssetId);
}

return mapPerson(person);
}

private async updateName(authUser: AuthUserDto, personId: string, name: string): Promise<PersonEntity> {
const person = await this.repository.update({ id: personId, name });

const relatedAsset = await this.getAssets(authUser, personId);
const assetIds = relatedAsset.map((asset) => asset.id);
await this.jobRepository.queue({ name: JobName.SEARCH_INDEX_ASSET, data: { ids: assetIds } });

return mapPerson(person);
return person;
}

private async updateFaceThumbnail(personId: string, assetId: string): Promise<void> {
const face = await this.repository.getFaceByAssetId({ assetId, personId });
alextran1502 marked this conversation as resolved.
Show resolved Hide resolved

return await this.jobRepository.queue({
name: JobName.GENERATE_FACE_THUMBNAIL,
data: {
assetId: assetId,
personId,
boundingBox: {
x1: face.boundingBoxX1,
x2: face.boundingBoxX2,
y1: face.boundingBoxY1,
y2: face.boundingBoxY2,
},
imageHeight: face.imageHeight,
imageWidth: face.imageWidth,
},
});
}

async handlePersonCleanup() {
Expand Down
18 changes: 18 additions & 0 deletions server/src/infra/entities/asset-face.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,24 @@ export class AssetFaceEntity {
})
embedding!: number[] | null;

@Column({ default: 0, type: 'int' })
imageWidth!: number;

@Column({ default: 0, type: 'int' })
imageHeight!: number;

@Column({ default: 0, type: 'int' })
boundingBoxX1!: number;

@Column({ default: 0, type: 'int' })
boundingBoxY1!: number;

@Column({ default: 0, type: 'int' })
boundingBoxX2!: number;

@Column({ default: 0, type: 'int' })
boundingBoxY2!: number;

@ManyToOne(() => AssetEntity, (asset) => asset.faces, { onDelete: 'CASCADE', onUpdate: 'CASCADE' })
asset!: AssetEntity;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class AddDetectFaceResultInfo1688241394489 implements MigrationInterface {
name = 'AddDetectFaceResultInfo1688241394489';

public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`ALTER TABLE "asset_faces" ADD "imageWidth" integer NOT NULL DEFAULT '0'`);
await queryRunner.query(`ALTER TABLE "asset_faces" ADD "imageHeight" integer NOT NULL DEFAULT '0'`);
await queryRunner.query(`ALTER TABLE "asset_faces" ADD "boundingBoxX1" integer NOT NULL DEFAULT '0'`);
await queryRunner.query(`ALTER TABLE "asset_faces" ADD "boundingBoxY1" integer NOT NULL DEFAULT '0'`);
await queryRunner.query(`ALTER TABLE "asset_faces" ADD "boundingBoxX2" integer NOT NULL DEFAULT '0'`);
await queryRunner.query(`ALTER TABLE "asset_faces" ADD "boundingBoxY2" integer NOT NULL DEFAULT '0'`);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`ALTER TABLE "asset_faces" DROP COLUMN "boundingBoxY2"`);
await queryRunner.query(`ALTER TABLE "asset_faces" DROP COLUMN "boundingBoxX2"`);
await queryRunner.query(`ALTER TABLE "asset_faces" DROP COLUMN "boundingBoxY1"`);
await queryRunner.query(`ALTER TABLE "asset_faces" DROP COLUMN "boundingBoxX1"`);
await queryRunner.query(`ALTER TABLE "asset_faces" DROP COLUMN "imageHeight"`);
await queryRunner.query(`ALTER TABLE "asset_faces" DROP COLUMN "imageWidth"`);
}
}
6 changes: 5 additions & 1 deletion server/src/infra/repositories/person.repository.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IPersonRepository, PersonSearchOptions } from '@app/domain';
import { AssetFaceId, IPersonRepository, PersonSearchOptions } from '@app/domain';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { AssetEntity, AssetFaceEntity, PersonEntity } from '../entities';
Expand Down Expand Up @@ -76,4 +76,8 @@ export class PersonRepository implements IPersonRepository {
const { id } = await this.personRepository.save(entity);
return this.personRepository.findOneByOrFail({ id });
}

async getFaceByAssetId({ personId, assetId }: AssetFaceId): Promise<AssetFaceEntity> {
return this.assetFaceRepository.findOneByOrFail({ assetId, personId });
}
}
Loading