diff mbox series

[FFmpeg-devel,3/6] avformat/mux: Fix leak when adding packet to interleavement queue fails

Message ID 20200411211955.20843-3-andreas.rheinhardt@gmail.com
State Accepted
Commit a43120b609db300a4b3fa086d6ac753c13e6bf6d
Headers show
Series [FFmpeg-devel,1/6] avformat/mux: Make uncoded frames av_packet_unref() compatible | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 11, 2020, 9:19 p.m. UTC 
When an error happened in ff_interleave_add_packet() when adding
a packet to the packet queue, said packet would not be unreferenced
in ff_interleave_add_packet(), but would be zeroed in
av_interleaved_write_frame(), which results in a memleak.

This has been fixed: ff_interleave_add_packet() now always unreferences
the input packet on error; as a result, it always returns blank packets
which has been documented. Relying on this a call to av_packet_unref()
in ff_audio_rechunk_interleave() can be removed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
This approach is way more robust than my old "let the caller clean up"
approach. And given that uncoded frames are no longer as horrible as
they are, one doesn't need to add further checks for whether one has
an uncoded frame to free on error in ff_interleave_add_packet().

 libavformat/audiointerleave.c | 4 +---
 libavformat/internal.h        | 4 ++--
 libavformat/mux.c             | 7 +++++--
 3 files changed, 8 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/audiointerleave.c b/libavformat/audiointerleave.c
index f10c83d7c9..36a3288242 100644
--- a/libavformat/audiointerleave.c
+++ b/libavformat/audiointerleave.c
@@ -136,10 +136,8 @@  int ff_audio_rechunk_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt
         if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
             AVPacket new_pkt;
             while ((ret = interleave_new_audio_packet(s, &new_pkt, i, flush)) > 0) {
-                if ((ret = ff_interleave_add_packet(s, &new_pkt, compare_ts)) < 0) {
-                    av_packet_unref(&new_pkt);
+                if ((ret = ff_interleave_add_packet(s, &new_pkt, compare_ts)) < 0)
                     return ret;
-                }
             }
             if (ret < 0)
                 return ret;
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 332477a532..e9d7d6754a 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -230,9 +230,9 @@  char *ff_data_to_hex(char *buf, const uint8_t *src, int size, int lowercase);
 int ff_hex_to_data(uint8_t *data, const char *p);
 
 /**
- * Add packet to AVFormatContext->packet_buffer list, determining its
+ * Add packet to an AVFormatContext's packet_buffer list, determining its
  * interleaved position using compare() function argument.
- * @return 0, or < 0 on error
+ * @return 0 on success, < 0 on error. pkt will always be blank on return.
  */
 int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt,
                              int (*compare)(AVFormatContext *, const AVPacket *, const AVPacket *));
diff --git a/libavformat/mux.c b/libavformat/mux.c
index e86214d585..f61dbd3c89 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -918,10 +918,13 @@  int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt,
     int chunked  = s->max_chunk_size || s->max_chunk_duration;
 
     this_pktl    = av_malloc(sizeof(AVPacketList));
-    if (!this_pktl)
+    if (!this_pktl) {
+        av_packet_unref(pkt);
         return AVERROR(ENOMEM);
+    }
     if ((ret = av_packet_make_refcounted(pkt)) < 0) {
         av_free(this_pktl);
+        av_packet_unref(pkt);
         return ret;
     }
 
@@ -1216,7 +1219,7 @@  int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt)
             av_init_packet(pkt);
             pkt = NULL;
         }
-        if (ret <= 0) //FIXME cleanup needed for ret<0 ?
+        if (ret <= 0)
             return ret;
 
         ret = write_packet(s, &opkt);