| Message ID | 279D2022-59BA-4338-A187-B51C6781CD59@chinaffmpeg.org |
|---|---|
| State | New |
| Headers | show |
> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: > > > >> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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. > > 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
> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： > >> >> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >> >> >> >>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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 ((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
> On Aug 6, 2018, at 7:19 AM, Liu Steven <lq@chinaffmpeg.org> wrote: > > > >>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： >>> >>> >>> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >>> >>> >>> >>>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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
> On Aug 6, 2018, at 19:29, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote: > >> >> On Aug 6, 2018, at 7:19 AM, Liu Steven <lq@chinaffmpeg.org> wrote: >> >> >> >>>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： >>>> >>>> >>>> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>> >>>> >>>> >>>>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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. Yes, I agreed with you and cannot agreed more. O(N!), but is the list is too short, that is not a problem. But, why do you create a list long time to 7 days? The hls_list_size default is 5 fragments, and the hls live streaming (No Endlist) is play from last 3 fragment, i can not understand what do you want. let me make sure, do you want record long list? or maybe you want make some files to use disk? > >>>> >>>> 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 Steven
> On Aug 6, 2018, at 10:20 AM, Steven Liu <lq@chinaffmpeg.org> wrote: > > > >> On Aug 6, 2018, at 19:29, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >> >>> >>> On Aug 6, 2018, at 7:19 AM, Liu Steven <lq@chinaffmpeg.org> wrote: >>> >>> >>> >>>>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： >>>>> >>>>> >>>>> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>>> >>>>> >>>>> >>>>>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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. > Yes, I agreed with you and cannot agreed more. O(N!), but is the list is too short, that is not a problem. But, why do you create a list long time to 7 days? The hls_list_size default is 5 fragments, and the hls live streaming (No Endlist) is play from last 3 fragment, i can not understand what do you want. let me make sure, do you want record long list? or maybe you want make some files to use disk? > Hi Steven, Sorry it's taken me some time to get back to you. I was busy with other things. So to clarify my exact situation and why this is a problem for me. We have really long audio files that we stream over HLS & DASH. We always stream in VOD mode. We DO NOT do any EVENT or LIVE streaming. Our audio files are generally at least 10 hours long, but can be as long as 153 hours long. So, in the trunk version of ffmpeg, if you try to run this command: ffmpeg -i output_22_32.aac -codec copy -hls_time 0.975238095238095 -hls_segment_type fmp4 -hls_flags single_file -hls_playlist_type vod output_22_32_1sec_ts.m3u8 The O(N!) algorithm that ffmpeg uses takes 7 days for this command to finish executing. This is unacceptable for us, we need to be able to prepare our content in seconds or minutes, not days or weeks. My patch is looking to resolve the crux of this issue which are: 1. The unnecessary creation/deletion of a temp file when I didn't even ask for one (HLS_TEMP option wasn't specified). 2. The unnecessary regeneration of the entire manifest when we are just appending a new segment. I hope this context helps you understand the problem. I'll submit a new patch with your comments resolved shortly. Ronak >> >>>>> >>>>> 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 > Steven > > > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> On Aug 8, 2018, at 3:52 PM, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote: > > > >> On Aug 6, 2018, at 10:20 AM, Steven Liu <lq@chinaffmpeg.org> wrote: >> >> >> >>> On Aug 6, 2018, at 19:29, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >>> >>>> >>>> On Aug 6, 2018, at 7:19 AM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>> >>>> >>>> >>>>>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： >>>>>> >>>>>> >>>>>> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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. >> Yes, I agreed with you and cannot agreed more. O(N!), but is the list is too short, that is not a problem. But, why do you create a list long time to 7 days? The hls_list_size default is 5 fragments, and the hls live streaming (No Endlist) is play from last 3 fragment, i can not understand what do you want. let me make sure, do you want record long list? or maybe you want make some files to use disk? >> > > Hi Steven, > > Sorry it's taken me some time to get back to you. I was busy with other things. So to clarify my exact situation and why this is a problem for me. We have really long audio files that we stream over HLS & DASH. We always stream in VOD mode. We DO NOT do any EVENT or LIVE streaming. Our audio files are generally at least 10 hours long, but can be as long as 153 hours long. > > So, in the trunk version of ffmpeg, if you try to run this command: > > ffmpeg -i output_22_32.aac -codec copy -hls_time 0.975238095238095 -hls_segment_type fmp4 -hls_flags single_file -hls_playlist_type vod output_22_32_1sec_ts.m3u8 > > The O(N!) algorithm that ffmpeg uses takes 7 days for this command to finish executing. This is unacceptable for us, we need to be able to prepare our content in seconds or minutes, not days or weeks. > > My patch is looking to resolve the crux of this issue which are: > > 1. The unnecessary creation/deletion of a temp file when I didn't even ask for one (HLS_TEMP option wasn't specified). > 2. The unnecessary regeneration of the entire manifest when we are just appending a new segment. > > I hope this context helps you understand the problem. > > I'll submit a new patch with your comments resolved shortly. > > Ronak > Hi Steven, Please see my new patch taking your feedback into account. Ronak >>> >>>>>> >>>>>> 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 >> Steven >> >> >> >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> On Aug 8, 2018, at 5:37 PM, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote: > > > >> On Aug 8, 2018, at 3:52 PM, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >> >> >> >>> On Aug 6, 2018, at 10:20 AM, Steven Liu <lq@chinaffmpeg.org> wrote: >>> >>> >>> >>>>> On Aug 6, 2018, at 19:29, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >>>>> >>>>> >>>>> On Aug 6, 2018, at 7:19 AM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>>> >>>>> >>>>> >>>>>>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： >>>>>>> >>>>>>> >>>>>>> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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. >>> Yes, I agreed with you and cannot agreed more. O(N!), but is the list is too short, that is not a problem. But, why do you create a list long time to 7 days? The hls_list_size default is 5 fragments, and the hls live streaming (No Endlist) is play from last 3 fragment, i can not understand what do you want. let me make sure, do you want record long list? or maybe you want make some files to use disk? >>> >> >> Hi Steven, >> >> Sorry it's taken me some time to get back to you. I was busy with other things. So to clarify my exact situation and why this is a problem for me. We have really long audio files that we stream over HLS & DASH. We always stream in VOD mode. We DO NOT do any EVENT or LIVE streaming. Our audio files are generally at least 10 hours long, but can be as long as 153 hours long. >> >> So, in the trunk version of ffmpeg, if you try to run this command: >> >> ffmpeg -i output_22_32.aac -codec copy -hls_time 0.975238095238095 -hls_segment_type fmp4 -hls_flags single_file -hls_playlist_type vod output_22_32_1sec_ts.m3u8 >> >> The O(N!) algorithm that ffmpeg uses takes 7 days for this command to finish executing. This is unacceptable for us, we need to be able to prepare our content in seconds or minutes, not days or weeks. >> >> My patch is looking to resolve the crux of this issue which are: >> >> 1. The unnecessary creation/deletion of a temp file when I didn't even ask for one (HLS_TEMP option wasn't specified). >> 2. The unnecessary regeneration of the entire manifest when we are just appending a new segment. >> >> I hope this context helps you understand the problem. >> >> I'll submit a new patch with your comments resolved shortly. >> >> Ronak >> > > Hi Steven, > > Please see my new patch taking your feedback into account. > > Ronak Hi Steven, Did you have a chance to review? I’m going to send you a new patch for dashenc.c shortly. For dashenc.c, ffmpeg again always assumes we are doing live streaming. It doesn’t even have another profile. I’m going to add a VOD profile to it and fix this problem there. I will have to add a new cli parameter so we can specify the profile now. Ronak > > >>>> >>>>>>> >>>>>>> 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 >>> Steven >>> >>> >>> >>> >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>> >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> >> >> Hi Steven, >> >> Please see my new patch taking your feedback into account. >> >> Ronak > > Hi Steven, > > Did you have a chance to review? I’m going to send you a new patch for dashenc.c shortly. Don’t worry, i will review it weekend, but i think your ways is not a better way, maybe just change the hls_window maybe ok. the other code need not modify. > > For dashenc.c, ffmpeg again always assumes we are doing live streaming. It doesn’t even have another profile. I’m going to add a VOD profile to it and fix this problem there. I will have to add a new cli parameter so we can specify the profile now. > > Ronak
On Aug 10, 2018, at 7:30 AM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>> >>> >>> Hi Steven, >>> >>> Please see my new patch taking your feedback into account. >>> >>> Ronak >> >> Hi Steven, >> >> Did you have a chance to review? I’m going to send you a new patch for dashenc.c shortly. > Don’t worry, i will review it weekend, but i think your ways is not a better way, maybe just change the hls_window maybe ok. the other code need not modify. >> The other code fixes the problem that HLS_TEMP option wasn’t working. Use_rename should only be on if it’s been specified on the command line. >> For dashenc.c, ffmpeg again always assumes we are doing live streaming. It doesn’t even have another profile. I’m going to add a VOD profile to it and fix this problem there. I will have to add a new cli parameter so we can specify the profile now. >> >> Ronak >
> On Aug 10, 2018, at 6:31 AM, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote: > >> >> On Aug 8, 2018, at 5:37 PM, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >> >> >> >>> On Aug 8, 2018, at 3:52 PM, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >>> >>> >>> >>>> On Aug 6, 2018, at 10:20 AM, Steven Liu <lq@chinaffmpeg.org> wrote: >>>> >>>> >>>> >>>>>> On Aug 6, 2018, at 19:29, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >>>>>> >>>>>> >>>>>> On Aug 6, 2018, at 7:19 AM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>>>> >>>>>> >>>>>> >>>>>>>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： >>>>>>>> >>>>>>>> >>>>>>>> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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. >>>> Yes, I agreed with you and cannot agreed more. O(N!), but is the list is too short, that is not a problem. But, why do you create a list long time to 7 days? The hls_list_size default is 5 fragments, and the hls live streaming (No Endlist) is play from last 3 fragment, i can not understand what do you want. let me make sure, do you want record long list? or maybe you want make some files to use disk? >>>> >>> >>> Hi Steven, >>> >>> Sorry it's taken me some time to get back to you. I was busy with other things. So to clarify my exact situation and why this is a problem for me. We have really long audio files that we stream over HLS & DASH. We always stream in VOD mode. We DO NOT do any EVENT or LIVE streaming. Our audio files are generally at least 10 hours long, but can be as long as 153 hours long. >>> >>> So, in the trunk version of ffmpeg, if you try to run this command: >>> >>> ffmpeg -i output_22_32.aac -codec copy -hls_time 0.975238095238095 -hls_segment_type fmp4 -hls_flags single_file -hls_playlist_type vod output_22_32_1sec_ts.m3u8 >>> >>> The O(N!) algorithm that ffmpeg uses takes 7 days for this command to finish executing. This is unacceptable for us, we need to be able to prepare our content in seconds or minutes, not days or weeks. >>> >>> My patch is looking to resolve the crux of this issue which are: >>> >>> 1. The unnecessary creation/deletion of a temp file when I didn't even ask for one (HLS_TEMP option wasn't specified). >>> 2. The unnecessary regeneration of the entire manifest when we are just appending a new segment. >>> >>> I hope this context helps you understand the problem. >>> >>> I'll submit a new patch with your comments resolved shortly. >>> >>> Ronak >>> >> >> Hi Steven, >> >> Please see my new patch taking your feedback into account. >> >> Ronak > > Hi Steven, > > Did you have a chance to review? I’m going to send you a new patch for dashenc.c shortly. > > For dashenc.c, ffmpeg again always assumes we are doing live streaming. It doesn’t even have another profile. I’m going to add a VOD profile to it and fix this problem there. I will have to add a new cli parameter so we can specify the profile now. > > Ronak Hi Steven, Did you have a chance to review this patch? I haven't seen your feedback yet. Did you have a chance to try it out? I would like to get this merged this week. Ronak > >> >> >>>>> >>>>>>>> >>>>>>>> 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 >>>> Steven >>>> >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>>> >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>> >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> > Hi Steven, > > Did you have a chance to review this patch? I haven't seen your feedback yet. Did you have a chance to try it out? > I would like to get this merged this week. 1. you MUST update the TARGET DURATION if the new fragment is long than old fragment, don't only test your audio files please, ffmpeg not only be used by you. 2. if the temp file have problem, you should seperate the patch into two patch, one fix temp file problem, the other implement your feature. 3. I need more time to think about how to improve the refresh problem when in VOD mode.Because this patch is not a good way, that is not a full care about the other function. Thanks > > Ronak >
On Aug 13, 2018, at 10:49 AM, Steven Liu <lingjiujianke@gmail.com> wrote: > >> >> Hi Steven, >> >> Did you have a chance to review this patch? I haven't seen your feedback yet. Did you have a chance to try it out? >> I would like to get this merged this week. > 1. you MUST update the TARGET DURATION if the new fragment is long > than old fragment, don't only test your audio files please, ffmpeg not > only be used by you. This patch already does that. I've confirmed with several video files. Please see the attached manifest that was generated by this patch as proof. This is a video file downloaded from here: https://www.sample-videos.com/index.php#sample-mp4-video <https://www.sample-videos.com/index.php#sample-mp4-video>. > 2. if the temp file have problem, you should seperate the patch into > two patch, one fix temp file problem, the other implement your > feature. The two problems are interrelated. The creation/destruction of temp files repeatedly adds to the generation latency here. In my use-case, ffmpeg generates and destroys the temp file 155000 times when: 1. This is offline VOD. I am just generating from my laptop to my laptop. No Web Servers are involved. 2. I did not explicitly ask for this. > 3. I need more time to think about how to improve the refresh problem > when in VOD mode.Because this patch is not a good way, that is not a > full care about the other function. The alternative way is the one I was proposing first; which was to write the header once; and then the segments only in the hls_window method. However, that has problems like you pointed out, due to the architecture that ffmpeg has. We can still pursue that path, but it will be a much more complicated fix. I would need much more of your help to fix this. Dashenc.c also suffers from this same problem. This needs to be fixed in both places, not just hlsenc.c. > > Thanks >> >> Ronak >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> On Aug 13, 2018, at 4:48 PM, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote: > > On Aug 13, 2018, at 10:49 AM, Steven Liu <lingjiujianke@gmail.com> wrote: >> >>> >>> Hi Steven, >>> >>> Did you have a chance to review this patch? I haven't seen your feedback yet. Did you have a chance to try it out? >>> I would like to get this merged this week. >> 1. you MUST update the TARGET DURATION if the new fragment is long >> than old fragment, don't only test your audio files please, ffmpeg not >> only be used by you. > > This patch already does that. I've confirmed with several video files. Please see the attached manifest that was generated by this patch as proof. This is a video file downloaded from here: https://www.sample-videos.com/index.php#sample-mp4-video <https://www.sample-videos.com/index.php#sample-mp4-video>. Trying to send the attachment again, it got removed for some reason. > >> 2. if the temp file have problem, you should seperate the patch into >> two patch, one fix temp file problem, the other implement your >> feature. > > The two problems are interrelated. The creation/destruction of temp files repeatedly adds to the generation latency here. In my use-case, ffmpeg generates and destroys the temp file 155000 times when: 1. This is offline VOD. I am just generating from my laptop to my laptop. No Web Servers are involved. 2. I did not explicitly ask for this. > >> 3. I need more time to think about how to improve the refresh problem >> when in VOD mode.Because this patch is not a good way, that is not a >> full care about the other function. > > The alternative way is the one I was proposing first; which was to write the header once; and then the segments only in the hls_window method. > However, that has problems like you pointed out, due to the architecture that ffmpeg has. We can still pursue that path, but it will be a much more complicated fix. I would need much more of your help to fix this. > Dashenc.c also suffers from this same problem. This needs to be fixed in both places, not just hlsenc.c. > >> >> 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
> On Aug 14, 2018, at 05:06, Ronak <ronak2121-at-yahoo.com@ffmpeg.org> wrote: > >> >> On Aug 13, 2018, at 10:49 AM, Steven Liu <lingjiujianke@gmail.com> wrote: >>> >>>> >>>> Hi Steven, >>>> >>>> Did you have a chance to review this patch? I haven't seen your feedback yet. Did you have a chance to try it out? >>>> I would like to get this merged this week. >>> 1. you MUST update the TARGET DURATION if the new fragment is long >>> than old fragment, don't only test your audio files please, ffmpeg not >>> only be used by you. >> >> This patch already does that. I've confirmed with several video files. Please see the attached manifest that was generated by this patch as proof. This is a video file downloaded from here: https://www.sample-videos.com/index.php#sample-mp4-video<https://www.sample-videos.com/index.php#sample-mp4-video>. > > Trying to send the attachment again, it got removed for some reason. > > <output_test_video.m3u8> >> >>> 2. if the temp file have problem, you should seperate the patch into >>> two patch, one fix temp file problem, the other implement your >>> feature. >> >> The two problems are interrelated. The creation/destruction of temp files repeatedly adds to the generation latency here. In my use-case, ffmpeg generates and destroys the temp file 155000 times when: 1. This is offline VOD. I am just generating from my laptop to my laptop. No Web Servers are involved. 2. I did not explicitly ask for this. >> >>> 3. I need more time to think about how to improve the refresh problem >>> when in VOD mode.Because this patch is not a good way, that is not a >>> full care about the other function. >> >> The alternative way is the one I was proposing first; which was to write the header once; and then the segments only in the hls_window method. >> However, that has problems like you pointed out, due to the architecture that ffmpeg has. We can still pursue that path, but it will be a much more complicated fix. I would need much more of your help to fix this. >> Dashenc.c also suffers from this same problem. This needs to be fixed in both places, not just hlsenc.c. >> Could you please resend a new version patch, use "git send-email”, make patch use “git format-patch -v” to make patch version. I cannot find the newest patch :(. Then the patch can be picked up by patchwork. Thanks Steven
2018.08.06. 16:20 keltezéssel, Steven Liu írta: > >> On Aug 6, 2018, at 19:29, Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> wrote: >> >>> On Aug 6, 2018, at 7:19 AM, Liu Steven <lq@chinaffmpeg.org> wrote: >>> >>> >>> >>>>> 在 2018年8月6日，下午7:12，Ronak Patel <ronak2121-at-yahoo.com@ffmpeg.org> 写道： >>>>> >>>>> >>>>> On Aug 5, 2018, at 10:54 PM, Liu Steven <lq@chinaffmpeg.org> wrote: >>>>> >>>>> >>>>> >>>>>> 在 2018年8月4日，上午2:17，Ronak <ronak2121@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)" <ronakp@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 <ronak2121@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. > Yes, I agreed with you and cannot agreed more. O(N!), but is the list is too short, that is not a problem. But, why do you create a list long time to 7 days? The hls_list_size default is 5 fragments, and the hls live streaming (No Endlist) is play from last 3 fragment, i can not understand what do you want. let me make sure, do you want record long list? or maybe you want make some files to use disk? I think both of you are right. There are cases when first behaviour is better but there are cases when the other way is more convinent. I suggest to have a new option to control the behaviour. The default setting/behaviour should the current one to not to break current apps. bb >>>>> 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 > Steven > > > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c -------you have modify dash 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 if (c->flags & HLS_SECOND_LEVEL_SEGMENT_SIZE) { char *filename = NULL;