GNOME Bugzilla – Bug 662664
API: add gst_element_class_add_static_pad_template() and fix template pad ref leaks
Last modified: 2011-11-28 17:04:55 UTC
Plenty of cut and pasted code pass a pad template ref to a function that does not take the ref, without unreffing the pad template afterwards (though a few do). The patches below touch a huge amount of files, but the alternatives seem unworkable (_add_pad_template has to take a ref, from a comment within, and gst_static_pad_template_get has to return a ref). Note that the cleanup function in gstelement.c does not seem to be called, so that ref taken by _add_pad_template isn't released, causing the object not actually be unreffed, so this needs looking at too (though some people may look at this as not-a-real-leak as it's called once per class and not once per object). One patch for the API addition, then one patch per repo to use it.
Created attachment 199911 [details] [review] gstelement: add gst_element_class_add_pad_template_from_static This function helps ensure the pad template is unreffed without having to complicate the calling code.
Created attachment 199912 [details] [review] various: fix pad template leaks
Created attachment 199913 [details] [review] various: fix pad template leaks
Created attachment 199914 [details] [review] various: fix pad template leaks
Created attachment 199915 [details] [review] various: fix pad template ref leaks
Created attachment 199916 [details] [review] various: fix pad template ref leaks
Created attachment 199917 [details] [review] ffmpeg: fix pad template ref leaks
Created attachment 199921 [details] [review] various: fix pad template leaks
Created attachment 199923 [details] [review] gstelement: add gst_element_class_add_pad_template_from_static This function helps ensure the pad template is unreffed without having to complicate the calling code. (add the new function to the def file)
Makes sense but please also merge this in 0.11 when pushing. Might be unnecessary with 0.11 because reference handling in that area changed IIRC, please check that.
Vincent, it would be great to get these in before the freeze.
Sorry for the bikeshedding, but why add_pad_template_from_static() and not just _add_static_pad_template()?
Because it's a copy of _add_pad_template, which takes a static template as paramter directly. The other name makes sense too I suppose. I'm going to merge this today, and don't care which name to use, so just confirm quick which you'd rather.
Committed with tpm's name change suggestion. In 0.11, the code seems to not need this indeed, it takes ownership of the floating ref. While I'm not fluent in ref language, I think this makes the patch unneeded for 0.11.
Yes, in 0.11 it takes ownership and ref_sink() the static pad template, to be consistent with other _add() API like gst_bin_add(). This change is not necessary in 0.11