video: fix and simplify video format changes and last frame display

The previous commit broke these things, and fixing them is separate in
this commit in order to reduce the volume of changes.

Move the image queue from the VO to the playback core. The image queue
is a remnant of the old way how vdpau was implemented, and increasingly
became more and more an artifact. In the end, it did only one thing:
computing the duration of the current frame. This was done by taking the
PTS difference between the current and the future frame. We keep this,
but by moving it out of the VO, we don't have to special-case format
changes anymore. This simplifies the code a lot.

Since we need the queue to compute the duration only, a queue size
larger than 2 makes no sense, and we can hardcode that.

Also change how the last frame is handled. The last frame is a bit of a
problem, because video timing works by showing one frame after another,
which makes it a special case. Make the VO provide a function to notify
us when the frame is done, instead. The frame duration is used for that.

This is not perfect. For example, changing playback speed during the
last frame doesn't update the end time. Pausing will not stop the clock
that times the last frame. But I don't think this matters for such a
corner case.

6 files changed, 127 insertions, 181 deletions

diff --git a/player/core.h b/player/core.h
index 1a396cf25f..4ee2916bbb 100644
--- a/player/core.h
+++ b/player/core.h
@@ -240,6 +240,9 @@ typedef struct MPContext {
     struct mp_audio_buffer *ao_buffer;  // queued audio; passed to ao_play() later
 
     struct vo *video_out;
+    // next_frame[0] is the next frame, next_frame[1] the one after that.
+    // Invariant: if next_frame[1] is set, next_frame[0] also is.
+    struct mp_image *next_frame[2];
 
     enum playback_status video_status, audio_status;
     bool restart_complete;
@@ -257,8 +260,6 @@ typedef struct MPContext {
     double delay;
     // AV sync: time until next frame should be shown
     double time_frame;
-    // Display duration (as "intended") of the last flipped frame.
-    double last_frame_duration;
     // Set to true some time after a new frame has been shown, and it turns out
     // that this frame was the last one before video ends.
     bool playing_last_frame;
diff --git a/player/loadfile.c b/player/loadfile.c
index a941f7af68..18a225568e 100644
--- a/player/loadfile.c
+++ b/player/loadfile.c
@@ -111,6 +111,7 @@ void uninit_player(struct MPContext *mpctx, unsigned int mask)
 
     if (mask & INITIALIZED_VCODEC) {
         mpctx->initialized_flags &= ~INITIALIZED_VCODEC;
+        reset_video_state(mpctx);
         if (mpctx->d_video)
             video_uninit(mpctx->d_video);
         mpctx->d_video = NULL;
diff --git a/player/playloop.c b/player/playloop.c
index b96ba2caa2..b21f1ae0e8 100644
--- a/player/playloop.c
+++ b/player/playloop.c
@@ -885,7 +885,7 @@ void run_playloop(struct MPContext *mpctx)
      * of the video, and we want to quit. */
     bool prevent_eof = mpctx->paused;
     if (mpctx->d_video && mpctx->video_status == STATUS_EOF)
-        prevent_eof &= mpctx->video_out && mpctx->video_out->hasframe;
+        prevent_eof &= mpctx->video_out && vo_has_frame(mpctx->video_out);
     /* Handles terminating on end of playback (or switching to next segment).
      *
      * It's possible for the user to simultaneously switch both audio
diff --git a/player/video.c b/player/video.c
index 1ee8ec1203..31a863bc36 100644
--- a/player/video.c
+++ b/player/video.c
@@ -211,11 +211,13 @@ void reset_video_state(struct MPContext *mpctx)
     if (mpctx->video_out)
         vo_seek_reset(mpctx->video_out);
 
+    mp_image_unrefp(&mpctx->next_frame[0]);
+    mp_image_unrefp(&mpctx->next_frame[1]);
+
     mpctx->delay = 0;
     mpctx->time_frame = 0;
     mpctx->video_next_pts = MP_NOPTS_VALUE;
     mpctx->playing_last_frame = false;
-    mpctx->last_frame_duration = 0;
     mpctx->total_avsync_change = 0;
     mpctx->drop_frame_cnt = 0;
     mpctx->dropped_frames = 0;
@@ -481,18 +483,10 @@ static void init_vo(struct MPContext *mpctx)
     mp_notify(mpctx, MPV_EVENT_VIDEO_RECONFIG, NULL);
 }
 
-// Fill the VO buffer with a newly filtered or decoded image.
+// Fill mpctx->next_frame[] with a newly filtered or decoded image.
 // returns VD_* code
-static int video_output_image(struct MPContext *mpctx, double endpts,
-                              bool reconfig_ok)
+static int video_output_image(struct MPContext *mpctx, double endpts)
 {
-    struct vo *vo = mpctx->video_out;
-
-    // Already enough video buffered in VO?
-    // (This implies vo_has_next_frame(vo, false/true) returns true.)
-    if (!vo_needs_new_image(vo) && vo->params)
-        return 1;
-
     // Filter a new frame.
     int r = video_decode_and_filter(mpctx);
     if (r < 0)
@@ -522,46 +516,12 @@ static int video_output_image(struct MPContext *mpctx, double endpts,
         }
     }
 
-    // Filter output is different from VO input?
-    bool need_vo_reconfig = !vo->params  ||
-        !mp_image_params_equal(&vf->output_params, vo->params);
-
-    if (need_vo_reconfig) {
-        // Draining VO buffers.
-        if (vo_has_next_frame(vo, true))
-            return 0; // EOF so that caller displays remaining VO frames
-
-        // There was no decoded image yet - must not signal fake EOF.
-        // Likewise, if there's no filtered frame yet, don't reconfig yet.
-        if (!vf->output_params.imgfmt || !vf->output)
-            return r;
-
-        // Force draining.
-        if (!reconfig_ok)
-            return 0;
-
-        struct mp_image_params p = vf->output_params;
-
-        const struct vo_driver *info = mpctx->video_out->driver;
-        MP_INFO(mpctx, "VO: [%s] %dx%d => %dx%d %s\n",
-                info->name, p.w, p.h, p.d_w, p.d_h, vo_format_name(p.imgfmt));
-        MP_VERBOSE(mpctx, "VO: Description: %s\n", info->description);
-
-        int vo_r = vo_reconfig(vo, &p, 0);
-        if (vo_r < 0) {
-            vf->initialized = -1;
-            return VD_ERROR;
-        }
-        init_vo(mpctx);
-        // Display the frame queued after this immediately.
-        // (Neutralizes frame time calculation in update_video.)
-        mpctx->video_next_pts = MP_NOPTS_VALUE;
-    }
-
     // Queue new frame, if there's one.
     struct mp_image *img = vf_read_output_frame(vf);
     if (img) {
-        vo_queue_image(vo, img);
+        int pos = mpctx->next_frame[0] ? 1 : 0;
+        assert(!mpctx->next_frame[pos]);
+        mpctx->next_frame[pos] = img;
         return VD_PROGRESS;
     }
 
@@ -569,34 +529,40 @@ static int video_output_image(struct MPContext *mpctx, double endpts,
 }
 
 // returns VD_* code
-static int update_video(struct MPContext *mpctx, double endpts, bool reconfig_ok,
+static int update_video(struct MPContext *mpctx, double endpts,
                         double *frame_duration)
 {
-    struct vo *video_out = mpctx->video_out;
+    struct vo *vo = mpctx->video_out;
+    bool eof = false;
+
+    // Already enough video buffered?
+    bool vo_framedrop = !!mpctx->video_out->driver->flip_page_timed;
+    int min_frames = vo_framedrop ? 2 : 1; // framedrop needs duration
+    if (!mpctx->next_frame[min_frames - 1]) {
+        int r = video_output_image(mpctx, endpts);
+        if (r < 0 || r == VD_WAIT)
+            return r;
+        eof = r == VD_EOF;
+    }
 
     if (mpctx->d_video->header->attached_picture) {
-        if (video_out->hasframe)
+        if (vo_has_frame(vo))
             return VD_EOF;
-        if (vo_has_next_frame(video_out, true))
+        if (mpctx->next_frame[0]) {
+            mpctx->video_next_pts = MP_NOPTS_VALUE;
             return VD_NEW_FRAME;
+        }
+        return VD_PROGRESS;
     }
 
-    int r = video_output_image(mpctx, endpts, reconfig_ok);
-    if (r < 0 || r == VD_WAIT)
-        return r;
+    // On EOF, we write the remaining frames; otherwise we must ensure that
+    // we have a frame (and with VO framedropping, the frame after that).
+    if (eof)
+        min_frames = 1;
+    if (!mpctx->next_frame[min_frames - 1])
+        return eof ? VD_EOF : VD_PROGRESS;
 
-    // On EOF, we always drain the VO; otherwise we must ensure that
-    // the VO will have enough frames buffered (matters especially for VO based
-    // frame dropping).
-    if (!vo_has_next_frame(video_out, r == VD_EOF))
-        return r ? VD_PROGRESS : VD_EOF;
-
-    if (mpctx->d_video->header->attached_picture) {
-        mpctx->video_next_pts = MP_NOPTS_VALUE;
-        return VD_NEW_FRAME;
-    }
-
-    double pts = vo_get_next_pts(video_out, 0);
+    double pts = mpctx->next_frame[0]->pts;
     double last_pts = mpctx->video_next_pts;
     if (last_pts == MP_NOPTS_VALUE)
         last_pts = pts;
@@ -677,41 +643,19 @@ void write_video(struct MPContext *mpctx, double endpts)
 
     update_fps(mpctx);
 
-    // Whether there's still at least 1 video frame that can be shown.
-    // If false, it means we can reconfig the VO if needed (normally, this
-    // would disrupt playback, so only do it on !still_playing).
-    bool still_playing = vo_has_next_frame(vo, true);
-    // For the last frame case (frame is being displayed).
-    still_playing |= mpctx->playing_last_frame;
-    still_playing |= mpctx->last_frame_duration > 0;
-
     double frame_time = 0;
-    int r = update_video(mpctx, endpts, !still_playing, &frame_time);
-    MP_TRACE(mpctx, "update_video: %d (still_playing=%d)\n", r, still_playing);
+    int r = update_video(mpctx, endpts, &frame_time);
+    MP_TRACE(mpctx, "update_video: %d\n", r);
 
     if (r == VD_WAIT) // Demuxer will wake us up for more packets to decode.
         return;
 
-    if (r < 0) {
-        MP_FATAL(mpctx, "Could not initialize video chain.\n");
-        int uninit = INITIALIZED_VCODEC;
-        if (!opts->force_vo)
-            uninit |= INITIALIZED_VO;
-        uninit_player(mpctx, uninit);
-        if (!mpctx->current_track[STREAM_AUDIO])
-            mpctx->stop_play = PT_NEXT_ENTRY;
-        mpctx->error_playing = true;
-        handle_force_window(mpctx, true);
-        return; // restart loop
-    }
+    if (r < 0)
+        goto error;
 
-    if (r == VD_EOF) {
-        if (!mpctx->playing_last_frame && mpctx->last_frame_duration > 0) {
-            mpctx->time_frame += mpctx->last_frame_duration;
-            mpctx->last_frame_duration = 0;
-            mpctx->playing_last_frame = true;
-            MP_VERBOSE(mpctx, "showing last frame\n");
-        }
+    if (r == VD_EOF && vo_still_displaying(vo)) {
+        mpctx->video_status = STATUS_DRAINING;
+        return;
     }
 
     if (r == VD_NEW_FRAME) {
@@ -724,10 +668,6 @@ void write_video(struct MPContext *mpctx, double endpts)
             mpctx->time_frame += frame_time / opts->playback_speed;
             adjust_sync(mpctx, frame_time);
         }
-    } else if (r == VD_EOF && mpctx->playing_last_frame) {
-        // Let video timing code continue displaying.
-        mpctx->video_status = STATUS_DRAINING;
-        MP_VERBOSE(mpctx, "still showing last frame\n");
     } else if (r <= 0) {
         // EOF or error
         mpctx->delay = 0;
@@ -789,12 +729,28 @@ void write_video(struct MPContext *mpctx, double endpts)
             mpctx->time_frame = 0;
     }
 
-    // last frame case
-    // TODO: should be _after_ wait
-    mpctx->playing_last_frame = false;
     if (r != VD_NEW_FRAME)
         return;
 
+    // Filter output is different from VO input?
+    struct mp_image_params p = mpctx->next_frame[0]->params;
+    if (!vo->params || !mp_image_params_equal(&p, vo->params)) {
+        // Changing config deletes the current frame; wait until it's finished.
+        if (vo_still_displaying(vo))
+            return;
+
+        const struct vo_driver *info = mpctx->video_out->driver;
+        MP_INFO(mpctx, "VO: [%s] %dx%d => %dx%d %s\n",
+                info->name, p.w, p.h, p.d_w, p.d_h, vo_format_name(p.imgfmt));
+        MP_VERBOSE(mpctx, "VO: Description: %s\n", info->description);
+
+        int vo_r = vo_reconfig(vo, &p, 0);
+        if (vo_r < 0)
+            goto error;
+        init_vo(mpctx);
+        mpctx->time_frame = 0; // display immediately
+    }
+
     double time_frame = MPMAX(mpctx->time_frame, -1);
     int64_t pts = mp_time_us() + (int64_t)(time_frame * 1e6);
 
@@ -802,11 +758,16 @@ void write_video(struct MPContext *mpctx, double endpts)
         return; // wait until VO wakes us up to get more frames
 
     int64_t duration = -1;
-    double vpts0 = vo_get_next_pts(vo, 0);
-    double vpts1 = vo_get_next_pts(vo, 1);
-    if (vpts0 != MP_NOPTS_VALUE && vpts1 != MP_NOPTS_VALUE) {
+    double diff = -1;
+    double vpts0 = mpctx->next_frame[0] ? mpctx->next_frame[0]->pts : MP_NOPTS_VALUE;
+    double vpts1 = mpctx->next_frame[1] ? mpctx->next_frame[1]->pts : MP_NOPTS_VALUE;
+    if (vpts0 != MP_NOPTS_VALUE && vpts1 != MP_NOPTS_VALUE)
+        diff = vpts1 - vpts0;
+    if (diff < 0 && mpctx->d_video->fps > 0)
+        diff = 1.0 / mpctx->d_video->fps; // fallback to demuxer-reported fps
+    if (diff >= 0) {
         // expected A/V sync correction is ignored
-        double diff = (vpts1 - vpts0) / opts->playback_speed;
+        diff /= opts->playback_speed;
         if (mpctx->time_frame < 0)
             diff += mpctx->time_frame;
         duration = MPCLAMP(diff, 0, 10) * 1e6;
@@ -819,7 +780,10 @@ void write_video(struct MPContext *mpctx, double endpts)
     update_subtitles(mpctx);
     update_osd_msg(mpctx);
 
-    vo_queue_frame(vo, pts, duration);
+    vo_queue_frame(vo, mpctx->next_frame[0], pts, duration);
+
+    mpctx->next_frame[0] = mpctx->next_frame[1];
+    mpctx->next_frame[1] = NULL;
 
     // For print_status - VO call finishing early is OK for sync
     mpctx->time_frame -= get_relative_time(mpctx);
@@ -849,4 +813,18 @@ void write_video(struct MPContext *mpctx, double endpts)
         if (mpctx->max_frames > 0)
             mpctx->max_frames--;
     }
+
+    return;
+
+error:
+    MP_FATAL(mpctx, "Could not initialize video chain.\n");
+    int uninit = INITIALIZED_VCODEC;
+    if (!opts->force_vo)
+        uninit |= INITIALIZED_VO;
+    uninit_player(mpctx, uninit);
+    if (!mpctx->current_track[STREAM_AUDIO])
+        mpctx->stop_play = PT_NEXT_ENTRY;
+    mpctx->error_playing = true;
+    handle_force_window(mpctx, true);
+    mpctx->sleeptime = 0;
 }
diff --git a/video/out/vo.c b/video/out/vo.c
index ec9781d613..a3e81e93fe 100644
--- a/video/out/vo.c
+++ b/video/out/vo.c
@@ -113,8 +113,6 @@ const struct vo_driver *const video_out_drivers[] =
         NULL
 };
 
-#define VO_MAX_QUEUE 2
-
 struct vo_internal {
     pthread_t thread;
     struct mp_dispatch_queue *dispatch;
@@ -130,14 +128,11 @@ struct vo_internal {
 
     char *window_title;
 
+    bool hasframe;
     bool request_redraw;
 
     int64_t flip_queue_offset; // queue flip events at most this much in advance
 
-    // Frames to display; the next (i.e. oldest, lowest PTS) image has index 0.
-    struct mp_image *video_queue[VO_MAX_QUEUE];
-    int num_video_queue;
-
     int64_t wakeup_pts;             // time at which to pull frame from decoder
 
     bool rendering;                 // true if an image is being rendered
@@ -329,7 +324,6 @@ static void run_reconfig(void *p)
         vo->params = NULL;
     }
     forget_frames(vo); // implicitly synchronized
-    vo->hasframe = false;
 }
 
 int vo_reconfig(struct vo *vo, struct mp_image_params *params, int flags)
@@ -364,52 +358,11 @@ int vo_control(struct vo *vo, uint32_t request, void *data)
 static void forget_frames(struct vo *vo)
 {
     struct vo_internal *in = vo->in;
-    for (int n = 0; n < in->num_video_queue; n++)
-        talloc_free(in->video_queue[n]);
-    in->num_video_queue = 0;
+    in->hasframe = false;
     in->frame_queued = false;
     mp_image_unrefp(&in->frame_image);
 }
 
-void vo_queue_image(struct vo *vo, struct mp_image *mpi)
-{
-    struct vo_internal *in = vo->in;
-    assert(mpi);
-    if (!vo->config_ok) {
-        talloc_free(mpi);
-        return;
-    }
-    pthread_mutex_lock(&in->lock);
-    assert(mp_image_params_equal(vo->params, &mpi->params));
-    assert(in->num_video_queue <= VO_MAX_QUEUE);
-    in->video_queue[in->num_video_queue++] = mpi;
-    pthread_mutex_unlock(&in->lock);
-}
-
-// Return whether vo_queue_image() should be called.
-bool vo_needs_new_image(struct vo *vo)
-{
-    return vo->config_ok && vo->in->num_video_queue < VO_MAX_QUEUE;
-}
-
-// Return whether a frame can be displayed.
-//  eof==true: return true if at least one frame is queued
-//  eof==false: return true if "enough" frames are queued
-bool vo_has_next_frame(struct vo *vo, bool eof)
-{
-    // Normally, buffer 1 image ahead, except if the queue is limited to less
-    // than 2 entries, or if EOF is reached and there aren't enough images left.
-    return eof ? vo->in->num_video_queue : vo->in->num_video_queue == VO_MAX_QUEUE;
-}
-
-// Return the PTS of a future frame (where index==0 is the next frame)
-double vo_get_next_pts(struct vo *vo, int index)
-{
-    if (index < 0 || index >= vo->in->num_video_queue)
-        return MP_NOPTS_VALUE;
-    return vo->in->video_queue[index]->pts;
-}
-
 #ifndef __MINGW32__
 static void wait_event_fd(struct vo *vo, int64_t until_time)
 {
@@ -487,9 +440,9 @@ void vo_wakeup(struct vo *vo)
     pthread_mutex_unlock(&in->lock);
 }
 
-// Whether vo_queue_frame() can be called. If the VO is not ready yet (even
-// though an image is queued), the function will return false, and the VO will
-// call the wakeup callback once it's ready.
+// Whether vo_queue_frame() can be called. If the VO is not ready yet, the
+// function will return false, and the VO will call the wakeup callback once
+// it's ready.
 // next_pts is the exact time when the next frame should be displayed. If the
 // VO is ready, but the time is too "early", return false, and call the wakeup
 // callback once the time is right.
@@ -497,7 +450,7 @@ bool vo_is_ready_for_frame(struct vo *vo, int64_t next_pts)
 {
     struct vo_internal *in = vo->in;
     pthread_mutex_lock(&in->lock);
-    bool r = vo->config_ok && in->num_video_queue && !in->frame_queued;
+    bool r = vo->config_ok && !in->frame_queued;
     if (r) {
         // Don't show the frame too early - it would basically freeze the
         // display by disallowing OSD redrawing or VO interaction.
@@ -518,20 +471,19 @@ bool vo_is_ready_for_frame(struct vo *vo, int64_t next_pts)
 
 // Direct the VO thread to put the currently queued image on the screen.
 // vo_is_ready_for_frame() must have returned true before this call.
-void vo_queue_frame(struct vo *vo, int64_t pts_us, int64_t duration)
+// Ownership of the image is handed to the vo.
+void vo_queue_frame(struct vo *vo, struct mp_image *image,
+                    int64_t pts_us, int64_t duration)
 {
     struct vo_internal *in = vo->in;
     pthread_mutex_lock(&in->lock);
-    assert(vo->config_ok && in->num_video_queue && !in->frame_queued);
-    vo->hasframe = true;
+    assert(vo->config_ok && !in->frame_queued);
+    in->hasframe = true;
     in->frame_queued = true;
     in->frame_pts = pts_us;
     in->frame_duration = duration;
-    in->frame_image = in->video_queue[0];
-    in->num_video_queue--;
-    for (int n = 0; n < in->num_video_queue; n++)
-        in->video_queue[n] = in->video_queue[n + 1];
-    in->wakeup_pts = 0;
+    in->frame_image = image;
+    in->wakeup_pts = in->frame_pts + MPMAX(duration, 0);
     wakeup_locked(vo);
     pthread_mutex_unlock(&in->lock);
 }
@@ -670,11 +622,29 @@ void vo_seek_reset(struct vo *vo)
 {
     pthread_mutex_lock(&vo->in->lock);
     forget_frames(vo);
-    vo->hasframe = false;
     pthread_mutex_unlock(&vo->in->lock);
     vo_control(vo, VOCTRL_RESET, NULL);
 }
 
+// Return true if there is still a frame being displayed (or queued).
+// If this returns true, a wakeup some time in the future is guaranteed.
+bool vo_still_displaying(struct vo *vo)
+{
+    struct vo_internal *in = vo->in;
+    pthread_mutex_lock(&vo->in->lock);
+    int64_t now = mp_time_us();
+    int64_t frame_end = in->frame_pts + MPMAX(in->frame_duration, 0);
+    bool working = now < frame_end || in->rendering || in->frame_queued;
+    pthread_mutex_unlock(&vo->in->lock);
+    return working && in->hasframe;
+}
+
+// Whether at least 1 frame was queued or rendered since last seek or reconfig.
+bool vo_has_frame(struct vo *vo)
+{
+    return vo->in->hasframe;
+}
+
 // Calculate the appropriate source and destination rectangle to
 // get a correctly scaled picture, including pan-scan.
 // out_src: visible part of the video
diff --git a/video/out/vo.h b/video/out/vo.h
index 0fe796dc73..0a8dca8c41 100644
--- a/video/out/vo.h
+++ b/video/out/vo.h
@@ -263,9 +263,6 @@ struct vo {
     int dwidth;
     int dheight;
     float monitor_par;
-
-    // --- Accessed by user-thread only.
-    bool hasframe;      // >= 1 frame has been drawn, so redraw is possible
 };
 
 struct mpv_global;
@@ -276,13 +273,12 @@ struct vo *init_best_video_out(struct mpv_global *global,
 int vo_reconfig(struct vo *vo, struct mp_image_params *p, int flags);
 
 int vo_control(struct vo *vo, uint32_t request, void *data);
-void vo_queue_image(struct vo *vo, struct mp_image *mpi);
-bool vo_has_next_frame(struct vo *vo, bool eof);
-double vo_get_next_pts(struct vo *vo, int index);
-bool vo_needs_new_image(struct vo *vo);
 bool vo_is_ready_for_frame(struct vo *vo, int64_t next_pts);
-void vo_queue_frame(struct vo *vo, int64_t pts_us, int64_t duration);
+void vo_queue_frame(struct vo *vo, struct mp_image *image,
+                    int64_t pts_us, int64_t duration);
 void vo_wait_frame(struct vo *vo);
+bool vo_still_displaying(struct vo *vo);
+bool vo_has_frame(struct vo *vo);
 void vo_redraw(struct vo *vo);
 void vo_seek_reset(struct vo *vo);
 void vo_destroy(struct vo *vo);