[FFmpeg-devel] mpegts memory leak

[Uve W. Rick]

> -----Original Message-----
> From: ffmpeg-devel-bounces at mplayerhq.hu [mailto:ffmpeg-devel-
> bounces at mplayerhq.hu] On Behalf Of Baptiste Coudurier
> Sent: Tuesday, September 28, 2010 1:53 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] mpegts memory leak
> 
> On 09/28/2010 01:42 PM, Uve W. Rick wrote:
> > While doing test, I ran across a memory leak in the mpegts.c.
> >
> > What I found was that more than one packet could be generated for
> each read.  Since only the last packet is returned, the memory for the
> other generated packets would be lost.
> >
> > It is possible if a packet has a length of 0 and the next packet has
> a very short length on the same stream.  A transport stream does not
> normally do this, but if the data is corrupt, it is possible.
> >
> > Here's a patch to fix the memory leak.  It's not very elegant, but
> the only real solution would be to put in a packet list, which did not
> seem appropriate.
> >
> > The patch keeps the functionality the same as before, where the
> previous packet is discarded.
> >
> > [...]
> >
> >
> > Index: libavformat/mpegts.c
> > ===================================================================
> > --- libavformat/mpegts.c	(revision 25229)
> > +++ libavformat/mpegts.c	(working copy)
> > @@ -650,6 +650,8 @@
> >       if(!ts->pkt)
> >           return 0;
> >
> > +    ts->pkt->data = NULL;
> > +
> >       if (is_start) {
> >           if (pes->state == MPEGTS_PAYLOAD&&  pes->data_index>  0) {
> >               new_pes_packet(pes, ts->pkt);
> > @@ -800,12 +802,15 @@
> >           case MPEGTS_PAYLOAD:
> >               if (buf_size>  0&&  pes->buffer) {
> >                   if (pes->data_index+buf_size>  pes->total_size) {
> > +                    if (!ts->pkt->data) {
> > +                        av_free(ts->pkt->data);
> > +                    }
> 
> This doesn't make sense, if ts->pkt->data is NULL there is no point
> freeing it.
> 

If you look right below where the ts->pkt->data is set to NULL, you have the new_pes_packet  this will set the ts->pkt->data.  Then in the line right after the patch, you have another new_pes_packet.  It is
possible that both instances of new_pes_packet are called.  In the current implementation, the ts->pkt->data is simply overwritten and the memory from the first call to new_pes_packet is lost.

> [...]
> 
> --
> Baptiste COUDURIER
> Key fingerprint
> 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> FFmpeg maintainer
> http://www.ffmpeg.org
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel