| Message ID | 20170817045911.56742-1-zhangjiejun1992@gmail.com |
|---|---|
| State | New |
| Headers | show |
2017-08-17 12:59 GMT+08:00 <zhangjiejun1992@gmail.com>: > From: Jiejun Zhang <zhangjiejun1992@gmail.com> > > When an edit list exists in a MOV file, counting by stscs no longer > works because stscs' order is different from the actual timeline. This > commit adds stsd-change markers to the actual timeline and changes stsd > according to these markers. > --- > libavformat/isom.h | 14 +++++-- > libavformat/mov.c | 120 +++++++++++++++++++++++++++++++++++++++++------------ > 2 files changed, 104 insertions(+), 30 deletions(-) > > diff --git a/libavformat/isom.h b/libavformat/isom.h > index ff009b0896..51cd333a8c 100644 > --- a/libavformat/isom.h > +++ b/libavformat/isom.h > @@ -127,6 +127,11 @@ typedef struct MOVIndexRange { > int64_t end; > } MOVIndexRange; > > +typedef struct MOVStsdChangeEntry { > + int64_t sample_index; ///< stsd changes at this sample index > + int stsd_id; ///< stsd changes to stsd_id > +} MOVStsdChangeEntry; > + > typedef struct MOVStreamContext { > AVIOContext *pb; > int pb_is_copied; > @@ -140,16 +145,17 @@ typedef struct MOVStreamContext { > MOVStts *ctts_data; > unsigned int stsc_count; > MOVStsc *stsc_data; > - int stsc_index; > - int stsc_sample; > + MOVStsdChangeEntry *stsd_change_data; > + int stsd_change_count; > + int stsd_change_index; > unsigned int stps_count; > unsigned *stps_data; ///< partial sync sample for mpeg-2 open gop > MOVElst *elst_data; > unsigned int elst_count; > int ctts_index; > int ctts_sample; > - unsigned int sample_size; ///< may contain value calculated from stsd or value from stsz atom > - unsigned int stsz_sample_size; ///< always contains sample size from stsz atom > + unsigned int sample_size; ///< may contain value calculated from stsd or value from stsz atom unsigned > + int stsz_sample_size; ///< always contains sample size from stsz atom > unsigned int sample_count; > int *sample_sizes; > int keyframe_absent; > diff --git a/libavformat/mov.c b/libavformat/mov.c > index c02caf6719..e775056c90 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -2926,6 +2926,35 @@ static int64_t add_index_entry(AVStream *st, int64_t pos, int64_t timestamp, > return index; > } > > +static int add_stsd_change_entry(MOVStreamContext *msc, int64_t sample_index, int stsd_id, > + unsigned int* allocated_size) > +{ > + MOVStsdChangeEntry *entries; > + const size_t min_size_needed = (msc->stsd_change_count + 1) * sizeof(MOVStsdChangeEntry); > + > + const size_t requested_size = > + min_size_needed > *allocated_size ? > + FFMAX(min_size_needed, 2 * (*allocated_size)) : > + min_size_needed; > + > + if ((unsigned)msc->stsd_change_count + 1 >= UINT_MAX / sizeof(MOVStsdChangeEntry)) > + return -1; > + > + entries = av_fast_realloc(msc->stsd_change_data, > + allocated_size, > + requested_size); > + > + if (!entries) > + return -1; > + > + msc->stsd_change_data = entries; > + entries[msc->stsd_change_count].sample_index = sample_index; > + entries[msc->stsd_change_count].stsd_id = stsd_id; > + msc->stsd_change_count++; > + > + return 0; > +} > + > /** > * Rewrite timestamps of index entries in the range [end_index - frame_duration_buffer_size, end_index) > * by subtracting end_ts successively by the amounts given in frame_duration_buffer. > @@ -3058,6 +3087,8 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) > int num_discarded_begin = 0; > int first_non_zero_audio_edit = -1; > int packet_skip_samples = 0; > + int stsc_index = 0; > + int stsc_sample = 0; > MOVIndexRange *current_index_range; > int i; > > @@ -3094,6 +3125,11 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) > > start_dts = edit_list_dts_entry_end; > > + msc->last_stsd_index = -1; > + unsigned int stsd_change_data_allocated_size = 0; > + msc->stsd_change_data = NULL; > + msc->stsd_change_count = 0; > + > while (get_edit_list_entry(mov, msc, edit_list_index, &edit_list_media_time, > &edit_list_duration, mov->time_scale)) { > av_log(mov->fc, AV_LOG_DEBUG, "Processing st: %d, edit list %"PRId64" - media time: %"PRId64", duration: %"PRId64"\n", > @@ -3156,6 +3192,20 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) > } > current = e_old + index; > > + // adjust stsd index > + int time_sample = 0; > + for (i = 0; i < msc->stsc_count; i++) { > + int next = time_sample + mov_get_stsc_samples(msc, i); > + if (next > index) { > + stsc_index = i; > + stsc_sample = index - time_sample; > + break; > + } > + time_sample = next; > + } > + > + av_log(mov->fc, AV_LOG_INFO, "stream %d, adjusted stsc_index: %d, stsc_sample: %d\n", st->index, stsc_index, stsc_sample); > + > ctts_index_old = 0; > ctts_sample_old = 0; > > @@ -3268,12 +3318,35 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) > } > } > > - if (add_index_entry(st, current->pos, edit_list_dts_counter, current->size, > - current->min_distance, flags) == -1) { > + int index_entry_index = add_index_entry(st, current->pos, edit_list_dts_counter, current->size, > + current->min_distance, flags); > + if (index_entry_index == -1) { > av_log(mov->fc, AV_LOG_ERROR, "Cannot add index entry\n"); > break; > } > > + if (msc->stsc_data) { > + if (msc->stsc_data[stsc_index].id > 0 && > + msc->stsc_data[stsc_index].id - 1 < msc->stsd_count && > + msc->stsc_data[stsc_index].id - 1 != msc->last_stsd_index) { > + msc->last_stsd_index = msc->stsc_data[stsc_index].id - 1; > + int ret = add_stsd_change_entry(msc, index_entry_index, msc->last_stsd_index, &stsd_change_data_allocated_size); > + if (ret < 0) { > + av_log(mov->fc, AV_LOG_ERROR, "Cannot add stsd change entry\n"); > + break; > + } > + av_log(mov->fc, AV_LOG_INFO, "added a stsd change entry sample_index: %d, stsd_index:%d\n", > + index_entry_index, > + msc->last_stsd_index); > + } > + stsc_sample++; > + if (stsc_index < msc->stsc_count - 1 && > + mov_get_stsc_samples(msc, stsc_index) == stsc_sample) { > + stsc_index++; > + stsc_sample = 0; > + } > + } > + > // Update the index ranges array > if (current_index_range < msc->index_ranges || index != current_index_range->end) { > current_index_range++; > @@ -3330,6 +3403,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) > current_index_range->start = 0; > current_index_range->end = 0; > msc->current_index = msc->index_ranges[0].start; > + msc->last_stsd_index = 0; > } > > static void mov_build_index(MOVContext *mov, AVStream *st) > @@ -6435,9 +6509,6 @@ static int mov_change_extradata(MOVStreamContext *sc, AVPacket *pkt) > uint8_t *side, *extradata; > int extradata_size; > > - /* Save the current index. */ > - sc->last_stsd_index = sc->stsc_data[sc->stsc_index].id - 1; > - > /* Notify the decoder that extradata changed. */ > extradata_size = sc->extradata_size[sc->last_stsd_index]; > extradata = sc->extradata[sc->last_stsd_index]; > @@ -6557,18 +6628,18 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt) > pkt->pos = sample->pos; > > /* Multiple stsd handling. */ > - if (sc->stsc_data) { > - /* Keep track of the stsc index for the given sample, then check > - * if the stsd index is different from the last used one. */ > - sc->stsc_sample++; > - if (mov_stsc_index_valid(sc->stsc_index, sc->stsc_count) && > - mov_get_stsc_samples(sc, sc->stsc_index) == sc->stsc_sample) { > - sc->stsc_index++; > - sc->stsc_sample = 0; > - /* Do not check indexes after a switch. */ > - } else if (sc->stsc_data[sc->stsc_index].id > 0 && > - sc->stsc_data[sc->stsc_index].id - 1 < sc->stsd_count && > - sc->stsc_data[sc->stsc_index].id - 1 != sc->last_stsd_index) { > + if (sc->stsd_change_data) { > + if (sc->stsd_change_index + 1 < sc->stsd_change_count && > + // sc->current_sample is already the next sample, so subtract 1 > + sc->stsd_change_data[sc->stsd_change_index + 1].sample_index == sc->current_sample - 1) { > + sc->stsd_change_index++; > + } > + if (sc->last_stsd_index != sc->stsd_change_data[sc->stsd_change_index].stsd_id) { > + av_log(mov->fc, AV_LOG_INFO, "stsd id changed from %d to %d, at sample %d\n", > + sc->last_stsd_index, > + sc->stsd_change_data[sc->stsd_change_index].stsd_id, > + sc->current_sample - 1); > + sc->last_stsd_index = sc->stsd_change_data[sc->stsd_change_index].stsd_id; > ret = mov_change_extradata(sc, pkt); > if (ret < 0) > return ret; > @@ -6646,16 +6717,13 @@ static int mov_seek_stream(AVFormatContext *s, AVStream *st, int64_t timestamp, > } > } > > - /* adjust stsd index */ > - time_sample = 0; > - for (i = 0; i < sc->stsc_count; i++) { > - int next = time_sample + mov_get_stsc_samples(sc, i); > - if (next > sc->current_sample) { > - sc->stsc_index = i; > - sc->stsc_sample = sc->current_sample - time_sample; > - break; > + /* adjust stsd change index */ > + for (i = 0; i < sc->stsd_change_count; i++) { > + if (sc->current_sample >= sc->stsd_change_data[i].sample_index && > + (i + 1 == sc->stsd_change_count || > + sc->current_sample < sc->stsd_change_data[i + 1].sample_index)) { > + sc->stsd_change_index = i; > } > - time_sample = next; > } > > return sample; > -- > 2.11.0 (Apple Git-81) > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel LGTM
On Thu, Aug 17, 2017 at 12:59:11PM +0800, zhangjiejun1992@gmail.com wrote: > From: Jiejun Zhang <zhangjiejun1992@gmail.com> > > When an edit list exists in a MOV file, counting by stscs no longer > works because stscs' order is different from the actual timeline. This > commit adds stsd-change markers to the actual timeline and changes stsd > according to these markers. > --- > libavformat/isom.h | 14 +++++-- > libavformat/mov.c | 120 +++++++++++++++++++++++++++++++++++++++++------------ > 2 files changed, 104 insertions(+), 30 deletions(-) seeking still fails with this and the sample from #6584 is that intended or a seperate issue ? [...]
On Fri, Aug 18, 2017 at 8:16 AM, Michael Niedermayer <michael@niedermayer.cc> wrote: > > seeking still fails with this and the sample from #6584 is that > intended or a seperate issue ? I didn't see any failure using "./ffmpeg -ss 00:00:10" (and also other time from 0s to 20s). How to reproduce a seeking failure?
On Sat, Aug 19, 2017 at 08:32:18AM +0800, Jiejun Zhang wrote: > On Fri, Aug 18, 2017 at 8:16 AM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > seeking still fails with this and the sample from #6584 is that > > intended or a seperate issue ? > > I didn't see any failure using "./ffmpeg -ss 00:00:10" (and also other > time from 0s to 20s). How to reproduce a seeking failure? i tested using ffplay and seeking with cursor keys and mouse there where some green frames flashing IIRC tell me if you can reproduce or not. If not ill try to come up with some deterministic testcase [...]
diff --git a/libavformat/isom.h b/libavformat/isom.h index ff009b0896..51cd333a8c 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -127,6 +127,11 @@ typedef struct MOVIndexRange { int64_t end; } MOVIndexRange; +typedef struct MOVStsdChangeEntry { + int64_t sample_index; ///< stsd changes at this sample index + int stsd_id; ///< stsd changes to stsd_id +} MOVStsdChangeEntry; + typedef struct MOVStreamContext { AVIOContext *pb; int pb_is_copied; @@ -140,16 +145,17 @@ typedef struct MOVStreamContext { MOVStts *ctts_data; unsigned int stsc_count; MOVStsc *stsc_data; - int stsc_index; - int stsc_sample; + MOVStsdChangeEntry *stsd_change_data; + int stsd_change_count; + int stsd_change_index; unsigned int stps_count; unsigned *stps_data; ///< partial sync sample for mpeg-2 open gop MOVElst *elst_data; unsigned int elst_count; int ctts_index; int ctts_sample; - unsigned int sample_size; ///< may contain value calculated from stsd or value from stsz atom - unsigned int stsz_sample_size; ///< always contains sample size from stsz atom + unsigned int sample_size; ///< may contain value calculated from stsd or value from stsz atom unsigned + int stsz_sample_size; ///< always contains sample size from stsz atom unsigned int sample_count; int *sample_sizes; int keyframe_absent; diff --git a/libavformat/mov.c b/libavformat/mov.c index c02caf6719..e775056c90 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2926,6 +2926,35 @@ static int64_t add_index_entry(AVStream *st, int64_t pos, int64_t timestamp, return index; } +static int add_stsd_change_entry(MOVStreamContext *msc, int64_t sample_index, int stsd_id, + unsigned int* allocated_size) +{ + MOVStsdChangeEntry *entries; + const size_t min_size_needed = (msc->stsd_change_count + 1) * sizeof(MOVStsdChangeEntry); + + const size_t requested_size = + min_size_needed > *allocated_size ? + FFMAX(min_size_needed, 2 * (*allocated_size)) : + min_size_needed; + + if ((unsigned)msc->stsd_change_count + 1 >= UINT_MAX / sizeof(MOVStsdChangeEntry)) + return -1; + + entries = av_fast_realloc(msc->stsd_change_data, + allocated_size, + requested_size); + + if (!entries) + return -1; + + msc->stsd_change_data = entries; + entries[msc->stsd_change_count].sample_index = sample_index; + entries[msc->stsd_change_count].stsd_id = stsd_id; + msc->stsd_change_count++; + + return 0; +} + /** * Rewrite timestamps of index entries in the range [end_index - frame_duration_buffer_size, end_index) * by subtracting end_ts successively by the amounts given in frame_duration_buffer. @@ -3058,6 +3087,8 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) int num_discarded_begin = 0; int first_non_zero_audio_edit = -1; int packet_skip_samples = 0; + int stsc_index = 0; + int stsc_sample = 0; MOVIndexRange *current_index_range; int i; @@ -3094,6 +3125,11 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) start_dts = edit_list_dts_entry_end; + msc->last_stsd_index = -1; + unsigned int stsd_change_data_allocated_size = 0; + msc->stsd_change_data = NULL; + msc->stsd_change_count = 0; + while (get_edit_list_entry(mov, msc, edit_list_index, &edit_list_media_time, &edit_list_duration, mov->time_scale)) { av_log(mov->fc, AV_LOG_DEBUG, "Processing st: %d, edit list %"PRId64" - media time: %"PRId64", duration: %"PRId64"\n", @@ -3156,6 +3192,20 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) } current = e_old + index; + // adjust stsd index + int time_sample = 0; + for (i = 0; i < msc->stsc_count; i++) { + int next = time_sample + mov_get_stsc_samples(msc, i); + if (next > index) { + stsc_index = i; + stsc_sample = index - time_sample; + break; + } + time_sample = next; + } + + av_log(mov->fc, AV_LOG_INFO, "stream %d, adjusted stsc_index: %d, stsc_sample: %d\n", st->index, stsc_index, stsc_sample); + ctts_index_old = 0; ctts_sample_old = 0; @@ -3268,12 +3318,35 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) } } - if (add_index_entry(st, current->pos, edit_list_dts_counter, current->size, - current->min_distance, flags) == -1) { + int index_entry_index = add_index_entry(st, current->pos, edit_list_dts_counter, current->size, + current->min_distance, flags); + if (index_entry_index == -1) { av_log(mov->fc, AV_LOG_ERROR, "Cannot add index entry\n"); break; } + if (msc->stsc_data) { + if (msc->stsc_data[stsc_index].id > 0 && + msc->stsc_data[stsc_index].id - 1 < msc->stsd_count && + msc->stsc_data[stsc_index].id - 1 != msc->last_stsd_index) { + msc->last_stsd_index = msc->stsc_data[stsc_index].id - 1; + int ret = add_stsd_change_entry(msc, index_entry_index, msc->last_stsd_index, &stsd_change_data_allocated_size); + if (ret < 0) { + av_log(mov->fc, AV_LOG_ERROR, "Cannot add stsd change entry\n"); + break; + } + av_log(mov->fc, AV_LOG_INFO, "added a stsd change entry sample_index: %d, stsd_index:%d\n", + index_entry_index, + msc->last_stsd_index); + } + stsc_sample++; + if (stsc_index < msc->stsc_count - 1 && + mov_get_stsc_samples(msc, stsc_index) == stsc_sample) { + stsc_index++; + stsc_sample = 0; + } + } + // Update the index ranges array if (current_index_range < msc->index_ranges || index != current_index_range->end) { current_index_range++; @@ -3330,6 +3403,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) current_index_range->start = 0; current_index_range->end = 0; msc->current_index = msc->index_ranges[0].start; + msc->last_stsd_index = 0; } static void mov_build_index(MOVContext *mov, AVStream *st) @@ -6435,9 +6509,6 @@ static int mov_change_extradata(MOVStreamContext *sc, AVPacket *pkt) uint8_t *side, *extradata; int extradata_size; - /* Save the current index. */ - sc->last_stsd_index = sc->stsc_data[sc->stsc_index].id - 1; - /* Notify the decoder that extradata changed. */ extradata_size = sc->extradata_size[sc->last_stsd_index]; extradata = sc->extradata[sc->last_stsd_index]; @@ -6557,18 +6628,18 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt) pkt->pos = sample->pos; /* Multiple stsd handling. */ - if (sc->stsc_data) { - /* Keep track of the stsc index for the given sample, then check - * if the stsd index is different from the last used one. */ - sc->stsc_sample++; - if (mov_stsc_index_valid(sc->stsc_index, sc->stsc_count) && - mov_get_stsc_samples(sc, sc->stsc_index) == sc->stsc_sample) { - sc->stsc_index++; - sc->stsc_sample = 0; - /* Do not check indexes after a switch. */ - } else if (sc->stsc_data[sc->stsc_index].id > 0 && - sc->stsc_data[sc->stsc_index].id - 1 < sc->stsd_count && - sc->stsc_data[sc->stsc_index].id - 1 != sc->last_stsd_index) { + if (sc->stsd_change_data) { + if (sc->stsd_change_index + 1 < sc->stsd_change_count && + // sc->current_sample is already the next sample, so subtract 1 + sc->stsd_change_data[sc->stsd_change_index + 1].sample_index == sc->current_sample - 1) { + sc->stsd_change_index++; + } + if (sc->last_stsd_index != sc->stsd_change_data[sc->stsd_change_index].stsd_id) { + av_log(mov->fc, AV_LOG_INFO, "stsd id changed from %d to %d, at sample %d\n", + sc->last_stsd_index, + sc->stsd_change_data[sc->stsd_change_index].stsd_id, + sc->current_sample - 1); + sc->last_stsd_index = sc->stsd_change_data[sc->stsd_change_index].stsd_id; ret = mov_change_extradata(sc, pkt); if (ret < 0) return ret; @@ -6646,16 +6717,13 @@ static int mov_seek_stream(AVFormatContext *s, AVStream *st, int64_t timestamp, } } - /* adjust stsd index */ - time_sample = 0; - for (i = 0; i < sc->stsc_count; i++) { - int next = time_sample + mov_get_stsc_samples(sc, i); - if (next > sc->current_sample) { - sc->stsc_index = i; - sc->stsc_sample = sc->current_sample - time_sample; - break; + /* adjust stsd change index */ + for (i = 0; i < sc->stsd_change_count; i++) { + if (sc->current_sample >= sc->stsd_change_data[i].sample_index && + (i + 1 == sc->stsd_change_count || + sc->current_sample < sc->stsd_change_data[i + 1].sample_index)) { + sc->stsd_change_index = i; } - time_sample = next; } return sample;
From: Jiejun Zhang <zhangjiejun1992@gmail.com> When an edit list exists in a MOV file, counting by stscs no longer works because stscs' order is different from the actual timeline. This commit adds stsd-change markers to the actual timeline and changes stsd according to these markers. --- libavformat/isom.h | 14 +++++-- libavformat/mov.c | 120 +++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 104 insertions(+), 30 deletions(-)