On Fri, Aug 25, 2017 at 5:43 AM, Michael Niedermayer <[email protected] > wrote:
> > This patch breaks: > http://samples.ffmpeg.org/mov/mp4/discont-frag.mp4 > > Hmm, indeed it does. The reason is that we read the sample count from the stsz box and then read the trun box. I don't think this block of code has ever been correct in that case: http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/mov.c;hb=HEAD# l4287 It shifts all the ctts entries incorrectly and even did so prior to my patch. I've uploaded a new version of my fix which simply deletes this block of code. It passes all the fate test cases and those we have in Chrome. Let me know if fails any of your private test cases. - dale
From 049f885ee972b0efb6dcbf456025e56dd627b8b9 Mon Sep 17 00:00:00 2001 From: Dale Curtis <[email protected]> Date: Mon, 31 Jul 2017 13:44:22 -0700 Subject: [PATCH] [mov] Bail when invalid sample data is present. ctts data in ffmpeg relies on the index entries array to be 1:1 with samples... yet sc->sample_count can be read directly from the 'stsz' box and index entries are only generated if a chunk count has been read from 'stco' box. Ensure that if sc->sample_count > 0, sc->chunk_count is too as a basic sanity check. Additionally we need to check that after the index is built we have the right number of entries, so we also check in mov_read_trun() that sc->sample_count == st->nb_index_entries. --- libavformat/mov.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 876f48d912..f8bcba4cd1 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3755,8 +3755,9 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom) c->trak_index = -1; /* sanity checks */ - if (sc->chunk_count && (!sc->stts_count || !sc->stsc_count || - (!sc->sample_size && !sc->sample_count))) { + if ((sc->chunk_count && (!sc->stts_count || !sc->stsc_count || + (!sc->sample_size && !sc->sample_count))) || + (!sc->chunk_count && sc->sample_count)) { av_log(c->fc, AV_LOG_ERROR, "stream %d, missing mandatory atoms, broken header\n", st->index); return 0; @@ -4284,26 +4285,6 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) entries = avio_rb32(pb); av_log(c->fc, AV_LOG_TRACE, "flags 0x%x entries %u\n", flags, entries); - /* Always assume the presence of composition time offsets. - * Without this assumption, for instance, we cannot deal with a track in fragmented movies that meet the following. - * 1) in the initial movie, there are no samples. - * 2) in the first movie fragment, there is only one sample without composition time offset. - * 3) in the subsequent movie fragments, there are samples with composition time offset. */ - if (!sc->ctts_count && sc->sample_count) - { - /* Complement ctts table if moov atom doesn't have ctts atom. */ - ctts_data = av_fast_realloc(NULL, &sc->ctts_allocated_size, sizeof(*sc->ctts_data) * sc->sample_count); - if (!ctts_data) - return AVERROR(ENOMEM); - /* Don't use a count greater than 1 here since it will leave a gap in - * the ctts index which the code below relies on being sequential. */ - sc->ctts_data = ctts_data; - for (i = 0; i < sc->sample_count; i++) { - sc->ctts_data[sc->ctts_count].count = 1; - sc->ctts_data[sc->ctts_count].duration = 0; - sc->ctts_count++; - } - } if ((uint64_t)entries+sc->ctts_count >= UINT_MAX/sizeof(*sc->ctts_data)) return AVERROR_INVALIDDATA; if (flags & MOV_TRUN_DATA_OFFSET) data_offset = avio_rb32(pb); -- 2.14.1.342.g6490525c54-goog
_______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
