[FFmpeg-devel] [PATCH] libavformat/hlsenc: fix broken -hls_flags +temp_file
Ronak Patel
ronak2121 at yahoo.com
Sat Dec 22 15:44:30 EET 2018
> On Dec 16, 2018, at 10:31 PM, Steven Liu <lingjiujianke at gmail.com> wrote:
>
> Aleksey Skripka <caspy at undev.ru> 于2018年12月14日周五 下午10:58写道：
>>
>>
>> From e85edcc4d8b0312c7871f78ed0859ec7436be460 Mon Sep 17 00:00:00 2001
>> From: Aleksey Skripka <caspy at undev.ru>
>> Date: Fri, 14 Dec 2018 14:48:31 +0000
>> Subject: [PATCH] libavformat/hlsenc: fix broken -hls_flags +temp_file
>>
>> 1. fix addressing '->flags' while assigning 'use_temp_file'.
>> 2. before 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6 playlist file was always created via .tmp for 'file' proto, now not. keep old behavior.
>> 3. rename logic in hls_write_packet() incorrectly placed (in fMP4-only code), thus not renaming files for mpegts.
>> 4. needless av_free() call.
Hi Aleksey,
Just wanted to make sure this wouldn’t affect standard VOD use cases running on a file on disk. We have some really gigantic audio files and we can’t sustain fragmentation times over a few minutes. Taking a week to fragment something is not acceptable.
I hope we don’t make temp files in those cases. Did you test that on the command line?
Ronak
>> ---
>> libavformat/hlsenc.c | 23 +++++++----------------
>> 1 file changed, 7 insertions(+), 16 deletions(-)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index bdd2a11..70eee19 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -1358,7 +1358,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_temp_file = proto && !strcmp(proto, "file") && (s->flags & HLS_TEMP_FILE);
>> + int use_temp_file = proto && !strcmp(proto, "file");
>> static unsigned warned_non_file;
>> char *key_uri = NULL;
>> char *iv_string = NULL;
>> @@ -1478,7 +1478,7 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
>> 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);
>> + int use_temp_file = proto && !strcmp(proto, "file") && (c->flags & HLS_TEMP_FILE);
>> char *filename, iv_string[KEYSIZE*2 + 1];
>> int err = 0;
>>
>> @@ -2143,7 +2143,7 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>> 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);
>> + int use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE);
>> uint8_t *buffer = NULL;
>> VariantStream *vs = NULL;
>> AVDictionary *options = NULL;
>> @@ -2266,7 +2266,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>> if (hls->flags & HLS_SINGLE_FILE) {
>> ret = flush_dynbuf(vs, &range_length);
>> if (ret < 0) {
>> - av_free(old_filename);
>> return ret;
>> }
>> vs->size = range_length;
>> @@ -2284,20 +2283,12 @@ 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);
>> - }
>> - }
>> }
>> }
>>
>> + if (use_temp_file && oc->url[0] && !(hls->flags & HLS_SINGLE_FILE))
>> + hls_rename_temp_file(s, oc);
>> +
>> old_filename = av_strdup(vs->avf->url);
>> if (!old_filename) {
>> return AVERROR(ENOMEM);
>> @@ -2367,7 +2358,7 @@ static int hls_write_trailer(struct AVFormatContext *s)
>> 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 use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE);
>> int i;
>> int ret = 0;
>> VariantStream *vs = NULL;
>> --
>> 1.8.3.1
>>
>> --
>> Aleksey Skripka
>>
>>
>>
>>> On 14 Dec 2018, at 17:21, Steven Liu <lq at chinaffmpeg.org> wrote:
>>>
>>>
>>>
>>>>> On Dec 14, 2018, at 19:23, Aleksey Skripka <caspy at undev.ru> wrote:
>>>>>
>>>>>
>>>>> On 14 Dec 2018, at 13:10, Liu Steven <lq at chinaffmpeg.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> 在 2018年12月14日，下午5:27，Aleksey Skripka <caspy at undev.ru> 写道：
>>>>>>
>>>>>> greetings!
>>>>>>
>>>>>> fixed version.
>>>>>> thanks.
>>>>> Is this patch create by git format-patch ?
>>>> no.
>>>> diff -up fileA fileB
>>> https://ffmpeg.org/developer.html#Submitting-patches-1
>>> This is the rule for make a patch, I think your code is ok. :D
>>>
>>>>
>>>>>>
>>>>>> ---
>>>>>> --- libavformat/hlsenc.c.orig 2018-12-14 09:25:06.541809226 +0000
>>>>>> +++ libavformat/hlsenc.c 2018-12-14 09:19:16.129377384 +0000
>>>>>> @@ -1348,7 +1348,7 @@ static int hls_window(AVFormatContext *s
>>>>>> 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_temp_file = proto && !strcmp(proto, "file") && (s->flags & HLS_TEMP_FILE);
>>>>>> + int use_temp_file = proto && !strcmp(proto, "file");
>>>>>> static unsigned warned_non_file;
>>>>>> char *key_uri = NULL;
>>>>>> char *iv_string = NULL;
>>>>>> @@ -1463,7 +1463,7 @@ static int hls_start(AVFormatContext *s,
>>>>>> 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);
>>>>>> + int use_temp_file = proto && !strcmp(proto, "file") && (c->flags & HLS_TEMP_FILE);
>>>>>> char *filename, iv_string[KEYSIZE*2 + 1];
>>>>>> int err = 0;
>>>>>>
>>>>>> @@ -2123,7 +2123,7 @@ static int hls_write_packet(AVFormatCont
>>>>>> 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);
>>>>>> + int use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE);
>>>>>> uint8_t *buffer = NULL;
>>>>>> VariantStream *vs = NULL;
>>>>>> AVDictionary *options = NULL;
>>>>>> @@ -2251,7 +2251,6 @@ static int hls_write_packet(AVFormatCont
>>>>>> if (hls->flags & HLS_SINGLE_FILE) {
>>>>>> ret = flush_dynbuf(vs, &range_length);
>>>>>> if (ret < 0) {
>>>>>> - av_free(old_filename);
>>>>>> return ret;
>>>>>> }
>>>>>> vs->size = range_length;
>>>>>> @@ -2269,20 +2268,12 @@ static int hls_write_packet(AVFormatCont
>>>>>> 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);
>>>>>> - }
>>>>>> - }
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> + if (use_temp_file && oc->url[0] && !(hls->flags & HLS_SINGLE_FILE))
>>>>>> + hls_rename_temp_file(s, oc);
>>>>>> +
>>>>>> old_filename = av_strdup(vs->avf->url);
>>>>>> if (!old_filename) {
>>>>>> return AVERROR(ENOMEM);
>>>>>> @@ -2348,7 +2339,7 @@ static int hls_write_trailer(struct AVFo
>>>>>> 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 use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE);
>>>>>> int i;
>>>>>> int ret = 0;
>>>>>> VariantStream *vs = NULL;
>>>>>> ---
>>>>>>
>>>>>> --
>>>>>> Aleksey Skripka
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 14 Dec 2018, at 11:38, Liu Steven <lq at chinaffmpeg.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> 在 2018年12月14日，下午3:04，Aleksey Skripka <caspy at undev.ru> 写道：
>>>>>>>>
>>>>>>>> greetings!
>>>>>>>>
>>>>>>>> after commit 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6 temp_file functionality totally broken.
>>>>>>>>
>>>>>>>> attached patch prototype will fix:
>>>>>>>> 1) while assigning 'use_temp_file' addressing to '->flags' is done incorrectly in 4 places.
>>>>>>>> 2) before that commit playlist was always created via .tmp for 'file' proto, now not. sure we should keep it in such way, thus not look for this flag in hls_window().
>>>>>>>> 3) rename logic in hls_write_packet() was accidentally moved to fMP4-only code, thus not renaming files for mpegts.
>>>>>>>> 4) av_free() call, where variable always NULL.
>>>>>>>>
>>>>>>>> please take a look.
>>>>>>>>
>>>>>>>> ---
>>>>>>>> --- libavformat/hlsenc.c.orig 2018-12-13 13:27:03.307499151 +0000
>>>>>>>> +++ libavformat/hlsenc.c 2018-12-13 20:19:59.781833259 +0000
>>>>>>>> @@ -1348,7 +1348,7 @@ static int hls_window(AVFormatContext *s
>>>>>>>> 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_temp_file = proto && !strcmp(proto, "file") && (s->flags & HLS_TEMP_FILE);
>>>>>>>> + int use_temp_file = proto && !strcmp(proto, "file")/* && (hls->flags & HLS_TEMP_FILE)*/; // always use .tmp logic for 'file' proto.
>>>>>>>> static unsigned warned_non_file;
>>>>>>>> char *key_uri = NULL;
>>>>>>>> char *iv_string = NULL;
>>>>>>>> @@ -1463,7 +1463,7 @@ static int hls_start(AVFormatContext *s,
>>>>>>>> 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);
>>>>>>>> + int use_temp_file = proto && !strcmp(proto, "file") && (c->flags & HLS_TEMP_FILE);
>>>>>>>> char *filename, iv_string[KEYSIZE*2 + 1];
>>>>>>>> int err = 0;
>>>>>>>>
>>>>>>>> @@ -2123,7 +2123,7 @@ static int hls_write_packet(AVFormatCont
>>>>>>>> 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);
>>>>>>>> + int use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE);
>>>>>>>> uint8_t *buffer = NULL;
>>>>>>>> VariantStream *vs = NULL;
>>>>>>>> AVDictionary *options = NULL;
>>>>>>>> @@ -2249,7 +2249,8 @@ static int hls_write_packet(AVFormatCont
>>>>>>>> if (hls->flags & HLS_SINGLE_FILE) {
>>>>>>>> ret = flush_dynbuf(vs, &range_length);
>>>>>>>> if (ret < 0) {
>>>>>>>> - av_free(old_filename);
>>>>>>>> +// old_filename not yet defined here
>>>>>>>> +// av_free(old_filename);
>>>>>>>> return ret;
>>>>>>>> }
>>>>>>>> vs->size = range_length;
>>>>>>>> @@ -2268,19 +2269,23 @@ static int hls_write_packet(AVFormatCont
>>>>>>>> }
>>>>>>>> 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);
>>>>>>>> - }
>>>>>>>> - }
>>>>>>>> +// bad place for rename here, it does not rename non-fmp4 files
>>>>>>>> +// // 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);
>>>>>>>> +// }
>>>>>>>> +// }
>>>>>>> remove the code block when it unused.
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>> + if (use_temp_file && oc->url[0] && !(hls->flags & HLS_SINGLE_FILE))
>>>>>>>> + hls_rename_temp_file(s, oc);
>>>>>>>> +
>>>>>>>> old_filename = av_strdup(vs->avf->url);
>>>>>>>> if (!old_filename) {
>>>>>>>> return AVERROR(ENOMEM);
>>>>>>>> @@ -2346,7 +2351,7 @@ static int hls_write_trailer(struct AVFo
>>>>>>>> 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 use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE);
>>>>>>>> int i;
>>>>>>>> int ret = 0;
>>>>>>>> VariantStream *vs = NULL;
>>>>>>>> ---
>>>>>>>> --
>>>>>>>> Aleksey Skripka
>>>>>>>> --
>>>>>>>> Aleksey Skripka
>>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Steven
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> ffmpeg-devel mailing list
>>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> ffmpeg-devel mailing list
>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>
>>>>>> _______________________________________________
>>>>>> ffmpeg-devel mailing list
>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>
>>>>> _______________________________________________
>>>>> ffmpeg-devel mailing list
>>>>> ffmpeg-devel at ffmpeg.org
>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel at ffmpeg.org
>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> Thanks
>>> Steven
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> will push
>
>
> Thanks
>
> Steven
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list