| Message ID | 20230328113051.3951-2-martin@martin.st |
|---|---|
| State | Accepted |
| Commit | c221036502751d7a6e1403a9868c60f1ff357963 |
| Headers | show |
| Series | [FFmpeg-devel,PATCHv3,1/2] libavformat: Account for negative position differences in ff_configure_buffers_for_index | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Tue, 28 Mar 2023, Martin Storsjö wrote: > Previously, the ff_configure_buffers_for_index function had > upper sanity limits of 16 MB (1<<24) for buffer_size and > 8 MB (1<<23) for short_seek_threshold. > > However, if the index contained entries with a much larger > delta, setting pos_delta to a value larger than the sanity > limit, we would end up not increasing the buffer size at all. > > Instead, ignore the individual deltas that are excessive, but > increase the buffer size based on the deltas that are below the > sanity limit. > > Only count deltas that are below 1<<23, 8 MB; pos_delta gets doubled > before setting the buffer size - this matches the previous maximum > buffer size of 1<<24, 16 MB. > > This can happen e.g. with a mov file with some tracks containing > some samples that belong in the start of the file, at the end of > the mdat, while the rest of the file is mostly reasonably interleaved; > previously those samples caused the maximum pos_delta to skyrocket, > skipping any buffer size enlargement. > > Signed-off-by: Martin Storsjö <martin@martin.st> > --- > v2: Ignore entries that are out of range instead of clipping to > the maximum allowed. > --- > libavformat/seek.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/libavformat/seek.c b/libavformat/seek.c > index faa47f961f..386312cd3a 100644 > --- a/libavformat/seek.c > +++ b/libavformat/seek.c > @@ -204,7 +204,9 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) > const AVIndexEntry *const e1 = &sti1->index_entries[i1]; > int64_t e1_pts = av_rescale_q(e1->timestamp, st1->time_base, AV_TIME_BASE_Q); > > - skip = FFMAX(skip, e1->size); > + if (e1->size < (1 << 23)) > + skip = FFMAX(skip, e1->size); > + > for (; i2 < sti2->nb_index_entries; i2++) { > const AVIndexEntry *const e2 = &sti2->index_entries[i2]; > int64_t e2_pts = av_rescale_q(e2->timestamp, st2->time_base, AV_TIME_BASE_Q); > @@ -212,7 +214,8 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) > if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < time_tolerance) > continue; > cur_delta = FFABS(e1->pos - e2->pos); > - pos_delta = FFMAX(pos_delta, cur_delta); > + if (cur_delta < (1 << 23)) > + pos_delta = FFMAX(pos_delta, cur_delta); > break; > } > } > @@ -222,7 +225,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) > pos_delta *= 2; > ctx = ffiocontext(s->pb); > /* XXX This could be adjusted depending on protocol*/ > - if (s->pb->buffer_size < pos_delta && pos_delta < (1<<24)) { > + if (s->pb->buffer_size < pos_delta) { > av_log(s, AV_LOG_VERBOSE, "Reconfiguring buffers to size %"PRId64"\n", pos_delta); > > /* realloc the buffer and the original data will be retained */ > @@ -234,9 +237,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) > ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, pos_delta/2); > } > > - if (skip < (1<<23)) { > - ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, skip); > - } > + ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, skip); > } > > int av_index_search_timestamp(AVStream *st, int64_t wanted_timestamp, int flags) > -- > 2.37.1 (Apple Git-137.1) Will push these two patches today, if there's no further input on it; the previous iteration was already accepted by some, and this version includes the other feedback that I got. // Martin
diff --git a/libavformat/seek.c b/libavformat/seek.c index faa47f961f..386312cd3a 100644 --- a/libavformat/seek.c +++ b/libavformat/seek.c @@ -204,7 +204,9 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) const AVIndexEntry *const e1 = &sti1->index_entries[i1]; int64_t e1_pts = av_rescale_q(e1->timestamp, st1->time_base, AV_TIME_BASE_Q); - skip = FFMAX(skip, e1->size); + if (e1->size < (1 << 23)) + skip = FFMAX(skip, e1->size); + for (; i2 < sti2->nb_index_entries; i2++) { const AVIndexEntry *const e2 = &sti2->index_entries[i2]; int64_t e2_pts = av_rescale_q(e2->timestamp, st2->time_base, AV_TIME_BASE_Q); @@ -212,7 +214,8 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < time_tolerance) continue; cur_delta = FFABS(e1->pos - e2->pos); - pos_delta = FFMAX(pos_delta, cur_delta); + if (cur_delta < (1 << 23)) + pos_delta = FFMAX(pos_delta, cur_delta); break; } } @@ -222,7 +225,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) pos_delta *= 2; ctx = ffiocontext(s->pb); /* XXX This could be adjusted depending on protocol*/ - if (s->pb->buffer_size < pos_delta && pos_delta < (1<<24)) { + if (s->pb->buffer_size < pos_delta) { av_log(s, AV_LOG_VERBOSE, "Reconfiguring buffers to size %"PRId64"\n", pos_delta); /* realloc the buffer and the original data will be retained */ @@ -234,9 +237,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance) ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, pos_delta/2); } - if (skip < (1<<23)) { - ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, skip); - } + ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, skip); } int av_index_search_timestamp(AVStream *st, int64_t wanted_timestamp, int flags)
Previously, the ff_configure_buffers_for_index function had upper sanity limits of 16 MB (1<<24) for buffer_size and 8 MB (1<<23) for short_seek_threshold. However, if the index contained entries with a much larger delta, setting pos_delta to a value larger than the sanity limit, we would end up not increasing the buffer size at all. Instead, ignore the individual deltas that are excessive, but increase the buffer size based on the deltas that are below the sanity limit. Only count deltas that are below 1<<23, 8 MB; pos_delta gets doubled before setting the buffer size - this matches the previous maximum buffer size of 1<<24, 16 MB. This can happen e.g. with a mov file with some tracks containing some samples that belong in the start of the file, at the end of the mdat, while the rest of the file is mostly reasonably interleaved; previously those samples caused the maximum pos_delta to skyrocket, skipping any buffer size enlargement. Signed-off-by: Martin Storsjö <martin@martin.st> --- v2: Ignore entries that are out of range instead of clipping to the maximum allowed. --- libavformat/seek.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)