Skip to content

Commit

Permalink
multihandlesink: Use the monotonic clock for detecting timeouts and c…
Browse files Browse the repository at this point in the history
…onnection durations

Otherwise real-time clock changes can wrongly trigger timeouts, or not
cause timeouts to happen in time.

Unfortunately real-time clock times still have to be kept track inside
the elements for the statistics. Switching those over to the monotonic
clock would cause behaviour changes from the application point of view.

The statistics are extended with fields with monotonic times though.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1137>
  • Loading branch information
sdroege authored and GStreamer Marge Bot committed May 5, 2021
1 parent 26b8a96 commit 5a65f5f
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 17 deletions.
8 changes: 5 additions & 3 deletions gst/tcp/gstmultifdsink.c
Expand Up @@ -671,7 +671,7 @@ gst_multi_fd_sink_handle_client_write (GstMultiFdSink * sink,
{
gboolean more;
gboolean flushing;
GstClockTime now;
GstClockTime now, now_monotonic;
GstMultiHandleSink *mhsink = GST_MULTI_HANDLE_SINK (sink);
GstMultiHandleSinkClass *mhsinkclass =
GST_MULTI_HANDLE_SINK_GET_CLASS (mhsink);
Expand All @@ -685,6 +685,7 @@ gst_multi_fd_sink_handle_client_write (GstMultiFdSink * sink,
gint maxsize;

now = g_get_real_time () * GST_USECOND;
now_monotonic = g_get_monotonic_time () * GST_USECOND;

if (!mhclient->sending) {
/* client is not working on a buffer */
Expand Down Expand Up @@ -811,6 +812,7 @@ gst_multi_fd_sink_handle_client_write (GstMultiFdSink * sink,
/* update stats */
mhclient->bytes_sent += wrote;
mhclient->last_activity_time = now;
mhclient->last_activity_time_monotonic = now_monotonic;
mhsink->bytes_served += wrote;
}
}
Expand Down Expand Up @@ -903,7 +905,7 @@ gst_multi_fd_sink_handle_clients (GstMultiFdSink * sink)
if (G_UNLIKELY (result == 0)) {
GstClockTime now;

now = g_get_real_time () * GST_USECOND;
now = g_get_monotonic_time () * GST_USECOND;

CLIENTS_LOCK (mhsink);
for (clients = mhsink->clients; clients; clients = next) {
Expand All @@ -914,7 +916,7 @@ gst_multi_fd_sink_handle_clients (GstMultiFdSink * sink)
mhclient = (GstMultiHandleClient *) client;
next = g_list_next (clients);
if (mhsink->timeout > 0
&& now - mhclient->last_activity_time > mhsink->timeout) {
&& now - mhclient->last_activity_time_monotonic > mhsink->timeout) {
mhclient->status = GST_CLIENT_STATUS_SLOW;
gst_multi_handle_sink_remove_client_link (mhsink, clients);
}
Expand Down
34 changes: 23 additions & 11 deletions gst/tcp/gstmultihandlesink.c
Expand Up @@ -543,9 +543,12 @@ gst_multi_handle_sink_client_init (GstMultiHandleClient * client,

/* update start time */
client->connect_time = g_get_real_time () * GST_USECOND;
client->connect_time_monotonic = g_get_monotonic_time () * GST_USECOND;
client->disconnect_time = 0;
client->disconnect_time_monotonic = 0;
/* set last activity time to connect time */
client->last_activity_time = client->connect_time;
client->last_activity_time_monotonic = client->connect_time_monotonic;
}

static void
Expand Down Expand Up @@ -807,21 +810,28 @@ gst_multi_handle_sink_get_stats (GstMultiHandleSink * sink,

result = gst_structure_new_empty ("multihandlesink-stats");

if (mhclient->disconnect_time == 0) {
interval = (g_get_real_time () * GST_USECOND) - mhclient->connect_time;
if (mhclient->disconnect_time_monotonic == 0) {
interval =
(g_get_monotonic_time () * GST_USECOND) -
mhclient->connect_time_monotonic;
} else {
interval = mhclient->disconnect_time - mhclient->connect_time;
interval =
mhclient->disconnect_time_monotonic -
mhclient->connect_time_monotonic;
}

gst_structure_set (result,
"bytes-sent", G_TYPE_UINT64, mhclient->bytes_sent,
"connect-time", G_TYPE_UINT64, mhclient->connect_time,
"disconnect-time", G_TYPE_UINT64, mhclient->disconnect_time,
"connect-duration", G_TYPE_UINT64, interval,
"last-activity-time", G_TYPE_UINT64, mhclient->last_activity_time,
"buffers-dropped", G_TYPE_UINT64, mhclient->dropped_buffers,
"first-buffer-ts", G_TYPE_UINT64, mhclient->first_buffer_ts,
"last-buffer-ts", G_TYPE_UINT64, mhclient->last_buffer_ts, NULL);
"connect-time-monotonic", G_TYPE_UINT64,
mhclient->connect_time_monotonic, "disconnect-time", G_TYPE_UINT64,
mhclient->disconnect_time, "disconnect-time-monotonic", G_TYPE_UINT64,
mhclient->disconnect_time_monotonic, "connect-duration", G_TYPE_UINT64,
interval, "last-activity-time-monotonic", G_TYPE_UINT64,
mhclient->last_activity_time_monotonic, "buffers-dropped",
G_TYPE_UINT64, mhclient->dropped_buffers, "first-buffer-ts",
G_TYPE_UINT64, mhclient->first_buffer_ts, "last-buffer-ts",
G_TYPE_UINT64, mhclient->last_buffer_ts, NULL);
}

noclient:
Expand Down Expand Up @@ -891,6 +901,7 @@ gst_multi_handle_sink_remove_client_link (GstMultiHandleSink * sink,
mhsinkclass->hash_removing (sink, mhclient);

mhclient->disconnect_time = g_get_real_time () * GST_USECOND;
mhclient->disconnect_time_monotonic = g_get_monotonic_time () * GST_USECOND;

/* free client buffers */
g_slist_foreach (mhclient->sending, (GFunc) gst_mini_object_unref, NULL);
Expand Down Expand Up @@ -1652,7 +1663,7 @@ gst_multi_handle_sink_queue_buffer (GstMultiHandleSink * mhsink,
}

max_buffer_usage = 0;
now = g_get_real_time () * GST_USECOND;
now = g_get_monotonic_time () * GST_USECOND;

/* now check for new or slow clients */
restart:
Expand All @@ -1670,7 +1681,8 @@ gst_multi_handle_sink_queue_buffer (GstMultiHandleSink * mhsink,
/* check hard max and timeout, remove client */
if ((max_buffers > 0 && mhclient->bufpos >= max_buffers) ||
(mhsink->timeout > 0
&& now - mhclient->last_activity_time > mhsink->timeout)) {
&& now - mhclient->last_activity_time_monotonic >
mhsink->timeout)) {
/* remove client */
GST_WARNING_OBJECT (sink, "%s client %p is too slow, removing",
mhclient->debug, mhclient);
Expand Down
3 changes: 3 additions & 0 deletions gst/tcp/gstmultihandlesink.h
Expand Up @@ -163,8 +163,11 @@ typedef struct {
/* stats */
guint64 bytes_sent;
guint64 connect_time;
guint64 connect_time_monotonic;
guint64 disconnect_time;
guint64 disconnect_time_monotonic;
guint64 last_activity_time;
guint64 last_activity_time_monotonic;
guint64 dropped_buffers;
guint64 avg_queue_size;
guint64 first_buffer_ts;
Expand Down
8 changes: 5 additions & 3 deletions gst/tcp/gstmultisocketsink.c
Expand Up @@ -825,7 +825,7 @@ gst_multi_socket_sink_handle_client_write (GstMultiSocketSink * sink,
{
gboolean more;
gboolean flushing;
GstClockTime now;
GstClockTime now, now_monotonic;
GError *err = NULL;
GstMultiHandleSink *mhsink = GST_MULTI_HANDLE_SINK (sink);
GstMultiHandleClient *mhclient = (GstMultiHandleClient *) client;
Expand All @@ -834,6 +834,7 @@ gst_multi_socket_sink_handle_client_write (GstMultiSocketSink * sink,


now = g_get_real_time () * GST_USECOND;
now_monotonic = g_get_monotonic_time () * GST_USECOND;

flushing = mhclient->status == GST_CLIENT_STATUS_FLUSHING;

Expand Down Expand Up @@ -951,6 +952,7 @@ gst_multi_socket_sink_handle_client_write (GstMultiSocketSink * sink,
/* update stats */
mhclient->bytes_sent += wrote;
mhclient->last_activity_time = now;
mhclient->last_activity_time_monotonic = now_monotonic;
mhsink->bytes_served += wrote;
}
}
Expand Down Expand Up @@ -1115,7 +1117,7 @@ gst_multi_socket_sink_timeout (GstMultiSocketSink * sink)
GList *clients;
GstMultiHandleSink *mhsink = GST_MULTI_HANDLE_SINK (sink);

now = g_get_real_time () * GST_USECOND;
now = g_get_monotonic_time () * GST_USECOND;

CLIENTS_LOCK (mhsink);
for (clients = mhsink->clients; clients; clients = clients->next) {
Expand All @@ -1125,7 +1127,7 @@ gst_multi_socket_sink_timeout (GstMultiSocketSink * sink)
client = clients->data;
mhclient = (GstMultiHandleClient *) client;
if (mhsink->timeout > 0
&& now - mhclient->last_activity_time > mhsink->timeout) {
&& now - mhclient->last_activity_time_monotonic > mhsink->timeout) {
mhclient->status = GST_CLIENT_STATUS_SLOW;
gst_multi_handle_sink_remove_client_link (mhsink, clients);
}
Expand Down

0 comments on commit 5a65f5f

Please sign in to comment.