video: refactor PTS code, add fall back heuristic to DTS

Refactor the PTS handling code to make it cleaner, and to separate the
bits that use PTS sorting.

Add a heuristic to fall back to DTS if the PTS us non-monotonic. This
code is based on what FFmpeg/Libav use for ffplay/avplay and also
best_effort_timestamp (which is only in FFmpeg). Basically, this 1. just
uses the DTS if PTS is unset, and 2. ignores PTS entirely if PTS is non-
monotonic, but DTS is sorted.

The code is pretty much the same as in Libav [1]. I'm not sure if all of
it is really needed, or if it does more than what the paragraph above
mentions. But maybe it's fine to cargo-cult this.

This heuristic fixes playback of mpeg4 in ogm, which returns packets
with PTS==DTS, even though the PTS timestamps should follow codec
reordering. This is probably a libavformat demuxer bug, but good luck
trying to fix it.

The way vd_lavc.c returns the frame PTS and DTS to dec_video.c is a bit
inelegant, but maybe better than trying to mess the PTS back into the
decoder callback again.

[1] https://git.libav.org/?p=libav.git;a=blob;f=cmdutils.c;h=3f1c667075724c5cde69d840ed5ed7d992898334;hb=fa515c2088e1d082d45741bbd5c05e13b0500804#l1431

mpvcore/av_common.c

@@ -91,12 +91,10 @@ void mp_set_av_packet(AVPacket *dst, struct demux_packet *mpkt)
 // Return the pts/dts from a frame returned by libavcodec. Note that this
 // assumes libavcodec was fed a packet setup with mp_set_av_packet()! If not,
 // the timestamps might contain garbage.
-// Normally, this returns the pts. If the pts is unknown, return dts instead.
+void mp_get_av_frame_pkt_ts(AVFrame *frame, double *out_pts, double *out_dts)
-double mp_get_av_frame_pkt_pdts(AVFrame *frame)
 {
-    double pts = (union pts){.i = frame->pkt_pts}.d;
+    *out_pts = (union pts){.i = frame->pkt_pts}.d;
-    double dts = (union pts){.i = frame->pkt_dts}.d;
+    *out_dts = (union pts){.i = frame->pkt_dts}.d;
-    return pts == MP_NOPTS_VALUE ? dts : pts;
 }
 
 void mp_add_lavc_decoders(struct mp_decoder_list *list, enum AVMediaType type)

mpvcore/av_common.h

@@ -26,7 +26,7 @@ struct demux_packet;
 
 void mp_copy_lav_codec_headers(AVCodecContext *avctx, AVCodecContext *st);
 void mp_set_av_packet(AVPacket *dst, struct demux_packet *mpkt);
-double mp_get_av_frame_pkt_pdts(AVFrame *frame);
+void mp_get_av_frame_pkt_ts(AVFrame *frame, double *out_pts, double *out_dts);
 void mp_add_lavc_decoders(struct mp_decoder_list *list, enum AVMediaType type);
 int mp_codec_to_av_codec_id(const char *codec);
 const char *mp_codec_from_av_codec_id(int codec_id);

video/decode/dec_video.c

@@ -61,11 +61,14 @@ void video_reset_decoding(struct dec_video *d_video)
     video_vd_control(d_video, VDCTRL_RESET, NULL);
     if (d_video->vf_initialized == 1)
         vf_chain_seek_reset(d_video->vfilter);
-    d_video->prev_codec_reordered_pts = MP_NOPTS_VALUE;
-    d_video->prev_sorted_pts = MP_NOPTS_VALUE;
     d_video->num_buffered_pts = 0;
     d_video->last_pts = MP_NOPTS_VALUE;
     d_video->last_packet_pdts = MP_NOPTS_VALUE;
+    d_video->decoded_pts = MP_NOPTS_VALUE;
+    d_video->codec_pts = MP_NOPTS_VALUE;
+    d_video->codec_dts = MP_NOPTS_VALUE;
+    d_video->sorted_pts = MP_NOPTS_VALUE;
+    d_video->unsorted_pts = MP_NOPTS_VALUE;
     d_video->pts = MP_NOPTS_VALUE;
 }
 
@@ -136,9 +139,6 @@ static int init_video_codec(struct dec_video *d_video, const char *decoder)
         mp_tmsg(MSGT_DECVIDEO, MSGL_V, "Video decoder init failed.\n");
         return 0;
     }
-
-    d_video->prev_codec_reordered_pts = MP_NOPTS_VALUE;
-    d_video->prev_sorted_pts = MP_NOPTS_VALUE;
     return 1;
 }
 
@@ -212,29 +212,65 @@ bool video_init_best_codec(struct dec_video *d_video, char* video_decoders)
     return !!d_video->vd_driver;
 }
 
-static void determine_frame_pts(struct dec_video *d_video)
+static void add_pts_to_sort(struct dec_video *d_video, double pts)
+{
+    if (pts != MP_NOPTS_VALUE) {
+        int delay = -1;
+        video_vd_control(d_video, VDCTRL_QUERY_UNSEEN_FRAMES, &delay);
+        if (delay >= 0 && delay < d_video->num_buffered_pts)
+            d_video->num_buffered_pts = delay;
+        if (d_video->num_buffered_pts ==
+            sizeof(d_video->buffered_pts) / sizeof(double))
+            mp_msg(MSGT_DECVIDEO, MSGL_ERR, "Too many buffered pts\n");
+        else {
+            int i, j;
+            for (i = 0; i < d_video->num_buffered_pts; i++)
+                if (d_video->buffered_pts[i] < pts)
+                    break;
+            for (j = d_video->num_buffered_pts; j > i; j--)
+                d_video->buffered_pts[j] = d_video->buffered_pts[j - 1];
+            d_video->buffered_pts[i] = pts;
+            d_video->num_buffered_pts++;
+        }
+    }
+}
+
+// Return true if pts1 comes before pts2. pts1 can be MP_NOPTS_VALUE, but pts2
+// always has to be valid. pts1 can't be equal or larger than pts2.
+#define PTS_IS_ORDERED(pts1, pts2) \
+    ((pts2) != MP_NOPTS_VALUE && ((pts1) == MP_NOPTS_VALUE || ((pts1) < (pts2))))
+
+static double retrieve_sorted_pts(struct dec_video *d_video, double codec_pts)
 {
     struct MPOpts *opts = d_video->opts;
 
-    if (!opts->correct_pts) {
+    double sorted_pts;
-        double frame_time = 1.0f / (d_video->fps > 0 ? d_video->fps : 25);
+    if (d_video->num_buffered_pts) {
-        double pkt_pts = d_video->last_packet_pdts;
+        d_video->num_buffered_pts--;
-        if (d_video->pts == MP_NOPTS_VALUE)
+        sorted_pts = d_video->buffered_pts[d_video->num_buffered_pts];
-            d_video->pts = pkt_pts == MP_NOPTS_VALUE ? 0 : pkt_pts;
+    } else {
-
+        mp_msg(MSGT_CPLAYER, MSGL_ERR,
-        d_video->pts = d_video->pts + frame_time;
+                "No pts value from demuxer to use for frame!\n");
-        return;
+        sorted_pts = MP_NOPTS_VALUE;
     }
 
+    if (!PTS_IS_ORDERED(d_video->sorted_pts, sorted_pts))
+        d_video->num_sorted_pts_problems++;
+    d_video->sorted_pts = sorted_pts;
+
+    if (!PTS_IS_ORDERED(d_video->unsorted_pts, codec_pts))
+        d_video->num_unsorted_pts_problems++;
+    d_video->unsorted_pts = codec_pts;
+
     if (opts->user_pts_assoc_mode)
         d_video->pts_assoc_mode = opts->user_pts_assoc_mode;
     else if (d_video->pts_assoc_mode == 0) {
-        if (d_video->codec_reordered_pts != MP_NOPTS_VALUE)
+        if (codec_pts != MP_NOPTS_VALUE)
             d_video->pts_assoc_mode = 1;
         else
             d_video->pts_assoc_mode = 2;
     } else {
-        int probcount1 = d_video->num_reordered_pts_problems;
+        int probcount1 = d_video->num_unsorted_pts_problems;
         int probcount2 = d_video->num_sorted_pts_problems;
         if (d_video->pts_assoc_mode == 2) {
             int tmp = probcount1;
@@ -248,8 +284,7 @@ static void determine_frame_pts(struct dec_video *d_video)
                    d_video->pts_assoc_mode);
         }
     }
-    d_video->pts = d_video->pts_assoc_mode == 1 ?
+    return d_video->pts_assoc_mode == 1 ? codec_pts : sorted_pts;
-                   d_video->codec_reordered_pts : d_video->sorted_pts;
 }
 
 struct mp_image *video_decode(struct dec_video *d_video,
@@ -265,25 +300,11 @@ struct mp_image *video_decode(struct dec_video *d_video,
     if (pkt_pdts != MP_NOPTS_VALUE)
         d_video->last_packet_pdts = pkt_pdts;
 
-    if (sort_pts && pkt_pdts != MP_NOPTS_VALUE) {
+    if (sort_pts)
-        int delay = -1;
+        add_pts_to_sort(d_video, pkt_pdts);
-        video_vd_control(d_video, VDCTRL_QUERY_UNSEEN_FRAMES, &delay);
+
-        if (delay >= 0 && delay < d_video->num_buffered_pts)
+    double prev_codec_pts = d_video->codec_pts;
-            d_video->num_buffered_pts = delay;
+    double prev_codec_dts = d_video->codec_dts;
-        if (d_video->num_buffered_pts ==
-            sizeof(d_video->buffered_pts) / sizeof(double))
-            mp_msg(MSGT_DECVIDEO, MSGL_ERR, "Too many buffered pts\n");
-        else {
-            int i, j;
-            for (i = 0; i < d_video->num_buffered_pts; i++)
-                if (d_video->buffered_pts[i] < pkt_pdts)
-                    break;
-            for (j = d_video->num_buffered_pts; j > i; j--)
-                d_video->buffered_pts[j] = d_video->buffered_pts[j - 1];
-            d_video->buffered_pts[i] = pkt_pdts;
-            d_video->num_buffered_pts++;
-        }
-    }
 
     struct mp_image *mpi = d_video->vd_driver->decode(d_video, packet, drop_frame);
 
@@ -299,32 +320,43 @@ struct mp_image *video_decode(struct dec_video *d_video,
     else if (opts->field_dominance == 1)
         mpi->fields &= ~MP_IMGFIELD_TOP_FIRST;
 
-    double pts = mpi->pts;
+    // Note: the PTS is reordered, but the DTS is not. Both should be monotonic.
+    double pts = d_video->codec_pts;
+    double dts = d_video->codec_dts;
 
-    double prevpts = d_video->codec_reordered_pts;
+    if (pts == MP_NOPTS_VALUE) {
-    d_video->prev_codec_reordered_pts = prevpts;
+        d_video->codec_pts = prev_codec_pts;
-    d_video->codec_reordered_pts = pts;
+    } else if (pts <= prev_codec_pts) {
-    if (prevpts != MP_NOPTS_VALUE && pts <= prevpts
+        d_video->num_codec_pts_problems++;
-        || pts == MP_NOPTS_VALUE)
-        d_video->num_reordered_pts_problems++;
-    prevpts = d_video->sorted_pts;
-    if (sort_pts) {
-        if (d_video->num_buffered_pts) {
-            d_video->num_buffered_pts--;
-            d_video->sorted_pts =
-                d_video->buffered_pts[d_video->num_buffered_pts];
-        } else {
-            mp_msg(MSGT_CPLAYER, MSGL_ERR,
-                   "No pts value from demuxer to use for frame!\n");
-            d_video->sorted_pts = MP_NOPTS_VALUE;
-        }
     }
-    pts = d_video->sorted_pts;
+
-    if (prevpts != MP_NOPTS_VALUE && pts <= prevpts
+    if (dts == MP_NOPTS_VALUE) {
-        || pts == MP_NOPTS_VALUE)
+        d_video->codec_dts = prev_codec_dts;
-        d_video->num_sorted_pts_problems++;
+    } else if (dts <= prev_codec_dts) {
-    determine_frame_pts(d_video);
+        d_video->num_codec_dts_problems++;
-    mpi->pts = d_video->pts;
+    }
+
+    // If PTS is unset, or non-monotonic, fall back to DTS.
+    if ((d_video->num_codec_pts_problems > d_video->num_codec_dts_problems ||
+         pts == MP_NOPTS_VALUE) && dts != MP_NOPTS_VALUE)
+        pts = dts;
+
+    // Alternative PTS determination methods
+    if (!opts->correct_pts) {
+        double frame_time = 1.0f / (d_video->fps > 0 ? d_video->fps : 25);
+        double base = d_video->last_packet_pdts;
+        pts = d_video->decoded_pts;
+        if (pts == MP_NOPTS_VALUE)
+            pts = base == MP_NOPTS_VALUE ? 0 : base;
+
+        pts += frame_time;
+    } else if (sort_pts) {
+        pts = retrieve_sorted_pts(d_video, pts);
+    }
+
+    mpi->pts = pts;
+    d_video->decoded_pts = pts;
+    d_video->pts = pts;
     return mpi;
 }
 

video/decode/dec_video.h

@@ -40,18 +40,31 @@ struct dec_video {
 
     char *decoder_desc;
 
-    void *priv;
+    void *priv; // for free use by vd_driver
 
+    // Last PTS from decoder (set with each vd_driver->decode() call)
+    double codec_pts;
+    int num_codec_pts_problems;
+
+    // Last packet DTS from decoder (passed through from source packets)
+    double codec_dts;
+    int num_codec_dts_problems;
+
+    // PTS sorting (obscure, non-default)
     double buffered_pts[32];
     int num_buffered_pts;
-    double codec_reordered_pts;
-    double prev_codec_reordered_pts;
-    int num_reordered_pts_problems;
     double sorted_pts;
-    double prev_sorted_pts;
     int num_sorted_pts_problems;
+    double unsorted_pts;
+    int num_unsorted_pts_problems;
     int pts_assoc_mode;
 
+    // PTS or DTS of packet last read
+    double last_packet_pdts;
+
+    // Final PTS of previously decoded image
+    double decoded_pts;
+
     // PTS of the last decoded frame (often overwritten by player)
     double pts;
 
@@ -60,7 +73,6 @@ struct dec_video {
     float fps;            // FPS from demuxer or from user override
     float initial_decoder_aspect;
 
-    double last_packet_pdts;
     // State used only by player/video.c
     double last_pts;
 };

video/decode/vd_lavc.c

@@ -757,7 +757,7 @@ static int decode(struct dec_video *vd, struct demux_packet *packet,
         return 0;
 
     update_image_params(vd, ctx->pic);
-    double out_pts = mp_get_av_frame_pkt_pdts(ctx->pic);
+    mp_get_av_frame_pkt_ts(ctx->pic, &vd->codec_pts, &vd->codec_dts);
 
     // Note: potentially resets ctx->pic as it is transferred to mpi
     struct mp_image *mpi = image_from_decoder(vd);
@@ -769,7 +769,6 @@ static int decode(struct dec_video *vd, struct demux_packet *packet,
 
     struct mp_image_params vo_params;
     mp_image_params_from_image(&vo_params, mpi);
-    mpi->pts = out_pts;
 
     if (!mp_image_params_equals(&vo_params, &ctx->vo_image_params)) {
         mp_image_pool_clear(ctx->non_dr1_pool);