openSUSE Build Service

Overview Repositories Revisions Requests Users Attributes Meta

File mpv_PR6782.patch of Package mpv (Revision a883b32f60ab3ec037d239d29139c5ef)

Currently displaying revision a883b32f60ab3ec037d239d29139c5ef , Show latest

From 2f7ff12f05cc7360b084cdddbba59e482d49cc62 Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Sun, 30 Jun 2019 00:28:46 +0200
Subject: [PATCH 1/2] vo_gpu: glx: move OML sync code to an independent file

So the next commit can make EGL use it. EGL has a quite similar
function, that practically works the same. Although it's relatively
trivial, it's still tricky, and probably shouldn't end up as duplicated
code.

There are no functional changes, except initialization, and how failure
of the glXGetSyncValues call is handled. Also, some comments mention the
EGL extension.

Note that there's no intention for this code to handle anything else
than the very specific OML sync extension (and its EGL equivalent). This
is just too weirdly specific to the weird idiosyncrasies of the
extension, and it makes no sense to extend it to handle anything else.
(Such as Wayland or DXGI presentation feedback.)
---
 video/out/opengl/context_glx.c | 101 ++---------------------------
 video/out/opengl/oml_sync.c    | 112 +++++++++++++++++++++++++++++++++
 video/out/opengl/oml_sync.h    |  28 +++++++++
 wscript_build.py               |   1 +
 4 files changed, 146 insertions(+), 96 deletions(-)
 create mode 100644 video/out/opengl/oml_sync.c
 create mode 100644 video/out/opengl/oml_sync.h

diff --git a/video/out/opengl/context_glx.c b/video/out/opengl/context_glx.c
index 1f3225a9ac..7dc8062c1c 100644
--- a/video/out/opengl/context_glx.c
+++ b/video/out/opengl/context_glx.c
@@ -40,6 +40,7 @@
 #include "osdep/timer.h"
 #include "video/out/x11_common.h"
 #include "context.h"
+#include "oml_sync.h"
 #include "utils.h"
 
 struct priv {
@@ -49,14 +50,7 @@ struct priv {
     GLXFBConfig fbc;
 
     Bool (*XGetSyncValues)(Display*, GLXDrawable, int64_t*, int64_t*, int64_t*);
-    int64_t last_ust;
-    int64_t last_msc;
-    int64_t last_sbc;
-    int64_t last_sbc_mp_time;
-    int64_t user_sbc;
-    int64_t vsync_duration;
-    int64_t last_skipped_vsyncs;
-    int64_t last_queue_display_time;
+    struct oml_sync sync;
 };
 
 static void glx_uninit(struct ra_ctx *ctx)
@@ -232,91 +226,12 @@ static void update_vsync_oml(struct ra_ctx *ctx)
 
     assert(p->XGetSyncValues);
 
-    p->last_skipped_vsyncs = 0;
-    p->user_sbc += 1;
-
-    // This extension returns two unrelated values:
-    //  (ust, msc): clock time and incrementing counter of last vsync (this is
-    //              reported continuously, even if we don't swap)
-    //  sbc:        swap counter of frame that was last displayed (every swap
-    //              increments the user_sbc, and the reported sbc is the sbc
-    //              of the frame that was just displayed)
-    // Invariants:
-    //  - ust and msc change in lockstep (no value can change with the other)
-    //  - msc is incremented; if you query it in a loop, and your thread isn't
-    //    frozen or starved by the scheduler, it will usually not change or
-    //    be incremented by 1 (while the ust will be incremented by vsync
-    //    duration)
-    //  - sbc is never higher than the user_sbc
-    //  - (ust, msc) are equal to or higher by vsync increments than the display
-    //    time of the frame referenced by the sbc
-    // Note that (ust, msc) and sbc are not locked to each other. The following
-    // can easily happen if vsync skips occur:
-    //  - you draw a frame, in the meantime hardware swaps sbc_1
-    //  - another display vsync happens during drawing
-    //  - you call swap()
-    //  - query (ust, msc) and sbc
-    //  - sbc contains sbc_1, but (ust, msc) contains the vsync after it
-    // As a consequence, it's hard to detect the latency or vsync skips.
     int64_t ust, msc, sbc;
     if (!p->XGetSyncValues(ctx->vo->x11->display, ctx->vo->x11->window,
                            &ust, &msc, &sbc))
-    {
-        // Assume the extension is effectively unsupported.
-        p->vsync_duration = -1;
-        p->last_skipped_vsyncs = -1;
-        p->last_queue_display_time = -1;
-        return;
-    }
-
-    int64_t ust_passed = p->last_ust ? ust - p->last_ust : 0;
-    p->last_ust = ust;
-
-    int64_t msc_passed = p->last_msc ? msc - p->last_msc : 0;
-    p->last_msc = msc;
-
-    int64_t sbc_passed = sbc - p->last_sbc;
-    p->last_sbc = sbc;
-
-    // Display frame duration. This makes assumptions about UST (see below).
-    if (msc_passed && ust_passed)
-        p->vsync_duration = ust_passed / msc_passed;
-
-    // Only if a new frame was displayed (sbc increased) we have sort-of a
-    // chance that the current (ust, msc) is for the sbc. But this is racy,
-    // because skipped frames drops could have increased the msc right after the
-    // display event and before we queried the values. This code hopes for the
-    // best and ignores this.
-    if (sbc_passed) {
-        // The extension spec doesn't define what the UST is (not even its unit).
-        // Simply assume UST is a simple CLOCK_MONOTONIC usec value. The swap
-        // buffer call happened "some" but very small time ago, so we can get
-        // away with querying the current time. There is also the implicit
-        // assumption that mpv's timer and the UST use the same clock (which it
-        // does on POSIX).
-        struct timespec ts;
-        if (clock_gettime(CLOCK_MONOTONIC, &ts))
-            return;
-        uint64_t now_monotonic = ts.tv_sec * 1000000LL + ts.tv_nsec / 1000;
-        uint64_t ust_mp_time = mp_time_us() - (now_monotonic - ust);
-
-        // Assume this is exactly when the actual display event for this sbc
-        // happened. This is subject to the race mentioned above.
-        p->last_sbc_mp_time = ust_mp_time;
-    }
+        ust = msc = sbc = -1;
 
-    // At least one frame needs to be actually displayed before
-    // p->last_sbc_mp_time is set.
-    if (!sbc)
-        return;
-
-    // Extrapolate from the last sbc time (when a frame was actually displayed),
-    // and by adding the number of frames that were queued since to it.
-    // For every unit the sbc is smaller than user_sbc, the actual display
-    // is one frame ahead (assumes glx_swap_buffers() is called for every
-    // vsync).
-    p->last_queue_display_time =
-        p->last_sbc_mp_time + (p->user_sbc - sbc) * p->vsync_duration;
+    oml_sync_swap(&p->sync, ust, msc, sbc);
 }
 
 static void glx_swap_buffers(struct ra_ctx *ctx)
@@ -332,9 +247,7 @@ static void glx_swap_buffers(struct ra_ctx *ctx)
 static void glx_get_vsync(struct ra_ctx *ctx, struct vo_vsync_info *info)
 {
     struct priv *p = ctx->priv;
-    info->vsync_duration = p->vsync_duration;
-    info->skipped_vsyncs = p->last_skipped_vsyncs;
-    info->last_queue_display_time = p->last_queue_display_time;
+    oml_sync_get_info(&p->sync, info);
 }
 
 static bool glx_init(struct ra_ctx *ctx)
@@ -426,10 +339,6 @@ static bool glx_init(struct ra_ctx *ctx)
     if (!ra_gl_ctx_init(ctx, gl, params))
         goto uninit;
 
-    p->vsync_duration = -1;
-    p->last_skipped_vsyncs = -1;
-    p->last_queue_display_time = -1;
-
     return true;
 
 uninit:
diff --git a/video/out/opengl/oml_sync.c b/video/out/opengl/oml_sync.c
new file mode 100644
index 0000000000..aaa97a7311
--- /dev/null
+++ b/video/out/opengl/oml_sync.c
@@ -0,0 +1,112 @@
+#include <time.h>
+
+#include "osdep/timer.h"
+#include "oml_sync.h"
+#include "video/out/vo.h"
+
+// General nonsense about the associated extension.
+//
+// This extension returns two unrelated values:
+//  (ust, msc): clock time and incrementing counter of last vsync (this is
+//              reported continuously, even if we don't swap)
+//  sbc:        swap counter of frame that was last displayed (every swap
+//              increments the user_sbc, and the reported sbc is the sbc
+//              of the frame that was just displayed)
+// Invariants:
+//  - ust and msc change in lockstep (no value can change with the other)
+//  - msc is incremented; if you query it in a loop, and your thread isn't
+//    frozen or starved by the scheduler, it will usually not change or
+//    be incremented by 1 (while the ust will be incremented by vsync
+//    duration)
+//  - sbc is never higher than the user_sbc
+//  - (ust, msc) are equal to or higher by vsync increments than the display
+//    time of the frame referenced by the sbc
+// Note that (ust, msc) and sbc are not locked to each other. The following
+// can easily happen if vsync skips occur:
+//  - you draw a frame, in the meantime hardware swaps sbc_1
+//  - another display vsync happens during drawing
+//  - you call swap()
+//  - query (ust, msc) and sbc
+//  - sbc contains sbc_1, but (ust, msc) contains the vsync after it
+// As a consequence, it's hard to detect the latency or vsync skips.
+
+static void oml_sync_reset(struct oml_sync *oml)
+{
+    oml->vsync_duration = -1;
+    oml->last_skipped_vsyncs = -1;
+    oml->last_queue_display_time = -1;
+}
+
+void oml_sync_swap(struct oml_sync *oml, int64_t ust, int64_t msc, int64_t sbc)
+{
+    if (!oml->state_ok)
+        oml_sync_reset(oml);
+
+    oml->last_skipped_vsyncs = 0;
+    oml->user_sbc += 1;
+
+    if (sbc < 0)
+        return;
+
+    oml->state_ok = true;
+
+    int64_t ust_passed = oml->last_ust ? ust - oml->last_ust : 0;
+    oml->last_ust = ust;
+
+    int64_t msc_passed = oml->last_msc ? msc - oml->last_msc : 0;
+    oml->last_msc = msc;
+
+    int64_t sbc_passed = sbc - oml->last_sbc;
+    oml->last_sbc = sbc;
+
+    // Display frame duration. This makes assumptions about UST (see below).
+    if (msc_passed && ust_passed)
+        oml->vsync_duration = ust_passed / msc_passed;
+
+    // Only if a new frame was displayed (sbc increased) we have sort-of a
+    // chance that the current (ust, msc) is for the sbc. But this is racy,
+    // because skipped frames drops could have increased the msc right after the
+    // display event and before we queried the values. This code hopes for the
+    // best and ignores this.
+    if (sbc_passed) {
+        // The GLX extension spec doesn't define what the UST is (not even its
+        // unit). Simply assume UST is a simple CLOCK_MONOTONIC usec value. This
+        // is what Mesa does, and what the Google EGL extension seems to imply
+        // (they mention CLOCK_MONOTONIC, but not the unit).
+        // The swap buffer call happened "some" but very small time ago, so we
+        // can get away with querying the current time. There is also the
+        // implicit assumption that mpv's timer and the UST use the same clock
+        // (which it does on POSIX).
+        struct timespec ts;
+        if (clock_gettime(CLOCK_MONOTONIC, &ts))
+            return;
+        uint64_t now_monotonic = ts.tv_sec * 1000000LL + ts.tv_nsec / 1000;
+        uint64_t ust_mp_time = mp_time_us() - (now_monotonic - ust);
+
+        // Assume this is exactly when the actual display event for this sbc
+        // happened. This is subject to the race mentioned above.
+        oml->last_sbc_mp_time = ust_mp_time;
+    }
+
+    // At least one frame needs to be actually displayed before
+    // oml->last_sbc_mp_time is set.
+    if (!sbc)
+        return;
+
+    // Extrapolate from the last sbc time (when a frame was actually displayed),
+    // and by adding the number of frames that were queued since to it.
+    // For every unit the sbc is smaller than user_sbc, the actual display
+    // is one frame ahead (assumes oml_sync_swap() is called for every
+    // vsync).
+    oml->last_queue_display_time =
+        oml->last_sbc_mp_time + (oml->user_sbc - sbc) * oml->vsync_duration;
+}
+
+void oml_sync_get_info(struct oml_sync *oml, struct vo_vsync_info *info)
+{
+    if (!oml->state_ok)
+        oml_sync_reset(oml);
+    info->vsync_duration = oml->vsync_duration;
+    info->skipped_vsyncs = oml->last_skipped_vsyncs;
+    info->last_queue_display_time = oml->last_queue_display_time;
+}
diff --git a/video/out/opengl/oml_sync.h b/video/out/opengl/oml_sync.h
new file mode 100644
index 0000000000..e27ccb943f
--- /dev/null
+++ b/video/out/opengl/oml_sync.h
@@ -0,0 +1,28 @@
+#pragma once
+
+#include <stdbool.h>
+#include <stdint.h>
+
+// Must be initialized to {0} by user.
+struct oml_sync {
+    bool state_ok;
+    int64_t last_ust;
+    int64_t last_msc;
+    int64_t last_sbc;
+    int64_t last_sbc_mp_time;
+    int64_t user_sbc;
+    int64_t vsync_duration;
+    int64_t last_skipped_vsyncs;
+    int64_t last_queue_display_time;
+};
+
+struct vo_vsync_info;
+
+// This must be called on every SwapBuffer call. Pass the ust/msc/sbc values
+// returned by a successful GetSyncValues call. Pass -1 for all these 3 values
+// if GetSyncValues returned failure (but note that you need to set them to -1
+// manually).
+void oml_sync_swap(struct oml_sync *oml, int64_t ust, int64_t msc, int64_t sbc);
+
+// Can be called any time; returns state determined by last oml_sync_swap() call.
+void oml_sync_get_info(struct oml_sync *oml, struct vo_vsync_info *info);
diff --git a/wscript_build.py b/wscript_build.py
index 0814b9122e..2a5889b8f3 100644
--- a/wscript_build.py
+++ b/wscript_build.py
@@ -479,6 +479,7 @@ def swift(task):
         ( "video/out/opengl/hwdec_vaegl.c",      "vaapi-egl" ),
         ( "video/out/opengl/hwdec_vdpau.c",      "vdpau-gl-x11" ),
         ( "video/out/opengl/libmpv_gl.c",        "gl" ),
+        ( "video/out/opengl/oml_sync.c",         "egl-x11 || gl-x11" ),
         ( "video/out/opengl/ra_gl.c",            "gl" ),
         ( "video/out/opengl/utils.c",            "gl" ),
         ( "video/out/vo.c" ),

From db35115d12d01a9afb524eb530eef5422ce6558c Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Sun, 30 Jun 2019 00:39:23 +0200
Subject: [PATCH 2/2] vo_gpu: x11egl: support Mesa OML sync extension

Mesa supports the EGL_CHROMIUM_sync_control extension, and it's
available out of the box with AMD drivers. In practice, this is exactly
the same as GLX_OML_sync_control, but for EGL. The extension
specification is separate from the GLX one though, and buried somewhere
in the Chromium code.

This appears to work, although I don't know if it really works.

In theory, this could be useful for other EGL targets. Support code for
it could have been added to egl_helpers.c to avoid some minor duplicated
glue code if another EGL target were to provide this extension. I didn't
bother with that. ANGLE on Windows can't support it, because the
extension spec. explicitly requires POSIX timers. ANGLE on Linux/OSX is
actively harmful for mpv and hopefully won't ever use it. Wayland uses
EGL, but has its own fancy presentation feedback stuff (and besides, I
don't think basic video player functionality works on Wayland at all).
context_drm_egl maybe? But I think DRM has its own stuff.
---
 video/out/opengl/context_x11egl.c | 24 ++++++++++++++++++++++++
 wscript_build.py                  |  2 +-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/video/out/opengl/context_x11egl.c b/video/out/opengl/context_x11egl.c
index 32530cc11d..b1aa0690d7 100644
--- a/video/out/opengl/context_x11egl.c
+++ b/video/out/opengl/context_x11egl.c
@@ -30,12 +30,18 @@
 #include "video/out/x11_common.h"
 #include "context.h"
 #include "egl_helpers.h"
+#include "oml_sync.h"
+#include "utils.h"
 
 struct priv {
     GL gl;
     EGLDisplay egl_display;
     EGLContext egl_context;
     EGLSurface egl_surface;
+
+    EGLBoolean (*GetSyncValues)(EGLDisplay, EGLSurface,
+                                int64_t*, int64_t*, int64_t*);
+    struct oml_sync sync;
 };
 
 static void mpegl_uninit(struct ra_ctx *ctx)
@@ -79,6 +85,19 @@ static void mpegl_swap_buffers(struct ra_ctx *ctx)
 {
     struct priv *p = ctx->priv;
     eglSwapBuffers(p->egl_display, p->egl_surface);
+
+    int64_t ust, msc, sbc;
+    if (!p->GetSyncValues || !p->GetSyncValues(p->egl_display, p->egl_surface,
+                                               &ust, &msc, &sbc))
+        ust = msc = sbc = -1;
+
+    oml_sync_swap(&p->sync, ust, msc, sbc);
+}
+
+static void mpegl_get_vsync(struct ra_ctx *ctx, struct vo_vsync_info *info)
+{
+    struct priv *p = ctx->priv;
+    oml_sync_get_info(&p->sync, info);
 }
 
 static bool mpegl_init(struct ra_ctx *ctx)
@@ -142,11 +161,16 @@ static bool mpegl_init(struct ra_ctx *ctx)
 
     struct ra_gl_ctx_params params = {
         .swap_buffers = mpegl_swap_buffers,
+        .get_vsync    = mpegl_get_vsync,
     };
 
     if (!ra_gl_ctx_init(ctx, &p->gl, params))
         goto uninit;
 
+    const char *exts = eglQueryString(eglGetCurrentDisplay(), EGL_EXTENSIONS);
+    if (gl_check_extension(exts, "EGL_CHROMIUM_sync_control"))
+        p->GetSyncValues = (void *)eglGetProcAddress("eglGetSyncValuesCHROMIUM");
+
     ra_add_native_resource(ctx->ra, "x11", vo->x11->display);
 
     return true;

Places

File mpv_PR6782.patch of Package mpv (Revision a883b32f60ab3ec037d239d29139c5ef)

Places