Re: [FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option

[Aman Gupta]

On Wed, Dec 20, 2017 at 11:36 AM, Aman Gupta  wrote:

>
>
> On Sun, Dec 17, 2017 at 1:13 PM, Anssi Hannula 
> wrote:
>
>> Aman Gupta kirjoitti 2017-12-17 22:41:
>>
>>> On Sun, Dec 17, 2017 at 12:34 PM Anssi Hannula 
>>> wrote:
>>>
>>> Hi!

 Aman Gupta kirjoitti 2017-12-13 02:35:
 > From: Aman Gupta 
 >
 > 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.
>

Patchset applied.

I ran some more tests before pushing to master, with:
`-i https://bitdash-a.akamaihd.net/content/sintel/hls/video/4000kbit.m3u8
-t 30 -f null /dev/null`

-http_persistent 0 -http_multiple 0
frame=  722 fps=296 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=12.3x
frame=  722 fps=324 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=13.4x

-http_persistent 0 -http_multiple 1
frame=  722 fps=304 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=12.6x
frame=  722 fps=325 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=13.5x

-http_persistent 1 -http_multiple 0
frame=  722 fps=682 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=28.3x
frame=  722 fps=713 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=29.6x

-http_persistent 1 -http_multiple 1
frame=  722 fps=0.0 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=39.4x
frame=  722 fps=0.0 q=-1.0 Lsize=N/A time=00:00:29.95 bitrate=N/A
speed=40.6x

In this case, the new default settings achieve a 3x throughput increase as
compared to the previous defaults.

Aman


>
> 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(>pb.buffer);
 >  if (pls->input)
 >  ff_format_io_close(c->ctx, >input);
 > +

Re: [FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option

[Aman Gupta]

On Sun, Dec 17, 2017 at 1:13 PM, Anssi Hannula  wrote:

> Aman Gupta kirjoitti 2017-12-17 22:41:
>
>> On Sun, Dec 17, 2017 at 12:34 PM Anssi Hannula 
>> wrote:
>>
>> Hi!
>>>
>>> Aman Gupta kirjoitti 2017-12-13 02:35:
>>> > From: Aman Gupta 
>>> >
>>> > 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(>pb.buffer);
>>> >  if (pls->input)
>>> >  ff_format_io_close(c->ctx, >input);
>>> > +pls->input_read_done = 0;
>>> >  if (pls->ctx) {
>>> >  pls->ctx->pb = NULL;
>>> >  avformat_close_input(>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, );
>>> > +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, >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, );
>>> > +}
>>> > +} else {
>>> > +ret = s->io_open(s, pb, url, AVIO_FLAG_READ, );
>>> > +}

Re: [FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option

[Aman Gupta]

On Sun, Dec 17, 2017 at 1:21 PM Anssi Hannula  wrote:

> Aman Gupta kirjoitti 2017-12-17 22:41:
> > On Sun, Dec 17, 2017 at 12:34 PM Anssi Hannula 
> > wrote:
> >
> >> Hi!
> >>
> >> Aman Gupta kirjoitti 2017-12-13 02:35:
> >> > From: Aman Gupta 
> >> >
> >> > 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.
> >> > +
> >> > +@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(>pb.buffer);
> >> >  if (pls->input)
> >> >  ff_format_io_close(c->ctx, >input);
> >> > +pls->input_read_done = 0;
> >> >  if (pls->ctx) {
> >> >  pls->ctx->pb = NULL;
> >> >  avformat_close_input(>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, );
> >> > +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);
> >> 

Re: [FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option

[Anssi Hannula]

Aman Gupta kirjoitti 2017-12-17 22:41:
On Sun, Dec 17, 2017 at 12:34 PM Anssi Hannula  
wrote:



Hi!

Aman Gupta kirjoitti 2017-12-13 02:35:
> From: Aman Gupta 
>
> 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(>pb.buffer);
>  if (pls->input)
>  ff_format_io_close(c->ctx, >input);
> +pls->input_read_done = 0;
>  if (pls->ctx) {
>  pls->ctx->pb = NULL;
>  avformat_close_input(>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, );
> +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, >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, );
> +}
> +} else {
> +ret = s->io_open(s, pb, url, AVIO_FLAG_READ, );
> +}
>  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, 

Re: [FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option

[Aman Gupta]

On Sun, Dec 17, 2017 at 12:34 PM Anssi Hannula  wrote:

> Hi!
>
> Aman Gupta kirjoitti 2017-12-13 02:35:
> > From: Aman Gupta 
> >
> > 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.


>
> > ---
> >  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(>pb.buffer);
> >  if (pls->input)
> >  ff_format_io_close(c->ctx, >input);
> > +pls->input_read_done = 0;
> >  if (pls->ctx) {
> >  pls->ctx->pb = NULL;
> >  avformat_close_input(>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, );
> > +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, >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, );
> > +}
> > +} else {
> > +ret = s->io_open(s, pb, url, AVIO_FLAG_READ, );
> > +}
> >  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, >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",
> > +  

Re: [FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option

[Anssi Hannula]

Hi!

Aman Gupta kirjoitti 2017-12-13 02:35:

From: Aman Gupta 

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?



---
 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(>pb.buffer);
 if (pls->input)
 ff_format_io_close(c->ctx, >input);
+pls->input_read_done = 0;
 if (pls->ctx) {
 pls->ctx->pb = NULL;
 avformat_close_input(>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, );
+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, >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, );
+}
+} else {
+ret = s->io_open(s, pb, url, AVIO_FLAG_READ, );
+}
 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, >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, >playlist_pb);
+in = NULL;
+}
+}
+


The above code seems duplicated, please put it in a common function.


[..]
--
Anssi Hannula
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel