demux: fix SEEK_FORWARD into end of cached range

This fixes that there were weird delay ("buffering") when seeking into
the last part of a seekable range. The exact case which triggers it if
SEEK_FORWARD is used, and the seek pts is after the second-last
keyframe, but before the end of the range. In that case,
find_seek_target() returned NULL, and the cache layer waited until the
_next_ keyframe the underlying demuxer returned until resuming playback.

find_seek_target() returned NULL, because the last keyframe had
kf_seek_pts unset. This field contains the lowest PTS in the packet
range from the keyframe until the next keyframe (or EOF). For normal
seeks, this is needed because keyframes don't necessarily have the
minimum PTS in the packet range, so it needs to be computed by waiting
for all packets until the next keyframe (or EOF).

Strictly speaking, this behavior was correct, but it meant that the
caller would set ds->skip_to_keyframe, which waits for the next newly
demuxed keyframe. No packets were returned to the decoder until this
happened, usually resulting in the frontend entering "buffering" mode.

What it really needs to do is returning the last keyframe in the cache.
In this situation, the seek target points in the middle of the last
completely cached packet range (as delimited by keyframes), and
SEEK_FORWARD is supposed to skip to the next keyframe. This is in line
with the basic assumptions the packet cache makes (e.g. the keyframe
flag means it's possible to start decoding, and the frames decoded from
it and following packets will strictly have PTS values above the
previous keyframe range). This means in this situation the kf_seek_pts
value doesn't matter either.

So fix this situation by explicitly detecting it and then returning the
last cached keyframe.

Should the search loop look at all packets, instead of only keyframe
ones? This would mean it can know that it's within the last keyframe
range (without looking at queue->seek_end). Maybe this would be a bit
more natural for the SEEK_FORWARD case, but due to PTS reordering it
doesn't sound like a useful thing to do.

Should skip_to_keyframe be checked by the code that sets kf_seek_pts to
a known value? This wouldn't help too much; the frontend would still go
into "buffering" mode for no reason until the packet range is completed,
although it would resume from the correct range.

Should a NULL return always unconditionally use keyframe_latest? This
makes sense because the seek PTS is usually already in the cached range,
so this is the only case that should happen. But there are scary special
cases, like sparse subtitle streams, or other uses of find_seek_target()
which could be out of range now or in future. Basically, don't "risk"
it.

One other potential problem with this is that the "adjust seek target"
code will be disabled in this case. It checks kf_seek_pts, and if it's
unset, the adjustment is not done. Maybe this could be changed to use
the queue's seek_end time, but I'm not sure if this is fully kosher. On
the other hand, I think the main use for this adjustment is with
backwards seeks, so this shouldn't matter.

A previous commit dealing with audio/video stream merging mentioned how
seeking forward entered "buffering" mode for unknown reasons; this
commit fixes this issue.

1 files changed, 16 insertions, 0 deletions

diff --git a/demux/demux.c b/demux/demux.c
index 6908bbcca5..5fdfee31cd 100644
--- a/demux/demux.c
+++ b/demux/demux.c
@@ -2614,6 +2614,22 @@ static struct demux_packet *find_seek_target(struct demux_queue *queue,
             break;
     }
 
+    // Usually, the last seen keyframe (keyframe_latest) has kf_seek_pts unset
+    // (because it needs to see all packets until the next keyframe or EOF in
+    // order to determine the minimum PTS the range provides). If the pts is
+    // within seek range, but the second-last keyframe is before the seek
+    // target, above search will return NULL, even though we should return
+    // keyframe_latest.
+    // This is only correct in the case when the target PTS is still within the
+    // seek range; the timestamps past it are unknown.
+    if (!target && (flags & SEEK_FORWARD) && queue->keyframe_latest &&
+        queue->keyframe_latest->kf_seek_pts == MP_NOPTS_VALUE &&
+        pts <= queue->seek_end)
+    {
+        target = queue->keyframe_latest;
+    }
+
+
     return target;
 }