1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2025-01-19 18:26:46 +01:00

refactor: tweaks for code review

This commit is contained in:
Eli Gao 2024-12-04 05:23:55 +08:00
parent bcb45f0e5b
commit c0b89a2eb1
16 changed files with 73 additions and 71 deletions

View file

@ -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()) {

Binary file not shown.

View file

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

View file

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

View file

@ -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;

View file

@ -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',

View file

@ -185,7 +185,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[]>;

View file

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

View file

@ -298,10 +298,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,

View file

@ -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 });

View file

@ -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;
}

View file

@ -627,9 +627,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,
@ -644,18 +644,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,
});
});
@ -666,10 +665,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();
});
@ -682,10 +680,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();
});
@ -699,15 +696,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,

View file

@ -27,7 +27,13 @@ import {
JobStatus,
QueueName,
} from 'src/interfaces/job.interface';
import { AudioStreamInfo, VideoFormat, VideoInterfaces, VideoStreamInfo } from 'src/interfaces/media.interface';
import {
AudioStreamInfo,
DecodeToBufferOptions,
VideoFormat,
VideoInterfaces,
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';
@ -135,7 +141,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);
@ -159,7 +165,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')) {
@ -171,7 +177,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 });
@ -181,8 +187,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) {
@ -200,18 +206,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) {
@ -231,48 +232,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) {

View file

@ -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),
});

View file

@ -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(),

View file

@ -149,7 +149,7 @@
});
let isWebCompatible = $derived(isWebCompatibleImage(asset));
// RAW files may have corresponding extracted JPEGs
// RAW files may have corresponding converted web-compatible original-sized files
let isRaw = $derived(isRawImage(asset));
let useOriginalByDefault = $derived(isWebCompatible && $alwaysLoadOriginalFile);