On Sun, Dec 17, 2017 at 1:13 PM, Anssi Hannula <anssi.hann...@iki.fi> wrote:
> Aman Gupta kirjoitti 2017-12-17 22:41: > >> On Sun, Dec 17, 2017 at 12:34 PM Anssi Hannula <anssi.hann...@iki.fi> >> wrote: >> >> Hi! >>> >>> Aman Gupta kirjoitti 2017-12-13 02:35: >>> > From: Aman Gupta <a...@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. >>> > >>> > Similar to the http_persistent option recently implemented in hlsenc.c >>> >>> Why is this implemented as an option instead of simply being always used >>> by the demuxer? >>> >> >> >> I have no strong feeling on this either way. I've tested the new option >> against a few different HLS servers and would be comfortable enabling it >> by >> default. I do think it's worth keeping as an option in case someone wants >> to turn it off for whatever reason. >> > > OK. The only other demuxer that reuses HTTP connections seems to be > rtmphttp, and it does not have an option. > I think I'd prefer no option here as well unless a use case is known, but > I'm not too strongly against it so I guess it could stay (but default to > true). > > Does anyone else have any thoughts? > If no one objects, I will push this patchset with the requested changed in a few days. Aman > > > Also, what happens if the hostname in the URI varies, does it properly > open a new connection then? > > >> >>> > --- >>> > doc/demuxers.texi | 3 +++ >>> > libavformat/hls.c | 68 >>> > +++++++++++++++++++++++++++++++++++++++++++++++++++---- >>> > 2 files changed, 67 insertions(+), 4 deletions(-) >>> > >>> > diff --git a/doc/demuxers.texi b/doc/demuxers.texi >>> > index 73dc0feec1..18ff7101ac 100644 >>> > --- a/doc/demuxers.texi >>> > +++ b/doc/demuxers.texi >>> > @@ -316,6 +316,9 @@ segment index to start live streams at (negative >>> > values are from the end). >>> > @item max_reload >>> > Maximum number of times a insufficient list is attempted to be >>> > reloaded. >>> > Default value is 1000. >>> > + >>> > +@item http_persistent >>> > +Use persistent HTTP connections. Applicable only for HTTP streams. >>> > @end table >>> > >>> > @section image2 >>> > diff --git a/libavformat/hls.c b/libavformat/hls.c >>> > index ab6ff187a6..5c1895c180 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_read_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_persistent; >>> > + AVIOContext *playlist_pb; >>> > } HLSContext; >>> > >>> > static int read_chomp_line(AVIOContext *s, char *buf, int maxlen) >>> > @@ -256,6 +260,7 @@ static void free_playlist_list(HLSContext *c) >>> > av_freep(&pls->pb.buffer); >>> > if (pls->input) >>> > ff_format_io_close(c->ctx, &pls->input); >>> > + pls->input_read_done = 0; >>> > if (pls->ctx) { >>> > pls->ctx->pb = NULL; >>> > avformat_close_input(&pls->ctx); >>> > @@ -640,7 +645,24 @@ 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_persistent && *pb && av_strstart(proto_name, "http", >>> > NULL)) { >>> > + URLContext *uc = ffio_geturlcontext(*pb); >>> > + av_assert0(uc); >>> > + (*pb)->eof_reached = 0; >>> > + ret = ff_http_do_new_request(uc, url); >>> > + if (ret == AVERROR_EXIT) { >>> > + ff_format_io_close(c->ctx, &c->playlist_pb); >>> > + return ret; >>> > + } else if (ret < 0) { >>> > + av_log(s, AV_LOG_WARNING, >>> > + "keepalive request failed for '%s', retrying with new >>> > connection: %s\n", >>> > + url, av_err2str(ret)); >>> > + 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 +705,27 @@ 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_persistent && c->playlist_pb) { >>> > + in = c->playlist_pb; >>> > + URLContext *uc = ffio_geturlcontext(in); >>> > + av_assert0(uc); >>> > + in->eof_reached = 0; >>> > + ret = ff_http_do_new_request(uc, url); >>> > + if (ret == AVERROR_EXIT) { >>> > + ff_format_io_close(c->ctx, &c->playlist_pb); >>> > + return ret; >>> > + } else if (ret < 0) { >>> > + av_log(c->ctx, AV_LOG_WARNING, >>> > + "keepalive request failed for '%s', retrying with new >>> > connection: %s\n", >>> > + url, av_err2str(ret)); >>> > + ff_format_io_close(c->ctx, &c->playlist_pb); >>> > + in = NULL; >>> > + } >>> > + } >>> > + >>> >>> The above code seems duplicated, please put it in a common function. >>> >> >> >> Sure, will do. >> >> Thanks for reviewing! >> >> >> >>> >>> [..] >>> -- >>> Anssi Hannula >>> >>> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > -- > Anssi Hannula > _______________________________________________ > 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
