[FFmpeg-devel] [PATCH] Fix for rc_eq memory leak

[Stefano Sabatini]

On date Thursday 2008-07-31 11:21:45 -0700, Art Clarke encoded:
> Stefano submitted a fix for the rc_eq setting bug earlier this month:
> http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-July/049805.html
> 
> As he pointed out, this introduced a memory leak on AVFormatContext:rc_eq.
> 
> This patch fixes one cause of leaking this in the libav* libraries (for well
> behaved applications): closing an input stream.
> 
> There isn't an easy fix for programs that allocate their own AVFormatContext
> objects (e.g. for output).  Folks will have to fix their sources for that.
> 
> Tests with both ffmpeg and vlideshow pass @ 100%, and leak is visible before
> fix and not after fix.
> 
> - Art

> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 14487)
> +++ libavformat/utils.c	(working copy)
> @@ -2212,6 +2212,7 @@
>          }
>          av_free(st->index_entries);
>          av_free(st->codec->extradata);
> +        av_free(st->codec->rc_eq);
>          av_free(st->codec);
>          av_free(st->filename);
>          av_free(st->priv_data);

No this shouldn't be necessary anymore since r14260, unless you still
didn't already close all the codecs with avcodec_close(), which I
think is *required* by the current API.

(BTW currently avcodec_close() doesn't free extradata, I think this is a
memleak, also it could be a good idea to use av_freep() rather than
av_free() to save some SIGSEGV.)

For example ffmpeg.c does exactly this, it closes all the decoders
before to call av_close_input_stream().

Maybe a better solution would be to do call avcodec_close() in
av_close_input_stream(), at least conditionally, for example:

   if(st->codec->codec)  /* already closed or never opened */
        avcodec_close(st->codec);

This looks like a better solution because it would simplify ffmpeg.c
(and others applications as well).

In the case of ffmpeg.c this could eliminate the need for the loop:
    for(i=0;i<nb_istreams;i++) {
        ist = ist_table[i];
        if (ist->decoding_needed) {
            avcodec_close(ist->st->codec);
        }
    }

at the end of av_encode().

Opinions?

Regards.
-- 
FFmpeg = Fostering and Fiendish Mind-dumbing Pitiful Everlasting Gymnast