Message ID | 20171031234256.29076-1-isasi@google.com |
---|---|
State | Superseded |
Headers | show |
Attaching the fate sample. On Tue, Oct 31, 2017 at 4:42 PM, Sasi Inguva <isasi@google.com> wrote: > Partially fixes t/6699. > --- > libavformat/mov.c | 125 ++++++++++++++++++++++++++++++ > +++++------------------ > tests/fate/mov.mak | 8 ++++ > 2 files changed, 90 insertions(+), 43 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 60f0228e2d..a295445651 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -3014,34 +3014,95 @@ static int get_edit_list_entry(MOVContext *mov, > } > > /** > - * Find the closest previous frame to the timestamp, in e_old index > + * Find the closest previous frame to the timestamp_pts, in e_old index > * entries. Searching for just any frame / just key frames can be > controlled by > * last argument 'flag'. > - * Returns the index of the entry in st->index_entries if successful, > - * else returns -1. > + * Here the timestamp_pts is considered to be a presentation timestamp and > + * the timestamp of index entries are considered to be decoding > timestamps. > + * > + * Returns 0 if successful in finding a frame, else returns -1. > + * Places the found index corresponding output arg. > + * > + * If ctts_old is not NULL, then refines the searched entry by searching > + * backwards from the found timestamp, to find the frame with correct PTS. > + * > + * Places the found ctts_index and ctts_sample in corresponding output > args. > */ > -static int64_t find_prev_closest_index(AVStream *st, > - AVIndexEntry *e_old, > - int nb_old, > - int64_t timestamp, > - int flag) > +static int find_prev_closest_index(AVStream *st, > + AVIndexEntry *e_old, > + int nb_old, > + MOVStts* ctts_data, > + int64_t ctts_count, > + int64_t timestamp_pts, > + int flag, > + int64_t* index, > + int64_t* ctts_index, > + int64_t* ctts_sample) > { > + MOVStreamContext *msc = st->priv_data; > AVIndexEntry *e_keep = st->index_entries; > int nb_keep = st->nb_index_entries; > - int64_t found = -1; > int64_t i = 0; > + int64_t index_ctts_count; > + > + av_assert0(index); > + > + // If dts_shift > 0, then all the index timestamps will have to be > offset by > + // at least dts_shift amount to obtain PTS. > + // Hence we decrement the searched timestamp_pts by dts_shift to find > the closest index element. > + if (msc->dts_shift > 0) { > + timestamp_pts -= msc->dts_shift; > + } > > st->index_entries = e_old; > st->nb_index_entries = nb_old; > - found = av_index_search_timestamp(st, timestamp, flag | > AVSEEK_FLAG_BACKWARD); > + *index = av_index_search_timestamp(st, timestamp_pts, flag | > AVSEEK_FLAG_BACKWARD); > > // Keep going backwards in the index entries until the timestamp is > the same. > - if (found >= 0) { > - for (i = found; i > 0 && e_old[i].timestamp == e_old[i - > 1].timestamp; > + if (*index >= 0) { > + for (i = *index; i > 0 && e_old[i].timestamp == e_old[i - > 1].timestamp; > i--) { > if ((flag & AVSEEK_FLAG_ANY) || > (e_old[i - 1].flags & AVINDEX_KEYFRAME)) { > - found = i - 1; > + *index = i - 1; > + } > + } > + } > + > + // If we have CTTS then refine the search, by searching backwards > over PTS > + // computed by adding corresponding CTTS durations to index > timestamps. > + if (ctts_data && *index >= 0) { > + av_assert0(ctts_index); > + av_assert0(ctts_sample); > + // Find out the ctts_index for the found frame. > + *ctts_index = 0; > + *ctts_sample = 0; > + for (index_ctts_count = 0; index_ctts_count < *index; > index_ctts_count++) { > + if (*ctts_index < ctts_count) { > + (*ctts_sample)++; > + if (ctts_data[*ctts_index].count == *ctts_sample) { > + (*ctts_index)++; > + *ctts_sample = 0; > + } > + } > + } > + > + while (*index >= 0 && (*ctts_index) >= 0) { > + // Find a "key frame" with PTS <= timestamp_pts (So that we > can decode B-frames correctly). > + // No need to add dts_shift to the timestamp here becase > timestamp_pts has already been > + // compensated by dts_shift above. > + if ((e_old[*index].timestamp + ctts_data[*ctts_index].duration) > <= timestamp_pts && > + (e_old[*index].flags & AVINDEX_KEYFRAME)) { > + break; > + } > + > + (*index)--; > + if (*ctts_sample == 0) { > + (*ctts_index)--; > + if (*ctts_index >= 0) > + *ctts_sample = ctts_data[*ctts_index].count - 1; > + } else { > + (*ctts_sample)--; > } > } > } > @@ -3049,7 +3110,7 @@ static int64_t find_prev_closest_index(AVStream *st, > /* restore AVStream state*/ > st->index_entries = e_keep; > st->nb_index_entries = nb_keep; > - return found; > + return *index >= 0 ? 0 : -1; > } > > /** > @@ -3220,10 +3281,8 @@ static void mov_fix_index(MOVContext *mov, AVStream > *st) > int64_t empty_edits_sum_duration = 0; > int64_t edit_list_index = 0; > int64_t index; > - int64_t index_ctts_count; > int flags; > int64_t start_dts = 0; > - int64_t edit_list_media_time_dts = 0; > int64_t edit_list_start_encountered = 0; > int64_t search_timestamp = 0; > int64_t* frame_duration_buffer = NULL; > @@ -3293,17 +3352,11 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > st->skip_samples = msc->start_pad = 0; > } > > - //find closest previous key frame > - edit_list_media_time_dts = edit_list_media_time; > - if (msc->dts_shift > 0) { > - edit_list_media_time_dts -= msc->dts_shift; > - } > - > // While reordering frame index according to edit list we must > handle properly > // the scenario when edit list entry starts from none key frame. > // We find closest previous key frame and preserve it and > consequent frames in index. > // All frames which are outside edit list entry time boundaries > will be dropped after decoding. > - search_timestamp = edit_list_media_time_dts; > + search_timestamp = edit_list_media_time; > if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { > // Audio decoders like AAC need need a decoder delay samples > previous to the current sample, > // to correctly decode this frame. Hence for audio we seek to > a frame 1 sec. before the > @@ -3311,38 +3364,24 @@ static void mov_fix_index(MOVContext *mov, > AVStream *st) > search_timestamp = FFMAX(search_timestamp - msc->time_scale, > e_old[0].timestamp); > } > > - index = find_prev_closest_index(st, e_old, nb_old, > search_timestamp, 0); > - if (index == -1) { > + if (find_prev_closest_index(st, e_old, nb_old, ctts_data_old, > ctts_count_old, search_timestamp, 0, > + &index, &ctts_index_old, > &ctts_sample_old) < 0) { > av_log(mov->fc, AV_LOG_WARNING, > "st: %d edit list: %"PRId64" Missing key frame while > searching for timestamp: %"PRId64"\n", > st->index, edit_list_index, search_timestamp); > - index = find_prev_closest_index(st, e_old, nb_old, > search_timestamp, AVSEEK_FLAG_ANY); > - > - if (index == -1) { > + if (find_prev_closest_index(st, e_old, nb_old, ctts_data_old, > ctts_count_old, search_timestamp, AVSEEK_FLAG_ANY, > + &index, &ctts_index_old, > &ctts_sample_old) < 0) { > av_log(mov->fc, AV_LOG_WARNING, > "st: %d edit list %"PRId64" Cannot find an index > entry before timestamp: %"PRId64".\n" > "Rounding edit list media time to zero.\n", > st->index, edit_list_index, search_timestamp); > index = 0; > + ctts_index_old = 0; > + ctts_sample_old = 0; > edit_list_media_time = 0; > } > } > current = e_old + index; > - > - ctts_index_old = 0; > - ctts_sample_old = 0; > - > - // set ctts_index properly for the found key frame > - for (index_ctts_count = 0; index_ctts_count < index; > index_ctts_count++) { > - if (ctts_data_old && ctts_index_old < ctts_count_old) { > - ctts_sample_old++; > - if (ctts_data_old[ctts_index_old].count == > ctts_sample_old) { > - ctts_index_old++; > - ctts_sample_old = 0; > - } > - } > - } > - > edit_list_start_ctts_sample = ctts_sample_old; > > // Iterate over index and arrange it according to edit list > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak > index 6815e4feca..01893a0767 100644 > --- a/tests/fate/mov.mak > +++ b/tests/fate/mov.mak > @@ -9,6 +9,7 @@ FATE_MOV = fate-mov-3elist \ > fate-mov-invalid-elst-entry-count \ > fate-mov-gpmf-remux \ > fate-mov-440hz-10ms \ > + fate-mov-ibi-elst-starts-b \ > > FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \ > fate-mov-zombie \ > @@ -47,6 +48,13 @@ fate-mov-440hz-10ms: CMD = framemd5 -i > $(TARGET_SAMPLES)/mov/440hz-10ms.m4a > # Makes sure that we handle invalid edit list entry count correctly. > fate-mov-invalid-elst-entry-count: CMD = framemd5 -flags +bitexact -i > $(TARGET_SAMPLES)/mov/invalid_elst_entry_count.mov > > +# Makes sure that 1st key-frame is picked when, > +# i) One B-frame between 2 key-frames > +# ii) Edit list starts on B-frame. > +# iii) Both key-frames have their DTS < edit list start > +# i.e. Pts Order: I-B-I > +fate-mov-ibi-elst-starts-b: CMD = framemd5 -flags +bitexact -i > $(TARGET_SAMPLES)/mov/mov_ibi_elst_starts_b.mov > + > fate-mov-aac-2048-priming: CMD = run ffprobe$(PROGSSUF)$(EXESUF) > -show_packets -print_format compact $(TARGET_SAMPLES)/mov/aac- > 2048-priming.mov > > fate-mov-zombie: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_streams > -show_packets -show_frames -bitexact -print_format compact > $(TARGET_SAMPLES)/mov/white_zombie_scrunch-part.mov > -- > 2.15.0.403.gc27cc4dac6-goog > >
On Tue, Oct 31, 2017 at 04:42:56PM -0700, Sasi Inguva wrote: > Partially fixes t/6699. > --- > libavformat/mov.c | 125 +++++++++++++++++++++++++++++++++++------------------ > tests/fate/mov.mak | 8 ++++ > 2 files changed, 90 insertions(+), 43 deletions(-) Missing: ./tests/ref/fate/mov-ibi-elst-starts-b [...]
Added the fate ref file to the commit. Thx.
Thanks. Sent the corrected patch. On Tue, Oct 31, 2017 at 7:14 PM, Michael Niedermayer <michael@niedermayer.cc > wrote: > On Tue, Oct 31, 2017 at 04:42:56PM -0700, Sasi Inguva wrote: > > Partially fixes t/6699. > > --- > > libavformat/mov.c | 125 ++++++++++++++++++++++++++++++ > +++++------------------ > > tests/fate/mov.mak | 8 ++++ > > 2 files changed, 90 insertions(+), 43 deletions(-) > > Missing: ./tests/ref/fate/mov-ibi-elst-starts-b > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Awnsering whenever a program halts or runs forever is > On a turing machine, in general impossible (turings halting problem). > On any real computer, always possible as a real computer has a finite > number > of states N, and will either halt in less than N cycles or never halt. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
On Tue, Oct 31, 2017 at 04:45:00PM -0700, Sasi Inguva wrote:
> Attaching the fate sample.
uploaded
[...]
Modified the comment for find_prev_index function. Thx. On Thu, Nov 2, 2017 at 4:22 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Oct 31, 2017 at 04:45:00PM -0700, Sasi Inguva wrote: > > Attaching the fate sample. > > uploaded > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > He who knows, does not speak. He who speaks, does not know. -- Lao Tsu > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
diff --git a/libavformat/mov.c b/libavformat/mov.c index 60f0228e2d..a295445651 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3014,34 +3014,95 @@ static int get_edit_list_entry(MOVContext *mov, } /** - * Find the closest previous frame to the timestamp, in e_old index + * Find the closest previous frame to the timestamp_pts, in e_old index * entries. Searching for just any frame / just key frames can be controlled by * last argument 'flag'. - * Returns the index of the entry in st->index_entries if successful, - * else returns -1. + * Here the timestamp_pts is considered to be a presentation timestamp and + * the timestamp of index entries are considered to be decoding timestamps. + * + * Returns 0 if successful in finding a frame, else returns -1. + * Places the found index corresponding output arg. + * + * If ctts_old is not NULL, then refines the searched entry by searching + * backwards from the found timestamp, to find the frame with correct PTS. + * + * Places the found ctts_index and ctts_sample in corresponding output args. */ -static int64_t find_prev_closest_index(AVStream *st, - AVIndexEntry *e_old, - int nb_old, - int64_t timestamp, - int flag) +static int find_prev_closest_index(AVStream *st, + AVIndexEntry *e_old, + int nb_old, + MOVStts* ctts_data, + int64_t ctts_count, + int64_t timestamp_pts, + int flag, + int64_t* index, + int64_t* ctts_index, + int64_t* ctts_sample) { + MOVStreamContext *msc = st->priv_data; AVIndexEntry *e_keep = st->index_entries; int nb_keep = st->nb_index_entries; - int64_t found = -1; int64_t i = 0; + int64_t index_ctts_count; + + av_assert0(index); + + // If dts_shift > 0, then all the index timestamps will have to be offset by + // at least dts_shift amount to obtain PTS. + // Hence we decrement the searched timestamp_pts by dts_shift to find the closest index element. + if (msc->dts_shift > 0) { + timestamp_pts -= msc->dts_shift; + } st->index_entries = e_old; st->nb_index_entries = nb_old; - found = av_index_search_timestamp(st, timestamp, flag | AVSEEK_FLAG_BACKWARD); + *index = av_index_search_timestamp(st, timestamp_pts, flag | AVSEEK_FLAG_BACKWARD); // Keep going backwards in the index entries until the timestamp is the same. - if (found >= 0) { - for (i = found; i > 0 && e_old[i].timestamp == e_old[i - 1].timestamp; + if (*index >= 0) { + for (i = *index; i > 0 && e_old[i].timestamp == e_old[i - 1].timestamp; i--) { if ((flag & AVSEEK_FLAG_ANY) || (e_old[i - 1].flags & AVINDEX_KEYFRAME)) { - found = i - 1; + *index = i - 1; + } + } + } + + // If we have CTTS then refine the search, by searching backwards over PTS + // computed by adding corresponding CTTS durations to index timestamps. + if (ctts_data && *index >= 0) { + av_assert0(ctts_index); + av_assert0(ctts_sample); + // Find out the ctts_index for the found frame. + *ctts_index = 0; + *ctts_sample = 0; + for (index_ctts_count = 0; index_ctts_count < *index; index_ctts_count++) { + if (*ctts_index < ctts_count) { + (*ctts_sample)++; + if (ctts_data[*ctts_index].count == *ctts_sample) { + (*ctts_index)++; + *ctts_sample = 0; + } + } + } + + while (*index >= 0 && (*ctts_index) >= 0) { + // Find a "key frame" with PTS <= timestamp_pts (So that we can decode B-frames correctly). + // No need to add dts_shift to the timestamp here becase timestamp_pts has already been + // compensated by dts_shift above. + if ((e_old[*index].timestamp + ctts_data[*ctts_index].duration) <= timestamp_pts && + (e_old[*index].flags & AVINDEX_KEYFRAME)) { + break; + } + + (*index)--; + if (*ctts_sample == 0) { + (*ctts_index)--; + if (*ctts_index >= 0) + *ctts_sample = ctts_data[*ctts_index].count - 1; + } else { + (*ctts_sample)--; } } } @@ -3049,7 +3110,7 @@ static int64_t find_prev_closest_index(AVStream *st, /* restore AVStream state*/ st->index_entries = e_keep; st->nb_index_entries = nb_keep; - return found; + return *index >= 0 ? 0 : -1; } /** @@ -3220,10 +3281,8 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) int64_t empty_edits_sum_duration = 0; int64_t edit_list_index = 0; int64_t index; - int64_t index_ctts_count; int flags; int64_t start_dts = 0; - int64_t edit_list_media_time_dts = 0; int64_t edit_list_start_encountered = 0; int64_t search_timestamp = 0; int64_t* frame_duration_buffer = NULL; @@ -3293,17 +3352,11 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) st->skip_samples = msc->start_pad = 0; } - //find closest previous key frame - edit_list_media_time_dts = edit_list_media_time; - if (msc->dts_shift > 0) { - edit_list_media_time_dts -= msc->dts_shift; - } - // While reordering frame index according to edit list we must handle properly // the scenario when edit list entry starts from none key frame. // We find closest previous key frame and preserve it and consequent frames in index. // All frames which are outside edit list entry time boundaries will be dropped after decoding. - search_timestamp = edit_list_media_time_dts; + search_timestamp = edit_list_media_time; if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { // Audio decoders like AAC need need a decoder delay samples previous to the current sample, // to correctly decode this frame. Hence for audio we seek to a frame 1 sec. before the @@ -3311,38 +3364,24 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) search_timestamp = FFMAX(search_timestamp - msc->time_scale, e_old[0].timestamp); } - index = find_prev_closest_index(st, e_old, nb_old, search_timestamp, 0); - if (index == -1) { + if (find_prev_closest_index(st, e_old, nb_old, ctts_data_old, ctts_count_old, search_timestamp, 0, + &index, &ctts_index_old, &ctts_sample_old) < 0) { av_log(mov->fc, AV_LOG_WARNING, "st: %d edit list: %"PRId64" Missing key frame while searching for timestamp: %"PRId64"\n", st->index, edit_list_index, search_timestamp); - index = find_prev_closest_index(st, e_old, nb_old, search_timestamp, AVSEEK_FLAG_ANY); - - if (index == -1) { + if (find_prev_closest_index(st, e_old, nb_old, ctts_data_old, ctts_count_old, search_timestamp, AVSEEK_FLAG_ANY, + &index, &ctts_index_old, &ctts_sample_old) < 0) { av_log(mov->fc, AV_LOG_WARNING, "st: %d edit list %"PRId64" Cannot find an index entry before timestamp: %"PRId64".\n" "Rounding edit list media time to zero.\n", st->index, edit_list_index, search_timestamp); index = 0; + ctts_index_old = 0; + ctts_sample_old = 0; edit_list_media_time = 0; } } current = e_old + index; - - ctts_index_old = 0; - ctts_sample_old = 0; - - // set ctts_index properly for the found key frame - for (index_ctts_count = 0; index_ctts_count < index; index_ctts_count++) { - if (ctts_data_old && ctts_index_old < ctts_count_old) { - ctts_sample_old++; - if (ctts_data_old[ctts_index_old].count == ctts_sample_old) { - ctts_index_old++; - ctts_sample_old = 0; - } - } - } - edit_list_start_ctts_sample = ctts_sample_old; // Iterate over index and arrange it according to edit list diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 6815e4feca..01893a0767 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -9,6 +9,7 @@ FATE_MOV = fate-mov-3elist \ fate-mov-invalid-elst-entry-count \ fate-mov-gpmf-remux \ fate-mov-440hz-10ms \ + fate-mov-ibi-elst-starts-b \ FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \ fate-mov-zombie \ @@ -47,6 +48,13 @@ fate-mov-440hz-10ms: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/440hz-10ms.m4a # Makes sure that we handle invalid edit list entry count correctly. fate-mov-invalid-elst-entry-count: CMD = framemd5 -flags +bitexact -i $(TARGET_SAMPLES)/mov/invalid_elst_entry_count.mov +# Makes sure that 1st key-frame is picked when, +# i) One B-frame between 2 key-frames +# ii) Edit list starts on B-frame. +# iii) Both key-frames have their DTS < edit list start +# i.e. Pts Order: I-B-I +fate-mov-ibi-elst-starts-b: CMD = framemd5 -flags +bitexact -i $(TARGET_SAMPLES)/mov/mov_ibi_elst_starts_b.mov + fate-mov-aac-2048-priming: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets -print_format compact $(TARGET_SAMPLES)/mov/aac-2048-priming.mov fate-mov-zombie: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packets -show_frames -bitexact -print_format compact $(TARGET_SAMPLES)/mov/white_zombie_scrunch-part.mov