-
Notifications
You must be signed in to change notification settings - Fork 256
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
Enable CDEF and restoration filters for 4:2:2 #2224
Conversation
16bd5b0
to
b54b8d0
Compare
I breathed life into the rav1e fuzzer, and encountered an
I was able to reproduce from the rav1e CLI:
|
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)); |
It sounds possibly related to #2212 |
It is, but I'm still figuring out in detail how much overlap there is.
…On Wed, Apr 8, 2020 at 12:42 PM Luca Barbato ***@***.***> wrote:
It sounds possibly related to #2212
<#2212>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#2224 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACWMHECEAJ22HREKZZPX7T3RLSSQHANCNFSM4MDTIFMQ>
.
|
Also see #970. |
d1d768e
to
a0f6a47
Compare
a0f6a47
to
9c4e982
Compare
9c4e982
to
fb08f7e
Compare
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. AWCY results on derf-720p-422 at the default speed show a 9.9% decrease in encoding speed:
|
fb08f7e
to
54d236b
Compare
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
30a4da4
to
127f92b
Compare
Added back enabling of LRF for 4:2:2 now that there is no desync locally or in AWCY. AWCY results on derf-720p-422 at the default speed show a 28% decrease in encoding speed:
|
The total effect of enabling both filters for 4:2:2 is substantial:
|
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.
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.
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. |
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.