GNOME Bugzilla – Bug 739662
gstobject: Add gst_object_has_parent()
Last modified: 2015-05-16 09:35:07 UTC
Created attachment 290012 [details] [review] patch Adds gst_object_has_parent, which works like gst_object_has_ancestor but does not ascend further.
Review of attachment 290012 [details] [review]: Please also add this to the has_ancestor() test and ensure in a test that both functions continue to have the same behaviour regarding the direct parent. ::: gst/gstobject.c @@ +790,3 @@ + * the parent of @object. Otherwise %FALSE. + * + * MT safe. Grabs and releases @object's locks. Since: 1.6 should be added here
Created attachment 290308 [details] [review] patch, v2 Here's an updated patch with added Since and expanded tests.
Created attachment 290309 [details] [review] patch, v2 Put the Since on the wrong function, whoops.
Review of attachment 290309 [details] [review]: ::: gst/gstobject.c @@ +800,3 @@ + if (G_LIKELY (GST_IS_OBJECT (object) && GST_IS_OBJECT (parent))) { + GST_OBJECT_LOCK (object); + result = GST_OBJECT_PARENT (object) == parent ? TRUE : FALSE; No need to do "result = condition ? TRUE : FALSE". Just do "result = condition" :) ::: tests/check/gst/gstobject.c @@ +380,3 @@ + result = gst_object_has_parent (object1, object2); + fail_if (result == TRUE, "GstFakeObject has a parent"); And don't compare booleans by equality. Just "fail_if (result)"
commit d600fc48f8486428b70cbfd841af80a1a36c2519 Author: Sebastian Dröge <sebastian@centricular.com> Date: Mon Nov 10 10:01:02 2014 +0100 gstobject: Don't check booleans for equality in the unit test Every value other than 0/FALSE is TRUE, == TRUE will only check for 1. commit 5a930aa642837e4732bfa7b7a83339dea30712c0 Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv> Date: Wed Nov 5 11:50:47 2014 +0100 gstobject: Add gst_object_has_parent() Adds gst_object_has_parent, which works like gst_object_has_ancestor but does not ascend further. API: gst_object_has_parent()
Created attachment 290311 [details] [review] patch, v3 Fixed those things.
Hmm the new api should maybe be called gst_object_has_object_as_parent() of something like it. gst_object_has_parent() sounds like it would check if object has any parent.
> gst_object_has_object_as_parent() That sounds like it checks if the parent is an object (rather than something else, which it can't be). How about: gst_object_check_parent() or gst_object_has_this_parent() or gst_object_compare_parent()?
I like gst_object_check_parent() and as a 2nd gst_object_compare_parent()
Not too late to change that, it wasn't in any release :) What about gst_object_has_as_parent()? I think all we had so far doesn't sound nice or is confusing :)
Maybe gst_object_is_parent() ?
Hehe! In the first place I think the idea was to follow the gst_object_has_ancestor. So that would make a reasonable reason to introduce gst_object_has_parent. Then I think gst_object_is_parent is good but maybe it needs to switch parent and child in parameter order. So personally I would go for bool gst_object_is_parent_of(GstObject * object, GstObject * child) Also bool gst_object_is_child_of(GstObject * object, GstObject * parent) We could also have int gst_object_depth_of_ancestor(GstObject * object, GstObject * ancestor) which returns -1 if not an ancestor, 0 if equal, and the depth if it is. Same with int gst_object_depth_of_descendant(GstObject * object, GstObject * descendant) :)
A few more thoughts: - in my projects I simply use GST_OBJECT_PARENT(obj) to implement gboolean gst_object_has_parent(obj) { return GST_OBJECT_PARENT(obj) != NULL; } gboolean gst_object_compare_parent(obj, parent) { return GST_OBJECT_PARENT(obj) == parent; } If we want to introduce them as functions, we could also simply add both. -1 for gst_object_is_parent() as I would understand this as (obj->children != NULL). I think I like gst_object_compare_parent() the most as this makes it obvious that you need a 2nd parameter passing what you want to compare to.
What should we do about this? What is people's preference? :) We need to decide on something soon.
I still like gst_object_has_parent most for symmetry with gst_object_has_ancestor.
Last chance..
Given the proposed options: # check if object has a specific parent gboolean gst_object_has_object_as_parent (GstObject *self, GstObject * parent) gboolean gst_object_check_parent (GstObject *self, GstObject * parent) gboolean gst_object_has_this_parent (GstObject *self, GstObject * parent) gboolean gst_object_compare_parent (GstObject *self, GstObject * parent) gboolean gst_object_has_as_parent (GstObject *self, GstObject * parent) this one gboolean gst_object_has_as_parent (GstObject *self, GstObject * parent) sound best for me. We could add a FIXME-2.X for gst_object_has_ancestor () to be renamed to gst_object_has_as_ancestor () to maintain symmetry.
Created attachment 303404 [details] [review] rename to gst_object_has_as_parent Here is a patch just in case. I wanted to create a poll, but did not found something that works without accounts :/
Comment on attachment 303404 [details] [review] rename to gst_object_has_as_parent Go for it :) maybe for symmetry already add gst_object_has_as_ancestor()?
Good idea, better than just the fixme. commit b8c6ebd0f2a3457f97a64835ec939fbec7bb26e0 Author: Stefan Sauer <ensonic@users.sf.net> Date: Fri May 15 13:43:12 2015 +0200 gstobject: add gst_object_has_as_ancestor and deprecate previous function The old gst_object_has_ancestor will call the new code. This establishes the symetry with the new gst_object_has_as_parent. API: gst_object_has_as_ancestor() commit 3492105a063f4f51e9f64352167e73df1c09b77c Author: Stefan Sauer <ensonic@users.sf.net> Date: Fri May 15 08:05:50 2015 +0200 gstobject: rename gst_object_has_parent to gst_object_has_as_parent This avoid confusion with a potential punction that check if a gstobject has-a parent. API: gst_object_has_as_parent()
(In reply to Stefan Sauer (gstreamer, gtkdoc dev) from comment #18) FWIW, you can use doodle.com in free text mode. Sample: http://doodle.com/nf9wk2txf3cb9b5k