[FFmpeg-devel] [PATCH] ffplay's primary function is not to trigger OOM killer

Aurelien Jacobs aurel
Thu Feb 19 23:42:18 CET 2009


Michael Niedermayer wrote:

> On Thu, Feb 19, 2009 at 10:00:10PM +0100, Aurelien Jacobs wrote:
> > Hi,
> > 
> > Commit r16484 introduced an infinite loop allocating memory in ffplay.
> > This is very efficient at filling memory and triggering the OOM killer.
> > The relevant part of the main loop now looks like this:
> > 
> >     for(;;) {
> >         [...]
> >         if(url_feof(ic->pb)) {
> >             av_init_packet(pkt);
> >             [...]
> >             packet_queue_put(&is->videoq, pkt);
> >             continue;
> >         }
> >         ret = av_read_frame(ic, pkt);
> >         if (ret < 0) {
> >             [...]
> >             break;
> >         }
> >         [...]
> >         packet_queue_put(&is->videoq, pkt);
> >     }
> > 
> > So when the demuxer read the last frame of the stream, the ByteIOContext
> > reaches EOF but av_read_frame() correctly return a positive value as it
> > actually read a full frame.
> > Then at next iteration of the main loop url_feof() is triggered, which
> > allocate a new packet and go back to the start of the loop, and
> > triggers url_feof() again, and so on.
> 
> try to RTFS again and harder, these kind of bug analysis and fixes is what
> leds projects to become what mplayer did ...

try to RTFS a bit more before writing such reply ;-)

> see, straight before the eof case:
> 
>         /* if the queue are full, no need to read more */
>         if (is->audioq.size > MAX_AUDIOQ_SIZE ||
>             is->videoq.size > MAX_VIDEOQ_SIZE ||
>             is->subtitleq.size > MAX_SUBTITLEQ_SIZE) {
>             /* wait 10 ms */
>             SDL_Delay(10);
>             continue;
>         }
> 
> thus the NULL packets are only inserted if the que is not full,

Right. But the definition of "queue is not full" may not be what you
would expect.

> this effectively limits the size and cannot trigger OOM (in theory ...)

No. This only limits the sum of all pkt.size. But this don't prevent
adding infinite amount of 0 sized packets. The total size of actual
data is indeed 0, but the size of the structures containing those
empty data is enough to fill memory...

Aurel




More information about the ffmpeg-devel mailing list