Misc display sync fixes #15259
Misc display sync fixes #15259
Conversation
|
Download the artifacts for this pull request: |
video/out/vo.c
Outdated
| @@ -489,11 +489,14 @@ static void update_vsync_timing_after_swap(struct vo *vo, | |||
| } | |||
|
|
|||
| in->num_successive_vsyncs++; | |||
| if (in->num_successive_vsyncs <= DELAY_VSYNC_SAMPLES) | |||
| if (in->num_successive_vsyncs <= DELAY_VSYNC_SAMPLES) { | |||
| in->base_vsync += in->vsync_interval; | |||
There was a problem hiding this comment.
I don't think it is correct to increment base_vsync here. It is updated in update_vsync_timing_after_swap and this function makes sure valid value is used. base_vsync is last valid point, incrementing with with interval will accumulate a lot of drift, making this value unreliable.
There was a problem hiding this comment.
I guess it makes more sense to set it to the actual vsync instead of incrementing by nominal vsync interval. Updated the patch
llyyr
force-pushed
the
display-sync-shitmess
branch
from
2c6e957
to
0e15601
Compare
into get_display_synced_frame_end. Used in next commit.
llyyr
force-pushed
the
display-sync-shitmess
branch
from
0e15601
to
b2503ef
Compare
This is still used for caclulating a/v sync and delay remaining even for initial samples, so we should always update it to the actual vsync for those initial samples so we have something to work with at least. And if we receive bogus values, also reset it to 0 along with prev_vsync. Not having base_vsync set to _some_ value completely breaks vsync_skip_detection, and mpv stays stuck in a permanent mistimed state where every frame is marked as delayed and never recovers from it.
llyyr
force-pushed
the
display-sync-shitmess
branch
from
b2503ef
to
4c0afdb
Compare
|
I'm little bit uneasy about those changes, it may break vsync skip detection and thing alongside. This vo sync code has a lot of tiny variables, that need to tick in certain way. But I don't want to trace this whole sync code again, so if you say it is ok, fine.
Could you explain this though? Specifically how do we know it doesn't affect good driver and real vsync timings? |
Happens with the current wlroots fifo implementation that's still very wip, and doesn't on mutter's. The problem specifically is that the first few vsync timings can be very jittery on the wlroots MR before it eventually becomes consistent and smooths out. To be exact, Increasing
Please test it locally first for a few days |
First two commits are entirely theoretical, it's more correct but it probably doesn't matter much because this code is only used to detect underruns and to wait for a frame to finish before applying new image_params.
Third commit fixes a real world problem with bad drivers that I encountered while testing various wip fifo-v1/commit-timing implementations. By not always updating base_vsync, we could potentially end up in a permanent underflow state and never recover from it even once the driver has stopped giving us bogus values.