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},
 };