diff mbox

[FFmpeg-devel] libavformat/hlsenc: fix broken -hls_flags +temp_file

Message ID 42AAD417-F8C1-4ED0-9167-7F02ED2E2929@undev.ru
State New
Headers show

Commit Message

Aleksey Skripka Dec. 14, 2018, 2:58 p.m. UTC 
From e85edcc4d8b0312c7871f78ed0859ec7436be460 Mon Sep 17 00:00:00 2001
From: Aleksey Skripka <caspy@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.
---
 libavformat/hlsenc.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

--
1.8.3.1

Comments

Steven Liu Dec. 17, 2018, 3:31 a.m. UTC | #1 
Aleksey Skripka <caspy@undev.ru> 于2018年12月14日周五 下午10:58写道：
>
>
> From e85edcc4d8b0312c7871f78ed0859ec7436be460 Mon Sep 17 00:00:00 2001
> From: Aleksey Skripka <caspy@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.
> ---
>  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@chinaffmpeg.org> wrote:
> >
> >
> >
> >> On Dec 14, 2018, at 19:23, Aleksey Skripka <caspy@undev.ru> wrote:
> >>
> >>>
> >>> On 14 Dec 2018, at 13:10, Liu Steven <lq@chinaffmpeg.org> wrote:
> >>>
> >>>
> >>>
> >>>> 在 2018年12月14日，下午5:27，Aleksey Skripka <caspy@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@chinaffmpeg.org> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>> 在 2018年12月14日，下午3:04，Aleksey Skripka <caspy@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@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
> >>
> >> _______________________________________________
> >> 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
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

will push


Thanks

Steven
Ronak Patel Dec. 22, 2018, 1:44 p.m. UTC | #2 
> On Dec 16, 2018, at 10:31 PM, Steven Liu <lingjiujianke@gmail.com> wrote:
> 
> Aleksey Skripka <caspy@undev.ru> 于2018年12月14日周五 下午10:58写道：
>> 
>> 
>> From e85edcc4d8b0312c7871f78ed0859ec7436be460 Mon Sep 17 00:00:00 2001
>> From: Aleksey Skripka <caspy@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@chinaffmpeg.org> wrote:
>>> 
>>> 
>>> 
>>>>> On Dec 14, 2018, at 19:23, Aleksey Skripka <caspy@undev.ru> wrote:
>>>>> 
>>>>> 
>>>>> On 14 Dec 2018, at 13:10, Liu Steven <lq@chinaffmpeg.org> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>>> 在 2018年12月14日，下午5:27，Aleksey Skripka <caspy@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@chinaffmpeg.org> wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>> 在 2018年12月14日，下午3:04，Aleksey Skripka <caspy@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@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
>>>> 
>>>> _______________________________________________
>>>> 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
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> will push
> 
> 
> Thanks
> 
> Steven
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

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;