fix(server): better metadata extraction for images (#2653)

server/apps/microservices/src/processors/metadata-extraction.processor.ts

@@ -15,14 +15,17 @@ import { Inject, Logger } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { InjectRepository } from '@nestjs/typeorm';
 import tz_lookup from '@photostructure/tz-lookup';
-import { ExifDateTime, exiftool, Tags } from 'exiftool-vendored';
+import { exiftool, Tags } from 'exiftool-vendored';
 import ffmpeg, { FfprobeData } from 'fluent-ffmpeg';
 import { Duration } from 'luxon';
 import fs from 'node:fs';
 import sharp from 'sharp';
 import { Repository } from 'typeorm/repository/Repository';
 import { promisify } from 'util';
-import { parseLatitude, parseLongitude } from '../utils/coordinates';
+import { parseLatitude, parseLongitude } from '../utils/exif/coordinates';
+import { exifTimeZone, exifToDate } from '../utils/exif/date-time';
+import { parseISO } from '../utils/exif/iso';
+import { toNumberOrNull } from '../utils/numbers';
 
 const ffprobe = promisify<string, FfprobeData>(ffmpeg.ffprobe);
 
@@ -116,32 +119,13 @@ export class MetadataExtractionProcessor {
         })
       : {};
 
-    const exifToDate = (exifDate: string | Date | ExifDateTime | undefined) => {
-      if (!exifDate) {
-        return null;
-      }
-
-      const date = exifDate instanceof ExifDateTime ? exifDate.toDate() : new Date(exifDate);
-      if (isNaN(date.valueOf())) {
-        return null;
-      }
-
-      return date;
-    };
-
-    const exifTimeZone = (exifDate: string | Date | ExifDateTime | undefined) => {
-      const isExifDate = exifDate instanceof ExifDateTime;
-      if (!isExifDate) {
-        return null;
-      }
-
-      return exifDate.zone ?? null;
-    };
-
-    const getExifProperty = <T extends keyof ImmichTags>(...properties: T[]): any | null => {
+    const getExifProperty = <T extends keyof ImmichTags>(
+      ...properties: T[]
+    ): NonNullable<ImmichTags[T]> | string | null => {
       for (const property of properties) {
         const value = sidecarExifData?.[property] ?? mediaExifData?.[property];
         if (value !== null && value !== undefined) {
+          // Can also be string when the value cannot be parsed
           return value;
         }
       }
@@ -160,25 +144,27 @@ export class MetadataExtractionProcessor {
     newExif.fileSizeInByte = fileSizeInBytes;
     newExif.make = getExifProperty('Make');
     newExif.model = getExifProperty('Model');
-    newExif.exifImageHeight = getExifProperty('ExifImageHeight', 'ImageHeight');
-    newExif.exifImageWidth = getExifProperty('ExifImageWidth', 'ImageWidth');
+    newExif.exifImageHeight = toNumberOrNull(getExifProperty('ExifImageHeight', 'ImageHeight'));
+    newExif.exifImageWidth = toNumberOrNull(getExifProperty('ExifImageWidth', 'ImageWidth'));
     newExif.exposureTime = getExifProperty('ExposureTime');
-    newExif.orientation = getExifProperty('Orientation')?.toString();
+    newExif.orientation = getExifProperty('Orientation')?.toString() ?? null;
     newExif.dateTimeOriginal = fileCreatedAt;
     newExif.modifyDate = fileModifiedAt;
     newExif.timeZone = timeZone;
     newExif.lensModel = getExifProperty('LensModel');
-    newExif.fNumber = getExifProperty('FNumber');
-    const focalLength = getExifProperty('FocalLength');
-    newExif.focalLength = focalLength ? parseFloat(focalLength) : null;
-    // This is unusual - exifData.ISO should return a number, but experienced that sidecar XMP
-    // files MAY return an array of numbers instead.
-    const iso = getExifProperty('ISO');
-    newExif.iso = Array.isArray(iso) ? iso[0] : iso || null;
-    newExif.latitude = parseLatitude(getExifProperty('GPSLatitude'));
-    newExif.longitude = parseLongitude(getExifProperty('GPSLongitude'));
-    newExif.livePhotoCID = getExifProperty('MediaGroupUUID');
+    newExif.fNumber = toNumberOrNull(getExifProperty('FNumber'));
+    newExif.focalLength = toNumberOrNull(getExifProperty('FocalLength'));
 
+    // Handle array values by converting to string
+    const iso = getExifProperty('ISO')?.toString();
+    newExif.iso = iso ? parseISO(iso) : null;
+
+    const latitude = getExifProperty('GPSLatitude');
+    const longitude = getExifProperty('GPSLongitude');
+    newExif.latitude = latitude !== null ? parseLatitude(latitude) : null;
+    newExif.longitude = longitude !== null ? parseLongitude(longitude) : null;
+
+    newExif.livePhotoCID = getExifProperty('MediaGroupUUID');
     if (newExif.livePhotoCID && !asset.livePhotoVideoId) {
       const motionAsset = await this.assetRepository.findLivePhotoMatch({
         livePhotoCID: newExif.livePhotoCID,

server/apps/microservices/src/utils/coordinates.ts

@@ -1,17 +0,0 @@
-export function parseLatitude(input: string): number | null {
-  const latitude = Number.parseFloat(input);
-
-  if (latitude < -90 || latitude > 90 || Number.isNaN(latitude)) {
-    return null;
-  }
-  return latitude;
-}
-
-export function parseLongitude(input: string): number | null {
-  const longitude = Number.parseFloat(input);
-
-  if (longitude < -180 || longitude > 180 || Number.isNaN(longitude)) {
-    return null;
-  }
-  return longitude;
-}

server/apps/microservices/src/utils/coordinates.spec.ts → server/apps/microservices/src/utils/exif/coordinates.spec.ts

@@ -8,17 +8,17 @@ describe('parsing latitude from string input', () => {
     expect(parseLatitude('Infinity')).toBeNull();
     expect(parseLatitude('-Infinity')).toBeNull();
     expect(parseLatitude('90.001')).toBeNull();
-    expect(parseLatitude('-90.000001')).toBeNull();
+    expect(parseLatitude(-90.000001)).toBeNull();
     expect(parseLatitude('1000')).toBeNull();
-    expect(parseLatitude('-1000')).toBeNull();
+    expect(parseLatitude(-1000)).toBeNull();
   });
 
   it('returns the numeric coordinate for valid inputs', () => {
     expect(parseLatitude('90')).toBeCloseTo(90);
     expect(parseLatitude('-90')).toBeCloseTo(-90);
-    expect(parseLatitude('89.999999')).toBeCloseTo(89.999999);
+    expect(parseLatitude(89.999999)).toBeCloseTo(89.999999);
     expect(parseLatitude('-89.9')).toBeCloseTo(-89.9);
-    expect(parseLatitude('0')).toBeCloseTo(0);
+    expect(parseLatitude(0)).toBeCloseTo(0);
     expect(parseLatitude('-0.0')).toBeCloseTo(-0.0);
   });
 });
@@ -27,19 +27,19 @@ describe('parsing longitude from string input', () => {
   it('returns null for invalid inputs', () => {
     expect(parseLongitude('')).toBeNull();
     expect(parseLongitude('NaN')).toBeNull();
-    expect(parseLongitude('Infinity')).toBeNull();
+    expect(parseLongitude(Infinity)).toBeNull();
     expect(parseLongitude('-Infinity')).toBeNull();
     expect(parseLongitude('180.001')).toBeNull();
     expect(parseLongitude('-180.000001')).toBeNull();
-    expect(parseLongitude('1000')).toBeNull();
+    expect(parseLongitude(1000)).toBeNull();
     expect(parseLongitude('-1000')).toBeNull();
   });
 
   it('returns the numeric coordinate for valid inputs', () => {
-    expect(parseLongitude('180')).toBeCloseTo(180);
+    expect(parseLongitude(180)).toBeCloseTo(180);
     expect(parseLongitude('-180')).toBeCloseTo(-180);
     expect(parseLongitude('179.999999')).toBeCloseTo(179.999999);
-    expect(parseLongitude('-179.9')).toBeCloseTo(-179.9);
+    expect(parseLongitude(-179.9)).toBeCloseTo(-179.9);
     expect(parseLongitude('0')).toBeCloseTo(0);
     expect(parseLongitude('-0.0')).toBeCloseTo(-0.0);
   });

server/apps/microservices/src/utils/exif/coordinates.ts

@@ -0,0 +1,19 @@
+import { isNumberInRange } from '../numbers';
+
+export function parseLatitude(input: string | number): number | null {
+  const latitude = typeof input === 'string' ? Number.parseFloat(input) : input;
+
+  if (isNumberInRange(latitude, -90, 90)) {
+    return latitude;
+  }
+  return null;
+}
+
+export function parseLongitude(input: string | number): number | null {
+  const longitude = typeof input === 'string' ? Number.parseFloat(input) : input;
+
+  if (isNumberInRange(longitude, -180, 180)) {
+    return longitude;
+  }
+  return null;
+}

server/apps/microservices/src/utils/exif/date-time.spec.ts

@@ -0,0 +1,36 @@
+import { describe, expect, it } from '@jest/globals';
+import { ExifDateTime } from 'exiftool-vendored';
+import { exifTimeZone, exifToDate } from './date-time';
+
+describe('converts exif date to JS date', () => {
+  it('returns null for invalid inputs', () => {
+    expect(exifToDate(undefined)).toBeNull();
+    expect(exifToDate('invalid')).toBeNull();
+    expect(exifToDate(new Date('invalid'))).toBeNull();
+    expect(exifToDate(ExifDateTime.fromEXIF('invalid'))).toBeNull();
+  });
+
+  it('returns a valid date object for valid inputs', () => {
+    const date = new Date('2023');
+    expect(exifToDate(date)).toBeInstanceOf(Date);
+    expect(exifToDate(date)?.toISOString()).toBe('2023-01-01T00:00:00.000Z');
+    expect(exifToDate('2023')).toBeInstanceOf(Date);
+
+    const exifDateTime = ExifDateTime.fromISO('2023-01-01T00:00:00.000Z');
+    expect(exifToDate(exifDateTime)).toBeInstanceOf(Date);
+    expect(exifToDate(exifDateTime)?.toISOString()).toBe('2023-01-01T00:00:00.000Z');
+  });
+});
+
+describe('extracts the timezone from a date', () => {
+  it('returns null for invalid inputs', () => {
+    expect(exifTimeZone(undefined)).toBeNull();
+    expect(exifTimeZone('')).toBeNull();
+    expect(exifTimeZone(new Date('2023'))).toBeNull();
+    expect(exifTimeZone(ExifDateTime.fromEXIF('invalid'))).toBeNull();
+  });
+
+  it('returns the timezone for valid inputs', () => {
+    expect(exifTimeZone(ExifDateTime.fromEXIF('2020:12:29 14:24:45.700-05:00'))).toBe('UTC-5');
+  });
+});

server/apps/microservices/src/utils/exif/date-time.ts

@@ -0,0 +1,24 @@
+import { ExifDateTime } from 'exiftool-vendored';
+import { isDecimalNumber } from '../numbers';
+
+export function exifToDate(exifDate: string | Date | ExifDateTime | undefined): Date | null {
+  if (!exifDate) {
+    return null;
+  }
+
+  const date = exifDate instanceof ExifDateTime ? exifDate.toDate() : new Date(exifDate);
+  if (!isDecimalNumber(date.valueOf())) {
+    return null;
+  }
+
+  return date;
+}
+
+export function exifTimeZone(exifDate: string | Date | ExifDateTime | undefined): string | null {
+  const isExifDate = exifDate instanceof ExifDateTime;
+  if (!isExifDate) {
+    return null;
+  }
+
+  return exifDate.zone ?? null;
+}

server/apps/microservices/src/utils/exif/iso.spec.ts

@@ -0,0 +1,24 @@
+import { describe, it, expect } from '@jest/globals';
+import { parseISO } from './iso';
+
+describe('parsing ISO values', () => {
+  it('returns null for invalid values', () => {
+    expect(parseISO('')).toBeNull();
+    expect(parseISO(',,,')).toBeNull();
+    expect(parseISO('invalid')).toBeNull();
+    expect(parseISO('-5')).toBeNull();
+    expect(parseISO('99999999999999')).toBeNull();
+  });
+
+  it('returns the ISO number for valid inputs', () => {
+    expect(parseISO('0.0')).toBe(0);
+    expect(parseISO('32000.9')).toBe(32000);
+  });
+
+  it('returns the first valid ISO number in a comma separated list', () => {
+    expect(parseISO('400, 200, 100')).toBe(400);
+    expect(parseISO('-1600,800')).toBe(800);
+    expect(parseISO('-1,   a., 1200')).toBe(1200);
+    expect(parseISO('NaN,50,100')).toBe(50);
+  });
+});

server/apps/microservices/src/utils/exif/iso.ts

@@ -0,0 +1,14 @@
+import { isNumberInRange } from '../numbers';
+
+export function parseISO(input: string): number | null {
+  const values = input.split(',');
+
+  for (const value of values) {
+    const iso = Number.parseInt(value, 10);
+    if (isNumberInRange(iso, 0, 2 ** 32)) {
+      return iso;
+    }
+  }
+
+  return null;
+}

server/apps/microservices/src/utils/numbers.spec.ts

@@ -0,0 +1,47 @@
+import { describe, it, expect } from '@jest/globals';
+import { isDecimalNumber, isNumberInRange, toNumberOrNull } from './numbers';
+
+describe('checks if a number is a decimal number', () => {
+  it('returns false for non-decimal numbers', () => {
+    expect(isDecimalNumber(NaN)).toBe(false);
+    expect(isDecimalNumber(Infinity)).toBe(false);
+    expect(isDecimalNumber(-Infinity)).toBe(false);
+  });
+
+  it('returns true for decimal numbers', () => {
+    expect(isDecimalNumber(0)).toBe(true);
+    expect(isDecimalNumber(-0)).toBe(true);
+    expect(isDecimalNumber(10.12345)).toBe(true);
+    expect(isDecimalNumber(Number.MAX_VALUE)).toBe(true);
+    expect(isDecimalNumber(Number.MIN_VALUE)).toBe(true);
+  });
+});
+
+describe('checks if a number is within a range', () => {
+  it('returns false for numbers outside the range', () => {
+    expect(isNumberInRange(0, 10, 10)).toBe(false);
+    expect(isNumberInRange(0.01, 10, 10)).toBe(false);
+    expect(isNumberInRange(50.1, 0, 50)).toBe(false);
+  });
+
+  it('returns true for numbers inside the range', () => {
+    expect(isNumberInRange(0, 0, 50)).toBe(true);
+    expect(isNumberInRange(50, 0, 50)).toBe(true);
+    expect(isNumberInRange(-50.12345, -50.12345, 0)).toBe(true);
+  });
+});
+
+describe('converts input to a number or null', () => {
+  it('returns null for invalid inputs', () => {
+    expect(toNumberOrNull(null)).toBeNull();
+    expect(toNumberOrNull(undefined)).toBeNull();
+    expect(toNumberOrNull('')).toBeNull();
+    expect(toNumberOrNull(NaN)).toBeNull();
+  });
+
+  it('returns a number for valid inputs', () => {
+    expect(toNumberOrNull(0)).toBeCloseTo(0);
+    expect(toNumberOrNull('0')).toBeCloseTo(0);
+    expect(toNumberOrNull('-123.45')).toBeCloseTo(-123.45);
+  });
+});

server/apps/microservices/src/utils/numbers.ts

@@ -0,0 +1,19 @@
+export function isDecimalNumber(num: number): boolean {
+  return !Number.isNaN(num) && Number.isFinite(num);
+}
+
+/**
+ * Check if `num` is a valid number and is between `start` and `end` (inclusive)
+ */
+export function isNumberInRange(num: number, start: number, end: number): boolean {
+  return isDecimalNumber(num) && num >= start && num <= end;
+}
+
+export function toNumberOrNull(input: number | string | null | undefined): number | null {
+  if (input === null || input === undefined) {
+    return null;
+  }
+
+  const num = typeof input === 'string' ? Number.parseFloat(input) : input;
+  return isDecimalNumber(num) ? num : null;
+}