Browse Source

fix(server): use srgb pipeline for srgb images (#4101)

* added color-related exif fields

* remove metadata check, conditional pipe colorspace

* check exif metadata for srgb

* added migration

* updated e2e fixture

* uncased srgb check, search substrings

* extracted exif logic into separate function

* handle images with no bit depth or color metadata

* added unit tests
Mert 1 year ago
parent
commit
56cf9464af

+ 84 - 1
server/src/domain/media/media.service.spec.ts

@@ -1,6 +1,7 @@
 import {
   AssetType,
   Colorspace,
+  ExifEntity,
   SystemConfigKey,
   ToneMapping,
   TranscodeHWAccel,
@@ -204,6 +205,25 @@ describe(MediaService.name, () => {
 
       expect(storageMock.mkdirSync).toHaveBeenCalledWith('upload/thumbs/user-id/as/se');
       expect(mediaMock.resize).toHaveBeenCalledWith('/original/path.jpg', 'upload/thumbs/user-id/as/se/asset-id.jpeg', {
+        size: 1440,
+        format: 'jpeg',
+        quality: 80,
+        colorspace: Colorspace.SRGB,
+      });
+      expect(assetMock.save).toHaveBeenCalledWith({
+        id: 'asset-id',
+        resizePath: 'upload/thumbs/user-id/asset-id.jpeg',
+      });
+    });
+
+    it('should generate a P3 thumbnail for a wide gamut image', async () => {
+      assetMock.getByIds.mockResolvedValue([
+        { ...assetStub.image, exifInfo: { profileDescription: 'Adobe RGB', bitsPerSample: 14 } as ExifEntity },
+      ]);
+      await sut.handleGenerateJpegThumbnail({ id: assetStub.image.id });
+
+      expect(storageMock.mkdirSync).toHaveBeenCalledWith('upload/thumbs/user-id');
+      expect(mediaMock.resize).toHaveBeenCalledWith('/original/path.jpg', 'upload/thumbs/user-id/asset-id.jpeg', {
         size: 1440,
         format: 'jpeg',
         quality: 80,
@@ -287,7 +307,7 @@ describe(MediaService.name, () => {
         format: 'webp',
         size: 250,
         quality: 80,
-        colorspace: Colorspace.P3,
+        colorspace: Colorspace.SRGB,
       });
       expect(assetMock.save).toHaveBeenCalledWith({
         id: 'asset-id',
@@ -296,6 +316,22 @@ describe(MediaService.name, () => {
     });
   });
 
+  it('should generate a P3 thumbnail for a wide gamut image', async () => {
+    assetMock.getByIds.mockResolvedValue([
+      { ...assetStub.image, exifInfo: { profileDescription: 'Adobe RGB', bitsPerSample: 14 } as ExifEntity },
+    ]);
+    await sut.handleGenerateWebpThumbnail({ id: assetStub.image.id });
+
+    expect(storageMock.mkdirSync).toHaveBeenCalledWith('upload/thumbs/user-id');
+    expect(mediaMock.resize).toHaveBeenCalledWith('/original/path.jpg', 'upload/thumbs/user-id/asset-id.webp', {
+      format: 'webp',
+      size: 250,
+      quality: 80,
+      colorspace: Colorspace.P3,
+    });
+    expect(assetMock.save).toHaveBeenCalledWith({ id: 'asset-id', webpPath: 'upload/thumbs/user-id/asset-id.webp' });
+  });
+
   describe('handleGenerateThumbhashThumbnail', () => {
     it('should skip thumbhash generation if asset not found', async () => {
       assetMock.getByIds.mockResolvedValue([]);
@@ -1539,4 +1575,51 @@ describe(MediaService.name, () => {
       },
     );
   });
+
+  describe('isSRGB', () => {
+    it('should return true for srgb colorspace', () => {
+      const asset = { ...assetStub.image, exifInfo: { colorspace: 'sRGB' } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(true);
+    });
+
+    it('should return true for srgb profile description', () => {
+      const asset = { ...assetStub.image, exifInfo: { profileDescription: 'sRGB v1.31' } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(true);
+    });
+
+    it('should return true for 8-bit image with no colorspace metadata', () => {
+      const asset = { ...assetStub.image, exifInfo: { bitsPerSample: 8 } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(true);
+    });
+
+    it('should return true for image with no colorspace or bit depth metadata', () => {
+      const asset = { ...assetStub.image, exifInfo: {} as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(true);
+    });
+
+    it('should return false for non-srgb colorspace', () => {
+      const asset = { ...assetStub.image, exifInfo: { colorspace: 'Adobe RGB' } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(false);
+    });
+
+    it('should return false for non-srgb profile description', () => {
+      const asset = { ...assetStub.image, exifInfo: { profileDescription: 'sP3C' } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(false);
+    });
+
+    it('should return false for 16-bit image with no colorspace metadata', () => {
+      const asset = { ...assetStub.image, exifInfo: { bitsPerSample: 16 } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(false);
+    });
+
+    it('should return true for 16-bit image with sRGB colorspace', () => {
+      const asset = { ...assetStub.image, exifInfo: { colorspace: 'sRGB', bitsPerSample: 16 } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(true);
+    });
+
+    it('should return true for 16-bit image with sRGB profile', () => {
+      const asset = { ...assetStub.image, exifInfo: { profileDescription: 'sRGB', bitsPerSample: 16 } as ExifEntity };
+      expect(sut.isSRGB(asset)).toEqual(true);
+    });
+  });
 });

+ 16 - 2
server/src/domain/media/media.service.ts

@@ -1,4 +1,4 @@
-import { AssetEntity, AssetType, TranscodeHWAccel, TranscodePolicy, VideoCodec } from '@app/infra/entities';
+import { AssetEntity, AssetType, Colorspace, TranscodeHWAccel, TranscodePolicy, VideoCodec } from '@app/infra/entities';
 import { Inject, Injectable, Logger, UnsupportedMediaTypeException } from '@nestjs/common';
 import { IAssetRepository, WithoutProperty } from '../asset';
 import { usePagination } from '../domain.util';
@@ -163,8 +163,9 @@ export class MediaService {
   async generateImageThumbnail(asset: AssetEntity, format: 'jpeg' | 'webp') {
     const { thumbnail } = await this.configCore.getConfig();
     const size = format === 'jpeg' ? thumbnail.jpegSize : thumbnail.webpSize;
-    const thumbnailOptions = { format, size, colorspace: thumbnail.colorspace, quality: thumbnail.quality };
     const path = this.ensureThumbnailPath(asset, format);
+    const colorspace = this.isSRGB(asset) ? Colorspace.SRGB : thumbnail.colorspace;
+    const thumbnailOptions = { format, size, colorspace, quality: thumbnail.quality };
     await this.mediaRepository.resize(asset.originalPath, path, thumbnailOptions);
     return path;
   }
@@ -384,4 +385,17 @@ export class MediaService {
   ensureEncodedVideoPath(asset: AssetEntity, extension: string): string {
     return this.storageCore.ensurePath(StorageFolder.ENCODED_VIDEO, asset.ownerId, `${asset.id}.${extension}`);
   }
+
+  isSRGB(asset: AssetEntity): boolean {
+    const { colorspace, profileDescription, bitsPerSample } = asset.exifInfo ?? {};
+    if (colorspace || profileDescription) {
+      return [colorspace, profileDescription].some((s) => s?.toLowerCase().includes('srgb'));
+    } else if (bitsPerSample) {
+      // assume sRGB for 8-bit images with no color profile or colorspace metadata
+      return bitsPerSample === 8;
+    } else {
+      // assume sRGB for images with no relevant metadata
+      return true;
+    }
+  }
 }

+ 9 - 0
server/src/infra/entities/exif.entity.ts

@@ -81,6 +81,15 @@ export class ExifEntity {
   @Column({ type: 'varchar', nullable: true })
   exposureTime!: string | null;
 
+  @Column({ type: 'varchar', nullable: true })
+  profileDescription!: string | null;
+
+  @Column({ type: 'varchar', nullable: true })
+  colorspace!: string | null;
+
+  @Column({ type: 'integer', nullable: true })
+  bitsPerSample!: number | null;
+
   /* Video info */
   @Column({ type: 'float8', nullable: true })
   fps?: number | null;

+ 18 - 0
server/src/infra/migrations/1694750975773-AddExifColorSpace.ts

@@ -0,0 +1,18 @@
+import { MigrationInterface, QueryRunner } from "typeorm";
+
+export class AddExifColorSpace1694750975773 implements MigrationInterface {
+    name = 'AddExifColorSpace1694750975773'
+
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(`ALTER TABLE "exif" ADD "profileDescription" character varying`);
+        await queryRunner.query(`ALTER TABLE "exif" ADD "colorspace" character varying`);
+        await queryRunner.query(`ALTER TABLE "exif" ADD "bitsPerSample" integer`);
+    }
+
+    public async down(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(`ALTER TABLE "exif" DROP COLUMN "bitsPerSample"`);
+        await queryRunner.query(`ALTER TABLE "exif" DROP COLUMN "colorspace"`);
+        await queryRunner.query(`ALTER TABLE "exif" DROP COLUMN "profileDescription"`);
+    }
+
+}

+ 2 - 14
server/src/infra/repositories/media.repository.ts

@@ -26,24 +26,12 @@ export class MediaRepository implements IMediaRepository {
   }
 
   async resize(input: string | Buffer, output: string, options: ResizeOptions): Promise<void> {
-    let colorProfile = options.colorspace;
-    if (options.colorspace !== Colorspace.SRGB) {
-      try {
-        const { space } = await sharp(input).metadata();
-        // if the image is already in srgb, keep it that way
-        if (space === 'srgb') {
-          colorProfile = Colorspace.SRGB;
-        }
-      } catch (err) {
-        this.logger.warn(`Could not determine colorspace of image, defaulting to ${colorProfile} profile`);
-      }
-    }
     const chromaSubsampling = options.quality >= 80 ? '4:4:4' : '4:2:0'; // this is default in libvips (except the threshold is 90), but we need to set it manually in sharp
     await sharp(input, { failOn: 'none' })
-      .pipelineColorspace('rgb16')
+      .pipelineColorspace(options.colorspace === Colorspace.SRGB ? 'srgb' : 'rgb16')
       .resize(options.size, options.size, { fit: 'outside', withoutEnlargement: true })
       .rotate()
-      .withMetadata({ icc: colorProfile })
+      .withMetadata({ icc: options.colorspace })
       .toFormat(options.format, { quality: options.quality, chromaSubsampling })
       .toFile(output);
   }

+ 23 - 0
server/src/microservices/processors/metadata-extraction.processor.ts

@@ -42,9 +42,11 @@ interface ImmichTags extends Tags {
   MotionPhotoVersion?: number;
   MotionPhotoPresentationTimestampUs?: number;
   MediaGroupUUID?: string;
+  ImagePixelDepth?: string;
 }
 
 const exifDate = (dt: ExifDateTime | string | undefined) => (dt instanceof ExifDateTime ? dt?.toDate() : null);
+// exiftool returns strings when it fails to parse non-string values, so this is used where a string is not expected
 const validate = <T>(value: T): T | null => (typeof value === 'string' ? null : value ?? null);
 
 export class MetadataExtractionProcessor {
@@ -289,6 +291,8 @@ export class MetadataExtractionProcessor {
       <ExifEntity>{
         // altitude: tags.GPSAltitude ?? null,
         assetId: asset.id,
+        bitsPerSample: this.getBitsPerSample(tags),
+        colorspace: tags.ColorSpace ?? null,
         dateTimeOriginal: exifDate(firstDateTime(tags)) ?? asset.fileCreatedAt,
         exifImageHeight: validate(tags.ImageHeight),
         exifImageWidth: validate(tags.ImageWidth),
@@ -306,10 +310,29 @@ export class MetadataExtractionProcessor {
         model: tags.Model ?? null,
         modifyDate: exifDate(tags.ModifyDate) ?? asset.fileModifiedAt,
         orientation: validate(tags.Orientation)?.toString() ?? null,
+        profileDescription: tags.ProfileDescription || tags.ProfileName || null,
         projectionType: tags.ProjectionType ? String(tags.ProjectionType).toUpperCase() : null,
         timeZone: tags.tz,
       },
       tags,
     ];
   }
+
+  getBitsPerSample(tags: ImmichTags): number | null {
+    const bitDepthTags = [
+      tags.BitsPerSample,
+      tags.ComponentBitDepth,
+      tags.ImagePixelDepth,
+      tags.BitDepth,
+      tags.ColorBitDepth,
+      // `numericTags` doesn't parse values like '12 12 12'
+    ].map((tag) => (typeof tag === 'string' ? Number.parseInt(tag) : tag));
+
+    let bitsPerSample = bitDepthTags.find((tag) => typeof tag === 'number' && !Number.isNaN(tag)) ?? null;
+    if (bitsPerSample && bitsPerSample >= 24 && bitsPerSample % 3 === 0) {
+      bitsPerSample /= 3; // converts per-pixel bit depth to per-channel
+    }
+
+    return bitsPerSample;
+  }
 }

+ 3 - 0
server/test/fixtures/shared-link.stub.ts

@@ -225,6 +225,9 @@ export const sharedLinkStub = {
             fps: 100,
             asset: null as any,
             exifTextSearchableColumn: '',
+            profileDescription: 'sRGB',
+            bitsPerSample: 8,
+            colorspace: 'sRGB',
           },
           tags: [],
           sharedLinks: [],