[spice-server,5/7] DisplayChannel: add GSource for video stream timeouts

Submitted by Jonathon Jongsma on Nov. 29, 2017, 8:16 p.m.

Details

Message ID 20171129201610.30738-6-jjongsma@redhat.com
State New
Headers show
Series "Stream cleanups" ( rev: 2 1 ) in Spice

Not browsing as part of any series.

Commit Message

Jonathon Jongsma Nov. 29, 2017, 8:16 p.m.
Rather than handling the video stream timeouts from the RedWorker,
create a separate GSource to handle these timeouts. This allows us to
encapsulate the DisplayChannel functionality within display-channel.c a
bit better. It also allowed us to avoid exposing
display_channel_get_streams_timeout().

detach_video_stream_gracefully() is temporarily moved to the header
but will be moved to display-channel.c in a subsequent commit.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 server/display-channel.c | 104 +++++++++++++++++++++++++++++++++++++----------
 server/display-channel.h |   1 -
 server/red-worker.c      |   9 +---
 server/video-stream.c    |  23 ++---------
 server/video-stream.h    |   4 +-
 5 files changed, 90 insertions(+), 51 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/display-channel.c b/server/display-channel.c
index 1c14344be..35c2a0197 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -202,27 +202,6 @@  static MonitorsConfig* monitors_config_new(QXLHead *heads, ssize_t nheads, ssize
     return mc;
 }
 
-int display_channel_get_streams_timeout(DisplayChannel *display)
-{
-    int timeout = INT_MAX;
-    Ring *ring = &display->priv->streams;
-    RingItem *item = ring;
-
-    red_time_t now = spice_get_monotonic_time_ns();
-    while ((item = ring_next(ring, item))) {
-        VideoStream *stream;
-
-        stream = SPICE_CONTAINEROF(item, VideoStream, link);
-        red_time_t delta = (stream->last_time + RED_STREAM_TIMEOUT) - now;
-
-        if (delta < NSEC_PER_MILLISEC) {
-            return 0;
-        }
-        timeout = MIN(timeout, (unsigned int)(delta / (NSEC_PER_MILLISEC)));
-    }
-    return timeout;
-}
-
 void display_channel_set_stream_video(DisplayChannel *display, int stream_video)
 {
     spice_return_if_fail(display);
@@ -2253,6 +2232,82 @@  static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces, uint32_t su
     return p->surfaces[surface_id].context.canvas;
 }
 
+typedef struct VideoTimeoutSource {
+    GSource source;
+    DisplayChannel *display;
+} VideoTimeoutSource;
+
+static gboolean video_timeout_source_prepare(GSource *source, gint *p_timeout)
+{
+    VideoTimeoutSource *vsource = SPICE_CONTAINEROF(source, VideoTimeoutSource, source);
+    int timeout = INT_MAX;
+    Ring *ring = &vsource->display->priv->streams;
+    RingItem *item = ring;
+
+    red_time_t now = spice_get_monotonic_time_ns();
+    while ((item = ring_next(ring, item))) {
+        VideoStream *stream;
+
+        stream = SPICE_CONTAINEROF(item, VideoStream, link);
+        red_time_t delta = (stream->last_time + RED_STREAM_TIMEOUT) - now;
+
+        if (delta < NSEC_PER_MILLISEC) {
+            return 0;
+        }
+        timeout = MIN(timeout, (unsigned int)(delta / (NSEC_PER_MILLISEC)));
+    }
+    *p_timeout = (timeout == INT_MAX) ? -1 : timeout;
+    if (*p_timeout == 0) {
+        return TRUE;
+    }
+
+    return FALSE;
+}
+
+static gboolean video_timeout_source_check(GSource *source)
+{
+    VideoTimeoutSource *vsource = SPICE_CONTAINEROF(source, VideoTimeoutSource, source);
+    DisplayChannel *display = vsource->display;
+    /* Since we have to iterate through all streams to see if one of them is
+     * actually ready, so if we have any video streams, we may as well simply
+     * return TRUE here and scan the list in the dispatch function */
+    /* FIXME: remove local variable */
+    bool ready = !(ring_is_empty(&display->priv->streams));
+    if (ready) spice_debug("ready");
+    return ready;
+}
+
+static gboolean video_timeout_source_dispatch(GSource *source, GSourceFunc callback,
+                                              gpointer user_data)
+{
+    VideoTimeoutSource *vsource = SPICE_CONTAINEROF(source, VideoTimeoutSource, source);
+    DisplayChannel *display = vsource->display;
+
+    Ring *ring = &display->priv->streams;
+    RingItem *item;
+
+    red_time_t now = spice_get_monotonic_time_ns();
+    item = ring_get_head(ring);
+    while (item) {
+        VideoStream *stream = SPICE_CONTAINEROF(item, VideoStream, link);
+        item = ring_next(ring, item);
+        if (now >= (stream->last_time + RED_STREAM_TIMEOUT)) {
+            spice_debug("stream timeout for %p", stream);
+            detach_video_stream_gracefully(display, stream, NULL);
+            video_stream_stop(stream);
+        }
+    }
+
+    /* G_SOURCE_CONTINUE */
+    return TRUE;
+}
+
+
+static GSourceFuncs video_timeout_source_funcs = {
+    .prepare = video_timeout_source_prepare,
+    .check = video_timeout_source_check,
+    .dispatch = video_timeout_source_dispatch,
+};
 DisplayChannel* display_channel_new(RedsState *reds,
                                     QXLInstance *qxl,
                                     const SpiceCoreInterfaceInternal *core,
@@ -2359,6 +2414,13 @@  display_channel_constructed(GObject *object)
     red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
     red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE);
     red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
+
+    /* set up source for handling video stream timeouts */
+    SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(RED_CHANNEL(self));
+    GSource *source = g_source_new(&video_timeout_source_funcs, sizeof(VideoTimeoutSource));
+    SPICE_CONTAINEROF(source, VideoTimeoutSource, source)->display = self;
+    g_source_attach(source, core->main_context);
+    g_source_unref(source);
 }
 
 void display_channel_process_surface_cmd(DisplayChannel *display,
diff --git a/server/display-channel.h b/server/display-channel.h
index a0470ec67..0718bb61c 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -133,7 +133,6 @@  void                       display_channel_set_stream_video          (DisplayCha
                                                                       int stream_video);
 void                       display_channel_set_video_codecs          (DisplayChannel *display,
                                                                       GArray *video_codecs);
-int                        display_channel_get_streams_timeout       (DisplayChannel *display);
 void                       display_channel_compress_stats_print      (DisplayChannel *display);
 void                       display_channel_compress_stats_reset      (DisplayChannel *display);
 void                       display_channel_surface_unref             (DisplayChannel *display,
diff --git a/server/red-worker.c b/server/red-worker.c
index eb41cef7e..cdf29b329 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -1221,12 +1221,8 @@  static gboolean worker_source_prepare(GSource *source, gint *p_timeout)
 {
     RedWorkerSource *wsource = SPICE_CONTAINEROF(source, RedWorkerSource, source);
     RedWorker *worker = wsource->worker;
-    unsigned int timeout;
 
-    timeout = MIN(worker->event_timeout,
-                  display_channel_get_streams_timeout(worker->display_channel));
-
-    *p_timeout = (timeout == INF_EVENT_WAIT) ? -1 : timeout;
+    *p_timeout = (worker->event_timeout == INF_EVENT_WAIT) ? -1 : worker->event_timeout;
     if (*p_timeout == 0)
         return TRUE;
 
@@ -1259,9 +1255,6 @@  static gboolean worker_source_dispatch(GSource *source, GSourceFunc callback,
     /* TODO: why is this here, and not in display_channel_create */
     display_channel_free_glz_drawables_to_free(display);
 
-    /* TODO: could use its own source */
-    video_stream_timeout(display);
-
     worker->event_timeout = INF_EVENT_WAIT;
     worker->was_blocked = FALSE;
     red_process_cursor(worker, &ring_is_empty);
diff --git a/server/video-stream.c b/server/video-stream.c
index ddb76a679..887433417 100644
--- a/server/video-stream.c
+++ b/server/video-stream.c
@@ -477,9 +477,9 @@  clear_vis_region:
     region_clear(&agent->vis_region);
 }
 
-static void detach_video_stream_gracefully(DisplayChannel *display,
-                                           VideoStream *stream,
-                                           Drawable *update_area_limit)
+void detach_video_stream_gracefully(DisplayChannel *display,
+                                    VideoStream *stream,
+                                    Drawable *update_area_limit)
 {
     DisplayChannelClient *dcc;
 
@@ -549,20 +549,3 @@  void video_stream_detach_and_stop(DisplayChannel *display)
     }
 }
 
-void video_stream_timeout(DisplayChannel *display)
-{
-    Ring *ring = &display->priv->streams;
-    RingItem *item;
-
-    red_time_t now = spice_get_monotonic_time_ns();
-    item = ring_get_head(ring);
-    while (item) {
-        VideoStream *stream = SPICE_CONTAINEROF(item, VideoStream, link);
-        item = ring_next(ring, item);
-        if (now >= (stream->last_time + RED_STREAM_TIMEOUT)) {
-            detach_video_stream_gracefully(display, stream, NULL);
-            video_stream_stop(stream);
-        }
-    }
-}
-
diff --git a/server/video-stream.h b/server/video-stream.h
index e5a35dd89..7e3fd8e70 100644
--- a/server/video-stream.h
+++ b/server/video-stream.h
@@ -130,7 +130,6 @@  struct VideoStream {
 
 void video_stream_stop(VideoStream *stream);
 void video_stream_unref(VideoStream *stream);
-void video_stream_timeout(DisplayChannel *display);
 void video_stream_detach_and_stop(DisplayChannel *display);
 void video_stream_detach_behind(DisplayChannel *display, QRegion *region,
                                 Drawable *drawable);
@@ -139,5 +138,8 @@  void video_stream_agent_unref(VideoStreamAgent *agent);
 void video_stream_agent_stop(VideoStreamAgent *agent);
 
 void video_stream_detach_drawable(VideoStream *stream);
+void detach_video_stream_gracefully(DisplayChannel *display,
+                                    VideoStream *stream,
+                                    Drawable *update_area_limit);
 
 #endif /* VIDEO_STREAM_H_ */