Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICCP support for new image formats #156

Merged
merged 3 commits into from
Mar 2, 2023
Merged

ICCP support for new image formats #156

merged 3 commits into from
Mar 2, 2023

Conversation

qbnu
Copy link
Contributor

@qbnu qbnu commented Mar 1, 2023

Adds support for ICC Profiles embedded in WebP, JPEG XL, HEIF/HEIC and AVIF images.
Fixes #148

@sylikc
Copy link
Owner

sylikc commented Mar 1, 2023

So from the code review, deletes cache for any of the animated support if it doesn't have animation right? (just want to put together a good changelog for all the awesome optimizations)

Does this also revert the "support ICC on PNG disables animation"... aka this supports ICC with animation?

@qbnu
Copy link
Contributor Author

qbnu commented Mar 2, 2023

So from the code review, deletes cache for any of the animated support if it doesn't have animation right? (just want to put together a good changelog for all the awesome optimizations)

yes

Does this also revert the "support ICC on PNG disables animation"... aka this supports ICC with animation?

No. PNG files can have other chunks besides an ICC profile that affect how the colors should look (example). GDI+ handles these, but I'm not sure how to with LCMS. I'll try adding ICCP support with TurboJPEG.


CMSAPI void CMSEXPORT cmsDeleteTransform(cmsHTRANSFORM hTransform);

CMSAPI void CMSEXPORT cmsDoTransform(cmsHTRANSFORM Transform,
Copy link
Owner

Choose a reason for hiding this comment

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

I guess the original function signature doesn't use that style

@qbnu
Copy link
Contributor Author

qbnu commented Mar 2, 2023

It looks like there's no way to get an ICC profile with the TurboJPEG API, so we'd have to use the libjpeg API to read the image separately. I think it would look something like this:

jpeg_decompress_struct cinfo;
JOCTET* icc_data_ptr = NULL;
unsigned int icc_data_len = 0;
jpeg_create_decompress(&cinfo);
jpeg_mem_src(&cinfo, (unsigned char*)buffer, sizebytes);
jpeg_save_markers(&cinfo, JPEG_APP0 + 2, 0xFFFF);
jpeg_read_header(&cinfo, false);
jpeg_read_icc_profile(&cinfo, &icc_data_ptr, &icc_data_len);
// ...

and then change the transform functions to handle 3 channel data, with proper padding for the output. I'll leave it as is for now.

@sylikc
Copy link
Owner

sylikc commented Mar 2, 2023

Cool. Thanks for the investigation. I'm sure people will be plenty happy with the addition of ICC on those 4 new formats! 😀

@sylikc sylikc added enhancement New feature or request format support Related to add/remove/change of a specific format support. labels Mar 2, 2023
sylikc added a commit that referenced this pull request Mar 2, 2023
… AVIF images. It applies automatically regardless of UseEmbeddedColorProfiles option.

Delete cache for immediately AVIF/JXL/PNG if not animated image.

Merge PR #156 by https://github.com/qbnu
@sylikc sylikc merged commit 351b8ad into sylikc:master Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request format support Related to add/remove/change of a specific format support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UseEmbeddedColorProfiles=true not working for WebP images
2 participants