From 1877d7933ec44b392291c3c1ddd220e8dc66758d Mon Sep 17 00:00:00 2001 From: Stephen Hutchinson Date: Thu, 14 Feb 2013 07:23:26 -0500 Subject: demux_mkv: Support playing Opus streams in Matroska FFmpeg recently changed how it writes Opus-in-Matroska to match the A_OPUS/EXPERIMENTAL name that mkvmerge uses, with the caveat that things will change and compatibility with old files can get worked out when the spec is finalized. This adds both A_OPUS and A_OPUS/EXPERIMENTAL so that *hopefully* it can play both the newer files that use A_OPUS/EXPERIMENTAL, and older ones muxed by FFmpeg that were simply A_OPUS, since this is also what FFmpeg seems to be doing to handle the situation. --- demux/demux_mkv.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index a769aac127..012ddba516 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -1314,6 +1314,8 @@ static struct mkv_audio_tag { { MKV_A_AAC_4LTP, 0, mmioFOURCC('M', 'P', '4', 'A') }, { MKV_A_AAC, 0, mmioFOURCC('M', 'P', '4', 'A') }, { MKV_A_VORBIS, 0, mmioFOURCC('v', 'r', 'b', 's') }, + { MKV_A_OPUS, 0, mmioFOURCC('O', 'p', 'u', 's') }, + { MKV_A_OPUS_EXP, 0, mmioFOURCC('O', 'p', 'u', 's') }, { MKV_A_QDMC, 0, mmioFOURCC('Q', 'D', 'M', 'C') }, { MKV_A_QDMC2, 0, mmioFOURCC('Q', 'D', 'M', '2') }, { MKV_A_WAVPACK, 0, mmioFOURCC('W', 'V', 'P', 'K') }, @@ -1462,6 +1464,9 @@ static int demux_mkv_open_audio(demuxer_t *demuxer, mkv_track_t *track, memcpy((unsigned char *) (sh_a->wf + 1), track->private_data, sh_a->wf->cbSize); } + } else if (!strcmp(track->codec_id, MKV_A_OPUS) + || !strcmp(track->codec_id, MKV_A_OPUS_EXP)) { + sh_a->format = mmioFOURCC('O', 'p', 'u', 's'); } else if (!strncmp(track->codec_id, MKV_A_REALATRC, 7)) { if (track->private_size < RAPROPERTIES4_SIZE) goto error; -- cgit v1.2.3 From e837d8ddaca6f72e29c69833bd46a0454d36221f Mon Sep 17 00:00:00 2001 From: wm4 Date: Fri, 15 Mar 2013 07:49:47 +0100 Subject: demux_mkv: support ALAC Test sample was produced with ffmpeg. Extradata handling closely follows libavformat/matroskadec.c. --- demux/demux_mkv.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 012ddba516..3917621384 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -1321,6 +1321,7 @@ static struct mkv_audio_tag { { MKV_A_WAVPACK, 0, mmioFOURCC('W', 'V', 'P', 'K') }, { MKV_A_TRUEHD, 0, mmioFOURCC('T', 'R', 'H', 'D') }, { MKV_A_FLAC, 0, mmioFOURCC('f', 'L', 'a', 'C') }, + { MKV_A_ALAC, 0, mmioFOURCC('a', 'L', 'a', 'C') }, { MKV_A_REAL28, 0, mmioFOURCC('2', '8', '_', '8') }, { MKV_A_REALATRC, 0, mmioFOURCC('a', 't', 'r', 'c') }, { MKV_A_REALCOOK, 0, mmioFOURCC('c', 'o', 'o', 'k') }, @@ -1549,6 +1550,16 @@ static int demux_mkv_open_audio(demuxer_t *demuxer, mkv_track_t *track, sh_a->codecdata_len = size; memcpy(sh_a->codecdata, ptr, size); } + } else if (!strcmp(track->codec_id, MKV_A_ALAC)) { + if (track->private_size && track->private_size < 10000000) { + sh_a->codecdata_len = track->private_size + 12; + sh_a->codecdata = malloc(sh_a->codecdata_len); + char *data = sh_a->codecdata; + AV_WB32(data + 0, sh_a->codecdata_len); + memcpy(data + 4, "alac", 4); + AV_WB32(data + 8, 0); + memcpy(data + 12, track->private_data, track->private_size); + } } else if (track->a_formattag == mmioFOURCC('W', 'V', 'P', 'K') || track->a_formattag == mmioFOURCC('T', 'R', 'H', 'D')) { copy_private_data: -- cgit v1.2.3 From 546ae23a0c40726a31a152da4fb6363c918065a0 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 28 Mar 2013 00:00:04 +0100 Subject: demux_mkv: set correct aspect ratio even if DisplayHeight is unset Fixes the file test2.mkv from the official Matroska test file collection. libavformat does the same thing. --- demux/demux_mkv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 3917621384..1e9a61806e 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -1278,8 +1278,10 @@ static int demux_mkv_open_video(demuxer_t *demuxer, mkv_track_t *track, if (!track->realmedia) { sh_v->disp_w = track->v_width; sh_v->disp_h = track->v_height; - if (track->v_dheight) - sh_v->aspect = (double) track->v_dwidth / track->v_dheight; + uint32_t dw = track->v_dwidth ? track->v_dwidth : track->v_width; + uint32_t dh = track->v_dheight ? track->v_dheight : track->v_height; + if (dw && dh) + sh_v->aspect = (double) dw / dh; } else { // vd_realvid.c will set aspect to disp_w/disp_h and rederive // disp_w and disp_h from the RealVideo stream contents returned -- cgit v1.2.3 From 3533ee3ae4b4dc2727dd499081d9edda55925749 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 28 Mar 2013 00:00:39 +0100 Subject: demux_mkv: fix skipping broken header elements Fixes test4.mkv from the Matroska test file collection. demux_mkv_open() contains a loop that reads header elements. It starts by reading the EBML element ID with ebml_read_id(). If there is broken data in the header, ebml_read_id() might return EBML_ID_INVALID. However, that is not handled specially, and the code for handling unknown tags is invoked. This reads the EBML element length in order to skip data, which, if the EBML ID is broken, is entirely random. This caused a seek beyond the end of the file, making the demuxer fail. So don't skip any data if the EBML ID was invalid, and simply try to read the next element. ebml_read_id() reads at least one byte, so the parsing loop won't get stuck. All in all this is rather questionable, but since this affects error situations only, makes behavior a bit more robust (no random seeks), and actually fixes at least one sample, it's ok. libavformat's demuxer handled this. --- demux/demux_mkv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 1e9a61806e..6d9ace28b6 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -1066,7 +1066,7 @@ static int read_header_element(struct demuxer *demuxer, uint32_t id, default: res = 2; } - if (!at_filepos) + if (!at_filepos && id != EBML_ID_INVALID) ebml_read_skip(s, NULL); return res; } -- cgit v1.2.3 From ac1c5e6e18afc5043fa078803ef465bb4017c07a Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 28 Mar 2013 00:01:17 +0100 Subject: demux_mkv: improve robustness against broken files Fixes test7.mkv from the Matroska test file collection, as well as some real broken files I've found in the wild. (Unfortunately, true recovery requires resetting the decoders and playback state with a manual seek, but it's still better than just exiting.) If there are broken EBML elements, try harder to skip them correctly. Do this by searching for the next cluster element. The cluster element intentionally has a long ID, so it's a suitable element for resynchronizing (mkvmerge does something similar). We know that data is corrupt if the ID or length fields of an element are malformed. Additionally, if skipping an unknown element goes past the end of the file, we assume it's corrupt and undo the seek. Do this because it often happens that corrupt data is interpreted as correct EBML elements. Since these elements will have a ridiculous values in their length fields due to the large value range that is possible (0-2^56-2), they will go past the end of the file. So instead of skipping them (which would result in playback termination), try to find the next cluster instead. (We still skip unknown elements that are within the file, as this is needed for correct operation. Also, we first execute the seek, because we don't really know where the file ends. Doing it this way is better for unseekable streams too, because it will still work in the non-error case.) This is done as special case in the packet reading function only. On the other hand, that's the only part of the file that's read after initialization is done. --- demux/demux_mkv.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 6d9ace28b6..aa6d9c21bb 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -2228,10 +2229,12 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) case EBML_ID_INVALID: free(block); - return 0; + ebml_resync_cluster(s); + goto find_next_cluster; default: - ebml_read_skip(s, &l); + if (ebml_read_skip_or_resync_cluster(s, &l) != 0) + goto find_next_cluster; break; } mkv_d->blockgroup_size -= l + il; @@ -2290,18 +2293,24 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) break; case EBML_ID_INVALID: - return 0; + ebml_resync_cluster(s); + goto find_next_cluster; - default: - ebml_read_skip(s, &l); + default: ; + if (ebml_read_skip_or_resync_cluster(s, &l) != 0) + goto find_next_cluster; break; } mkv_d->cluster_size -= l + il; } } - while (ebml_read_id(s, &il) != MATROSKA_ID_CLUSTER) { - ebml_read_skip(s, NULL); + find_next_cluster: + for (;;) { + uint32_t id = ebml_read_id(s, &il); + if (id == MATROSKA_ID_CLUSTER) + break; + ebml_read_skip_or_resync_cluster(s, NULL); if (s->eof) return 0; } -- cgit v1.2.3 From 014298522875f7f41d10eb3e41fcd97926cdedee Mon Sep 17 00:00:00 2001 From: wm4 Date: Sat, 30 Mar 2013 20:43:06 +0100 Subject: demux_mkv: don't print non-sense warning on normal EOF Commit ac1c5e6 (demux_mkv: improve robustness against broken files) added code to skip to the next cluster on error conditions. However, reaching normal EOF triggers this code as well, so explicitly check for EOF before this happens. Note that the EOF flag is only set _after_ reading the last byte, so EOF needs to be checked after the fact. (Or in other words, we must check for EOF after the ebml_read_id() call.) (To answer the question why reading packets actually reaches EOF, even if there's the seek index between the last packet and the end of the file: the cluster reading code skips the seeking related EBML elements as normal part of operation, so it hits EOF gracefully when trying to find the next cluster.) --- demux/demux_mkv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index aa6d9c21bb..400bb59ad7 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -2310,9 +2310,9 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) uint32_t id = ebml_read_id(s, &il); if (id == MATROSKA_ID_CLUSTER) break; - ebml_read_skip_or_resync_cluster(s, NULL); if (s->eof) return 0; + ebml_read_skip_or_resync_cluster(s, NULL); } mkv_d->cluster_start = stream_tell(s) - il; mkv_d->cluster_size = ebml_read_length(s, NULL); -- cgit v1.2.3 From 061b99d7b9cac8c6e6be3dc4763cf44284614b7c Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 4 Apr 2013 01:20:06 +0200 Subject: demux_mkv: fix handling of 0 DisplayWidth/Height Commit 546ae23 fixed aspect ratio if the DisplayWidth or DisplayHeight elements were missing. However, some bogus files [1] can have these elements present in the file, but set to 0. Use 1:1 pixel aspect for such files. [1] https://ffmpeg.org/trac/ffmpeg/ticket/2424 --- demux/demux_mkv.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 400bb59ad7..5b178d66b9 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -92,6 +92,7 @@ typedef struct mkv_track { int type; uint32_t v_width, v_height, v_dwidth, v_dheight; + bool v_dwidth_set, v_dheight_set; double v_frate; uint32_t colorspace; @@ -538,11 +539,13 @@ static void parse_trackvideo(struct demuxer *demuxer, struct mkv_track *track, } if (video->n_display_width) { track->v_dwidth = video->display_width; + track->v_dwidth_set = true; mp_msg(MSGT_DEMUX, MSGL_V, "[mkv] | + Display width: %u\n", track->v_dwidth); } if (video->n_display_height) { track->v_dheight = video->display_height; + track->v_dheight_set = true; mp_msg(MSGT_DEMUX, MSGL_V, "[mkv] | + Display height: %u\n", track->v_dheight); } @@ -1279,8 +1282,8 @@ static int demux_mkv_open_video(demuxer_t *demuxer, mkv_track_t *track, if (!track->realmedia) { sh_v->disp_w = track->v_width; sh_v->disp_h = track->v_height; - uint32_t dw = track->v_dwidth ? track->v_dwidth : track->v_width; - uint32_t dh = track->v_dheight ? track->v_dheight : track->v_height; + uint32_t dw = track->v_dwidth_set ? track->v_dwidth : track->v_width; + uint32_t dh = track->v_dheight_set ? track->v_dheight : track->v_height; if (dw && dh) sh_v->aspect = (double) dw / dh; } else { -- cgit v1.2.3 From 75afa370b9aba90be73b8acc97eb9669bc0f2133 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 4 Apr 2013 01:43:14 +0200 Subject: demux_mkv: try to show current subtitle when seeking Makes sure that seeking to a given time position shows the subtitle at that position. This can fail if the subtitle packet is not close enough to the seek target. Always enabled for hr-seeks, and can be manually enabled for normal seeks with --mkv-subtitle-preroll. This helps displaying subtitles correctly with ordered chapters. When switching ordered chapter segments, a seek is performed. If the subtitle is timed slightly before the start of the segment, it normally won't be demuxed. This is a problem with all seeks, but in this case normal playback is affected. Since switching segments always uses hr-seeks, the code added by this commit is always active in this situation. If no subtitles are selected or the subtitles come from an external file, the demuxer should behave exactly as before this commit. --- demux/demux_mkv.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 5b178d66b9..0171b1e9a2 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -178,6 +178,7 @@ typedef struct mkv_demuxer { uint64_t skip_to_timecode; int v_skip_to_keyframe, a_skip_to_keyframe; + bool subtitle_preroll; int num_audio_tracks; int num_video_tracks; @@ -2114,6 +2115,18 @@ static int handle_block(demuxer_t *demuxer, uint8_t *block, uint64_t length, track->fix_i_bps = 0; } } + } else if (track->type == MATROSKA_TRACK_SUBTITLE + && track->id == demuxer->sub->id) { + if (tc < mkv_d->skip_to_timecode && !mkv_d->subtitle_preroll) + use_this_block = 0; + if (use_this_block) { + ds = demuxer->sub; + if (laces > 1) { + mp_msg(MSGT_DEMUX, MSGL_WARN, "[mkv] Subtitles use Matroska " + "lacing. This is abnormal and not supported.\n"); + use_this_block = 0; + } + } } else if (tc < mkv_d->skip_to_timecode) use_this_block = 0; else if (track->type == MATROSKA_TRACK_VIDEO @@ -2121,14 +2134,6 @@ static int handle_block(demuxer_t *demuxer, uint8_t *block, uint64_t length, ds = demuxer->video; if (mkv_d->v_skip_to_keyframe) use_this_block = keyframe; - } else if (track->type == MATROSKA_TRACK_SUBTITLE - && track->id == demuxer->sub->id) { - ds = demuxer->sub; - if (laces > 1) { - mp_msg(MSGT_DEMUX, MSGL_WARN, "[mkv] Subtitles use Matroska " - "lacing. This is abnormal and not supported.\n"); - use_this_block = 0; - } } else use_this_block = 0; @@ -2170,6 +2175,7 @@ static int handle_block(demuxer_t *demuxer, uint8_t *block, uint64_t length, if (ds == demuxer->video) { mkv_d->v_skip_to_keyframe = 0; mkv_d->skip_to_timecode = 0; + mkv_d->subtitle_preroll = false; } else if (ds == demuxer->audio) mkv_d->a_skip_to_keyframe = 0; @@ -2410,6 +2416,7 @@ static struct mkv_index *seek_with_cues(struct demuxer *demuxer, int seek_id, if (flags & SEEK_BACKWARD) min_diff = -min_diff; min_diff = FFMAX(min_diff, 1); + for (int i = 0; i < mkv_d->num_indexes; i++) if (seek_id < 0 || mkv_d->indexes[i].tnum == seek_id) { int64_t diff = @@ -2427,8 +2434,22 @@ static struct mkv_index *seek_with_cues(struct demuxer *demuxer, int seek_id, } if (index) { /* We've found an entry. */ + uint64_t seek_pos = index->filepos; + if (mkv_d->subtitle_preroll && demuxer->sub->id >= 0) { + uint64_t prev_target = 0; + for (int i = 0; i < mkv_d->num_indexes; i++) { + if (seek_id < 0 || mkv_d->indexes[i].tnum == seek_id) { + uint64_t index_pos = mkv_d->indexes[i].filepos; + if (index_pos > prev_target && index_pos < seek_pos) + prev_target = index_pos; + } + } + if (prev_target) + seek_pos = prev_target; + } + mkv_d->cluster_size = mkv_d->blockgroup_size = 0; - stream_seek(demuxer->stream, index->filepos); + stream_seek(demuxer->stream, seek_pos); } return index; } @@ -2445,6 +2466,7 @@ static void demux_mkv_seek(demuxer_t *demuxer, float rel_seek_secs, if (demuxer->audio->id >= 0) a_tnum = find_track_by_num(mkv_d, demuxer->audio->id, MATROSKA_TRACK_AUDIO)->tnum; + mkv_d->subtitle_preroll = !!(flags & SEEK_SUBPREROLL); if (!(flags & (SEEK_BACKWARD | SEEK_FORWARD))) { if (flags & SEEK_ABSOLUTE || rel_seek_secs < 0) flags |= SEEK_BACKWARD; -- cgit v1.2.3 From c49aa35380a44341e3ffc34301dec3b5d4602522 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 4 Apr 2013 15:24:04 +0200 Subject: demux_mkv: move preroll subtitle check to the right place No subtitle selected was supposed to disable the preroll logic completely. However, the packet skipping logic was not properly enabled, so the demuxer would still return subtitle packets from before the seek target timecode. This shouldn't matter at all in practice, but fixing this makes the code clearer. --- demux/demux_mkv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 0171b1e9a2..ee39038c77 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -2435,7 +2435,7 @@ static struct mkv_index *seek_with_cues(struct demuxer *demuxer, int seek_id, if (index) { /* We've found an entry. */ uint64_t seek_pos = index->filepos; - if (mkv_d->subtitle_preroll && demuxer->sub->id >= 0) { + if (mkv_d->subtitle_preroll) { uint64_t prev_target = 0; for (int i = 0; i < mkv_d->num_indexes; i++) { if (seek_id < 0 || mkv_d->indexes[i].tnum == seek_id) { @@ -2466,7 +2466,7 @@ static void demux_mkv_seek(demuxer_t *demuxer, float rel_seek_secs, if (demuxer->audio->id >= 0) a_tnum = find_track_by_num(mkv_d, demuxer->audio->id, MATROSKA_TRACK_AUDIO)->tnum; - mkv_d->subtitle_preroll = !!(flags & SEEK_SUBPREROLL); + mkv_d->subtitle_preroll = (flags & SEEK_SUBPREROLL) && demuxer->sub->id >= 0; if (!(flags & (SEEK_BACKWARD | SEEK_FORWARD))) { if (flags & SEEK_ABSOLUTE || rel_seek_secs < 0) flags |= SEEK_BACKWARD; -- cgit v1.2.3 From c4e43aaf896ac2267f8f2c10c8c37c3cb565f640 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 11 Apr 2013 17:40:23 +0200 Subject: demux_mkv: use normal index data structure even for incomplete files Incomplete files don't have a valid index, because the index is usually located near the end of a file. In this case, an index is created on the fly during demuxing, or when seeks are done. This used a completely different code path, which leads to unnecessary complications and code duplication. Use the normal index data structure instead. The seeking code at the end of seek_creating_index() (in this commit renamed to create_index_until()) is removed. The normal seek code does the same thing instead. --- demux/demux_mkv.c | 147 +++++++++++++++++++++++------------------------------- 1 file changed, 63 insertions(+), 84 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index ee39038c77..6dd84b20c9 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -161,6 +161,8 @@ typedef struct mkv_demuxer { mkv_index_t *indexes; int num_indexes; + uint64_t index_max_timecode; + bool index_complete; int64_t *parsed_pos; int num_parsed_pos; @@ -170,12 +172,6 @@ typedef struct mkv_demuxer { bool parsed_chapters; bool parsed_attachments; - struct cluster_pos { - uint64_t filepos; - uint64_t timecode; - } *cluster_positions; - int num_cluster_pos; - uint64_t skip_to_timecode; int v_skip_to_keyframe, a_skip_to_keyframe; bool subtitle_preroll; @@ -235,26 +231,6 @@ static mkv_track_t *find_track_by_num(struct mkv_demuxer *d, int n, int type) return NULL; } -static void add_cluster_position(mkv_demuxer_t *mkv_d, uint64_t filepos, - uint64_t timecode) -{ - if (mkv_d->indexes) - return; - - int n = mkv_d->num_cluster_pos; - if (n > 0 && mkv_d->cluster_positions[n-1].filepos >= filepos) - return; - - mkv_d->cluster_positions = - grow_array(mkv_d->cluster_positions, mkv_d->num_cluster_pos, - sizeof(*mkv_d->cluster_positions)); - mkv_d->cluster_positions[mkv_d->num_cluster_pos++] = (struct cluster_pos){ - .filepos = filepos, - .timecode = timecode, - }; -} - - #define AAC_SYNC_EXTENSION_TYPE 0x02b7 static int aac_get_sample_rate_index(uint32_t sample_rate) { @@ -712,6 +688,32 @@ static int demux_mkv_read_tracks(demuxer_t *demuxer) return 0; } +static void cue_index_add(demuxer_t *demuxer, int track_id, uint64_t filepos, + uint64_t timecode) +{ + mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; + + mkv_d->indexes = grow_array(mkv_d->indexes, mkv_d->num_indexes, + sizeof(mkv_index_t)); + mkv_d->indexes[mkv_d->num_indexes].tnum = track_id; + mkv_d->indexes[mkv_d->num_indexes].timecode = timecode; + mkv_d->indexes[mkv_d->num_indexes].filepos = filepos; + mkv_d->num_indexes++; + + mkv_d->index_max_timecode = FFMAX(mkv_d->index_max_timecode, timecode); +} + +static void add_cluster_position(demuxer_t *demuxer, uint64_t filepos, + uint64_t timecode) +{ + mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; + + if (mkv_d->index_complete) + return; + if (timecode > mkv_d->index_max_timecode) + cue_index_add(demuxer, -1, filepos, timecode); +} + static int demux_mkv_read_cues(demuxer_t *demuxer) { mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; @@ -737,23 +739,18 @@ static int demux_mkv_read_cues(demuxer_t *demuxer) for (int i = 0; i < cuepoint->n_cue_track_positions; i++) { struct ebml_cue_track_positions *trackpos = &cuepoint->cue_track_positions[i]; - uint64_t track = trackpos->cue_track; - uint64_t pos = trackpos->cue_cluster_position; - mkv_d->indexes = - grow_array(mkv_d->indexes, mkv_d->num_indexes, - sizeof(mkv_index_t)); - mkv_d->indexes[mkv_d->num_indexes].tnum = track; - mkv_d->indexes[mkv_d->num_indexes].timecode = time; - mkv_d->indexes[mkv_d->num_indexes].filepos = - mkv_d->segment_start + pos; + uint64_t pos = mkv_d->segment_start + trackpos->cue_cluster_position; + cue_index_add(demuxer, trackpos->cue_track, pos, time); mp_msg(MSGT_DEMUX, MSGL_DBG2, "[mkv] |+ found cue point for track %" PRIu64 - ": timecode %" PRIu64 ", filepos: %" PRIu64 "\n", track, - time, mkv_d->segment_start + pos); - mkv_d->num_indexes++; + ": timecode %" PRIu64 ", filepos: %" PRIu64 "\n", + trackpos->cue_track, time, pos); } } + // Do not attempt to create index on the fly. + mkv_d->index_complete = true; + mp_msg(MSGT_DEMUX, MSGL_V, "[mkv] \\---- [ parsing cues ] -----------\n"); talloc_free(parse_ctx.talloc_ctx); return 0; @@ -1646,7 +1643,6 @@ static void mkv_free(struct demuxer *demuxer) for (int i = 0; i < mkv_d->num_tracks; i++) demux_mkv_free_trackentry(mkv_d->tracks[i]); free(mkv_d->indexes); - free(mkv_d->cluster_positions); } static int demux_mkv_open(demuxer_t *demuxer) @@ -2267,8 +2263,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) if (num == EBML_UINT_INVALID) return 0; mkv_d->cluster_tc = num * mkv_d->tc_scale; - add_cluster_position(mkv_d, mkv_d->cluster_start, - mkv_d->cluster_tc); + add_cluster_position(demuxer, mkv_d->cluster_start, num); break; case MATROSKA_ID_BLOCKGROUP: @@ -2330,27 +2325,29 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) return 0; } -static int seek_creating_index(struct demuxer *demuxer, float rel_seek_secs, - int flags) +static int create_index_until(struct demuxer *demuxer, uint64_t timecode) { struct mkv_demuxer *mkv_d = demuxer->priv; struct stream *s = demuxer->stream; - int64_t target_tc_ns = (int64_t) (rel_seek_secs * 1e9); - if (target_tc_ns < 0) - target_tc_ns = 0; - uint64_t max_filepos = 0; - int64_t max_tc = -1; - int n = mkv_d->num_cluster_pos; - if (n > 0) { - max_filepos = mkv_d->cluster_positions[n - 1].filepos; - max_tc = mkv_d->cluster_positions[n - 1].timecode; - } - if (target_tc_ns > max_tc) { - if ((int64_t) max_filepos > stream_tell(s)) + if (mkv_d->index_complete) + return 0; + + if (mkv_d->index_max_timecode * mkv_d->tc_scale < timecode) { + int64_t cur_filepos = stream_tell(s); + uint64_t max_filepos = 0; + for (int n = 0; n < mkv_d->num_indexes; n++) { + if (mkv_d->indexes[n].timecode == mkv_d->index_max_timecode) { + max_filepos = mkv_d->indexes[n].filepos; + break; + } + } + if ((int64_t) max_filepos > cur_filepos) stream_seek(s, max_filepos); else - stream_seek(s, stream_tell(s) + mkv_d->cluster_size); + stream_seek(s, cur_filepos + mkv_d->cluster_size); + mp_msg(MSGT_DEMUX, MSGL_V, + "[mkv] creating index until TC %" PRIu64 "\n", timecode); /* parse all the clusters upto target_filepos */ while (!s->eof) { uint64_t start = stream_tell(s); @@ -2361,9 +2358,8 @@ static int seek_creating_index(struct demuxer *demuxer, float rel_seek_secs, while (!s->eof && stream_tell(s) < end) { if (ebml_read_id(s, NULL) == MATROSKA_ID_TIMECODE) { uint64_t tc = ebml_read_uint(s, NULL); - tc *= mkv_d->tc_scale; - add_cluster_position(mkv_d, start, tc); - if (tc >= target_tc_ns) + add_cluster_position(demuxer, start, tc); + if (tc * mkv_d->tc_scale >= timecode) goto enough_index; break; } @@ -2374,29 +2370,12 @@ static int seek_creating_index(struct demuxer *demuxer, float rel_seek_secs, stream_seek(s, end); } enough_index: - if (s->eof) - stream_reset(s); + stream_seek(s, cur_filepos); } - if (!mkv_d->num_cluster_pos) { - mp_msg(MSGT_DEMUX, MSGL_V, "[mkv] no target for seek found\n"); + if (!mkv_d->indexes) { + mp_msg(MSGT_DEMUX, MSGL_WARN, "[mkv] no target for seek found\n"); return -1; } - uint64_t cluster_pos = mkv_d->cluster_positions[0].filepos; - /* Let's find the nearest cluster */ - int64_t min_diff = 0xFFFFFFFFFFFFFFF; - for (int i = 0; i < mkv_d->num_cluster_pos; i++) { - int64_t diff = mkv_d->cluster_positions[i].timecode - target_tc_ns; - if (flags & SEEK_BACKWARD && diff < 0 && -diff < min_diff) { - cluster_pos = mkv_d->cluster_positions[i].filepos; - min_diff = -diff; - } else if (flags & SEEK_FORWARD - && (diff < 0 ? -1 * diff : diff) < min_diff) { - cluster_pos = mkv_d->cluster_positions[i].filepos; - min_diff = diff < 0 ? -1 * diff : diff; - } - } - mkv_d->cluster_size = mkv_d->blockgroup_size = 0; - stream_seek(s, cluster_pos); return 0; } @@ -2406,6 +2385,9 @@ static struct mkv_index *seek_with_cues(struct demuxer *demuxer, int seek_id, struct mkv_demuxer *mkv_d = demuxer->priv; struct mkv_index *index = NULL; + if (!mkv_d->index_complete) + seek_id = -1; + /* Find the entry in the index closest to the target timecode in the * give direction. If there are no such entries - we're trying to seek * backward from a target time before the first entry or forward from a @@ -2485,10 +2467,7 @@ static void demux_mkv_seek(demuxer_t *demuxer, float rel_seek_secs, rel_seek_secs = FFMAX(rel_seek_secs, 0); int64_t target_timecode = rel_seek_secs * 1e9 + 0.5; - if (mkv_d->indexes == NULL) { /* no index was found */ - if (seek_creating_index(demuxer, rel_seek_secs, flags) < 0) - return; - } else { + if (create_index_until(demuxer, target_timecode) >= 0) { int seek_id = (demuxer->video->id < 0) ? a_tnum : v_tnum; index = seek_with_cues(demuxer, seek_id, target_timecode, flags); @@ -2514,7 +2493,7 @@ static void demux_mkv_seek(demuxer_t *demuxer, float rel_seek_secs, mkv_index_t *index = NULL; int i; - if (mkv_d->indexes == NULL) { /* not implemented without index */ + if (!mkv_d->index_complete) { /* not implemented without index */ mp_msg(MSGT_DEMUX, MSGL_V, "[mkv] seek unsupported flags\n"); return; } -- cgit v1.2.3 From c2bf06f63e6080503ff19db10ef892252e441156 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 11 Apr 2013 20:31:58 +0200 Subject: demux_mkv: simplify cluster reading code The end positions of the current cluster and block were managed by tracking their size and how much of them were read, instead of just using the absolute end positions. I'm not sure about the reasons why this code was originally written this way. One obvious concern is reading from pipes and such, but the stream layers hides this. stream_tell(s) works even when reading from pipes. It's also a fast call, and doesn't involve the stream implementation or syscalls. Keeping track of the cluster/block end is simpler and there's no reason why this wouldn't work. --- demux/demux_mkv.c | 56 ++++++++++++++++++++++++------------------------------- 1 file changed, 24 insertions(+), 32 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 6dd84b20c9..77abbab85f 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -156,8 +156,8 @@ typedef struct mkv_demuxer { uint64_t tc_scale, cluster_tc; uint64_t cluster_start; - uint64_t cluster_size; - uint64_t blockgroup_size; + uint64_t cluster_end; + uint64_t blockgroup_end; mkv_index_t *indexes; int num_indexes; @@ -2187,19 +2187,17 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) { mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; stream_t *s = demuxer->stream; - uint64_t l; - int il, tmp; while (1) { - while (mkv_d->cluster_size > 0) { + while (stream_tell(s) < mkv_d->cluster_end) { uint64_t block_duration = 0, block_length = 0; bool keyframe = true; uint8_t *block = NULL; - while (mkv_d->blockgroup_size > 0) { - switch (ebml_read_id(s, &il)) { + while (stream_tell(s) < mkv_d->blockgroup_end) { + switch (ebml_read_id(s, NULL)) { case MATROSKA_ID_BLOCKDURATION: - block_duration = ebml_read_uint(s, &l); + block_duration = ebml_read_uint(s, NULL); if (block_duration == EBML_UINT_INVALID) { free(block); return 0; @@ -2208,7 +2206,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) break; case MATROSKA_ID_BLOCK: - block_length = ebml_read_length(s, &tmp); + block_length = ebml_read_length(s, NULL); free(block); if (block_length > 500000000) return 0; @@ -2219,11 +2217,10 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) free(block); return 0; } - l = tmp + block_length; break; case MATROSKA_ID_REFERENCEBLOCK:; - int64_t num = ebml_read_int(s, &l); + int64_t num = ebml_read_int(s, NULL); if (num == EBML_INT_INVALID) { free(block); return 0; @@ -2238,12 +2235,10 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) goto find_next_cluster; default: - if (ebml_read_skip_or_resync_cluster(s, &l) != 0) + if (ebml_read_skip_or_resync_cluster(s, NULL) != 0) goto find_next_cluster; break; } - mkv_d->blockgroup_size -= l + il; - mkv_d->cluster_size -= l + il; } if (block) { @@ -2256,10 +2251,10 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) return 1; } - if (mkv_d->cluster_size > 0) { - switch (ebml_read_id(s, &il)) { + if (stream_tell(s) < mkv_d->cluster_end) { + switch (ebml_read_id(s, NULL)) { case MATROSKA_ID_TIMECODE:; - uint64_t num = ebml_read_uint(s, &l); + uint64_t num = ebml_read_uint(s, NULL); if (num == EBML_UINT_INVALID) return 0; mkv_d->cluster_tc = num * mkv_d->tc_scale; @@ -2267,13 +2262,13 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) break; case MATROSKA_ID_BLOCKGROUP: - mkv_d->blockgroup_size = ebml_read_length(s, &tmp); - l = tmp; + mkv_d->blockgroup_end = ebml_read_length(s, NULL); + mkv_d->blockgroup_end += stream_tell(s); break; case MATROSKA_ID_SIMPLEBLOCK:; int res; - block_length = ebml_read_length(s, &tmp); + block_length = ebml_read_length(s, NULL); if (block_length > 500000000) return 0; block = malloc(block_length); @@ -2283,17 +2278,13 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) free(block); return 0; } - l = tmp + block_length; res = handle_block(demuxer, block, block_length, block_duration, false, true); free(block); - mkv_d->cluster_size -= l + il; if (res < 0) return 0; else if (res) return 1; - else - mkv_d->cluster_size += l + il; break; case EBML_ID_INVALID: @@ -2301,25 +2292,26 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) goto find_next_cluster; default: ; - if (ebml_read_skip_or_resync_cluster(s, &l) != 0) + if (ebml_read_skip_or_resync_cluster(s, NULL) != 0) goto find_next_cluster; break; } - mkv_d->cluster_size -= l + il; } } find_next_cluster: + mkv_d->cluster_end = mkv_d->blockgroup_end = 0; for (;;) { - uint32_t id = ebml_read_id(s, &il); + mkv_d->cluster_start = stream_tell(s); + uint32_t id = ebml_read_id(s, NULL); if (id == MATROSKA_ID_CLUSTER) break; if (s->eof) return 0; ebml_read_skip_or_resync_cluster(s, NULL); } - mkv_d->cluster_start = stream_tell(s) - il; - mkv_d->cluster_size = ebml_read_length(s, NULL); + mkv_d->cluster_end = ebml_read_length(s, NULL); + mkv_d->cluster_end += stream_tell(s); } return 0; @@ -2345,7 +2337,7 @@ static int create_index_until(struct demuxer *demuxer, uint64_t timecode) if ((int64_t) max_filepos > cur_filepos) stream_seek(s, max_filepos); else - stream_seek(s, cur_filepos + mkv_d->cluster_size); + stream_seek(s, mkv_d->cluster_end); mp_msg(MSGT_DEMUX, MSGL_V, "[mkv] creating index until TC %" PRIu64 "\n", timecode); /* parse all the clusters upto target_filepos */ @@ -2430,7 +2422,7 @@ static struct mkv_index *seek_with_cues(struct demuxer *demuxer, int seek_id, seek_pos = prev_target; } - mkv_d->cluster_size = mkv_d->blockgroup_size = 0; + mkv_d->cluster_end = mkv_d->blockgroup_end = 0; stream_seek(demuxer->stream, seek_pos); } return index; @@ -2510,7 +2502,7 @@ static void demux_mkv_seek(demuxer_t *demuxer, float rel_seek_secs, if (!index) return; - mkv_d->cluster_size = mkv_d->blockgroup_size = 0; + mkv_d->cluster_end = mkv_d->blockgroup_end = 0; stream_seek(s, index->filepos); if (demuxer->video->id >= 0) -- cgit v1.2.3 From 75178af8b4dd9f42e29fcad62778679888234ef0 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 11 Apr 2013 23:28:27 +0200 Subject: demux_mkv: fix streaming clusters Matroska files prepared for streaming have clusters with unknown size. These files are pretty rare, see e.g. test4.mkv from the official Matroska test file collection. --- demux/demux_mkv.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 77abbab85f..5f41cdbab6 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -2252,6 +2252,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) } if (stream_tell(s) < mkv_d->cluster_end) { + int64_t start_filepos = stream_tell(s); switch (ebml_read_id(s, NULL)) { case MATROSKA_ID_TIMECODE:; uint64_t num = ebml_read_uint(s, NULL); @@ -2287,6 +2288,10 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) return 1; break; + case MATROSKA_ID_CLUSTER: + mkv_d->cluster_start = start_filepos; + goto next_cluster; + case EBML_ID_INVALID: ebml_resync_cluster(s); goto find_next_cluster; @@ -2310,8 +2315,11 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) return 0; ebml_read_skip_or_resync_cluster(s, NULL); } + next_cluster: mkv_d->cluster_end = ebml_read_length(s, NULL); - mkv_d->cluster_end += stream_tell(s); + // mkv files for "streaming" can have this legally + if (mkv_d->cluster_end != EBML_UINT_INVALID) + mkv_d->cluster_end += stream_tell(s); } return 0; -- cgit v1.2.3 From 4e531d0f2a27f8f4036acb4c3403503f7853e9da Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 11 Apr 2013 21:42:46 +0200 Subject: demux_mkv: factor block reading The code for reading block data was duplicated. Move it into a function. Instead of returning on error (possibly due to corrupt data) and signalling EOF, continue by trying to find the next block. This makes error handling slightly simpler too, because you don't have to care about freeing the current block. We could still signal EOF in this case, but trying to resync sounds better for dealing with corrupted files. --- demux/demux_mkv.c | 116 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 61 insertions(+), 55 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 5f41cdbab6..e2569cae0b 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -2047,9 +2047,36 @@ static void handle_realaudio(demuxer_t *demuxer, mkv_track_t *track, } } -static int handle_block(demuxer_t *demuxer, uint8_t *block, uint64_t length, - uint64_t block_duration, bool keyframe, - bool simpleblock) +struct block_info { + uint64_t length; + uint64_t duration; + bool simple, keyframe; + uint8_t *data; +}; + +static void reset_block(struct block_info *block) +{ + free(block->data); + *block = (struct block_info){0}; + block->keyframe = true; +} + +static int read_block(demuxer_t *demuxer, struct block_info *block) +{ + stream_t *s = demuxer->stream; + + block->length = ebml_read_length(s, NULL); + if (block->length > 500000000) + return -1; + free(block->data); + block->data = malloc(block->length + AV_LZO_INPUT_PADDING); + demuxer->filepos = stream_tell(s); + if (stream_read(s, block->data, block->length) != (int) block->length) + return -1; + return 0; +} + +static int handle_block(demuxer_t *demuxer, struct block_info *block_info) { mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; mkv_track_t *track = NULL; @@ -2061,6 +2088,10 @@ static int handle_block(demuxer_t *demuxer, uint8_t *block, uint64_t length, int i, num, tmp, use_this_block = 1; double current_pts; int16_t time; + uint8_t *block = block_info->data; + uint64_t length = block_info->length; + bool keyframe = block_info->keyframe; + uint64_t block_duration = block_info->duration; /* first byte(s): track num */ num = ebml_read_vlen_uint(block, &tmp); @@ -2071,7 +2102,7 @@ static int handle_block(demuxer_t *demuxer, uint8_t *block, uint64_t length, length -= tmp + 2; old_length = length; flags = block[0]; - if (simpleblock) + if (block_info->simple) keyframe = flags & 0x80; if (demux_mkv_read_block_lacing(block, &length, &laces, &lace_size)) return 0; @@ -2187,51 +2218,36 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) { mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; stream_t *s = demuxer->stream; + struct block_info block = {0}; + reset_block(&block); while (1) { while (stream_tell(s) < mkv_d->cluster_end) { - uint64_t block_duration = 0, block_length = 0; - bool keyframe = true; - uint8_t *block = NULL; + reset_block(&block); while (stream_tell(s) < mkv_d->blockgroup_end) { switch (ebml_read_id(s, NULL)) { case MATROSKA_ID_BLOCKDURATION: - block_duration = ebml_read_uint(s, NULL); - if (block_duration == EBML_UINT_INVALID) { - free(block); - return 0; - } - block_duration *= mkv_d->tc_scale; + block.duration = ebml_read_uint(s, NULL); + if (block.duration == EBML_UINT_INVALID) + goto find_next_cluster; + block.duration *= mkv_d->tc_scale; break; case MATROSKA_ID_BLOCK: - block_length = ebml_read_length(s, NULL); - free(block); - if (block_length > 500000000) - return 0; - block = malloc(block_length + AV_LZO_INPUT_PADDING); - demuxer->filepos = stream_tell(s); - if (stream_read(s, block, block_length) != - (int) block_length) { - free(block); - return 0; - } + if (read_block(demuxer, &block) < 0) + goto find_next_cluster; break; case MATROSKA_ID_REFERENCEBLOCK:; int64_t num = ebml_read_int(s, NULL); - if (num == EBML_INT_INVALID) { - free(block); - return 0; - } + if (num == EBML_INT_INVALID) + goto find_next_cluster; if (num) - keyframe = false; + block.keyframe = false; break; case EBML_ID_INVALID: - free(block); - ebml_resync_cluster(s); goto find_next_cluster; default: @@ -2241,13 +2257,12 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) } } - if (block) { - int res = handle_block(demuxer, block, block_length, - block_duration, keyframe, false); - free(block); + if (block.data) { + int res = handle_block(demuxer, &block); + reset_block(&block); if (res < 0) - return 0; - if (res) + goto find_next_cluster; + if (res > 0) return 1; } @@ -2257,7 +2272,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) case MATROSKA_ID_TIMECODE:; uint64_t num = ebml_read_uint(s, NULL); if (num == EBML_UINT_INVALID) - return 0; + goto find_next_cluster; mkv_d->cluster_tc = num * mkv_d->tc_scale; add_cluster_position(demuxer, mkv_d->cluster_start, num); break; @@ -2268,23 +2283,14 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) break; case MATROSKA_ID_SIMPLEBLOCK:; - int res; - block_length = ebml_read_length(s, NULL); - if (block_length > 500000000) - return 0; - block = malloc(block_length); - demuxer->filepos = stream_tell(s); - if (stream_read(s, block, block_length) != - (int) block_length) { - free(block); - return 0; - } - res = handle_block(demuxer, block, block_length, - block_duration, false, true); - free(block); + block.simple = true; + if (read_block(demuxer, &block) < 0) + goto find_next_cluster; + int res = handle_block(demuxer, &block); + reset_block(&block); if (res < 0) - return 0; - else if (res) + goto find_next_cluster; + if (res > 0) return 1; break; @@ -2293,7 +2299,6 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) goto next_cluster; case EBML_ID_INVALID: - ebml_resync_cluster(s); goto find_next_cluster; default: ; @@ -2305,6 +2310,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) } find_next_cluster: + reset_block(&block); mkv_d->cluster_end = mkv_d->blockgroup_end = 0; for (;;) { mkv_d->cluster_start = stream_tell(s); -- cgit v1.2.3 From 9afe9d7061c948f2be4507e568ce45acf188939e Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 11 Apr 2013 23:33:42 +0200 Subject: demux_mkv: move BlockGroup reading code to a separate function Somehow this was setup such that a BlockGroup can be incrementally read (at least in theory). This makes no sense, as BlockGroup can contain only one Block (despite its name). There's no need to read this incrementally, and makes the code confusing for no gain. Read all the BlockGroup sub-elements with a single function call, without keeping global state for BlockGroup parsing. --- demux/demux_mkv.c | 110 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 61 insertions(+), 49 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index e2569cae0b..acc1849f37 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -157,7 +157,6 @@ typedef struct mkv_demuxer { uint64_t cluster_start; uint64_t cluster_end; - uint64_t blockgroup_end; mkv_index_t *indexes; int num_indexes; @@ -2214,57 +2213,58 @@ static int handle_block(demuxer_t *demuxer, struct block_info *block_info) return 0; } -static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) +static int read_block_group(demuxer_t *demuxer, int64_t end, + struct block_info *block) { mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; stream_t *s = demuxer->stream; - struct block_info block = {0}; - reset_block(&block); + reset_block(block); + + while (stream_tell(s) < end) { + switch (ebml_read_id(s, NULL)) { + case MATROSKA_ID_BLOCKDURATION: + block->duration = ebml_read_uint(s, NULL); + if (block->duration == EBML_UINT_INVALID) + return -1; + block->duration *= mkv_d->tc_scale; + break; - while (1) { - while (stream_tell(s) < mkv_d->cluster_end) { - reset_block(&block); + case MATROSKA_ID_BLOCK: + if (read_block(demuxer, block) < 0) + return -1; + break; - while (stream_tell(s) < mkv_d->blockgroup_end) { - switch (ebml_read_id(s, NULL)) { - case MATROSKA_ID_BLOCKDURATION: - block.duration = ebml_read_uint(s, NULL); - if (block.duration == EBML_UINT_INVALID) - goto find_next_cluster; - block.duration *= mkv_d->tc_scale; - break; + case MATROSKA_ID_REFERENCEBLOCK:; + int64_t num = ebml_read_int(s, NULL); + if (num == EBML_INT_INVALID) + return -1; + if (num) + block->keyframe = false; + break; - case MATROSKA_ID_BLOCK: - if (read_block(demuxer, &block) < 0) - goto find_next_cluster; - break; + case EBML_ID_INVALID: + return -1; - case MATROSKA_ID_REFERENCEBLOCK:; - int64_t num = ebml_read_int(s, NULL); - if (num == EBML_INT_INVALID) - goto find_next_cluster; - if (num) - block.keyframe = false; - break; + default: + if (ebml_read_skip_or_resync_cluster(s, NULL) != 0) + return -1; + break; + } + } - case EBML_ID_INVALID: - goto find_next_cluster; + return 0; +} - default: - if (ebml_read_skip_or_resync_cluster(s, NULL) != 0) - goto find_next_cluster; - break; - } - } +static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) +{ + mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; + stream_t *s = demuxer->stream; + struct block_info block = {0}; + reset_block(&block); - if (block.data) { - int res = handle_block(demuxer, &block); - reset_block(&block); - if (res < 0) - goto find_next_cluster; - if (res > 0) - return 1; - } + while (1) { + while (stream_tell(s) < mkv_d->cluster_end) { + reset_block(&block); if (stream_tell(s) < mkv_d->cluster_end) { int64_t start_filepos = stream_tell(s); @@ -2277,12 +2277,23 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) add_cluster_position(demuxer, mkv_d->cluster_start, num); break; - case MATROSKA_ID_BLOCKGROUP: - mkv_d->blockgroup_end = ebml_read_length(s, NULL); - mkv_d->blockgroup_end += stream_tell(s); + case MATROSKA_ID_BLOCKGROUP: { + int64_t end = ebml_read_length(s, NULL); + end += stream_tell(s); + if (read_block_group(demuxer, end, &block) < 0) + goto find_next_cluster; + if (block.data) { + int res = handle_block(demuxer, &block); + reset_block(&block); + if (res < 0) + goto find_next_cluster; + if (res > 0) + return 1; + } break; + } - case MATROSKA_ID_SIMPLEBLOCK:; + case MATROSKA_ID_SIMPLEBLOCK: { block.simple = true; if (read_block(demuxer, &block) < 0) goto find_next_cluster; @@ -2293,6 +2304,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) if (res > 0) return 1; break; + } case MATROSKA_ID_CLUSTER: mkv_d->cluster_start = start_filepos; @@ -2311,7 +2323,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) find_next_cluster: reset_block(&block); - mkv_d->cluster_end = mkv_d->blockgroup_end = 0; + mkv_d->cluster_end = 0; for (;;) { mkv_d->cluster_start = stream_tell(s); uint32_t id = ebml_read_id(s, NULL); @@ -2436,7 +2448,7 @@ static struct mkv_index *seek_with_cues(struct demuxer *demuxer, int seek_id, seek_pos = prev_target; } - mkv_d->cluster_end = mkv_d->blockgroup_end = 0; + mkv_d->cluster_end = 0; stream_seek(demuxer->stream, seek_pos); } return index; @@ -2516,7 +2528,7 @@ static void demux_mkv_seek(demuxer_t *demuxer, float rel_seek_secs, if (!index) return; - mkv_d->cluster_end = mkv_d->blockgroup_end = 0; + mkv_d->cluster_end = 0; stream_seek(s, index->filepos); if (demuxer->video->id >= 0) -- cgit v1.2.3 From 3fbd6d4e9c2c9dd023fae1d7035d45946fcdb2d4 Mon Sep 17 00:00:00 2001 From: wm4 Date: Fri, 12 Apr 2013 00:43:34 +0200 Subject: demux_mkv: split reading blocks and reading packets Move most code from demux_mkv_fill_buffer() to read_next_block(). The former is supposed to read raw blocks, while ..fill_buffer() reads blocks and turns them into packets. --- demux/demux_mkv.c | 84 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 37 deletions(-) (limited to 'demux/demux_mkv.c') diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index acc1849f37..d2e92befc8 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -2053,26 +2053,29 @@ struct block_info { uint8_t *data; }; -static void reset_block(struct block_info *block) +static void free_block(struct block_info *block) { free(block->data); - *block = (struct block_info){0}; - block->keyframe = true; + block->data = NULL; } static int read_block(demuxer_t *demuxer, struct block_info *block) { stream_t *s = demuxer->stream; + free_block(block); block->length = ebml_read_length(s, NULL); if (block->length > 500000000) - return -1; - free(block->data); + goto error; block->data = malloc(block->length + AV_LZO_INPUT_PADDING); demuxer->filepos = stream_tell(s); if (stream_read(s, block->data, block->length) != (int) block->length) - return -1; - return 0; + goto error; + return 1; + +error: + free_block(block); + return -1; } static int handle_block(demuxer_t *demuxer, struct block_info *block_info) @@ -2218,87 +2221,80 @@ static int read_block_group(demuxer_t *demuxer, int64_t end, { mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; stream_t *s = demuxer->stream; - reset_block(block); + *block = (struct block_info){ .keyframe = true }; while (stream_tell(s) < end) { switch (ebml_read_id(s, NULL)) { case MATROSKA_ID_BLOCKDURATION: block->duration = ebml_read_uint(s, NULL); if (block->duration == EBML_UINT_INVALID) - return -1; + goto error; block->duration *= mkv_d->tc_scale; break; case MATROSKA_ID_BLOCK: if (read_block(demuxer, block) < 0) - return -1; + goto error; break; case MATROSKA_ID_REFERENCEBLOCK:; int64_t num = ebml_read_int(s, NULL); if (num == EBML_INT_INVALID) - return -1; + goto error; if (num) block->keyframe = false; break; case EBML_ID_INVALID: - return -1; + goto error; default: if (ebml_read_skip_or_resync_cluster(s, NULL) != 0) - return -1; + goto error; break; } } - return 0; + return block->data ? 1 : 0; + +error: + free_block(block); + return -1; } -static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) +static int read_next_block(demuxer_t *demuxer, struct block_info *block) { mkv_demuxer_t *mkv_d = (mkv_demuxer_t *) demuxer->priv; stream_t *s = demuxer->stream; - struct block_info block = {0}; - reset_block(&block); while (1) { while (stream_tell(s) < mkv_d->cluster_end) { - reset_block(&block); - if (stream_tell(s) < mkv_d->cluster_end) { int64_t start_filepos = stream_tell(s); switch (ebml_read_id(s, NULL)) { - case MATROSKA_ID_TIMECODE:; + case MATROSKA_ID_TIMECODE: { uint64_t num = ebml_read_uint(s, NULL); if (num == EBML_UINT_INVALID) goto find_next_cluster; mkv_d->cluster_tc = num * mkv_d->tc_scale; add_cluster_position(demuxer, mkv_d->cluster_start, num); break; + } case MATROSKA_ID_BLOCKGROUP: { int64_t end = ebml_read_length(s, NULL); end += stream_tell(s); - if (read_block_group(demuxer, end, &block) < 0) + int res = read_block_group(demuxer, end, block); + if (res < 0) goto find_next_cluster; - if (block.data) { - int res = handle_block(demuxer, &block); - reset_block(&block); - if (res < 0) - goto find_next_cluster; - if (res > 0) - return 1; - } + if (res > 0) + return 1; break; } case MATROSKA_ID_SIMPLEBLOCK: { - block.simple = true; - if (read_block(demuxer, &block) < 0) - goto find_next_cluster; - int res = handle_block(demuxer, &block); - reset_block(&block); + *block = (struct block_info){ .simple = true }; + int res = read_block(demuxer, block); if (res < 0) goto find_next_cluster; if (res > 0) @@ -2322,7 +2318,6 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) } find_next_cluster: - reset_block(&block); mkv_d->cluster_end = 0; for (;;) { mkv_d->cluster_start = stream_tell(s); @@ -2330,7 +2325,7 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) if (id == MATROSKA_ID_CLUSTER) break; if (s->eof) - return 0; + return -1; ebml_read_skip_or_resync_cluster(s, NULL); } next_cluster: @@ -2339,8 +2334,23 @@ static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) if (mkv_d->cluster_end != EBML_UINT_INVALID) mkv_d->cluster_end += stream_tell(s); } +} - return 0; +static int demux_mkv_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) +{ + for (;;) { + int res; + struct block_info block; + res = read_next_block(demuxer, &block); + if (res < 0) + return 0; + if (res > 0) { + res = handle_bl