[FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option
Aman Gupta
ffmpeg at tmm1.net
Mon Dec 18 00:41:24 EET 2017
On Sun, Dec 17, 2017 at 1:21 PM Anssi Hannula <anssi.hannula at iki.fi> wrote:
> Aman Gupta kirjoitti 2017-12-17 22:41:
> > On Sun, Dec 17, 2017 at 12:34 PM Anssi Hannula <anssi.hannula at iki.fi>
> > wrote:
> >
> >> Hi!
> >>
> >> Aman Gupta kirjoitti 2017-12-13 02:35:
> >> > From: Aman Gupta <aman at 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?
>
>
> Also, what happens if the hostname in the URI varies, does it properly
> open a new connection then?
Yes, ff_http_do_new_request will return EINVAL because of an earlier patch
in the series, and so the demuxer will print "keepalive request failed" and
fallback to a new connection.
I also tested with an http server configured to allow only N keepalive
requests on a socket. An ECONNRESET is similarly caught and printed, and
then a new connection is made.
There might be other cases or specific webserver/proxy behaviors not
correctly accounted for, which I why I think the option should be left in
place for now in case anyone need to force http/1.0 compatible behavior.
Aman
>
> >
> >>
> >> > ---
> >> > 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.
> >> > +
> >> > + at 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 at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> --
> Anssi Hannula
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
More information about the ffmpeg-devel
mailing list