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?


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

Reply via email to