diff mbox

[FFmpeg-devel,v2,3/3] libavformat/hlsenc: Persistent HTTP connections supported as an option

Message ID 1511240064-18791-3-git-send-email-kjeyapal@akamai.com
State Superseded
Headers show

Commit Message

Jeyapal, Karthick Nov. 21, 2017, 4:54 a.m. UTC
---
 doc/muxers.texi      |  3 +++
 libavformat/hlsenc.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 7 deletions(-)

Comments

Steven Liu Nov. 21, 2017, 2:48 p.m. UTC | #1
2017-11-21 12:54 GMT+08:00 Karthick J <kjeyapal@akamai.com>:
> ---
>  doc/muxers.texi      |  3 +++
>  libavformat/hlsenc.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index 0bb8ad2..c1d753b 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -850,6 +850,9 @@ ffmpeg -re -i in.ts -f hls -master_pl_name master.m3u8 \
>  This example creates HLS master playlist with name master.m3u8 and keep
>  publishing it repeatedly every after 30 segments i.e. every after 60s.
>
> +@item http_persistent
> +Use persistent HTTP connections. Applicable only for HTTP output.
> +
>  @end table
>
>  @anchor{ico}
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 3c47ced..32c99a0 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -45,6 +45,7 @@
>
>  #include "avformat.h"
>  #include "avio_internal.h"
> +#include "http.h"
>  #include "internal.h"
>  #include "os_support.h"
>
> @@ -204,6 +205,7 @@ typedef struct HLSContext {
>      char *var_stream_map; /* user specified variant stream map string */
>      char *master_pl_name;
>      unsigned int master_publish_rate;
> +    int http_persistent;
>  } HLSContext;
>
>  static int get_int_from_double(double val)
> @@ -244,10 +246,38 @@ static int mkdir_p(const char *path) {
>      return ret;
>  }
>
> +static int is_http_proto(char *filename) {
> +    const char *proto = avio_find_protocol_name(filename);
> +    return proto ? (!av_strcasecmp(proto, "http") || !av_strcasecmp(proto, "https")) : 0;
> +}
> +
> +static int hlsenc_io_open(AVFormatContext *s, AVIOContext **pb, char *filename,
> +                          AVDictionary **options) {
> +    HLSContext *hls = s->priv_data;
> +    int http_base_proto = is_http_proto(filename);
> +    int err;
> +    if (*pb == NULL || !http_base_proto || !hls->http_persistent) {
What about !*pb ?
> +        err = s->io_open(s, pb, filename, AVIO_FLAG_WRITE, options);
> +    } else {
> +        URLContext *http_url_context = ffio_geturlcontext(*pb);
> +        av_assert0(http_url_context);
> +        err = ff_http_do_new_request(http_url_context, filename);
> +    }
> +    return err;
> +}
> +
> +static void hlsenc_io_close(AVFormatContext *s, AVIOContext **pb, char *filename) {
> +    HLSContext *hls = s->priv_data;
> +    int http_base_proto = is_http_proto(filename);
> +
> +    if (!http_base_proto || !hls->http_persistent || hls->key_info_file || hls->encrypt) {
> +        ff_format_io_close(s, pb);
> +    }
> +}
> +
>  static void set_http_options(AVFormatContext *s, AVDictionary **options, HLSContext *c)
>  {
> -    const char *proto = avio_find_protocol_name(s->filename);
> -    int http_base_proto = proto ? (!av_strcasecmp(proto, "http") || !av_strcasecmp(proto, "https")) : 0;
> +    int http_base_proto = is_http_proto(s->filename);
>
>      if (c->method) {
>          av_dict_set(options, "method", c->method, 0);
> @@ -257,6 +287,8 @@ static void set_http_options(AVFormatContext *s, AVDictionary **options, HLSCont
>      }
>      if (c->user_agent)
>          av_dict_set(options, "user_agent", c->user_agent, 0);
> +    if (c->http_persistent)
> +        av_dict_set_int(options, "multiple_requests", 1, 0);
>
>  }
>
> @@ -1430,17 +1462,17 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
>              err = AVERROR(ENOMEM);
>              goto fail;
>          }
> -        err = s->io_open(s, &oc->pb, filename, AVIO_FLAG_WRITE, &options);
> +        err = hlsenc_io_open(s, &oc->pb, filename, &options);
>          av_free(filename);
>          av_dict_free(&options);
>          if (err < 0)
>              return err;
>      } else
> -        if ((err = s->io_open(s, &oc->pb, oc->filename, AVIO_FLAG_WRITE, &options)) < 0)
> +        if ((err = hlsenc_io_open(s, &oc->pb, oc->filename, &options)) < 0)
>              goto fail;
>      if (vs->vtt_basename) {
>          set_http_options(s, &options, c);
> -        if ((err = s->io_open(s, &vtt_oc->pb, vtt_oc->filename, AVIO_FLAG_WRITE, &options)) < 0)
> +        if ((err = hlsenc_io_open(s, &vtt_oc->pb, vtt_oc->filename, &options)) < 0)
>              goto fail;
>      }
>      av_dict_free(&options);
> @@ -2148,11 +2180,12 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>                  avio_open_dyn_buf(&oc->pb);
>                  vs->packets_written = 0;
>                  ff_format_io_close(s, &vs->out);
> +                hlsenc_io_close(s, &vs->out, vs->base_output_dirname);
>              } else {
> -                ff_format_io_close(s, &oc->pb);
> +                hlsenc_io_close(s, &oc->pb, oc->filename);
>              }
>              if (vs->vtt_avf) {
> -                ff_format_io_close(s, &vs->vtt_avf->pb);
> +                hlsenc_io_close(s, &vs->vtt_avf->pb, vs->vtt_avf->filename);
>              }
>          }
>          if ((hls->flags & HLS_TEMP_FILE) && oc->filename[0]) {
> @@ -2337,6 +2370,7 @@ static const AVOption options[] = {
>      {"var_stream_map", "Variant stream map string", OFFSET(var_stream_map), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>      {"master_pl_name", "Create HLS master playlist with this name", OFFSET(master_pl_name), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
>      {"master_pl_publish_rate", "Publish master play list every after this many segment intervals", OFFSET(master_publish_rate), AV_OPT_TYPE_INT, {.i64 = 0}, 0, UINT_MAX, E},
> +    {"http_persistent", "Use persistent HTTP connections", OFFSET(http_persistent), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, E },
>      { NULL },
>  };
>
> --
> 1.9.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jeyapal, Karthick Nov. 22, 2017, 3:39 a.m. UTC | #2
>On 11/21/17, 8:24 PM, "Steven Liu" <lingjiujianke@gmail.com> wrote:

>

[…]
>> +    if (*pb == NULL || !http_base_proto || !hls->http_persistent) {

>What about !*pb ?


Thanks for your comments. 
I have modified the patch as per your suggestion and have attached the same.

Regards,
Karthick
Liu Steven Nov. 22, 2017, 4:01 a.m. UTC | #3
> 在 2017年11月22日,11:39,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:
> 
>> On 11/21/17, 8:24 PM, "Steven Liu" <lingjiujianke@gmail.com> wrote:
>> 
> […]
>>> +    if (*pb == NULL || !http_base_proto || !hls->http_persistent) {
>> What about !*pb ?
> 
> Thanks for your comments. 
> I have modified the patch as per your suggestion and have attached the same.
> 
This patch is ok, but i see it need an API: ffio_geturlcontext

it in the other patch, i see it have some thing need talk? If that is ok, this patch should be apply.

Thanks
> Regards,
> Karthick
> 
> 
> <0003-libavformat-hlsenc-Persistent-HTTP-connections-suppo.patch>_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jeyapal, Karthick Nov. 22, 2017, 7:56 a.m. UTC | #4
>On 11/22/17, 9:32 AM, "刘歧" <lq@chinaffmpeg.org> wrote:

>

>This patch is ok, but i see it need an API: ffio_geturlcontext

>

>it in the other patch, i see it have some thing need talk? If that is ok, this patch should be apply.

Thanks for the reply. 
All concerns raised by Nicolas has been addressed in this latest patch set.
Let us wait for few days to check if anyone still has any objections.
>

>Thanks


regards,
Karthick
Jeyapal, Karthick Nov. 24, 2017, 10:02 a.m. UTC | #5
On 11/22/17, 1:27 PM, "Jeyapal, Karthick" <kjeyapal@akamai.com> wrote:

>>On 11/22/17, 9:32 AM, "刘歧" <lq@chinaffmpeg.org> wrote:

>>

>>This patch is ok, but i see it need an API: ffio_geturlcontext

>>

>>it in the other patch, i see it have some thing need talk? If that is ok, this patch should be apply.

>Thanks for the reply. 

>All concerns raised by Nicolas has been addressed in this latest patch set.

>Let us wait for few days to check if anyone still has any objections.

Looks like nobody has anymore objections to this patchset. 
Well, it has already passed through multiple rounds of reviews.
Could somebody please push this to the master. 
I have few more patches waiting to be submitted, dependent on this feature.
>>

>>Thanks


regards,
Karthick
Liu Steven Nov. 24, 2017, 10:10 a.m. UTC | #6
> 在 2017年11月24日,18:02,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:
> 
> On 11/22/17, 1:27 PM, "Jeyapal, Karthick" <kjeyapal@akamai.com> wrote:
> 
>>> On 11/22/17, 9:32 AM, "刘歧" <lq@chinaffmpeg.org> wrote:
>>> 
>>> This patch is ok, but i see it need an API: ffio_geturlcontext
>>> 
>>> it in the other patch, i see it have some thing need talk? If that is ok, this patch should be apply.
Is this ok?

Thanks
>> Thanks for the reply. 
>> All concerns raised by Nicolas has been addressed in this latest patch set.
>> Let us wait for few days to check if anyone still has any objections.
> Looks like nobody has anymore objections to this patchset. 
> Well, it has already passed through multiple rounds of reviews.
> Could somebody please push this to the master. 
> I have few more patches waiting to be submitted, dependent on this feature.
>>> 
>>> Thanks
> 
> regards,
> Karthick
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jeyapal, Karthick Nov. 24, 2017, 10:18 a.m. UTC | #7
On 11/24/17, 3:40 PM, "刘歧" <lq@chinaffmpeg.org> wrote:

>> 在 2017年11月24日,18:02,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:

>> 

>> On 11/22/17, 1:27 PM, "Jeyapal, Karthick" <kjeyapal@akamai.com> wrote:

>> 

>>>> On 11/22/17, 9:32 AM, "刘歧" <lq@chinaffmpeg.org> wrote:

>>>> 

>>>> This patch is ok, but i see it need an API: ffio_geturlcontext

>>>> 

>>>> it in the other patch, i see it have some thing need talk? If that is ok, this patch should be apply.

>Is this ok?

Yes, ffio_geturlcontext has already been accepted by Nicolas.
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220014.html
He had concerns on calling prot->url_write directly from hlsenc, which has been removed on new patchset(v2).
Also av_assert0 has been added as suggested by him
>

>Thanks

>>> Thanks for the reply. 

>>> All concerns raised by Nicolas has been addressed in this latest patch set.

>>> Let us wait for few days to check if anyone still has any objections.

>> Looks like nobody has anymore objections to this patchset. 

>> Well, it has already passed through multiple rounds of reviews.

>> Could somebody please push this to the master. 

>> I have few more patches waiting to be submitted, dependent on this feature.
Jeyapal, Karthick Nov. 29, 2017, 4:05 a.m. UTC | #8
On 11/24/17, 3:49 PM, "Jeyapal, Karthick" <kjeyapal@akamai.com> wrote:
>

>On 11/24/17, 3:40 PM, "刘歧" <lq@chinaffmpeg.org> wrote:

>

>>> 在 2017年11月24日,18:02,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:

>>> 

>>> On 11/22/17, 1:27 PM, "Jeyapal, Karthick" <kjeyapal@akamai.com> wrote:

>>> 

>>>>> On 11/22/17, 9:32 AM, "刘歧" <lq@chinaffmpeg.org> wrote:

>>>>> 

>>>>> This patch is ok, but i see it need an API: ffio_geturlcontext

>>>>> 

>>>>> it in the other patch, i see it have some thing need talk? If that is ok, this patch should be apply.

>>Is this ok?

>Yes, ffio_geturlcontext has already been accepted by Nicolas.

>http://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220014.html

>He had concerns on calling prot->url_write directly from hlsenc, which has been removed on new patchset(v2).

>Also av_assert0 has been added as suggested by him

A gentle reminder. 
7 days have gone by since the last version of this patchset was submitted. And no objections so far.
Could this patchset be pushed, please? Thanks.
>>

>>Thanks

>>>> Thanks for the reply. 

>>>> All concerns raised by Nicolas has been addressed in this latest patch set.

>>>> Let us wait for few days to check if anyone still has any objections.

>>> Looks like nobody has anymore objections to this patchset. 

>>> Well, it has already passed through multiple rounds of reviews.

>>> Could somebody please push this to the master. 

>>> I have few more patches waiting to be submitted, dependent on this feature.
Liu Steven Nov. 29, 2017, 6:11 a.m. UTC | #9
> 在 2017年11月29日,12:05,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:
> 
> 
> On 11/24/17, 3:49 PM, "Jeyapal, Karthick" <kjeyapal@akamai.com> wrote:
>> 
>> On 11/24/17, 3:40 PM, "刘歧" <lq@chinaffmpeg.org> wrote:
>> 
>>>> 在 2017年11月24日,18:02,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:
>>>> 
>>>> On 11/22/17, 1:27 PM, "Jeyapal, Karthick" <kjeyapal@akamai.com> wrote:
>>>> 
>>>>>> On 11/22/17, 9:32 AM, "刘歧" <lq@chinaffmpeg.org> wrote:
>>>>>> 
>>>>>> This patch is ok, but i see it need an API: ffio_geturlcontext
>>>>>> 
>>>>>> it in the other patch, i see it have some thing need talk? If that is ok, this patch should be apply.
>>> Is this ok?
>> Yes, ffio_geturlcontext has already been accepted by Nicolas.
>> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220014.html
>> He had concerns on calling prot->url_write directly from hlsenc, which has been removed on new patchset(v2).
>> Also av_assert0 has been added as suggested by him
> A gentle reminder. 
> 7 days have gone by since the last version of this patchset was submitted. And no objections so far.
> Could this patchset be pushed, please? Thanks.
Can you resubmit a new version of this patchiest, There have too many version and ignore version patches.


Thanks
>>> 
>>> Thanks
>>>>> Thanks for the reply. 
>>>>> All concerns raised by Nicolas has been addressed in this latest patch set.
>>>>> Let us wait for few days to check if anyone still has any objections.
>>>> Looks like nobody has anymore objections to this patchset. 
>>>> Well, it has already passed through multiple rounds of reviews.
>>>> Could somebody please push this to the master. 
>>>> I have few more patches waiting to be submitted, dependent on this feature.
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jeyapal, Karthick Nov. 29, 2017, 6:22 a.m. UTC | #10
On 11/29/17, 11:41 AM, "刘歧" <lq@chinaffmpeg.org> wrote:

>> 在 2017年11月29日,12:05,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:

>> 

>> A gentle reminder. 

>> 7 days have gone by since the last version of this patchset was submitted. And no objections so far.

>> Could this patchset be pushed, please? Thanks.

>Can you resubmit a new version of this patchiest, There have too many version and ignore version patches.


Thanks for the reply. 
I have resent the patches as new version v3 for the sake of clarity.

>Thanks


regards,
Karthick
diff mbox

Patch

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 0bb8ad2..c1d753b 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -850,6 +850,9 @@  ffmpeg -re -i in.ts -f hls -master_pl_name master.m3u8 \
 This example creates HLS master playlist with name master.m3u8 and keep
 publishing it repeatedly every after 30 segments i.e. every after 60s.
 
+@item http_persistent
+Use persistent HTTP connections. Applicable only for HTTP output.
+
 @end table
 
 @anchor{ico}
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 3c47ced..32c99a0 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -45,6 +45,7 @@ 
 
 #include "avformat.h"
 #include "avio_internal.h"
+#include "http.h"
 #include "internal.h"
 #include "os_support.h"
 
@@ -204,6 +205,7 @@  typedef struct HLSContext {
     char *var_stream_map; /* user specified variant stream map string */
     char *master_pl_name;
     unsigned int master_publish_rate;
+    int http_persistent;
 } HLSContext;
 
 static int get_int_from_double(double val)
@@ -244,10 +246,38 @@  static int mkdir_p(const char *path) {
     return ret;
 }
 
+static int is_http_proto(char *filename) {
+    const char *proto = avio_find_protocol_name(filename);
+    return proto ? (!av_strcasecmp(proto, "http") || !av_strcasecmp(proto, "https")) : 0;
+}
+
+static int hlsenc_io_open(AVFormatContext *s, AVIOContext **pb, char *filename,
+                          AVDictionary **options) {
+    HLSContext *hls = s->priv_data;
+    int http_base_proto = is_http_proto(filename);
+    int err;
+    if (*pb == NULL || !http_base_proto || !hls->http_persistent) {
+        err = s->io_open(s, pb, filename, AVIO_FLAG_WRITE, options);
+    } else {
+        URLContext *http_url_context = ffio_geturlcontext(*pb);
+        av_assert0(http_url_context);
+        err = ff_http_do_new_request(http_url_context, filename);
+    }
+    return err;
+}
+
+static void hlsenc_io_close(AVFormatContext *s, AVIOContext **pb, char *filename) {
+    HLSContext *hls = s->priv_data;
+    int http_base_proto = is_http_proto(filename);
+
+    if (!http_base_proto || !hls->http_persistent || hls->key_info_file || hls->encrypt) {
+        ff_format_io_close(s, pb);
+    }
+}
+
 static void set_http_options(AVFormatContext *s, AVDictionary **options, HLSContext *c)
 {
-    const char *proto = avio_find_protocol_name(s->filename);
-    int http_base_proto = proto ? (!av_strcasecmp(proto, "http") || !av_strcasecmp(proto, "https")) : 0;
+    int http_base_proto = is_http_proto(s->filename);
 
     if (c->method) {
         av_dict_set(options, "method", c->method, 0);
@@ -257,6 +287,8 @@  static void set_http_options(AVFormatContext *s, AVDictionary **options, HLSCont
     }
     if (c->user_agent)
         av_dict_set(options, "user_agent", c->user_agent, 0);
+    if (c->http_persistent)
+        av_dict_set_int(options, "multiple_requests", 1, 0);
 
 }
 
@@ -1430,17 +1462,17 @@  static int hls_start(AVFormatContext *s, VariantStream *vs)
             err = AVERROR(ENOMEM);
             goto fail;
         }
-        err = s->io_open(s, &oc->pb, filename, AVIO_FLAG_WRITE, &options);
+        err = hlsenc_io_open(s, &oc->pb, filename, &options);
         av_free(filename);
         av_dict_free(&options);
         if (err < 0)
             return err;
     } else
-        if ((err = s->io_open(s, &oc->pb, oc->filename, AVIO_FLAG_WRITE, &options)) < 0)
+        if ((err = hlsenc_io_open(s, &oc->pb, oc->filename, &options)) < 0)
             goto fail;
     if (vs->vtt_basename) {
         set_http_options(s, &options, c);
-        if ((err = s->io_open(s, &vtt_oc->pb, vtt_oc->filename, AVIO_FLAG_WRITE, &options)) < 0)
+        if ((err = hlsenc_io_open(s, &vtt_oc->pb, vtt_oc->filename, &options)) < 0)
             goto fail;
     }
     av_dict_free(&options);
@@ -2148,11 +2180,12 @@  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
                 avio_open_dyn_buf(&oc->pb);
                 vs->packets_written = 0;
                 ff_format_io_close(s, &vs->out);
+                hlsenc_io_close(s, &vs->out, vs->base_output_dirname);
             } else {
-                ff_format_io_close(s, &oc->pb);
+                hlsenc_io_close(s, &oc->pb, oc->filename);
             }
             if (vs->vtt_avf) {
-                ff_format_io_close(s, &vs->vtt_avf->pb);
+                hlsenc_io_close(s, &vs->vtt_avf->pb, vs->vtt_avf->filename);
             }
         }
         if ((hls->flags & HLS_TEMP_FILE) && oc->filename[0]) {
@@ -2337,6 +2370,7 @@  static const AVOption options[] = {
     {"var_stream_map", "Variant stream map string", OFFSET(var_stream_map), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
     {"master_pl_name", "Create HLS master playlist with this name", OFFSET(master_pl_name), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,    E},
     {"master_pl_publish_rate", "Publish master play list every after this many segment intervals", OFFSET(master_publish_rate), AV_OPT_TYPE_INT, {.i64 = 0}, 0, UINT_MAX, E},
+    {"http_persistent", "Use persistent HTTP connections", OFFSET(http_persistent), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, E },
     { NULL },
 };