This patchset is primarily concerned with fixing memleaks in the hls muxer. I was drawn to this by a memleak created by ed897633; it is fixed in the first patch.*
Looking through the code I found more improvable stuff; and some of this is fixed in the patches that follow. Yet there is still stuff left to do: 1. hls_init() currently calls hls_start() which writes the subtitle header (if there are subtitles). Yet it is forbidden for an init function to output anything. 2. There are currently two places in the code where there is an unchecked call to avio_open_dyn_buf(), one in hls_write_packet() and one in hls_write_trailer(): range_length = avio_close_dyn_buf(oc->pb, &buffer); avio_write(vs->out, buffer, range_length); av_freep(&buffer); vs->init_range_length = range_length; avio_open_dyn_buf(&oc->pb); vs->packets_written = 0; vs->start_pos = range_length; I was unsure whether one should simply add a check for this (and make sure that oc->pb is NULL when there is no dynamic buffer open) or whether one should use flush_dynbuf() (the latter also flushes vs->out after the write, don't know whether this would be desirable here). 3. av_write_trailer(oc) is always called, even when oc->pb doesn't exist because of the allocation failure at 2. This should probably be made conditional on the existence of oc->pb? 4. Whatever gets written in av_write_trailer(oc) gets written into a dynamic buffer that gets freed a few lines below writing the trailer; it doesn't seem to get output at all. 5. The subtitle output context meanwhile only writes its trailer when it actually has an AVIOContext; yet even said AVIOContext is NULL, avio_tell() is called with it as argument. The result will be nonsense. 6. It seems to me that it is intended for the subtitles to be output directly, without being written into a dynamic buffer first. Yet there is a catch: It seems to be possible (in the sense of: I can't rule out that it happens) for the subtitle AVIOContext to be treated as dynamic buffer, despite not being one. This might happen in the code snippet I inserted in 2., because in general oc might be the subtitle AVFormatContext. 7. There are still some possible memleaks: a) The dynamic buffers are not yet freed in the deinit function (that is added in this patchset), because I only want to add this when it is certain that they are valid which means: When the second point is fixed. b) Neither vs->out nor the subtitles AVIOContext is closed in the deinit function yet. The latter depends upon 6. c) The temp_buffer that is sometimes used to "reflush" data if the first write attempt failed might currently leak in some situations. This has not been addressed yet, because the temp_buffer's lifetime is not supposed to extend beyond hls_write_packet() resp. hls_write_trailer() so that I wonder whether it should not be removed from the VariantStream structure and instead replaced by something on the stack. 8. Is using av_opt_set() as is done in hls_start() actually legal? I thought one is supposed to provide these options inside a dictionary when opening the output context. - Andreas *: The valgrind fate-boxes on http://fate.ffmpeg.org/ don't show the hls tests as failing because the memleaks already happen when generating sample files for said tests. These tests are therefore not run at all. In other words: If the number of tests for the valgrind boxes is less than for the normal boxes, there is probably a memleak somewhere. Andreas Rheinhardt (16): avformat/hlsenc: Fix leak of child AVFormatContext avformat/hlsenc: Fix typo in error message avformat/hlsenc: Only allocate when data is known to be needed avformat/hlsenc: Fix leak of options when initializing muxing fails avformat/hlsenc: Fix leak of options when writing packets avformat/hlsenc: Use smaller scope for some variables avformat/hlsenc: Fix potential segfault upon allocation failure avformat/hlsenc: Add deinit function avformat/hlsenc: Check some unchecked allocations avformat/hlsenc: Fix return value from localtime_r failure avformat/hlsenc: Fix memleaks with repeating parameters avformat/hlsenc: Unconditionally free some strings avformat/hlsenc: Fix check for presence of webvtt muxer avformat/hlsenc: Localize initialization of subtitle streams avformat/hlsenc: Factor check out of loop avformat/hlsenc: Cosmetics libavformat/hlsenc.c | 350 +++++++++++++++++++------------------------ 1 file changed, 158 insertions(+), 192 deletions(-) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
