-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update to the latest image crate #16
Conversation
The resizer should attempt to fit the resize into the desired size. One way to do this, if the first attempt does not succeed, is to keep trying by lowering the quality until we hit a pre-defined floor of 15. Return the same error as before if even that doesn't work.
wasm-thumbnail/src/lib.rs
Outdated
|
||
let mut mquality = nquality; | ||
|
||
while mquality >= 15 { |
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.
So this is definitely an improvement, but I think we could theoretically still not manage to create a small enough image for the size. (maybe for a massive resolution noisy image, with a small nsize
).
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.
Yes, but if we've made ~17 attempts to resize the image and failed, I think it's fair to fall back to previous behavior and have the external program notify the user that a different set of parameters is required, (smaller rez, different image, etc)
Closes https://github.com/brave-intl/creators-private-issues/issues/1754
This PR started as work to add looping into the library so that we could iteratively reduce the quality until the resized image would fit into the requested bytes, throwing an exception if we couldn't do it. Instead, we'll add that work to the rust service calling it, and just update the crate for now. Note that we investigated adding avif-decode support, and could do it if we didn't need to also compile to wasm. As it stands, libC support is not fully baked for the rust wasm compiler, so we'll need to hold off for now.