Fix decoder and parser memleaks #32

iekashaev · 2022-05-01T12:20:01Z

Fixed two potential memory leaks:

Decoder:
Fix free decoderContext, AVCodecContext should be freed with avcodec_free_context() not avcodec_close(). Docs about avcodec_alloc_context3.
Parser:
Fix memleak ralated with incorrect memory allocation in avformat_open_input func.

I attach some logs from valgrind:

Parser memleak:

==9512== Memcheck, a memory error detector
==9512== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9512== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==9512== Command: ./sample
==9512== Parent PID: 17
==9512== 
==9512== 
==9512== HEAP SUMMARY:
==9512==     in use at exit: 5,100 bytes in 400 blocks
==9512==   total heap usage: 201,136 allocs, 200,736 frees, 1,856,204,892 bytes allocated
==9512== 
==9512== 5,100 (1,600 direct, 3,500 indirect) bytes in 100 blocks are definitely lost in loss record 4 of 4
==9512==    at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9512==    by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9512==    by 0x6B87B82: av_malloc (in /usr/local/lib/libavutil.so.56.31.100)
==9512==    by 0x6B87D88: av_mallocz (in /usr/local/lib/libavutil.so.56.31.100)
==9512==    by 0x6B75B08: av_dict_set (in /usr/local/lib/libavutil.so.56.31.100)
==9512==    by 0x6B8C10B: av_opt_set_dict2 (in /usr/local/lib/libavutil.so.56.31.100)
==9512==    by 0x700CFDA: avformat_open_input (in /usr/local/lib/libavformat.so.58.29.100)
==9512==    by 0x11FE28: Parser::Init(ParserParameters&, std::shared_ptr<Logger>) (in /home/proj/sample)
==9512==    by 0x112AD9: main (in /home/proj/sample)
==9512== 
==9512== LEAK SUMMARY:
==9512==    definitely lost: 1,600 bytes in 100 blocks
==9512==    indirectly lost: 3,500 bytes in 300 blocks
==9512==      possibly lost: 0 bytes in 0 blocks
==9512==    still reachable: 0 bytes in 0 blocks
==9512==         suppressed: 0 bytes in 0 blocks
==9512== 
==9512== For counts of detected and suppressed errors, rerun with: -v
==9512== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Decoder memleak:

...
==11055== 115,500 (105,600 direct, 9,900 indirect) bytes in 100 blocks are definitely lost in loss record 1,220 of 1,236
==11055==    at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11055==    by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11055==    by 0x6B87B82: av_malloc (in /usr/local/lib/libavutil.so.56.31.100)
==11055==    by 0x59EDE42: avcodec_alloc_context3 (in /usr/local/lib/libavcodec.so.58.54.100)
==11055==    by 0x1194B1: Decoder::Init(DecoderParameters&, std::shared_ptr<Logger>) (in /home/proj/sample)
==11055==    by 0x112B84: main (in /home/proj/sample)
==11055== 
==11055== LEAK SUMMARY:
==11055==    definitely lost: 107,200 bytes in 200 blocks
==11055==    indirectly lost: 13,400 bytes in 400 blocks
==11055==      possibly lost: 19,624 bytes in 168 blocks
==11055==    still reachable: 9,211,914 bytes in 8,558 blocks
==11055==         suppressed: 0 bytes in 0 blocks
==11055== Reachable blocks (those to which a pointer was found) are not shown.
==11055== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==11055== 
==11055== For counts of detected and suppressed errors, rerun with: -v
==11055== ERROR SUMMARY: 113 errors from 113 contexts (suppressed: 0 from 0)

After my fixes:

...
==11861== LEAK SUMMARY:
==11861==    definitely lost: 0 bytes in 0 blocks
==11861==    indirectly lost: 0 bytes in 0 blocks
==11861==      possibly lost: 19,624 bytes in 168 blocks
==11861==    still reachable: 9,211,930 bytes in 8,558 blocks
==11861==         suppressed: 0 bytes in 0 blocks
==11861== Reachable blocks (those to which a pointer was found) are not shown.
==11861== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==11861== 
==11861== For counts of detected and suppressed errors, rerun with: -v
==11861== ERROR SUMMARY: 111 errors from 111 contexts (suppressed: 0 from 0)

Also attach the code to reproduce the problem:

Parser test:

#include "Parser.h"

int main() {
  for (int i = 0; i < 100; i++) {
    Parser parser;
    ParserParameters parserArgs = {
        "rtmp://37.228.119.44:1935/vod/big_buck_bunny.mp4"};
    parser.Init(parserArgs, std::make_shared<Logger>());

    parser.Close();
  }
  return 0;
}

Decoder test:

#include "Parser.h"
#include "Decoder.h"

int main() {
  for (int i = 0; i < 100; i++) {
    std::shared_ptr<Parser> parser = std::make_shared<Parser>();
    ParserParameters parserArgs = {
        "rtmp://37.228.119.44:1935/vod/big_buck_bunny.mp4"};
    parser->Init(parserArgs, std::make_shared<Logger>());
    
    Decoder decoder;
    DecoderParameters decoderArgs = { parser, false };
    decoder.Init(decoderArgs, std::make_shared<Logger>());

    parser->Close();
    decoder.Close();
  }
  return 0;
}

Thanks!

BykadorovR · 2022-05-04T07:04:54Z

@UstinovAlex could you please check C++ and Python tests with this fix and basic C++ samples? Thanks!

iekashaev · 2022-05-04T18:47:48Z

Fix decoder decode memleak:
When we decode a first frame, the avcodec_receive_frame func will return EAGAIN.
Important! The leak occurs only for the first frame.

I attach logs from valgrind:

...
==1244== 536 bytes in 1 blocks are definitely lost in loss record 906 of 1,241
==1244==    at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1244==    by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1244==    by 0x6B87B82: av_malloc (in /usr/local/lib/libavutil.so.56.31.100)
==1244==    by 0x6B87D88: av_mallocz (in /usr/local/lib/libavutil.so.56.31.100)
==1244==    by 0x6B7B9AA: av_frame_alloc (in /usr/local/lib/libavutil.so.56.31.100)
==1244==    by 0x120165: Decoder::Decode(AVPacket*) (in /home/tensor-stream/sample)
==1244==    by 0x1376E2: TensorStream::processingLoop() (in /home/tensor-stream/sample)
==1244==    by 0x13A6D2: TensorStream::startProcessing() (in /home/tensor-stream/sample)
==1244==    by 0x1138BF: main::{lambda()#1}::operator()() const (in /home/tensor-stream/sample)
==1244==    by 0x11424C: void std::__invoke_impl<void, main::{lambda()#1}>(std::__invoke_other, main::{lambda()#1}&&) (in /home/tensor-stream/sample)
==1244==    by 0x114068: std::__invoke_result<main::{lambda()#1}>::type std::__invoke<main::{lambda()#1}>(std::__invoke_result&&, (main::{lambda()#1}&&)...) (in /home/tensor-stream/sample)
==1244==    by 0x1144E1: decltype (__invoke((_S_declval<0ul>)())) std::thread::_Invoker<std::tuple<main::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) (in /home/tensor-stream/sample)
==1244== LEAK SUMMARY:
==1244==    definitely lost: 536 bytes in 1 blocks
==1244==    indirectly lost: 0 bytes in 0 blocks
==1244==      possibly lost: 20,072 bytes in 168 blocks
==1244==    still reachable: 9,235,226 bytes in 8,594 blocks
==1244==         suppressed: 0 bytes in 0 blocks
==1244== Reachable blocks (those to which a pointer was found) are not shown.
==1244== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==1244== 
==1244== For counts of detected and suppressed errors, rerun with: -v
==1244== Use --track-origins=yes to see where uninitialised values come from
==1244== ERROR SUMMARY: 168 errors from 117 contexts (suppressed: 0 from 0)

For reproduce the problem you can use Sample.cpp from c_examples.

After my fixes:

...
==3264== LEAK SUMMARY:
==3264==    definitely lost: 0 bytes in 0 blocks
==3264==    indirectly lost: 0 bytes in 0 blocks
==3264==      possibly lost: 20,072 bytes in 168 blocks
==3264==    still reachable: 9,235,226 bytes in 8,594 blocks
==3264==         suppressed: 0 bytes in 0 blocks
==3264== Reachable blocks (those to which a pointer was found) are not shown.
==3264== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==3264== 
==3264== For counts of detected and suppressed errors, rerun with: -v
==3264== Use --track-origins=yes to see where uninitialised values come from
==3264== ERROR SUMMARY: 167 errors from 116 contexts (suppressed: 0 from 0)

Thanks!

UstinovAlex · 2022-05-05T20:27:20Z

Decoder context & decode memleak fixes are OK.
But parser memleak fix is not very good. Context metadata is used for source stream metadata store, not for stream open parameters. I think it would be better to use av_dict_free(&opts) after avformat_open_input() call.

iekashaev · 2022-05-06T15:28:02Z

@UstinovAlex Yes, this variant looks really better. I fixed it. Thanks!

BykadorovR · 2022-05-07T05:42:42Z

src/Decoder.cpp

 		return sts;
 	}
 	AVFrame* decodedFrame = av_frame_alloc();
 	sts = avcodec_receive_frame(decoderContext, decodedFrame);

 	if (sts == AVERROR(EAGAIN) || sts == AVERROR_EOF) {
 		av_frame_free(&decodedFrame);
+		av_packet_unref(pkt);


Maybe we can move unref from 146, 150 lines to 143 line, just to avoid code duplication

@BykadorovR Fixed

iekashaev added 2 commits May 1, 2022 16:53

Fix decoder context memleak

c7370e2

Fix parser init memleak

bcfa725

Fix decoder decode memleak

b2f614b

iekashaev changed the title ~~Fix decoder and parser init memleaks~~ Fix decoder and parser memleaks May 4, 2022

Change to default avdict

0498a00

UstinovAlex approved these changes May 6, 2022

View reviewed changes

BykadorovR reviewed May 7, 2022

View reviewed changes

Delete code duplication

d55c500

BykadorovR approved these changes May 7, 2022

View reviewed changes

BykadorovR merged commit 58844a9 into osai-ai:dev May 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix decoder and parser memleaks #32

Fix decoder and parser memleaks #32

iekashaev commented May 1, 2022 •

edited

Loading

BykadorovR commented May 4, 2022

iekashaev commented May 4, 2022

UstinovAlex commented May 5, 2022 •

edited

Loading

iekashaev commented May 6, 2022 •

edited

Loading

BykadorovR May 7, 2022

iekashaev May 7, 2022

Fix decoder and parser memleaks #32

Fix decoder and parser memleaks #32

Conversation

iekashaev commented May 1, 2022 • edited Loading

BykadorovR commented May 4, 2022

iekashaev commented May 4, 2022

UstinovAlex commented May 5, 2022 • edited Loading

iekashaev commented May 6, 2022 • edited Loading

BykadorovR May 7, 2022

Choose a reason for hiding this comment

iekashaev May 7, 2022

Choose a reason for hiding this comment

iekashaev commented May 1, 2022 •

edited

Loading

UstinovAlex commented May 5, 2022 •

edited

Loading

iekashaev commented May 6, 2022 •

edited

Loading