memory leak in calling avcodec_alloc_context3 and then avcodec_copy_context

[vinxxe]

Summary of the bug:

in the documentation of the avcodec_copy_context function is clearly stated that the dest target codec context should be initialized with avcodec_alloc_context3.
If you perform this pair of calls, one after the other, the avcodec_alloc_context3 will allocate some memory pointed by the priv_data field of the codec context (file libavcodec/options.c, line 128) through the call of the avcodec_get_context_defaults3 function, but the following call to the avcodec_copy_context (with dest parameter set to the newly allocated codec context) will overwrite the priv_data pointer without deallocating the memory reserved by the avcodec_get_context_defaults3, so causing a memory leak.

How to reproduce:

   AVCodec  *codec;
   AVCodecContext    *SourceCodecCtx, *DestCodecCtx;

   ...

   DestCodecCtx = avcodec_alloc_context3(codec);
   avcodec_copy_context(DestCodecCtx, SourceCodecCtx);

[Carl Eugen Hoyos]

Please provide source code that allows to reproduce the leak together with valgrind output.

[vinxxe]

Replying to cehoyos:

Please provide source code that allows to reproduce the leak together with valgrind output.

here's the code and a movie file to reproduce the leak. to compile I used this command

gcc -g -ggdb -o leak main.c -lpthread -lm -lz -lbz2 -lswscale -lavdevice -lavformat -lavcodec -lavutil -lmp3lame -lx264 -lvpx -lvorbis -lvorbisenc -lfaac -lva -ltcmalloc

/*#######################################################*/
#include <libavcodec/avcodec.h>
#include <libavformat/avformat.h>
#include <libavutil/log.h>

int main(int argc, char **argv){
  AVFormatContext *formatCtx=NULL;
  AVCodec         *codec=NULL;
  AVCodecContext  *SourceCodecCtx=NULL;
  AVCodecContext  *DestCodecCtx=NULL;
  int i=0;

  av_register_all();
  av_log_set_level(100);

  avformat_open_input(&formatCtx, "Loading.mp4", NULL, NULL);
  SourceCodecCtx=formatCtx->streams[0]->codec;
  codec=avcodec_find_decoder(SourceCodecCtx->codec_id);

  for (i=0;i<10;i++){
    DestCodecCtx = avcodec_alloc_context3(codec);
    avcodec_copy_context(DestCodecCtx,SourceCodecCtx);
    av_free(DestCodecCtx);
  }
  
  av_free(SourceCodecCtx);
  avformat_close_input(&formatCtx);

}

/*#######################################################*/

Unfortunately valgrind fails to detect the leak (perhaps due to the use of tcmalloc ?) but the heap profiler of the tcmalloc lib succeeds. here's both the valgrind output and the tcmalloc heap profiler output.

TCMALLOC HEAP PROFILER OUTPUT :

[mov,mp4,m4a,3gp,3g2,mj2 @ 0x2b50000] Format mov,mp4,m4a,3gp,3g2,mj2 probed with size=2048 and score=100
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x2b50000] ISO: File Type Major Brand: mp42
[AVIOContext @ 0x2b68000] Statistics: 38663 bytes read, 1 seeks
Leak check _main_ detected leaks of 4230690 bytes in 20 objects
The 2 largest leaks:
Using local file ./leak.
Leak of 4230080 bytes in 10 objects allocated from:
	@ af5d8a av_mallocz
Leak of 610 bytes in 10 objects allocated from:
	@ af5cf5 av_malloc


If the preceding stack traces are not enough to find the leaks, try running THIS shell command:

pprof ./leak "/tmp/leak.19693._main_-end.heap" --inuse_objects --lines --heapcheck  --edgefraction=1e-10 --nodefraction=1e-10 --gv

If you are still puzzled about why the leaks are there, try rerunning this program with HEAP_CHECK_TEST_POINTER_ALIGNMENT=1 and/or with HEAP_CHECK_MAX_POINTER_OFFSET=-1
If the leak report occurs in a small fraction of runs, try running with TCMALLOC_MAX_FREE_QUEUE_SIZE of few hundred MB or with TCMALLOC_RECLAIM_MEMORY=false, it might help find leaks more repeatably
Exiting with error code (instead of crashing) because of whole-program memory leaks

VALGRIND OUTPUT :

==19649== Memcheck, a memory error detector
==19649== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==19649== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==19649== Command: ./leak
==19649== 
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x4348000] Format mov,mp4,m4a,3gp,3g2,mj2 probed with size=2048 and score=100
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x4348000] ISO: File Type Major Brand: mp42
[AVIOContext @ 0x4360000] Statistics: 38663 bytes read, 1 seeks
==19649== 
==19649== HEAP SUMMARY:
==19649==     in use at exit: 0 bytes in 0 blocks
==19649==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==19649== 
==19649== All heap blocks were freed -- no leaks are possible
==19649== 
==19649== For counts of detected and suppressed errors, rerun with: -v
==19649== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)

[Cigaes]

Is there a reason you are using av_free instead of avcodec_close?

[vinxxe]

Replying to Cigaes:

Is there a reason you are using av_free instead of avcodec_close?

I did not open the codec context so there's no reason to close it, I presume. By the way things get unchanged even if you close the context or you open and close it. the leak is in between the two calls

    DestCodecCtx = avcodec_alloc_context3(codec);
    avcodec_copy_context(DestCodecCtx,SourceCodecCtx);

and closing and freeing the DestCodecCtx will not avoid it, because in the first call some memory is allocated for
DestCodecCtx->priv_data and in the second call the pointer is set to NULL without freeing the memory previously allocated.

[Michael Niedermayer]

Fixed in cba9a40d47aefc6853ca6bb8d72096079baac50c

Note, the example code is buggy and is missing needed cleanup like avcodec_close()

[vinxxe]

Just to make things clear, buggy is your brain and I did already explain that avcodec_close does not make any difference.

[Thilo Borgmann]

Replying to vinxxe:

...buggy is your brain...

Forgot your manners?

The bug reported is fixed in cba9a40d47aefc6853ca6bb8d72096079baac50c, see the log yourself.

The missing avcodec_close() is for sure just mentioned for the case anybody uses this snipped as a code example to work on...

[vinxxe]

Yes, I forgot my manners.
Apologize me, but as I did already explain you should not close a codec never opened. Moreover, in submitting a bug report, you should write precisely the correct code exploiting the bug, nothing more nothing less.
If the code is buggy you should explain why.
I've spent time in writing the snippet, in testing the code with tcmalloc and valgrind and this is only fair in an open source community, because I use the great work someone did. What is not fair, at least in my opinion, is to treat someone who just wants to thank you for the great work you've done as a pain in the ass.
This has been my feeling.
If I misunderstood I apologize twice.

[Thilo Borgmann]

You've made a valueable contribution in reporting this bug.
Even more to such detailed information and doing well in finding the memleak.

However, you've just taken the follow-ups too personal.
The reply from Cigaes has nothing to do with the bug itself and I consider it just bikeshedding.
Nevertheless you treated that statement like a proposal to fix your bug (but you should have already known better at that time).

Miachael just fixed your bug. His note is more a hint that even this example might have been implemented cleaner (avcodec_close() is mentioned as an example there) - most likely, as I said, not that your example will be used by others as-is.

Please continue contributing!

[vinxxe]

No prob. I surely will continue to contribute. I've just forgot my manners and apologized. But this has nothing to do with the appreciation for your work. Sometimes just loose my temper.