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

Submitted by Aleksey Skripka on Dec. 14, 2018, 7:04 a.m.

Details

Message ID 6095A590-81BB-4B94-A60F-DBF87B853ECF@undev.ru
State New
Headers show

Commit Message

Aleksey Skripka Dec. 14, 2018, 7:04 a.m.
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.

---
---

Comments

Steven Liu Dec. 14, 2018, 8:38 a.m.
> 在 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

Patch hide | download patch | download mbox

--- 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);
+//                    }
+//                }
             }
         }

+        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;