Repository navigation
Conversation
write_icc_profile numbered its APP2 chunks from 0. ICC.1 Annex B.4 numbers them from 1, as libjpeg's own jpeg_write_icc_profile does, and readers that validate the sequence (zune-jpeg, through the image crate, for one) ignore a profile that starts at 0, so the file reads as untagged. The icc_profile test now checks each chunk's sequence number and count and that the chunks reassemble to the original profile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Willyham
marked this pull request as ready for review
September 27, 2026 13:03
Author
|
@lilith not sure if you're still maintaining this or not - if you are, can you take a look? Thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Compress::write_icc_profilenumbers itsAPP2chunks from 0 (enumerate()is zero-based), so a one-chunk profile is written as chunk 0 of 1. The ICC specification (ICC.1, Annex B.4, "Embedding ICC profiles in JPEG files") numbers them from 1, and libjpeg's ownjpeg_write_icc_profilein the vendored source does the same (int cur_marker = 1; /* per spec, counting starts at 1 */,jcicc.c).Readers that validate the sequence drop a profile that starts at 0. We hit this with
zune-jpeg(the decoder behind theimagecrate), which returned no ICC profile at all for files this crate wrote, so they read as untagged. Lenient readers that ignore the sequence byte still find the profile, which is probably why this went unnoticed.The fix is one line:
current_marker + 1. Theicc_profiletest previously checked only that each chunk begins withICC_PROFILE\0; it now also checks each chunk's sequence number and chunk count, and that the chunks reassemble totests/test.icc. The new assertions fail on the current code (sequence number: left 0, right 1) and pass with the fix.cargo testpasses; the clippy and rustfmt output is unchanged by this patch.(An alternative would be to call
ffi::jpeg_write_icc_profiledirectly, but that binding is behindmozjpeg-sys'sicc_iofeature, which this crate doesn't enable, so I kept the change minimal.)🤖 Generated with Claude Code