[Ffmpeg-devel] H.264 errors/leaks found with valgrind

[Haakon Riiser]

While running valgrind on a program that uses ffmpeg, I've discovered
some minor bugs in ffmpeg's h.264 decoder.  The (potentially) most serious
one is in golomb.h:  Valgrind claims that buf is used uninitialized in
get_ue_golomb().  Specifically, in this line:

  if(buf >= (1<<27)){

Because buf is (supposed to be) initialized through a series of macros,
it was a bit of a hassle to investigate it further.  It can easily be
reproduced using any h.264 stream and ffplay.  I've made available
a tiny h.264 clip on http://folk.uio.no/hakonrk/tmp/h264.avi which is
used in the following examples:

$ valgrind ffplay h264.avi
[...]
==5552== Conditional jump or move depends on uninitialised value(s)
==5552==    at 0x42BB4FB: get_ue_golomb (golomb.h:54)
[...]


There is also a memory leak:

$ valgrind --leak-check=yes ffplay h264.avi
==5625== 9,768 bytes in 1 blocks are definitely lost in loss record 3 of 4
==5625==    at 0x401DBF8: realloc (vg_replace_malloc.c:306)
==5625==    by 0x4875B1F: av_realloc (mem.c:120)
==5625==    by 0x41C92D4: av_fast_realloc (utils.c:72)
==5625==    by 0x430017D: ff_combine_frame (parser.c:245)
==5625==    by 0x43B3FFC: h264_parse (h264.c:8025)

The interesting lines are probably parser.c:245

  pc->buffer= av_fast_realloc(pc->buffer, &pc->buffer_size, (*buf_size)
               + pc->index + FF_INPUT_BUFFER_PADDING_SIZE);

and h264.c:8025

  if (ff_combine_frame(pc, next, (uint8_t **)&buf, &buf_size) < 0) {

I don't know whose responsibility it is to free the memory
allocated this way, but I assume it's not the application
developer's responsibility.


There's also one more leak, but it cannot be reproduced with ffplay,
only with my own application:

==5625== 10,041 bytes in 22 blocks are definitely lost in loss record 4 of 4
==5625==    at 0x401DCC6: memalign (vg_replace_malloc.c:332)
==5625==    by 0x4875AE8: av_malloc (mem.c:66)
==5625==    by 0x4115B6C: av_dup_packet (utils.c:247)
==5625==    by 0x41195A0: av_find_stream_info (utils.c:1905)

What I do is basically

  1. av_register_all();
  2. av_open_input_file(&fcx, filename, 0, 0, 0);
  3. av_find_stream_info(fcx);
  4. <read packets and decode audio/video>
  5. av_close_input_file(fcx);

If I add

  av_close_input_file(fcx);
  av_open_input_file(&fcx, filename, 0, 0, 0);

between steps 3 and 4, the leak disappears.  Is there a better
way to free memory allocated by av_find_stream_info()?

-- 
 Haakon