-
Notifications
You must be signed in to change notification settings - Fork 208
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
Turn gain map flag on by default. #2513
Conversation
Remove 'EXPERIMENTAL' from its name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pull request.
Can we remove the AVIF_ENABLE_GAIN_MAP
option?
Remove EXPERIMENTAL
from AVIF_ENABLE_EXPERIMENTAL_JPEG_GAIN_MAP_CONVERSION
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the whole pull request. Other than whether AVIF_ENABLE_GAIN_MAP
should be kept or removed, it looks good. Here are some more comments.
@@ -676,7 +677,7 @@ if(AVIF_BUILD_APPS OR (AVIF_BUILD_TESTS AND (AVIF_ENABLE_FUZZTEST OR AVIF_ENABLE | |||
find_package(JPEG REQUIRED) | |||
endif() | |||
|
|||
if(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) | |||
if(AVIF_ENABLE_GAIN_MAP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove EXPERIMENTAL
from AVIF_ENABLE_EXPERIMENTAL_JPEG_GAIN_MAP_CONVERSION
?
Can do this in a follow-up CL.
I kept the AVIF_ENABLE_GAIN_MAP option for now just in case we needed to revert it to OFF for some reason, and to keep this PR simple (even though the rename means that there are still quite a few changes). |
As for AVIF_ENABLE_EXPERIMENTAL_JPEG_GAIN_MAP_CONVERSION, since it's still off by default, I kept the "experimental" name for now. Again, can be changed later. |
Remove 'EXPERIMENTAL' from its name.