-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add AVIF plugin (decoder + encoder using libavif) #5201
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
Merged
Merged
Changes from 2 commits
Commits
Show all changes
64 commits
Select commit
Hold shift + click to select a range
3878b58
Add AVIF plugin (using libavif)
fdintino e2add24
Added type hints (#2)
radarhere e5494a2
Fix PLAT envvar in cibuildwheel container
fdintino 8b8bbba
Update Tests/check_avif_leaks.py
fdintino 58ef692
Simplified code (#3)
radarhere d6a0a15
Merge branch 'main' into libavif-plugin
radarhere 50b993a
Set default max threads in Python (#4)
radarhere 671e3c8
Removed unused upsampling setting (#5)
radarhere 658cdf3
Merge branch 'main' into libavif-plugin
radarhere 7225cb9
Merge branch 'main' into libavif-plugin
radarhere 3730bf2
Merge branch 'main' into libavif-plugin
radarhere c40bcbf
Improved error handling
radarhere d76ae2f
Do not ignore SyntaxError when saving EXIF data (#8)
radarhere 9ad8311
Allow libavif to install rav1e, except on manylinux2014 and aarch64 (#7)
radarhere de4c6c1
Removed ld64 flag (#6)
radarhere 524d802
fix: set exif orientation from irot/imir when decoding AVIF
fdintino 7b73d77
Merge branch 'main' into libavif-plugin
radarhere a56acd8
Removed unittest mock (#10)
radarhere f5dc957
Use cmds_cmake (#9)
radarhere 8d77678
chore(docs): update quality and speed with correct defaults
fdintino 4eaa6b7
Merge branch 'main' into libavif-plugin
radarhere bdb24f9
Removed `_avif.HAVE_AVIF` and `_avif.VERSION` (#11)
radarhere ddc8e7e
Use "rav1e" if available as default ("auto") avif encoder
fdintino b585f9e
Simplified EXIF code (#12)
radarhere da2e18d
Revert "Use "rav1e" if available as default ("auto") avif encoder"
fdintino 9b6e575
Merge branch 'main' into libavif-plugin
radarhere 3a9a3ab
Reduced epsilons (#13)
radarhere 9328932
Removed avifEncOptions (#14)
radarhere be02830
Merge branch 'main' into libavif-plugin
radarhere 4135664
Merge branch 'main' into libavif-plugin
radarhere 29c158d
Merge branch 'main' into libavif-plugin
radarhere 4c63ea6
Fixed series of tuples as advanced argument (#15)
radarhere ce6bf21
Merge branch 'main' into libavif-plugin
radarhere 4b29af4
Skip building libavif on 32-bit Windows (#16)
radarhere 38f0d10
Merge branch 'main' into libavif-plugin
radarhere 1410d23
Removed qmin and qmax (#17)
radarhere 6cbad27
Merge branch 'main' into libavif-plugin
radarhere 19ba2dd
Use rgb.rowBytes in overflow check (#18)
radarhere 4508f37
Use aom LICENSE instead of PATENTS (#19)
radarhere 7de1212
Merge branch 'main' into libavif-plugin
radarhere e1509ee
Removed memset and ignoreAlpha (#20)
radarhere 0590f08
Handle avifDecoderCreate and avifEncoderCreate errors (#21)
radarhere 5761b44
Merge branch 'main' into libavif-plugin
radarhere 38b9941
Sort formats alphabetically
radarhere 10dfa63
Simplified code
radarhere 9abfdbc
Merge branch 'main' into libavif-plugin
radarhere d80ac3c
Use default PyTypeObject values
radarhere b4eec64
Merge branch 'main' into libavif-plugin
radarhere d552087
Updated libavif to 1.2.1 (#26)
radarhere 79f7339
Updated wording
radarhere 46f4508
Added version comments
radarhere 9bebf37
Sort alphabetically
radarhere 5da2113
Simplified code
radarhere 0732554
Merge branch 'main' into libavif-plugin
radarhere 9ea5e3d
Remove support for libavif < 1.0.0
radarhere fca6df2
Added get_codec_version() (#29)
radarhere 024a894
Merge branch 'radarhere-avif_1' into libavif-plugin
fdintino 2ba9356
Update rust (#33)
radarhere fdc68e6
Merge branch 'main' into libavif-plugin
radarhere 9e63868
Continue to build libyuv on macOS
radarhere eff2680
Added release notes
radarhere fb096e1
Derive some avif test images from existing Pillow test images
fdintino 1276543
Updated size
radarhere c8d0408
Continue to build libyuv on Linux
radarhere File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is this guaranteed to not overflow, even in the face of invalid input?
Uh oh!
There was an error while loading. Please reload this page.
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.
libavif currently restricts images to a maximum of 2^28 pixels. If the dimensions are larger than 16384x16384 then the function that sets
decoder->image->width
anddecoder->image->height
fails. So I suppose that a 4-channel 16384x16384 8-bit image could overflow on a 32-bit platform. I'm not certain because the codecs used by libavif have their own overflow limit checks. For instance, dav1d enforces a maximum of 2^26 pixels on 32-bit systems. Should I add a check againstPY_SSIZE_T_MAX
to be sure? (edit: answering my own question and adding this check)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.
Added here
Pillow/src/_avif.c
Lines 619 to 622 in b84a8e0
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.
Basically, I'm the one who will get a CVE on this if there's a problem, and I'd like really clear guidelines about what the assumptions are for sizes of things and where they come from for dangerous operations like memset, malloc, and pointer reads/writes. This isn't so much for now, but a couple years down the line, things need to be clear. This will be fuzzed, this will be run under valgrind, so hopefully there won't be problems.
I've basically had to reverse engineer how SgiRleDecode works over the last month or so, and I'd like to be preventing that sort of experience in the future.
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.
Does raising a
MemoryError
ifrgb.height > PY_SSIZE_T_MAX / row_bytes
(as I have in the latest PR push) suffice to address that concern?