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

[Andreas Rheinhardt]

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 at 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 --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);
-- 
2.20.1