Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OutputContainer.__del__ is never called. #427

Closed
mikeboers opened this issue Oct 12, 2018 · 6 comments
Closed

OutputContainer.__del__ is never called. #427

mikeboers opened this issue Oct 12, 2018 · 6 comments

Comments

@mikeboers
Copy link
Member

@jlaine pointed out that OutputContainer.__del__ is never called, and should be __dealloc__.

Maybe related to #370 and my own problems with my security cameras (in which I'm leaking ~120B of memory on every output file I make (once a minute, which builds up after a few days)).

@jlaine
Copy link
Collaborator

jlaine commented Oct 12, 2018

I think #370 is a different problem as it applies to input containers.

@mikeboers
Copy link
Member Author

As hinted at in chat, my problem may be with CodecContext's not getting dealloced because we don't know how that works. I'm not too concerned with this in any immediate sense for my own stuff, I just restart my camera process every two weeks or so.

@jlaine
Copy link
Collaborator

jlaine commented Oct 12, 2018

As far as I can tell, the AVStream.codec should get deallocated when we call avformat_free_context:

  • for each stream it calls ff_free_stream

https://github.com/FFmpeg/FFmpeg/blob/ef23ed6fe96e4ec8d11c82f8be19a2877f9737ee/libavformat/utils.c#L4402

  • which calls free_stream

https://github.com/FFmpeg/FFmpeg/blob/ef23ed6fe96e4ec8d11c82f8be19a2877f9737ee/libavformat/utils.c#L4385

  • which calls avcodec_free_context

https://github.com/FFmpeg/FFmpeg/blob/ef23ed6fe96e4ec8d11c82f8be19a2877f9737ee/libavformat/utils.c#L4364

@jlaine
Copy link
Collaborator

jlaine commented Oct 12, 2018

After some digging around, this simple program triggers a leak according to valgrind:

import av
  
container = av.open('foo.mp4', 'w')
stream = container.add_stream('mpeg4')
container.close()

The output looks like:

==31210== 10,760 bytes in 1 blocks are definitely lost in loss record 525 of 538
==31210==    at 0x4837B26: memalign (vg_replace_malloc.c:857)
==31210==    by 0x4837C41: posix_memalign (vg_replace_malloc.c:1020)
==31210==    by 0x6BC728A: av_malloc (in /usr/lib/x86_64-linux-gnu/libavutil.so.56.14.100)
==31210==    by 0x6BC761C: av_mallocz (in /usr/lib/x86_64-linux-gnu/libavutil.so.56.14.100)
==31210==    by 0x5B8D0DD: ??? (in /usr/lib/x86_64-linux-gnu/libavcodec.so.58.18.100)
==31210==    by 0x5B8D1F5: avcodec_alloc_context3 (in /usr/lib/x86_64-linux-gnu/libavcodec.so.58.18.100)
==31210==    by 0x6DED735: avformat_new_stream (in /usr/lib/x86_64-linux-gnu/libavformat.so.58.12.100)
==31210==    by 0x1C1EBDBE: __pyx_pf_2av_9container_6output_15OutputContainer_4add_stream (output.c:2555)
==31210==    by 0x1C1EB435: __pyx_pw_2av_9container_6output_15OutputContainer_5add_stream (output.c:2280)
==31210==    by 0x50C3E4: UnknownInlinedFun (methodobject.c:231)
==31210==    by 0x50C3E4: UnknownInlinedFun (methodobject.c:294)
==31210==    by 0x50C3E4: call_function.lto_priv.1801 (ceval.c:4830)
==31210==    by 0x50FAD8: _PyEval_EvalFrameDefault (ceval.c:3328)
==31210==    by 0x50DEE6: UnknownInlinedFun (ceval.c:754)
==31210==    by 0x50DEE6: _PyEval_EvalCodeWithName.lto_priv.1766 (ceval.c:4159)

@jlaine
Copy link
Collaborator

jlaine commented Oct 12, 2018

.. and I believe I've found the source of the leak! This is the line which is causing us grief:

lib.avcodec_get_context_defaults3(stream.codec, codec)

We should not call this method, since when avcodec_alloc_context3 already does it for us (at least since 3.2, it may not always have been so):

https://www.ffmpeg.org/doxygen/3.2/libavcodec_2options_8c_source.html#l00156
https://www.ffmpeg.org/doxygen/4.0/libavcodec_2options_8c_source.html#l00156

The same applies to CodecContext.create.

jlaine added a commit that referenced this issue Oct 12, 2018
We never need to call avcodec_get_context_defaults3 ourselves,
calling avcodec_alloc_context3 already takes care of it. If we do call
it ourselves again, we get a memory leak since it malloc's some memory
for private structures.

See discussion in issue #427.
@jlaine jlaine closed this as completed in 965e9f1 Oct 12, 2018
jlaine added a commit that referenced this issue Oct 13, 2018
Cython classes use __dealloc__ and not __del__ for their destructors.
@mikeboers
Copy link
Member Author

Wonderful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants