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

Do not allow keeping the ICC when converting from RGB to gray #2669

Merged
merged 4 commits into from
Mar 6, 2025

Conversation

vrabaud
Copy link
Collaborator

@vrabaud vrabaud commented Mar 5, 2025

This is related to #2639

@vrabaud vrabaud requested a review from maryla-uc March 5, 2025 14:01
@vrabaud vrabaud marked this pull request as ready for review March 5, 2025 14:10
@vrabaud vrabaud changed the title Do not allow to keep the ICC when converting from RGB to gray Do not allow keeping the ICC when converting from RGB to gray Mar 5, 2025
@vrabaud vrabaud force-pushed the icc_gray branch 2 times, most recently from 15fa390 to b08700a Compare March 5, 2025 14:19
@vrabaud vrabaud added this to the v1.2.1 milestone Mar 5, 2025
Copy link
Collaborator

@maryla-uc maryla-uc left a comment

Choose a reason for hiding this comment

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

This sounds like a fairly cryptic error message for a user to get, especially since they didn't explicitly "ask to keep" the icc profile, it's just the default behavior (I think?). Mentioning the flags would be clearer but in that case it would be better to do it in avifenc.c directly...
Something like
"The image contains an RGB ICC profile which is incompatible with the requested output format YUV400 (grayscale). Pass --ignore-icc to discard the ICC profile."

@vrabaud
Copy link
Collaborator Author

vrabaud commented Mar 6, 2025

Indeed, this API is internal and only used by tests and exposed in avifenc. I changed the message.

@vrabaud vrabaud merged commit 999fcf5 into AOMediaCodec:main Mar 6, 2025
19 checks passed
@vrabaud vrabaud deleted the icc_gray branch March 6, 2025 11:40
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