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 766923 - object: Notify name change when using _set_name()
object: Notify name change when using _set_name()
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-05-26 17:20 UTC by Nicolas Dufresne (ndufresne)
Modified: 2016-05-26 19:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
object: Notify name change when using _set_name() (1.05 KB, patch)
2016-05-26 17:20 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
object: Check that name change are notified once (2.34 KB, patch)
2016-05-26 18:44 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
object: Add _set_name() test on parented object (1.28 KB, patch)
2016-05-26 18:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2016-05-26 17:20:35 UTC
I came across this FIXME while chasing a bug in a case were I wanted to cache
the presence of certain elements using their name. If the name was change using
the method instead of the property, the element remains with invalid name
there.
Comment 1 Nicolas Dufresne (ndufresne) 2016-05-26 17:20:38 UTC
Created attachment 328578 [details] [review]
object: Notify name change when using _set_name()

There was a 0.11 FIXME about notifying the name change or removing that
function. Clearly we can't remove this function, so let's notify it.
Comment 2 Tim-Philipp Müller 2016-05-26 17:34:01 UTC
Comment on attachment 328578 [details] [review]
object: Notify name change when using _set_name()

>-  /* FIXME-0.11: this misses a g_object_notify (object, "name"); unless called
>-   * from gst_object_set_property.
>-   * Ideally remove such custom setters (or make it static).
>-   */
>+
>+  g_object_notify (G_OBJECT (object), "name");
>   return result;

I wonder about the "unless called from gst_object_set_property" - did you check if we get notified twice if set using g_object_set()?
Comment 3 Nicolas Dufresne (ndufresne) 2016-05-26 17:48:15 UTC
It's implicit for set_property/get_property() virtual method.
Comment 4 Nicolas Dufresne (ndufresne) 2016-05-26 17:48:43 UTC
(I meant just set_property() obviously)
Comment 5 Tim-Philipp Müller 2016-05-26 18:02:47 UTC
What I meant was that if someone does g_object_set(obj, "name", ...) will we now get two notifies, one from GObject and one we do inside _set_name(), or will GObject "compress" them so that it will only be emitted once in that case?
Comment 6 Nicolas Dufresne (ndufresne) 2016-05-26 18:44:18 UTC
Created attachment 328591 [details] [review]
object: Check that name change are notified once

GObject allow calling g_object_notify() withing set_property() and
won't notify it twice. As it was raised, add a unit test for to
make sure.
Comment 7 Nicolas Dufresne (ndufresne) 2016-05-26 18:45:56 UTC
Created attachment 328592 [details] [review]
object: Add _set_name() test on parented object

This is not allowed, and set_name() should fail.
Comment 8 Nicolas Dufresne (ndufresne) 2016-05-26 18:51:29 UTC
(In reply to Tim-Philipp Müller from comment #5)
> What I meant was that if someone does g_object_set(obj, "name", ...) will we
> now get two notifies, one from GObject and one we do inside _set_name(), or
> will GObject "compress" them so that it will only be emitted once in that
> case?

No, GObject have a fency notification queue. This is used to allow calling g_object_notify () without notifying twice, but also to not notify twice if one call:

  g_object_set (object, "prop", val, "prop", val, NULL);

We even use this feature from within GStreamer (see calls to g_object_notify_freeze/g_object_notify_thaw).
Comment 9 Tim-Philipp Müller 2016-05-26 18:54:27 UTC
Ok, cool, sorry for the noise then :) (I remembered something like that, but wasn't sure)
Comment 10 Nicolas Dufresne (ndufresne) 2016-05-26 19:37:11 UTC
Attachment 328578 [details] pushed as 4467784 - object: Notify name change when using _set_name()
Attachment 328591 [details] pushed as 1a5f799 - object: Check that name change are notified once
Attachment 328592 [details] pushed as 850510f - object: Add _set_name() test on parented object