Change default upscaling to bilinear interpolation #147
Conversation
| } | ||
| } | ||
|
|
||
| fn get_fancy_chroma_value(main: u8, secondary1: u8, secondary2: u8, tertiary: u8) -> u8 { |
There was a problem hiding this comment.
It's better to mark all tiny methods as #[inline] in hot points. They may already be inlined, but it's better to explicitly indicate that inlining is desired.
There was a problem hiding this comment.
Thanks, updated some of the small functions with that
|
Updated to incorporate some of the buffer changes from the other PR and also updated all the lossy reference images to be decoded with the default dwebp decoding settings and looks like it matches. Remaining question I have is do we want to incorporate the non-fancy option in the public API for this crate so it's usable? We currently don't have any decoding parameters in decode_frame(). |
src/yuv.rs
Outdated
| } | ||
|
|
||
| let remainder = rgb_chunks.into_remainder(); | ||
| if remainder.len() >= 4 { |
There was a problem hiding this comment.
It seems this method could be called with different BPP values. Is it correct that the indexing in this method is fixed to RGBA?
There was a problem hiding this comment.
Sorry, this was just a temporary thing leaving the function there for use later. Have changed now to work for both RGB and RGBA
|
I'm in favor of switching the default to bilinear interpolation and adding a method Please do update tests/CREDITS.md with the precise command to produce the reference images. |
Returns the old tests with the nofancy set for gallery1
|
Okay thanks, have added some options to the decoder so that the user can choose which method to use. Have updated tests/CREDITS.md as well |
src/decoder.rs
Outdated
| /// Sets the upsampling method used in conversion from lossy yuv to rgb | ||
| /// | ||
| /// Defaults to `Bilinear`. | ||
| pub fn set_lossy_upsampling(&mut self, lossy_upsampling: UpsamplingMethod) { |
There was a problem hiding this comment.
Would be preferable to make lossy_upsampling public and not need this method
There was a problem hiding this comment.
Thanks, changed
Fixes #142
Changes the default upscaling to instead of just taking the closest u/v chroma value average them out depending on their distance from the pixel, the same way that libwebp does it
Here's the bsk image from the issue decoded with this change
Still need to work out whether we want the non fancy upscaling as a configuration option in the API (currently have it unused) and also the tests currently fail since the difference between the references and the output is now more than 10%