From d37e8ede3bed27aa29625a24e42fb6e611b11cca Mon Sep 17 00:00:00 2001 From: Saschl <19493808+Saschl@users.noreply.github.com> Date: Thu, 18 Jul 2024 18:07:22 +0200 Subject: [PATCH] feat: optionally generate thumbnails for invalid images (#11126) --- docs/docs/install/environment-variables.md | 23 ++++++++-------- server/src/interfaces/media.interface.ts | 1 + server/src/repositories/media.repository.ts | 3 ++- server/src/services/media.service.spec.ts | 30 +++++++++++++++++++++ server/src/services/media.service.ts | 8 +++++- server/src/services/person.service.spec.ts | 4 +++ server/src/services/person.service.ts | 1 + 7 files changed, 57 insertions(+), 13 deletions(-) diff --git a/docs/docs/install/environment-variables.md b/docs/docs/install/environment-variables.md index 3a0db9928d..e13282b45e 100644 --- a/docs/docs/install/environment-variables.md +++ b/docs/docs/install/environment-variables.md @@ -38,17 +38,18 @@ Regardless of filesystem, it is not recommended to use a network share for your ## General -| Variable | Description | Default | Containers | Workers | -| :---------------------------------- | :---------------------------------------------- | :--------------------------: | :----------------------- | :----------------- | -| `TZ` | Timezone | | server | microservices | -| `IMMICH_ENV` | Environment (production, development) | `production` | server, machine learning | api, microservices | -| `IMMICH_LOG_LEVEL` | Log Level (verbose, debug, log, warn, error) | `log` | server, machine learning | api, microservices | -| `IMMICH_MEDIA_LOCATION` | Media Location | `./upload`\*1 | server | api, microservices | -| `IMMICH_CONFIG_FILE` | Path to config file | | server | api, microservices | -| `NO_COLOR` | Set to `true` to disable color-coded log output | `false` | server, machine learning | | -| `CPU_CORES` | Amount of cores available to the immich server | auto-detected cpu core count | server | | -| `IMMICH_API_METRICS_PORT` | Port for the OTEL metrics | `8081` | server | api | -| `IMMICH_MICROSERVICES_METRICS_PORT` | Port for the OTEL metrics | `8082` | server | microservices | +| Variable | Description | Default | Containers | Workers | +| :---------------------------------- | :-------------------------------------------------- | :--------------------------: | :----------------------- | :----------------- | +| `TZ` | Timezone | | server | microservices | +| `IMMICH_ENV` | Environment (production, development) | `production` | server, machine learning | api, microservices | +| `IMMICH_LOG_LEVEL` | Log Level (verbose, debug, log, warn, error) | `log` | server, machine learning | api, microservices | +| `IMMICH_MEDIA_LOCATION` | Media Location | `./upload`\*1 | server | api, microservices | +| `IMMICH_CONFIG_FILE` | Path to config file | | server | api, microservices | +| `NO_COLOR` | Set to `true` to disable color-coded log output | `false` | server, machine learning | | +| `CPU_CORES` | Amount of cores available to the immich server | auto-detected cpu core count | server | | +| `IMMICH_API_METRICS_PORT` | Port for the OTEL metrics | `8081` | server | api | +| `IMMICH_MICROSERVICES_METRICS_PORT` | Port for the OTEL metrics | `8082` | server | microservices | +| `IMMICH_PROCESS_INVALID_IMAGES` | When `true`, generate thumbnails for invalid images | | server | microservices | \*1: With the default `WORKDIR` of `/usr/src/app`, this path will resolve to `/usr/src/app/upload`. It only need to be set if the Immich deployment method is changing. diff --git a/server/src/interfaces/media.interface.ts b/server/src/interfaces/media.interface.ts index fdf70865ef..f7389d3d06 100644 --- a/server/src/interfaces/media.interface.ts +++ b/server/src/interfaces/media.interface.ts @@ -16,6 +16,7 @@ export interface ThumbnailOptions { colorspace: string; quality: number; crop?: CropOptions; + processInvalidImages: boolean; } export interface VideoStreamInfo { diff --git a/server/src/repositories/media.repository.ts b/server/src/repositories/media.repository.ts index 6e56bf48c3..4003193ad4 100644 --- a/server/src/repositories/media.repository.ts +++ b/server/src/repositories/media.repository.ts @@ -45,7 +45,8 @@ export class MediaRepository implements IMediaRepository { } async generateThumbnail(input: string | Buffer, output: string, options: ThumbnailOptions): Promise { - const pipeline = sharp(input, { failOn: 'error', limitInputPixels: false }) + // some invalid images can still be processed by sharp, but we want to fail on them by default to avoid crashes + const pipeline = sharp(input, { failOn: options.processInvalidImages ? 'none' : 'error', limitInputPixels: false }) .pipelineColorspace(options.colorspace === Colorspace.SRGB ? 'srgb' : 'rgb16') .rotate(); diff --git a/server/src/services/media.service.spec.ts b/server/src/services/media.service.spec.ts index d4ec8e6533..173af4bd3e 100644 --- a/server/src/services/media.service.spec.ts +++ b/server/src/services/media.service.spec.ts @@ -296,6 +296,7 @@ describe(MediaService.name, () => { format, quality: 80, colorspace: Colorspace.SRGB, + processInvalidImages: false, }); expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', previewPath }); }); @@ -326,6 +327,7 @@ describe(MediaService.name, () => { format: ImageFormat.JPEG, quality: 80, colorspace: Colorspace.P3, + processInvalidImages: false, }, ); expect(assetMock.update).toHaveBeenCalledWith({ @@ -468,6 +470,7 @@ describe(MediaService.name, () => { format, quality: 80, colorspace: Colorspace.SRGB, + processInvalidImages: false, }); expect(assetMock.update).toHaveBeenCalledWith({ id: 'asset-id', thumbnailPath }); }, @@ -498,6 +501,7 @@ describe(MediaService.name, () => { size: 250, quality: 80, colorspace: Colorspace.P3, + processInvalidImages: false, }, ); expect(assetMock.update).toHaveBeenCalledWith({ @@ -524,6 +528,7 @@ describe(MediaService.name, () => { size: 250, quality: 80, colorspace: Colorspace.P3, + processInvalidImages: false, }, ], ]); @@ -548,6 +553,7 @@ describe(MediaService.name, () => { size: 250, quality: 80, colorspace: Colorspace.P3, + processInvalidImages: false, }, ], ]); @@ -570,6 +576,7 @@ describe(MediaService.name, () => { size: 250, quality: 80, colorspace: Colorspace.P3, + processInvalidImages: false, }, ); expect(mediaMock.getImageDimensions).not.toHaveBeenCalled(); @@ -590,11 +597,34 @@ describe(MediaService.name, () => { size: 250, quality: 80, colorspace: Colorspace.P3, + processInvalidImages: false, }, ); expect(mediaMock.getImageDimensions).not.toHaveBeenCalled(); }); + it('should process invalid images if enabled', async () => { + vi.stubEnv('IMMICH_PROCESS_INVALID_IMAGES', 'true'); + + assetMock.getByIds.mockResolvedValue([assetStub.imageDng]); + + await sut.handleGenerateThumbnail({ id: assetStub.image.id }); + + expect(mediaMock.generateThumbnail).toHaveBeenCalledWith( + assetStub.imageDng.originalPath, + 'upload/thumbs/user-id/as/se/asset-id-thumbnail.webp', + { + format: ImageFormat.WEBP, + size: 250, + quality: 80, + colorspace: Colorspace.P3, + processInvalidImages: true, + }, + ); + expect(mediaMock.getImageDimensions).not.toHaveBeenCalled(); + vi.unstubAllEnvs(); + }); + describe('handleGenerateThumbhash', () => { it('should skip thumbhash generation if asset not found', async () => { assetMock.getByIds.mockResolvedValue([]); diff --git a/server/src/services/media.service.ts b/server/src/services/media.service.ts index 33b80c9416..9dbf269007 100644 --- a/server/src/services/media.service.ts +++ b/server/src/services/media.service.ts @@ -199,7 +199,13 @@ export class MediaService { try { const useExtracted = didExtract && (await this.shouldUseExtractedImage(extractedPath, image.previewSize)); const colorspace = this.isSRGB(asset) ? Colorspace.SRGB : image.colorspace; - const imageOptions = { format, size, colorspace, quality: image.quality }; + const imageOptions = { + format, + size, + colorspace, + quality: image.quality, + processInvalidImages: process.env.IMMICH_PROCESS_INVALID_IMAGES === 'true', + }; const outputPath = useExtracted ? extractedPath : asset.originalPath; await this.mediaRepository.generateThumbnail(outputPath, path, imageOptions); diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index 46087436ab..70f20d6250 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -958,6 +958,7 @@ describe(PersonService.name, () => { width: 274, height: 274, }, + processInvalidImages: false, }, ); expect(personMock.update).toHaveBeenCalledWith({ @@ -987,6 +988,7 @@ describe(PersonService.name, () => { width: 510, height: 510, }, + processInvalidImages: false, }, ); }); @@ -1012,6 +1014,7 @@ describe(PersonService.name, () => { width: 408, height: 408, }, + processInvalidImages: false, }, ); }); @@ -1038,6 +1041,7 @@ describe(PersonService.name, () => { width: 588, height: 588, }, + processInvalidImages: false, }, ); }); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index 18b7ce6315..338628e469 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -559,6 +559,7 @@ export class PersonService { colorspace: image.colorspace, quality: image.quality, crop: this.getCrop({ old: { width: oldWidth, height: oldHeight }, new: { width, height } }, { x1, y1, x2, y2 }), + processInvalidImages: process.env.IMMICH_PROCESS_INVALID_IMAGES === 'true', } as const; await this.mediaRepository.generateThumbnail(inputPath, thumbnailPath, thumbnailOptions);