[FFmpeg-devel] [PATCH 00/16] Memleaks in hlsenc

[Andreas Rheinhardt]

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