[FFmpeg-devel] Fwd: Leaking memory when decoding H264 streams + proposed patch fix to the problem
Monica Morogan
dmonica
Thu Apr 22 23:29:43 CEST 2010
Hi guys,
I am currently using libavcodec/libavformat libraries from the latest
ffmpeg-0.5.1 for
decoding h264 streams (1080p). A short background information, I have
a program that
spawns a thread every time a request comes in to decode a h264 stream.
It was leaking a lot of memory, and since I needed a quick fix I started to
debug the problem myself. Since no tool could tell me what the problem
was exactly,
I wrote some wrapper functions (for all the function in
libavutil/mem.c) that pretty much
added to a list pointers to all the memory allocated (with av_malloc
and av_realloc) along with the
file name and line number, and removed them once av_free/av_freep was called.
It helped me identify that one H264Context object was leaking. I have
not included
these wrappers in the patch below, but if you think that it might help
you I will be glad
to provide them.
Here is what I found:
1) in mem.c (libavutil), the av_realloc function leaks memory if
realloc in unsuccessful
2) in h264_parser.c, the field priv-data of the object
AVCodecParserContext was not
deleted (and this is the H264Context object that was actually leaking)
3) on the decoding part, in h264.c file, the free_tables function
which cleans up
H264Context object was missing some fields that were allocated previously.
Running valgrind I can see that everything looks good, no memory leaks.
Please find my patch below and let me know your opinion.
Thank you very much for your time,
Monica
diff -urp /media0/monicaff/ffmpeg-0.5.1/libavcodec/h264.c ./libavcodec/h264.c
--- /media0/monicaff/ffmpeg-0.5.1/libavcodec/h264.c 2010-02-09
11:02:39.000000000 -0800
+++ ./libavcodec/h264.c 2010-04-06 13:51:26.960044907 -0700
@@ -1412,7 +1412,7 @@ const uint8_t *ff_h264_decode_nal(H264Co
bufidx = h->nal_unit_type == NAL_DPC ? 1 : 0; // use second
escape buffer for inter data
h->rbsp_buffer[bufidx]= av_fast_realloc(h->rbsp_buffer[bufidx],
&h->rbsp_buffer_size[bufidx], length+FF_INPUT_BUFFER_PADDING_SIZE);
- dst= h->rbsp_buffer[bufidx];
+ dst = h->rbsp_buffer[bufidx];
if (dst == NULL){
return NULL;
@@ -1973,28 +1973,51 @@ static av_cold void decode_init_vlc(void
}
}
-static void free_tables(H264Context *h){
- int i;
- H264Context *hx;
- av_freep(&h->intra4x4_pred_mode);
+void static free_tables(H264Context* h) {
+
+ av_freep(&h->intra4x4_pred_mode);
+ av_freep(&h->non_zero_count);
av_freep(&h->chroma_pred_mode_table);
av_freep(&h->cbp_table);
av_freep(&h->mvd_table[0]);
av_freep(&h->mvd_table[1]);
av_freep(&h->direct_table);
- av_freep(&h->non_zero_count);
av_freep(&h->slice_table_base);
- h->slice_table= NULL;
+ av_freep(&h->top_borders[1]);
+ av_freep(&h->top_borders[0]);
+ if (h->rbsp_buffer_size[1]) {
+ av_freep(&h->rbsp_buffer[1]);
+ h->rbsp_buffer_size[1] = 0;
+ }
+ if (h->rbsp_buffer_size[0]) {
+ av_freep(&h->rbsp_buffer[0]);
+ h->rbsp_buffer_size[0] = 0;
+ }
av_freep(&h->mb2b_xy);
av_freep(&h->mb2b8_xy);
- for(i = 0; i < h->s.avctx->thread_count; i++) {
+ for(unsigned int j = 0; j < MAX_SPS_COUNT; j++) {
+ if (h->sps_buffers[j])
+ av_freep(&h->sps_buffers[j]);
+ }
+ for(unsigned j = 0; j < MAX_PPS_COUNT; j++) {
+ if (h->pps_buffers[j])
+ av_freep(&h->pps_buffers[j]);
+ }
+}
+
+void delete_H264Context(H264Context *h){
+ int i;
+ H264Context *hx;
+
+ free_tables(h);
+ for(unsigned int i = 0; i < h->s.avctx->thread_count; i++) {
hx = h->thread_context[i];
if(!hx) continue;
- av_freep(&hx->top_borders[1]);
- av_freep(&hx->top_borders[0]);
+ free_tables(hx);
av_freep(&hx->s.obmc_scratchpad);
+ if (i) av_free(h->thread_context[i]);
}
}
@@ -7259,6 +7282,7 @@ int ff_h264_decode_picture_parameter_set
if(pps == NULL)
return -1;
pps->sps_id= get_ue_golomb_31(&s->gb);
+
if((unsigned)pps->sps_id>=MAX_SPS_COUNT ||
h->sps_buffers[pps->sps_id] == NULL){
av_log(h->s.avctx, AV_LOG_ERROR, "sps_id out of range\n");
goto fail;
@@ -8094,20 +8118,10 @@ static av_cold int decode_end(AVCodecCon
MpegEncContext *s = &h->s;
int i;
- av_freep(&h->rbsp_buffer[0]);
- av_freep(&h->rbsp_buffer[1]);
- free_tables(h); //FIXME cleanup init stuff perhaps
-
- for(i = 0; i < MAX_SPS_COUNT; i++)
- av_freep(h->sps_buffers + i);
-
- for(i = 0; i < MAX_PPS_COUNT; i++)
- av_freep(h->pps_buffers + i);
+ delete_H264Context(h);
MPV_common_end(s);
-// memset(h, 0, sizeof(H264Context));
-
return 0;
}
diff -urp /media0/monicaff/ffmpeg-0.5.1/libavcodec/h264.h ./libavcodec/h264.h
--- /media0/monicaff/ffmpeg-0.5.1/libavcodec/h264.h 2009-02-28
00:38:33.000000000 -0800
+++ ./libavcodec/h264.h 2010-04-06 13:51:27.096046490 -0700
@@ -547,6 +547,8 @@ int ff_h264_decode_seq_parameter_set(H26
*/
int ff_h264_decode_picture_parameter_set(H264Context *h, int bit_length);
+void delete_H264Context(H264Context* context);
+
/**
* Decodes a network abstraction layer unit.
* @param consumed is the number of bytes used as input
diff -urp /media0/monicaff/ffmpeg-0.5.1/libavcodec/h264_parser.c
./libavcodec/h264_parser.c
--- /media0/monicaff/ffmpeg-0.5.1/libavcodec/h264_parser.c 2009-02-26
12:36:47.000000000 -0800
+++ ./libavcodec/h264_parser.c 2010-04-06 13:51:27.212047840 -0700
@@ -302,13 +302,13 @@ static int h264_split(AVCodecContext *av
static void close(AVCodecParserContext *s)
{
+
H264Context *h = s->priv_data;
ParseContext *pc = &h->s.parse_context;
-
+ delete_H264Context(h);
av_free(pc->buffer);
}
-
AVCodecParser h264_parser = {
{ CODEC_ID_H264 },
sizeof(H264Context),
diff -urp /media0/monicaff/ffmpeg-0.5.1/libavutil/mem.c ./libavutil/mem.c
--- /media0/monicaff/ffmpeg-0.5.1/libavutil/mem.c 2009-02-21
12:38:27.000000000 -0800
+++ ./libavutil/mem.c 2010-04-06 21:19:22.859168945 -0700
@@ -33,6 +33,7 @@
#include <malloc.h>
#endif
+
#include "mem.h"
/* here we can use OS-dependent allocation functions */
@@ -57,7 +58,7 @@ void *av_malloc(unsigned int size)
#if CONFIG_MEMALIGN_HACK
ptr = malloc(size+16);
- if(!ptr)
+ if(!ptr)
return ptr;
diff= ((-(long)ptr - 1)&15) + 1;
ptr = (char*)ptr + diff;
@@ -115,7 +116,12 @@ void *av_realloc(void *ptr, unsigned int
diff= ((char*)ptr)[-1];
return (char*)realloc((char*)ptr - diff, size + diff) + diff;
#else
- return realloc(ptr, size);
+ void* saveptr = ptr;
+ void* retptr = realloc(ptr, size);
+ if (retptr == NULL) {
+ av_free(saveptr);
+ }
+ return retptr;
#endif
}
More information about the ffmpeg-devel
mailing list