refactor: tweaks for code review

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

@@ -1222,7 +1222,7 @@ describe('/asset', () => {
       },
     ];
 
-    it(`should upload and generate a thumbnail for different file types`, async () => {
+    it(`should upload and generate a thumbnail for different file types`, { timeout: 60_000 }, async () => {
       // upload in parallel
       const assets = await Promise.all(
         tests.map(async ({ input }) => {
@@ -1235,7 +1235,8 @@ describe('/asset', () => {
 
       for (const { id, status } of assets) {
         expect(status).toBe(AssetMediaStatus.Created);
-        await utils.waitForWebsocketEvent({ event: 'assetUpload', id });
+        // longer timeout as the thumbnail generation from full-size raw files can take a while
+        await utils.waitForWebsocketEvent({ event: 'assetUpload', id, timeout: 60_000 });
       }
 
       for (const [i, { id }] of assets.entries()) {

open-api/immich-openapi-specs.json

@@ -10021,7 +10021,7 @@
       "PathType": {
         "enum": [
           "original",
-          "extracted",
+          "converted",
           "preview",
           "thumbnail",
           "encoded_video",

open-api/typescript-sdk/src/fetch-client.ts

@@ -3480,7 +3480,7 @@ export enum PathEntityType {
 }
 export enum PathType {
     Original = "original",
-    Extracted = "extracted",
+    Converted = "converted",
     Preview = "preview",
     Thumbnail = "thumbnail",
     EncodedVideo = "encoded_video",

server/src/cores/storage.core.ts

@@ -26,7 +26,7 @@ export interface MoveRequest {
   };
 }
 
-export type GeneratedImageType = AssetPathType.PREVIEW | AssetPathType.THUMBNAIL | AssetPathType.EXTRACTED;
+export type GeneratedImageType = AssetPathType.PREVIEW | AssetPathType.THUMBNAIL | AssetPathType.CONVERTED;
 export type GeneratedAssetType = GeneratedImageType | AssetPathType.ENCODED_VIDEO;
 
 let instance: StorageCore | null;

server/src/enum.ts

@@ -36,7 +36,7 @@ export enum AssetFileType {
   /**
    * An full/large-size image extracted/converted from RAW photos
    */
-  EXTRACTED = 'extracted',
+  CONVERTED = 'converted',
   PREVIEW = 'preview',
   THUMBNAIL = 'thumbnail',
 }
@@ -241,7 +241,7 @@ export enum ManualJobName {
 
 export enum AssetPathType {
   ORIGINAL = 'original',
-  EXTRACTED = 'extracted',
+  CONVERTED = 'converted',
   PREVIEW = 'preview',
   THUMBNAIL = 'thumbnail',
   ENCODED_VIDEO = 'encoded_video',

server/src/interfaces/asset.interface.ts

@@ -180,7 +180,6 @@ export interface IAssetRepository {
   updateDuplicates(options: AssetUpdateDuplicateOptions): Promise<void>;
   update(asset: AssetUpdateOptions): Promise<void>;
   remove(asset: AssetEntity): Promise<void>;
-  removeAssetFile(path: string): Promise<void>;
   findLivePhotoMatch(options: LivePhotoSearchOptions): Promise<AssetEntity | null>;
   getStatistics(ownerId: string, options: AssetStatsOptions): Promise<AssetStats>;
   getTimeBuckets(options: TimeBucketOptions): Promise<TimeBucketItem[]>;

server/src/interfaces/media.interface.ts

@@ -30,7 +30,7 @@ interface DecodeImageOptions {
 }
 
 export interface DecodeToBufferOptions extends DecodeImageOptions {
-  size: number;
+  size?: number;
   orientation?: ExifOrientation;
 }
 

server/src/repositories/asset.repository.ts

@@ -281,10 +281,6 @@ export class AssetRepository implements IAssetRepository {
     await this.repository.remove(asset);
   }
 
-  async removeAssetFile(path: string): Promise<void> {
-    await this.fileRepository.delete({ path });
-  }
-
   @GenerateSql({ params: [{ ownerId: DummyValue.UUID, libraryId: DummyValue.UUID, checksum: DummyValue.BUFFER }] })
   getByChecksum({
     ownerId,

server/src/repositories/media.repository.ts

@@ -103,8 +103,8 @@ export class MediaRepository implements IMediaRepository {
       pipeline = pipeline.extract(options.crop);
     }
 
-    // Infinity is a special value that means no resizing
-    if (options.size === Infinity) {
+    // No size, no resizing
+    if (options.size !== undefined) {
       return pipeline;
     }
     return pipeline.resize(options.size, options.size, { fit: 'outside', withoutEnlargement: true });

server/src/services/asset-media.service.ts

@@ -208,14 +208,14 @@ export class AssetMediaService extends BaseService {
     const asset = await this.findOrFail(id);
     const size = dto.size ?? AssetMediaSize.THUMBNAIL;
 
-    const { thumbnailFile, previewFile, extractedFile } = getAssetFiles(asset.files);
+    const { thumbnailFile, previewFile, convertedFile } = getAssetFiles(asset.files);
     let filepath = previewFile?.path;
     if (size === AssetMediaSize.THUMBNAIL && thumbnailFile) {
       filepath = thumbnailFile.path;
     } else if (size === AssetMediaSize.ORIGINAL) {
       // eslint-disable-next-line unicorn/prefer-ternary
       if (mimeTypes.isRaw(asset.originalPath)) {
-        filepath = extractedFile?.path ?? previewFile?.path;
+        filepath = convertedFile?.path ?? previewFile?.path;
       } else {
         filepath = asset.originalPath;
       }

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

@@ -628,9 +628,9 @@ describe(MediaService.name, () => {
 
       await sut.handleGenerateThumbnails({ id: assetStub.image.id });
 
-      const extractedPath = mediaMock.extract.mock.calls.at(-1)?.[1].toString();
+      const convertedPath = mediaMock.extract.mock.calls.at(-1)?.[1].toString();
       expect(mediaMock.decodeImage).toHaveBeenCalledOnce();
-      expect(mediaMock.decodeImage).toHaveBeenCalledWith(extractedPath, {
+      expect(mediaMock.decodeImage).toHaveBeenCalledWith(convertedPath, {
         colorspace: Colorspace.P3,
         processInvalidImages: false,
         size: 1440,
@@ -645,18 +645,17 @@ describe(MediaService.name, () => {
 
       await sut.handleGenerateThumbnails({ id: assetStub.image.id });
 
-      const extractedPath = mediaMock.extract.mock.calls.at(-1)?.[1].toString();
+      const convertedPath = mediaMock.extract.mock.calls.at(-1)?.[1].toString();
       expect(mediaMock.generateThumbnail).toHaveBeenCalledWith(
-        assetStub.imageDng.originalPath,
-        expect.objectContaining({ size: Infinity }),
-        extractedPath,
+        rawBuffer,
+        expect.objectContaining({ size: 1440 }),
+        convertedPath,
       );
-      expect(extractedPath).toMatch(/-extracted\.jpeg$/);
+      expect(convertedPath).toMatch(/-converted\.jpeg$/);
       expect(mediaMock.decodeImage).toHaveBeenCalledOnce();
-      expect(mediaMock.decodeImage).toHaveBeenCalledWith(extractedPath, {
+      expect(mediaMock.decodeImage).toHaveBeenCalledWith(assetStub.imageDng.originalPath, {
         colorspace: Colorspace.P3,
         processInvalidImages: false,
-        size: 1440,
       });
     });
 
@@ -667,10 +666,9 @@ describe(MediaService.name, () => {
       await sut.handleGenerateThumbnails({ id: assetStub.image.id });
 
       expect(mediaMock.decodeImage).toHaveBeenCalledOnce();
-      expect(mediaMock.decodeImage).toHaveBeenCalledWith('upload/thumbs/user-id/as/se/asset-id-extracted.jpeg', {
+      expect(mediaMock.decodeImage).toHaveBeenCalledWith(assetStub.imageDng.originalPath, {
         colorspace: Colorspace.P3,
         processInvalidImages: false,
-        size: 1440,
       });
       expect(mediaMock.getImageDimensions).not.toHaveBeenCalled();
     });
@@ -683,10 +681,9 @@ describe(MediaService.name, () => {
 
       expect(mediaMock.extract).not.toHaveBeenCalled();
       expect(mediaMock.decodeImage).toHaveBeenCalledOnce();
-      expect(mediaMock.decodeImage).toHaveBeenCalledWith('upload/thumbs/user-id/as/se/asset-id-extracted.jpeg', {
+      expect(mediaMock.decodeImage).toHaveBeenCalledWith(assetStub.imageDng.originalPath, {
         colorspace: Colorspace.P3,
         processInvalidImages: false,
-        size: 1440,
       });
       expect(mediaMock.getImageDimensions).not.toHaveBeenCalled();
     });
@@ -700,15 +697,15 @@ describe(MediaService.name, () => {
 
       expect(mediaMock.decodeImage).toHaveBeenCalledOnce();
       expect(mediaMock.decodeImage).toHaveBeenCalledWith(
-        'upload/thumbs/user-id/as/se/asset-id-extracted.jpeg',
+        assetStub.imageDng.originalPath,
         expect.objectContaining({ processInvalidImages: true }),
       );
 
       expect(mediaMock.generateThumbnail).toHaveBeenCalledTimes(3);
       expect(mediaMock.generateThumbnail).toHaveBeenCalledWith(
-        assetStub.imageDng.originalPath,
+        rawBuffer,
         expect.objectContaining({ processInvalidImages: true }),
-        'upload/thumbs/user-id/as/se/asset-id-extracted.jpeg',
+        'upload/thumbs/user-id/as/se/asset-id-converted.jpeg',
       );
       expect(mediaMock.generateThumbnail).toHaveBeenCalledWith(
         rawBuffer,

server/src/services/media.service.ts

@@ -27,7 +27,13 @@ import {
   JobStatus,
   QueueName,
 } from 'src/interfaces/job.interface';
-import { AudioStreamInfo, TranscodeCommand, VideoFormat, VideoStreamInfo } from 'src/interfaces/media.interface';
+import {
+  AudioStreamInfo,
+  DecodeToBufferOptions,
+  TranscodeCommand,
+  VideoFormat,
+  VideoStreamInfo,
+} from 'src/interfaces/media.interface';
 import { BaseService } from 'src/services/base.service';
 import { getAssetFiles } from 'src/utils/asset.util';
 import { BaseConfig, ThumbnailConfig } from 'src/utils/media';
@@ -130,7 +136,7 @@ export class MediaService extends BaseService {
       return JobStatus.FAILED;
     }
 
-    await this.storageCore.moveAssetImage(asset, AssetPathType.EXTRACTED, ImageFormat.JPEG);
+    await this.storageCore.moveAssetImage(asset, AssetPathType.CONVERTED, ImageFormat.JPEG);
     await this.storageCore.moveAssetImage(asset, AssetPathType.PREVIEW, image.preview.format);
     await this.storageCore.moveAssetImage(asset, AssetPathType.THUMBNAIL, image.thumbnail.format);
     await this.storageCore.moveAssetVideo(asset);
@@ -154,7 +160,7 @@ export class MediaService extends BaseService {
     let generated: {
       previewPath: string;
       thumbnailPath: string;
-      extractedPath?: string;
+      convertedPath?: string;
       thumbhash: Buffer;
     };
     if (asset.type === AssetType.VIDEO || asset.originalFileName.toLowerCase().endsWith('.gif')) {
@@ -166,7 +172,7 @@ export class MediaService extends BaseService {
       return JobStatus.SKIPPED;
     }
 
-    const { previewFile, thumbnailFile, extractedFile } = getAssetFiles(asset.files);
+    const { previewFile, thumbnailFile, convertedFile } = getAssetFiles(asset.files);
     const toUpsert: UpsertFileOptions[] = [];
     if (previewFile?.path !== generated.previewPath) {
       toUpsert.push({ assetId: asset.id, path: generated.previewPath, type: AssetFileType.PREVIEW });
@@ -176,8 +182,8 @@ export class MediaService extends BaseService {
       toUpsert.push({ assetId: asset.id, path: generated.thumbnailPath, type: AssetFileType.THUMBNAIL });
     }
 
-    if (generated.extractedPath && extractedFile?.path !== generated.extractedPath) {
-      toUpsert.push({ assetId: asset.id, path: generated.extractedPath, type: AssetFileType.EXTRACTED });
+    if (generated.convertedPath && convertedFile?.path !== generated.convertedPath) {
+      toUpsert.push({ assetId: asset.id, path: generated.convertedPath, type: AssetFileType.CONVERTED });
     }
 
     if (toUpsert.length > 0) {
@@ -195,18 +201,13 @@ export class MediaService extends BaseService {
       pathsToDelete.push(thumbnailFile.path);
     }
 
-    if (extractedFile && extractedFile.path !== generated.extractedPath) {
+    if (convertedFile && convertedFile.path !== generated.convertedPath) {
       this.logger.debug(`Deleting old extracted image for asset ${asset.id}`);
-      pathsToDelete.push(extractedFile.path);
+      pathsToDelete.push(convertedFile.path);
     }
 
     if (pathsToDelete.length > 0) {
-      await Promise.all(
-        pathsToDelete.map(async (path) => {
-          await this.storageRepository.unlink(path);
-          await this.assetRepository.removeAssetFile(path);
-        }),
-      );
+      await Promise.all(pathsToDelete.map((path) => this.storageRepository.unlink(path)));
     }
 
     if (asset.thumbhash != generated.thumbhash) {
@@ -226,48 +227,57 @@ export class MediaService extends BaseService {
 
     const processInvalidImages = process.env.IMMICH_PROCESS_INVALID_IMAGES === 'true';
     const colorspace = this.isSRGB(asset) ? Colorspace.SRGB : image.colorspace;
-    const decodeOptions = { colorspace, processInvalidImages, size: image.preview.size };
+    const imageIsRaw = mimeTypes.isRaw(asset.originalPath);
+    const decodeOptions: DecodeToBufferOptions = {
+      colorspace,
+      processInvalidImages,
+      size: image.preview.size,
+    };
 
-    let fullsizePath: string;
-    let extractedPath: string | undefined;
-    if (mimeTypes.isRaw(asset.originalPath)) {
+    let fullsizePath: string = asset.originalPath;
+    /**
+     * Converted or extracted image from RAW
+     */
+    let convertedPath: string | undefined;
+    let shouldConvertFromRaw = false;
+    if (imageIsRaw) {
       let useExtracted = false;
-      extractedPath = StorageCore.getImagePath(asset, AssetPathType.EXTRACTED, ImageFormat.JPEG);
+      // extracted image from RAW is always in JPEG format, as implied from the `jpgFromRaw` tag name
+      convertedPath = StorageCore.getImagePath(asset, AssetPathType.CONVERTED, ImageFormat.JPEG);
       if (image.extractEmbedded) {
         // try extracting embedded preview from RAW
-        // extracted image from RAW is always in JPEG format, as implied from the `jpgFromRaw` tag name
-        const didExtract = await this.mediaRepository.extract(asset.originalPath, extractedPath);
-        useExtracted = didExtract && (await this.shouldUseExtractedImage(extractedPath, image.preview.size));
+        const didExtract = await this.mediaRepository.extract(asset.originalPath, convertedPath);
+        useExtracted = didExtract && (await this.shouldUseExtractedImage(convertedPath, image.preview.size));
       }
 
       if (useExtracted) {
-        fullsizePath = extractedPath;
+        fullsizePath = convertedPath;
+        // proper orientation metadata is missing in extracted images, specify separately
+        // TODO: remove this when proper EXIF is included in extracted images
+        decodeOptions.orientation = asset.exifInfo?.orientation ? Number(asset.exifInfo.orientation) : undefined;
       } else {
         // did not extract or extracted preview is smaller than target size,
         // convert a full-sized thumbnail from original instead
-        extractedPath = StorageCore.getImagePath(asset, AssetPathType.EXTRACTED, image.preview.format);
-        // const orientation = asset.exifInfo?.orientation ? Number(asset.exifInfo.orientation) : undefined;
-        await this.mediaRepository.generateThumbnail(
-          asset.originalPath,
-          { ...image.preview, colorspace, processInvalidImages, size: Infinity },
-          extractedPath,
-        );
+        convertedPath = StorageCore.getImagePath(asset, AssetPathType.CONVERTED, image.preview.format);
+        // unset size to disable resizing
+        decodeOptions.size = undefined;
+        shouldConvertFromRaw = true;
       }
-      fullsizePath = extractedPath;
-    } else {
-      fullsizePath = asset.originalPath;
     }
 
     const { info, data } = await this.mediaRepository.decodeImage(fullsizePath, decodeOptions);
 
     const thumbnailOptions = { colorspace, processInvalidImages, raw: info };
     const outputs = await Promise.all([
+      this.mediaRepository.generateThumbhash(data, thumbnailOptions),
       this.mediaRepository.generateThumbnail(data, { ...image.thumbnail, ...thumbnailOptions }, thumbnailPath),
       this.mediaRepository.generateThumbnail(data, { ...image.preview, ...thumbnailOptions }, previewPath),
-      this.mediaRepository.generateThumbhash(data, thumbnailOptions),
+      shouldConvertFromRaw && convertedPath
+        ? this.mediaRepository.generateThumbnail(data, { ...image.preview, ...thumbnailOptions }, convertedPath)
+        : undefined,
     ]);
 
-    return { previewPath, thumbnailPath, extractedPath, thumbhash: outputs[2] };
+    return { previewPath, thumbnailPath, convertedPath, thumbhash: outputs[0] };
   }
 
   private async generateVideoThumbnails(asset: AssetEntity) {

server/src/utils/asset.util.ts

@@ -25,7 +25,7 @@ const getFileByType = (files: AssetFileEntity[] | undefined, type: AssetFileType
 };
 
 export const getAssetFiles = (files?: AssetFileEntity[]) => ({
-  extractedFile: getFileByType(files, AssetFileType.EXTRACTED),
+  convertedFile: getFileByType(files, AssetFileType.CONVERTED),
   previewFile: getFileByType(files, AssetFileType.PREVIEW),
   thumbnailFile: getFileByType(files, AssetFileType.THUMBNAIL),
 });

server/test/repositories/asset.repository.mock.ts

@@ -28,7 +28,6 @@ export const newAssetRepositoryMock = (): Mocked<IAssetRepository> => {
     deleteAll: vitest.fn(),
     update: vitest.fn(),
     remove: vitest.fn(),
-    removeAssetFile: vitest.fn(),
     findLivePhotoMatch: vitest.fn(),
     getStatistics: vitest.fn(),
     getTimeBucket: vitest.fn(),