[Spice-devel] [spice v2] display-channel: monitors_config: add 200ms delay

[Victor Toso]

From: Victor Toso <me at victortoso.com>

The device driver might take a few interactions to reconfigure all
displays but in each change it can send it down to spice-server. The
client is not interested in temporary states, only the final monitor
config can be helpful.

This patch adds a 200ms delay to wait for more changes from guest
device. Tested only with QXL in Fedora 29.

Simple example below, with 4 displays enabled but removing the display 1
(starting from 0), spice server receives:

 | 16:50:14.964: display-channel.c:173:monitors_config_unref: freeing monitors config
 | 16:50:14.964: display-channel.c:181:monitors_config_debug: monitors config count:3 max:4
 | 16:50:14.964: display-channel.c:185:monitors_config_debug: +0+0 960x997
 | 16:50:14.964: display-channel.c:185:monitors_config_debug: +960+0 1024x740
 | 16:50:14.964: display-channel.c:185:monitors_config_debug: +1984+0 1024x740

Sends to the client #1

 | 16:50:14.965: display-channel.c:173:monitors_config_unref: freeing monitors config
 | 16:50:14.965: display-channel.c:181:monitors_config_debug: monitors config count:3 max:4
 | 16:50:14.965: display-channel.c:185:monitors_config_debug: +0+0 960x997
 | 16:50:14.965: display-channel.c:185:monitors_config_debug: +0+0 0x0
 | 16:50:14.965: display-channel.c:185:monitors_config_debug: +1984+0 1024x740

Sends to the client #2

 | 16:50:14.966: red-worker.c:475:dev_create_primary_surface: trace
 | 16:50:14.967: display-channel.c:173:monitors_config_unref: freeing monitors config
 | 16:50:14.967: display-channel.c:181:monitors_config_debug: monitors config count:1 max:4
 | 16:50:14.967: display-channel.c:185:monitors_config_debug: +0+0 3008x997
 | 16:50:14.972: display-channel.c:173:monitors_config_unref: freeing monitors config
 | 16:50:14.972: display-channel.c:181:monitors_config_debug: monitors config count:3 max:4
 | 16:50:14.972: display-channel.c:185:monitors_config_debug: +0+0 960x997
 | 16:50:14.973: display-channel.c:185:monitors_config_debug: +0+0 0x0
 | 16:50:14.973: display-channel.c:185:monitors_config_debug: +960+0 1024x740

Sends to the client #3

 | 16:50:14.973: display-channel.c:2462:display_channel_update_monitors_config: Will update monitors in 200ms ~~
 | 16:50:14.975: display-channel.c:173:monitors_config_unref: freeing monitors config
 | 16:50:14.975: display-channel.c:181:monitors_config_debug: monitors config count:4 max:4
 | 16:50:14.975: display-channel.c:185:monitors_config_debug: +0+0 960x997
 | 16:50:14.975: display-channel.c:185:monitors_config_debug: +0+0 0x0
 | 16:50:14.975: display-channel.c:185:monitors_config_debug: +960+0 1024x740
 | 16:50:14.975: display-channel.c:185:monitors_config_debug: +1984+0 1024x740

Sends to the client #4 and final

With this patch, only the last one is sent.
Resolves: rhbz#1673072
Signed-off-by: Victor Toso <victortoso at redhat.com>
---

Changes v1->v2
* Using reds_core_timer_* api, which calls the timeout function from the
  right context (Frediano);
* Remove the SpiceTimer on finalize (Frediano)

 server/display-channel-private.h |  2 ++
 server/display-channel.c         | 27 +++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 58179531..848abe81 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -118,6 +118,8 @@ struct DisplayChannelPrivate
 
     int gl_draw_async_count;
 
+    SpiceTimer *monitors_config_update_timer;
+
 /* TODO: some day unify this, make it more runtime.. */
     stat_info_t add_stat;
     stat_info_t exclude_stat;
diff --git a/server/display-channel.c b/server/display-channel.c
index 9bb7fa44..a2f95956 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -89,6 +89,12 @@ display_channel_finalize(GObject *object)
     display_channel_destroy_surfaces(self);
     image_cache_reset(&self->priv->image_cache);
 
+    if (self->priv->monitors_config_update_timer) {
+        RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
+        reds_core_timer_remove(reds, self->priv->monitors_config_update_timer);
+        self->priv->monitors_config_update_timer = NULL;
+    }
+
     if (spice_extra_checks) {
         unsigned int count;
         _Drawable *drawable;
@@ -2249,6 +2255,8 @@ DisplayChannel* display_channel_new(RedsState *reds,
                            NULL);
     if (display) {
         display_channel_set_stream_video(display, stream_video);
+        display->priv->monitors_config_update_timer = reds_core_timer_add(reds,
+                (SpiceTimerFunc) display_channel_push_monitors_config, display);
     }
     return display;
 }
@@ -2435,6 +2443,11 @@ void display_channel_push_monitors_config(DisplayChannel *display)
 {
     DisplayChannelClient *dcc;
 
+    if (display->priv->monitors_config_update_timer) {
+        RedsState *reds = red_channel_get_server(RED_CHANNEL(display));
+        reds_core_timer_cancel(reds, display->priv->monitors_config_update_timer);
+    }
+
     FOREACH_DCC(display, dcc) {
         dcc_push_monitors_config(dcc);
     }
@@ -2444,14 +2457,24 @@ void display_channel_update_monitors_config(DisplayChannel *display,
                                             QXLMonitorsConfig *config,
                                             uint16_t count, uint16_t max_allowed)
 {
+    RedsState *reds = red_channel_get_server(RED_CHANNEL(display));
 
-    if (display->priv->monitors_config)
+    if (display->priv->monitors_config) {
         monitors_config_unref(display->priv->monitors_config);
+    }
 
     display->priv->monitors_config =
         monitors_config_new(config->heads, count, max_allowed);
 
-    display_channel_push_monitors_config(display);
+    if (!display->priv->monitors_config_update_timer) {
+        spice_warning("No timer for monitor_config updates");
+        display_channel_push_monitors_config(display);
+        return;
+    }
+
+    /* Cancel previous timer, if it was set */
+    reds_core_timer_cancel(reds, display->priv->monitors_config_update_timer);
+    reds_core_timer_start(reds, display->priv->monitors_config_update_timer, 200);
 }
 
 void display_channel_set_monitors_config_to_primary(DisplayChannel *display)
-- 
2.20.1