[FFmpeg-devel] [PATCH] libavformat/hlsenc: fix broken -hls_flags +temp_file
Steven Liu
lingjiujianke at gmail.com
Mon Dec 17 05:31:49 EET 2018
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.
> ---
> 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
More information about the ffmpeg-devel
mailing list