GNOME Bugzilla – Bug 733545
GL element leaks FBO when caps changed dynamically
Last modified: 2014-07-24 01:52:45 UTC
Created attachment 281373 [details] [review] misc patch to fix FBO leak if caps dynamically changed, decide_allocation callback of GstGLFilter/GstGLMixer didn't release previously allocated FBO. attachment is a misc patch to fix this.
plus, I found GstGLFilter didn't release in/out_tex_id in gst_gl_filter_decide_allocation , except these, there're other GL texture leaks ( with apitrace) and I have not found all of them.
Created attachment 281446 [details] [review] improved patch to fix more gl object leaks
seems that GLFilter only calls display_init_cb but not display_reset_cb in decide_allocation , which makes all GLFilter sub class lacks chance to release texture in display_reset_cb ,should we call display_reset_cb first as well?
Review of attachment 281446 [details] [review]: I'll leave it to Matthew on final review, I just have one small comment. Thanks for taking the time tracking this. ::: gst-libs/gst/gl/gstglfilter.c @@ +1105,3 @@ + gst_gl_context_del_texture (filter->context, &filter->out_tex_id); + filter->out_tex_id = 0; + } Just a nithpick, style would e better if you first free the resources, and then start creating them, would also keep it clean in case one of the creation failed.
Review of attachment 281446 [details] [review]: Looks good. Just some formatting nitpicks on top of Nicolas' comment. Thanks for looking at this. ::: gst-libs/gst/gl/gstglfilter.c @@ +280,3 @@ + } + + Spurious new line. @@ +1083,3 @@ out_height = GST_VIDEO_INFO_HEIGHT (&filter->out_info); + if (filter->fbo != 0) { Any particular reason why the sudden use of != 0 ? :) @@ +1093,3 @@ goto context_error; + Spurious new line
Created attachment 281479 [details] [review] improved patch v2 based on review suggestion. (In reply to comment #5) > Review of attachment 281446 [details] [review]: > > Looks good. Just some formatting nitpicks on top of Nicolas' comment. Thanks for reviewing, a improved patch with Nicolas' suggestion, release FBO/texture before creating new ones ,this should also avoid leaks when new FBO creation failed. also cleaned != 0 code. I just quickly copied code from reset function to test if this worked, without re-check the style .sorry for that :)
5d1ccc93665dbfeea747f184015afc4a9ec93e63 Author: Wang Xin-yu (王昕宇) <comicfans44@gmail.com> Date: Wed Jul 23 10:25:31 2014 +0800 gl: fix multi gl object leaks 1. fix FBO leaks in decide_allocation 2. fix texture leaks in decide_allocation and reset 3. fix texture leaks in FBO incomplete error path