sharp icon indicating copy to clipboard operation
sharp copied to clipboard

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

Open phal0r opened this issue 5 months ago • 12 comments

Possible bug

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

  • [x] Running npm install sharp completes without error.
  • [x] 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?

  • [x] 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 avatar Jan 14 '24 11:01 phal0r

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

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

lovell avatar Jan 15 '24 08:01 lovell

Email sent with all information.

phal0r avatar Jan 15 '24 15:01 phal0r

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.

lovell avatar Jan 15 '24 22:01 lovell

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?

phal0r avatar Jan 15 '24 22:01 phal0r

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

lovell avatar Jan 15 '24 23:01 lovell

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 avatar Jan 16 '24 14:01 phal0r

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.

phal0r avatar Jan 16 '24 16:01 phal0r

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.

lovell avatar Jan 17 '24 12:01 lovell

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

phal0r avatar Jan 19 '24 12:01 phal0r

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?

phal0r avatar Jan 19 '24 15:01 phal0r

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).

lovell avatar Jan 19 '24 19:01 lovell

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();

setrol avatar Apr 19 '24 13:04 setrol