[FFmpeg-devel] [PATCH] mov: fix decode of fragments that overlap in time

[John Stebbins]

On 10/11/2017 06:25 PM, Michael Niedermayer wrote:
> On Tue, Oct 10, 2017 at 06:11:08PM -0700, John Stebbins wrote:
>> When keyframe intervals of dash segments are not perfectly aligned,
>> fragments in the stream can overlap in time. The previous sorting by
>> timestamp causes packets to be read out of decode order and results
>> in decode errors.
>>
>> Insert new "trun" index entries into index_entries in the order that
>> the trun are referenced by the sidx.
>> ---
>>   libavformat/isom.h |  26 +-
>>   libavformat/mov.c  | 684 ++++++++++++++++++++++++++++++++++++-----------------
>>   2 files changed, 485 insertions(+), 225 deletions(-)
> This still crashes: (not sure i can share the file, tell me if the
> valgrind output is enough)
>
> ==16990== Conditional jump or move depends on uninitialised value(s)
> ==16990==    at 0x6C518B: search_frag_moof_offset (mov.c:1227)
> ==16990==    by 0x6C543D: update_frag_index (mov.c:1318)
> ==16990==    by 0x6D4F77: read_tfra (mov.c:6384)
> ==16990==    by 0x6D51EF: mov_read_mfra (mov.c:6432)
> ==16990==    by 0x6C57F7: mov_read_moof (mov.c:1384)
> ==16990==    by 0x6D38E5: mov_read_default (mov.c:5949)
> ==16990==    by 0x6D5395: mov_read_header (mov.c:6473)
> ==16990==    by 0x7AB6FA: avformat_open_input (utils.c:595)
> ==16990==    by 0x41540A: open_input_file (ffmpeg_opt.c:1060)
> ==16990==    by 0x41F0F9: open_files (ffmpeg_opt.c:3278)
> ==16990==    by 0x41F28B: ffmpeg_parse_options (ffmpeg_opt.c:3318)
> ==16990==    by 0x43D263: main (ffmpeg.c:4794)
> ==16990==
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x1187f060] wav header size < 14 is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented.
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x1187f060] If you want to help, upload a sample of this file to ftp://upload.ffmpeg.org/incoming/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel at ffmpeg.org)
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x1187f060] get_wav_header failed
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x1187f060] error reading header
> ==16990==    at 0x117D019: VALGRIND_PRINTF_BACKTRACE (valgrind.h:4550)
> ==16990==    by 0x117DA89: av_log_default_callback (log.c:355)
> ==16990==    by 0x117DC28: av_vlog (log.c:383)
> ==16990==    by 0x117DBE8: av_log (log.c:375)
> ==16990==    by 0x6D53C2: mov_read_header (mov.c:6474)
> ==16990==    by 0x7AB6FA: avformat_open_input (utils.c:595)
> ==16990==    by 0x41540A: open_input_file (ffmpeg_opt.c:1060)
> ==16990==    by 0x41F0F9: open_files (ffmpeg_opt.c:3278)
> ==16990==    by 0x41F28B: ffmpeg_parse_options (ffmpeg_opt.c:3318)
> ==16990==    by 0x43D263: main (ffmpeg.c:4794)
> ==16990== Conditional jump or move depends on uninitialised value(s)
> ==16990==    at 0x4C2B58C: free (vg_replace_malloc.c:446)
> ==16990==    by 0x11807BD: av_free (mem.c:209)
> ==16990==    by 0x11807F5: av_freep (mem.c:219)
> ==16990==    by 0x6D4CA9: mov_read_close (mov.c:6303)
> ==16990==    by 0x6D53D1: mov_read_header (mov.c:6475)
> ==16990==    by 0x7AB6FA: avformat_open_input (utils.c:595)
> ==16990==    by 0x41540A: open_input_file (ffmpeg_opt.c:1060)
> ==16990==    by 0x41F0F9: open_files (ffmpeg_opt.c:3278)
> ==16990==    by 0x41F28B: ffmpeg_parse_options (ffmpeg_opt.c:3318)
> ==16990==    by 0x43D263: main (ffmpeg.c:4794)
> ==16990==
> test.mp4: Invalid data found when processing input
> ==16990==    at 0x117D019: VALGRIND_PRINTF_BACKTRACE (valgrind.h:4550)
> ==16990==    by 0x117DA89: av_log_default_callback (log.c:355)
> ==16990==    by 0x117DC28: av_vlog (log.c:383)
> ==16990==    by 0x117DBE8: av_log (log.c:375)
> ==16990==    by 0x4274D1: print_error (cmdutils.c:1058)
> ==16990==    by 0x415430: open_input_file (ffmpeg_opt.c:1062)
> ==16990==    by 0x41F0F9: open_files (ffmpeg_opt.c:3278)
> ==16990==    by 0x41F28B: ffmpeg_parse_options (ffmpeg_opt.c:3318)
> ==16990==    by 0x43D263: main (ffmpeg.c:4794)
> ==16990==    at 0x117D019: VALGRIND_PRINTF_BACKTRACE (valgrind.h:4550)
> ==16990==    by 0x117DA89: av_log_default_callback (log.c:355)
> ==16990==    by 0x117DC28: av_vlog (log.c:383)
> ==16990==    by 0x117DBE8: av_log (log.c:375)
> ==16990==    by 0x42B964: term_exit (ffmpeg.c:317)
> ==16990==    by 0x42C64B: ffmpeg_cleanup (ffmpeg.c:618)
> ==16990==    by 0x42483F: exit_program (cmdutils.c:138)
> ==16990==    by 0x415469: open_input_file (ffmpeg_opt.c:1065)
> ==16990==    by 0x41F0F9: open_files (ffmpeg_opt.c:3278)
> ==16990==    by 0x41F28B: ffmpeg_parse_options (ffmpeg_opt.c:3318)
> ==16990==    by 0x43D263: main (ffmpeg.c:4794)
>
>
>
>

Something seems off with the valgrind output.  For example, the first 
warning:

==16990== Conditional jump or move depends on uninitialised value(s)
==16990==    at 0x6C518B: search_frag_moof_offset (mov.c:1227)

Line 1227 is:
     if (!frag_index->nb_items ||

frag_index here is &MOVContext.frag_index (a MOVFragmentIndex) and 
*should* be initialized to zero by the mallocz of priv_data_size in 
avformat_open_input (so initial value of frag_index->nb_items should be 
zero).  If for some reason this *isn't* being initialized to zero, I 
could see why the av_freep in mov_read_close would crash freeing 
unallocated data (line 6303).  But I just don't see how this could be 
uninitialized.

It's not really clear from the valgrind output where the crash is though.