> On Aug 16, 2018, at 08:39, Ronak Patel <ronak2...@yahoo.com> wrote: > >> >> On Aug 15, 2018, at 8:21 PM, Steven Liu <l...@chinaffmpeg.org> wrote: >> >> >> >>>> On Aug 16, 2018, at 07:41, Ronak Patel <ronak2...@yahoo.com> wrote: >>>> >>>> >>>> On Aug 15, 2018, at 11:08 AM, Steven Liu <l...@chinaffmpeg.org> wrote: >>>> >>>> >>>> >>>>> On Aug 15, 2018, at 09:31, Ronak <ronak2121-at-yahoo....@ffmpeg.org> >>>>> wrote: >>>>> >>>>> From: "Ronak Patel" <ronak2121@ <mailto:ron...@audible.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); >>>>> + } >>>>> + } >>>> All of these is TAB? >>> >>> If I switch to spaces, do you have any other comments? Are you happy with >>> this approach overall? >> mhmm, maybe have other comments, maybe have no, I don’t think this is a good >> way, but you need read https://ffmpeg.org/developer.html at first for the >> code style. >>> >>> I don’t want to waste time on an approach you dislike here. >>> >>> We still have to go fix dashenc.c for this same problem. >> Ok, you can do that first if you dislike continue to hlsenc. >> >> Then, if your patch merged, there have have other problem, for example: >> people want request the m3u8 list when ffmpeg processing the list. > > I would like to fix both here (dash and hls), not just one. > > I’ll fix the tabs to spaces here. > > For VOD, I doubt that would happen where we’d need to look at an intermediary > point in the playlist. ok, maybe i should modify this after your patch, append the fragment info to the list and flush the file every time. but i think that is waste time double coding for the VOD type hls, i cannot sure there have or have no people to request the list as live streaming when use this mode, I think that maybe better compatible all possibility. > > For EVENT and LIVE it makes more sense. > >>> >>>>> } >>>>> } >>>>> @@ -2334,10 +2350,12 @@ 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) { >>>>> 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)) { >>>> Is this TAB? >>>>> + 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 >>>> >>>> Thanks >>>> Steven >>>> >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> Thanks >> Steven >> >> >> >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Thanks Steven _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
