| Message ID | 20171213003511.25342-6-ffmpeg@tmm1.net |
|---|---|
| State | New |
| Headers | show |
Hi! Aman Gupta kirjoitti 2017-12-13 02:35: > From: Aman Gupta <aman@tmm1.net> > > This improves network throughput of the hls demuxer by avoiding > the latency introduced by downloading segments one at a time. > > The problem is particularly noticable over high-latency network > connections: for instance, if RTT is 250ms, there will a 250ms idle > period between when one segment response is read and the next one > starts. > > The obvious solution to this is to use HTTP pipelining, where a > second request can be sent (on the persistent http/1.1 connection) > before the first response is fully read. Unfortunately the way the > http protocol is implemented in avformat makes implementing pipleining > very complex. > > Instead, this commit simulates pipelining using two separate persistent > http connections. This has the advantage of working independently of > the http_persistent option, and can be used with http/1.0 servers as > well. The pair of connections is swapped every time a new segment > starts > downloading, and a request for the next segment is sent on the > secondary > connection right away. This means the second response will be ready and > waiting by the time the current response is fully read. Thanks, seems like an OK idea and the code seems straight-forward. Why is this a defaults-to-disabled option, instead of defaulting to enabled or just not having an option at all? I guess there may be a good reason, but I'd like to hear your thoughts on that. > --- > doc/demuxers.texi | 3 +++ > libavformat/hls.c | 51 > ++++++++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 51 insertions(+), 3 deletions(-) > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi > index 18ff7101ac..f2181fbb93 100644 > --- a/doc/demuxers.texi > +++ b/doc/demuxers.texi > @@ -319,6 +319,9 @@ Default value is 1000. > > @item http_persistent > Use persistent HTTP connections. Applicable only for HTTP streams. > + > +@item http_multiple > +Use multiple HTTP connections for downloading HTTP segments. > @end table > > @section image2 > diff --git a/libavformat/hls.c b/libavformat/hls.c > index f75c8f5eaa..487fa9a82f 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c [...] > @@ -1426,11 +1440,20 @@ reload: > if (ret) > return ret; > > - ret = open_input(c, v, seg, &v->input); > + if (c->http_multiple && v->input_next_requested) { > + AVIOContext *tmp = v->input; > + v->input = v->input_next; > + v->input_next = tmp; Use FFSWAP(). [...]
On Sun, Dec 17, 2017 at 12:53 PM Anssi Hannula <anssi.hannula@iki.fi> wrote: > Hi! > > Aman Gupta kirjoitti 2017-12-13 02:35: > > From: Aman Gupta <aman@tmm1.net> > > > > This improves network throughput of the hls demuxer by avoiding > > the latency introduced by downloading segments one at a time. > > > > The problem is particularly noticable over high-latency network > > connections: for instance, if RTT is 250ms, there will a 250ms idle > > period between when one segment response is read and the next one > > starts. > > > > The obvious solution to this is to use HTTP pipelining, where a > > second request can be sent (on the persistent http/1.1 connection) > > before the first response is fully read. Unfortunately the way the > > http protocol is implemented in avformat makes implementing pipleining > > very complex. > > > > Instead, this commit simulates pipelining using two separate persistent > > http connections. This has the advantage of working independently of > > the http_persistent option, and can be used with http/1.0 servers as > > well. The pair of connections is swapped every time a new segment > > starts > > downloading, and a request for the next segment is sent on the > > secondary > > connection right away. This means the second response will be ready and > > waiting by the time the current response is fully read. > > Thanks, seems like an OK idea and the code seems straight-forward. > > Why is this a defaults-to-disabled option, instead of defaulting to > enabled or just not having an option at all? > I guess there may be a good reason, but I'd like to hear your thoughts > on that. I actually had both options enabled by default but changed it before submitting. There is no good reason to disable by default other than the fact that it is new code and may still include bugs. However in my testing it works quite well so I think it is safe to enable by default and let users disable if they need to for some reason. > > > --- > > doc/demuxers.texi | 3 +++ > > libavformat/hls.c | 51 > > ++++++++++++++++++++++++++++++++++++++++++++++++--- > > 2 files changed, 51 insertions(+), 3 deletions(-) > > > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi > > index 18ff7101ac..f2181fbb93 100644 > > --- a/doc/demuxers.texi > > +++ b/doc/demuxers.texi > > @@ -319,6 +319,9 @@ Default value is 1000. > > > > @item http_persistent > > Use persistent HTTP connections. Applicable only for HTTP streams. > > + > > +@item http_multiple > > +Use multiple HTTP connections for downloading HTTP segments. > > @end table > > > > @section image2 > > diff --git a/libavformat/hls.c b/libavformat/hls.c > > index f75c8f5eaa..487fa9a82f 100644 > > --- a/libavformat/hls.c > > +++ b/libavformat/hls.c > [...] > > @@ -1426,11 +1440,20 @@ reload: > > if (ret) > > return ret; > > > > - ret = open_input(c, v, seg, &v->input); > > + if (c->http_multiple && v->input_next_requested) { > > + AVIOContext *tmp = v->input; > > + v->input = v->input_next; > > + v->input_next = tmp; > > Use FFSWAP(). Cool, didn't know about that one. Will switch. > > [...] > > -- > Anssi Hannula >
diff --git a/doc/demuxers.texi b/doc/demuxers.texi index 18ff7101ac..f2181fbb93 100644 --- a/doc/demuxers.texi +++ b/doc/demuxers.texi @@ -319,6 +319,9 @@ Default value is 1000. @item http_persistent Use persistent HTTP connections. Applicable only for HTTP streams. + +@item http_multiple +Use multiple HTTP connections for downloading HTTP segments. @end table @section image2 diff --git a/libavformat/hls.c b/libavformat/hls.c index f75c8f5eaa..487fa9a82f 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -96,6 +96,8 @@ struct playlist { uint8_t* read_buffer; AVIOContext *input; int input_read_done; + AVIOContext *input_next; + int input_next_requested; AVFormatContext *parent; int index; AVFormatContext *ctx; @@ -209,6 +211,7 @@ typedef struct HLSContext { char *allowed_extensions; int max_reload; int http_persistent; + int http_multiple; AVIOContext *playlist_pb; } HLSContext; @@ -261,6 +264,9 @@ static void free_playlist_list(HLSContext *c) if (pls->input) ff_format_io_close(c->ctx, &pls->input); pls->input_read_done = 0; + if (pls->input_next) + ff_format_io_close(c->ctx, &pls->input_next); + pls->input_next_requested = 0; if (pls->ctx) { pls->ctx->pb = NULL; avformat_close_input(&pls->ctx); @@ -920,6 +926,14 @@ static struct segment *current_segment(struct playlist *pls) return pls->segments[pls->cur_seq_no - pls->start_seq_no]; } +static struct segment *next_segment(struct playlist *pls) +{ + int n = pls->cur_seq_no - pls->start_seq_no + 1; + if (n >= pls->n_segments) + return NULL; + return pls->segments[n]; +} + enum ReadFromURLMode { READ_NORMAL, READ_COMPLETE, @@ -1361,6 +1375,7 @@ static int read_data(void *opaque, uint8_t *buf, int buf_size) int ret; int just_opened = 0; int reload_count = 0; + struct segment *seg; restart: if (!v->needed) @@ -1368,7 +1383,6 @@ restart: if (!v->input || (c->http_persistent && v->input_read_done)) { int64_t reload_interval; - struct segment *seg; /* Check that the playlist is still needed before opening a new * segment. */ @@ -1426,11 +1440,20 @@ reload: if (ret) return ret; - ret = open_input(c, v, seg, &v->input); + if (c->http_multiple && v->input_next_requested) { + AVIOContext *tmp = v->input; + v->input = v->input_next; + v->input_next = tmp; + v->input_next_requested = 0; + ret = 0; + } else { + ret = open_input(c, v, seg, &v->input); + } if (ret < 0) { if (ff_check_interrupt(c->interrupt_callback)) return AVERROR_EXIT; - av_log(v->parent, AV_LOG_WARNING, "Failed to open segment of playlist %d\n", + av_log(v->parent, AV_LOG_WARNING, "Failed to open segment %d of playlist %d\n", + v->cur_seq_no, v->index); v->cur_seq_no += 1; goto reload; @@ -1438,6 +1461,20 @@ reload: just_opened = 1; } + seg = next_segment(v); + if (c->http_multiple && !v->input_next_requested && seg) { + ret = open_input(c, v, seg, &v->input_next); + if (ret < 0) { + if (ff_check_interrupt(c->interrupt_callback)) + return AVERROR_EXIT; + av_log(v->parent, AV_LOG_WARNING, "Failed to open segment %d of playlist %d\n", + v->cur_seq_no + 1, + v->index); + } else { + v->input_next_requested = 1; + } + } + if (v->init_sec_buf_read_offset < v->init_sec_data_len) { /* Push init section out first before first actual segment */ int copy_size = FFMIN(v->init_sec_data_len - v->init_sec_buf_read_offset, buf_size); @@ -1960,6 +1997,9 @@ static int recheck_discard_flags(AVFormatContext *s, int first) if (pls->input) ff_format_io_close(pls->parent, &pls->input); pls->input_read_done = 0; + if (pls->input_next) + ff_format_io_close(pls->parent, &pls->input_next); + pls->input_next_requested = 0; pls->needed = 0; changed = 1; av_log(s, AV_LOG_INFO, "No longer receiving playlist %d\n", i); @@ -2195,6 +2235,9 @@ static int hls_read_seek(AVFormatContext *s, int stream_index, if (pls->input) ff_format_io_close(pls->parent, &pls->input); pls->input_read_done = 0; + if (pls->input_next) + ff_format_io_close(pls->parent, &pls->input_next); + pls->input_next_requested = 0; av_packet_unref(&pls->pkt); reset_packet(&pls->pkt); pls->pb.eof_reached = 0; @@ -2251,6 +2294,8 @@ static const AVOption hls_options[] = { OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 1000}, 0, INT_MAX, FLAGS}, {"http_persistent", "Use persistent HTTP connections", OFFSET(http_persistent), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS }, + {"http_multiple", "Use multiple HTTP connections for fetching segments", + OFFSET(http_multiple), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS}, {NULL} };
From: Aman Gupta <aman@tmm1.net> This improves network throughput of the hls demuxer by avoiding the latency introduced by downloading segments one at a time. The problem is particularly noticable over high-latency network connections: for instance, if RTT is 250ms, there will a 250ms idle period between when one segment response is read and the next one starts. The obvious solution to this is to use HTTP pipelining, where a second request can be sent (on the persistent http/1.1 connection) before the first response is fully read. Unfortunately the way the http protocol is implemented in avformat makes implementing pipleining very complex. Instead, this commit simulates pipelining using two separate persistent http connections. This has the advantage of working independently of the http_persistent option, and can be used with http/1.0 servers as well. The pair of connections is swapped every time a new segment starts downloading, and a request for the next segment is sent on the secondary connection right away. This means the second response will be ready and waiting by the time the current response is fully read. --- doc/demuxers.texi | 3 +++ libavformat/hls.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 3 deletions(-)