| Message ID | 20171004220334.79264-3-ffmpeg@tmm1.net |
|---|---|
| State | New |
| Headers | show |
On Wed, 4 Oct 2017 15:03:34 -0700 Aman Gupta <ffmpeg@tmm1.net> wrote: > From: Aman Gupta <aman@tmm1.net> > > This teaches the HLS demuxer to use the HTTP protocols > multiple_requests=1 option, to take advantage of "Connection: > Keep-Alive" when downloading playlists and segments from the HLS server. > > With the new option, you can avoid TCP connection and TLS negotiation > overhead, which is particularly beneficial when streaming via a > high-latency internet connection. > --- > libavformat/hls.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 47 insertions(+), 4 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 786934af03..bf90582755 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -26,6 +26,7 @@ > * http://tools.ietf.org/html/draft-pantos-http-live-streaming > */ > > +#include "libavformat/http.h" > #include "libavutil/avstring.h" > #include "libavutil/avassert.h" > #include "libavutil/intreadwrite.h" > @@ -94,6 +95,7 @@ struct playlist { > AVIOContext pb; > uint8_t* read_buffer; > AVIOContext *input; > + int input_done; > AVFormatContext *parent; > int index; > AVFormatContext *ctx; > @@ -206,6 +208,8 @@ typedef struct HLSContext { > int strict_std_compliance; > char *allowed_extensions; > int max_reload; > + int http_keepalive; > + AVIOContext *playlist_pb; > } HLSContext; > > static int read_chomp_line(AVIOContext *s, char *buf, int maxlen) > @@ -640,7 +644,17 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url, > else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5)) > return AVERROR_INVALIDDATA; > > - ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > + if (c->http_keepalive && *pb && av_strstart(proto_name, "http", NULL)) { > + URLContext *uc = (URLContext *)av_opt_child_next(*pb, NULL); > + (*pb)->eof_reached = 0; > + ret = ff_http_do_new_request(uc, url); > + if (ret < 0) { > + ff_format_io_close(c->ctx, pb); > + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > + } > + } else { > + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > + } > if (ret >= 0) { > // update cookies on http response with setcookies. > char *new_cookies = NULL; > @@ -683,10 +697,20 @@ static int parse_playlist(HLSContext *c, const char *url, > char tmp_str[MAX_URL_SIZE]; > struct segment *cur_init_section = NULL; > > + if (!in && c->http_keepalive && c->playlist_pb) { > + in = c->playlist_pb; > + URLContext *uc = (URLContext *)av_opt_child_next(in, NULL); > + in->eof_reached = 0; > + ret = ff_http_do_new_request(uc, url); > + if (ret < 0) { > + ff_format_io_close(c->ctx, &c->playlist_pb); > + in = NULL; > + } > + } > + > if (!in) { > #if 1 > AVDictionary *opts = NULL; > - close_in = 1; > /* Some HLS servers don't like being sent the range header */ > av_dict_set(&opts, "seekable", "0", 0); > > @@ -696,10 +720,18 @@ static int parse_playlist(HLSContext *c, const char *url, > av_dict_set(&opts, "headers", c->headers, 0); > av_dict_set(&opts, "http_proxy", c->http_proxy, 0); > > + if (c->http_keepalive) > + av_dict_set(&opts, "multiple_requests", "1", 0); > + > ret = c->ctx->io_open(c->ctx, &in, url, AVIO_FLAG_READ, &opts); > av_dict_free(&opts); > if (ret < 0) > return ret; > + > + if (c->http_keepalive) > + c->playlist_pb = in; > + else > + close_in = 1; > #else > ret = open_in(c, &in, url); > if (ret < 0) > @@ -1111,6 +1143,9 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg) > av_dict_set(&opts, "http_proxy", c->http_proxy, 0); > av_dict_set(&opts, "seekable", "0", 0); > > + if (c->http_keepalive) > + av_dict_set(&opts, "multiple_requests", "1", 0); > + > if (seg->size >= 0) { > /* try to restrict the HTTP request to the part we want > * (if this is in fact a HTTP request) */ > @@ -1270,7 +1305,7 @@ restart: > if (!v->needed) > return AVERROR_EOF; > > - if (!v->input) { > + if (!v->input || (c->http_keepalive && v->input_done)) { > int64_t reload_interval; > struct segment *seg; > > @@ -1329,6 +1364,7 @@ reload: > goto reload; > } > > + v->input_done = 0; > seg = current_segment(v); > > /* load/update Media Initialization Section, if any */ > @@ -1366,7 +1402,11 @@ reload: > > return ret; > } > - ff_format_io_close(v->parent, &v->input); > + if (c->http_keepalive) { > + v->input_done = 1; > + } else { > + ff_format_io_close(v->parent, &v->input); > + } > v->cur_seq_no++; > > c->cur_seq_no = v->cur_seq_no; > @@ -1627,6 +1667,7 @@ static int hls_close(AVFormatContext *s) > free_rendition_list(c); > > av_dict_free(&c->avio_opts); > + ff_format_io_close(c->ctx, &c->playlist_pb); > > return 0; > } > @@ -2157,6 +2198,8 @@ static const AVOption hls_options[] = { > INT_MIN, INT_MAX, FLAGS}, > {"max_reload", "Maximum number of times a insufficient list is attempted to be reloaded", > OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 1000}, 0, INT_MAX, FLAGS}, > + {"http_keepalive", "re-use http connections when reloading playlists and fetching new segments", > + OFFSET(http_keepalive), AV_OPT_TYPE_BOOL, {.i64 = 0}, INT_MIN, 1, FLAGS}, > {NULL} > }; > This is pretty hacky of course. But it's likely also rather important for good performance, so a short term hack into this direction might be ok. In the long term, I think we should have proper support for this: some sort of connection pool abstraction, which could possibly also take care of managing persistent cookies and the user-agent, which is done in a very hacky way currently (and duplicated across HLS and DASH). I would argue that to get the short term hack accepted, it should at least move the state to a separate struct, which also hosts the cached connection. I don't really see where this patch checks whether the host changes? The minimal requirements for reusing the connection would be using the same protocol and the same host.
On Wed, Oct 04, 2017 at 15:03:34 -0700, Aman Gupta wrote: > From: Aman Gupta <aman@tmm1.net> > > This teaches the HLS demuxer to use the HTTP protocols > multiple_requests=1 option, to take advantage of "Connection: > Keep-Alive" when downloading playlists and segments from the HLS server. Nice. Quick test: A random akamaihd.net HLS stream (they provide great bandwidth) with quite small segments (~10s per segment) goes from 14.8x to 16x. The overhead reduction seems significant, and this isn't even https segments. Issue: The hls demuxer now inconsistently no longer reports each opened URL, only the first one. (Reporting of these at default log-level was only recently added though.) > > With the new option, you can avoid TCP connection and TLS negotiation > overhead, which is particularly beneficial when streaming via a > high-latency internet connection. > --- > libavformat/hls.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 47 insertions(+), 4 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 786934af03..bf90582755 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -26,6 +26,7 @@ > * http://tools.ietf.org/html/draft-pantos-http-live-streaming > */ > > +#include "libavformat/http.h" > #include "libavutil/avstring.h" > #include "libavutil/avassert.h" > #include "libavutil/intreadwrite.h" > @@ -94,6 +95,7 @@ struct playlist { > AVIOContext pb; > uint8_t* read_buffer; > AVIOContext *input; > + int input_done; > AVFormatContext *parent; > int index; > AVFormatContext *ctx; > @@ -206,6 +208,8 @@ typedef struct HLSContext { > int strict_std_compliance; > char *allowed_extensions; > int max_reload; > + int http_keepalive; > + AVIOContext *playlist_pb; > } HLSContext; > > static int read_chomp_line(AVIOContext *s, char *buf, int maxlen) > @@ -640,7 +644,17 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url, > else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5)) > return AVERROR_INVALIDDATA; > > - ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > + if (c->http_keepalive && *pb && av_strstart(proto_name, "http", NULL)) { > + URLContext *uc = (URLContext *)av_opt_child_next(*pb, NULL); > + (*pb)->eof_reached = 0; > + ret = ff_http_do_new_request(uc, url); > + if (ret < 0) { > + ff_format_io_close(c->ctx, pb); > + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > + } > + } else { > + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > + } > if (ret >= 0) { > // update cookies on http response with setcookies. > char *new_cookies = NULL; > @@ -683,10 +697,20 @@ static int parse_playlist(HLSContext *c, const char *url, > char tmp_str[MAX_URL_SIZE]; > struct segment *cur_init_section = NULL; > > + if (!in && c->http_keepalive && c->playlist_pb) { > + in = c->playlist_pb; > + URLContext *uc = (URLContext *)av_opt_child_next(in, NULL); > + in->eof_reached = 0; > + ret = ff_http_do_new_request(uc, url); > + if (ret < 0) { > + ff_format_io_close(c->ctx, &c->playlist_pb); > + in = NULL; > + } > + } > + > if (!in) { > #if 1 > AVDictionary *opts = NULL; > - close_in = 1; > /* Some HLS servers don't like being sent the range header */ > av_dict_set(&opts, "seekable", "0", 0); > > @@ -696,10 +720,18 @@ static int parse_playlist(HLSContext *c, const char *url, > av_dict_set(&opts, "headers", c->headers, 0); > av_dict_set(&opts, "http_proxy", c->http_proxy, 0); > > + if (c->http_keepalive) > + av_dict_set(&opts, "multiple_requests", "1", 0); > + > ret = c->ctx->io_open(c->ctx, &in, url, AVIO_FLAG_READ, &opts); > av_dict_free(&opts); > if (ret < 0) > return ret; > + > + if (c->http_keepalive) > + c->playlist_pb = in; > + else > + close_in = 1; > #else > ret = open_in(c, &in, url); > if (ret < 0) > @@ -1111,6 +1143,9 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg) > av_dict_set(&opts, "http_proxy", c->http_proxy, 0); > av_dict_set(&opts, "seekable", "0", 0); > > + if (c->http_keepalive) > + av_dict_set(&opts, "multiple_requests", "1", 0); > + > if (seg->size >= 0) { > /* try to restrict the HTTP request to the part we want > * (if this is in fact a HTTP request) */ > @@ -1270,7 +1305,7 @@ restart: > if (!v->needed) > return AVERROR_EOF; > > - if (!v->input) { > + if (!v->input || (c->http_keepalive && v->input_done)) { > int64_t reload_interval; > struct segment *seg; > > @@ -1329,6 +1364,7 @@ reload: > goto reload; > } > > + v->input_done = 0; > seg = current_segment(v); > > /* load/update Media Initialization Section, if any */ > @@ -1366,7 +1402,11 @@ reload: > > return ret; > } > - ff_format_io_close(v->parent, &v->input); > + if (c->http_keepalive) { > + v->input_done = 1; > + } else { > + ff_format_io_close(v->parent, &v->input); > + } > v->cur_seq_no++; > > c->cur_seq_no = v->cur_seq_no; > @@ -1627,6 +1667,7 @@ static int hls_close(AVFormatContext *s) > free_rendition_list(c); > > av_dict_free(&c->avio_opts); > + ff_format_io_close(c->ctx, &c->playlist_pb); > > return 0; > } > @@ -2157,6 +2198,8 @@ static const AVOption hls_options[] = { > INT_MIN, INT_MAX, FLAGS}, > {"max_reload", "Maximum number of times a insufficient list is attempted to be reloaded", > OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 1000}, 0, INT_MAX, FLAGS}, > + {"http_keepalive", "re-use http connections when reloading playlists and fetching new segments", > + OFFSET(http_keepalive), AV_OPT_TYPE_BOOL, {.i64 = 0}, INT_MIN, 1, FLAGS}, > {NULL} > }; > > -- > 2.13.5 (Apple Git-94) > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Thu, Oct 05, 2017 at 13:41:07 +0200, wm4 wrote: > I don't really see where this patch checks whether the host changes? > The minimal requirements for reusing the connection would be using > the same protocol and the same host. I *believe* it just tries to reuse the connection and falls back to re-opening on failure. On any failure - that could be a cookie issue, a timeout, or whatever: > + ret = ff_http_do_new_request(uc, url); > + if (ret < 0) { > + ff_format_io_close(c->ctx, pb); > + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > + } I wonder what the patch as a whole does if the server doesn't support this? The same as above - just fail and reconnect? Does the hls demuxer support HTTP 1.0? This would need a "Connection: keep-alive" header in the request (and such a response from the server to confirm). Moritz
On Thu, 5 Oct 2017 16:39:25 +0200 Moritz Barsnick <barsnick@gmx.net> wrote: > On Thu, Oct 05, 2017 at 13:41:07 +0200, wm4 wrote: > > I don't really see where this patch checks whether the host changes? > > The minimal requirements for reusing the connection would be using > > the same protocol and the same host. > > I *believe* it just tries to reuse the connection and falls back to > re-opening on failure. On any failure - that could be a cookie issue, a > timeout, or whatever: Couldn't find anything that would make ff_http_do_new_request fail if the host changed, but maybe I didn't look close enough. > > + ret = ff_http_do_new_request(uc, url); > > + if (ret < 0) { > > + ff_format_io_close(c->ctx, pb); > > + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); > > + } > > I wonder what the patch as a whole does if the server doesn't support > this? The same as above - just fail and reconnect? I'd think so. > Does the hls demuxer support HTTP 1.0? This would need a "Connection: > keep-alive" header in the request (and such a response from the > server to confirm). That's definitely added. Not sure what happens if the server doesn't support it.
2017-10-05 16:32 GMT+02:00 Moritz Barsnick <barsnick@gmx.net>: > Issue: The hls demuxer now inconsistently no longer reports > each opened URL, only the first one. (Reporting of these at > default log-level was only recently added though.) Could you remind me why this was useful? I was about to start a patch to stop printing those debug lines... Carl Eugen
On Thu, Oct 5, 2017 at 10:21 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-10-05 16:32 GMT+02:00 Moritz Barsnick <barsnick@gmx.net>: > >> Issue: The hls demuxer now inconsistently no longer reports >> each opened URL, only the first one. (Reporting of these at >> default log-level was only recently added though.) > > Could you remind me why this was useful? > I was about to start a patch to stop printing those > debug lines... > Supposedly its a "security" feature. - Hendrik
2017-10-06 0:19 GMT+02:00 Hendrik Leppkes <h.leppkes@gmail.com>: > On Thu, Oct 5, 2017 at 10:21 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2017-10-05 16:32 GMT+02:00 Moritz Barsnick <barsnick@gmx.net>: >> >>> Issue: The hls demuxer now inconsistently no longer reports >>> each opened URL, only the first one. (Reporting of these at >>> default log-level was only recently added though.) >> >> Could you remind me why this was useful? >> I was about to start a patch to stop printing those >> debug lines... > > Supposedly its a "security" feature. Thank you, Carl Eugen
On Thu, Oct 05, 2017 at 22:21:09 +0200, Carl Eugen Hoyos wrote:
> Could you remind me why this was useful?
It never was to me. (Except at loglevel debug and above.)
Moritz
On Fri, 6 Oct 2017 00:19:33 +0200 Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Thu, Oct 5, 2017 at 10:21 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2017-10-05 16:32 GMT+02:00 Moritz Barsnick <barsnick@gmx.net>: > > > >> Issue: The hls demuxer now inconsistently no longer reports > >> each opened URL, only the first one. (Reporting of these at > >> default log-level was only recently added though.) > > > > Could you remind me why this was useful? > > I was about to start a patch to stop printing those > > debug lines... > > > > Supposedly its a "security" feature. wat
diff --git a/libavformat/hls.c b/libavformat/hls.c index 786934af03..bf90582755 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -26,6 +26,7 @@ * http://tools.ietf.org/html/draft-pantos-http-live-streaming */ +#include "libavformat/http.h" #include "libavutil/avstring.h" #include "libavutil/avassert.h" #include "libavutil/intreadwrite.h" @@ -94,6 +95,7 @@ struct playlist { AVIOContext pb; uint8_t* read_buffer; AVIOContext *input; + int input_done; AVFormatContext *parent; int index; AVFormatContext *ctx; @@ -206,6 +208,8 @@ typedef struct HLSContext { int strict_std_compliance; char *allowed_extensions; int max_reload; + int http_keepalive; + AVIOContext *playlist_pb; } HLSContext; static int read_chomp_line(AVIOContext *s, char *buf, int maxlen) @@ -640,7 +644,17 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url, else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5)) return AVERROR_INVALIDDATA; - ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); + if (c->http_keepalive && *pb && av_strstart(proto_name, "http", NULL)) { + URLContext *uc = (URLContext *)av_opt_child_next(*pb, NULL); + (*pb)->eof_reached = 0; + ret = ff_http_do_new_request(uc, url); + if (ret < 0) { + ff_format_io_close(c->ctx, pb); + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); + } + } else { + ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp); + } if (ret >= 0) { // update cookies on http response with setcookies. char *new_cookies = NULL; @@ -683,10 +697,20 @@ static int parse_playlist(HLSContext *c, const char *url, char tmp_str[MAX_URL_SIZE]; struct segment *cur_init_section = NULL; + if (!in && c->http_keepalive && c->playlist_pb) { + in = c->playlist_pb; + URLContext *uc = (URLContext *)av_opt_child_next(in, NULL); + in->eof_reached = 0; + ret = ff_http_do_new_request(uc, url); + if (ret < 0) { + ff_format_io_close(c->ctx, &c->playlist_pb); + in = NULL; + } + } + if (!in) { #if 1 AVDictionary *opts = NULL; - close_in = 1; /* Some HLS servers don't like being sent the range header */ av_dict_set(&opts, "seekable", "0", 0); @@ -696,10 +720,18 @@ static int parse_playlist(HLSContext *c, const char *url, av_dict_set(&opts, "headers", c->headers, 0); av_dict_set(&opts, "http_proxy", c->http_proxy, 0); + if (c->http_keepalive) + av_dict_set(&opts, "multiple_requests", "1", 0); + ret = c->ctx->io_open(c->ctx, &in, url, AVIO_FLAG_READ, &opts); av_dict_free(&opts); if (ret < 0) return ret; + + if (c->http_keepalive) + c->playlist_pb = in; + else + close_in = 1; #else ret = open_in(c, &in, url); if (ret < 0) @@ -1111,6 +1143,9 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg) av_dict_set(&opts, "http_proxy", c->http_proxy, 0); av_dict_set(&opts, "seekable", "0", 0); + if (c->http_keepalive) + av_dict_set(&opts, "multiple_requests", "1", 0); + if (seg->size >= 0) { /* try to restrict the HTTP request to the part we want * (if this is in fact a HTTP request) */ @@ -1270,7 +1305,7 @@ restart: if (!v->needed) return AVERROR_EOF; - if (!v->input) { + if (!v->input || (c->http_keepalive && v->input_done)) { int64_t reload_interval; struct segment *seg; @@ -1329,6 +1364,7 @@ reload: goto reload; } + v->input_done = 0; seg = current_segment(v); /* load/update Media Initialization Section, if any */ @@ -1366,7 +1402,11 @@ reload: return ret; } - ff_format_io_close(v->parent, &v->input); + if (c->http_keepalive) { + v->input_done = 1; + } else { + ff_format_io_close(v->parent, &v->input); + } v->cur_seq_no++; c->cur_seq_no = v->cur_seq_no; @@ -1627,6 +1667,7 @@ static int hls_close(AVFormatContext *s) free_rendition_list(c); av_dict_free(&c->avio_opts); + ff_format_io_close(c->ctx, &c->playlist_pb); return 0; } @@ -2157,6 +2198,8 @@ static const AVOption hls_options[] = { INT_MIN, INT_MAX, FLAGS}, {"max_reload", "Maximum number of times a insufficient list is attempted to be reloaded", OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 1000}, 0, INT_MAX, FLAGS}, + {"http_keepalive", "re-use http connections when reloading playlists and fetching new segments", + OFFSET(http_keepalive), AV_OPT_TYPE_BOOL, {.i64 = 0}, INT_MIN, 1, FLAGS}, {NULL} };
From: Aman Gupta <aman@tmm1.net> This teaches the HLS demuxer to use the HTTP protocols multiple_requests=1 option, to take advantage of "Connection: Keep-Alive" when downloading playlists and segments from the HLS server. With the new option, you can avoid TCP connection and TLS negotiation overhead, which is particularly beneficial when streaming via a high-latency internet connection. --- libavformat/hls.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 4 deletions(-)