| Message ID | 20220618200303.17054-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avformat/mov: Avoid cloning encryption info if its unchanged | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Sat, 18 Jun 2022, Michael Niedermayer wrote: > This extra code is ugly, better solution is welcome If you work on fixing these issues, it is kind of your job to find an elegant and maintainable solution. If you can't find one with reasonable amount of work, then IMHO it is better to leave the timeout issue in the code. Regards, Marton > > Fixes: Timeout > Fixes: 45700/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6141847792123904 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/mov.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index c93e13c8cd..3d9e866d4e 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -5092,6 +5092,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) > if (index_entry_pos > 0) > prev_dts = sti->index_entries[index_entry_pos-1].timestamp; > > + if (flags & 0xF00) { > for (i = 0; i < entries && !pb->eof_reached; i++) { > unsigned sample_size = frag->size; > int sample_flags = i ? frag->flags : first_sample_flags; > @@ -5166,6 +5167,74 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) > sc->nb_frames_for_fps ++; > } > } > + } else { > + unsigned sample_size = frag->size; > + unsigned sample_duration = frag->duration; > + > + if (pts != AV_NOPTS_VALUE) { > + dts = pts - sc->dts_shift - sc->time_offset; > + av_log(c->fc, AV_LOG_DEBUG, > + "pts %"PRId64" calculated dts %"PRId64 > + " sc->dts_shift %d ctts.duration %d" > + " sc->time_offset %"PRId64 > + " flags & MOV_TRUN_SAMPLE_CTS %d\n", > + pts, dts, > + sc->dts_shift, 0, > + sc->time_offset, 0); > + } > + > + if (av_sat_add64(dts, sample_duration * entries) != dts + (uint64_t)sample_duration * entries) > + return AVERROR_INVALIDDATA; > + > + for (i = 0; i < entries && !pb->eof_reached; i++) { > + int sample_flags = i ? frag->flags : first_sample_flags; > + int keyframe = 0; > + int index_entry_flags = 0; > + > + if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) > + keyframe = 1; > + else > + keyframe = > + !(sample_flags & (MOV_FRAG_SAMPLE_FLAG_IS_NON_SYNC | > + MOV_FRAG_SAMPLE_FLAG_DEPENDS_YES)); > + if (keyframe) { > + distance = 0; > + index_entry_flags |= AVINDEX_KEYFRAME; > + } > + // Fragments can overlap in time. Discard overlapping frames after > + // decoding. > + if (prev_dts >= dts) > + index_entry_flags |= AVINDEX_DISCARD_FRAME; > + > + sti->index_entries[index_entry_pos].pos = offset; > + sti->index_entries[index_entry_pos].timestamp = dts; > + sti->index_entries[index_entry_pos].size = sample_size; > + sti->index_entries[index_entry_pos].min_distance = distance; > + sti->index_entries[index_entry_pos].flags = index_entry_flags; > + > + sc->ctts_data[index_entry_pos].count = 1; > + sc->ctts_data[index_entry_pos].duration = 0; > + index_entry_pos++; > + > + av_log(c->fc, AV_LOG_TRACE, "AVIndex stream %d, sample %d, offset %"PRIx64", dts %"PRId64", " > + "size %u, distance %d, keyframe %d\n", st->index, > + index_entry_pos, offset, dts, sample_size, distance, keyframe); > + distance++; > + dts += sample_duration; > + offset += sample_size; > + sc->data_size += sample_size; > + > + if (sample_duration <= INT64_MAX - sc->duration_for_fps && > + 1 <= INT_MAX - sc->nb_frames_for_fps > + ) { > + sc->duration_for_fps += sample_duration; > + sc->nb_frames_for_fps ++; > + } > + } > + > + } > + > + > if (frag_stream_info) > frag_stream_info->next_trun_dts = dts + sc->time_offset; > if (i < entries) { > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Sun, Jun 19, 2022 at 12:17:54AM +0200, Marton Balint wrote: > > > On Sat, 18 Jun 2022, Michael Niedermayer wrote: > > > This extra code is ugly, better solution is welcome > > If you work on fixing these issues, it is kind of your job to find an > elegant and maintainable solution. If you can't find one with reasonable > amount of work, then IMHO it is better to leave the timeout issue in the > code. yes, you are correct still such patches should be posted to the mailing list thx [...]
diff --git a/libavformat/mov.c b/libavformat/mov.c index c93e13c8cd..3d9e866d4e 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -5092,6 +5092,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) if (index_entry_pos > 0) prev_dts = sti->index_entries[index_entry_pos-1].timestamp; + if (flags & 0xF00) { for (i = 0; i < entries && !pb->eof_reached; i++) { unsigned sample_size = frag->size; int sample_flags = i ? frag->flags : first_sample_flags; @@ -5166,6 +5167,74 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->nb_frames_for_fps ++; } } + } else { + unsigned sample_size = frag->size; + unsigned sample_duration = frag->duration; + + if (pts != AV_NOPTS_VALUE) { + dts = pts - sc->dts_shift - sc->time_offset; + av_log(c->fc, AV_LOG_DEBUG, + "pts %"PRId64" calculated dts %"PRId64 + " sc->dts_shift %d ctts.duration %d" + " sc->time_offset %"PRId64 + " flags & MOV_TRUN_SAMPLE_CTS %d\n", + pts, dts, + sc->dts_shift, 0, + sc->time_offset, 0); + } + + if (av_sat_add64(dts, sample_duration * entries) != dts + (uint64_t)sample_duration * entries) + return AVERROR_INVALIDDATA; + + for (i = 0; i < entries && !pb->eof_reached; i++) { + int sample_flags = i ? frag->flags : first_sample_flags; + int keyframe = 0; + int index_entry_flags = 0; + + if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) + keyframe = 1; + else + keyframe = + !(sample_flags & (MOV_FRAG_SAMPLE_FLAG_IS_NON_SYNC | + MOV_FRAG_SAMPLE_FLAG_DEPENDS_YES)); + if (keyframe) { + distance = 0; + index_entry_flags |= AVINDEX_KEYFRAME; + } + // Fragments can overlap in time. Discard overlapping frames after + // decoding. + if (prev_dts >= dts) + index_entry_flags |= AVINDEX_DISCARD_FRAME; + + sti->index_entries[index_entry_pos].pos = offset; + sti->index_entries[index_entry_pos].timestamp = dts; + sti->index_entries[index_entry_pos].size = sample_size; + sti->index_entries[index_entry_pos].min_distance = distance; + sti->index_entries[index_entry_pos].flags = index_entry_flags; + + sc->ctts_data[index_entry_pos].count = 1; + sc->ctts_data[index_entry_pos].duration = 0; + index_entry_pos++; + + av_log(c->fc, AV_LOG_TRACE, "AVIndex stream %d, sample %d, offset %"PRIx64", dts %"PRId64", " + "size %u, distance %d, keyframe %d\n", st->index, + index_entry_pos, offset, dts, sample_size, distance, keyframe); + distance++; + dts += sample_duration; + offset += sample_size; + sc->data_size += sample_size; + + if (sample_duration <= INT64_MAX - sc->duration_for_fps && + 1 <= INT_MAX - sc->nb_frames_for_fps + ) { + sc->duration_for_fps += sample_duration; + sc->nb_frames_for_fps ++; + } + } + + } + + if (frag_stream_info) frag_stream_info->next_trun_dts = dts + sc->time_offset; if (i < entries) {
This extra code is ugly, better solution is welcome Fixes: Timeout Fixes: 45700/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6141847792123904 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/mov.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)