After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 739662 - gstobject: Add gst_object_has_parent()
gstobject: Add gst_object_has_parent()
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal blocker
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-05 10:59 UTC by Jan Alexander Steffens (heftig)
Modified: 2015-05-16 09:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (4.72 KB, patch)
2014-11-05 10:59 UTC, Jan Alexander Steffens (heftig)
needs-work Details | Review
patch, v2 (7.30 KB, patch)
2014-11-10 08:03 UTC, Jan Alexander Steffens (heftig)
none Details | Review
patch, v2 (7.05 KB, patch)
2014-11-10 08:06 UTC, Jan Alexander Steffens (heftig)
committed Details | Review
patch, v3 (7.01 KB, patch)
2014-11-10 09:06 UTC, Jan Alexander Steffens (heftig)
committed Details | Review
rename to gst_object_has_as_parent (7.42 KB, patch)
2015-05-15 06:17 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
accepted-commit_now Details | Review

Description Jan Alexander Steffens (heftig) 2014-11-05 10:59:10 UTC
Created attachment 290012 [details] [review]
patch

Adds gst_object_has_parent, which works like gst_object_has_ancestor but does not ascend further.
Comment 1 Sebastian Dröge (slomo) 2014-11-06 11:08:19 UTC
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
Comment 2 Jan Alexander Steffens (heftig) 2014-11-10 08:03:24 UTC
Created attachment 290308 [details] [review]
patch, v2

Here's an updated patch with added Since and expanded tests.
Comment 3 Jan Alexander Steffens (heftig) 2014-11-10 08:06:30 UTC
Created attachment 290309 [details] [review]
patch, v2

Put the Since on the wrong function, whoops.
Comment 4 Sebastian Dröge (slomo) 2014-11-10 08:56:21 UTC
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)"
Comment 5 Sebastian Dröge (slomo) 2014-11-10 09:04:19 UTC
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()
Comment 6 Jan Alexander Steffens (heftig) 2014-11-10 09:06:54 UTC
Created attachment 290311 [details] [review]
patch, v3

Fixed those things.
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2015-01-21 13:03:23 UTC
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.
Comment 8 Tim-Philipp Müller 2015-01-21 13:14:13 UTC
> 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()?
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2015-01-21 13:17:54 UTC
I like gst_object_check_parent() and as a 2nd gst_object_compare_parent()
Comment 10 Sebastian Dröge (slomo) 2015-01-21 16:52:27 UTC
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 :)
Comment 11 Olivier Crête 2015-01-21 18:12:06 UTC
Maybe gst_object_is_parent() ?
Comment 12 Julien Isorce 2015-01-23 00:14:34 UTC
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) 

:)
Comment 13 Stefan Sauer (gstreamer, gtkdoc dev) 2015-01-23 07:53:25 UTC
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.
Comment 14 Sebastian Dröge (slomo) 2015-03-15 14:57:30 UTC
What should we do about this? What is people's preference? :)

We need to decide on something soon.
Comment 15 Jan Alexander Steffens (heftig) 2015-03-15 16:16:10 UTC
I still like gst_object_has_parent most for symmetry with gst_object_has_ancestor.
Comment 16 Tim-Philipp Müller 2015-05-10 18:43:22 UTC
Last chance..
Comment 17 Stefan Sauer (gstreamer, gtkdoc dev) 2015-05-15 05:54:56 UTC
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.
Comment 18 Stefan Sauer (gstreamer, gtkdoc dev) 2015-05-15 06:17:23 UTC
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 19 Sebastian Dröge (slomo) 2015-05-15 07:50:46 UTC
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()?
Comment 20 Stefan Sauer (gstreamer, gtkdoc dev) 2015-05-15 12:06:24 UTC
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()
Comment 21 Jan Alexander Steffens (heftig) 2015-05-16 09:35:07 UTC
(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