avifenc: add --ignore-alpha flag to discard alpha channel on encode#3216
avifenc: add --ignore-alpha flag to discard alpha channel on encode#3216wantehchang wants to merge 3 commits into
Conversation
wantehchang
force-pushed
the
copy-of-enyetsuen-pr-3130
branch
3 times, most recently
from
499b3c2 to
bd45ed0
Compare
wantehchang
left a comment
wantehchang
left a comment
There was a problem hiding this comment.
Vincent: I requested a review from you because I remember you worked on the avifReadImage() function. This PR is quite large, but there are a lot of mechanical changes. I suggest reviewing the code in the following order:
- CHANGELOG.md
- apps/avifenc.c
- doc/avifenc.1.md
- apps/shared/avifutil.{h,c}
- apps/shared/avifpng.{h,c}
- apps/shared/y4m.{h,c}
- tests/test_cmd.sh
- Other files
Thank you!
@vigneshvg Vignesh: FYI. You requested the --ignore-alpha option in issue #1665.
wantehchang
force-pushed
the
copy-of-enyetsuen-pr-3130
branch
from
bd45ed0 to
e28e60f
Compare
Remove the --noalpha flag. Just add --ignore-alpha. Make stylistic changes. Have avifInputReadImage() propagate the ignoreAlpha parameter further down the call chain all the way to the leaf functions. Add a changelog entry. Update doc/avifenc.1.md. Add a test for PNG input.
wantehchang
force-pushed
the
copy-of-enyetsuen-pr-3130
branch
from
e28e60f to
e481fbb
Compare
| ignoreColorProfile, | ||
| ignoreExif, | ||
| ignoreXMP, | ||
| ignoreAlpha, |
There was a problem hiding this comment.
Vincent: When I wrote this PR, I didn't realize that avifImageFreePlanes() is a public function. Since adding the ignoreAlpha parameter to avifReadImage() makes it necessary to update its callers, an alternative solution is to leave avifReadImage() unchanged and call avifImageFreePlanes(dstImage, AVIF_PLANES_A) if ignoreAlpha is true. This will make the PR much smaller at the cost of allocating and copying the alpha plane from the input image unnecessarily. Would you prefer this alternative solution?
There was a problem hiding this comment.
It's ok, it's all internal. We should probably merge those ignores flags into a struct anyway.
| ignoreColorProfile, | ||
| ignoreExif, | ||
| ignoreXMP, | ||
| ignoreAlpha, |
There was a problem hiding this comment.
It's ok, it's all internal. We should probably merge those ignores flags into a struct anyway.
3 participants
The first and second commits are copies of the first and second commits in pull request #3130 by @enyetsuen.
The third commit contains my edits and further changes.