> On Aug 15, 2018, at 6:46 AM, Liu Steven <l...@chinaffmpeg.org> wrote:
> 
> 
> 
>>> 在 2018年8月15日,下午6:37,Ronak Patel <ronak2...@yahoo.com> 写道:
>>> 
>>> 
>>> On Aug 14, 2018, at 10:57 PM, Liu Steven <l...@chinaffmpeg.org> wrote:
>>> 
>>> 
>>> 
>>>> 在 2018年8月15日,上午9:31,Ronak <ronak2121-at-yahoo....@ffmpeg.org> 写道:
>>>> 
>>>> From: "Ronak Patel" <ronak2121@ <mailto:ronak2...@yahoo.com>yahoo.com>
>>>> 
>>>> This fixes the creation of the hls manifest in hlsenc.c by writing the 
>>>> entire manifest at the end for VOD playlists. Live & Event Playlists are 
>>>> unaffected.
>>>> This also fixes the behavior with HLS_TEMP_FILE to work correctly when 
>>>> -hlsflags temp_file is specified, instead of always relying on use_rename, 
>>>> which caused these problems.
>>>> 
>>>> Files that would previously take over a week to fragment now take 1 minute 
>>>> on the same hardware. This was a 153 hour audio file (2.2GB of audio).
>>>> 
>>>> Signed-off-by: Ronak Patel <ronak2...@yahoo.com 
>>>> <mailto:ronak2...@yahoo.com>>
>>>> ---
>>>> libavformat/hlsenc.c | 51 
>>>> ++++++++++++++++++++++++++++++++++++---------------
>>>> 1 file changed, 36 insertions(+), 15 deletions(-)
>>>> 
>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>> index b5644f0..7933b79 100644
>>>> --- a/libavformat/hlsenc.c
>>>> +++ b/libavformat/hlsenc.c
>>>> @@ -1168,7 +1168,6 @@ static int hls_rename_temp_file(AVFormatContext *s, 
>>>> AVFormatContext *oc)
>>>>      return AVERROR(ENOMEM);
>>>>  final_filename[len-4] = '\0';
>>>>  ret = ff_rename(oc->url, final_filename, s);
>>>> -    oc->url[len-4] = '\0';
>>>>  av_freep(&final_filename);
>>>>  return ret;
>>>> }
>>>> @@ -1374,7 +1373,7 @@ static int hls_window(AVFormatContext *s, int last, 
>>>> VariantStream *vs)
>>>>  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");
>>>> +    int use_temp_file = proto && !strcmp(proto, "file") && (s->flags & 
>>>> HLS_TEMP_FILE);
>>>>  static unsigned warned_non_file;
>>>>  char *key_uri = NULL;
>>>>  char *iv_string = NULL;
>>>> @@ -1397,11 +1396,11 @@ static int hls_window(AVFormatContext *s, int 
>>>> last, VariantStream *vs)
>>>>      hls->version = 7;
>>>>  }
>>>> -    if (!use_rename && !warned_non_file++)
>>>> +    if (!use_temp_file && !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");
>>>>  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);
>>>>  if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 
>>>> 0)
>>>>      goto fail;
>>>> @@ -1472,8 +1471,8 @@ 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)
>>>> @@ -1488,6 +1487,8 @@ static int hls_start(AVFormatContext *s, 
>>>> VariantStream *vs)
>>>>  AVFormatContext *oc = vs->avf;
>>>>  AVFormatContext *vtt_oc = vs->vtt_avf;
>>>>  AVDictionary *options = NULL;
>>>> +    const char *proto = avio_find_protocol_name(s->url);
>>>> +    int use_temp_file = proto && !strcmp(proto, "file") && (s->flags & 
>>>> HLS_TEMP_FILE);
>>>>  char *filename, iv_string[KEYSIZE*2 + 1];
>>>>  int err = 0;
>>>> @@ -1583,7 +1584,7 @@ static int hls_start(AVFormatContext *s, 
>>>> VariantStream *vs)
>>>>  set_http_options(s, &options, c);
>>>> -    if (c->flags & HLS_TEMP_FILE) {
>>>> +    if (use_temp_file) {
>>>>      char *new_name = av_asprintf("%s.tmp", oc->url);
>>>>      if (!new_name)
>>>>          return AVERROR(ENOMEM);
>>>> @@ -2145,6 +2146,8 @@ static int hls_write_packet(AVFormatContext *s, 
>>>> AVPacket *pkt)
>>>>  int ret = 0, can_split = 1, i, j;
>>>>  int stream_index = 0;
>>>>  int range_length = 0;
>>>> +    const char *proto = avio_find_protocol_name(s->url);
>>>> +    int use_temp_file = proto && !strcmp(proto, "file") && (s->flags & 
>>>> HLS_TEMP_FILE);
>>>>  uint8_t *buffer = NULL;
>>>>  VariantStream *vs = NULL;
>>>>  AVDictionary *options = NULL;
>>>> @@ -2214,10 +2217,10 @@ static int hls_write_packet(AVFormatContext *s, 
>>>> AVPacket *pkt)
>>>>  }
>>>> +    char *old_filename = NULL;
>>>>  if (vs->packets_written && can_split && av_compare_ts(pkt->pts - 
>>>> vs->start_pts, st->time_base,
>>>>                                                        end_pts, 
>>>> AV_TIME_BASE_Q) >= 0) {
>>>>      int64_t new_start_pos;
>>>> -        char *old_filename = NULL;
>>>>      int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || 
>>>> (hls->max_seg_size > 0);
>>>>      av_write_frame(vs->avf, NULL); /* Flush any buffered data */
>>>> @@ -2253,11 +2256,13 @@ static int hls_write_packet(AVFormatContext *s, 
>>>> AVPacket *pkt)
>>>>              hlsenc_io_close(s, &vs->vtt_avf->pb, vs->vtt_avf->url);
>>>>          }
>>>>      }
>>>> -        if ((hls->flags & HLS_TEMP_FILE) && oc->url[0]) {
>>>> +
>>>> +        // look to rename the asset name
>>>> +        if (use_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 ((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);
>>>> +                }
>>>>      }
>>>>      if (vs->fmp4_init_mode) {
>>>> @@ -2286,6 +2291,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 (use_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,10 +2350,12 @@ static int hls_write_packet(AVFormatContext *s, 
>>>> AVPacket *pkt)
>>>>          return ret;
>>>>      }
>>>> -        if (!vs->fmp4_init_mode || byterange_mode)
>>> why do you remove this line?
>> 
>> Because the above code sets fmp4_init_mode to 0 and this is always true. I 
>> tested mpegts and fmp4 in single file and multiple file modes and this is 
>> always true.
> There have a logic:
> 
> 771     if (hls->segment_type == SEGMENT_TYPE_FMP4) {
> 772         if (hls->max_seg_size > 0) {
> 773             av_log(s, AV_LOG_WARNING, "Multi-file byterange mode is 
> currently unsupported in the HLS muxer.\n");
> 774             return AVERROR_PATCHWELCOME;
> 775         }
> 776
> 777         vs->packets_written = 0;
> 778         vs->init_range_length = 0;
> 779         vs->fmp4_init_mode = !byterange_mode;
> 

This is not called between the lines that sets fmp4_init_mode to 0. It’s also 
not called by “hls_start” either. hls_mux_init is not invoked here. It is only 
invoked once at init.

Even then, this logic sets fmp4_init_mode to be the converse of byterange_mode. 
In the if statement I removed, this will always be true.

>> 
>>>> +        // 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) {
>>> maybe your mean is just write one time when PLAYLIST_TYPE_VOD, that is 
>>> happen at the end of the full list, is that right?
>> 
>> Yes. Like I mentioned before there is no need to refresh the whole file 
>> every single time.
>> 
>> This is not the best fix in my opinion either but it is the simplest. The 
>> best way would be to split the code based on the playlist type so it was 
>> easier to follow. This is altogether confusing code.
>> 
>>>>          if ((ret = hls_window(s, 0, vs)) < 0) {
>>>>              return ret;
>>>>          }
>>>> +        }
>>>>  }
>>>>  vs->packets_written++;
>>>> @@ -2352,6 +2370,8 @@ static int hls_write_trailer(struct AVFormatContext 
>>>> *s)
>>>>  AVFormatContext *oc = NULL;
>>>>  AVFormatContext *vtt_oc = NULL;
>>>>  char *old_filename = NULL;
>>>> +    const char *proto = avio_find_protocol_name(s->url);
>>>> +    int use_temp_file = proto && !strcmp(proto, "file") && (s->flags & 
>>>> HLS_TEMP_FILE);
>>>>  int i;
>>>>  int ret = 0;
>>>>  VariantStream *vs = NULL;
>>>> @@ -2394,8 +2414,9 @@ 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 (use_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);
>>>> -- 
>>>> 2.6.3
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel@ffmpeg.org
>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to