GNOME Bugzilla – Bug 775538
Property "name" is not set for the net client clock
Last modified: 2016-12-07 09:23:51 UTC
I've noticed that "name" is not set in following methods: * gst_net_client_clock_new() * gst_ntp_clock_new() However, those functions take "name" parameter. I'm attaching a patch.
Created attachment 341278 [details] [review] fix
Comment on attachment 341278 [details] [review] fix Why is this a problem? The "name" is set on the external clock, but not on the internal one (and doing so there seems weird: this internal clock can be shared among multiple external ones!)
If so, the parameter "name" should be removed, because it seems to be unused in the _new() method. The change should probably be done on the next API break.
Review of attachment 341278 [details] [review]: ::: libs/gst/net/gstnetclientclock.c @@ +1331,3 @@ + g_object_new (GST_TYPE_NET_CLIENT_INTERNAL_CLOCK, "name", + GST_OBJECT_NAME (self), "address", self->priv->address, + "port", self->priv->port, "is-ntp", self->priv->is_ntp, NULL); This is the one that is wrong, the others are ok
Ok, so there is no problem with the internal clock. Can you remove the wrong part of the patch, then I'll merge it :)
Created attachment 341439 [details] [review] fix v2 that's correct, setting a name makes sense for external clocks, not internal ones. My intention was to uptake unused 'name' parameter of the constructor, so new patch fixes it. Thanks!
commit 90f101981cccdd5aed7266298e466e5f00e4737a Author: Marcin Kolny <marcin.kolny@gmail.com> Date: Fri Dec 2 22:47:32 2016 +0100 net: set clock name in the constructor gst_net_client_clock_new() and gst_ntp_clock_new() didn't set the "name" property. https://bugzilla.gnome.org/show_bug.cgi?id=775538