diff mbox series

[FFmpeg-devel] avformat/aviobuf: don't reduce short seek threshold

Message ID 20210314045432.214832-1-andriy.gelman@gmail.com
State Accepted
Commit 9383885c0d2155b1f9e65c08c3f35d190ae2ba98
Headers show
Series [FFmpeg-devel] avformat/aviobuf: don't reduce short seek threshold | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andriy Gelman March 14, 2021, 4:54 a.m. UTC
From: Andriy Gelman <andriy.gelman@gmail.com>

Commit 8c8e5d5286bf598a89ef9993a2cf6ea409d03a32 added a way to reduce
seek time by waiting for the windowed tcp packets instead of creating a
new socket connection. It implemented this by overwriting
s->short_seek_threshold in the avio_seek(). However,
s->short_seek_threshold could already be set and be higher than the
threshold set by the protocol (i.e. s->short_seek_threshold is set in
ff_configure_buffers_for_index()).

This new feature was only enabled for tls connections in
70d8077b795766e2486e6ec8110f22a97362d6d7. As in Ticket #9148 it reduced
performance because instead of waiting to refill the AVIOContext buffers
with an existing connections, a new HTTP request was often made instead.

Fixes Ticket #9148.

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libavformat/aviobuf.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Martin Storsjö March 15, 2021, 9:52 a.m. UTC | #1
On Sat, 13 Mar 2021, Andriy Gelman wrote:

> From: Andriy Gelman <andriy.gelman@gmail.com>
>
> Commit 8c8e5d5286bf598a89ef9993a2cf6ea409d03a32 added a way to reduce
> seek time by waiting for the windowed tcp packets instead of creating a
> new socket connection. It implemented this by overwriting
> s->short_seek_threshold in the avio_seek(). However,
> s->short_seek_threshold could already be set and be higher than the
> threshold set by the protocol (i.e. s->short_seek_threshold is set in
> ff_configure_buffers_for_index()).
>
> This new feature was only enabled for tls connections in
> 70d8077b795766e2486e6ec8110f22a97362d6d7. As in Ticket #9148 it reduced

This commit reference is typoed, the last char should be a 6, not a 7.

> performance because instead of waiting to refill the AVIOContext buffers
> with an existing connections, a new HTTP request was often made instead.
>
> Fixes Ticket #9148.
>
> Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> ---
> libavformat/aviobuf.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index 78cc60b2ae..518cb11129 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -283,13 +283,9 @@ int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
>     if (offset < 0)
>         return AVERROR(EINVAL);
> 
> -    if (s->short_seek_get) {
> -        short_seek = s->short_seek_get(s->opaque);
> -        /* fallback to default short seek */
> -        if (short_seek <= 0)
> -            short_seek = s->short_seek_threshold;
> -    } else
> -        short_seek = s->short_seek_threshold;
> +    short_seek = s->short_seek_threshold;
> +    if (s->short_seek_get)
> +        short_seek = FFMAX(s->short_seek_get(s->opaque), short_seek);
>
>     offset1 = offset - pos; // "offset1" is the relative offset from the beginning of s->buffer
>     s->buf_ptr_max = FFMAX(s->buf_ptr_max, s->buf_ptr);
> -- 
> 2.30.2

LGTM, thanks!

// Martin
Andriy Gelman March 17, 2021, 2:44 a.m. UTC | #2
On Mon, 15. Mar 11:52, Martin Storsjö wrote:
> On Sat, 13 Mar 2021, Andriy Gelman wrote:
> 
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> > 
> > Commit 8c8e5d5286bf598a89ef9993a2cf6ea409d03a32 added a way to reduce
> > seek time by waiting for the windowed tcp packets instead of creating a
> > new socket connection. It implemented this by overwriting
> > s->short_seek_threshold in the avio_seek(). However,
> > s->short_seek_threshold could already be set and be higher than the
> > threshold set by the protocol (i.e. s->short_seek_threshold is set in
> > ff_configure_buffers_for_index()).
> > 
> > This new feature was only enabled for tls connections in
> > 70d8077b795766e2486e6ec8110f22a97362d6d7. As in Ticket #9148 it reduced
> 
> This commit reference is typoed, the last char should be a 6, not a 7.
> 
> > performance because instead of waiting to refill the AVIOContext buffers
> > with an existing connections, a new HTTP request was often made instead.
> > 
> > Fixes Ticket #9148.
> > 
> > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > ---
> > libavformat/aviobuf.c | 10 +++-------
> > 1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> > index 78cc60b2ae..518cb11129 100644
> > --- a/libavformat/aviobuf.c
> > +++ b/libavformat/aviobuf.c
> > @@ -283,13 +283,9 @@ int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
> >     if (offset < 0)
> >         return AVERROR(EINVAL);
> > 
> > -    if (s->short_seek_get) {
> > -        short_seek = s->short_seek_get(s->opaque);
> > -        /* fallback to default short seek */
> > -        if (short_seek <= 0)
> > -            short_seek = s->short_seek_threshold;
> > -    } else
> > -        short_seek = s->short_seek_threshold;
> > +    short_seek = s->short_seek_threshold;
> > +    if (s->short_seek_get)
> > +        short_seek = FFMAX(s->short_seek_get(s->opaque), short_seek);
> > 
> >     offset1 = offset - pos; // "offset1" is the relative offset from the beginning of s->buffer
> >     s->buf_ptr_max = FFMAX(s->buf_ptr_max, s->buf_ptr);
> > -- 
> > 2.30.2

> 
> LGTM, thanks!

Thanks, will apply with the typo fix.
diff mbox series

Patch

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 78cc60b2ae..518cb11129 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -283,13 +283,9 @@  int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
     if (offset < 0)
         return AVERROR(EINVAL);
 
-    if (s->short_seek_get) {
-        short_seek = s->short_seek_get(s->opaque);
-        /* fallback to default short seek */
-        if (short_seek <= 0)
-            short_seek = s->short_seek_threshold;
-    } else
-        short_seek = s->short_seek_threshold;
+    short_seek = s->short_seek_threshold;
+    if (s->short_seek_get)
+        short_seek = FFMAX(s->short_seek_get(s->opaque), short_seek);
 
     offset1 = offset - pos; // "offset1" is the relative offset from the beginning of s->buffer
     s->buf_ptr_max = FFMAX(s->buf_ptr_max, s->buf_ptr);