Skip to content

Add support for reading ICC metadata and fix StreamingDecoder::last_ext#216

Merged
197g merged 2 commits intoimage-rs:masterfrom
1c3t3a:icc-profile
Sep 5, 2025
Merged

Add support for reading ICC metadata and fix StreamingDecoder::last_ext#216
197g merged 2 commits intoimage-rs:masterfrom
1c3t3a:icc-profile

Conversation

@1c3t3a
Copy link
Contributor

@1c3t3a 1c3t3a commented Sep 4, 2025

This change builds on #215 and fixes the now unfortunately broken last_ext method (sorry) + adds support for the ICC profile.

The XMP patch that landed recently did a `std::mem::take` on the extension
data - this broke the `last_ext` API.

Co-authored-by: Rasmus Piorr <piorr@google.com>
Copy link
Member

@197g 197g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too fond of mixing the block lengths with the data but it'll work, I guess? What we should go for is probably the more common option or the one that takes more processing. I'm not sure if that is ICC or XMP but it'll work for now so we can split data and block lengths in a separate PR. A few nits.

Copy link
Member

@197g 197g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed a behavior change that we didn't catch previously. Otherwise looks okay.

@1c3t3a 1c3t3a requested a review from 197g September 4, 2025 14:57
@1c3t3a 1c3t3a requested a review from kornelski September 5, 2025 08:29
@1c3t3a 1c3t3a force-pushed the icc-profile branch 3 times, most recently from 91d1c95 to b8d702c Compare September 5, 2025 08:38
@kornelski
Copy link
Contributor

There's "sublock" typo (sub_block) but otherwise looks ok

Copy link
Member

@197g 197g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 me as well with the typo in the internal field fixed.

@197g
Copy link
Member

197g commented Sep 5, 2025

(When releasing this we mustn't forget that it should be 0.14).

Co-authored-by: Rasmus Piorr <piorr@google.com>
@1c3t3a
Copy link
Contributor Author

1c3t3a commented Sep 5, 2025

Thanks a lot for the review!! :) Happy to see this landing!

@197g 197g merged commit 901951b into image-rs:master Sep 5, 2025
13 checks passed
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.

3 participants