Skip to content

Commit

Permalink
x11: remove PresentNotifyMSC from egl/glx/vulkan to fix xpresent timing
Browse files Browse the repository at this point in the history
PresentNotifyMSC turns out to be not only redundant, but also harmful with
mesa-backed egl/glx/vulkan VOs because for all of them, mesa uses
PresentPixmap behind the scenes when DRI3 is available, which already
spawns a PresentCompleteNotify event when the buffer swap actually
finishes. This is important because without using the timing information
from these PresentCompleteKindPixmap events, there's no way for mpv to know
exactly when a frame becomes visible on the display.

By using PresentNotifyMSC in conjunction with DRI3-enabled mesa, two
problems are created:
1. mpv assumes that a vblank won't elapse (i.e., it assumes the current MSC
   won't change) between the time when mesa enqueues the buffer swap and
   the time when mpv calls PresentNotifyMSC to ask xorg for a notification
   at the next MSC, relative to the current MSC at the time that xorg reads
   it for the PresentNotifyMSC call. This means that mpv could get a
   notification one or more vblanks later than it expects, since the
   intention here is for mpv to get a notification at the MSC that the
   buffer swap completes.
2. mpv assumes that a buffer swap always takes one vblank to complete,
   which isn't always true. A buffer swap (i.e., a page flip) could take
   longer than that depending on hardware conditions (if the GPU is running
   slowly or needs to exit a low-power state), scheduling delays (under
   heavy system or GPU load), or unfortunate timing (if the raster scan
   line happens to be at one of the last few rows of pixels and a vblank
   elapses just before the buffer swap is enqueued).

This causes mpv to have a faulty assumption of when frames become visible.

Since mpv already receives the PresentCompleteNotify events generated by
mesa's buffer swaps under the hood, the PresentNotifyMSC usage is unneeded
and just throws a wrench in mpv's vsync timing when xpresent is enabled.

Simply removing the PresentNotifyMSC usage from the egl, glx, and vulkan
VOs fixes the xpresent vsync timing.
  • Loading branch information
kerneltoast authored and sfan5 committed Jan 28, 2023
1 parent 8e2dc8b commit db52498
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 9 deletions.
4 changes: 1 addition & 3 deletions video/out/opengl/context_glx.c
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,8 @@ static bool glx_check_visible(struct ra_ctx *ctx)
static void glx_swap_buffers(struct ra_ctx *ctx)
{
glXSwapBuffers(ctx->vo->x11->display, ctx->vo->x11->window);
if (ctx->vo->x11->use_present) {
vo_x11_present(ctx->vo);
if (ctx->vo->x11->use_present)
present_sync_swap(ctx->vo->x11->present);
}
}

static void glx_get_vsync(struct ra_ctx *ctx, struct vo_vsync_info *info)
Expand Down
4 changes: 1 addition & 3 deletions video/out/opengl/context_x11egl.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,8 @@ static void mpegl_swap_buffers(struct ra_ctx *ctx)
struct priv *p = ctx->priv;

eglSwapBuffers(p->egl_display, p->egl_surface);
if (ctx->vo->x11->use_present) {
vo_x11_present(ctx->vo);
if (ctx->vo->x11->use_present)
present_sync_swap(ctx->vo->x11->present);
}
}

static void mpegl_get_vsync(struct ra_ctx *ctx, struct vo_vsync_info *info)
Expand Down
4 changes: 1 addition & 3 deletions video/out/vulkan/context_xlib.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,8 @@ static bool xlib_check_visible(struct ra_ctx *ctx)

static void xlib_vk_swap_buffers(struct ra_ctx *ctx)
{
if (ctx->vo->x11->use_present) {
vo_x11_present(ctx->vo);
if (ctx->vo->x11->use_present)
present_sync_swap(ctx->vo->x11->present);
}
}

static void xlib_vk_get_vsync(struct ra_ctx *ctx, struct vo_vsync_info *info)
Expand Down

0 comments on commit db52498

Please sign in to comment.