Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#7630 closed defect (invalid)

Memory leak in avformat_free_context

Reported by: Ace17 Owned by:
Priority: normal Component: avformat
Version: unspecified Keywords: leak
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

#include <cassert>

extern "C" {
#include <libavformat/avformat.h> // AVOutputFormat
#include <libavformat/avio.h> // avio_open2
}

auto const FILENAME = "hello.mp4";

AVFormatContext* g_avformatCtx;

void CreateAvContext() {
  g_avformatCtx = avformat_alloc_context();
  assert(g_avformatCtx);

  g_avformatCtx->oformat = av_guess_format("mpegts", nullptr, nullptr);
  assert(g_avformatCtx->oformat);
  assert(!(g_avformatCtx->oformat->flags & AVFMT_NOFILE));

  int ret = avio_open2(&g_avformatCtx->pb, FILENAME, AVIO_FLAG_WRITE, nullptr, nullptr);
  assert(ret >= 0);

  g_avformatCtx->url = av_strdup(FILENAME);
}

void DestroyAvContext() {
  //av_write_trailer(g_avformatCtx);
  avio_close(g_avformatCtx->pb);
  avformat_free_context(g_avformatCtx);
}

void AddStream() {
  auto const codec = avcodec_find_decoder_by_name("mpeg2video");
  assert(codec);

  auto stream = avformat_new_stream(g_avformatCtx, codec);
  assert(stream);

  auto codecpar = stream->codecpar;
  codecpar->codec_id = codec->id;
  codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
  codecpar->width = 128;
  codecpar->height = 128;
}

int main()
{
  CreateAvContext();
  AddStream();

  int ret = avformat_write_header(g_avformatCtx, nullptr);
  assert(ret == 0);

  DestroyAvContext();
  return 0;
}

How to reproduce:

g++ -g3 repro_libavformat_mux_leak.cpp `pkg-config libavformat libavcodec libavutil --cflags --libs`
valgrind --leak-check=full ./a.out

==7541== 2,930 bytes in 1 blocks are definitely lost in loss record 243 of 245
==7541==    at 0x4837EC3: memalign (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7541==    by 0x4837FF0: posix_memalign (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7541==    by 0x5FFC28A: av_malloc (in /usr/lib/x86_64-linux-gnu/libavutil.so.56.14.100)
==7541==    by 0x5FFC61C: av_mallocz (in /usr/lib/x86_64-linux-gnu/libavutil.so.56.14.100)
==7541==    by 0x498EE76: ??? (in /usr/lib/x86_64-linux-gnu/libavformat.so.58.12.100)
==7541==    by 0x4996F98: avformat_init_output (in /usr/lib/x86_64-linux-gnu/libavformat.so.58.12.100)
==7541==    by 0x4997754: avformat_write_header (in /usr/lib/x86_64-linux-gnu/libavformat.so.58.12.100)
==7541==    by 0x109409: main (repro_libavformat_mux_leak.cpp:54)
==7541== 

Calling av_write_trailer makes the leak disappear. However, it's unconvenient in some situation where you want to quickly cancel the encoding/muxing, and discard the output file.

Change History (4)

comment:1 Changed 3 months ago by Ace17

  • Summary changed from Memory leak in avformat_ to Memory leak in avformat_free_context

comment:2 Changed 3 months ago by cehoyos

  • Component changed from undetermined to avformat
  • Keywords valgrind avformat_init_output av_write_trailer removed
  • Resolution set to invalid
  • Status changed from new to closed

avformat_write_header() will obviously allocate resources at least for some formats: These can only be freed by calling av_write_trailer(), how else could this work?

If you really want to suggest such an API change, please write a patch and send it to the development mailing list.

comment:3 Changed 3 months ago by Ace17

Dear cehoyos, thanks for your comments.

avformat_write_header() will obviously allocate resources at least for some formats:

I agree it's obvious that this function might allocate.
However it's non-obvious that it might require the caller to do an explicit deallocation step (av_write_trailer).
Especially with file formats that don't have a trailer (e.g mpegts).

I'm sincere ; tracking down this leak took me three days!

These can only be freed by calling av_write_trailer(), how else could this work?

I think it could work in the following way: avformat_free_context does the deallocation itself, if needed.
This way, the avformat code has to keep track of whether something owned (and only visible) by the AVFormatContext needs to be deallocated (at the moment, the caller code has to do this).

Does it make sense?

Please don't dismiss this use case: there really are situations where you want to cleanly destroy the app without keeping the output file (and, more importantly, without writing more stuff to the filesystem, which might be on a dying network). At the moment, the only way to do this is to let avformat_free_context leak memory.

comment:4 Changed 3 months ago by cehoyos

As said, feel free to send a patch, this is not a good place to discuss api changes.

Note: See TracTickets for help on using tickets.