feat: 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`, { timeout: 60_000 }, async () => {
+    it(`should upload and generate a thumbnail for different file types`, async () => {
       // upload in parallel
       const assets = await Promise.all(
         tests.map(async ({ input }) => {

server/src/interfaces/media.interface.ts

@@ -1,4 +1,5 @@
 import { Writable } from 'node:stream';
+import { ExifEntity } from 'src/entities/exif.entity';
 import { ExifOrientation, ImageFormat, TranscodeTarget, VideoCodec } from 'src/enum';
 
 export const IMediaRepository = 'IMediaRepository';
@@ -32,7 +33,6 @@ interface DecodeImageOptions {
 export interface DecodeToBufferOptions extends DecodeImageOptions {
   size?: number;
   orientation?: ExifOrientation;
-  keepExif?: boolean;
 }
 
 export type GenerateThumbnailOptions = Pick<ImageOptions, 'format' | 'quality'> & DecodeToBufferOptions;
@@ -139,7 +139,7 @@ export interface VideoInterfaces {
 export interface IMediaRepository {
   // image
   extract(input: string, output: string, withExif?: boolean): Promise<boolean>;
-  cloneExif(input: string, output: string): Promise<boolean>;
+  writeExif(tags: ExifEntity, output: string): Promise<boolean>;
   decodeImage(input: string, options: DecodeToBufferOptions): Promise<ImageBuffer>;
   generateThumbnail(input: string, options: GenerateThumbnailOptions, outputFile: string): Promise<void>;
   generateThumbnail(input: Buffer, options: GenerateThumbnailFromBufferOptions, outputFile: string): Promise<void>;

server/src/repositories/media.repository.ts

@@ -1,19 +1,20 @@
 import { Inject, Injectable } from '@nestjs/common';
-import { BinaryField, exiftool } from 'exiftool-vendored';
+import { ExifDateTime, exiftool, WriteTags } from 'exiftool-vendored';
 import ffmpeg, { FfprobeData } from 'fluent-ffmpeg';
 import { Duration } from 'luxon';
 import fs from 'node:fs/promises';
 import { Writable } from 'node:stream';
 import sharp from 'sharp';
 import { ORIENTATION_TO_SHARP_ROTATION } from 'src/constants';
+import { ExifEntity } from 'src/entities/exif.entity';
 import { Colorspace, LogLevel } from 'src/enum';
 import { ILoggerRepository } from 'src/interfaces/logger.interface';
 import {
   DecodeToBufferOptions,
   GenerateThumbhashOptions,
   GenerateThumbnailOptions,
-  IMediaRepository,
   ImageDimensions,
+  IMediaRepository,
   ProbeOptions,
   TranscodeCommand,
   VideoInfo,
@@ -62,40 +63,40 @@ export class MediaRepository implements IMediaRepository {
     return true;
   }
 
-  async cloneExif(input: string, output: string): Promise<boolean> {
+  async writeExif(tags: ExifEntity, output: string): Promise<boolean> {
     try {
-      // exclude some non-tag fields that interfere with writing back to the image
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      const { errors, warnings, OriginalFileName, FileName, Directory, ...exifTags } = await exiftool.read(input, {
-        ignoreMinorErrors: true,
-      });
-      this.logger.debug('Read exif data from original image:', exifTags);
-      if (errors?.length) {
-        this.logger.debug('Error reading exif data', JSON.stringify(errors));
-      }
-      if (warnings?.length) {
-        this.logger.debug('Warning reading exif data', JSON.stringify(warnings));
-      }
-      // filter out binary fields as they emit errors like:
-      // Could not extract exif data from image: cannot encode {"_ctor":"BinaryField","bytes":4815633,"rawValue":"(Binary data 4815633 bytes, use -b option to extract)"}
-      const exifTagsToWrite = Object.fromEntries(
-        Object.entries(exifTags).filter(([, value]) => !(value instanceof BinaryField)),
-      );
-      // GOTCHA: "Orientation" is read as a number by default,
-      // but when writing back, it has to be keyed "Orientation#"
-      // @see https://github.com/photostructure/exiftool-vendored.js/blob/f77b0f097fb26b68326d325caaf1642cf29cfe3d/src/WriteTags.ts#L22
-      if (exifTags.Orientation != null) {
-        exifTagsToWrite['Orientation#'] = exifTags.Orientation;
-        delete exifTagsToWrite['Orientation'];
-      }
-      const result = await exiftool.write(output, exifTagsToWrite, {
+      const tagsToWrite: WriteTags = {
+        ExifImageWidth: tags.exifImageWidth,
+        ExifImageHeight: tags.exifImageHeight,
+        DateTimeOriginal: tags.dateTimeOriginal && ExifDateTime.fromMillis(tags.dateTimeOriginal.getTime()),
+        ModifyDate: tags.modifyDate && ExifDateTime.fromMillis(tags.modifyDate.getTime()),
+        TimeZone: tags.timeZone,
+        GPSLatitude: tags.latitude,
+        GPSLongitude: tags.longitude,
+        ProjectionType: tags.projectionType,
+        City: tags.city,
+        Country: tags.country,
+        Make: tags.make,
+        Model: tags.model,
+        LensModel: tags.lensModel,
+        Fnumber: tags.fNumber?.toFixed(1),
+        FocalLength: tags.focalLength?.toFixed(1),
+        ISO: tags.iso,
+        ExposureTime: tags.exposureTime,
+        ProfileDescription: tags.profileDescription,
+        ColorSpace: tags.colorspace,
+        Rating: tags.rating,
+        // specially convert Orientation to numeric Orientation# for exiftool
+        'Orientation#': tags.orientation ? Number(tags.orientation) : undefined,
+      };
+
+      await exiftool.write(output, tagsToWrite, {
         ignoreMinorErrors: true,
         writeArgs: ['-overwrite_original'],
       });
-      this.logger.debug('Wrote exif data to extracted image:', result);
       return true;
     } catch (error: any) {
-      this.logger.warn(`Could not extract exif data from image: ${error.message}`);
+      this.logger.warn(`Could not write exif data to image: ${error.message}`);
       return false;
     }
   }
@@ -105,17 +106,13 @@ export class MediaRepository implements IMediaRepository {
   }
 
   async generateThumbnail(input: string | Buffer, options: GenerateThumbnailOptions, output: string): Promise<void> {
-    let pipeline = this.getImageDecodingPipeline(input, options).toFormat(options.format, {
-      quality: options.quality,
-      // this is default in libvips (except the threshold is 90), but we need to set it manually in sharp
-      chromaSubsampling: options.quality >= 80 ? '4:4:4' : '4:2:0',
-    });
-
-    if (options.keepExif === true) {
-      pipeline = pipeline.keepExif();
-    }
-
-    await pipeline.toFile(output);
+    await this.getImageDecodingPipeline(input, options)
+      .toFormat(options.format, {
+        quality: options.quality,
+        // this is default in libvips (except the threshold is 90), but we need to set it manually in sharp
+        chromaSubsampling: options.quality >= 80 ? '4:4:4' : '4:2:0',
+      })
+      .toFile(output);
   }
 
   private getImageDecodingPipeline(input: string | Buffer, options: DecodeToBufferOptions) {

server/src/services/media.service.ts

@@ -246,7 +246,7 @@ export class MediaService extends BaseService {
     let useExtracted = false;
     let decodeInputPath: string = asset.originalPath;
     // Converted or extracted image from non-web-supported formats (e.g. RAW)
-    let fullsizePath = StorageCore.getImagePath(asset, AssetPathType.FULLSIZE, image.preview.format);
+    let fullsizePath: string | undefined;
     if (shouldConvertFullsize) {
       // unset size to decode fullsize image
       decodeOptions.size = undefined;
@@ -258,7 +258,7 @@ export class MediaService extends BaseService {
       // Assume extracted image from RAW always in JPEG format, as implied from the `jpgFromRaw` tag name
       const extractedPath = StorageCore.getImagePath(asset, AssetPathType.FULLSIZE, ImageFormat.JPEG);
       const didExtract = await this.mediaRepository.extract(asset.originalPath, extractedPath, true);
-      useExtracted = didExtract && (await this.shouldUseExtractedImage(fullsizePath, image.preview.size));
+      useExtracted = didExtract && (await this.shouldUseExtractedImage(extractedPath, image.preview.size));
 
       if (useExtracted) {
         if (shouldConvertFullsize) {
@@ -268,9 +268,12 @@ export class MediaService extends BaseService {
         }
         // use this as origin of preview and thumbnail
         decodeInputPath = extractedPath;
-        // clone EXIF to persist orientation and other metadata
-        // this is delayed to reduce I/O overhead as we cannot do it in one go with extraction due to exiftool limitations
-        await this.mediaRepository.cloneExif(asset.originalPath, extractedPath);
+        if (asset.exifInfo) {
+          // write EXIF, especially orientation and colorspace essential for subsequent processing
+          await this.mediaRepository.writeExif(asset.exifInfo, extractedPath);
+        }
+      } else {
+        fullsizePath = StorageCore.getImagePath(asset, AssetPathType.FULLSIZE, image.preview.format);
       }
     }
 
@@ -281,11 +284,11 @@ export class MediaService extends BaseService {
       this.mediaRepository.generateThumbhash(data, thumbnailOptions),
       this.mediaRepository.generateThumbnail(data, { ...image.thumbnail, ...thumbnailOptions }, thumbnailPath),
       this.mediaRepository.generateThumbnail(data, { ...image.preview, ...thumbnailOptions }, previewPath),
-      shouldConvertFullsize &&
+      fullsizePath &&
         !useExtracted && // did not extract a usable image from RAW
         this.mediaRepository.generateThumbnail(
           data,
-          { ...image.preview, ...thumbnailOptions, size: undefined, keepExif: true },
+          { ...image.preview, ...thumbnailOptions, size: undefined },
           fullsizePath,
         ),
     ]);