vo_opengl_cb: make operation more similar to normal VOs

vo_opengl_cb is a special case, because we somehow have to render video
asynchronously, all while "trusting" the API user to do it correctly.
This didn't quite work, and a while ago a compromise using a timeout to
prevent theoretically possible deadlocks was added.

Make it even more synchronous. Basically, go all the way, and
synchronize rendering between VO and user renderer thread to the
full extent possible.

This means the silly frame queue is dropped, and we event attempt to
synchronize the GL SwapBuffer call (via mpv_opengl_cb_report_flip()).

The changes introduced with commit dc33eb56 are effectively dropped. I
don't even remember if they mattered.

In the future, we might make all VOs fetch asynchronously from a frame
queue, which would mostly remove the differences between vo_opengl and
vo_opengl_cb, but this will take a while (if it will even be done).

4 files changed, 61 insertions, 140 deletions

diff --git a/DOCS/interface-changes.rst b/DOCS/interface-changes.rst
index fa041fc534..b81312f8e9 100644
--- a/DOCS/interface-changes.rst
+++ b/DOCS/interface-changes.rst
@@ -19,6 +19,8 @@ Interface changes
 
 ::
 
+ --- mpv 0.13.0 ---
+    - remove VO opengl-cb frame queue suboptions (no replacement)
  --- mpv 0.12.0 ---
     - remove --use-text-osd (useless; fontconfig isn't a requirement anymore,
       and text rendering is also lazily initialized)
diff --git a/DOCS/man/vo.rst b/DOCS/man/vo.rst
index 6d90046ea2..f59b9cbea7 100644
--- a/DOCS/man/vo.rst
+++ b/DOCS/man/vo.rst
@@ -1089,27 +1089,6 @@ Available video output drivers are:
 ``opengl-cb``
     For use with libmpv direct OpenGL embedding; useless in any other contexts.
     (See ``<mpv/opengl_cb.h>``.)
-    Usually, ``opengl-cb`` renders frames asynchronously by client and this
-    can cause some frame drops. In order to provide a way to handle this
-    situation, ``opengl-cb`` has its own frame queue and calls update callback
-    more frequently if the queue is not empty regardless of existence of new frame.
-    Once the queue is filled, ``opengl-cb`` drops frames automatically.
-
-    With default options, ``opengl-cb`` renders only the latest frame and drops
-    all frames handed over while waiting render function after update callback.
-
-    ``frame-queue-size=<1..100>``
-        The maximum count of frames which the frame queue can hold (default: 1)
-
-    ``frame-drop-mode=<pop|clear|block>``
-        Select the behavior when the frame queue is full.
-
-        pop
-            Drop the oldest frame in the frame queue.
-        clear
-            Drop all frames in the frame queue.
-        block
-            Wait for a short time, behave like ``clear`` on timeout. (default)
 
     This also supports many of the suboptions the ``opengl`` VO has. Run
     ``mpv --vo=opengl-cb:help`` for a list.
diff --git a/libmpv/opengl_cb.h b/libmpv/opengl_cb.h
index 112f254a59..d117213c53 100644
--- a/libmpv/opengl_cb.h
+++ b/libmpv/opengl_cb.h
@@ -242,7 +242,7 @@ int mpv_opengl_cb_init_gl(mpv_opengl_cb_context *ctx, const char *exts,
  * @param h Height of the framebuffer. Same as with the w parameter, except
  *          that this parameter can be negative. In this case, the video
  *          frame will be rendered flipped.
- * @return the number of left frames in the internal queue to be rendered
+ * @return 0
  */
 int mpv_opengl_cb_draw(mpv_opengl_cb_context *ctx, int fbo, int w, int h);
 
@@ -263,10 +263,14 @@ int mpv_opengl_cb_render(mpv_opengl_cb_context *ctx, int fbo, int vp[4]);
  * Tell the renderer that a frame was flipped at the given time. This is
  * optional, but can help the player to achieve better timing.
  *
+ * Note that calling this at least once informs libmpv that you will use this
+ * function. If you use it inconsistently, expect bad video playback.
+ *
  * If this is called while no video or no OpenGL is initialized, it is ignored.
  *
  * @param time The mpv time (using mpv_get_time_us()) at which the flip call
  *             returned. If 0 is passed, mpv_get_time_us() is used instead.
+ *             Currently, this parameter is ignored.
  * @return error code
  */
 int mpv_opengl_cb_report_flip(mpv_opengl_cb_context *ctx, int64_t time);
diff --git a/video/out/vo_opengl_cb.c b/video/out/vo_opengl_cb.c
index 29b22939cf..15faa36c88 100644
--- a/video/out/vo_opengl_cb.c
+++ b/video/out/vo_opengl_cb.c
@@ -35,12 +35,17 @@
  * can access it any time, even if the VO is destroyed (or not created yet).
  * The OpenGL object allows initializing the renderer etc. The VO object is only
  * here to transfer the video frames somehow.
+ *
+ * Locking hierarchy:
+ * - the libmpv user can mix openglcb and normal API; thus openglcb API
+ *   functions can wait on the core, but not the reverse
+ * - the core does blocking calls into the VO thread, thus the VO functions
+ *   can't wait on the user calling the API functions
+ * - to make video timing work like it should, the VO thread waits on the
+ *   openglcb API user anyway, and the (unlikely) deadlock is avoided with
+ *   a timeout
  */
 
-#define FRAME_DROP_POP      0 // drop the oldest frame in queue
-#define FRAME_DROP_CLEAR    1 // drop all frames in queue
-#define FRAME_DROP_BLOCK    2
-
 struct vo_priv {
     struct vo *vo;
 
@@ -49,8 +54,6 @@ struct vo_priv {
     // Immutable after VO init
     int use_gl_debug;
     struct gl_video_opts *renderer_opts;
-    int frame_queue_size;
-    int frame_drop_mode;
 };
 
 struct mpv_opengl_cb_context {
@@ -64,9 +67,10 @@ struct mpv_opengl_cb_context {
     bool initialized;
     mpv_opengl_cb_update_fn update_cb;
     void *update_cb_ctx;
-    struct vo_frame *waiting_frame;
-    struct vo_frame **frame_queue;
-    int queued_frames;
+    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
+    int64_t flip_count;
     struct vo_frame *cur_frame;
     struct mp_image_params img_params;
     bool reconfigured, reset;
@@ -80,8 +84,6 @@ struct mpv_opengl_cb_context {
     struct m_config *new_opts_cfg;
     bool eq_changed;
     struct mp_csp_equalizer eq;
-    int64_t recent_flip;
-    bool frozen; // libmpv user is not redrawing frames
     struct vo *active;
     int hwdec_api;
 
@@ -96,68 +98,9 @@ struct mpv_opengl_cb_context {
 
 static void update(struct vo_priv *p);
 
-// all queue manipulation functions shold be called under locked state
-
-static struct vo_frame *frame_queue_pop(struct mpv_opengl_cb_context *ctx)
-{
-    if (ctx->queued_frames == 0)
-        return NULL;
-    struct vo_frame *ret = ctx->frame_queue[0];
-    MP_TARRAY_REMOVE_AT(ctx->frame_queue, ctx->queued_frames, 0);
-    pthread_cond_broadcast(&ctx->wakeup);
-    return ret;
-}
-
-static void frame_queue_drop(struct mpv_opengl_cb_context *ctx)
-{
-    struct vo_frame *frame = frame_queue_pop(ctx);
-    if (frame) {
-        talloc_free(frame);
-        if (ctx->active)
-            vo_increment_drop_count(ctx->active, 1);
-        pthread_cond_broadcast(&ctx->wakeup);
-    }
-}
-
-static void frame_queue_clear(struct mpv_opengl_cb_context *ctx)
-{
-    for (int i = 0; i < ctx->queued_frames; i++)
-        talloc_free(ctx->frame_queue[i]);
-    talloc_free(ctx->frame_queue);
-    ctx->frame_queue = NULL;
-    ctx->queued_frames = 0;
-    pthread_cond_broadcast(&ctx->wakeup);
-}
-
-static void frame_queue_drop_all(struct mpv_opengl_cb_context *ctx)
-{
-    int frames = ctx->queued_frames;
-    frame_queue_clear(ctx);
-    if (ctx->active && frames > 0)
-        vo_increment_drop_count(ctx->active, frames);
-    pthread_cond_broadcast(&ctx->wakeup);
-}
-
-static void frame_queue_push(struct mpv_opengl_cb_context *ctx,
-                             struct vo_frame *frame)
-{
-    MP_TARRAY_APPEND(ctx, ctx->frame_queue, ctx->queued_frames, frame);
-    pthread_cond_broadcast(&ctx->wakeup);
-}
-
-static void frame_queue_shrink(struct mpv_opengl_cb_context *ctx, int size)
-{
-    pthread_cond_broadcast(&ctx->wakeup);
-    while (ctx->queued_frames > size)
-        frame_queue_drop(ctx);
-}
-
 static void forget_frames(struct mpv_opengl_cb_context *ctx, bool all)
 {
     pthread_cond_broadcast(&ctx->wakeup);
-    frame_queue_clear(ctx);
-    talloc_free(ctx->waiting_frame);
-    ctx->waiting_frame = NULL;
     if (all) {
         talloc_free(ctx->cur_frame);
         ctx->cur_frame = NULL;
@@ -294,7 +237,6 @@ int mpv_opengl_cb_draw(mpv_opengl_cb_context *ctx, int fbo, int vp_w, int vp_h)
     struct vo *vo = ctx->active;
 
     ctx->force_update |= ctx->reconfigured;
-    ctx->frozen = false;
 
     if (ctx->vp_w != vp_w || ctx->vp_h != vp_h)
         ctx->force_update = true;
@@ -326,7 +268,6 @@ int mpv_opengl_cb_draw(mpv_opengl_cb_context *ctx, int fbo, int vp_w, int vp_h)
             gl_video_configure_queue(ctx->renderer, vo);
             ctx->gl->debug_context = opts->use_gl_debug;
             gl_video_set_debug(ctx->renderer, opts->use_gl_debug);
-            frame_queue_shrink(ctx, opts->frame_queue_size);
         }
     }
     ctx->reconfigured = false;
@@ -346,8 +287,12 @@ int mpv_opengl_cb_draw(mpv_opengl_cb_context *ctx, int fbo, int vp_w, int vp_h)
     }
     ctx->eq_changed = false;
 
-    struct vo_frame *frame = frame_queue_pop(ctx);
+    struct vo_frame *frame = ctx->next_frame;
+    int64_t wait_present_count = ctx->present_count;
     if (frame) {
+        ctx->next_frame = NULL;
+        wait_present_count += 1;
+        pthread_cond_signal(&ctx->wakeup);
         talloc_free(ctx->cur_frame);
         ctx->cur_frame = vo_frame_ref(frame);
     } else {
@@ -371,12 +316,11 @@ int mpv_opengl_cb_draw(mpv_opengl_cb_context *ctx, int fbo, int vp_w, int vp_h)
         talloc_free(frame);
 
     pthread_mutex_lock(&ctx->lock);
-    const int left = ctx->queued_frames;
-    if (vo && left > 0)
-        update(vo->priv);
+    while (wait_present_count > ctx->present_count)
+        pthread_cond_wait(&ctx->wakeup, &ctx->lock);
     pthread_mutex_unlock(&ctx->lock);
 
-    return left;
+    return 0;
 }
 
 int mpv_opengl_cb_report_flip(mpv_opengl_cb_context *ctx, int64_t time)
@@ -384,7 +328,8 @@ int mpv_opengl_cb_report_flip(mpv_opengl_cb_context *ctx, int64_t time)
     MP_STATS(ctx, "glcb-reportflip");
 
     pthread_mutex_lock(&ctx->lock);
-    ctx->recent_flip = time > 0 ? time : mp_time_us();
+    ctx->flip_count += 1;
+    pthread_cond_signal(&ctx->wakeup);
     pthread_mutex_unlock(&ctx->lock);
 
     return 0;
@@ -402,38 +347,49 @@ static void draw_frame(struct vo *vo, struct vo_frame *frame)
     struct vo_priv *p = vo->priv;
 
     pthread_mutex_lock(&p->ctx->lock);
-    talloc_free(p->ctx->waiting_frame);
-    p->ctx->waiting_frame = vo_frame_ref(frame);
+    assert(!p->ctx->next_frame);
+    p->ctx->next_frame = vo_frame_ref(frame);
+    p->ctx->expected_flip_count = p->ctx->flip_count + 1;
+    update(p);
     pthread_mutex_unlock(&p->ctx->lock);
 }
 
 static void flip_page(struct vo *vo)
 {
     struct vo_priv *p = vo->priv;
+    struct timespec ts = mp_rel_time_to_timespec(0.2);
 
     pthread_mutex_lock(&p->ctx->lock);
-    while (p->ctx->queued_frames >= p->frame_queue_size) {
-        switch (p->frame_drop_mode) {
-        case FRAME_DROP_CLEAR:
-            frame_queue_drop_all(p->ctx);
+
+    // Wait until frame was rendered
+    while (p->ctx->next_frame) {
+        if (pthread_cond_timedwait(&p->ctx->wakeup, &p->ctx->lock, &ts))
             break;
-        case FRAME_DROP_POP:
-            frame_queue_shrink(p->ctx, p->frame_queue_size - 1);
+    }
+
+    // Unblock mpv_opengl_cb_draw().
+    p->ctx->present_count += 1;
+    pthread_cond_signal(&p->ctx->wakeup);
+
+    // Wait until frame was presented
+    while (p->ctx->expected_flip_count > p->ctx->flip_count) {
+        // mpv_opengl_cb_report_flip() is declared as optional API.
+        // Assume the user calls it consistently _if_ it's called at all.
+        if (!p->ctx->flip_count)
             break;
-        case FRAME_DROP_BLOCK: ;
-            struct timespec ts = mp_rel_time_to_timespec(0.2);
-            if (p->ctx->frozen ||
-                pthread_cond_timedwait(&p->ctx->wakeup, &p->ctx->lock, &ts))
-            {
-                frame_queue_drop_all(p->ctx);
-                p->ctx->frozen = true;
-            }
+        if (pthread_cond_timedwait(&p->ctx->wakeup, &p->ctx->lock, &ts))
             break;
-        }
     }
-    frame_queue_push(p->ctx, p->ctx->waiting_frame);
-    p->ctx->waiting_frame = NULL;
-    update(p);
+
+    // The API user is not reacting, or is being unusually slow => drop.
+    if (p->ctx->next_frame) {
+        talloc_free(p->ctx->next_frame);
+        p->ctx->next_frame = NULL;
+        p->ctx->present_count += 1;
+        pthread_cond_signal(&p->ctx->wakeup);
+        vo_increment_drop_count(vo, 1);
+    }
+
     pthread_mutex_unlock(&p->ctx->lock);
 }
 
@@ -466,11 +422,6 @@ static int reconfig(struct vo *vo, struct mp_image_params *params)
 #define OPT_BASE_STRUCT struct vo_priv
 static const struct m_option change_opts[] = {
     OPT_FLAG("debug", use_gl_debug, 0),
-    OPT_INTRANGE("frame-queue-size", frame_queue_size, 0, 1, 100, OPTDEF_INT(2)),
-    OPT_CHOICE("frame-drop-mode", frame_drop_mode, 0,
-               ({"pop", FRAME_DROP_POP},
-                {"clear", FRAME_DROP_CLEAR},
-                {"block", FRAME_DROP_BLOCK})),
     OPT_SUBSTRUCT("", renderer_opts, gl_video_conf, 0),
     {0}
 };
@@ -553,16 +504,6 @@ static int control(struct vo *vo, uint32_t request, void *data)
         *arg = p->ctx ? &p->ctx->hwdec_info : NULL;
         return true;
     }
-    case VOCTRL_GET_RECENT_FLIP_TIME: {
-        int r = VO_FALSE;
-        pthread_mutex_lock(&p->ctx->lock);
-        if (p->ctx->recent_flip) {
-            *(int64_t *)data = p->ctx->recent_flip;
-            r = VO_TRUE;
-        }
-        pthread_mutex_unlock(&p->ctx->lock);
-        return r;
-    }
     }
 
     return VO_NOTIMPL;
@@ -611,11 +552,6 @@ static int preinit(struct vo *vo)
 #define OPT_BASE_STRUCT struct vo_priv
 static const struct m_option options[] = {
     OPT_FLAG("debug", use_gl_debug, 0),
-    OPT_INTRANGE("frame-queue-size", frame_queue_size, 0, 1, 100, OPTDEF_INT(2)),
-    OPT_CHOICE("frame-drop-mode", frame_drop_mode, 0,
-               ({"pop", FRAME_DROP_POP},
-                {"clear", FRAME_DROP_CLEAR},
-                {"block", FRAME_DROP_BLOCK}), OPTDEF_INT(FRAME_DROP_BLOCK)),
     OPT_SUBSTRUCT("", renderer_opts, gl_video_conf, 0),
     {0},
 };