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.

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];