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

[tmatth]

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

[coveralls]

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

[KyleSiefring]

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

[xiphmont]

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 ***@***.***> 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 <#1788?email_source=notifications&email_token=ACWMHEH5EP7KUQWG6PZAVCDQPXNO5A5CNFSM4JC7CVAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB3BBBQ#issuecomment-544608390>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACWMHEAS62YHEKVZGFUHPZLQPXNO5ANCNFSM4JC7CVAA> .

[tmatth]

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).

[xiphmont]

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

[xiphmont]

r+

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

[tmatth]

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?)

[ycho]

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]

So, don't need more bits even for smaller QPs?

[tmatth]

@ycho as this is a work in progress, I'm planning on refining further once I can get some AWCY runs working (see above), this is just to get a baseline idea first.

[xiphmont]

PR #1800 fixes your test case.

[tmatth]

PR #1800 fixes your test case.

Confirmed!

[lu-zero]

@tmatth is this ready to land?

[tmatth]

@tmatth is this ready to land?

No still a WIP.

[ycho]

@tmatth is this ready to land?

No still a WIP.

Why? Still weird?!