> - // Offset the DTS by ctts[0] to make the PTS of the first frame 0 > - if (ctts_data_old && ctts_count_old > 0) { > - edit_list_dts_entry_end -= ctts_data_old[0].duration; > - av_log(mov->fc, AV_LOG_DEBUG, "Offset DTS by ctts[%d].duration: %d\n", 0, ctts_data_old[0].duration); > + av_log(mov->fc, AV_LOG_DEBUG, "Shifting DTS by %d because of negative CTTS.\n", msc->dts_shift); > }
The above lines were the cause of making the first frame PTS of videos starting with B-frames negative. If the videos starts with B frame, then the minimum composition time as computed by stts + ctts will be non-zero. Hence we need to shift the DTS, so that the first pts is zero. This was the intention of that code-block. However it was subtracting by the wrong amount. For example, for one of the videos in the bug nonFormatted.mp4 we have stts: sample_count duration 960 1001 ctts: sample_count duration 1 3003 2 0 1 3003 .... The resulting composition times are : 3003, 1001, 2002, 6006, ... Hence the minimum composition time or PTS is 1001, which should be used to offset DTS. However the code block was wrongly using ctts[0] which is 3003 . Hence the PTS was negative. About the fate test expectations, fate-suite/h264/twofields_packet.mp4 is a similar file starting with 2 Bframes. Before this change the PTS of first two B-frames was -6006 and -3003, and I am guessing one of them got dropped when being decoded and remuxed to the framecrc before, and now it is not being dropped but I dont understand why. On Sat, Jun 3, 2017 at 5:40 AM, wm4 <nfx...@googlemail.com> wrote: > On Fri, 2 Jun 2017 18:17:22 -0700 > Sasi Inguva <isasi-at-google....@ffmpeg.org> wrote: > > > Fixes t/6421 > > > > Signed-off-by: Sasi Inguva <is...@google.com> > > --- > > libavformat/mov.c | 57 ++++++++++++++++++++++++------ > ---- > > tests/ref/fate/h264-twofields-packet | 60 > ++++++++++++++++++------------------ > > 2 files changed, 70 insertions(+), 47 deletions(-) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index 3845e63b53..d7d64c3361 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -3039,6 +3039,9 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > > int64_t edit_list_dts_entry_end = 0; > > int64_t edit_list_start_ctts_sample = 0; > > int64_t curr_cts; > > + int64_t curr_ctts = 0; > > + int64_t min_corrected_pts = -1; > > + int64_t empty_edits_sum_duration = 0; > > int64_t edit_list_index = 0; > > int64_t index; > > int64_t index_ctts_count; > > @@ -3053,6 +3056,7 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > > int first_non_zero_audio_edit = -1; > > int packet_skip_samples = 0; > > MOVIndexRange *current_index_range; > > + int i; > > > > if (!msc->elst_data || msc->elst_count <= 0 || nb_old <= 0) { > > return; > > @@ -3080,13 +3084,9 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > > > > // If the dts_shift is positive (in case of negative ctts values in > mov), > > // then negate the DTS by dts_shift > > - if (msc->dts_shift > 0) > > + if (msc->dts_shift > 0) { > > edit_list_dts_entry_end -= msc->dts_shift; > > - > > - // Offset the DTS by ctts[0] to make the PTS of the first frame 0 > > - if (ctts_data_old && ctts_count_old > 0) { > > - edit_list_dts_entry_end -= ctts_data_old[0].duration; > > - av_log(mov->fc, AV_LOG_DEBUG, "Offset DTS by ctts[%d].duration: > %d\n", 0, ctts_data_old[0].duration); > > + av_log(mov->fc, AV_LOG_DEBUG, "Shifting DTS by %d because of > negative CTTS.\n", msc->dts_shift); > > } > > > > start_dts = edit_list_dts_entry_end; > > @@ -3100,6 +3100,7 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > > edit_list_dts_entry_end += edit_list_duration; > > num_discarded_begin = 0; > > if (edit_list_media_time == -1) { > > + empty_edits_sum_duration += edit_list_duration; > > continue; > > } > > > > @@ -3179,11 +3180,13 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > > > > // frames (pts) before or after edit list > > curr_cts = current->timestamp + msc->dts_shift; > > + curr_ctts = 0; > > > > if (ctts_data_old && ctts_index_old < ctts_count_old) { > > - av_log(mov->fc, AV_LOG_DEBUG, "shifted frame pts, > curr_cts: %"PRId64" @ %"PRId64", ctts: %d, ctts_count: %"PRId64"\n", > > - curr_cts, ctts_index_old, > ctts_data_old[ctts_index_old].duration, ctts_count_old); > > - curr_cts += ctts_data_old[ctts_index_old].duration; > > + curr_ctts = ctts_data_old[ctts_index_old].duration; > > + av_log(mov->fc, AV_LOG_DEBUG, "stts: %"PRId64" ctts: > %"PRId64", ctts_index: %"PRId64", ctts_count: %"PRId64"\n", > > + curr_cts, curr_ctts, ctts_index_old, > ctts_count_old); > > + curr_cts += curr_ctts; > > ctts_sample_old++; > > if (ctts_sample_old == ctts_data_old[ctts_index_old].count) > { > > if (add_ctts_entry(&msc->ctts_data, > &msc->ctts_count, > > @@ -3244,14 +3247,21 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > > } > > } > > } > > - } else if (edit_list_start_encountered == 0) { > > - edit_list_start_encountered = 1; > > - // Make timestamps strictly monotonically increasing > for audio, by rewriting timestamps for > > - // discarded packets. > > - if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && > frame_duration_buffer) { > > - fix_index_entry_timestamps(st, > st->nb_index_entries, edit_list_dts_counter, > > - frame_duration_buffer, > num_discarded_begin); > > - av_freep(&frame_duration_buffer); > > + } else { > > + if (min_corrected_pts < 0) { > > + min_corrected_pts = edit_list_dts_counter + > curr_ctts + msc->dts_shift; > > + } else { > > + min_corrected_pts = FFMIN(min_corrected_pts, > edit_list_dts_counter + curr_ctts + msc->dts_shift); > > + } > > + if (edit_list_start_encountered == 0) { > > + edit_list_start_encountered = 1; > > + // Make timestamps strictly monotonically > increasing for audio, by rewriting timestamps for > > + // discarded packets. > > + if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO > && frame_duration_buffer) { > > + fix_index_entry_timestamps(st, > st->nb_index_entries, edit_list_dts_counter, > > + > frame_duration_buffer, num_discarded_begin); > > + av_freep(&frame_duration_buffer); > > + } > > } > > } > > > > @@ -3292,6 +3302,19 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > > } > > } > > } > > + // If there are empty edits, then min_corrected_pts might be > positive intentionally. So we subtract the > > + // sum duration of emtpy edits here. > > + min_corrected_pts -= empty_edits_sum_duration; > > + > > + // If the minimum pts turns out to be greater than zero after > fixing the index, then we subtract the > > + // dts by that amount to make the first pts zero. > > + if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && > min_corrected_pts > 0) { > > + av_log(mov->fc, AV_LOG_DEBUG, "Offset DTS by %"PRId64" to make > first pts zero.\n", min_corrected_pts); > > + for (i = 0; i < st->nb_index_entries; ++i) { > > + st->index_entries[i].timestamp -= min_corrected_pts; > > + } > > + } > > + > > // Update av stream length > > st->duration = edit_list_dts_entry_end - start_dts; > > > > diff --git a/tests/ref/fate/h264-twofields-packet b/tests/ref/fate/h264- > twofields-packet > > index 4cff0a15bd..db12498f15 100644 > > --- a/tests/ref/fate/h264-twofields-packet > > +++ b/tests/ref/fate/h264-twofields-packet > > @@ -3,33 +3,33 @@ > > #codec_id 0: rawvideo > > #dimensions 0: 1920x1080 > > #sar 0: 1/1 > > -0, 0, 0, 1, 3110400, 0x40d65f69 > > -0, 1, 1, 1, 3110400, 0xdcbc50bf > > -0, 2, 2, 1, 3110400, 0x73a2276a > > -0, 3, 3, 1, 3110400, 0x84a2b3c6 > > -0, 4, 4, 1, 3110400, 0x7cf3b570 > > -0, 5, 5, 1, 3110400, 0xa2d1e03a > > -0, 6, 6, 1, 3110400, 0x03220fb1 > > -0, 7, 7, 1, 3110400, 0x89cd526a > > -0, 8, 8, 1, 3110400, 0xbb4b7531 > > -0, 9, 9, 1, 3110400, 0x0a69f053 > > -0, 10, 10, 1, 3110400, 0x0187994b > > -0, 11, 11, 1, 3110400, 0x26ed49fa > > -0, 12, 12, 1, 3110400, 0xbe8966d4 > > -0, 13, 13, 1, 3110400, 0x248d203c > > -0, 14, 14, 1, 3110400, 0x3139c754 > > -0, 15, 15, 1, 3110400, 0xf22380c4 > > -0, 16, 16, 1, 3110400, 0x3e00dcc1 > > -0, 17, 17, 1, 3110400, 0x8cbe2483 > > -0, 18, 18, 1, 3110400, 0x6951cd63 > > -0, 19, 19, 1, 3110400, 0x36aca4c5 > > -0, 20, 20, 1, 3110400, 0x4d4f6fbe > > -0, 21, 21, 1, 3110400, 0x997247aa > > -0, 22, 22, 1, 3110400, 0x0fd40e06 > > -0, 23, 23, 1, 3110400, 0xa10d2d67 > > -0, 24, 24, 1, 3110400, 0x87c481da > > -0, 25, 25, 1, 3110400, 0xe3dca3cd > > -0, 26, 26, 1, 3110400, 0x5f77b078 > > -0, 27, 27, 1, 3110400, 0xf1ddd098 > > -0, 28, 28, 1, 3110400, 0xedcd1754 > > -0, 29, 29, 1, 3110400, 0x14ac7153 > > +0, 0, 0, 1, 3110400, 0x48e39acd > > +0, 1, 1, 1, 3110400, 0x40d65f69 > > +0, 2, 2, 1, 3110400, 0xdcbc50bf > > +0, 3, 3, 1, 3110400, 0x73a2276a > > +0, 4, 4, 1, 3110400, 0x84a2b3c6 > > +0, 5, 5, 1, 3110400, 0x7cf3b570 > > +0, 6, 6, 1, 3110400, 0xa2d1e03a > > +0, 7, 7, 1, 3110400, 0x03220fb1 > > +0, 8, 8, 1, 3110400, 0x89cd526a > > +0, 9, 9, 1, 3110400, 0xbb4b7531 > > +0, 10, 10, 1, 3110400, 0x0a69f053 > > +0, 11, 11, 1, 3110400, 0x0187994b > > +0, 12, 12, 1, 3110400, 0x26ed49fa > > +0, 13, 13, 1, 3110400, 0xbe8966d4 > > +0, 14, 14, 1, 3110400, 0x248d203c > > +0, 15, 15, 1, 3110400, 0x3139c754 > > +0, 16, 16, 1, 3110400, 0xf22380c4 > > +0, 17, 17, 1, 3110400, 0x3e00dcc1 > > +0, 18, 18, 1, 3110400, 0x8cbe2483 > > +0, 19, 19, 1, 3110400, 0x6951cd63 > > +0, 20, 20, 1, 3110400, 0x36aca4c5 > > +0, 21, 21, 1, 3110400, 0x4d4f6fbe > > +0, 22, 22, 1, 3110400, 0x997247aa > > +0, 23, 23, 1, 3110400, 0x0fd40e06 > > +0, 24, 24, 1, 3110400, 0xa10d2d67 > > +0, 25, 25, 1, 3110400, 0x87c481da > > +0, 26, 26, 1, 3110400, 0xe3dca3cd > > +0, 27, 27, 1, 3110400, 0x5f77b078 > > +0, 28, 28, 1, 3110400, 0xf1ddd098 > > +0, 29, 29, 1, 3110400, 0xedcd1754 > > Thanks for the fix, but is there any explanation why this happens? And > why does the FATE test change? > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel