Ping! Anyone have a chance to review my latest patch which leverages the read-ahead/discard logic in avio_seek?
Thanks, Joel > On Jan 13, 2017, at 11:36 AM, Joel Cunningham <joel.cunning...@me.com> wrote: > >> >> On Jan 12, 2017, at 10:59 AM, Joel Cunningham <joel.cunning...@me.com> wrote: >> >> Nicolas, >> >> I’ve found existing “read-ahead logic” in avio_seek to do what I’ve >> implemented in http_stream_forward(). This is controlled by >> SHORT_SEEK_THRESHOLD, currently set to 4KB. I proto-typed increasing this >> to the 256KB (matches the initial TCP window in my test setup) and saw the >> same number of reduction in HTTP GETs and the number of seeks! Thanks for >> the heads up, this should reduce my patch size! >> >> I could plumb this setting (s->short_seek_threshold) to a URL function that >> would get the desired value from HTTP/TCP. Looking at how >> ffio_init_context() is implemented, it doesn’t appear to have access to the >> URLContext pointer. Any guidance on how I can plumb the protocol layer with >> aviobuf without a public API change? >> >> Thanks, >> >> Joel > > > I managed to figure the plumbing out in a way that doesn’t modify any public > APIs. I added a new callback to struct AVIOContext that gets a short seek > threshold value from URLContext if available (otherwise fallback to > s->short_seek_threshold). This allows using the read-ahead/discard logic in > avio_seek and eliminates my forwarding logic in the HTTP layer (thanks to > Nicolas for pointing me in this direction). See new patch below > > I also updated my commit message to include assumptions/considerations about > congestion control on the sender (thanks to Andy for calling out the > discussion on it). Lastly, I have upload new captures/log in dropbox for > those that want to take a look at my testing output (see > ffplay_short_seek_output.log and mac-ffplay-short-seek-patch.pcapng) > > https://www.dropbox.com/sh/4q4ru8isdv22joj/AABU3XyXmgLMiEFqucf1LdZ3a?dl=0 > > Thanks for the feedback so far, > > Joel > > From 9bb2f184591c2d6e6a91d3760e63b013ca4c95e5 Mon Sep 17 00:00:00 2001 > From: Joel Cunningham <joel.cunning...@me.com> > Date: Fri, 13 Jan 2017 10:52:25 -0600 > Subject: [PATCH] HTTP: improve performance by reducing forward seeks > > This commit optimizes HTTP performance by reducing forward seeks, instead > favoring a read-ahead and discard on the current connection (referred to > as a short seek) for seeks that are within a TCP window's worth of data. > This improves performance because with TCP flow control, a window's worth > of data will be in the local socket buffer already or in-flight from the > sender once congestion control on the sender is fully utilizing the window. > > Note: this approach doesn't attempt to differentiate from a newly opened > connection which may not be fully utilizing the window due to congestion > control vs one that is. The receiver can't get at this information, so we > assume worst case; that full window is in use (we did advertise it after all) > and that data could be in-flight > > The previous behavior of closing the connection, then opening a new > with a new HTTP range value results in a massive amounts of discarded > and re-sent data when large TCP windows are used. This has been observed > on MacOS/iOS which starts with an inital window of 256KB and grows up to > 1MB depending on the bandwidth-product delay. > > When seeking within a window's worth of data and we close the connection, > then open a new one within the same window's worth of data, we discard > from the current offset till the end of the window. Then on the new > connection the server ends up re-sending the previous data from new > offset till the end of old window. > > Example (assumes full window utilization): > > TCP window size: 64KB > Position: 32KB > Forward seek position: 40KB > > * (Next window) > 32KB |--------------| 96KB |---------------| 160KB > * > 40KB |---------------| 104KB > > Re-sent amount: 96KB - 40KB = 56KB > > For a real world test example, I have MP4 file of ~25MB, which ffplay > only reads ~16MB and performs 177 seeks. With current ffmpeg, this results > in 177 HTTP GETs and ~73MB worth of TCP data communication. With this > patch, ffmpeg issues 4 HTTP GETs and 3 seeks for a total of ~22MB of TCP data > communication. > > To support this feature, the short seek logic in avio_seek() has been > extended to call a function to get the short seek threshold value. This > callback has been plumbed to the URLProtocol structure, which now has > infrastructure in HTTP and TCP to get the underlying receiver window size > via SO_RCVBUF. If the underlying URL and protocol don't support returning > a short seek threshold, the default s->short_seek_threshold is used > > This feature has been tested on Windows 7 and MacOS/iOS. Windows support > is slightly complicated by the fact that when TCP window auto-tuning is > enabled, SO_RCVBUF doesn't report the real window size, but it does if > SO_RCVBUF was manually set (disabling auto-tuning). So we can only use > this optimization on Windows in the later case > --- > libavformat/avio.c | 7 +++++++ > libavformat/avio.h | 6 ++++++ > libavformat/aviobuf.c | 18 +++++++++++++++++- > libavformat/http.c | 8 ++++++++ > libavformat/tcp.c | 21 +++++++++++++++++++++ > libavformat/url.h | 8 ++++++++ > 6 files changed, 67 insertions(+), 1 deletion(-) > > diff --git a/libavformat/avio.c b/libavformat/avio.c > index 3606eb0..ecf6801 100644 > --- a/libavformat/avio.c > +++ b/libavformat/avio.c > @@ -645,6 +645,13 @@ int ffurl_get_multi_file_handle(URLContext *h, int > **handles, int *numhandles) > return h->prot->url_get_multi_file_handle(h, handles, numhandles); > } > > +int ffurl_get_short_seek(URLContext *h) > +{ > + if (!h->prot->url_get_short_seek) > + return -1; > + return h->prot->url_get_short_seek(h); > +} > + > int ffurl_shutdown(URLContext *h, int flags) > { > if (!h->prot->url_shutdown) > diff --git a/libavformat/avio.h b/libavformat/avio.h > index b1ce1d1..0480981 100644 > --- a/libavformat/avio.h > +++ b/libavformat/avio.h > @@ -287,6 +287,12 @@ typedef struct AVIOContext { > int short_seek_threshold; > > /** > + * A callback that is used instead of short_seek_threshold. > + * This is current internal only, do not use from outside. > + */ > + int (*short_seek_get)(void *opaque); > + > + /** > * ',' separated list of allowed protocols. > */ > const char *protocol_whitelist; > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > index 134d627..2df428f 100644 > --- a/libavformat/aviobuf.c > +++ b/libavformat/aviobuf.c > @@ -233,6 +233,7 @@ int64_t avio_seek(AVIOContext *s, int64_t offset, int > whence) > int64_t pos; > int force = whence & AVSEEK_FORCE; > int buffer_size; > + int short_seek; > whence &= ~AVSEEK_FORCE; > > if(!s) > @@ -254,13 +255,21 @@ 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; > + > offset1 = offset - pos; // "offset1" is the relative offset from the > beginning of s->buffer > if (!s->must_flush && (!s->direct || !s->seek) && > offset1 >= 0 && offset1 <= buffer_size - s->write_flag) { > /* can do the seek inside the buffer */ > s->buf_ptr = s->buffer + offset1; > } else if ((!s->seekable || > - offset1 <= buffer_size + s->short_seek_threshold) && > + offset1 <= buffer_size + short_seek) && > !s->write_flag && offset1 >= 0 && > (!s->direct || !s->seek) && > (whence != SEEK_END || force)) { > @@ -858,6 +867,12 @@ static int64_t io_seek(void *opaque, int64_t offset, int > whence) > return ffurl_seek(internal->h, offset, whence); > } > > +static int io_short_seek(void *opaque) > +{ > + AVIOInternal *internal = opaque; > + return ffurl_get_short_seek(internal->h); > +} > + > static int io_read_pause(void *opaque, int pause) > { > AVIOInternal *internal = opaque; > @@ -919,6 +934,7 @@ int ffio_fdopen(AVIOContext **s, URLContext *h) > (*s)->read_pause = io_read_pause; > (*s)->read_seek = io_read_seek; > } > + (*s)->short_seek_get = io_short_seek; > (*s)->av_class = &ff_avio_class; > return 0; > fail: > diff --git a/libavformat/http.c b/libavformat/http.c > index 944a6cf..b354c87 100644 > --- a/libavformat/http.c > +++ b/libavformat/http.c > @@ -1524,6 +1524,12 @@ static int http_get_file_handle(URLContext *h) > return ffurl_get_file_handle(s->hd); > } > > +static int http_get_short_seek(URLContext *h) > +{ > + HTTPContext *s = h->priv_data; > + return ffurl_get_short_seek(s->hd); > +} > + > #define HTTP_CLASS(flavor) \ > static const AVClass flavor ## _context_class = { \ > .class_name = # flavor, \ > @@ -1545,6 +1551,7 @@ const URLProtocol ff_http_protocol = { > .url_seek = http_seek, > .url_close = http_close, > .url_get_file_handle = http_get_file_handle, > + .url_get_short_seek = http_get_short_seek, > .url_shutdown = http_shutdown, > .priv_data_size = sizeof(HTTPContext), > .priv_data_class = &http_context_class, > @@ -1564,6 +1571,7 @@ const URLProtocol ff_https_protocol = { > .url_seek = http_seek, > .url_close = http_close, > .url_get_file_handle = http_get_file_handle, > + .url_get_short_seek = http_get_short_seek, > .url_shutdown = http_shutdown, > .priv_data_size = sizeof(HTTPContext), > .priv_data_class = &https_context_class, > diff --git a/libavformat/tcp.c b/libavformat/tcp.c > index 25abafc..a07c6cd 100644 > --- a/libavformat/tcp.c > +++ b/libavformat/tcp.c > @@ -265,6 +265,26 @@ static int tcp_get_file_handle(URLContext *h) > return s->fd; > } > > +static int tcp_get_window_size(URLContext *h) > +{ > + TCPContext *s = h->priv_data; > + int avail; > + int avail_len = sizeof(avail); > + > + #if HAVE_WINSOCK2_H > + /* SO_RCVBUF with winsock only reports the actual TCP window size when > + auto-tuning has been disabled via setting SO_RCVBUF */ > + if (s->recv_buffer_size < 0) { > + return AVERROR(ENOSYS); > + } > + #endif > + > + if (getsockopt(s->fd, SOL_SOCKET, SO_RCVBUF, &avail, &avail_len)) { > + return ff_neterrno(); > + } > + return avail; > +} > + > const URLProtocol ff_tcp_protocol = { > .name = "tcp", > .url_open = tcp_open, > @@ -273,6 +293,7 @@ const URLProtocol ff_tcp_protocol = { > .url_write = tcp_write, > .url_close = tcp_close, > .url_get_file_handle = tcp_get_file_handle, > + .url_get_short_seek = tcp_get_window_size, > .url_shutdown = tcp_shutdown, > .priv_data_size = sizeof(TCPContext), > .flags = URL_PROTOCOL_FLAG_NETWORK, > diff --git a/libavformat/url.h b/libavformat/url.h > index 5c50245..910f1e0 100644 > --- a/libavformat/url.h > +++ b/libavformat/url.h > @@ -84,6 +84,7 @@ typedef struct URLProtocol { > int (*url_get_file_handle)(URLContext *h); > int (*url_get_multi_file_handle)(URLContext *h, int **handles, > int *numhandles); > + int (*url_get_short_seek)(URLContext *h); > int (*url_shutdown)(URLContext *h, int flags); > int priv_data_size; > const AVClass *priv_data_class; > @@ -249,6 +250,13 @@ int ffurl_get_file_handle(URLContext *h); > int ffurl_get_multi_file_handle(URLContext *h, int **handles, int > *numhandles); > > /** > + * Return the current short seek threshold value for this URL. > + * > + * @return threshold (>0) on success or <=0 on error. > + */ > +int ffurl_get_short_seek(URLContext *h); > + > +/** > * Signal the URLContext that we are done reading or writing the stream. > * > * @param h pointer to the resource > -- > 2.10.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
