mbox

[FFmpeg-devel,00/10] libavformat/mux patches

Message ID 20200331123745.6461-1-andreas.rheinhardt@gmail.com
Headers show

Message

Andreas Rheinhardt March 31, 2020, 12:37 p.m. UTC 
This patchset is a slightly revised version of the remaining patches of
a patchset I originally sent last August [1]. I resend it because Marton
requested it [2]. I have also added three new patches at the end (but
they actually don't depend on any of the earlier patches).

This patchset intends to address two issues with the same root cause:
After trying to put a packet in the interleavement queue (during
av_interleaved_write_frame()), the packet is immediately zeroed and
reinitialized.

The first issue stemming from this is a memleak, because upon error the
packet to be zeroed must be unreferenced first.

The second issue is that upon success, both zeroing as well as
reinitializing are usually redundant, because the packet is already
blank (when no custom interleavement function is used).

There is unfortunately an obstacle that complicates fixing the memleak:
Uncoded frames. Using av_packet_unref() on a packet containing an
uncoded frame leaks the frame. So one needs to check first whether it is
an uncoded frame and therefore I have tried to concentrate the
unreferencing code (with the checks) to as few places as possible by
creating a contract for interleavement functions.

Notice that there would be another way to handle this: Make the packets
containing the uncoded frames reference-counted by creating a special
AVBuffer with a custom free function. Then av_packet_unref() could be
used to unreference this packet. But this would come at the cost of two
more allocations per frame. (The write_uncoded_frame-functions currently
have the right to keep the underlying AVFrame (it is transferred as
an AVFrame **); this would have to changed -- the best a muxer could do
to keep a frame would be to use av_frame_move_ref() to transfer it to
another AVFrame. Currently no muxer with write_uncoded_frame support makes
use of this right.)

Finally, there is one potential memleak that I only noticed recently and
that is not fixed in this patchset: If the muxer is closed without writing
the header, uncoded frames in the muxing queue would leak, because
ff_packet_list_free() is not aware of uncoded frames. Making the uncoded
frames reference-counted would automatically fix this. The alternative
would be to infect another place (besides libavformat/mux.c) with uncoded
frames.


- Andreas


[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-August/248140.html
[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/259218.html


Andreas Rheinhardt (10):
  avformat/mux: Only prepare input packet if there is a packet
  avformat/audiointerleave: Fix memleak
  avformat/avformat.h: Correct some comments
  avformat/mux: Fix leaks of uncoded frames on errors
  avformat/mux: Fix memleaks on errors II, improve documentation
  avformat/mux: Don't modify packets we don't own
  avformat/mux: Remove pointless timestamp backups
  avformat/avformat: Clarify documentation of
    av_interleaved_write_frame()
  fftools/ffmpeg, doc/examples/remuxing: Remove redundant
    av_packet_unref
  libavformat/mux, mxfenc: Don't initialize unnecessarily

 doc/APIchanges                |   4 ++
 doc/examples/remuxing.c       |   1 -
 fftools/ffmpeg.c              |   1 -
 libavformat/audiointerleave.c |   7 +-
 libavformat/audiointerleave.h |   4 ++
 libavformat/avformat.h        |  25 ++++---
 libavformat/internal.h        |  20 +++---
 libavformat/mux.c             | 118 +++++++++++++++++++---------------
 libavformat/mxfenc.c          |   1 -
 9 files changed, 101 insertions(+), 80 deletions(-)