From: "Martin Storsjö" <martin@martin.st>
To: ffmpeg-devel@ffmpeg.org
Cc: Michael Niedermayer <michael@niedermayer.cc>,
Marton Balint <cus@passwd.hu>
Subject: Re: [FFmpeg-devel] [PATCHv2 2/2] libavformat: Improve ff_configure_buffers_for_index for excessive deltas
Date: Fri, 31 Mar 2023 09:52:59 +0300 (EEST)
Message-ID: <a0b0285f-c6aa-459f-f1e6-045618bf085@martin.st> (raw)
In-Reply-To: <20230328113051.3951-2-martin@martin.st>
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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
prev parent reply other threads:[~2023-03-31 6:53 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-28 11:30 [FFmpeg-devel] [PATCHv3 1/2] libavformat: Account for negative position differences in ff_configure_buffers_for_index Martin Storsjö
2023-03-28 11:30 ` [FFmpeg-devel] [PATCHv2 2/2] libavformat: Improve ff_configure_buffers_for_index for excessive deltas Martin Storsjö
2023-03-31 6:52 ` Martin Storsjö [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a0b0285f-c6aa-459f-f1e6-045618bf085@martin.st \
--to=martin@martin.st \
--cc=cus@passwd.hu \
--cc=ffmpeg-devel@ffmpeg.org \
--cc=michael@niedermayer.cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git