vdpau: make mp_vdpau_ctx thread-safe

Preparation so that various things related to video can run in different
threads. One part to this is making the video surface pool safe.

Another issue is the preemption mechanism, which continues to give us
endless pain. In theory, it's probably impossible to handle preemption
100% correctly and race-condition free, unless _every_ API user in the
same process uses a central, shared mutex to protect every vdpau API
call. Otherwise, it could happen that one thread recovering from
preemption allocates a vdpau object, and then another thread (which
hasn't recovered yet) happens to free the object for some reason. This
is because objects are referenced by integer IDs, and vdpau will reuse
IDs invalidated by preemption after preemption.

Since this is unreasonable, we're as lazy as possible when it comes to
handling preemption. We don't do any locking around the mp_vdpau_ctx
fields that are normally immutable, and only can change when recovering
from preemption. In practice, this will work, because it doesn't matter
whether not-yet-recovered components use the old or new vdpau function
pointers or device ID. Code calls mp_vdpau_handle_preemption() anyway to
check for the preemption event and possibly to recover, and that
function acquires the lock protecting the preemption state.

Another possible source of potential grandiose fuckup is the fact that
the vdpau library is in fact only a tiny wrapper, and the real driver
lives in a shared object dlopen()ed by the wrapper. The wrapper also
calls dlclose() on the loaded shared object in some situations. One
possible danger is that failing to recreate a vdpau device could trigger
a dlclose() call, and that glibc might unload it. Currently, glibc
implements full unloading of shared objects on the last dlclose() call,
and if that happens, calls to function pointers pointing into the shared
object would obviously crash. Fortunately, it seems the existing vdpau
wrapper won't trigger this case and never unloads the driver once it's
successfully loaded.

To make it short, vdpau preemption opens up endless depths of WTFs.

Another issue is that any participating thread might do the preemption
recovery (whichever comes first). This is easier to implement. The
implication is that we need threadsafe xlib. We just hope and pray that
this will actually work. This also means that once vdpau code is
actually involved in a multithreaded scenario, we have to add
XInitThreads() to the X11 code.

2 files changed, 67 insertions, 21 deletions

diff --git a/video/vdpau.c b/video/vdpau.c
index b9109c36a8..e59befebdc 100644
--- a/video/vdpau.c
+++ b/video/vdpau.c
@@ -19,6 +19,7 @@
 
 #include "vdpau.h"
 
+#include "osdep/threads.h"
 #include "osdep/timer.h"
 
 #include "video/out/x11_common.h"
@@ -35,7 +36,10 @@ static void mark_vdpau_objects_uninitialized(struct mp_vdpau_ctx *ctx)
 static void preemption_callback(VdpDevice device, void *context)
 {
     struct mp_vdpau_ctx *ctx = context;
+
+    pthread_mutex_lock(&ctx->preempt_lock);
     ctx->is_preempted = true;
+    pthread_mutex_unlock(&ctx->preempt_lock);
 }
 
 static int win_x11_init_vdpau_procs(struct mp_vdpau_ctx *ctx)
@@ -130,32 +134,52 @@ static int handle_preemption(struct mp_vdpau_ctx *ctx)
 //   1: everything is fine, no preemption happened
 int mp_vdpau_handle_preemption(struct mp_vdpau_ctx *ctx, uint64_t *counter)
 {
+    int r = 1;
+    pthread_mutex_lock(&ctx->preempt_lock);
+
     // First time init
     if (!*counter)
         *counter = ctx->preemption_counter;
 
     if (handle_preemption(ctx) < 0)
-        return -1;
+        r = -1;
 
-    if (*counter < ctx->preemption_counter) {
+    if (r > 0 && *counter < ctx->preemption_counter) {
         *counter = ctx->preemption_counter;
-        return 0; // signal recovery after preemption
+        r = 0; // signal recovery after preemption
     }
-    return 1;
+
+    pthread_mutex_unlock(&ctx->preempt_lock);
+    return r;
 }
 
+struct surface_ref {
+    struct mp_vdpau_ctx *ctx;
+    int index;
+};
+
 static void release_decoder_surface(void *ptr)
 {
-    bool *in_use_ptr = ptr;
-    *in_use_ptr = false;
+    struct surface_ref *r = ptr;
+    struct mp_vdpau_ctx *ctx = r->ctx;
+
+    pthread_mutex_lock(&ctx->pool_lock);
+    assert(ctx->video_surfaces[r->index].in_use);
+    ctx->video_surfaces[r->index].in_use = false;
+    pthread_mutex_unlock(&ctx->pool_lock);
+
+    talloc_free(r);
 }
 
-static struct mp_image *create_ref(struct surface_entry *e)
+static struct mp_image *create_ref(struct mp_vdpau_ctx *ctx, int index)
 {
+    struct surface_entry *e = &ctx->video_surfaces[index];
     assert(!e->in_use);
     e->in_use = true;
+    struct surface_ref *ref = talloc_ptrtype(NULL, ref);
+    *ref = (struct surface_ref){ctx, index};
     struct mp_image *res =
-        mp_image_new_custom_ref(&(struct mp_image){0}, &e->in_use,
+        mp_image_new_custom_ref(&(struct mp_image){0}, ref,
                                 release_decoder_surface);
     mp_image_setfmt(res, IMGFMT_VDPAU);
     mp_image_set_size(res, e->w, e->h);
@@ -168,8 +192,11 @@ struct mp_image *mp_vdpau_get_video_surface(struct mp_vdpau_ctx *ctx,
                                             VdpChromaType chroma, int w, int h)
 {
     struct vdp_functions *vdp = &ctx->vdp;
+    int surface_index = -1;
     VdpStatus vdp_st;
 
+    pthread_mutex_lock(&ctx->pool_lock);
+
     // Destroy all unused surfaces that don't have matching parameters
     for (int n = 0; n < MAX_VIDEO_SURFACES; n++) {
         struct surface_entry *e = &ctx->video_surfaces[n];
@@ -188,7 +215,8 @@ struct mp_image *mp_vdpau_get_video_surface(struct mp_vdpau_ctx *ctx,
         if (!e->in_use && e->surface != VDP_INVALID_HANDLE) {
             assert(e->w == w && e->h == h);
             assert(e->chroma == chroma);
-            return create_ref(e);
+            surface_index = n;
+            goto done;
         }
     }
 
@@ -203,12 +231,21 @@ struct mp_image *mp_vdpau_get_video_surface(struct mp_vdpau_ctx *ctx,
             vdp_st = vdp->video_surface_create(ctx->vdp_device, chroma,
                                                w, h, &e->surface);
             CHECK_VDP_WARNING(ctx, "Error when calling vdp_video_surface_create");
-            return create_ref(e);
+            surface_index = n;
+            goto done;
         }
     }
 
-    MP_ERR(ctx, "no surfaces available in mp_vdpau_get_video_surface\n");
-    return NULL;
+done: ;
+    struct mp_image *mpi = NULL;
+    if (surface_index >= 0)
+        mpi = create_ref(ctx, surface_index);
+
+    pthread_mutex_unlock(&ctx->pool_lock);
+
+    if (!mpi)
+        MP_ERR(ctx, "no surfaces available in mp_vdpau_get_video_surface\n");
+    return mpi;
 }
 
 struct mp_vdpau_ctx *mp_vdpau_create_device_x11(struct mp_log *log,
@@ -220,13 +257,13 @@ struct mp_vdpau_ctx *mp_vdpau_create_device_x11(struct mp_log *log,
         .x11 = x11,
         .preemption_counter = 1,
     };
+    mpthread_mutex_init_recursive(&ctx->preempt_lock);
+    pthread_mutex_init(&ctx->pool_lock, NULL);
 
     mark_vdpau_objects_uninitialized(ctx);
 
     if (win_x11_init_vdpau_procs(ctx) < 0) {
-        if (ctx->vdp.device_destroy)
-            ctx->vdp.device_destroy(ctx->vdp_device);
-        talloc_free(ctx);
+        mp_vdpau_destroy(ctx);
         return NULL;
     }
     return ctx;
@@ -246,11 +283,13 @@ void mp_vdpau_destroy(struct mp_vdpau_ctx *ctx)
         }
     }
 
-    if (ctx->vdp_device != VDP_INVALID_HANDLE) {
+    if (vdp->device_destroy && ctx->vdp_device != VDP_INVALID_HANDLE) {
         vdp_st = vdp->device_destroy(ctx->vdp_device);
         CHECK_VDP_WARNING(ctx, "Error when calling vdp_device_destroy");
     }
 
+    pthread_mutex_destroy(&ctx->pool_lock);
+    pthread_mutex_destroy(&ctx->preempt_lock);
     talloc_free(ctx);
 }
 
diff --git a/video/vdpau.h b/video/vdpau.h
index 38e679db25..acf4280266 100644
--- a/video/vdpau.h
+++ b/video/vdpau.h
@@ -4,6 +4,8 @@
 #include <stdbool.h>
 #include <inttypes.h>
 
+#include <pthread.h>
+
 #include <vdpau/vdpau.h>
 #include <vdpau/vdpau_x11.h>
 
@@ -35,26 +37,31 @@ struct vdp_functions {
 // Shared state. Objects created from different VdpDevices are often (always?)
 // incompatible to each other, so all code must use a shared VdpDevice.
 struct mp_vdpau_ctx {
+    struct mp_log *log;
+    struct vo_x11_state *x11;
+
+    // These are mostly immutable, except on preemption. We don't really care
+    // to synchronize the preemption case fully correctly, because it's an
+    // extremely obscure corner case, and basically a vdpau API design bug.
+    // What we do will sort-of work anyway (no memory errors are possible).
     struct vdp_functions vdp;
     VdpGetProcAddress *get_proc_address;
     VdpDevice vdp_device;
+
+    pthread_mutex_t preempt_lock;
     bool is_preempted;                  // set to true during unavailability
     uint64_t preemption_counter;        // incremented after _restoring_
-
     bool preemption_user_notified;
     double last_preemption_retry_fail;
 
-    struct vo_x11_state *x11;
-
     // Surface pool
+    pthread_mutex_t pool_lock;
     struct surface_entry {
         VdpVideoSurface surface;
         int w, h;
         VdpChromaType chroma;
         bool in_use;
     } video_surfaces[MAX_VIDEO_SURFACES];
-
-    struct mp_log *log;
 };
 
 struct mp_vdpau_ctx *mp_vdpau_create_device_x11(struct mp_log *log,