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

[Michael Niedermayer]

On Wed, Mar 20, 2013 at 06:59:45PM +0100, Clément Bœsch wrote:
> 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.

>  utils.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> fb01c3b004ec1c743d63c50147b68955c010b70e  0001-lavc-utils-fix-audio-frame-memleak-in-case-of-non-re.patch
> 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.

LGTM

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130321/b1862815/attachment.asc>