diff mbox

[FFmpeg-devel] First Patch for hlsenc.c for https://trac.ffmpeg.org/ticket/7281

Message ID DD6EBAB1-1B3F-4E70-B6CA-87081613CCF1@chinaffmpeg.org
State Superseded
Headers show

Commit Message

Liu Steven Aug. 1, 2018, 8:41 p.m. UTC
> On Aug 2, 2018, at 03:50, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote:
> 
> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>
From e7ff03ed52c709d647a112833427b44c41e3ed12 Mon Sep 17 00:00:00 2001
From: "Ronak Patel (Audible)" <ronakp@audible.com>
Date: Tue, 31 Jul 2018 19:05:18 -0400
Subject: [PATCH] libavformat/hlsenc: Fix HLS Manifest Generation from an N^2
 algorithm to N.

This fixes the creation of the hls manifest in hlsenc.c by writing the header once in hls_write_header, and the segments individually in the hls_window method.
Files that would previously take over a week to fragment now take 1 minute on the same hardware. This was a 153 hour audio file (2.2GB of audio).

Signed-off-by: Ronak Patel <ronak2121@yahoo.com>
---
 libavformat/hlsenc.c | 143 +++++++++++++++++++++++++++------------------------
 1 file changed, 76 insertions(+), 67 deletions(-)

Comments

Ronak Patel Aug. 1, 2018, 10:20 p.m. UTC | #1
> On Aug 1, 2018, at 4:41 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> 
> 
>> On Aug 2, 2018, at 03:50, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote:
>> 
>> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>
> From e7ff03ed52c709d647a112833427b44c41e3ed12 Mon Sep 17 00:00:00 2001
> From: "Ronak Patel (Audible)" <ronakp@audible.com>
> Date: Tue, 31 Jul 2018 19:05:18 -0400
> Subject: [PATCH] libavformat/hlsenc: Fix HLS Manifest Generation from an N^2
> algorithm to N.
> 
> This fixes the creation of the hls manifest in hlsenc.c by writing the header once in hls_write_header, and the segments individually in the hls_window method.
> Files that would previously take over a week to fragment now take 1 minute on the same hardware. This was a 153 hour audio file (2.2GB of audio).
> 
> Signed-off-by: Ronak Patel <ronak2121@yahoo.com>
> ---
> libavformat/hlsenc.c | 143 +++++++++++++++++++++++++++------------------------
> 1 file changed, 76 insertions(+), 67 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index b5644f0..b15645d 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -488,7 +488,6 @@ static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls,
>         }
>         p = (char *)av_basename(dirname);
>         *p = '\0';
> -
>     }
> 
>     while (segment) {
> @@ -1365,63 +1364,37 @@ fail:
>     return ret;
> }
> 
> -static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
> +static int hls_write_manifest_segment(AVFormatContext *s, int last, VariantStream *vs)
> {
>     HLSContext *hls = s->priv_data;
>     HLSSegment *en;
> -    int target_duration = 0;
>     int ret = 0;
> -    char temp_filename[1024];
> -    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
> -    const char *proto = avio_find_protocol_name(s->url);
> -    int use_rename = proto && !strcmp(proto, "file");
> -    static unsigned warned_non_file;
> +    int byterange_mode;
>     char *key_uri = NULL;
>     char *iv_string = NULL;
>     AVDictionary *options = NULL;
>     double prog_date_time = vs->initial_prog_date_time;
>     double *prog_date_time_p = (hls->flags & HLS_PROGRAM_DATE_TIME) ? &prog_date_time : NULL;
> -    int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
> -
> -    hls->version = 3;
> -    if (byterange_mode) {
> -        hls->version = 4;
> -        sequence = 0;
> -    }
> -
> -    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
> -        hls->version = 6;
> -    }
> 
> -    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
> -        hls->version = 7;
> -    }
> +    if (last) {
> 
> -    if (!use_rename && !warned_non_file++)
> -        av_log(s, AV_LOG_ERROR, "Cannot use rename on non file protocol, this may lead to races and temporary partial files\n");
> +        if ((hls->flags & HLS_OMIT_ENDLIST==0)) {
> +            ff_hls_write_end_list(hls->m3u8_out);
> +        }
> 
> -    set_http_options(s, &options, hls);
> -    snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : "%s", vs->m3u8_name);
> -    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 0)
> -        goto fail;
> +        if (vs->vtt_m3u8_name) {
> +            ff_hls_write_end_list(hls->sub_m3u8_out);
> +        }
> +    } else {
> 
> -    for (en = vs->segments; en; en = en->next) {
> -        if (target_duration <= en->duration)
> -            target_duration = lrint(en->duration);
> -    }
> +        byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
> 
> -    vs->discontinuity_set = 0;
> -    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache,
> -                                 target_duration, sequence, hls->pl_type);
> +        en = vs->last_segment;
> +        if (en == NULL) {
> +            ret = 1;
> +            goto fail;
> +        }
> 
> -    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
> -        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
> -        vs->discontinuity_set = 1;
> -    }
> -    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
> -        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
> -    }
> -    for (en = vs->segments; en; en = en->next) {
>         if ((hls->encrypt || hls->key_info_file) && (!key_uri || strcmp(en->key_uri, key_uri) ||
>                                     av_strcasecmp(en->iv_string, iv_string))) {
>             avio_printf(hls->m3u8_out, "#EXT-X-KEY:METHOD=AES-128,URI=\"%s\"", en->key_uri);
> @@ -1444,17 +1417,8 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>         if (ret < 0) {
>             av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>         }
> -    }
> -
> -    if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
> -        ff_hls_write_end_list(hls->m3u8_out);
> 
> -    if( vs->vtt_m3u8_name ) {
> -        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0)
> -            goto fail;
> -        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
> -                                     target_duration, sequence, PLAYLIST_TYPE_NONE);
> -        for (en = vs->segments; en; en = en->next) {
> +        if( vs->vtt_m3u8_name ) {
>             ret = ff_hls_write_file_entry(hls->sub_m3u8_out, 0, byterange_mode,
>                                           en->duration, 0, en->size, en->pos,
>                                           vs->baseurl, en->sub_filename, NULL);
> @@ -1462,22 +1426,16 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>                 av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>             }
>         }
> -
> -        if (last)
> -            ff_hls_write_end_list(hls->sub_m3u8_out);
> -
>     }
> 
> fail:
>     av_dict_free(&options);
> -    hlsenc_io_close(s, &hls->m3u8_out, temp_filename);
> -    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
> -    if (ret >= 0 && use_rename)
> -        ff_rename(temp_filename, vs->m3u8_name, s);
> 
> -    if (ret >= 0 && hls->master_pl_name)
> -        if (create_master_playlist(s, vs) < 0)
> +    if (ret >= 0 && hls->master_pl_name) {
> +        if (create_master_playlist(s, vs) < 0) {
>             av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");
> +        }
> +    }
> 
>     return ret;
> }
> @@ -2078,6 +2036,11 @@ static int hls_write_header(AVFormatContext *s)
>     int ret, i, j;
>     AVDictionary *options = NULL;
>     VariantStream *vs = NULL;
> +    int target_duration = 0;
> +    int byterange_mode = 0;
> +
> +    target_duration = lrint(hls->time);
> +    byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
> 
>     for (i = 0; i < hls->nb_varstreams; i++) {
>         vs = &hls->var_streams[i];
> @@ -2091,7 +2054,7 @@ static int hls_write_header(AVFormatContext *s)
>             goto fail;
>         }
>         av_dict_free(&options);
> -        //av_assert0(s->nb_streams == hls->avf->nb_streams);
> +
>         for (j = 0; j < vs->nb_streams; j++) {
>             AVStream *inner_st;
>             AVStream *outer_st = vs->streams[j];
> @@ -2130,6 +2093,47 @@ static int hls_write_header(AVFormatContext *s)
>             }
>         }
>     }
> +
> +    // write the header for the main hls playlist
> +    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
> +    hls->version = 3;
> +    if (byterange_mode) {
> +        hls->version = 4;
> +        sequence = 0;
> +    }
> +
> +    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
> +        hls->version = 6;
> +    }
> +
> +    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
> +        hls->version = 7;
> +    }
> +
> +    set_http_options(s, &options, hls);
> +    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, vs->m3u8_name, &options)) < 0) {
> +        goto fail;
> +    }
> +
> +    vs->discontinuity_set = 0;
> +    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache, target_duration, sequence, hls->pl_type);
> +
> +    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
> +        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
> +        vs->discontinuity_set = 1;
> +    }
> +    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
> +        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
> +    }
> +
> +    if( vs->vtt_m3u8_name ) {
> +        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0) {
> +            goto fail;
> +        }
> +
> +        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache, target_duration, sequence, PLAYLIST_TYPE_NONE);
> +    }
> +
> fail:
> 
>     return ret;
> @@ -2334,10 +2338,11 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>             return ret;
>         }
> 
> -        if (!vs->fmp4_init_mode || byterange_mode)
> -            if ((ret = hls_window(s, 0, vs)) < 0) {
> +        if (!vs->fmp4_init_mode || byterange_mode) {
> +            if ((ret = hls_write_manifest_segment(s, 0, vs)) < 0) {
>                 return ret;
>             }
> +        }
>     }
> 
>     vs->packets_written++;
> @@ -2421,7 +2426,7 @@ failed:
>         avformat_free_context(oc);
> 
>         vs->avf = NULL;
> -        hls_window(s, 1, vs);
> +        hls_write_manifest_segment(s, 1, vs);
> 
>         av_freep(&vs->fmp4_init_filename);
>         if (vtt_oc) {
> @@ -2447,6 +2452,9 @@ failed:
>         av_freep(&ccs->language);
>     }
> 
> +    hlsenc_io_close(s, &hls->m3u8_out, vs->m3u8_name);
> +    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
> +
>     ff_format_io_close(s, &hls->m3u8_out);
>     ff_format_io_close(s, &hls->sub_m3u8_out);
>     av_freep(&hls->key_basename);
> @@ -2884,3 +2892,4 @@ AVOutputFormat ff_hls_muxer = {
>     .write_trailer  = hls_write_trailer,
>     .priv_class     = &hls_class,
> };
> +
> -- 
> 2.6.3
> 
> 
> 
> 
> 
> 
> 
> I have read this patch some problem for this patch.
> 
> 1. maybe there will have a problem when duration is not same when every fragment, for example:
> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 output_test.m3u8
> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
> #EXTM3U
> #EXT-X-VERSION:3
> #EXT-X-TARGETDURATION:8
> #EXT-X-MEDIA-SEQUENCE:0
> #EXTINF:3.866667,
> output_test0.ts
> #EXTINF:7.300000,
> output_test1.ts
> #EXTINF:8.333333,
> output_test2.ts
> 
> the output_test0.ts’s duration is short than output_test1.ts, the #EXT-X-TARGETDURATION need update to the longest duration.
> this operation (check the longest duration) will happen at every fragment write complete.
> it will not update when move the update option to the hls_write_header,
> 

This is a problem in the code that splits the mpegts files. I've filed a separate issue for this here: https://trac.ffmpeg.org/ticket/7341 <https://trac.ffmpeg.org/ticket/7341>. Mpegts segmentation should be following the hls_time parameter (or the default length). This is happening now with fMP4 assets, but not with mpegts. I unfortunately don't know where to make this fix to make mpegts work better. Can you advise? I will submit fixes to this in a separate patch.

> 2. the version maybe will update when use hls_segment_type or append_list etc. when the operation is support from different version m3u8.

I don't follow what you mean here. The version number is known up front, based on the options that were passed in. It should be illegal to switch between versions when trying to update an existing manifest. When can this legitimately happen?

> 3. need update segments vs->segments when hls_list_size option is set.
> 

What do you mean by this and where should I do it?

> 
> Thanks
> Steven
> 
> 
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Liu Steven Aug. 1, 2018, 11:22 p.m. UTC | #2
> On Aug 2, 2018, at 06:20, Ronak <ronak2121@yahoo.com> wrote:
> 
>> 
>> On Aug 1, 2018, at 4:41 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>> 
>> 
>> 
>>> On Aug 2, 2018, at 03:50, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote:
>>> 
>>> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>
>> From e7ff03ed52c709d647a112833427b44c41e3ed12 Mon Sep 17 00:00:00 2001
>> From: "Ronak Patel (Audible)" <ronakp@audible.com>
>> Date: Tue, 31 Jul 2018 19:05:18 -0400
>> Subject: [PATCH] libavformat/hlsenc: Fix HLS Manifest Generation from an N^2
>> algorithm to N.
>> 
>> This fixes the creation of the hls manifest in hlsenc.c by writing the header once in hls_write_header, and the segments individually in the hls_window method.
>> Files that would previously take over a week to fragment now take 1 minute on the same hardware. This was a 153 hour audio file (2.2GB of audio).
>> 
>> Signed-off-by: Ronak Patel <ronak2121@yahoo.com>
>> ---
>> libavformat/hlsenc.c | 143 +++++++++++++++++++++++++++------------------------
>> 1 file changed, 76 insertions(+), 67 deletions(-)
>> 
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index b5644f0..b15645d 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -488,7 +488,6 @@ static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls,
>>         }
>>         p = (char *)av_basename(dirname);
>>         *p = '\0';
>> -
>>     }
>> 
>>     while (segment) {
>> @@ -1365,63 +1364,37 @@ fail:
>>     return ret;
>> }
>> 
>> -static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>> +static int hls_write_manifest_segment(AVFormatContext *s, int last, VariantStream *vs)
>> {
>>     HLSContext *hls = s->priv_data;
>>     HLSSegment *en;
>> -    int target_duration = 0;
>>     int ret = 0;
>> -    char temp_filename[1024];
>> -    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>> -    const char *proto = avio_find_protocol_name(s->url);
>> -    int use_rename = proto && !strcmp(proto, "file");
>> -    static unsigned warned_non_file;
>> +    int byterange_mode;
>>     char *key_uri = NULL;
>>     char *iv_string = NULL;
>>     AVDictionary *options = NULL;
>>     double prog_date_time = vs->initial_prog_date_time;
>>     double *prog_date_time_p = (hls->flags & HLS_PROGRAM_DATE_TIME) ? &prog_date_time : NULL;
>> -    int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>> -
>> -    hls->version = 3;
>> -    if (byterange_mode) {
>> -        hls->version = 4;
>> -        sequence = 0;
>> -    }
>> -
>> -    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>> -        hls->version = 6;
>> -    }
>> 
>> -    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>> -        hls->version = 7;
>> -    }
>> +    if (last) {
>> 
>> -    if (!use_rename && !warned_non_file++)
>> -        av_log(s, AV_LOG_ERROR, "Cannot use rename on non file protocol, this may lead to races and temporary partial files\n");
>> +        if ((hls->flags & HLS_OMIT_ENDLIST==0)) {
>> +            ff_hls_write_end_list(hls->m3u8_out);
>> +        }
>> 
>> -    set_http_options(s, &options, hls);
>> -    snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : "%s", vs->m3u8_name);
>> -    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 0)
>> -        goto fail;
>> +        if (vs->vtt_m3u8_name) {
>> +            ff_hls_write_end_list(hls->sub_m3u8_out);
>> +        }
>> +    } else {
>> 
>> -    for (en = vs->segments; en; en = en->next) {
>> -        if (target_duration <= en->duration)
>> -            target_duration = lrint(en->duration);
>> -    }
>> +        byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>> 
>> -    vs->discontinuity_set = 0;
>> -    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache,
>> -                                 target_duration, sequence, hls->pl_type);
>> +        en = vs->last_segment;
>> +        if (en == NULL) {
>> +            ret = 1;
>> +            goto fail;
>> +        }
>> 
>> -    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>> -        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>> -        vs->discontinuity_set = 1;
>> -    }
>> -    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>> -        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>> -    }
>> -    for (en = vs->segments; en; en = en->next) {
>>         if ((hls->encrypt || hls->key_info_file) && (!key_uri || strcmp(en->key_uri, key_uri) ||
>>                                     av_strcasecmp(en->iv_string, iv_string))) {
>>             avio_printf(hls->m3u8_out, "#EXT-X-KEY:METHOD=AES-128,URI=\"%s\"", en->key_uri);
>> @@ -1444,17 +1417,8 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>         if (ret < 0) {
>>             av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>         }
>> -    }
>> -
>> -    if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
>> -        ff_hls_write_end_list(hls->m3u8_out);
>> 
>> -    if( vs->vtt_m3u8_name ) {
>> -        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0)
>> -            goto fail;
>> -        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
>> -                                     target_duration, sequence, PLAYLIST_TYPE_NONE);
>> -        for (en = vs->segments; en; en = en->next) {
>> +        if( vs->vtt_m3u8_name ) {
>>             ret = ff_hls_write_file_entry(hls->sub_m3u8_out, 0, byterange_mode,
>>                                           en->duration, 0, en->size, en->pos,
>>                                           vs->baseurl, en->sub_filename, NULL);
>> @@ -1462,22 +1426,16 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>                 av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>             }
>>         }
>> -
>> -        if (last)
>> -            ff_hls_write_end_list(hls->sub_m3u8_out);
>> -
>>     }
>> 
>> fail:
>>     av_dict_free(&options);
>> -    hlsenc_io_close(s, &hls->m3u8_out, temp_filename);
>> -    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>> -    if (ret >= 0 && use_rename)
>> -        ff_rename(temp_filename, vs->m3u8_name, s);
>> 
>> -    if (ret >= 0 && hls->master_pl_name)
>> -        if (create_master_playlist(s, vs) < 0)
>> +    if (ret >= 0 && hls->master_pl_name) {
>> +        if (create_master_playlist(s, vs) < 0) {
>>             av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");
>> +        }
>> +    }
>> 
>>     return ret;
>> }
>> @@ -2078,6 +2036,11 @@ static int hls_write_header(AVFormatContext *s)
>>     int ret, i, j;
>>     AVDictionary *options = NULL;
>>     VariantStream *vs = NULL;
>> +    int target_duration = 0;
>> +    int byterange_mode = 0;
>> +
>> +    target_duration = lrint(hls->time);
>> +    byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>> 
>>     for (i = 0; i < hls->nb_varstreams; i++) {
>>         vs = &hls->var_streams[i];
>> @@ -2091,7 +2054,7 @@ static int hls_write_header(AVFormatContext *s)
>>             goto fail;
>>         }
>>         av_dict_free(&options);
>> -        //av_assert0(s->nb_streams == hls->avf->nb_streams);
>> +
>>         for (j = 0; j < vs->nb_streams; j++) {
>>             AVStream *inner_st;
>>             AVStream *outer_st = vs->streams[j];
>> @@ -2130,6 +2093,47 @@ static int hls_write_header(AVFormatContext *s)
>>             }
>>         }
>>     }
>> +
>> +    // write the header for the main hls playlist
>> +    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>> +    hls->version = 3;
>> +    if (byterange_mode) {
>> +        hls->version = 4;
>> +        sequence = 0;
>> +    }
>> +
>> +    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>> +        hls->version = 6;
>> +    }
>> +
>> +    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>> +        hls->version = 7;
>> +    }
>> +
>> +    set_http_options(s, &options, hls);
>> +    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, vs->m3u8_name, &options)) < 0) {
>> +        goto fail;
>> +    }
>> +
>> +    vs->discontinuity_set = 0;
>> +    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache, target_duration, sequence, hls->pl_type);
>> +
>> +    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>> +        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>> +        vs->discontinuity_set = 1;
>> +    }
>> +    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>> +        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>> +    }
>> +
>> +    if( vs->vtt_m3u8_name ) {
>> +        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0) {
>> +            goto fail;
>> +        }
>> +
>> +        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache, target_duration, sequence, PLAYLIST_TYPE_NONE);
>> +    }
>> +
>> fail:
>> 
>>     return ret;
>> @@ -2334,10 +2338,11 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>>             return ret;
>>         }
>> 
>> -        if (!vs->fmp4_init_mode || byterange_mode)
>> -            if ((ret = hls_window(s, 0, vs)) < 0) {
>> +        if (!vs->fmp4_init_mode || byterange_mode) {
>> +            if ((ret = hls_write_manifest_segment(s, 0, vs)) < 0) {
>>                 return ret;
>>             }
>> +        }
>>     }
>> 
>>     vs->packets_written++;
>> @@ -2421,7 +2426,7 @@ failed:
>>         avformat_free_context(oc);
>> 
>>         vs->avf = NULL;
>> -        hls_window(s, 1, vs);
>> +        hls_write_manifest_segment(s, 1, vs);
>> 
>>         av_freep(&vs->fmp4_init_filename);
>>         if (vtt_oc) {
>> @@ -2447,6 +2452,9 @@ failed:
>>         av_freep(&ccs->language);
>>     }
>> 
>> +    hlsenc_io_close(s, &hls->m3u8_out, vs->m3u8_name);
>> +    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>> +
>>     ff_format_io_close(s, &hls->m3u8_out);
>>     ff_format_io_close(s, &hls->sub_m3u8_out);
>>     av_freep(&hls->key_basename);
>> @@ -2884,3 +2892,4 @@ AVOutputFormat ff_hls_muxer = {
>>     .write_trailer  = hls_write_trailer,
>>     .priv_class     = &hls_class,
>> };
>> +
>> -- 
>> 2.6.3
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> I have read this patch some problem for this patch.
>> 
>> 1. maybe there will have a problem when duration is not same when every fragment, for example:
>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 output_test.m3u8
>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>> #EXTM3U
>> #EXT-X-VERSION:3
>> #EXT-X-TARGETDURATION:8
>> #EXT-X-MEDIA-SEQUENCE:0
>> #EXTINF:3.866667,
>> output_test0.ts
>> #EXTINF:7.300000,
>> output_test1.ts
>> #EXTINF:8.333333,
>> output_test2.ts
>> 
>> the output_test0.ts’s duration is short than output_test1.ts, the #EXT-X-TARGETDURATION need update to the longest duration.
>> this operation (check the longest duration) will happen at every fragment write complete.
>> it will not update when move the update option to the hls_write_header,
>> 
> 
> This is a problem in the code that splits the mpegts files. I've filed a separate issue for this here: https://trac.ffmpeg.org/ticket/7341. Mpegts segmentation should be following the hls_time parameter (or the default length).
This is whatever hls_time, is decide by keyframe position, this is happen when GOP size is not a permanent t position.

> This is happening now with fMP4 assets, but not with mpegts.
Whatever fmp4 or mpegts, all of them need fix the problem of duration refresh.

for example:

liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -ss -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
#EXTM3U
#EXT-X-VERSION:7
#EXT-X-TARGETDURATION:8
#EXT-X-MEDIA-SEQUENCE:0
#EXT-X-MAP:URI="init.mp4"
#EXTINF:3.866667,
output_test0.m4s
#EXTINF:7.300000,
output_test1.m4s
#EXTINF:8.333333,
liuqideMacBook-Pro:xxx liuqi$

> I unfortunately don't know where to make this fix to make mpegts work better. Can you advise? I will submit fixes to this in a separate patch.

Or what about get the #EXT-X-TARGETDURATION position, to update it? looks like flvenc.c or movenc.c date shift

> 
>> 2. the version maybe will update when use hls_segment_type or append_list etc. when the operation is support from different version m3u8.
> 
> I don't follow what you mean here. The version number is known up front, based on the options that were passed in. It should be illegal to switch between versions when trying to update an existing manifest. When can this legitimately happen?
there maybe have some player cannot support high version of m3u8, for example old parser or player just support the VERSION 3,
this must think about all of the player or parser, because ffmpeg is not used only by myself.

Or what about get the #EXT-X-VERSION position, to update it? looks like flvenc.c or movenc.c date shift

> 
>> 3. need update segments vs->segments when hls_list_size option is set.
>> 
> 
> What do you mean by this and where should I do it?
for example, hls_list_size is 4, the m3u8 list should refresh every time when make a new fragment.

first time:
1.m4s
2.m4s
3.m4s
4.m4s

sencond time:
2.m4s
3.m4s
4.m4s
5.m4s


Thanks
Steven
Liu Steven Aug. 1, 2018, 11:30 p.m. UTC | #3
> On Aug 2, 2018, at 07:22, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> 
> 
>> On Aug 2, 2018, at 06:20, Ronak <ronak2121@yahoo.com> wrote:
>> 
>>> 
>>> On Aug 1, 2018, at 4:41 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>>> 
>>> 
>>> 
>>>> On Aug 2, 2018, at 03:50, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote:
>>>> 
>>>> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>
>>> From e7ff03ed52c709d647a112833427b44c41e3ed12 Mon Sep 17 00:00:00 2001
>>> From: "Ronak Patel (Audible)" <ronakp@audible.com>
>>> Date: Tue, 31 Jul 2018 19:05:18 -0400
>>> Subject: [PATCH] libavformat/hlsenc: Fix HLS Manifest Generation from an N^2
>>> algorithm to N.
>>> 
>>> This fixes the creation of the hls manifest in hlsenc.c by writing the header once in hls_write_header, and the segments individually in the hls_window method.
>>> Files that would previously take over a week to fragment now take 1 minute on the same hardware. This was a 153 hour audio file (2.2GB of audio).
>>> 
>>> Signed-off-by: Ronak Patel <ronak2121@yahoo.com>
>>> ---
>>> libavformat/hlsenc.c | 143 +++++++++++++++++++++++++++------------------------
>>> 1 file changed, 76 insertions(+), 67 deletions(-)
>>> 
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index b5644f0..b15645d 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -488,7 +488,6 @@ static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls,
>>>        }
>>>        p = (char *)av_basename(dirname);
>>>        *p = '\0';
>>> -
>>>    }
>>> 
>>>    while (segment) {
>>> @@ -1365,63 +1364,37 @@ fail:
>>>    return ret;
>>> }
>>> 
>>> -static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>> +static int hls_write_manifest_segment(AVFormatContext *s, int last, VariantStream *vs)
>>> {
>>>    HLSContext *hls = s->priv_data;
>>>    HLSSegment *en;
>>> -    int target_duration = 0;
>>>    int ret = 0;
>>> -    char temp_filename[1024];
>>> -    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>>> -    const char *proto = avio_find_protocol_name(s->url);
>>> -    int use_rename = proto && !strcmp(proto, "file");
>>> -    static unsigned warned_non_file;
>>> +    int byterange_mode;
>>>    char *key_uri = NULL;
>>>    char *iv_string = NULL;
>>>    AVDictionary *options = NULL;
>>>    double prog_date_time = vs->initial_prog_date_time;
>>>    double *prog_date_time_p = (hls->flags & HLS_PROGRAM_DATE_TIME) ? &prog_date_time : NULL;
>>> -    int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>> -
>>> -    hls->version = 3;
>>> -    if (byterange_mode) {
>>> -        hls->version = 4;
>>> -        sequence = 0;
>>> -    }
>>> -
>>> -    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>>> -        hls->version = 6;
>>> -    }
>>> 
>>> -    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>>> -        hls->version = 7;
>>> -    }
>>> +    if (last) {
>>> 
>>> -    if (!use_rename && !warned_non_file++)
>>> -        av_log(s, AV_LOG_ERROR, "Cannot use rename on non file protocol, this may lead to races and temporary partial files\n");
>>> +        if ((hls->flags & HLS_OMIT_ENDLIST==0)) {
>>> +            ff_hls_write_end_list(hls->m3u8_out);
>>> +        }
>>> 
>>> -    set_http_options(s, &options, hls);
>>> -    snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : "%s", vs->m3u8_name);
>>> -    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 0)
>>> -        goto fail;
>>> +        if (vs->vtt_m3u8_name) {
>>> +            ff_hls_write_end_list(hls->sub_m3u8_out);
>>> +        }
>>> +    } else {
>>> 
>>> -    for (en = vs->segments; en; en = en->next) {
>>> -        if (target_duration <= en->duration)
>>> -            target_duration = lrint(en->duration);
>>> -    }
>>> +        byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>> 
>>> -    vs->discontinuity_set = 0;
>>> -    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache,
>>> -                                 target_duration, sequence, hls->pl_type);
>>> +        en = vs->last_segment;
>>> +        if (en == NULL) {
>>> +            ret = 1;
>>> +            goto fail;
>>> +        }
>>> 
>>> -    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>>> -        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>>> -        vs->discontinuity_set = 1;
>>> -    }
>>> -    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>>> -        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>>> -    }
>>> -    for (en = vs->segments; en; en = en->next) {
>>>        if ((hls->encrypt || hls->key_info_file) && (!key_uri || strcmp(en->key_uri, key_uri) ||
>>>                                    av_strcasecmp(en->iv_string, iv_string))) {
>>>            avio_printf(hls->m3u8_out, "#EXT-X-KEY:METHOD=AES-128,URI=\"%s\"", en->key_uri);
>>> @@ -1444,17 +1417,8 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>        if (ret < 0) {
>>>            av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>>        }
>>> -    }
>>> -
>>> -    if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
>>> -        ff_hls_write_end_list(hls->m3u8_out);
>>> 
>>> -    if( vs->vtt_m3u8_name ) {
>>> -        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0)
>>> -            goto fail;
>>> -        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
>>> -                                     target_duration, sequence, PLAYLIST_TYPE_NONE);
>>> -        for (en = vs->segments; en; en = en->next) {
>>> +        if( vs->vtt_m3u8_name ) {
>>>            ret = ff_hls_write_file_entry(hls->sub_m3u8_out, 0, byterange_mode,
>>>                                          en->duration, 0, en->size, en->pos,
>>>                                          vs->baseurl, en->sub_filename, NULL);
>>> @@ -1462,22 +1426,16 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>                av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>>            }
>>>        }
>>> -
>>> -        if (last)
>>> -            ff_hls_write_end_list(hls->sub_m3u8_out);
>>> -
>>>    }
>>> 
>>> fail:
>>>    av_dict_free(&options);
>>> -    hlsenc_io_close(s, &hls->m3u8_out, temp_filename);
>>> -    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>>> -    if (ret >= 0 && use_rename)
>>> -        ff_rename(temp_filename, vs->m3u8_name, s);
>>> 
>>> -    if (ret >= 0 && hls->master_pl_name)
>>> -        if (create_master_playlist(s, vs) < 0)
>>> +    if (ret >= 0 && hls->master_pl_name) {
>>> +        if (create_master_playlist(s, vs) < 0) {
>>>            av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");
>>> +        }
>>> +    }
>>> 
>>>    return ret;
>>> }
>>> @@ -2078,6 +2036,11 @@ static int hls_write_header(AVFormatContext *s)
>>>    int ret, i, j;
>>>    AVDictionary *options = NULL;
>>>    VariantStream *vs = NULL;
>>> +    int target_duration = 0;
>>> +    int byterange_mode = 0;
>>> +
>>> +    target_duration = lrint(hls->time);
>>> +    byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>> 
>>>    for (i = 0; i < hls->nb_varstreams; i++) {
>>>        vs = &hls->var_streams[i];
>>> @@ -2091,7 +2054,7 @@ static int hls_write_header(AVFormatContext *s)
>>>            goto fail;
>>>        }
>>>        av_dict_free(&options);
>>> -        //av_assert0(s->nb_streams == hls->avf->nb_streams);
>>> +
>>>        for (j = 0; j < vs->nb_streams; j++) {
>>>            AVStream *inner_st;
>>>            AVStream *outer_st = vs->streams[j];
>>> @@ -2130,6 +2093,47 @@ static int hls_write_header(AVFormatContext *s)
>>>            }
>>>        }
>>>    }
>>> +
>>> +    // write the header for the main hls playlist
>>> +    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>>> +    hls->version = 3;
>>> +    if (byterange_mode) {
>>> +        hls->version = 4;
>>> +        sequence = 0;
>>> +    }
>>> +
>>> +    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>>> +        hls->version = 6;
>>> +    }
>>> +
>>> +    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>>> +        hls->version = 7;
>>> +    }
>>> +
>>> +    set_http_options(s, &options, hls);
>>> +    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, vs->m3u8_name, &options)) < 0) {
>>> +        goto fail;
>>> +    }
>>> +
>>> +    vs->discontinuity_set = 0;
>>> +    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache, target_duration, sequence, hls->pl_type);
>>> +
>>> +    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>>> +        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>>> +        vs->discontinuity_set = 1;
>>> +    }
>>> +    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>>> +        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>>> +    }
>>> +
>>> +    if( vs->vtt_m3u8_name ) {
>>> +        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0) {
>>> +            goto fail;
>>> +        }
>>> +
>>> +        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache, target_duration, sequence, PLAYLIST_TYPE_NONE);
>>> +    }
>>> +
>>> fail:
>>> 
>>>    return ret;
>>> @@ -2334,10 +2338,11 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>>>            return ret;
>>>        }
>>> 
>>> -        if (!vs->fmp4_init_mode || byterange_mode)
>>> -            if ((ret = hls_window(s, 0, vs)) < 0) {
>>> +        if (!vs->fmp4_init_mode || byterange_mode) {
>>> +            if ((ret = hls_write_manifest_segment(s, 0, vs)) < 0) {
>>>                return ret;
>>>            }
>>> +        }
>>>    }
>>> 
>>>    vs->packets_written++;
>>> @@ -2421,7 +2426,7 @@ failed:
>>>        avformat_free_context(oc);
>>> 
>>>        vs->avf = NULL;
>>> -        hls_window(s, 1, vs);
>>> +        hls_write_manifest_segment(s, 1, vs);
>>> 
>>>        av_freep(&vs->fmp4_init_filename);
>>>        if (vtt_oc) {
>>> @@ -2447,6 +2452,9 @@ failed:
>>>        av_freep(&ccs->language);
>>>    }
>>> 
>>> +    hlsenc_io_close(s, &hls->m3u8_out, vs->m3u8_name);
>>> +    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>>> +
>>>    ff_format_io_close(s, &hls->m3u8_out);
>>>    ff_format_io_close(s, &hls->sub_m3u8_out);
>>>    av_freep(&hls->key_basename);
>>> @@ -2884,3 +2892,4 @@ AVOutputFormat ff_hls_muxer = {
>>>    .write_trailer  = hls_write_trailer,
>>>    .priv_class     = &hls_class,
>>> };
>>> +
>>> -- 
>>> 2.6.3
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> I have read this patch some problem for this patch.
>>> 
>>> 1. maybe there will have a problem when duration is not same when every fragment, for example:
>>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 output_test.m3u8
>>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>>> #EXTM3U
>>> #EXT-X-VERSION:3
>>> #EXT-X-TARGETDURATION:8
>>> #EXT-X-MEDIA-SEQUENCE:0
>>> #EXTINF:3.866667,
>>> output_test0.ts
>>> #EXTINF:7.300000,
>>> output_test1.ts
>>> #EXTINF:8.333333,
>>> output_test2.ts
>>> 
>>> the output_test0.ts’s duration is short than output_test1.ts, the #EXT-X-TARGETDURATION need update to the longest duration.
>>> this operation (check the longest duration) will happen at every fragment write complete.
>>> it will not update when move the update option to the hls_write_header,
>>> 
>> 
>> This is a problem in the code that splits the mpegts files. I've filed a separate issue for this here: https://trac.ffmpeg.org/ticket/7341. Mpegts segmentation should be following the hls_time parameter (or the default length).
> This is whatever hls_time, is decide by keyframe position, this is happen when GOP size is not a permanent t position.
> 
>> This is happening now with fMP4 assets, but not with mpegts.
> Whatever fmp4 or mpegts, all of them need fix the problem of duration refresh.
> 
> for example:
> 
> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -ss -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
> #EXTM3U
> #EXT-X-VERSION:7
> #EXT-X-TARGETDURATION:8
> #EXT-X-MEDIA-SEQUENCE:0
> #EXT-X-MAP:URI="init.mp4"
> #EXTINF:3.866667,
> output_test0.m4s
> #EXTINF:7.300000,
> output_test1.m4s
> #EXTINF:8.333333,
> liuqideMacBook-Pro:xxx liuqi$

This is after your patch:
liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -ss 17 -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
#EXTM3U
#EXT-X-VERSION:7
#EXT-X-TARGETDURATION:3
#EXT-X-MEDIA-SEQUENCE:0
#EXT-X-MAP:URI="init.mp4"
#EXTINF:3.866667,
output_test0.m4s
#EXTINF:7.300000,
output_test1.m4s
#EXTINF:8.333333,

The RFC https://www.rfc-editor.org/rfc/rfc8216.txt describe:

4.3.3.1.  EXT-X-TARGETDURATION

   The EXT-X-TARGETDURATION tag specifies the maximum Media Segment
   duration.  The EXTINF duration of each Media Segment in the Playlist
   file, when rounded to the nearest integer, MUST be less than or equal
   to the target duration; longer segments can trigger playback stalls
   or other errors.  It applies to the entire Playlist file.  Its format
   is:

   #EXT-X-TARGETDURATION:<s>

   where s is a decimal-integer indicating the target duration in
   seconds.  The EXT-X-TARGETDURATION tag is REQUIRED.

your patch make the EXT-X-TARGETDURATION less than EXTINF:7.300000 EXTINF:8.333333


> 
>> I unfortunately don't know where to make this fix to make mpegts work better. Can you advise? I will submit fixes to this in a separate patch.
> 
> Or what about get the #EXT-X-TARGETDURATION position, to update it? looks like flvenc.c or movenc.c date shift
> 
>> 
>>> 2. the version maybe will update when use hls_segment_type or append_list etc. when the operation is support from different version m3u8.
>> 
>> I don't follow what you mean here. The version number is known up front, based on the options that were passed in. It should be illegal to switch between versions when trying to update an existing manifest. When can this legitimately happen?
> there maybe have some player cannot support high version of m3u8, for example old parser or player just support the VERSION 3,
> this must think about all of the player or parser, because ffmpeg is not used only by myself.
> 
> Or what about get the #EXT-X-VERSION position, to update it? looks like flvenc.c or movenc.c date shift
> 
>> 
>>> 3. need update segments vs->segments when hls_list_size option is set.
>>> 
>> 
>> What do you mean by this and where should I do it?
> for example, hls_list_size is 4, the m3u8 list should refresh every time when make a new fragment.
> 
> first time:
> 1.m4s
> 2.m4s
> 3.m4s
> 4.m4s
> 
> sencond time:
> 2.m4s
> 3.m4s
> 4.m4s
> 5.m4s

after your patch:

liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 4 -hls_segment_type fmp4 -hls_time 3 -t 50 output_test.m3u8
liuqideMacBook-Pro:xxx liuqi$ cat output_test.m3u8
#EXTM3U
#EXT-X-VERSION:7
#EXT-X-TARGETDURATION:3
#EXT-X-MEDIA-SEQUENCE:0
#EXT-X-MAP:URI="init.mp4"
#EXTINF:3.866667,
output_test0.m4s
#EXTINF:7.300000,
output_test1.m4s
#EXTINF:8.333333,
output_test2.m4s
#EXTINF:3.966667,
output_test3.m4s
#EXTINF:8.333333,
output_test4.m4s
#EXTINF:4.033333,
output_test5.m4s
#EXTINF:8.333333,
output_test6.m4s
#EXTINF:4.633333,
output_test7.m4s
liuqideMacBook-Pro:xxx liuqi$
liuqideMacBook-Pro:xxx liuqi$

the m3u8 list is incorrect, because users want control the m3u8 list length, because their disk do not have enough space to save the fragments.

> 
> 
> Thanks
> Steven

Thanks
Steven
Ronak Patel Aug. 2, 2018, 1:30 a.m. UTC | #4
> On Aug 1, 2018, at 7:30 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> 
> 
>> On Aug 2, 2018, at 07:22, Steven Liu <lq@chinaffmpeg.org> wrote:
>> 
>> 
>> 
>>>> On Aug 2, 2018, at 06:20, Ronak <ronak2121@yahoo.com> wrote:
>>>> 
>>>> 
>>>> On Aug 1, 2018, at 4:41 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Aug 2, 2018, at 03:50, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote:
>>>>> 
>>>>> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>
>>>> From e7ff03ed52c709d647a112833427b44c41e3ed12 Mon Sep 17 00:00:00 2001
>>>> From: "Ronak Patel (Audible)" <ronakp@audible.com>
>>>> Date: Tue, 31 Jul 2018 19:05:18 -0400
>>>> Subject: [PATCH] libavformat/hlsenc: Fix HLS Manifest Generation from an N^2
>>>> algorithm to N.
>>>> 
>>>> This fixes the creation of the hls manifest in hlsenc.c by writing the header once in hls_write_header, and the segments individually in the hls_window method.
>>>> Files that would previously take over a week to fragment now take 1 minute on the same hardware. This was a 153 hour audio file (2.2GB of audio).
>>>> 
>>>> Signed-off-by: Ronak Patel <ronak2121@yahoo.com>
>>>> ---
>>>> libavformat/hlsenc.c | 143 +++++++++++++++++++++++++++------------------------
>>>> 1 file changed, 76 insertions(+), 67 deletions(-)
>>>> 
>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>> index b5644f0..b15645d 100644
>>>> --- a/libavformat/hlsenc.c
>>>> +++ b/libavformat/hlsenc.c
>>>> @@ -488,7 +488,6 @@ static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls,
>>>>       }
>>>>       p = (char *)av_basename(dirname);
>>>>       *p = '\0';
>>>> -
>>>>   }
>>>> 
>>>>   while (segment) {
>>>> @@ -1365,63 +1364,37 @@ fail:
>>>>   return ret;
>>>> }
>>>> 
>>>> -static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>> +static int hls_write_manifest_segment(AVFormatContext *s, int last, VariantStream *vs)
>>>> {
>>>>   HLSContext *hls = s->priv_data;
>>>>   HLSSegment *en;
>>>> -    int target_duration = 0;
>>>>   int ret = 0;
>>>> -    char temp_filename[1024];
>>>> -    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>>>> -    const char *proto = avio_find_protocol_name(s->url);
>>>> -    int use_rename = proto && !strcmp(proto, "file");
>>>> -    static unsigned warned_non_file;
>>>> +    int byterange_mode;
>>>>   char *key_uri = NULL;
>>>>   char *iv_string = NULL;
>>>>   AVDictionary *options = NULL;
>>>>   double prog_date_time = vs->initial_prog_date_time;
>>>>   double *prog_date_time_p = (hls->flags & HLS_PROGRAM_DATE_TIME) ? &prog_date_time : NULL;
>>>> -    int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>>> -
>>>> -    hls->version = 3;
>>>> -    if (byterange_mode) {
>>>> -        hls->version = 4;
>>>> -        sequence = 0;
>>>> -    }
>>>> -
>>>> -    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>>>> -        hls->version = 6;
>>>> -    }
>>>> 
>>>> -    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>>>> -        hls->version = 7;
>>>> -    }
>>>> +    if (last) {
>>>> 
>>>> -    if (!use_rename && !warned_non_file++)
>>>> -        av_log(s, AV_LOG_ERROR, "Cannot use rename on non file protocol, this may lead to races and temporary partial files\n");
>>>> +        if ((hls->flags & HLS_OMIT_ENDLIST==0)) {
>>>> +            ff_hls_write_end_list(hls->m3u8_out);
>>>> +        }
>>>> 
>>>> -    set_http_options(s, &options, hls);
>>>> -    snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : "%s", vs->m3u8_name);
>>>> -    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 0)
>>>> -        goto fail;
>>>> +        if (vs->vtt_m3u8_name) {
>>>> +            ff_hls_write_end_list(hls->sub_m3u8_out);
>>>> +        }
>>>> +    } else {
>>>> 
>>>> -    for (en = vs->segments; en; en = en->next) {
>>>> -        if (target_duration <= en->duration)
>>>> -            target_duration = lrint(en->duration);
>>>> -    }
>>>> +        byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>>> 
>>>> -    vs->discontinuity_set = 0;
>>>> -    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache,
>>>> -                                 target_duration, sequence, hls->pl_type);
>>>> +        en = vs->last_segment;
>>>> +        if (en == NULL) {
>>>> +            ret = 1;
>>>> +            goto fail;
>>>> +        }
>>>> 
>>>> -    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>>>> -        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>>>> -        vs->discontinuity_set = 1;
>>>> -    }
>>>> -    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>>>> -        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>>>> -    }
>>>> -    for (en = vs->segments; en; en = en->next) {
>>>>       if ((hls->encrypt || hls->key_info_file) && (!key_uri || strcmp(en->key_uri, key_uri) ||
>>>>                                   av_strcasecmp(en->iv_string, iv_string))) {
>>>>           avio_printf(hls->m3u8_out, "#EXT-X-KEY:METHOD=AES-128,URI=\"%s\"", en->key_uri);
>>>> @@ -1444,17 +1417,8 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>>       if (ret < 0) {
>>>>           av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>>>       }
>>>> -    }
>>>> -
>>>> -    if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
>>>> -        ff_hls_write_end_list(hls->m3u8_out);
>>>> 
>>>> -    if( vs->vtt_m3u8_name ) {
>>>> -        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0)
>>>> -            goto fail;
>>>> -        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
>>>> -                                     target_duration, sequence, PLAYLIST_TYPE_NONE);
>>>> -        for (en = vs->segments; en; en = en->next) {
>>>> +        if( vs->vtt_m3u8_name ) {
>>>>           ret = ff_hls_write_file_entry(hls->sub_m3u8_out, 0, byterange_mode,
>>>>                                         en->duration, 0, en->size, en->pos,
>>>>                                         vs->baseurl, en->sub_filename, NULL);
>>>> @@ -1462,22 +1426,16 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>>               av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>>>           }
>>>>       }
>>>> -
>>>> -        if (last)
>>>> -            ff_hls_write_end_list(hls->sub_m3u8_out);
>>>> -
>>>>   }
>>>> 
>>>> fail:
>>>>   av_dict_free(&options);
>>>> -    hlsenc_io_close(s, &hls->m3u8_out, temp_filename);
>>>> -    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>>>> -    if (ret >= 0 && use_rename)
>>>> -        ff_rename(temp_filename, vs->m3u8_name, s);
>>>> 
>>>> -    if (ret >= 0 && hls->master_pl_name)
>>>> -        if (create_master_playlist(s, vs) < 0)
>>>> +    if (ret >= 0 && hls->master_pl_name) {
>>>> +        if (create_master_playlist(s, vs) < 0) {
>>>>           av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");
>>>> +        }
>>>> +    }
>>>> 
>>>>   return ret;
>>>> }
>>>> @@ -2078,6 +2036,11 @@ static int hls_write_header(AVFormatContext *s)
>>>>   int ret, i, j;
>>>>   AVDictionary *options = NULL;
>>>>   VariantStream *vs = NULL;
>>>> +    int target_duration = 0;
>>>> +    int byterange_mode = 0;
>>>> +
>>>> +    target_duration = lrint(hls->time);
>>>> +    byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>>> 
>>>>   for (i = 0; i < hls->nb_varstreams; i++) {
>>>>       vs = &hls->var_streams[i];
>>>> @@ -2091,7 +2054,7 @@ static int hls_write_header(AVFormatContext *s)
>>>>           goto fail;
>>>>       }
>>>>       av_dict_free(&options);
>>>> -        //av_assert0(s->nb_streams == hls->avf->nb_streams);
>>>> +
>>>>       for (j = 0; j < vs->nb_streams; j++) {
>>>>           AVStream *inner_st;
>>>>           AVStream *outer_st = vs->streams[j];
>>>> @@ -2130,6 +2093,47 @@ static int hls_write_header(AVFormatContext *s)
>>>>           }
>>>>       }
>>>>   }
>>>> +
>>>> +    // write the header for the main hls playlist
>>>> +    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>>>> +    hls->version = 3;
>>>> +    if (byterange_mode) {
>>>> +        hls->version = 4;
>>>> +        sequence = 0;
>>>> +    }
>>>> +
>>>> +    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>>>> +        hls->version = 6;
>>>> +    }
>>>> +
>>>> +    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>>>> +        hls->version = 7;
>>>> +    }
>>>> +
>>>> +    set_http_options(s, &options, hls);
>>>> +    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, vs->m3u8_name, &options)) < 0) {
>>>> +        goto fail;
>>>> +    }
>>>> +
>>>> +    vs->discontinuity_set = 0;
>>>> +    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache, target_duration, sequence, hls->pl_type);
>>>> +
>>>> +    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>>>> +        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>>>> +        vs->discontinuity_set = 1;
>>>> +    }
>>>> +    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>>>> +        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>>>> +    }
>>>> +
>>>> +    if( vs->vtt_m3u8_name ) {
>>>> +        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0) {
>>>> +            goto fail;
>>>> +        }
>>>> +
>>>> +        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache, target_duration, sequence, PLAYLIST_TYPE_NONE);
>>>> +    }
>>>> +
>>>> fail:
>>>> 
>>>>   return ret;
>>>> @@ -2334,10 +2338,11 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>>>>           return ret;
>>>>       }
>>>> 
>>>> -        if (!vs->fmp4_init_mode || byterange_mode)
>>>> -            if ((ret = hls_window(s, 0, vs)) < 0) {
>>>> +        if (!vs->fmp4_init_mode || byterange_mode) {
>>>> +            if ((ret = hls_write_manifest_segment(s, 0, vs)) < 0) {
>>>>               return ret;
>>>>           }
>>>> +        }
>>>>   }
>>>> 
>>>>   vs->packets_written++;
>>>> @@ -2421,7 +2426,7 @@ failed:
>>>>       avformat_free_context(oc);
>>>> 
>>>>       vs->avf = NULL;
>>>> -        hls_window(s, 1, vs);
>>>> +        hls_write_manifest_segment(s, 1, vs);
>>>> 
>>>>       av_freep(&vs->fmp4_init_filename);
>>>>       if (vtt_oc) {
>>>> @@ -2447,6 +2452,9 @@ failed:
>>>>       av_freep(&ccs->language);
>>>>   }
>>>> 
>>>> +    hlsenc_io_close(s, &hls->m3u8_out, vs->m3u8_name);
>>>> +    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>>>> +
>>>>   ff_format_io_close(s, &hls->m3u8_out);
>>>>   ff_format_io_close(s, &hls->sub_m3u8_out);
>>>>   av_freep(&hls->key_basename);
>>>> @@ -2884,3 +2892,4 @@ AVOutputFormat ff_hls_muxer = {
>>>>   .write_trailer  = hls_write_trailer,
>>>>   .priv_class     = &hls_class,
>>>> };
>>>> +
>>>> -- 
>>>> 2.6.3
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> I have read this patch some problem for this patch.
>>>> 
>>>> 1. maybe there will have a problem when duration is not same when every fragment, for example:
>>>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 output_test.m3u8
>>>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>>>> #EXTM3U
>>>> #EXT-X-VERSION:3
>>>> #EXT-X-TARGETDURATION:8
>>>> #EXT-X-MEDIA-SEQUENCE:0
>>>> #EXTINF:3.866667,
>>>> output_test0.ts
>>>> #EXTINF:7.300000,
>>>> output_test1.ts
>>>> #EXTINF:8.333333,
>>>> output_test2.ts
>>>> 
>>>> the output_test0.ts’s duration is short than output_test1.ts, the #EXT-X-TARGETDURATION need update to the longest duration.
>>>> this operation (check the longest duration) will happen at every fragment write complete.
>>>> it will not update when move the update option to the hls_write_header,
>>>> 
>>> 
>>> This is a problem in the code that splits the mpegts files. I've filed a separate issue for this here: https://trac.ffmpeg.org/ticket/7341. Mpegts segmentation should be following the hls_time parameter (or the default length).
>> This is whatever hls_time, is decide by keyframe position, this is happen when GOP size is not a permanent t position.
>> 
>>> This is happening now with fMP4 assets, but not with mpegts.
>> Whatever fmp4 or mpegts, all of them need fix the problem of duration refresh.
>> 
>> for example:
>> 
>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -ss -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>> #EXTM3U
>> #EXT-X-VERSION:7
>> #EXT-X-TARGETDURATION:8
>> #EXT-X-MEDIA-SEQUENCE:0
>> #EXT-X-MAP:URI="init.mp4"
>> #EXTINF:3.866667,
>> output_test0.m4s
>> #EXTINF:7.300000,
>> output_test1.m4s
>> #EXTINF:8.333333,
>> liuqideMacBook-Pro:xxx liuqi$
> 
> This is after your patch:
> liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -ss 17 -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
> #EXTM3U
> #EXT-X-VERSION:7
> #EXT-X-TARGETDURATION:3
> #EXT-X-MEDIA-SEQUENCE:0
> #EXT-X-MAP:URI="init.mp4"
> #EXTINF:3.866667,
> output_test0.m4s
> #EXTINF:7.300000,
> output_test1.m4s
> #EXTINF:8.333333,
> 
> The RFC https://www.rfc-editor.org/rfc/rfc8216.txt describe:
> 
> 4.3.3.1.  EXT-X-TARGETDURATION
> 
>   The EXT-X-TARGETDURATION tag specifies the maximum Media Segment
>   duration.  The EXTINF duration of each Media Segment in the Playlist
>   file, when rounded to the nearest integer, MUST be less than or equal
>   to the target duration; longer segments can trigger playback stalls
>   or other errors.  It applies to the entire Playlist file.  Its format
>   is:
> 
>   #EXT-X-TARGETDURATION:<s>
> 
>   where s is a decimal-integer indicating the target duration in
>   seconds.  The EXT-X-TARGETDURATION tag is REQUIRED.
> 
> your patch make the EXT-X-TARGETDURATION less than EXTINF:7.300000 EXTINF:8.333333

Why is the upstream fragmenting in such a way? The hls_time is not being honored here. You specified 3 and no segment should be longer than 3. Your proposed “fix” is just masking the root problem here which is ffmpeg does not fragment properly.

If you are tolerating the hls_time being ignored then why specify it?

I’ve tested this with audio AAC MP4 files and it works fine except for mpegts. If this is not working for video then this is a problem further upstream.

>> 
>>> I unfortunately don't know where to make this fix to make mpegts work better. Can you advise? I will submit fixes to this in a separate patch.
>> 
>> Or what about get the #EXT-X-TARGETDURATION position, to update it? looks like flvenc.c or movenc.c date shift
>> 
>>> 

See above.

>>>> 2. the version maybe will update when use hls_segment_type or append_list etc. when the operation is support from different version m3u8.
>>> 
>>> I don't follow what you mean here. The version number is known up front, based on the options that were passed in. It should be illegal to switch between versions when trying to update an existing manifest. When can this legitimately happen?
>> there maybe have some player cannot support high version of m3u8, for example old parser or player just support the VERSION 3,
>> this must think about all of the player or parser, because ffmpeg is not used only by myself.
>> 
>> Or what about get the #EXT-X-VERSION position, to update it? looks like flvenc.c or movenc.c date shift
>> 

I just copied the existing code here. If we use fmp4 or byterange mode we automatically adjust.

>>> 
>>>> 3. need update segments vs->segments when hls_list_size option is set.
>>>> 
>>> 
>>> What do you mean by this and where should I do it?
>> for example, hls_list_size is 4, the m3u8 list should refresh every time when make a new fragment.
>> 
>> first time:
>> 1.m4s
>> 2.m4s
>> 3.m4s
>> 4.m4s
>> 
>> sencond time:
>> 2.m4s
>> 3.m4s
>> 4.m4s
>> 5.m4s
> 
> after your patch:
> 
> liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 4 -hls_segment_type fmp4 -hls_time 3 -t 50 output_test.m3u8
> liuqideMacBook-Pro:xxx liuqi$ cat output_test.m3u8
> #EXTM3U
> #EXT-X-VERSION:7
> #EXT-X-TARGETDURATION:3
> #EXT-X-MEDIA-SEQUENCE:0
> #EXT-X-MAP:URI="init.mp4"
> #EXTINF:3.866667,
> output_test0.m4s
> #EXTINF:7.300000,
> output_test1.m4s
> #EXTINF:8.333333,
> output_test2.m4s
> #EXTINF:3.966667,
> output_test3.m4s
> #EXTINF:8.333333,
> output_test4.m4s
> #EXTINF:4.033333,
> output_test5.m4s
> #EXTINF:8.333333,
> output_test6.m4s
> #EXTINF:4.633333,
> output_test7.m4s
> liuqideMacBook-Pro:xxx liuqi$
> liuqideMacBook-Pro:xxx liuqi$
> 
> the m3u8 list is incorrect, because users want control the m3u8 list length, because their disk do not have enough space to save the fragments.
> 

Ok I will fix this.


>> 
>> 
>> Thanks
>> Steven
> 
> Thanks
> Steven
> 
> 
>
Ronak Patel Aug. 2, 2018, 1:47 a.m. UTC | #5
> On Aug 1, 2018, at 9:30 PM, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote:
> 
> 
>> On Aug 1, 2018, at 7:30 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>> 
>> 
>> 
>>> On Aug 2, 2018, at 07:22, Steven Liu <lq@chinaffmpeg.org> wrote:
>>> 
>>> 
>>> 
>>>>> On Aug 2, 2018, at 06:20, Ronak <ronak2121@yahoo.com> wrote:
>>>>> 
>>>>> 
>>>>> On Aug 1, 2018, at 4:41 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Aug 2, 2018, at 03:50, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote:
>>>>>> 
>>>>>> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>
>>>>> From e7ff03ed52c709d647a112833427b44c41e3ed12 Mon Sep 17 00:00:00 2001
>>>>> From: "Ronak Patel (Audible)" <ronakp@audible.com>
>>>>> Date: Tue, 31 Jul 2018 19:05:18 -0400
>>>>> Subject: [PATCH] libavformat/hlsenc: Fix HLS Manifest Generation from an N^2
>>>>> algorithm to N.
>>>>> 
>>>>> This fixes the creation of the hls manifest in hlsenc.c by writing the header once in hls_write_header, and the segments individually in the hls_window method.
>>>>> Files that would previously take over a week to fragment now take 1 minute on the same hardware. This was a 153 hour audio file (2.2GB of audio).
>>>>> 
>>>>> Signed-off-by: Ronak Patel <ronak2121@yahoo.com>
>>>>> ---
>>>>> libavformat/hlsenc.c | 143 +++++++++++++++++++++++++++------------------------
>>>>> 1 file changed, 76 insertions(+), 67 deletions(-)
>>>>> 
>>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>>> index b5644f0..b15645d 100644
>>>>> --- a/libavformat/hlsenc.c
>>>>> +++ b/libavformat/hlsenc.c
>>>>> @@ -488,7 +488,6 @@ static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls,
>>>>>      }
>>>>>      p = (char *)av_basename(dirname);
>>>>>      *p = '\0';
>>>>> -
>>>>>  }
>>>>> 
>>>>>  while (segment) {
>>>>> @@ -1365,63 +1364,37 @@ fail:
>>>>>  return ret;
>>>>> }
>>>>> 
>>>>> -static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>>> +static int hls_write_manifest_segment(AVFormatContext *s, int last, VariantStream *vs)
>>>>> {
>>>>>  HLSContext *hls = s->priv_data;
>>>>>  HLSSegment *en;
>>>>> -    int target_duration = 0;
>>>>>  int ret = 0;
>>>>> -    char temp_filename[1024];
>>>>> -    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>>>>> -    const char *proto = avio_find_protocol_name(s->url);
>>>>> -    int use_rename = proto && !strcmp(proto, "file");
>>>>> -    static unsigned warned_non_file;
>>>>> +    int byterange_mode;
>>>>>  char *key_uri = NULL;
>>>>>  char *iv_string = NULL;
>>>>>  AVDictionary *options = NULL;
>>>>>  double prog_date_time = vs->initial_prog_date_time;
>>>>>  double *prog_date_time_p = (hls->flags & HLS_PROGRAM_DATE_TIME) ? &prog_date_time : NULL;
>>>>> -    int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>>>> -
>>>>> -    hls->version = 3;
>>>>> -    if (byterange_mode) {
>>>>> -        hls->version = 4;
>>>>> -        sequence = 0;
>>>>> -    }
>>>>> -
>>>>> -    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>>>>> -        hls->version = 6;
>>>>> -    }
>>>>> 
>>>>> -    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>>>>> -        hls->version = 7;
>>>>> -    }
>>>>> +    if (last) {
>>>>> 
>>>>> -    if (!use_rename && !warned_non_file++)
>>>>> -        av_log(s, AV_LOG_ERROR, "Cannot use rename on non file protocol, this may lead to races and temporary partial files\n");
>>>>> +        if ((hls->flags & HLS_OMIT_ENDLIST==0)) {
>>>>> +            ff_hls_write_end_list(hls->m3u8_out);
>>>>> +        }
>>>>> 
>>>>> -    set_http_options(s, &options, hls);
>>>>> -    snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : "%s", vs->m3u8_name);
>>>>> -    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 0)
>>>>> -        goto fail;
>>>>> +        if (vs->vtt_m3u8_name) {
>>>>> +            ff_hls_write_end_list(hls->sub_m3u8_out);
>>>>> +        }
>>>>> +    } else {
>>>>> 
>>>>> -    for (en = vs->segments; en; en = en->next) {
>>>>> -        if (target_duration <= en->duration)
>>>>> -            target_duration = lrint(en->duration);
>>>>> -    }
>>>>> +        byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>>>> 
>>>>> -    vs->discontinuity_set = 0;
>>>>> -    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache,
>>>>> -                                 target_duration, sequence, hls->pl_type);
>>>>> +        en = vs->last_segment;
>>>>> +        if (en == NULL) {
>>>>> +            ret = 1;
>>>>> +            goto fail;
>>>>> +        }
>>>>> 
>>>>> -    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>>>>> -        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>>>>> -        vs->discontinuity_set = 1;
>>>>> -    }
>>>>> -    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>>>>> -        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>>>>> -    }
>>>>> -    for (en = vs->segments; en; en = en->next) {
>>>>>      if ((hls->encrypt || hls->key_info_file) && (!key_uri || strcmp(en->key_uri, key_uri) ||
>>>>>                                  av_strcasecmp(en->iv_string, iv_string))) {
>>>>>          avio_printf(hls->m3u8_out, "#EXT-X-KEY:METHOD=AES-128,URI=\"%s\"", en->key_uri);
>>>>> @@ -1444,17 +1417,8 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>>>      if (ret < 0) {
>>>>>          av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>>>>      }
>>>>> -    }
>>>>> -
>>>>> -    if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
>>>>> -        ff_hls_write_end_list(hls->m3u8_out);
>>>>> 
>>>>> -    if( vs->vtt_m3u8_name ) {
>>>>> -        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0)
>>>>> -            goto fail;
>>>>> -        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
>>>>> -                                     target_duration, sequence, PLAYLIST_TYPE_NONE);
>>>>> -        for (en = vs->segments; en; en = en->next) {
>>>>> +        if( vs->vtt_m3u8_name ) {
>>>>>          ret = ff_hls_write_file_entry(hls->sub_m3u8_out, 0, byterange_mode,
>>>>>                                        en->duration, 0, en->size, en->pos,
>>>>>                                        vs->baseurl, en->sub_filename, NULL);
>>>>> @@ -1462,22 +1426,16 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>>>>>              av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>>>>>          }
>>>>>      }
>>>>> -
>>>>> -        if (last)
>>>>> -            ff_hls_write_end_list(hls->sub_m3u8_out);
>>>>> -
>>>>>  }
>>>>> 
>>>>> fail:
>>>>>  av_dict_free(&options);
>>>>> -    hlsenc_io_close(s, &hls->m3u8_out, temp_filename);
>>>>> -    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>>>>> -    if (ret >= 0 && use_rename)
>>>>> -        ff_rename(temp_filename, vs->m3u8_name, s);
>>>>> 
>>>>> -    if (ret >= 0 && hls->master_pl_name)
>>>>> -        if (create_master_playlist(s, vs) < 0)
>>>>> +    if (ret >= 0 && hls->master_pl_name) {
>>>>> +        if (create_master_playlist(s, vs) < 0) {
>>>>>          av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");
>>>>> +        }
>>>>> +    }
>>>>> 
>>>>>  return ret;
>>>>> }
>>>>> @@ -2078,6 +2036,11 @@ static int hls_write_header(AVFormatContext *s)
>>>>>  int ret, i, j;
>>>>>  AVDictionary *options = NULL;
>>>>>  VariantStream *vs = NULL;
>>>>> +    int target_duration = 0;
>>>>> +    int byterange_mode = 0;
>>>>> +
>>>>> +    target_duration = lrint(hls->time);
>>>>> +    byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
>>>>> 
>>>>>  for (i = 0; i < hls->nb_varstreams; i++) {
>>>>>      vs = &hls->var_streams[i];
>>>>> @@ -2091,7 +2054,7 @@ static int hls_write_header(AVFormatContext *s)
>>>>>          goto fail;
>>>>>      }
>>>>>      av_dict_free(&options);
>>>>> -        //av_assert0(s->nb_streams == hls->avf->nb_streams);
>>>>> +
>>>>>      for (j = 0; j < vs->nb_streams; j++) {
>>>>>          AVStream *inner_st;
>>>>>          AVStream *outer_st = vs->streams[j];
>>>>> @@ -2130,6 +2093,47 @@ static int hls_write_header(AVFormatContext *s)
>>>>>          }
>>>>>      }
>>>>>  }
>>>>> +
>>>>> +    // write the header for the main hls playlist
>>>>> +    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
>>>>> +    hls->version = 3;
>>>>> +    if (byterange_mode) {
>>>>> +        hls->version = 4;
>>>>> +        sequence = 0;
>>>>> +    }
>>>>> +
>>>>> +    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
>>>>> +        hls->version = 6;
>>>>> +    }
>>>>> +
>>>>> +    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
>>>>> +        hls->version = 7;
>>>>> +    }
>>>>> +
>>>>> +    set_http_options(s, &options, hls);
>>>>> +    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, vs->m3u8_name, &options)) < 0) {
>>>>> +        goto fail;
>>>>> +    }
>>>>> +
>>>>> +    vs->discontinuity_set = 0;
>>>>> +    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache, target_duration, sequence, hls->pl_type);
>>>>> +
>>>>> +    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
>>>>> +        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
>>>>> +        vs->discontinuity_set = 1;
>>>>> +    }
>>>>> +    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
>>>>> +        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
>>>>> +    }
>>>>> +
>>>>> +    if( vs->vtt_m3u8_name ) {
>>>>> +        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0) {
>>>>> +            goto fail;
>>>>> +        }
>>>>> +
>>>>> +        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache, target_duration, sequence, PLAYLIST_TYPE_NONE);
>>>>> +    }
>>>>> +
>>>>> fail:
>>>>> 
>>>>>  return ret;
>>>>> @@ -2334,10 +2338,11 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>>>>>          return ret;
>>>>>      }
>>>>> 
>>>>> -        if (!vs->fmp4_init_mode || byterange_mode)
>>>>> -            if ((ret = hls_window(s, 0, vs)) < 0) {
>>>>> +        if (!vs->fmp4_init_mode || byterange_mode) {
>>>>> +            if ((ret = hls_write_manifest_segment(s, 0, vs)) < 0) {
>>>>>              return ret;
>>>>>          }
>>>>> +        }
>>>>>  }
>>>>> 
>>>>>  vs->packets_written++;
>>>>> @@ -2421,7 +2426,7 @@ failed:
>>>>>      avformat_free_context(oc);
>>>>> 
>>>>>      vs->avf = NULL;
>>>>> -        hls_window(s, 1, vs);
>>>>> +        hls_write_manifest_segment(s, 1, vs);
>>>>> 
>>>>>      av_freep(&vs->fmp4_init_filename);
>>>>>      if (vtt_oc) {
>>>>> @@ -2447,6 +2452,9 @@ failed:
>>>>>      av_freep(&ccs->language);
>>>>>  }
>>>>> 
>>>>> +    hlsenc_io_close(s, &hls->m3u8_out, vs->m3u8_name);
>>>>> +    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
>>>>> +
>>>>>  ff_format_io_close(s, &hls->m3u8_out);
>>>>>  ff_format_io_close(s, &hls->sub_m3u8_out);
>>>>>  av_freep(&hls->key_basename);
>>>>> @@ -2884,3 +2892,4 @@ AVOutputFormat ff_hls_muxer = {
>>>>>  .write_trailer  = hls_write_trailer,
>>>>>  .priv_class     = &hls_class,
>>>>> };
>>>>> +
>>>>> -- 
>>>>> 2.6.3
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> I have read this patch some problem for this patch.
>>>>> 
>>>>> 1. maybe there will have a problem when duration is not same when every fragment, for example:
>>>>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 output_test.m3u8
>>>>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>>>>> #EXTM3U
>>>>> #EXT-X-VERSION:3
>>>>> #EXT-X-TARGETDURATION:8
>>>>> #EXT-X-MEDIA-SEQUENCE:0
>>>>> #EXTINF:3.866667,
>>>>> output_test0.ts
>>>>> #EXTINF:7.300000,
>>>>> output_test1.ts
>>>>> #EXTINF:8.333333,
>>>>> output_test2.ts
>>>>> 
>>>>> the output_test0.ts’s duration is short than output_test1.ts, the #EXT-X-TARGETDURATION need update to the longest duration.
>>>>> this operation (check the longest duration) will happen at every fragment write complete.
>>>>> it will not update when move the update option to the hls_write_header,
>>>>> 
>>>> 
>>>> This is a problem in the code that splits the mpegts files. I've filed a separate issue for this here: https://trac.ffmpeg.org/ticket/7341. Mpegts segmentation should be following the hls_time parameter (or the default length).
>>> This is whatever hls_time, is decide by keyframe position, this is happen when GOP size is not a permanent t position.
>>> 

I’m seeing problems with mpegts with MP4 assets that have nothing but AAC audio in it. There’s no video involved. And thus there should be no I frames or GOP. Just AAC packets.

>>>> This is happening now with fMP4 assets, but not with mpegts.
>>> Whatever fmp4 or mpegts, all of them need fix the problem of duration refresh.
>>> 
>>> for example:
>>> 
>>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -ss -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
>>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>>> #EXTM3U
>>> #EXT-X-VERSION:7
>>> #EXT-X-TARGETDURATION:8
>>> #EXT-X-MEDIA-SEQUENCE:0
>>> #EXT-X-MAP:URI="init.mp4"
>>> #EXTINF:3.866667,
>>> output_test0.m4s
>>> #EXTINF:7.300000,
>>> output_test1.m4s
>>> #EXTINF:8.333333,
>>> liuqideMacBook-Pro:xxx liuqi$
>> 
>> This is after your patch:
>> liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -ss 17 -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>> #EXTM3U
>> #EXT-X-VERSION:7
>> #EXT-X-TARGETDURATION:3
>> #EXT-X-MEDIA-SEQUENCE:0
>> #EXT-X-MAP:URI="init.mp4"
>> #EXTINF:3.866667,
>> output_test0.m4s
>> #EXTINF:7.300000,
>> output_test1.m4s
>> #EXTINF:8.333333,
>> 
>> The RFC https://www.rfc-editor.org/rfc/rfc8216.txt describe:
>> 
>> 4.3.3.1.  EXT-X-TARGETDURATION
>> 
>>  The EXT-X-TARGETDURATION tag specifies the maximum Media Segment
>>  duration.  The EXTINF duration of each Media Segment in the Playlist
>>  file, when rounded to the nearest integer, MUST be less than or equal
>>  to the target duration; longer segments can trigger playback stalls
>>  or other errors.  It applies to the entire Playlist file.  Its format
>>  is:
>> 
>>  #EXT-X-TARGETDURATION:<s>
>> 
>>  where s is a decimal-integer indicating the target duration in
>>  seconds.  The EXT-X-TARGETDURATION tag is REQUIRED.
>> 
>> your patch make the EXT-X-TARGETDURATION less than EXTINF:7.300000 EXTINF:8.333333
> 
> Why is the upstream fragmenting in such a way? The hls_time is not being honored here. You specified 3 and no segment should be longer than 3. Your proposed “fix” is just masking the root problem here which is ffmpeg does not fragment properly.
> 
> If you are tolerating the hls_time being ignored then why specify it?
> 
> I’ve tested this with audio AAC MP4 files and it works fine except for mpegts. If this is not working for video then this is a problem further upstream.
> 
>>> 
>>>> I unfortunately don't know where to make this fix to make mpegts work better. Can you advise? I will submit fixes to this in a separate patch.
>>> 
>>> Or what about get the #EXT-X-TARGETDURATION position, to update it? looks like flvenc.c or movenc.c date shift
>>> 
>>>> 
> 
> See above.
> 
>>>>> 2. the version maybe will update when use hls_segment_type or append_list etc. when the operation is support from different version m3u8.
>>>> 
>>>> I don't follow what you mean here. The version number is known up front, based on the options that were passed in. It should be illegal to switch between versions when trying to update an existing manifest. When can this legitimately happen?
>>> there maybe have some player cannot support high version of m3u8, for example old parser or player just support the VERSION 3,
>>> this must think about all of the player or parser, because ffmpeg is not used only by myself.
>>> 
>>> Or what about get the #EXT-X-VERSION position, to update it? looks like flvenc.c or movenc.c date shift
>>> 
> 
> I just copied the existing code here. If we use fmp4 or byterange mode we automatically adjust.
> 
>>>> 
>>>>> 3. need update segments vs->segments when hls_list_size option is set.
>>>>> 
>>>> 
>>>> What do you mean by this and where should I do it?
>>> for example, hls_list_size is 4, the m3u8 list should refresh every time when make a new fragment.
>>> 
>>> first time:
>>> 1.m4s
>>> 2.m4s
>>> 3.m4s
>>> 4.m4s
>>> 
>>> sencond time:
>>> 2.m4s
>>> 3.m4s
>>> 4.m4s
>>> 5.m4s
>> 
>> after your patch:
>> 
>> liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 4 -hls_segment_type fmp4 -hls_time 3 -t 50 output_test.m3u8
>> liuqideMacBook-Pro:xxx liuqi$ cat output_test.m3u8
>> #EXTM3U
>> #EXT-X-VERSION:7
>> #EXT-X-TARGETDURATION:3
>> #EXT-X-MEDIA-SEQUENCE:0
>> #EXT-X-MAP:URI="init.mp4"
>> #EXTINF:3.866667,
>> output_test0.m4s
>> #EXTINF:7.300000,
>> output_test1.m4s
>> #EXTINF:8.333333,
>> output_test2.m4s
>> #EXTINF:3.966667,
>> output_test3.m4s
>> #EXTINF:8.333333,
>> output_test4.m4s
>> #EXTINF:4.033333,
>> output_test5.m4s
>> #EXTINF:8.333333,
>> output_test6.m4s
>> #EXTINF:4.633333,
>> output_test7.m4s
>> liuqideMacBook-Pro:xxx liuqi$
>> liuqideMacBook-Pro:xxx liuqi$
>> 
>> the m3u8 list is incorrect, because users want control the m3u8 list length, because their disk do not have enough space to save the fragments.
>> 
> 
> Ok I will fix this.
> 
> 
>>> 
>>> 
>>> Thanks
>>> Steven
>> 
>> Thanks
>> Steven
>> 
>> 
>> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Ronak Patel Aug. 3, 2018, 6:17 p.m. UTC | #6
>>>>>> I have read this patch some problem for this patch.
>>>>>> 
>>>>>> 1. maybe there will have a problem when duration is not same when every fragment, for example:
>>>>>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 output_test.m3u8
>>>>>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>>>>>> #EXTM3U
>>>>>> #EXT-X-VERSION:3
>>>>>> #EXT-X-TARGETDURATION:8
>>>>>> #EXT-X-MEDIA-SEQUENCE:0
>>>>>> #EXTINF:3.866667,
>>>>>> output_test0.ts
>>>>>> #EXTINF:7.300000,
>>>>>> output_test1.ts
>>>>>> #EXTINF:8.333333,
>>>>>> output_test2.ts
>>>>>> 
>>>>>> the output_test0.ts’s duration is short than output_test1.ts, the #EXT-X-TARGETDURATION need update to the longest duration.
>>>>>> this operation (check the longest duration) will happen at every fragment write complete.
>>>>>> it will not update when move the update option to the hls_write_header,
>>>>>> 
>>>>> 
>>>>> This is a problem in the code that splits the mpegts files. I've filed a separate issue for this here: https://trac.ffmpeg.org/ticket/7341. Mpegts segmentation should be following the hls_time parameter (or the default length).
>>>> This is whatever hls_time, is decide by keyframe position, this is happen when GOP size is not a permanent t position.
>>>> 
> 
>>>>> This is happening now with fMP4 assets, but not with mpegts.
>>>> Whatever fmp4 or mpegts, all of them need fix the problem of duration refresh.
>>>> 
>>>> for example:
>>>> 
>>>> liuqideMacBook-Pro:xxx liuqi$ ./ffmpeg -ss -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
>>>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>>>> #EXTM3U
>>>> #EXT-X-VERSION:7
>>>> #EXT-X-TARGETDURATION:8
>>>> #EXT-X-MEDIA-SEQUENCE:0
>>>> #EXT-X-MAP:URI="init.mp4"
>>>> #EXTINF:3.866667,
>>>> output_test0.m4s
>>>> #EXTINF:7.300000,
>>>> output_test1.m4s
>>>> #EXTINF:8.333333,
>>>> liuqideMacBook-Pro:xxx liuqi$
>>> 
>>> This is after your patch:
>>> liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -ss 17 -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 0 -hls_segment_type fmp4 -hls_time 3 output_test.m3u8
>>> liuqideMacBook-Pro:xxx liuqi$ head -n 10  output_test.m3u8
>>> #EXTM3U
>>> #EXT-X-VERSION:7
>>> #EXT-X-TARGETDURATION:3
>>> #EXT-X-MEDIA-SEQUENCE:0
>>> #EXT-X-MAP:URI="init.mp4"
>>> #EXTINF:3.866667,
>>> output_test0.m4s
>>> #EXTINF:7.300000,
>>> output_test1.m4s
>>> #EXTINF:8.333333,
>>> 
>>> The RFC https://www.rfc-editor.org/rfc/rfc8216.txt describe:
>>> 
>>> 4.3.3.1.  EXT-X-TARGETDURATION
>>> 
>>> The EXT-X-TARGETDURATION tag specifies the maximum Media Segment
>>> duration.  The EXTINF duration of each Media Segment in the Playlist
>>> file, when rounded to the nearest integer, MUST be less than or equal
>>> to the target duration; longer segments can trigger playback stalls
>>> or other errors.  It applies to the entire Playlist file.  Its format
>>> is:
>>> 
>>> #EXT-X-TARGETDURATION:<s>
>>> 
>>> where s is a decimal-integer indicating the target duration in
>>> seconds.  The EXT-X-TARGETDURATION tag is REQUIRED.
>>> 
>>> your patch make the EXT-X-TARGETDURATION less than EXTINF:7.300000 EXTINF:8.333333
>> 
>> 
>>>>>> 2. the version maybe will update when use hls_segment_type or append_list etc. when the operation is support from different version m3u8.
>>>>> 
>>>>> I don't follow what you mean here. The version number is known up front, based on the options that were passed in. It should be illegal to switch between versions when trying to update an existing manifest. When can this legitimately happen?
>>>> there maybe have some player cannot support high version of m3u8, for example old parser or player just support the VERSION 3,
>>>> this must think about all of the player or parser, because ffmpeg is not used only by myself.
>>>> 
>>>> Or what about get the #EXT-X-VERSION position, to update it? looks like flvenc.c or movenc.c date shift
>>>> 
>>>>> 
>>>>>> 3. need update segments vs->segments when hls_list_size option is set.
>>>>>> 
>>>>> 
>>>>> What do you mean by this and where should I do it?
>>>> for example, hls_list_size is 4, the m3u8 list should refresh every time when make a new fragment.
>>>> 
>>>> first time:
>>>> 1.m4s
>>>> 2.m4s
>>>> 3.m4s
>>>> 4.m4s
>>>> 
>>>> sencond time:
>>>> 2.m4s
>>>> 3.m4s
>>>> 4.m4s
>>>> 5.m4s
>>> 
>>> after your patch:
>>> 
>>> liuqideMacBook-Pro:xxx liuqi$  ./ffmpeg -v quiet -i ~/Movies/Test/bbb_sunflower_1080p_30fps_normal.mp4 -c copy -f hls -hls_list_size 4 -hls_segment_type fmp4 -hls_time 3 -t 50 output_test.m3u8
>>> liuqideMacBook-Pro:xxx liuqi$ cat output_test.m3u8
>>> #EXTM3U
>>> #EXT-X-VERSION:7
>>> #EXT-X-TARGETDURATION:3
>>> #EXT-X-MEDIA-SEQUENCE:0
>>> #EXT-X-MAP:URI="init.mp4"
>>> #EXTINF:3.866667,
>>> output_test0.m4s
>>> #EXTINF:7.300000,
>>> output_test1.m4s
>>> #EXTINF:8.333333,
>>> output_test2.m4s
>>> #EXTINF:3.966667,
>>> output_test3.m4s
>>> #EXTINF:8.333333,
>>> output_test4.m4s
>>> #EXTINF:4.033333,
>>> output_test5.m4s
>>> #EXTINF:8.333333,
>>> output_test6.m4s
>>> #EXTINF:4.633333,
>>> output_test7.m4s
>>> liuqideMacBook-Pro:xxx liuqi$
>>> liuqideMacBook-Pro:xxx liuqi$
>>> 
>>> the m3u8 list is incorrect, because users want control the m3u8 list length, because their disk do not have enough space to save the fragments.
>>> 
>> 
>> Ok I will fix this.


I'm attaching a new patch that resolves all of these issues, while still resolving this bug for VOD playlists.

Can you please review?
Thanks,

Ronak
Ronak Patel Aug. 5, 2018, 1:48 p.m. UTC | #7
> I'm attaching a new patch that resolves all of these issues, while still resolving this bug for VOD playlists.
> 
> Can you please review?
> 
> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>

Hi Steven,

Can you please have a look at the updated patch and tell me what you think?

I also fixed the temp_file implementation.

If you like this approach, I’ll do this for dash as well.

I’ve noticed that ffmpeg always assumes the dash stream is live until the very end and doesn’t support profiles (onDemand, live, etc) like mp4box does. I’m going to have to add that to dashenc.

Ronak

> 
> Thanks,
> 
> Ronak
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index b5644f0..b15645d 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -488,7 +488,6 @@  static int hls_delete_old_segments(AVFormatContext *s, HLSContext *hls,
         }
         p = (char *)av_basename(dirname);
         *p = '\0';
-
     }
 
     while (segment) {
@@ -1365,63 +1364,37 @@  fail:
     return ret;
 }
 
-static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
+static int hls_write_manifest_segment(AVFormatContext *s, int last, VariantStream *vs)
 {
     HLSContext *hls = s->priv_data;
     HLSSegment *en;
-    int target_duration = 0;
     int ret = 0;
-    char temp_filename[1024];
-    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
-    const char *proto = avio_find_protocol_name(s->url);
-    int use_rename = proto && !strcmp(proto, "file");
-    static unsigned warned_non_file;
+    int byterange_mode;
     char *key_uri = NULL;
     char *iv_string = NULL;
     AVDictionary *options = NULL;
     double prog_date_time = vs->initial_prog_date_time;
     double *prog_date_time_p = (hls->flags & HLS_PROGRAM_DATE_TIME) ? &prog_date_time : NULL;
-    int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
-
-    hls->version = 3;
-    if (byterange_mode) {
-        hls->version = 4;
-        sequence = 0;
-    }
-
-    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
-        hls->version = 6;
-    }
 
-    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
-        hls->version = 7;
-    }
+    if (last) {
 
-    if (!use_rename && !warned_non_file++)
-        av_log(s, AV_LOG_ERROR, "Cannot use rename on non file protocol, this may lead to races and temporary partial files\n");
+        if ((hls->flags & HLS_OMIT_ENDLIST==0)) {
+            ff_hls_write_end_list(hls->m3u8_out);
+        }
 
-    set_http_options(s, &options, hls);
-    snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : "%s", vs->m3u8_name);
-    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 0)
-        goto fail;
+        if (vs->vtt_m3u8_name) {
+            ff_hls_write_end_list(hls->sub_m3u8_out);
+        }
+    } else {
 
-    for (en = vs->segments; en; en = en->next) {
-        if (target_duration <= en->duration)
-            target_duration = lrint(en->duration);
-    }
+        byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
 
-    vs->discontinuity_set = 0;
-    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache,
-                                 target_duration, sequence, hls->pl_type);
+        en = vs->last_segment;
+        if (en == NULL) {
+            ret = 1;
+            goto fail;
+        }
 
-    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
-        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
-        vs->discontinuity_set = 1;
-    }
-    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
-        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
-    }
-    for (en = vs->segments; en; en = en->next) {
         if ((hls->encrypt || hls->key_info_file) && (!key_uri || strcmp(en->key_uri, key_uri) ||
                                     av_strcasecmp(en->iv_string, iv_string))) {
             avio_printf(hls->m3u8_out, "#EXT-X-KEY:METHOD=AES-128,URI=\"%s\"", en->key_uri);
@@ -1444,17 +1417,8 @@  static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
         if (ret < 0) {
             av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
         }
-    }
-
-    if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
-        ff_hls_write_end_list(hls->m3u8_out);
 
-    if( vs->vtt_m3u8_name ) {
-        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0)
-            goto fail;
-        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
-                                     target_duration, sequence, PLAYLIST_TYPE_NONE);
-        for (en = vs->segments; en; en = en->next) {
+        if( vs->vtt_m3u8_name ) {
             ret = ff_hls_write_file_entry(hls->sub_m3u8_out, 0, byterange_mode,
                                           en->duration, 0, en->size, en->pos,
                                           vs->baseurl, en->sub_filename, NULL);
@@ -1462,22 +1426,16 @@  static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
                 av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
             }
         }
-
-        if (last)
-            ff_hls_write_end_list(hls->sub_m3u8_out);
-
     }
 
 fail:
     av_dict_free(&options);
-    hlsenc_io_close(s, &hls->m3u8_out, temp_filename);
-    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
-    if (ret >= 0 && use_rename)
-        ff_rename(temp_filename, vs->m3u8_name, s);
 
-    if (ret >= 0 && hls->master_pl_name)
-        if (create_master_playlist(s, vs) < 0)
+    if (ret >= 0 && hls->master_pl_name) {
+        if (create_master_playlist(s, vs) < 0) {
             av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");
+        }
+    }
 
     return ret;
 }
@@ -2078,6 +2036,11 @@  static int hls_write_header(AVFormatContext *s)
     int ret, i, j;
     AVDictionary *options = NULL;
     VariantStream *vs = NULL;
+    int target_duration = 0;
+    int byterange_mode = 0;
+
+    target_duration = lrint(hls->time);
+    byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
 
     for (i = 0; i < hls->nb_varstreams; i++) {
         vs = &hls->var_streams[i];
@@ -2091,7 +2054,7 @@  static int hls_write_header(AVFormatContext *s)
             goto fail;
         }
         av_dict_free(&options);
-        //av_assert0(s->nb_streams == hls->avf->nb_streams);
+
         for (j = 0; j < vs->nb_streams; j++) {
             AVStream *inner_st;
             AVStream *outer_st = vs->streams[j];
@@ -2130,6 +2093,47 @@  static int hls_write_header(AVFormatContext *s)
             }
         }
     }
+
+    // write the header for the main hls playlist
+    int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - vs->nb_entries);
+    hls->version = 3;
+    if (byterange_mode) {
+        hls->version = 4;
+        sequence = 0;
+    }
+
+    if (hls->flags & HLS_INDEPENDENT_SEGMENTS) {
+        hls->version = 6;
+    }
+
+    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
+        hls->version = 7;
+    }
+
+    set_http_options(s, &options, hls);
+    if ((ret = hlsenc_io_open(s, &hls->m3u8_out, vs->m3u8_name, &options)) < 0) {
+        goto fail;
+    }
+
+    vs->discontinuity_set = 0;
+    ff_hls_write_playlist_header(hls->m3u8_out, hls->version, hls->allowcache, target_duration, sequence, hls->pl_type);
+
+    if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && vs->discontinuity_set==0 ){
+        avio_printf(hls->m3u8_out, "#EXT-X-DISCONTINUITY\n");
+        vs->discontinuity_set = 1;
+    }
+    if (vs->has_video && (hls->flags & HLS_INDEPENDENT_SEGMENTS)) {
+        avio_printf(hls->m3u8_out, "#EXT-X-INDEPENDENT-SEGMENTS\n");
+    }
+
+    if( vs->vtt_m3u8_name ) {
+        if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, &options)) < 0) {
+            goto fail;
+        }
+
+        ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache, target_duration, sequence, PLAYLIST_TYPE_NONE);
+    }
+
 fail:
 
     return ret;
@@ -2334,10 +2338,11 @@  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
             return ret;
         }
 
-        if (!vs->fmp4_init_mode || byterange_mode)
-            if ((ret = hls_window(s, 0, vs)) < 0) {
+        if (!vs->fmp4_init_mode || byterange_mode) {
+            if ((ret = hls_write_manifest_segment(s, 0, vs)) < 0) {
                 return ret;
             }
+        }
     }
 
     vs->packets_written++;
@@ -2421,7 +2426,7 @@  failed:
         avformat_free_context(oc);
 
         vs->avf = NULL;
-        hls_window(s, 1, vs);
+        hls_write_manifest_segment(s, 1, vs);
 
         av_freep(&vs->fmp4_init_filename);
         if (vtt_oc) {
@@ -2447,6 +2452,9 @@  failed:
         av_freep(&ccs->language);
     }
 
+    hlsenc_io_close(s, &hls->m3u8_out, vs->m3u8_name);
+    hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
+
     ff_format_io_close(s, &hls->m3u8_out);
     ff_format_io_close(s, &hls->sub_m3u8_out);
     av_freep(&hls->key_basename);
@@ -2884,3 +2892,4 @@  AVOutputFormat ff_hls_muxer = {
     .write_trailer  = hls_write_trailer,
     .priv_class     = &hls_class,
 };
+