Skip to content

End-Of-Image markers for more robust Progressive JPEG decoding#835

Merged
kean merged 2 commits into
kean:mainfrom
theop-luma:progressive-jpeg-decode
Feb 28, 2026
Merged

End-Of-Image markers for more robust Progressive JPEG decoding#835
kean merged 2 commits into
kean:mainfrom
theop-luma:progressive-jpeg-decode

Conversation

@theop-luma

@theop-luma theop-luma commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

This PR does a few things:

  1. Makes sure that JPEGs with thumbnails still decode correctly and the markers for the thumbnails do not affect the decoding. Previously if a progressive JPEG had a baseline thumbnail the image would be misidentified as baseline and the scan numbers would be off causing failures.
  2. Adds EOI (end of image, 0xFFD9) markers before attempting to decode a partial progressive JPEG binary. This seems to be the more 'canonical' way to decode, even though it looks like Apple's decoder sometimes manages to decode correctly even without that marker (that's why the Unit tests with progressive.jpeg image were passing). Added another image tricky_progressive.jpeg which trips up the decoder and requires those markers. Cross-checked with libjpeg end it also needs EOI markers.
  3. Switched byte while-loops with Data.firstIndex which seems to give a decent speed up. Also tried Data.range(of:) but that was slower.

@kean

kean commented Feb 28, 2026

Copy link
Copy Markdown
Owner

Thank you!

I download the attached file and was able to reproduce the issue. It does seem to be related to the embedded thumbnail. I will merge the PR and make a couple of changes on top of it: refactor a bit of the code and add a different test image transcoded for the existing baseline.jpeg (all the test images are the ones I own).

@kean kean merged commit 07fc472 into kean:main Feb 28, 2026
@kean

kean commented Feb 28, 2026

Copy link
Copy Markdown
Owner

Also, thank you for including the example based Image I/O example – that's the direction I'm going to be taking. I now plan to include it in the upcoming Nuke 13 release as it will require a relatively significant change.

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.

2 participants