rav1e icon indicating copy to clipboard operation
rav1e copied to clipboard

[WIP] encoder: allow 1 CDEF bits for key frames

Open tmatth opened this issue 4 years ago • 12 comments

This exposes a weird issue, where rdo_loop_decision gets stuck in an infinite loop where it's alternating between two loop restoration filters:

New best_new_lrf Sgrproj 12 0 -20
New best_new_lrf Sgrproj 13 0 -24
New best_new_lrf Sgrproj 12 0 -20
New best_new_lrf Sgrproj 13 0 -24
New best_new_lrf Sgrproj 12 0 -20
New best_new_lrf Sgrproj 13 0 -24
New best_new_lrf Sgrproj 12 0 -20
....

@xiphmont any ideas?

To reproduce (with this branch):

target/release/rav1e /derf-media/objective-1-fast/aspen_1080p_60f.y4m --quantizer 172 -o out.ivf

tmatth avatar Oct 21 '19 15:10 tmatth

Coverage Status

Coverage increased (+0.02%) to 75.593% when pulling 4158287f25e6df3a80745829f0d4d197252a013e on feature/better-cdef-strengths into e4683b1846c08412fe26d4487af358383b54c81a on master.

coveralls avatar Oct 21 '19 16:10 coveralls

The loop in rdo_loop_decision iterates until both cdef or lrf have stopped changing. Are you sure cdef isn't changing as well?

KyleSiefring avatar Oct 21 '19 16:10 KyleSiefring

Limit cycles should be impossible (it won't pick something new unless cost goes down), so the bug is in that decision. I'll look now.

Monty

On Mon, Oct 21, 2019 at 12:57 PM Kyle Siefring [email protected] wrote:

The loop in rdo_loop_decision checks if both cdef or lrf has changed. Are you sure cdef isn't changing as well?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xiph/rav1e/pull/1788?email_source=notifications&email_token=ACWMHEH5EP7KUQWG6PZAVCDQPXNO5A5CNFSM4JC7CVAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB3BBBQ#issuecomment-544608390, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACWMHEAS62YHEKVZGFUHPZLQPXNO5ANCNFSM4JC7CVAA .

xiphmont avatar Oct 21 '19 17:10 xiphmont

The loop in rdo_loop_decision iterates until both cdef or lrf have stopped changing. Are you sure cdef isn't changing as well?

No, cdef_change is always false here, it's just lrf_change that is always true (because it keeps flipping).

tmatth avatar Oct 21 '19 18:10 tmatth

OK, I see what's happening. The logic behind my 'cost always goes down' invariant is incorrect.

xiphmont avatar Oct 22 '19 01:10 xiphmont

r+

The bug being caused in loop rdo is unrelated and I will fix it separately-- this change is simply triggering it.

Yeah that's what I gathered, unfortunately I can't run AWCY for this PR until that bug gets fixed (speaking of which, should I open a proper issue?)

tmatth avatar Oct 22 '19 21:10 tmatth

OK, I see what's happening. The logic behind my 'cost always goes down' invariant is incorrect.

_ Okay, I haven't seen this. Nice catch, @tmatth!

ycho avatar Oct 22 '19 21:10 ycho

PR #1800 fixes your test case.

xiphmont avatar Oct 26 '19 17:10 xiphmont

PR #1800 fixes your test case.

Confirmed!

tmatth avatar Oct 28 '19 15:10 tmatth

@tmatth is this ready to land?

lu-zero avatar Nov 01 '19 09:11 lu-zero

@tmatth is this ready to land?

No still a WIP.

tmatth avatar Nov 01 '19 13:11 tmatth

@tmatth is this ready to land?

No still a WIP.

Why? Still weird?!

ycho avatar Dec 09 '19 22:12 ycho