From 95dce50347cd9110c4a575483b1dfbd8b27f50f6 Mon Sep 17 00:00:00 2001 From: wm4 Date: Fri, 5 Jan 2018 15:51:40 +0100 Subject: demux: fix crash due to incorrect seek range accounting update_seek_ranges() has some special code that attempts to correctly adjust seek ranges for subtitle tracks. (Subtitle are a nightmare for seek ranges, because they are sparse, so using the packet list is not enough to reliably determine the valid cached range.) This had code like this inside the modified if statement: range->seek_start = MP_PTS_MAX(range->seek_start, ); If seek_start is NOPTS, then seek_start will be set to , breaking some other code that checks seek_start for NOPTS to see if it's empty. Fix this by explicitly checking whether seek_start is NOPTS before adjusting it. The crash happened in prune_old_packets() because the range was marked as non-empty, yet there was no packet in it to prune. This was with files with muxed subtitles, when seeking back to the start. This should not happen anymore with the change. Also add an assert() to check_queue_consistency() that checks for this specific case. There's still some mess. In theory, subtitle tracks could be completely empty, yet their seek range would span the entire file. Seek range tracking of subtitle files is slightly broken (even before this change). Some of this should probably be revisited later, including not just using seek_start to determine whether a seek range should be pruned due to being empty. --- demux/demux.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/demux/demux.c b/demux/demux.c index 8c8600d63b..485857453d 100644 --- a/demux/demux.c +++ b/demux/demux.c @@ -314,6 +314,7 @@ static void check_queue_consistency(struct demux_internal *in) for (int n = 0; n < in->num_ranges; n++) { struct demux_cached_range *range = in->ranges[n]; + int range_num_packets = 0; assert(range->num_streams == in->num_streams); @@ -341,6 +342,7 @@ static void check_queue_consistency(struct demux_internal *in) assert(range == in->current_range); assert(queue->ds->queue == queue); } + range_num_packets += 1; if (!dp->next) assert(queue->tail == dp); @@ -373,6 +375,11 @@ static void check_queue_consistency(struct demux_internal *in) if (queue->keyframe_latest) assert(queue->keyframe_latest->keyframe); } + + // Invariant needed by pruning; violation has worse effects than just + // e.g. broken seeking due to incorrect seek ranges. + if (range->seek_start != MP_NOPTS_VALUE) + assert(range_num_packets > 0); } assert(in->total_bytes == total_bytes); @@ -442,7 +449,8 @@ static void update_seek_ranges(struct demux_cached_range *range) for (int n = 0; n < range->num_streams; n++) { struct demux_queue *queue = range->streams[n]; if (queue->ds->selected && !queue->ds->eager && - queue->last_pruned != MP_NOPTS_VALUE) + queue->last_pruned != MP_NOPTS_VALUE && + range->seek_start != MP_NOPTS_VALUE) { // (last_pruned is _exclusive_ to the seekable range, so add a small // value to exclude it from the valid range.) -- cgit v1.2.3