[FFmpeg-cvslog] vsrc_buffer: remove dependency on AVFrame

[Stefano Sabatini]

On date Thursday 2011-06-02 01:27:01 +0200, Etienne Buira wrote:
> On Thu, Jun 02, 2011 at 01:07:18AM +0200, Stefano Sabatini wrote:
> > On date Wednesday 2011-06-01 19:53:48 +0200, Etienne Buira wrote:
> > > On Thu, May 19, 2011 at 11:33:08PM +0200, Stefano Sabatini wrote:
> > > > ffmpeg | branch: master | Stefano Sabatini <stefano.sabatini-lala at poste.it> | Sat May  7 21:35:08 2011 +0200| [6070b7e1c520e9ca389403bae20a2ad04c7d54c7] | committer: Stefano Sabatini
> > > > 
> > > > vsrc_buffer: remove dependency on AVFrame
> > > > 
> > > > Rename av_vsrc_buffer_add_frame to
> > > > av_vsrc_buffer_add_video_buffer_ref(), and change its inteface to make
> > > > it accept in input an AVFilterBufferRef rather than an AVFrame.
> > > > 
> > > > This way the interface can be used without requiring the
> > > > inclusion/installation of libavcodec headers.
> > > > 
> > > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=6070b7e1c520e9ca389403bae20a2ad04c7d54c7
> > > 
> > > Hi all.
> > > 
> > > This commit seems to introduce/exhibit a serious memory leak that still
> > > shows up in current master. Seems that frames that are decoded, but not
> > > encoded (-ss) are not freed.
> > 
> > Have you evidence that there is currently a leak (I mean e.g. by using
> 
> Hit C-c before OOM killer got into the place. Usually my system only
> swaps very old pages, here it saturated io.
> 
> > valgrind)? Also can you confirm that the leak if any was introduced by
> > this commit and was not already present before?
> 
> git bisect, and checked that 6070b7e1c520e9ca389403bae20a2ad04c7d54c7^
> does not show this behaviour.
> Did you try my command (with -ss placed like I did holding a big enough
> offset, eventually same filters too)?

I confirm there is a leak.

The leak is occurring in av_vsrc_buffer_add_video_buffer_ref(), the
buffer is already filled but the new picref is added to the cache
overwriting the old picref (which is never released).

Options:

1) make av_vsrc_buffer_add_video_buffer_ref() abort if the buffer is
   already filled.
   This has a problem though, since we *want* to store the last added
   frame.

2) Extend the API of vsrc_buffer. In particular we may add this:

   // it is already filled, reset it
   if (avfilter_poll_frame(vsrc_buffer->inputs[0] > 0)
      av_vsrc_buffer_reset(vsrc_buffer); // clean the buffer
   av_vsrc_buffer_add_frame(vsrc_buffer, &picture);

   Alternatively we may add a mode for *always* overwriting the last
   frame (during initialization or with a flag in
   av_vsrc_buffer_add_frame()), like in:

   av_vsrc_buffer_add_frame(vsrc_buffer, &picture, AV_VSRC_BUF_FLAG_OVERWRITE);

Comments are welcome.