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

Out-of-order EXIF tags generated by buggy Samsung devices are ignored #3947

Open
3 tasks done
phal0r opened this issue Jan 14, 2024 · 12 comments
Open
3 tasks done

Out-of-order EXIF tags generated by buggy Samsung devices are ignored #3947

phal0r opened this issue Jan 14, 2024 · 12 comments

Comments

@phal0r
Copy link

phal0r commented Jan 14, 2024

Possible bug

Is this a possible bug in a feature of sharp, unrelated to installation?

  • Running npm install sharp completes without error.
  • Running node -e "require('sharp')" completes without error.

If you cannot confirm both of these, please open an installation issue instead.

Are you using the latest version of sharp?

  • I am using the latest version of sharp as reported by npm view sharp dist-tags.latest.

If you cannot confirm this, please upgrade to the latest version and try again before opening an issue.

If you are using another package which depends on a version of sharp that is not the latest, please open an issue against that package instead.

What is the output of running npx envinfo --binaries --system --npmPackages=sharp --npmGlobalPackages=sharp?

What are the steps to reproduce?

We are running a community, where people an upload profile pictures. In the frontend they can select the picture and crop it, so the result is expected by the user. This works 99% of the time, but we get some pictures, where rotate().extract(crop) does not rotate the picture properly and it fails with bad extract area. Browsers and the default image tool on MacOS rotate these pictures properly.

Since these pictures are private, I would like to send an example with the expected crop coordinates on a private channel to verify, if the picture is detected wrong or if there is a bug in our code.

What is the expected behaviour?

It should rotate the picture according to the metadata correctly and extract the correct crop.

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem

let sharpStream = sharp({ failOn: 'none' })
sharpStream = sharpStream.rotate().extract({
      left: crop.x,
      top: crop.y,
      width: crop.width,
      height: crop.height,
    })

Please provide sample image(s) that help explain this problem

As mentioned above due to privacy concerns, I would like to send the picture via a private channel. Thanks in advance!

@phal0r phal0r added the triage label Jan 14, 2024
@lovell lovell added question and removed triage labels Jan 15, 2024
@lovell
Copy link
Owner

lovell commented Jan 15, 2024

Please provide a specific image and actual values that allow someone else to reproduce.

"author": "Lovell Fuller <npm@lovell.info>",

@phal0r
Copy link
Author

phal0r commented Jan 15, 2024

Email sent with all information.

@lovell
Copy link
Owner

lovell commented Jan 15, 2024

It looks like the image you've provided contains invalid EXIF:

$ exiftool -validate -warning -a 301816880.jpeg 
Validate                        : 46 Warnings (17 minor)
Warning                         : Entries in IFD0 are out of order
...

Some of the other metadata suggests this image was generated by a Samsung device, which are notorious for terrible firmware, and this would appear to be another example of it.

@phal0r
Copy link
Author

phal0r commented Jan 15, 2024

Thanks for the response and the insights. How do the browsers and other tools deal with it?

Because there it seems to work there and I cannot change, that our customers use Samsung devices or other Software, that might create such data.

Do you think it's possible to apply more relaxed rules for parsing like other Software does?

@lovell
Copy link
Owner

lovell commented Jan 15, 2024

It's possible it might relate to this logic in libvips, but I guess changing this behaviour could then break something else.

https://github.com/libvips/libvips/blob/6ee3da80afa0e5211d977cfc8e6034c874caa38c/libvips/foreign/exif.c#L184

@phal0r
Copy link
Author

phal0r commented Jan 16, 2024

Ok, thanks for digging in the sourcecode.

I am thinking about, what I could do to drive this issue forward. I think sharp and libvips should be able to process all common metadata in the same way browsers and other Software can do.

What do we need to make this happen?

@phal0r
Copy link
Author

phal0r commented Jan 16, 2024

One other thought: Would it be better to manually read the Orientation flag? I tried exiftool and I tried another node library and both read the orientation flag as 270 degree clockwise. So in the code example I would call rotate() not with the auto value, but always provide the value, I manually extracted.

@lovell
Copy link
Owner

lovell commented Jan 17, 2024

I had a quick look and cannot see any way of coercing libexif to process out of order tags.

For the image you provided, the Make tag is out of order and processing stops at that point.

$ exiftool -D 301816880.jpeg
...
  259 Compression                     : JPEG (old-style)
  513 Other Image Start               : 972
  514 Other Image Length              : 19899
  271 Make                            : samsung
  274 Orientation                     : Rotate 270 CW
...

So yes, as you suggest, perhaps pre-process images that might contain invalid EXIF with another tool first.

@phal0r
Copy link
Author

phal0r commented Jan 19, 2024

I tried different libraries and also exiftool and all were able to detect the correct orientation. Only libvips seems to fail.

@phal0r
Copy link
Author

phal0r commented Jan 19, 2024

I will try the route to determine the orientation with another library and pass it explicitely to rotate(). This should fix this.

I still think, libvips should also support this out of the box, because it makes the library harder to use, if we need to add third party libs for basic features. However I am not sure, if the issue here is the right on to track this (or at least request this). What do you think?

@lovell
Copy link
Owner

lovell commented Jan 19, 2024

From what I can tell, this is libexif behaviour.

I guess the next step would be for someone to produce a small program that proves this using only libexif, which might then also provide motivation and/or a test case for changing libexif to allow out-of-order tags (perhaps as part of its existing EXIF_DATA_OPTION_FOLLOW_SPECIFICATION option).

@setrol

This comment was marked as off-topic.

@lovell lovell changed the title rotate() and extract() do not rotate certain pictures properly (bad extract area) Out-of-order EXIF tags generated by buggy Samsung devices are ignored Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants