> On Aug 6, 2018, at 7:19 AM, Liu Steven <l...@chinaffmpeg.org> wrote:
>
>
>
>>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo....@ffmpeg.org> 写道：
>>>
>>>
>>> On Aug 5, 2018, at 10:54 PM, Liu Steven <l...@chinaffmpeg.org> wrote:
>>>
>>>
>>>
>>>> 在 2018年8月4日，上午2:17，Ronak <ronak2...@yahoo.com> 写道：
>>>>
>>>>>>>>>> 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?
>>>>
>>>>
>>>> <0001-libavformat-hlsenc-Fix-HLS-Manifest-Generation-from-.patch>
>>>
>>> From bbc4870c0d685f5c07e82042c3f2ef153d83f3d1 Mon Sep 17 00:00:00 2001
>>> From: "Ronak Patel (Audible)" <ron...@audible.com>
>>> Date: Thu, 2 Aug 2018 09:25:12 -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
>>> entire manifest at the end for VOD playlists. Live & Event Playlists are
>>> unaffected.
>>> This also fixes the behavior with HLS_TEMP_FILE to work correctly when
>>> -hlsflags temp_file is specified, instead of always relying on use_rename,
>>> which caused these problems.
>>>
>>> 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 <ronak2...@yahoo.com>
>>> ---
>>> libavformat/dashenc.c | 2 +-
>>> libavformat/hlsenc.c | 54
>>> +++++++++++++++++++++++++++++++++------------------
>>> 2 files changed, 36 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>>
>>> -------you have modify dash
>>
>> Sorry I will submit this separately. Will remove.
>>
>>> index ae57fd5..ae22c08 100644
>>> --- a/libavformat/dashenc.c
>>> +++ b/libavformat/dashenc.c
>>> @@ -483,7 +483,7 @@ static void output_segment_list(OutputStream *os,
>>> AVIOContext *out, AVFormatCont
>>> target_duration = lrint(duration);
>>> }
>>>
>>> - ff_hls_write_playlist_header(c->m3u8_out, 6, -1, target_duration,
>>> + ff_hls_write_playlist_header(c->m3u8_out, 7, -1, target_duration,
>>> start_number, PLAYLIST_TYPE_NONE);
>>>
>>> ff_hls_write_init_file(c->m3u8_out, os->initfile, c->single_file,
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index b5644f0..0eb0801 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -942,6 +942,7 @@ static int
>>> sls_flag_use_localtime_filename(AVFormatContext *oc, HLSContext *c, V
>>> if (c->flags & (HLS_SECOND_LEVEL_SEGMENT_SIZE |
>>> HLS_SECOND_LEVEL_SEGMENT_DURATION)) {
>>> av_strlcpy(vs->current_segment_final_filename_fmt, oc->url,
>>> sizeof(vs->current_segment_final_filename_fmt));
>>> +
>>> ——you add a empty line
>> Ok will remove
>>
>>> if (c->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) {
>>> char *filename = NULL;
>>> if (replace_int_data_in_filename(&filename, oc->url, 's', 0) < 1)
>>> {
>>> @@ -1166,9 +1167,10 @@ static int hls_rename_temp_file(AVFormatContext *s,
>>> AVFormatContext *oc)
>>>
>>> if (!final_filename)
>>> return AVERROR(ENOMEM);
>>> +
>>> ——you add a empty line
>> Ok will remove
>>>
>>> final_filename[len-4] = '\0';
>>> +
>>>
>>> ——you add a empty line
>> Ok will remove
>>> ret = ff_rename(oc->url, final_filename, s);
>>> - oc->url[len-4] = '\0’;
>>> ——Why do you give the len - 4 = 0?
>> This code was already there. All I’m doing is removing this part that was
>> causing a problem when you use the HLS_TEMP option.
>>
>>> av_freep(&final_filename);
>>> return ret;
>>> }
>>> @@ -1373,9 +1375,7 @@ static int hls_window(AVFormatContext *s, int last,
>>> VariantStream *vs)
>>> 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 use_temp_file = (s->flags & HLS_TEMP_FILE);
>>> ——What will have if use http put method?
>>
>> I’ll test it.
>>
>>> char *key_uri = NULL;
>>> char *iv_string = NULL;
>>> AVDictionary *options = NULL;
>>> @@ -1397,11 +1397,9 @@ static int hls_window(AVFormatContext *s, int last,
>>> VariantStream *vs)
>>> hls->version = 7;
>>> }
>>>
>>> - 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");
>>> -
>>> ——I have see this message long time, i have not remove this message because
>>> this is used to http method. Why do you remove it?
>>
>> I can keep it for http put and HLS_TEMP if that’s what you want.
>>
>>>
>>> set_http_options(s, &options, hls);
>>> - snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" :
>>> "%s", vs->m3u8_name);
>>> + snprintf(temp_filename, sizeof(temp_filename), use_temp_file ?
>>> "%s.tmp" : "%s", vs->m3u8_name);
>>> + //av_log(s, AV_LOG_INFO, "We're going to write out to %s",
>>> temp_filename);
>>> ------this info message is unused?
>>
>> Ok will remove.
>>
>>> if ((ret = hlsenc_io_open(s, has->m3u8_out, temp_filename, &options)) < 0)
>>> goto fail;
>>>
>>> @@ -1472,8 +1470,9 @@ 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 (use_temp_file) {
>>> + ff_rename(temp_filename, vs->m3u8_name, s);
>>> + }
>>>
>>> if (ret >= 0 && hls->master_pl_name)
>>> if (create_master_playlist(s, vs) < 0)
>>> @@ -2253,11 +2252,14 @@ static int hls_write_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>> hlsenc_io_close(s, &vs->vtt_avf->pb, vs->vtt_avf->url);
>>> }
>>> }
>>> +
>>> + // look to rename the asset name
>>> if ((hls->flags & HLS_TEMP_FILE) && oc->url[0]) {
>>> - if (!(hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size <=
>>> 0))
>>> - if ((vs->avf->oformat->priv_class && vs->avf->priv_data)
>>> && hls->segment_type != SEGMENT_TYPE_FMP4)
>>> - av_opt_set(vs->avf->priv_data, "mpegts_flags",
>>> "resend_headers", 0);
>>> - hls_rename_temp_file(s, oc);
>>> + if (!(hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size <=
>>> 0)) {
>>> + if ((vs->avf->oformat->priv_class && vs->avf->priv_data)
>>> && hls->segment_type != SEGMENT_TYPE_FMP4) {
>>> + av_opt_set(vs->avf->priv_data, "mpegts_flags",
>>> "resend_headers", 0);
>>> + }
>>> + }
>>> ——Just reindent?
>>
>> Yes I put the code properly under braces. But if you don’t like it, I can
>> remove.
>>
>>> }
>>>
>>> if (vs->fmp4_init_mode) {
>>> @@ -2286,6 +2288,17 @@ static int hls_write_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>> return ret;
>>> }
>>> ff_format_io_close(s, &vs->out);
>>> +
>>> + // rename that segment from .tmp to the real one
>>> + if ((hls->flags & HLS_TEMP_FILE) && oc->url[0]) {
>>> + hls_rename_temp_file(s, oc);
>>> + av_free(old_filename);
>>> + old_filename = av_strdup(vs->avf->url);
>>> +
>>> + if (!old_filename) {
>>> + return AVERROR(ENOMEM);
>>> + }
>>> + }
>>> }
>>> }
>>>
>>> @@ -2334,14 +2347,16 @@ static int hls_write_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>> return ret;
>>> }
>>>
>>> - if (!vs->fmp4_init_mode || byterange_mode)
>>> + // if we're building a VOD playlist, skip writing the manifest
>>> multiple times, and just wait until the end
>>> + if (hls->pl_type != PLAYLIST_TYPE_VOD) {
>>> ------Whatever event VOD or Live Streaming, the EXT-X-TARGETDURATION need
>>> refresh when lrint(current fragment duration) is large than lrint(the
>>> before duration).
>>
>> You do not need to do this for VOD. This is the main reason why ffmpeg takes
>> over 7 days to fragment a 153 hour VOD audio file. Other tools can do it in
>> less than 5 minutes. Why would you write the same VOD playlist over 155000
>> times on disk when the input is never changing.
>>
>> VOD does not ever change once it is written so it makes sense to wait until
>> the very end to write out the entire manifest. There’s no refreshing
>> required. Only live or event playlists need any sort of refresh. Ffmpeg is
>> written predominantly for the event or live use case I’ve noticed, which has
>> forced you to make poor decisions for VOD.
>>
>> If I could I would rewrite all of this logic to clearly separate VOD from
>> event and live playlist generation logic to make the code clearer. This is
>> hard to understand code in general.
>
> Maybe you want record the fragments to a VOD Service, or time shift for the
> history stream. Do i guess right or wrong?
>>
If that’s what you want, you would wait until the very end anyway. Why would
you upload a partial manifest file?
For VOD, you don’t do anything until you get the whole manifest. If you wanted
a partial one, that’s an event, and you can pass that option to get the
refreshing behavior.
Even with event, this algorithm of writing out the entire manifest just to add
a new segment is very wasteful, slow and unnecessary. This is an O(N!)
algorithm for something you can solve in O(N) time. You can just append and
adjust the EXT-X-TARGETDURATION for your video content.
>>>
>>> if ((ret = hls_window(s, 0, vs)) < 0) {
>>> return ret;
>>> }
>>> + }
>>> }
>>>
>>> - vs->packets_written++;
>>> ret = ff_write_chained(oc, stream_index, pkt, s, 0);
>>> + vs->packets_written++;
>>>
>>> return ret;
>>> }
>>> @@ -2394,15 +2409,16 @@ failed:
>>> if (hls->segment_type != SEGMENT_TYPE_FMP4)
>>> ff_format_io_close(s, &oc->pb);
>>>
>>> - if ((hls->flags & HLS_TEMP_FILE) && oc->url[0]) {
>>> - hls_rename_temp_file(s, oc);
>>> + // rename that segment from .tmp to the real one
>>> + if ((hls->flags & HLS_TEMP_FILE) && oc->url[0] && !(hls->flags
>>> & HLS_SINGLE_FILE)) {
>>> + hls_rename_temp_file(s, oc);
>>> av_free(old_filename);
>>> old_filename = av_strdup(vs->avf->url);
>>>
>>> if (!old_filename) {
>>> return AVERROR(ENOMEM);
>>> }
>>> - }
>>> + }
>>>
>>> /* after av_write_trailer, then duration + 1 duration per packet
>>> */
>>> hls_append_segment(s, hls, vs, vs->duration + vs->dpp,
>>> vs->start_pos, vs->size);
>>> --
>>> 2.6.3
>>>
>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>>>> Thanks,
>>>>
>>>> Ronak
>>>>
>>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
