Investigate why some tests lossy test images aren't bit-exact with reference

[fintelia]

Our reference tests for lossy WebP images currently check that 90+% of pixels are bit-exact with the output:

image-webp/tests/decode.rs

Lines 87 to 97 in af3d0f3

	// NOTE: WebP lossy images are stored in YUV format. The conversion to RGB is not precisely
	// defined, but we currently attempt to match the dwebp's "-nofancy" conversion option.
	//
	// TODO: Investigate why we don't get bit exact output for all pixels.
	let num_bytes_different = data
	.iter()
	.zip(reference_data.iter())
	.filter(|(a, b)| a != b)
	.count();
	let percentage_different = 100 * num_bytes_different / data.len();
	if percentage_different >= 10 {

This is because there are still some small fraction of pixels that differ from the references produced by libwebp despite our attempts to exactly match its output. In particular, the actual percentage differences are:

reftest_animated_random_lossy: 0.016%

reftest_gallery1_2: 0.15%
reftest_gallery1_4: 0.1%
reftest_gallery1_5: 0.0035%
 
reftest_gallery2_1_webp_a: 5.3%
reftest_gallery2_2_webp_a: 1.4%
reftest_gallery2_3_webp_a: 0.96%
reftest_gallery2_4_webp_a: 1.7%
reftest_gallery2_5_webp_a: 1.6%

[Shnatsel]

Since reftest_gallery2_1_webp_a has the highest divergence, I computed a diff of it with dwebp -nofancy output and boosted the levels to make it visible, since the difference is not perceptible by the naked eye:

Or overlaid on the original image:

[tristanphease]

From debugging through libwebp think this is the issue in loop filtering:
https://github.com/image-rs/image-webp/blob/af3d0f3d177475e30a5e8d54ce3034a8f54fc3b7/src/vp8.rs#L2076C9-L2090C10

if self.frame.keyframe {
    if filter_level >= 40 {
        hev_threshold = 2;
    } else {
        hev_threshold = 1;
    }
} else {
    ...
}

but in the vp8 spec on page 96: https://datatracker.ietf.org/doc/html/rfc6386#section-15.4

if (we_are_decoding_akey_frame)   /* current frame is a key frame */
   {
       if (loop_filter_level >= 40)
           hev_threshold = 2;
       else if (loop_filter_level >= 15)
           hev_threshold = 1;
   }
   else                            /* current frame is an interframe */
   {
       ...
   }

Missing one if statement in the else if :/

[tristanphease]

Okay investigating further and found another issue where it's doing subblock loop filtering when it shouldn't be.
From the docs: https://datatracker.ietf.org/doc/html/rfc6386#section-15.1 (page 86)

Steps 2 and 4 are skipped for macroblocks that satisfy both of the following two conditions:
   1.  Macroblock coding mode is neither B_PRED nor SPLITMV; and

   2.  There is no DCT coefficient coded for the whole macroblock.

We instead do it just when if mb.luma_mode == LumaMode::B || !mb.coeffs_skipped
The coding mode check seems correct(not sure how libwebp checks that) but the coeffs skipped check is incorrect in that it should also check whether we have a non zero dct when we read in read_residual_data().
After making that change the only differences I could find between the libwebp data after loop filtering and our webp decoder after loop filtering was in some of the bottom macroblocks. We might have an issue where we're not doing the loop filtering there since we don't have the data since we're doing it on the actual frame data instead of the block data so we might need to move the loop filtering to happen as part of the macroblock decoding which is a bit of rework to do.

[Shnatsel]

After #148 and #149 there are no more divergences in still images on my corpus of 7500 WebPs scraped from the web! 🎉

Closing as completed.