GNOME Bugzilla – Bug 754212
cutter: Fix buffer leak
Last modified: 2015-10-07 18:05:05 UTC
Buffer is added to the internal cache, and pushed only when accumulated buffer duration crosses 200 ms. So when the chain ends, the buffer accumulated is not freed. Adding a finalize method to free the cache. leak with the below pipeline audiotestsrc num-buffers=200 ! cutter threshold=1 ! fakesink ==27003== 18,424 (12 direct, 18,412 indirect) bytes in 1 blocks are definitely lost in loss record 3,577 of 3,579 ==27003== at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) ==27003== by 0x43D1BE2: g_malloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0) ==27003== by 0x43E8281: g_slice_alloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0) ==27003== by 0x43C83E0: g_list_append (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0) ==27003== by 0x404817E: gst_cutter_chain (gstcutter.c:370) ==27003== by 0x41F0BC7: gst_validate_pad_monitor_chain_func (gst-validate-pad-monitor.c:2039) ==27003== by 0x427EC7D: gst_pad_push_data (gstpad.c:4085) ==27003== by 0x4874A5A: gst_base_src_loop (gstbasesrc.c:2845) ==27003== by 0x42B3138: gst_task_func (gsttask.c:331) ==27003== by 0x42B41EE: default_func (gsttaskpool.c:68) ==27003== by 0x43F3404: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0) ==27003== by 0x43F29A9: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0) ==27003== by 0x4498F6F: start_thread (pthread_create.c:312) ==27003== by 0x4599BED: clone (clone.S:129)
Created attachment 310146 [details] [review] fix buffer leak
Comment on attachment 310146 [details] [review] fix buffer leak Anything stream specific should be freed at latest in PAUSED->READY, or when the caps change, or in the case of buffers when FLUSH_STOP happens.
Created attachment 310152 [details] [review] fix buffer leak Freeing the buffer in PAUSED to READY, as flush stop is not getting called for this. Please review :)
Review of attachment 310152 [details] [review]: ::: gst/cutter/gstcutter.c @@ +253,3 @@ + { + GstBuffer *prebuf; + while (filter->pre_buffer) { Why not just g_list_free_full(filter->pre_buffer, (GFunc) gst_buffer_unref, NULL)? :) And I don't know what this element is doing or if it works at all, so please check if it makes sense to also free those buffers on caps changes or flushing ;)
Created attachment 310156 [details] [review] fix buffer leak I think, freeing here is enough. Not needed in caps change and flushing
Review of attachment 310156 [details] [review]: ::: gst/cutter/gstcutter.c @@ +251,3 @@ + switch (transition) { + case GST_STATE_CHANGE_PAUSED_TO_READY: + g_list_free_full (filter->pre_buffer, (GDestroyNotify) gst_buffer_unref); You should set the list to NULL after freeing.
Created attachment 312660 [details] [review] fix buffer leak Setting list to NULL after freeing
commit 15b08e0bd509df13cafdff13bc067aaa7c6bb2aa Author: Vineeth TM <vineeth.tm@samsung.com> Date: Mon Oct 5 13:10:56 2015 +0900 cutter: Fix buffer leak Buffer is added to the internal cache, and pushed only when accumulated buffer duration crosses 200 ms. So when the chain ends, the buffer accumulated is not freed. Freeing the cache when the state changes from PAUSED to READY. https://bugzilla.gnome.org/show_bug.cgi?id=754212