From 4a8647b6b85be7a514488f0b43cdc0bd199b2407 Mon Sep 17 00:00:00 2001 From: wm4 Date: Sun, 14 Aug 2016 20:27:37 +0200 Subject: sub: don't potentially discard too many subtitles on seek The accepts_packet packet callback is supposed to deal with subtitle decoders which have only a small queue of current subtitle events (i.e. sd_lavc.c), in case feeding it too many packets would discard events that are still needed. Normally, the number of subtitles that need to be preserved is estimated by the rendering pts (get_bitmaps() argument). Rendering lags behind decoding, so normally the rendering pts is smaller than the next video frame pts, and we simply discard all subtitle events until the rendering pts. This breaks down in some annoying corner cases. One of them is seeking backwards: the VO will still try to render the old PTS during seeks, which passes a high PTS to the subtitle renderer, which in turn would discard more subtitles than it should. There is a similar issue with forward seeks. Add hacks to deal with those issues. There should be a better way to deal with the essentially unknown "rendering position", which is made worse by screenshots or rendering with vf_sub. At the very least, we could handle seeks better, and e.g. either force the VO not to re-render subs after seeks (ugly), or introduce seek sequence numbers to distinguish attempts to render earlier subtitles when a seek is done. --- sub/dec_sub.c | 2 +- sub/sd.h | 2 +- sub/sd_lavc.c | 12 +++++++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/sub/dec_sub.c b/sub/dec_sub.c index 22dc3328b8..39eb032982 100644 --- a/sub/dec_sub.c +++ b/sub/dec_sub.c @@ -208,7 +208,7 @@ bool sub_read_packets(struct dec_sub *sub, double video_pts) while (1) { bool read_more = true; if (sub->sd->driver->accepts_packet) - read_more = sub->sd->driver->accepts_packet(sub->sd); + read_more = sub->sd->driver->accepts_packet(sub->sd, video_pts); if (!read_more) break; diff --git a/sub/sd.h b/sub/sd.h index c8056d379f..178a6d5be6 100644 --- a/sub/sd.h +++ b/sub/sd.h @@ -34,7 +34,7 @@ struct sd_functions { void (*select)(struct sd *sd, bool selected); void (*uninit)(struct sd *sd); - bool (*accepts_packet)(struct sd *sd); // implicit default if NULL: true + bool (*accepts_packet)(struct sd *sd, double pts); // implicit default if NULL: true int (*control)(struct sd *sd, enum sd_ctrl cmd, void *arg); void (*get_bitmaps)(struct sd *sd, struct mp_osd_res dim, int format, diff --git a/sub/sd_lavc.c b/sub/sd_lavc.c index fca437405c..9c217a6936 100644 --- a/sub/sd_lavc.c +++ b/sub/sd_lavc.c @@ -485,11 +485,21 @@ static void get_bitmaps(struct sd *sd, struct mp_osd_res d, int format, osd_rescale_bitmaps(res, insize[0], insize[1], d, video_par); } -static bool accepts_packet(struct sd *sd) +static bool accepts_packet(struct sd *sd, double min_pts) { struct sd_lavc_priv *priv = sd->priv; double pts = priv->current_pts; + if (min_pts != MP_NOPTS_VALUE) { + // guard against bogus rendering PTS in the future. + if (pts == MP_NOPTS_VALUE || min_pts < pts) + pts = min_pts; + // Heuristic: we assume rendering cannot lag behind more than 1 second + // behind decoding. + if (pts + 1 < min_pts) + pts = min_pts; + } + int last_needed = -1; for (int n = 0; n < MAX_QUEUE; n++) { struct sub *sub = &priv->subs[n]; -- cgit v1.2.3