[FFmpeg-devel] [PATCH] libavformat/hlsenc: fix broken -hls_flags +temp_file
Steven Liu
lq at chinaffmpeg.org
Fri Dec 14 16:21:21 EET 2018
> 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
More information about the ffmpeg-devel
mailing list