Merge pull request #826 from grasdk/master

Fix for issue #794 (bugfix)
2024-12-23 01:27:14 +02:00 · 2024-02-08 23:12:21 +01:00 · 2024-02-08 23:12:21 +01:00 · 8c3bd650ad
commit 8c3bd650ad
parent 16350a9c24 59506d7acd
14 changed files with 213 additions and 83 deletions
--- a/.gitignore
+++ b/.gitignore
@ -9,10 +9,14 @@ src/frontend/dist
 test/coverage
 test/backend/**/*.js
 test/backend/**/*.js.map
+test/frontend/**/*.js
+test/frontend/**/*.js.map
 test/common/**/*.js
 test/common/**/*.js.map
 test/e2e/**/*.js
 test/e2e/**/*.js.map
+test/*.js
+test/*.js.map
 benchmark/**/*.js
 benchmark/**/*.js.map
 gulpfile.js
@ -34,3 +38,5 @@ test.*
 *.sublime-project
 *.sublime-workspace
 .DS_Store
+/coverage/
+.nyc_output/
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@ -13,7 +13,8 @@ In general, I'm happy to merge PRs, but I recommend filling a ticket and ask fir
 1. Download the source files
 2. install dependencies `npm install`
 3. Build client  `npm run run-dev`
-   * This will build the client with english localization and will keep building if you change the source files
+   * This will build the client with english localization and will keep building if you change the source files. 
+   * Note: This process does not exit, so you need another terminal to run the next step.
 4. Build the backend `npm run build-backend`
   * This runs `tsc` that transpiles `.ts` files to `.js` so node can run them. 
     * To rebuild on change run `tsc -w`
--- a/demo/images/Chars_exiftool.jpg
+++ b/demo/images/Chars_exiftool.jpg
--- a/package-lock.json
+++ b/package-lock.json
@ -17,7 +17,6 @@
        "csurf": "1.11.0",
        "ejs": "3.1.8",
        "exifr": "7.1.3",
-        "exifreader": "4.10.0",
        "express": "4.18.2",
        "express-unless": "2.1.3",
        "fluent-ffmpeg": "2.1.2",
@ -4810,14 +4809,6 @@
        "@xtuc/long": "4.2.2"
      }
    },
-    "node_modules/@xmldom/xmldom": {
-      "version": "0.7.9",
-      "license": "MIT",
-      "optional": true,
-      "engines": {
-        "node": ">=10.0.0"
-      }
-    },
    "node_modules/@xtuc/ieee754": {
      "version": "1.2.0",
      "dev": true,
@ -9660,14 +9651,6 @@
      "version": "7.1.3",
      "license": "MIT"
    },
-    "node_modules/exifreader": {
-      "version": "4.10.0",
-      "hasInstallScript": true,
-      "license": "MPL-2.0",
-      "optionalDependencies": {
-        "@xmldom/xmldom": "^0.7.8"
-      }
-    },
    "node_modules/exit-on-epipe": {
      "version": "1.0.1",
      "license": "Apache-2.0",
@ -25032,10 +25015,6 @@
        "@xtuc/long": "4.2.2"
      }
    },
-    "@xmldom/xmldom": {
-      "version": "0.7.9",
-      "optional": true
-    },
    "@xtuc/ieee754": {
      "version": "1.2.0",
      "dev": true
@ -28208,12 +28187,6 @@
    "exifr": {
      "version": "7.1.3"
    },
-    "exifreader": {
-      "version": "4.10.0",
-      "requires": {
-        "@xmldom/xmldom": "^0.7.8"
-      }
-    },
    "exit-on-epipe": {
      "version": "1.0.1"
    },
--- a/package.json
+++ b/package.json
@ -44,7 +44,6 @@
    "csurf": "1.11.0",
    "ejs": "3.1.8",
    "exifr": "7.1.3",
-    "exifreader": "4.10.0",
    "express": "4.18.2",
    "express-unless": "2.1.3",
    "fluent-ffmpeg": "2.1.2",
--- a/src/backend/model/fileaccess/MetadataLoader.ts
+++ b/src/backend/model/fileaccess/MetadataLoader.ts
@ -1,23 +1,22 @@
-import {VideoMetadata} from '../../../common/entities/VideoDTO';
-import {FaceRegion, PhotoMetadata} from '../../../common/entities/PhotoDTO';
-import {SideCar} from '../../../common/entities/MediaDTO';
-import {Config} from '../../../common/config/private/Config';
-import {Logger} from '../../Logger';
 import * as fs from 'fs';
-import {imageSize} from 'image-size';
+import { imageSize } from 'image-size';
+import { Config } from '../../../common/config/private/Config';
+import { SideCar } from '../../../common/entities/MediaDTO';
+import { FaceRegion, PhotoMetadata } from '../../../common/entities/PhotoDTO';
+import { VideoMetadata } from '../../../common/entities/VideoDTO';
+import { Logger } from '../../Logger';
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore
-import * as ExifReader from 'exifreader';
-import {ExifParserFactory, OrientationTypes} from 'ts-exif-parser';
-import {IptcParser} from 'ts-node-iptc';
-import {FFmpegFactory} from '../FFmpegFactory';
-import {FfprobeData} from 'fluent-ffmpeg';
-import {Utils} from '../../../common/Utils';
-import {ExtensionDecorator} from '../extension/ExtensionDecorator';
 import * as exifr from 'exifr';
-import * as path from 'path';
+import { FfprobeData } from 'fluent-ffmpeg';
+import { FileHandle } from 'fs/promises';
 import * as util from 'node:util';
-import {FileHandle} from 'fs/promises';
+import * as path from 'path';
+import { ExifParserFactory, OrientationTypes } from 'ts-exif-parser';
+import { IptcParser } from 'ts-node-iptc';
+import { Utils } from '../../../common/Utils';
+import { FFmpegFactory } from '../FFmpegFactory';
+import { ExtensionDecorator } from '../extension/ExtensionDecorator';

 const LOG_TAG = '[MetadataLoader]';
 const ffmpeg = FFmpegFactory.get();
@ -358,33 +357,44 @@ export class MetadataLoader {
        }

        try {
-          // TODO: clean up the three different exif readers,
-          //  and keep the minimum amount only
-          const exif: ExifReader.Tags & ExifReader.XmpTags & ExifReader.IccTags = ExifReader.load(data);
-          if (exif.Rating) {
-            metadata.rating = parseInt(exif.Rating.value as string, 10) as 0 | 1 | 2 | 3 | 4 | 5;
+          const exifrOptions = {
+            tiff: true,
+            xmp: true,
+            icc: false,
+            jfif: false, //not needed and not supported for png
+            ihdr: true,
+            iptc: false, //exifr reads UTF8-encoded data wrongly
+            exif: true,
+            gps: true,
+            translateValues: false, //don't translate orientation from numbers to strings etc.
+            mergeOutput: false //don't merge output, because things like Microsoft Rating (percent) and xmp.rating will be merged
+        };
+        
+        const exif = await exifr.parse(data, exifrOptions);
+          if (exif.xmp && exif.xmp.Rating) {
+            metadata.rating = exif.xmp.Rating;
            if (metadata.rating < 0) {
              metadata.rating = 0;
            }
          }
-          if (
-            exif.subject &&
-            exif.subject.value &&
-            exif.subject.value.length > 0
-          ) {
+          if (exif.dc &&
+              exif.dc.subject &&
+              exif.dc.subject.length > 0) {
+            const subj = Array.isArray(exif.dc.subject) ? exif.dc.subject : [exif.dc.subject];
            if (metadata.keywords === undefined) {
-              metadata.keywords = [];
+                metadata.keywords = [];
            }
-            for (const kw of exif.subject.value as ExifReader.XmpTag[]) {
-              if (metadata.keywords.indexOf(kw.description) === -1) {
-                metadata.keywords.push(kw.description);
-              }
+            for (const kw of subj) {
+                if (metadata.keywords.indexOf(kw) === -1) {
+                    metadata.keywords.push(kw);
+                }
            }
-          }
+        }
          let orientation = OrientationTypes.TOP_LEFT;
-          if (exif.Orientation) {
+          if (exif.ifd0 &&
+            exif.ifd0.Orientation) {
            orientation = parseInt(
-              exif.Orientation.value as any,
+              exif.ifd0.Orientation as any,
              10
            ) as number;
          }
@ -396,9 +406,11 @@ export class MetadataLoader {
            metadata.size.height = height;
          }

-          if (Config.Faces.enabled) {
+          if (Config.Faces.enabled &&
+            exif["mwg-rs"] &&
+            exif["mwg-rs"].Regions) {
            const faces: FaceRegion[] = [];
-            const regionListVal = ((exif.Regions?.value as any)?.RegionList)?.value;
+            const regionListVal = Array.isArray(exif["mwg-rs"].Regions.RegionList) ? exif["mwg-rs"].Regions.RegionList : [exif["mwg-rs"].Regions.RegionList];
            if (regionListVal) {
              for (const regionRoot of regionListVal) {
                let type;
@ -442,16 +454,16 @@ export class MetadataLoader {

                /* Adobe Lightroom based face region structure */
                if (
-                  regionRoot.value &&
-                  regionRoot.value['rdf:Description'] &&
-                  regionRoot.value['rdf:Description'].value &&
-                  regionRoot.value['rdf:Description'].value['mwg-rs:Area']
+                  regionRoot &&
+                  regionRoot['rdf:Description'] &&
+                  regionRoot['rdf:Description'] &&
+                  regionRoot['rdf:Description']['mwg-rs:Area']
                ) {
-                  const region = regionRoot.value['rdf:Description'];
-                  const regionBox = region.value['mwg-rs:Area'].attributes;
+                  const region = regionRoot['rdf:Description'];
+                  const regionBox = region['mwg-rs:Area'].attributes;

-                  name = region.attributes['mwg-rs:Name'];
-                  type = region.attributes['mwg-rs:Type'];
+                  name = region['mwg-rs:Name'];
+                  type = region['mwg-rs:Type'];
                  box = createFaceBox(
                    regionBox['stArea:w'],
                    regionBox['stArea:h'],
@ -460,18 +472,19 @@ export class MetadataLoader {
                  );
                  /* Load exiftool edited face region structure, see github issue #191 */
                } else if (
-                  regionRoot.Area &&
+                  regionRoot &&
                  regionRoot.Name &&
-                  regionRoot.Type
+                  regionRoot.Type &&
+                  regionRoot.Area
                ) {
-                  const regionBox = regionRoot.Area.value;
-                  name = regionRoot.Name.value;
-                  type = regionRoot.Type.value;
+                  const regionBox = regionRoot.Area;
+                  name = regionRoot.Name;
+                  type = regionRoot.Type;
                  box = createFaceBox(
-                    regionBox.w.value,
-                    regionBox.h.value,
-                    regionBox.x.value,
-                    regionBox.y.value
+                    regionBox.w,
+                    regionBox.h,
+                    regionBox.x,
+                    regionBox.y
                  );
                }

--- a/test/backend/assets/Chars.jpg
+++ b/test/backend/assets/Chars.jpg
--- a/test/backend/assets/Chars.json
+++ b/test/backend/assets/Chars.json
@ -0,0 +1,49 @@
+{
+  "size": {
+    "width": 1920,
+    "height": 1080
+  },
+  "creationDate": 1706659327000,
+  "fileSize": 111432,
+  "positionData": {
+    "GPSData": {
+      "longitude": 14.162922,
+      "latitude": 57.780696
+    },
+    "country": "Sverige",
+    "state": "Jönköping",
+    "city": "Jönköping"
+  },
+  "keywords": [
+  ],
+  "rating": 0,
+  "faces": [
+    {
+      "box": {
+        "width": 206,
+        "height": 257,
+        "left": 566,
+        "top": 144
+      },
+      "name": "æÆøØåÅéÉüÜäÄöÖïÏñÑ"
+    },
+    {
+      "name": "abcdefghijklmnopqrstuvwxyz",
+      "box": {
+        "width": 212,
+        "height": 265,
+        "left": 866,
+        "top": 144
+      }
+    },
+    {
+      "name": "abcdefghijklmnopqrstuvwxyz",
+      "box": {
+        "width": 212,
+        "height": 265,
+        "left": 1162,
+        "top": 150
+      }
+    }
+  ]
+}
--- a/test/backend/assets/Chars_exiftool.jpg
+++ b/test/backend/assets/Chars_exiftool.jpg
--- a/test/backend/assets/Chars_exiftool.json
+++ b/test/backend/assets/Chars_exiftool.json
@ -0,0 +1,49 @@
+{
+  "size": {
+    "width": 1920,
+    "height": 1080
+  },
+  "creationDate": 1706616000000,
+  "fileSize": 111050,
+  "positionData": {
+    "GPSData": {
+      "longitude": 14.162922,
+      "latitude": 57.780696
+    },
+    "country": "Sverige",
+    "state": "Jönköping",
+    "city": "Jönköping"
+  },
+  "keywords": [
+  ],
+  "rating": 0,
+  "faces": [
+    {
+      "box": {
+        "width": 206,
+        "height": 257,
+        "left": 566,
+        "top": 144
+      },
+      "name": "æÆøØåÅéÉüÜäÄöÖïÏñÑ"
+    },
+    {
+      "name": "abcdefghijklmnopqrstuvwxyz",
+      "box": {
+        "width": 212,
+        "height": 265,
+        "left": 866,
+        "top": 144
+      }
+    },
+    {
+      "name": "abcdefghijklmnopqrstuvwxyz",
+      "box": {
+        "width": 212,
+        "height": 265,
+        "left": 1162,
+        "top": 150
+      }
+    }
+  ]
+}
--- a/test/backend/assets/png_with_keyword_and_dates.json
+++ b/test/backend/assets/png_with_keyword_and_dates.json
@ -0,0 +1,31 @@
+
+{
+  "size": {
+    "width": 26,
+    "height": 26
+  },
+  "creationDate": 1707167247786,
+  "fileSize": 5758,
+  "keywords": [
+  ],
+  "faces": [
+    {
+      "name": "raspberry",
+      "box": {
+        "width": 21,
+        "height": 18,
+        "left": 3,
+        "top": 8
+      }
+    },
+    {
+      "name": "leaf",
+      "box": {
+        "width": 9,
+        "height": 7,
+        "left": 14,
+        "top": 1
+      }
+    }
+  ]
+}
--- a/test/backend/assets/png_with_keyword_and_dates.png
+++ b/test/backend/assets/png_with_keyword_and_dates.png
--- a/test/backend/unit/model/threading/DiskManagerWorker.spec.ts
+++ b/test/backend/unit/model/threading/DiskManagerWorker.spec.ts
@ -24,12 +24,11 @@ describe('DiskMangerWorker', () => {
    ProjectPath.ImageFolder = path.join(__dirname, '/../../../assets');
    const dir = await DiskManager.scanDirectory('/');
    // should match the number of media (photo/video) files in the assets folder
-    expect(dir.media.length).to.be.equals(11);
+    expect(dir.media.length).to.be.equals(14);
    // eslint-disable-next-line @typescript-eslint/no-var-requires
    const expected = require(path.join(__dirname, '/../../../assets/test image öüóőúéáű-.,.json'));
    const i = dir.media.findIndex(m => m.name === 'test image öüóőúéáű-.,.jpg');
    expect(Utils.clone(dir.media[i].name)).to.be.deep.equal('test image öüóőúéáű-.,.jpg');
    expect(Utils.clone(dir.media[i].metadata)).to.be.deep.equal(expected);
  });
-
 });
--- a/test/backend/unit/model/threading/MetaDataLoader.spec.ts
+++ b/test/backend/unit/model/threading/MetaDataLoader.spec.ts
@ -58,6 +58,16 @@ describe('MetadataLoader', () => {
    const expected = require(path.join(__dirname, '/../../../assets/old_photo.json'));
    expect(Utils.clone(data)).to.be.deep.equal(expected);
  });
+  it('should load jpg with special characters', async () => {
+    const data = await MetadataLoader.loadPhotoMetadata(path.join(__dirname, '/../../../assets/Chars.jpg'));
+    const expected = require(path.join(__dirname, '/../../../assets/Chars.json'));
+    expect(Utils.clone(data)).to.be.deep.equal(expected);
+  });
+  it('should load jpg with special characters saved by exiftool', async () => {
+    const data = await MetadataLoader.loadPhotoMetadata(path.join(__dirname, '/../../../assets/Chars_exiftool.jpg'));
+    const expected = require(path.join(__dirname, '/../../../assets/Chars_exiftool.json'));
+    expect(Utils.clone(data)).to.be.deep.equal(expected);
+  });

  describe('should load jpg with proper height and orientation', () => {
    it('jpg 1', async () => {