[FFmpeg-devel] [PATCH] lavc/utils: fix audio frame memleak in case of non refcounted frames.

[Clément Bœsch]

On Wed, Mar 20, 2013 at 11:25:16AM +0100, Hendrik Leppkes wrote:
> On Tue, Mar 19, 2013 at 10:49 PM, Clément Bœsch <ubitux at gmail.com> wrote:
> > This is a simple code duplication from what's in the decode video
> > function. This change notably fixes leaks with the metadata filter
> > tests.
> > ---
> >  libavcodec/utils.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index b245914..424ffab 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -2116,6 +2116,13 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
> >
> >          if (ret < 0 && frame->data[0])
> >              av_frame_unref(frame);
> > +
> > +        if (*got_frame_ptr) {
> > +            if (!avctx->refcounted_frames) {
> > +                avci->to_free = *frame;
> > +                avci->to_free.extended_data = avci->to_free.data;
> > +            }
> > +        }
> >      }
> >
> >      /* many decoders assign whole AVFrames, thus overwriting extended_data;
> > --
> > 1.8.2
> >
> 
> This piece of code is already in that function, line 2066.
> I was a bit confused, because i only converted my video to refcounted
> frames, and would've noticed such a bad leak.
> 
> The problem appears to be that the metadata is only added into the
> frame after the frame is assigned to avci->to_free in the existing
> block, and thus leaks.
> Maybe add_metadata_from_side_data should be moved into the block thats
> only run on successfull decoding, ie. a few lines above it - before
> the code that sets avci->to_free
> 

Oups. Indeed, much better. New patch attached.

-- 
Clément B.
-------------- next part --------------
From 66e99ab71536269cadfb2e73e4cbb0740db0e47b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Tue, 19 Mar 2013 22:47:49 +0100
Subject: [PATCH] lavc/utils: fix audio frame memleak in case of non refcounted
 frames.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The metadata must be set before saving the frame to avci->to_free,
otherwise it will leak.

Signed-off-by: Hendrik Leppkes <h.leppkes at gmail.com>
Signed-off-by: Clément Bœsch <ubitux at gmail.com>
---
 libavcodec/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 887f6b1..c15b772 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2051,6 +2051,7 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
         avctx->pkt = &tmp;
         ret = avctx->codec->decode(avctx, frame, got_frame_ptr, &tmp);
         if (ret >= 0 && *got_frame_ptr) {
+            add_metadata_from_side_data(avctx, frame);
             avctx->frame_number++;
             frame->pkt_dts = avpkt->dts;
             av_frame_set_best_effort_timestamp(frame,
@@ -2070,7 +2071,6 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
                 avci->to_free.extended_data = avci->to_free.data;
             }
         }
-        add_metadata_from_side_data(avctx, frame);
 
         side= av_packet_get_side_data(avctx->pkt, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
         if(side && side_size>=10) {
-- 
1.8.2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130320/d7b5648c/attachment.asc>