[FFmpeg-devel] [PATCH v2 3/5] avformat/hls: add http_persistent option
Aman Gupta
ffmpeg at tmm1.net
Sat Dec 23 01:02:37 EET 2017
On Wed, Dec 20, 2017 at 11:36 AM, Aman Gupta <ffmpeg at tmm1.net> wrote:
>
>
> On Sun, Dec 17, 2017 at 1:13 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?
>>
>
> 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.
>>>> > +
>>>> > + 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