lovell / sharp

High performance Node.js image processing, the fastest module to resize JPEG, PNG, WebP, AVIF and TIFF images. Uses the libvips library.
https://sharp.pixelplumbing.com
Apache License 2.0
29.19k stars 1.3k forks source link

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

Open phal0r opened 9 months ago

phal0r commented 9 months ago

Possible bug

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

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

Are you using the latest version of sharp?

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!

lovell commented 9 months ago

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

https://github.com/lovell/sharp/blob/0dcc7d50a820867e310c48b7316e4d2686241113/package.json#L5

phal0r commented 9 months ago

Email sent with all information.

lovell commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

phal0r commented 9 months ago

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 commented 9 months ago

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 commented 6 months ago

Hi,

it is also possible to recreate this behaviour with the picture provided in issue #4059 (which appears to be a rotated iPhone 13 snapshot) for example with following area:

const sharpStream = sharp(bufferInput, { failOn: 'none' });
sharpStream.rotate().extract({
      left: 287,
      top: 575,
      width: 3583,
      height: 1116,
    })
sharpStream.toBuffer();

I am not sure if it helps, but i could provide a rotated iPhone 14 snapshot with the same behaviour.

At least for the iPhone images the rotate() function by itself seem to work as expected. It's just the combination of rotate().extract() that fails. The following is not recommended for anybody as bugfix but the result is as expected:

let sharpStream = sharp(bufferInput, { failOn: 'none' });
sharpStream.rotate();
sharpStream = sharp(await sharpStream.toBuffer(), { failOn: 'none' });
sharpStream.extract({
      left: 287,
      top: 575,
      width: 3583,
      height: 1116,
    })
sharpStream.toBuffer();