AVIF encoder changes. #1838
Merged
AVIF encoder changes. #1838
Conversation
This file contains 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
|
If I understand things correctly, the failure(s) here are due to an external issue, namely cross-rs/cross#1177. I hadn't bothered with figuring that out until now, because we're still in a part of the year where fairly little happens. :) |
fintelia
reviewed
Jan 6, 2023
This moves a buffer used for temporary storage off of the struct and into the function where it's used. No change to public APIs.
fintelia
approved these changes
Jan 7, 2023
|
I made an additional change, which is that the encoder now declares that it implements the |
fintelia
reviewed
Jan 7, 2023
|
|
||
| impl<W: Write> ImageEncoder for AvifEncoder<W> { |
For some reason, AvifEncoder previously did not declare an implementation of the ImageEncoder trait. This commit also includes a couple of style suggestions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As noted in #1760, AVIF encoding is very slow. In looking at why, I noticed that the default encoder settings are very different from what the
cavifCLI provided by upstream uses - in particular, upstream documentation suggests that a quality setting of 100 is not a useful configuration.The majority of the changes here are to upgrade the
ravifdependency. The images I was using for testing appear to compress significantly better using the new version.The project requests this statement to be made:
I would like to add that I do not believe the changes in question to be significant enough to warrant copyright.