client API: preparations for allowing render API to use DR etc.

DR (letting the decoder allocate texture memory) requires running the
allocation on the render thread. This is rather hard with the render
API, because the user controls this thread and when it's entered. It was
not possible until now.

This commit adds a bunch of infrastructure to make this possible. We add
a new optional mode (MPV_RENDER_PARAM_ADVANCED_CONTROL) which basically
lets the user's render thread and libmpv agree how this should be done.

Misuse would lead to deadlocks. To make this less likely, strictly
document thread safety/locking issues. In particular, document which
libmpv functions can be called without issues. (The rest has to be
assumed unsafe.)

The worst issue is destruction of the render context while video is
still active. To avoid certain unintended recursive locks (i.e.
deadlocks, unless we'd make the locks recursive), make the update
callback lock separate. Make "killing" the video chain asynchronous, so
we can do extra work while video is being destroyed.

Because losing wakeups is a big deal, setting the update callback now
triggers a wakeup. (It would have been better if the wakeup callback
were a parameter to mpv_render_context_create(), but too late.)

This commit does not add DR yet; the following commit does this.

2 files changed, 113 insertions, 22 deletions

diff --git a/video/out/libmpv.h b/video/out/libmpv.h
index 8b99587984..ae154bbd24 100644
--- a/video/out/libmpv.h
+++ b/video/out/libmpv.h
@@ -12,6 +12,9 @@
 void *get_mpv_render_param(mpv_render_param *params, mpv_render_param_type type,
                            void *def);
 
+#define GET_MPV_RENDER_PARAM(params, type, ctype, def) \
+    (*(ctype *)get_mpv_render_param(params, type, &(ctype){(def)}))
+
 typedef int (*mp_render_cb_control_fn)(void *cb_ctx, int *events,
                                        uint32_t request, void *data);
 void mp_render_context_set_control_callback(mpv_render_context *ctx,
diff --git a/video/out/vo_libmpv.c b/video/out/vo_libmpv.c
index 61f73aee07..75ac514595 100644
--- a/video/out/vo_libmpv.c
+++ b/video/out/vo_libmpv.c
@@ -12,6 +12,7 @@
 #include "mpv_talloc.h"
 #include "common/common.h"
 #include "misc/bstr.h"
+#include "misc/dispatch.h"
 #include "common/msg.h"
 #include "options/m_config.h"
 #include "options/options.h"
@@ -39,13 +40,14 @@
  *   render API user anyway, and the (unlikely) deadlock is avoided with
  *   a timeout
  *
- *  So: mpv core > VO > mpv_render_context > mp_client_api.lock (locking)
+ *  Locking:  mpv core > VO > mpv_render_context.lock > mp_client_api.lock
+ *              > mpv_render_context.update_lock
  *  And: render thread > VO (wait for present)
  *       VO > render thread (wait for present done, via timeout)
  */
 
 struct vo_priv {
-    struct mpv_render_context *ctx;
+    struct mpv_render_context *ctx; // immutable after init
 };
 
 struct mpv_render_context {
@@ -55,16 +57,27 @@ struct mpv_render_context {
 
     atomic_bool in_use;
 
+    // --- Immutable after init
+    bool advanced_control;
+    struct mp_dispatch_queue *dispatch; // NULL if advanced_control disabled
+
     pthread_mutex_t control_lock;
+    // --- Protected by control_lock
     mp_render_cb_control_fn control_cb;
     void *control_cb_ctx;
 
-    pthread_mutex_t lock;
-    pthread_cond_t wakeup;
+    pthread_mutex_t update_lock;
+    pthread_cond_t update_cond;     // paired with update_lock
 
-    // --- Protected by lock
+    // --- Protected by update_lock
     mpv_render_update_fn update_cb;
     void *update_cb_ctx;
+    bool had_kill_update;           // update during termination
+
+    pthread_mutex_t lock;
+    pthread_cond_t video_wait;      // paired with lock
+
+    // --- Protected by lock
     struct vo_frame *next_frame;    // next frame to draw
     int64_t present_count;          // incremented when next frame can be shown
     int64_t expected_flip_count;    // next vsync event for next_frame
@@ -110,20 +123,29 @@ void *get_mpv_render_param(mpv_render_param *params, mpv_render_param_type type,
 
 static void forget_frames(struct mpv_render_context *ctx, bool all)
 {
-    pthread_cond_broadcast(&ctx->wakeup);
+    pthread_cond_broadcast(&ctx->video_wait);
     if (all) {
         talloc_free(ctx->cur_frame);
         ctx->cur_frame = NULL;
     }
 }
 
+static void dispatch_wakeup(void *ptr)
+{
+    struct mpv_render_context *ctx = ptr;
+
+    update(ctx);
+}
+
 int mpv_render_context_create(mpv_render_context **res, mpv_handle *mpv,
                               mpv_render_param *params)
 {
     mpv_render_context *ctx = talloc_zero(NULL, mpv_render_context);
     pthread_mutex_init(&ctx->control_lock, NULL);
     pthread_mutex_init(&ctx->lock, NULL);
-    pthread_cond_init(&ctx->wakeup, NULL);
+    pthread_mutex_init(&ctx->update_lock, NULL);
+    pthread_cond_init(&ctx->update_cond, NULL);
+    pthread_cond_init(&ctx->video_wait, NULL);
 
     ctx->global = mp_client_get_global(mpv);
     ctx->client_api = ctx->global->client_api;
@@ -132,6 +154,12 @@ int mpv_render_context_create(mpv_render_context **res, mpv_handle *mpv,
     ctx->vo_opts_cache = m_config_cache_alloc(ctx, ctx->global, &vo_sub_opts);
     ctx->vo_opts = ctx->vo_opts_cache->opts;
 
+    if (GET_MPV_RENDER_PARAM(params, MPV_RENDER_PARAM_ADVANCED_CONTROL, int, 0)) {
+        ctx->advanced_control = true;
+        ctx->dispatch = mp_dispatch_create(ctx);
+        mp_dispatch_set_wakeup_fn(ctx->dispatch, dispatch_wakeup, ctx);
+    }
+
     int err = MPV_ERROR_NOT_IMPLEMENTED;
     for (int n = 0; render_backends[n]; n++) {
         ctx->renderer = talloc_zero(NULL, struct render_backend);
@@ -176,10 +204,12 @@ void mpv_render_context_set_update_callback(mpv_render_context *ctx,
                                             mpv_render_update_fn callback,
                                             void *callback_ctx)
 {
-    pthread_mutex_lock(&ctx->lock);
+    pthread_mutex_lock(&ctx->update_lock);
     ctx->update_cb = callback;
     ctx->update_cb_ctx = callback_ctx;
-    pthread_mutex_unlock(&ctx->lock);
+    if (ctx->update_cb)
+        ctx->update_cb(ctx->update_cb_ctx);
+    pthread_mutex_unlock(&ctx->update_lock);
 }
 
 void mp_render_context_set_control_callback(mpv_render_context *ctx,
@@ -192,6 +222,16 @@ void mp_render_context_set_control_callback(mpv_render_context *ctx,
     pthread_mutex_unlock(&ctx->control_lock);
 }
 
+static void kill_cb(void *ptr)
+{
+    struct mpv_render_context *ctx = ptr;
+
+    pthread_mutex_lock(&ctx->update_lock);
+    ctx->had_kill_update = true;
+    pthread_cond_broadcast(&ctx->update_cond);
+    pthread_mutex_unlock(&ctx->update_lock);
+}
+
 void mpv_render_context_free(mpv_render_context *ctx)
 {
     if (!ctx)
@@ -205,19 +245,43 @@ void mpv_render_context_free(mpv_render_context *ctx)
     // also bring down the decoder etc., which still might be using the hwdec
     // context. The above removal guarantees it can't come back (so ctx->vo
     // can't change to non-NULL).
-    if (atomic_load(&ctx->in_use))
-        kill_video(ctx->client_api);
+    if (atomic_load(&ctx->in_use)) {
+        kill_video_async(ctx->client_api, kill_cb, ctx);
+
+        while (atomic_load(&ctx->in_use)) {
+            // As long as the video decoders are not destroyed, they can still
+            // try to allocate new DR images and so on. This is a grotesque
+            // corner case, but possible. Also, more likely, DR images need to
+            // be released while the video chain is destroyed.
+            if (ctx->dispatch)
+                mp_dispatch_queue_process(ctx->dispatch, 0);
+
+            // Wait for kill_cb() or update() calls.
+            pthread_mutex_lock(&ctx->update_lock);
+            if (!ctx->had_kill_update)
+                pthread_cond_wait(&ctx->update_cond, &ctx->update_lock);
+            ctx->had_kill_update = false;
+            pthread_mutex_unlock(&ctx->update_lock);
+        }
+    }
 
     assert(!atomic_load(&ctx->in_use));
     assert(!ctx->vo);
 
+    // Possibly remaining outstanding work.
+    if (ctx->dispatch)
+        mp_dispatch_queue_process(ctx->dispatch, 0);
+
     forget_frames(ctx, true);
 
     ctx->renderer->fns->destroy(ctx->renderer);
     talloc_free(ctx->renderer->priv);
     talloc_free(ctx->renderer);
+    talloc_free(ctx->dispatch);
 
-    pthread_cond_destroy(&ctx->wakeup);
+    pthread_cond_destroy(&ctx->update_cond);
+    pthread_cond_destroy(&ctx->video_wait);
+    pthread_mutex_destroy(&ctx->update_lock);
     pthread_mutex_destroy(&ctx->lock);
     pthread_mutex_destroy(&ctx->control_lock);
 
@@ -282,7 +346,7 @@ int mpv_render_context_render(mpv_render_context *ctx, mpv_render_param *params)
         ctx->next_frame = NULL;
         if (!(frame->redraw || !frame->current))
             wait_present_count += 1;
-        pthread_cond_signal(&ctx->wakeup);
+        pthread_cond_broadcast(&ctx->video_wait);
         talloc_free(ctx->cur_frame);
         ctx->cur_frame = vo_frame_ref(frame);
     } else {
@@ -306,7 +370,7 @@ int mpv_render_context_render(mpv_render_context *ctx, mpv_render_param *params)
 
     pthread_mutex_lock(&ctx->lock);
     while (wait_present_count > ctx->present_count)
-        pthread_cond_wait(&ctx->wakeup, &ctx->lock);
+        pthread_cond_wait(&ctx->video_wait, &ctx->lock);
     pthread_mutex_unlock(&ctx->lock);
 
     return err;
@@ -318,8 +382,22 @@ void mpv_render_context_report_swap(mpv_render_context *ctx)
 
     pthread_mutex_lock(&ctx->lock);
     ctx->flip_count += 1;
-    pthread_cond_signal(&ctx->wakeup);
+    pthread_cond_broadcast(&ctx->video_wait);
+    pthread_mutex_unlock(&ctx->lock);
+}
+
+uint64_t mpv_render_context_update(mpv_render_context *ctx)
+{
+    uint64_t res = 0;
+
+    if (ctx->dispatch)
+        mp_dispatch_queue_process(ctx->dispatch, 0);
+
+    pthread_mutex_lock(&ctx->lock);
+    if (ctx->next_frame)
+        res |= MPV_RENDER_UPDATE_FRAME;
     pthread_mutex_unlock(&ctx->lock);
+    return res;
 }
 
 int mpv_render_context_set_parameter(mpv_render_context *ctx,
@@ -336,11 +414,16 @@ int mpv_render_context_set_parameter(mpv_render_context *ctx,
     return err;
 }
 
-// Called locked.
 static void update(struct mpv_render_context *ctx)
 {
+    pthread_mutex_lock(&ctx->update_lock);
     if (ctx->update_cb)
         ctx->update_cb(ctx->update_cb_ctx);
+
+    // For the termination code.
+    ctx->had_kill_update = true;
+    pthread_cond_broadcast(&ctx->update_cond);
+    pthread_mutex_unlock(&ctx->update_lock);
 }
 
 static void draw_frame(struct vo *vo, struct vo_frame *frame)
@@ -352,8 +435,9 @@ static void draw_frame(struct vo *vo, struct vo_frame *frame)
     p->ctx->next_frame = vo_frame_ref(frame);
     p->ctx->expected_flip_count = p->ctx->flip_count + 1;
     p->ctx->redrawing = frame->redraw || !frame->current;
-    update(p->ctx);
     pthread_mutex_unlock(&p->ctx->lock);
+
+    update(p->ctx);
 }
 
 static void flip_page(struct vo *vo)
@@ -365,7 +449,7 @@ static void flip_page(struct vo *vo)
 
     // Wait until frame was rendered
     while (p->ctx->next_frame) {
-        if (pthread_cond_timedwait(&p->ctx->wakeup, &p->ctx->lock, &ts)) {
+        if (pthread_cond_timedwait(&p->ctx->video_wait, &p->ctx->lock, &ts)) {
             if (p->ctx->next_frame) {
                 MP_VERBOSE(vo, "mpv_render_context_render() not being called "
                            "or stuck.\n");
@@ -376,7 +460,7 @@ static void flip_page(struct vo *vo)
 
     // Unblock mpv_render_context_render().
     p->ctx->present_count += 1;
-    pthread_cond_signal(&p->ctx->wakeup);
+    pthread_cond_broadcast(&p->ctx->video_wait);
 
     if (p->ctx->redrawing)
         goto done; // do not block for redrawing
@@ -387,7 +471,7 @@ static void flip_page(struct vo *vo)
         // Assume the user calls it consistently _if_ it's called at all.
         if (!p->ctx->flip_count)
             break;
-        if (pthread_cond_timedwait(&p->ctx->wakeup, &p->ctx->lock, &ts)) {
+        if (pthread_cond_timedwait(&p->ctx->video_wait, &p->ctx->lock, &ts)) {
             MP_VERBOSE(vo, "mpv_render_report_swap() not being called.\n");
             goto done;
         }
@@ -401,7 +485,7 @@ done:
         p->ctx->cur_frame = p->ctx->next_frame;
         p->ctx->next_frame = NULL;
         p->ctx->present_count += 2;
-        pthread_cond_signal(&p->ctx->wakeup);
+        pthread_cond_signal(&p->ctx->video_wait);
         vo_increment_drop_count(vo, 1);
     }
 
@@ -474,6 +558,7 @@ static int reconfig(struct vo *vo, struct mp_image_params *params)
     p->ctx->need_reconfig = true;
     p->ctx->need_resize = true;
     pthread_mutex_unlock(&p->ctx->lock);
+
     control(vo, VOCTRL_RECONFIG, NULL);
 
     return 0;
@@ -484,7 +569,9 @@ static void uninit(struct vo *vo)
     struct vo_priv *p = vo->priv;
 
     control(vo, VOCTRL_UNINIT, NULL);
+
     pthread_mutex_lock(&p->ctx->lock);
+
     forget_frames(p->ctx, true);
     p->ctx->img_params = (struct mp_image_params){0};
     p->ctx->need_reconfig = true;
@@ -492,11 +579,12 @@ static void uninit(struct vo *vo)
     p->ctx->need_update_external = true;
     p->ctx->need_reset = true;
     p->ctx->vo = NULL;
-    update(p->ctx);
     pthread_mutex_unlock(&p->ctx->lock);
 
     bool state = atomic_exchange(&p->ctx->in_use, false);
     assert(state); // obviously must have been set
+
+    update(p->ctx);
 }
 
 static int preinit(struct vo *vo)