GNOME Bugzilla – Bug 750189
gstchildproxy: some improvements
Last modified: 2018-11-03 12:27:49 UTC
The purpose of these patches is to remove redundant code in GESTrackElement, I can imagine a lot of use cases for fetching all the elements exposing a property in a bin aside from that.
Created attachment 304329 [details] [review] gstchildproxy: [API] get_child_name virtual method. To allow easier implementation of non-GstObject containers.
Created attachment 304330 [details] [review] gstchildproxy: [API] gst_child_proxy_lookup_all This method allows getting all the elements that expose a given property, recursively. The property name can be partially specified, ie if child_proxy1 contains child_proxy2 which has a property property_name, the user can ask for child_proxy2::property_name. + Adds a test, valgrind shows no leaks, documentation and introspection build OK.
Review of attachment 304329 [details] [review]: This looks correct to me. See comment on the ABI. ::: gst/gstchildproxy.h @@ +64,3 @@ GObject * (*get_child_by_index) (GstChildProxy * parent, guint index); guint (*get_children_count) (GstChildProxy * parent); + gchar * (*get_child_name) (GstChildProxy * parent, GObject *child); That's an ABI break. You need to reduce the padding. Fortunately you have added a pointer and the padding array is of type gpointer. So that would do: gpointer _gst_reserved[GST_PADDING - 1]; Normally, running "make distcheck" should run the ABI check, hence catch this kind of error.
I don't think interfaces need padding actually, you can just extend the interface structs without breaking ABI.
(In reply to Tim-Philipp Müller from comment #4) > I don't think interfaces need padding actually, you can just extend the > interface structs without breaking ABI. Yes, that's correct. Because interface structs are never subclassed, and they are only allocated from inside gobject.
Review of attachment 304330 [details] [review]: You need to update windows definition to. Looks like useful feature. ::: gst/gstchildproxy.c @@ +113,3 @@ + guint count, i; + GObject *object; + gchar *object_name = gst_object_get_name (GST_OBJECT (proxy)); A bit unrelated, but the proxy have to be a GstObject ? @@ +337,3 @@ + * objects that implement the given property. + * + * MT safe. This will requires a Since 1.6 mark ::: gst/gstchildproxy.h @@ +84,3 @@ GObject **target, GParamSpec **pspec); +GList * +gst_child_proxy_lookup_all (GstChildProxy * object, const gchar * name); Style is different here, all other lines put that on same line.
*** Bug 750188 has been marked as a duplicate of this bug. ***
*** Bug 750187 has been marked as a duplicate of this bug. ***
Created attachment 304332 [details] [review] gstchildproxy: [API] gst_child_proxy_lookup_all This method allows getting all the elements that expose a given property, recursively. The property name can be partially specified, ie if child_proxy1 contains child_proxy2 which has a property property_name, the user can ask for child_proxy2::property_name. + Adds a test, valgrind shows no leaks, documentation and introspection build OK.
(In reply to Nicolas Dufresne (stormer) from comment #6) > Review of attachment 304330 [details] [review] [review]: > > You need to update windows definition to. Looks like useful feature. > > ::: gst/gstchildproxy.c > @@ +113,3 @@ > + guint count, i; > + GObject *object; > + gchar *object_name = gst_object_get_name (GST_OBJECT (proxy)); > > A bit unrelated, but the proxy have to be a GstObject ? > Indeed it doesn't have to, I have renamed the get_child_name to get_object_name and use it to get the name of the proxy too. > @@ +337,3 @@ > + * objects that implement the given property. > + * > + * MT safe. > > This will requires a Since 1.6 mark > Done > ::: gst/gstchildproxy.h > @@ +84,3 @@ > GObject **target, GParamSpec > **pspec); > +GList * > +gst_child_proxy_lookup_all (GstChildProxy * object, const gchar * name); > > Style is different here, all other lines put that on same line. Done
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/114.