netclientclock: Don't ever store failed internal clocks in the cache

If starting the internal clock fails we would still store a broken clock in the
cache despite it being unusable and never recovering.

Not storing it allows the application to simply create a new one at a later time
and have starting it retried.

Also signal to the application that such a clock is not synced.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8334>
This commit is contained in:
Sebastian Dröge 2025-01-22 12:32:24 +02:00 committed by GStreamer Marge Bot
parent 154ea45111
commit dd58ae8cc8

View file

@ -331,6 +331,7 @@ gst_net_client_internal_clock_constructed (GObject * object)
if (!gst_net_client_internal_clock_start (self)) {
g_warning ("failed to start clock '%s'", GST_OBJECT_NAME (self));
self->marked_corrupted = TRUE;
}
/* all systems go, cap'n */
@ -1181,41 +1182,44 @@ static void
gst_net_client_clock_finalize (GObject * object)
{
GstNetClientClock *self = GST_NET_CLIENT_CLOCK (object);
GList *l;
if (self->priv->synced_id)
g_signal_handler_disconnect (self->priv->internal_clock,
self->priv->synced_id);
self->priv->synced_id = 0;
if (self->priv->internal_clock) {
GList *l;
G_LOCK (clocks_lock);
for (l = clocks; l; l = l->next) {
ClockCache *cache = l->data;
if (self->priv->synced_id)
g_signal_handler_disconnect (self->priv->internal_clock,
self->priv->synced_id);
self->priv->synced_id = 0;
if (cache->clock == self->priv->internal_clock) {
cache->clocks = g_list_remove (cache->clocks, self);
G_LOCK (clocks_lock);
for (l = clocks; l; l = l->next) {
ClockCache *cache = l->data;
if (cache->clocks) {
update_clock_cache (cache);
} else {
GstClock *sysclock = gst_system_clock_obtain ();
GstClockTime time = gst_clock_get_time (sysclock);
GstNetClientInternalClock *internal_clock =
GST_NET_CLIENT_INTERNAL_CLOCK (cache->clock);
if (cache->clock == self->priv->internal_clock) {
cache->clocks = g_list_remove (cache->clocks, self);
/* only defer deletion if the clock is not marked corrupted */
if (!internal_clock->marked_corrupted)
time += 60 * GST_SECOND;
if (cache->clocks) {
update_clock_cache (cache);
} else {
GstClock *sysclock = gst_system_clock_obtain ();
GstClockTime time = gst_clock_get_time (sysclock);
GstNetClientInternalClock *internal_clock =
GST_NET_CLIENT_INTERNAL_CLOCK (cache->clock);
cache->remove_id = gst_clock_new_single_shot_id (sysclock, time);
gst_clock_id_wait_async (cache->remove_id, remove_clock_cache, cache,
NULL);
gst_object_unref (sysclock);
/* only defer deletion if the clock is not marked corrupted */
if (!internal_clock->marked_corrupted)
time += 60 * GST_SECOND;
cache->remove_id = gst_clock_new_single_shot_id (sysclock, time);
gst_clock_id_wait_async (cache->remove_id, remove_clock_cache, cache,
NULL);
gst_object_unref (sysclock);
}
break;
}
break;
}
G_UNLOCK (clocks_lock);
}
G_UNLOCK (clocks_lock);
g_free (self->priv->address);
self->priv->address = NULL;
@ -1362,7 +1366,6 @@ static void
gst_net_client_clock_constructed (GObject * object)
{
GstNetClientClock *self = GST_NET_CLIENT_CLOCK (object);
GstClock *internal_clock;
GList *l;
ClockCache *cache = NULL;
@ -1390,34 +1393,49 @@ gst_net_client_clock_constructed (GObject * object)
}
if (!cache) {
cache = g_new0 (ClockCache, 1);
GstNetClientInternalClock *internal_clock;
cache->clock =
internal_clock =
g_object_new (GST_TYPE_NET_CLIENT_INTERNAL_CLOCK, "address",
self->priv->address, "port", self->priv->port, "is-ntp",
self->priv->is_ntp, NULL);
gst_object_ref_sink (cache->clock);
clocks = g_list_prepend (clocks, cache);
gst_object_ref_sink (internal_clock);
/* Not actually leaked but is cached for a while before being disposed,
* see gst_net_client_clock_finalize, so pretend it is to not confuse
* tests. */
GST_OBJECT_FLAG_SET (cache->clock, GST_OBJECT_FLAG_MAY_BE_LEAKED);
if (internal_clock->marked_corrupted) {
GST_WARNING_OBJECT (self, "Internal clock couldn't start");
gst_object_unref (internal_clock);
} else {
cache = g_new0 (ClockCache, 1);
cache->clock = GST_CLOCK (internal_clock);
clocks = g_list_prepend (clocks, cache);
/* Not actually leaked but is cached for a while before being disposed,
* see gst_net_client_clock_finalize, so pretend it is to not confuse
* tests. */
GST_OBJECT_FLAG_SET (cache->clock, GST_OBJECT_FLAG_MAY_BE_LEAKED);
}
}
cache->clocks = g_list_prepend (cache->clocks, self);
if (cache) {
cache->clocks = g_list_prepend (cache->clocks, self);
GST_OBJECT_LOCK (cache->clock);
if (gst_clock_is_synced (cache->clock))
gst_clock_set_synced (GST_CLOCK (self), TRUE);
self->priv->synced_id =
g_signal_connect (cache->clock, "synced",
G_CALLBACK (gst_net_client_clock_synced_cb), self);
GST_OBJECT_UNLOCK (cache->clock);
GST_OBJECT_LOCK (cache->clock);
if (gst_clock_is_synced (cache->clock))
gst_clock_set_synced (GST_CLOCK (self), TRUE);
self->priv->synced_id =
g_signal_connect (cache->clock, "synced",
G_CALLBACK (gst_net_client_clock_synced_cb), self);
GST_OBJECT_UNLOCK (cache->clock);
self->priv->internal_clock = cache->clock;
}
G_UNLOCK (clocks_lock);
self->priv->internal_clock = internal_clock = cache->clock;
/* Mark clock as unsynced if creation of the internal clock failed */
if (!cache)
gst_clock_set_synced (GST_CLOCK (self), FALSE);
/* all systems go, cap'n */
}
@ -1427,6 +1445,9 @@ gst_net_client_clock_get_internal_time (GstClock * clock)
{
GstNetClientClock *self = GST_NET_CLIENT_CLOCK (clock);
if (!self->priv->internal_clock)
return self->priv->base_time;
if (!gst_clock_is_synced (self->priv->internal_clock)) {
GstClockTime now = gst_clock_get_internal_time (self->priv->internal_clock);
return gst_clock_adjust_with_calibration (self->priv->internal_clock, now,