Skip to content
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

Enable CDEF and restoration filters for 4:2:2 #2224

Merged
merged 2 commits into from
May 11, 2020

Conversation

barrbrain
Copy link
Collaborator

@barrbrain barrbrain commented Apr 8, 2020

As far as I can tell, there are no outstanding issues with this combination.
However, I will attempt to revive the fuzzer to answer this with confidence.
Closes #1149.

@barrbrain barrbrain requested a review from xiphmont April 8, 2020 03:31
@barrbrain barrbrain force-pushed the enable-restoration-filters-422 branch from 16bd5b0 to b54b8d0 Compare April 8, 2020 04:20
@barrbrain
Copy link
Collaborator Author

barrbrain commented Apr 8, 2020

I breathed life into the rav1e fuzzer, and encountered an attempt to subtract with overflow at src/lrf.rs:1416 with w = 16, h = 262 and tiles=2.

w = 16
h = 262
speed = 10
q = 255
limit = 1
min_keyint = 3
max_keyint = 3
low_latency = false
bitrate = 0
--
uv_unit_size = 32
uv_unit_log2 = 5
uv_sb_h_log2 = 5
uv_sb_v_log2 = 6
fi.sb_width = 1
fi.sb_height = 5
stripe_uv_decimate = 0
uv_cols = 1
uv_rows = 8

I was able to reproduce from the rav1e CLI:

ffmpeg -f lavfi -i "testsrc=duration=1:size=16x262:rate=30" -pix_fmt yuv422p 422.y4m
cargo run -- -o /dev/null 422.y4m --tiles 2

@barrbrain
Copy link
Collaborator Author

The cause may be at src/lrf.rs:1368:

uv_unit_size =
  uv_unit_size.min(tile_aligned_uv_h_unit_size.min(tile_aligned_uv_v_unit_size));

@lu-zero
Copy link
Collaborator

lu-zero commented Apr 8, 2020

It sounds possibly related to #2212

@xiphmont
Copy link
Contributor

xiphmont commented Apr 8, 2020 via email

@rzumer
Copy link
Collaborator

rzumer commented Apr 8, 2020

Also see #970.

@barrbrain barrbrain marked this pull request as draft April 10, 2020 04:40
@barrbrain barrbrain force-pushed the enable-restoration-filters-422 branch 2 times, most recently from d1d768e to a0f6a47 Compare May 4, 2020 14:54
@barrbrain barrbrain added the msu2020 MSU Annual Video Codecs Comparison 2020 label May 9, 2020
@barrbrain barrbrain force-pushed the enable-restoration-filters-422 branch from a0f6a47 to 9c4e982 Compare May 11, 2020 02:59
@coveralls
Copy link
Collaborator

coveralls commented May 11, 2020

Coverage Status

Coverage increased (+0.005%) to 82.595% when pulling 54d236b on barrbrain:enable-restoration-filters-422 into 4e922fe on xiph:master.

@barrbrain barrbrain force-pushed the enable-restoration-filters-422 branch from 9c4e982 to fb08f7e Compare May 11, 2020 09:39
@barrbrain barrbrain marked this pull request as ready for review May 11, 2020 09:44
@barrbrain
Copy link
Collaborator Author

barrbrain commented May 11, 2020

The newest commit resolves the desync in #1149. The crash in #2212 occurs independent of enabling CDEF. Further analysis is needed for the decoding error when enabling LRF for 4:2:2.
See Cdef_Uv_Dir in AV1 spec 7.15.1 - CDEF block process.

AWCY results on derf-720p-422 at the default speed show a 9.9% decrease in encoding speed:

PSNR PSNR Cb PSNR Cr PSNR HVS SSIM MS SSIM CIEDE 2000
-2.5572 0.4582 -1.0744 -2.0057 -0.1799 -1.0506 -2.9736

@barrbrain barrbrain changed the title Enable restoration filters for 4:2:2 Enable CDEF filter for 4:2:2 May 11, 2020
src/cdef.rs Show resolved Hide resolved
@barrbrain barrbrain force-pushed the enable-restoration-filters-422 branch from fb08f7e to 54d236b Compare May 11, 2020 12:21
@barrbrain barrbrain requested a review from lu-zero May 11, 2020 12:29
Add missing logic to map CDEF direction for chroma planes.

AWCY results on derf-720p-422 at the default speed show a 9.9% decrease
in encoding speed:

   PSNR | PSNR Cb | PSNR Cr | PSNR HVS |    SSIM | MS SSIM | CIEDE 2000
-2.5572 |  0.4582 | -1.0744 |  -2.0057 | -0.1799 | -1.0506 |    -2.9736
AWCY results on derf-720p-422 at the default speed show a 28% decrease
in encoding speed:

   PSNR | PSNR Cb | PSNR Cr | PSNR HVS |    SSIM | MS SSIM | CIEDE 2000
-1.4957 | -1.2369 | -0.5441 |  -1.0947 | -0.9361 | -0.7724 |    -2.1054
@barrbrain barrbrain force-pushed the enable-restoration-filters-422 branch from 30a4da4 to 127f92b Compare May 11, 2020 15:13
@barrbrain
Copy link
Collaborator Author

Added back enabling of LRF for 4:2:2 now that there is no desync locally or in AWCY.
Added as a separate commit so that we may document the impact.

AWCY results on derf-720p-422 at the default speed show a 28% decrease in encoding speed:

PSNR PSNR Cb PSNR Cr PSNR HVS SSIM MS SSIM CIEDE 2000
-1.4957 -1.2369 -0.5441 -1.0947 -0.9361 -0.7724 -2.1054

@barrbrain
Copy link
Collaborator Author

The total effect of enabling both filters for 4:2:2 is substantial:

PSNR PSNR Cb PSNR Cr PSNR HVS SSIM MS SSIM CIEDE 2000
-4.0057 -0.8299 -1.6084 -3.0774 -1.1085 -1.8073 -5.0115

@barrbrain barrbrain changed the title Enable CDEF filter for 4:2:2 Enable CDEF and restoration filters for 4:2:2 May 11, 2020
Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, the cdef logic might or might not live in a stand alone function since it got a little longer than I'd like.

@barrbrain
Copy link
Collaborator Author

Until #2212 is resolved, I will minimise changes to LRF, CDEF or tiling parts of the code to avoid confusing anyone looking at that bug. I agree that they could be more readable with some work.

@barrbrain barrbrain merged commit 03a11a2 into xiph:master May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
msu2020 MSU Annual Video Codecs Comparison 2020
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chroma 422 mode causes enc/dec mismatch and its test cannot detect this
5 participants