> 在 2018年8月6日，下午7:12，Ronak Patel <[email protected]> 写道：
>
>>
>> On Aug 5, 2018, at 10:54 PM, Liu Steven <[email protected]> wrote:
>>
>>
>>
>>> 在 2018年8月4日，上午2:17，Ronak <[email protected]> 写道：
>>>
>>>>>>>>> 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)" <[email protected]>
>> 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 <[email protected]>
>> ---
>> 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
>> [email protected]
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> _______________________________________________
> ffmpeg-devel mailing list
> [email protected]
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
[email protected]
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
