Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the built-in jpg.js decoder for JPEG images with non-default EXIF orientation (bug 1942064) #19349

Closed
wants to merge 1 commit into from

Conversation

Snuffleupagus
Copy link
Collaborator

The new EXIF parsing, during JpegImage.canUseImageDecoder, takes well below 1 ms in my testing which should be fast enough to not be an issue.

The EXIF parsing is done using https://github.com/exif-js/exif-js/, which is released under the MIT License. Note that while it's available on NPM we're however not using that since:

  • The package is not fully up-to-date, given the latest changes in the Git repository.
  • We only need a subset of the functionality, and this way we're able to simplify and shorten the code.
  • Given the age of that project the code isn't directly compatible with JavaScript modules.

external/exif-js/exif.js Fixed Show fixed Hide fixed
@Snuffleupagus Snuffleupagus force-pushed the bug-1942064 branch 3 times, most recently from f7c00a2 to cc1c91b Compare January 19, 2025 11:37
@Snuffleupagus
Copy link
Collaborator Author

/botio unittest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/3dbc03217247b4a/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.193.163.58:8877/66c431f69767b4e/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/3dbc03217247b4a/output.txt

Total script time: 2.33 mins

  • Unit Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/66c431f69767b4e/output.txt

Total script time: 8.16 mins

  • Unit Tests: FAILED

@Snuffleupagus
Copy link
Collaborator Author

/botio browsertest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_browsertest from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.241.84.105:8877/a619053271dc48e/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_browsertest from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.193.163.58:8877/9a2074e01a17677/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/a619053271dc48e/output.txt

Total script time: 16.55 mins

  • Regression tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/9a2074e01a17677/output.txt

Total script time: 30.12 mins

  • Regression tests: Passed

…IF orientation (bug 1942064)

The new EXIF parsing, during `JpegImage.canUseImageDecoder`, takes *well below* `1 ms` in my testing which should be fast enough to not be an issue.

The EXIF parsing is done using https://github.com/exif-js/exif-js/, which is released under the MIT License. Note that while it's available on NPM we're however not using that since:
 - The package is not fully up-to-date, given the latest changes in the Git repository.
 - We only need a *subset* of the functionality, and this way we're able to simplify and shorten the code.
 - Given the age of that project the code isn't directly compatible with JavaScript modules.
@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Jan 20, 2025

The diff below should be a safe way to only overwrite the EXIF orientation, by "validating" the format of the data explicitly first.

However, as discussed, if it's safe to just replace the entire EXIF-block with fill-bytes (0xFF) that'd obviously be a lot easier and shorter by not requiring an EXIF parser.

diff --git a/external/exif-js/exif.js b/external/exif-js/exif.js
index 3bba4a643..97de3991f 100644
--- a/external/exif-js/exif.js
+++ b/external/exif-js/exif.js
@@ -75,8 +75,19 @@ function readTags(file, tiffStart, dirStart, strings, bigEnd) {
 
   for (let i = 0; i < entries; i++) {
     const entryOffset = dirStart + i * 12 + 2;
-    const tag = strings[file.getUint16(entryOffset, !bigEnd)];
+    const rawTag = file.getUint16(entryOffset, !bigEnd);
+    const tag = strings[rawTag];
     tags[tag] = readTagValue(file, entryOffset, tiffStart, dirStart, bigEnd);
+
+    //
+    if (rawTag === 0x0112 && bigEnd) {
+      const type = file.getUint16(entryOffset + 2, !bigEnd),
+        numValues = file.getUint32(entryOffset + 4, !bigEnd);
+
+      if (type === 3 && numValues === 1) {
+        tags[`_${tag}_offset`] = entryOffset + 8;
+      }
+    }
   }
   return tags;
 }
diff --git a/src/core/jpg.js b/src/core/jpg.js
index cf4e16193..88eb95021 100644
--- a/src/core/jpg.js
+++ b/src/core/jpg.js
@@ -847,7 +847,14 @@ class JpegImage {
               // Skip images with non-default orientation
               // (fixes bug1942064.pdf).
               if (exif.Orientation !== undefined && exif.Orientation !== 1) {
-                return false;
+                const orientionOffset = exif._Orientation_offset;
+
+                if (orientionOffset) {
+                  appData[orientionOffset] = 0x00;
+                  appData[orientionOffset + 1] = 0x01;
+                } else {
+                  return false;
+                }
               }
             }
           }

@Snuffleupagus Snuffleupagus removed the request for review from calixteman January 20, 2025 15:08
@Snuffleupagus Snuffleupagus deleted the bug-1942064 branch January 20, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants