GNOME Bugzilla – Bug 731301
Make sure GstAggregator's API is bindings friendly
Last modified: 2017-11-26 11:18:39 UTC
As they are the functions are not introspectable. I believe there should be a version of all the gst_collect_pads_set_*function functions which also includes a GDestroyNotify argument for the user data. Everything gets stuffed into a private data structure, so it wouldn't be difficult to add GDestroyNotify fields. FWIW I'd be willing to write the patch, just want to confirm that (scope async) is insufficient first... these may be called a number of times != 1, correct?
CollectPads might get replaced by a BaseMixer element base class soon, so don't know if it's worth it.
We would be happy to have GstAggregator introspected, we did not take the time to do it just yet, but it should not be complicated at all. Patch are very welcome, otherwize we should do it soon anyway.
The hard part is that we can only add annotations on vfuncs if there is a matching regular function on the class, by using the (invoker ..) annotation. As for the gst_aggregator_iterate_sinkpads() function, it's only a wrapper around GstIterator that makes sure the function is called once. I wonder if we could replace it with a GstIterator filter instead of having a new API. So I think bindings can safely skip it.
Not sure what we can do about the vfunc annotation stuff, that's something that would need to be solved in g-i, no? Is there a bug for it already? Would we only need vfunc annotations for non-default transfer annotations, or is aggregator not subclassable/usable by bindings at all without annotations? Don't think any of the vfuncs take ownerships of any arguments and always return transfer full things for non-const returns, which I believe matches default assumptions? In general the API looks like it should be introspection-friendly to me otherwise. If not, now is the time to speak up.
Don't know how bindings-friendly the pad GType in the class structure is. Either way, I don't think we're going to block a move to core on this if no one's really bothered.
> Don't know how bindings-friendly the pad GType in the class structure is. I do not think that can work in python out of the box, but I agree we should not block on that tbh!
Created attachment 363083 [details] [review] aggregator: Remove klass->sinkpads_type This posed problems for the python bindings (and possibly others). Instead, subclasses now use add_pad_template_with_type.
Review of attachment 363083 [details] [review]: Rejected, not attached to the right bug
See https://bugzilla.gnome.org/show_bug.cgi?id=789986 for the sinkpads_type part
Comment on attachment 363083 [details] [review] aggregator: Remove klass->sinkpads_type This was committed in some form (updated for api changes).
Let's close this, I don't see anything else that needs doing here. And if there is indeed a problem with vfuncs needing annotations, then that needs to be fixed in g-i first anyway.