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

[Aurelien Jacobs]

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