Skip to content

fix: JPEG detection fails on files with trailing data (Motion Photos) - #379

Open
tetsugakusha256 wants to merge 1 commit into
3rd:masterfrom
tetsugakusha256:fix/jpeg-trailing-data-detection
Open

fix: JPEG detection fails on files with trailing data (Motion Photos)#379
tetsugakusha256 wants to merge 1 commit into
3rd:masterfrom
tetsugakusha256:fix/jpeg-trailing-data-detection

Conversation

@tetsugakusha256

Copy link
Copy Markdown

detect_format() in magic.lua has a check that requires FFD9 (JPEG EOI) to
be the literal last 2 bytes of the file. That breaks on any JPEG that has
data appended after the actual image — Google/Samsung Motion Photos being
the common case (JPEG + embedded MP4 + trailer footer).

For these files the real EOI is somewhere in the middle, so
has_jpeg_end_signature() returns false and detect_format() returns nil for
the whole file, even though it's a perfectly valid JPEG. ImageMagick has
no problem with these files (tested manually with identify/convert), it
just stops reading at EOI like it's supposed to.

I don't think this check is worth keeping — no other format in the table
gets this kind of extra validation, and detect_format() has no fallback
to the identify-based path used elsewhere in the plugin, so this was a
silent permanent failure rather than just a perf hit.

Removed the EOI check and the now-unused has_jpeg_end_signature function.

Tested:

  • Motion Photo JPEGs now detected/rendered correctly
  • truncated JPEGs still fail sensibly downstream - partial render if
    there's enough data, clean identify error ("insufficient image data")
    if not

Files with data appended after the JPEG EOI marker (Google/Samsung
Motion Photos, etc.) were being rejected by detect_format/is_image
since has_jpeg_end_signature required FFD9 to be the file's final two
bytes. These are valid, fully renderable JPEGs — ImageMagick decodes
them fine, it just stops at EOI and ignores trailing data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant