Message ID | 20191216000418.24707-6-andreas.rheinhardt@gmail.com |
---|---|
State | Accepted |
Commit | bd131b64bc308ab036d0bbe9da0a49f482ef94f9 |
Headers | show |
> 在 2019年12月16日,08:04,Andreas Rheinhardt <andreas.rheinhardt@gmail.com> 写道: > > Under certain circumstances hls_write_packet() would add options to an > AVDictionary. Said dictionary was never explicitly freed, instead it was > presumed that these options would be consumed when opening a new > IO-context. This left several possibilities for memleaks: > > a) When no new IO-context would be opened at all. This is possible when > using both the flags temp_file and single_file together with a file > output. > b) When an error happens before one actually tries to open the new > IO-context. > c) When the new IO-context does not consume all options. > > All three have been fixed; furthermore, the AVDictionary has been put > into a smaller scope (namely the only part of hls_write_packet() where > it is actually used). > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > This patch is concerned with fixing memleaks. Yet I noticed a > discrepancy in the handling of options between the first try to open > output and the second one (the one for reflushing in case the first one > fails): The second one only gets the options not consumed in the first > one. Is this intended? > > libavformat/hlsenc.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index 5695af2208..5b3856099c 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -2241,7 +2241,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) > int use_temp_file = 0; > uint8_t *buffer = NULL; > VariantStream *vs = NULL; > - AVDictionary *options = NULL; > char *old_filename = NULL; > > for (i = 0; i < hls->nb_varstreams; i++) { > @@ -2343,11 +2342,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) > use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE); > } > > - // look to rename the asset name > - if (use_temp_file) { > - av_dict_set(&options, "mpegts_flags", "resend_headers", 0); > - } > - > if (hls->flags & HLS_SINGLE_FILE) { > ret = flush_dynbuf(vs, &range_length); > av_freep(&vs->temp_buffer); > @@ -2356,8 +2350,8 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) > } > vs->size = range_length; > } else { > - set_http_options(s, &options, hls); > if ((hls->max_seg_size > 0 && (vs->size >= hls->max_seg_size)) || !byterange_mode) { > + AVDictionary *options = NULL; > char *filename = NULL; > if (hls->key_info_file || hls->encrypt) { > av_dict_set(&options, "encryption_key", hls->key_string, 0); > @@ -2367,12 +2361,21 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) > filename = av_asprintf("%s", oc->url); > } > if (!filename) { > + av_dict_free(&options); > return AVERROR(ENOMEM); > } > + > + // look to rename the asset name > + if (use_temp_file) > + av_dict_set(&options, "mpegts_flags", "resend_headers", 0); > + > + set_http_options(s, &options, hls); > + > ret = hlsenc_io_open(s, &vs->out, filename, &options); > if (ret < 0) { > av_log(s, hls->ignore_io_errors ? AV_LOG_WARNING : AV_LOG_ERROR, > "Failed to open file '%s'\n", filename); > + av_dict_free(&options); > return hls->ignore_io_errors ? 0 : ret; > } > if (hls->segment_type == SEGMENT_TYPE_FMP4) { > @@ -2380,6 +2383,7 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) > } > ret = flush_dynbuf(vs, &range_length); > if (ret < 0) { > + av_dict_free(&options); > return ret; > } > ret = hlsenc_io_close(s, &vs->out, filename); > @@ -2391,6 +2395,7 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) > reflush_dynbuf(vs, &range_length); > ret = hlsenc_io_close(s, &vs->out, filename); > } > + av_dict_free(&options); > av_freep(&vs->temp_buffer); > av_freep(&filename); > } > -- > 2.20.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". LGTM Thanks Steven
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 5695af2208..5b3856099c 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -2241,7 +2241,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) int use_temp_file = 0; uint8_t *buffer = NULL; VariantStream *vs = NULL; - AVDictionary *options = NULL; char *old_filename = NULL; for (i = 0; i < hls->nb_varstreams; i++) { @@ -2343,11 +2342,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) use_temp_file = proto && !strcmp(proto, "file") && (hls->flags & HLS_TEMP_FILE); } - // look to rename the asset name - if (use_temp_file) { - av_dict_set(&options, "mpegts_flags", "resend_headers", 0); - } - if (hls->flags & HLS_SINGLE_FILE) { ret = flush_dynbuf(vs, &range_length); av_freep(&vs->temp_buffer); @@ -2356,8 +2350,8 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) } vs->size = range_length; } else { - set_http_options(s, &options, hls); if ((hls->max_seg_size > 0 && (vs->size >= hls->max_seg_size)) || !byterange_mode) { + AVDictionary *options = NULL; char *filename = NULL; if (hls->key_info_file || hls->encrypt) { av_dict_set(&options, "encryption_key", hls->key_string, 0); @@ -2367,12 +2361,21 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) filename = av_asprintf("%s", oc->url); } if (!filename) { + av_dict_free(&options); return AVERROR(ENOMEM); } + + // look to rename the asset name + if (use_temp_file) + av_dict_set(&options, "mpegts_flags", "resend_headers", 0); + + set_http_options(s, &options, hls); + ret = hlsenc_io_open(s, &vs->out, filename, &options); if (ret < 0) { av_log(s, hls->ignore_io_errors ? AV_LOG_WARNING : AV_LOG_ERROR, "Failed to open file '%s'\n", filename); + av_dict_free(&options); return hls->ignore_io_errors ? 0 : ret; } if (hls->segment_type == SEGMENT_TYPE_FMP4) { @@ -2380,6 +2383,7 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) } ret = flush_dynbuf(vs, &range_length); if (ret < 0) { + av_dict_free(&options); return ret; } ret = hlsenc_io_close(s, &vs->out, filename); @@ -2391,6 +2395,7 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) reflush_dynbuf(vs, &range_length); ret = hlsenc_io_close(s, &vs->out, filename); } + av_dict_free(&options); av_freep(&vs->temp_buffer); av_freep(&filename); }
Under certain circumstances hls_write_packet() would add options to an AVDictionary. Said dictionary was never explicitly freed, instead it was presumed that these options would be consumed when opening a new IO-context. This left several possibilities for memleaks: a) When no new IO-context would be opened at all. This is possible when using both the flags temp_file and single_file together with a file output. b) When an error happens before one actually tries to open the new IO-context. c) When the new IO-context does not consume all options. All three have been fixed; furthermore, the AVDictionary has been put into a smaller scope (namely the only part of hls_write_packet() where it is actually used). Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- This patch is concerned with fixing memleaks. Yet I noticed a discrepancy in the handling of options between the first try to open output and the second one (the one for reflushing in case the first one fails): The second one only gets the options not consumed in the first one. Is this intended? libavformat/hlsenc.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)