diff options
author | wm4 <wm4@nowhere> | 2014-01-14 17:38:21 +0100 |
---|---|---|
committer | wm4 <wm4@nowhere> | 2014-01-15 20:56:16 +0100 |
commit | a735a2abd734748aace06987632353f161e8faf5 (patch) | |
tree | 41452c9ca04e24111996256c58d1673a1de59834 /demux/demux_mkv.c | |
parent | 112c1abe31674fd5f43119109f94b09d885ed357 (diff) | |
download | mpv-a735a2abd734748aace06987632353f161e8faf5.tar.bz2 mpv-a735a2abd734748aace06987632353f161e8faf5.tar.xz |
demux_mkv: improve robustness by explicitly checking for level 1 elements
Matroska makes it pretty hard to resync correctly on broken files:
random data returns "valid" EBML IDs with a high probability, and when
trying to skip them it's likely that you skip a random amount of data
(instead of considering the element length invalid).
Improve upon this by skipping known level 1 elements only. Consider
everything else invalid and call the resync code. This might result in
annoying behavior when Matroska adds new level 1 elements, although it
won't be particularly harmful. Matroska doesn't really allow us to do
better (even mkvtoolnix explicitly checks for known level 1 elements).
Since we now don't always want to combine EBML element skipping and
resyncing, remove ebml_read_skip_or_resync_cluster(), and make
ebml_read_skip() more tolerant against skipping broken elements.
Also, don't resync when reading sub-elements, and instead do resyncing
when reading them results in an error.
Diffstat (limited to 'demux/demux_mkv.c')
-rw-r--r-- | demux/demux_mkv.c | 18 |
1 files changed, 12 insertions, 6 deletions
diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 90a62ee90f..d7fb04bec1 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -723,7 +723,7 @@ static int demux_mkv_read_cues(demuxer_t *demuxer) stream_t *s = demuxer->stream; if (opts->index_mode == 0 || opts->index_mode == 2) { - ebml_read_skip(s, NULL); + ebml_read_skip(demuxer->log, -1, s); return 0; } @@ -1128,7 +1128,7 @@ static int read_header_element(struct demuxer *demuxer, uint32_t id, res = 2; } if (!at_filepos && id != EBML_ID_INVALID) - ebml_read_skip(s, NULL); + ebml_read_skip(demuxer->log, -1, s); return res; } @@ -2454,11 +2454,12 @@ static int read_block_group(demuxer_t *demuxer, int64_t end, block->keyframe = false; break; + case MATROSKA_ID_CLUSTER: case EBML_ID_INVALID: goto error; default: - if (ebml_read_skip_or_resync_cluster(demuxer->log, end, s) != 0) + if (ebml_read_skip(demuxer->log, end, s) != 0) goto error; break; } @@ -2519,8 +2520,7 @@ static int read_next_block(demuxer_t *demuxer, struct block_info *block) goto find_next_cluster; default: ; - if (ebml_read_skip_or_resync_cluster - (demuxer->log, mkv_d->cluster_end, s) != 0) + if (ebml_read_skip(demuxer->log, mkv_d->cluster_end, s) != 0) goto find_next_cluster; break; } @@ -2535,7 +2535,13 @@ static int read_next_block(demuxer_t *demuxer, struct block_info *block) break; if (s->eof) return -1; - ebml_read_skip_or_resync_cluster(demuxer->log, -1, s); + // For the sake of robustness, consider even unknown level 1 + // elements the same as unknown/broken IDs. + if (!ebml_is_mkv_level1_id(id) || + ebml_read_skip(demuxer->log, -1, s) != 0) + { + ebml_resync_cluster(demuxer->log, s); + } } next_cluster: mkv_d->cluster_end = ebml_read_length(s, NULL); |