[Spice-devel] [PATCH linux vdagent v4 9/9] Make cursor work with mjpeg streaming

Jonathon Jongsma jjongsma at redhat.com
Thu Jan 17 22:14:28 UTC 2019


In the case where we have an mjpeg plugin running in the streaming
agent, we have two spice displays representing the same guest display.
In that scenario, the session agent may maintain the following guest
output mapping:
 spice channel 0 (QXL) => X output 0
 spice channel 1 (streaming-agent) => X output 0

While this is not necessarily a supported scenario, it would be nice if
the cursor input worked properly in this case. The root problem is that
when the session agent sends down the guest xorg resolutions to the
system daemon, it simply loops through the list of xorg displays, and
for each X display it looks up the first spice display ID associated
with it and sends that down to the daemon. In the scenario mentioned
above, since there is only a single X display configured (albeit
represented by two different spice displays), we would send down a
single display resolution to the system daemon:
 - { width=1280, height=1024, x=0, y=0, display_id=0 }

Notice that there is no entry for display_id=1. When the agent receives
a cursor input message for display channel 1, that message will get
passed to the systemn daemon, which will attempt to look up display_id 1
in order to convert the event coordinates to global coordinates. Finding
no entry for display_id=1, the mouse events do not work.

In this patch, when we want to send the guest resolutions down to the
system daemon, we still loop through the list of X outputs, but for each
output we also loop through the guest output mapping table and send a
resolution structure down to the daemon for each registered output
mapping.  This means that in the previously mentioned scenario, we would
send down the following information:
 - { width=1280, height=1024, x=0, y=0, display_id=0 }
 - { width=1280, height=1024, x=0, y=0, display_id=1 }

 This means that when the client sends a mouse event for display_id=1,
 the system daemon knows the coordinates of the guest display associated
 with that ID and can process the mouse event properly.
---
 src/vdagent/x11-randr.c | 106 ++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
index 283dbc8..fe48c46 100644
--- a/src/vdagent/x11-randr.c
+++ b/src/vdagent/x11-randr.c
@@ -688,35 +688,6 @@ static int config_size(int num_of_monitors)
                            num_of_monitors * sizeof(VDAgentMonConfig);
 }
 
-static int get_display_id_for_output_index(struct vdagent_x11 *x11, int output_index)
-{
-    // invalid output index
-    if (output_index >= x11->randr.res->noutput) {
-        syslog(LOG_WARNING, "Invalid output index %d (>%d)", output_index, x11->randr.res->noutput);
-        return -1;
-    }
-
-    if (g_hash_table_size(x11->guest_output_map) == 0) {
-        syslog(LOG_DEBUG, "No guest output map, using output index as display id");
-        return output_index;
-    }
-
-    int display_id = -1;
-    RROutput output_id = x11->randr.res->outputs[output_index];
-    GHashTableIter iter;
-    gpointer key, value;
-    g_hash_table_iter_init(&iter, x11->guest_output_map);
-    while (g_hash_table_iter_next(&iter, &key, &value)) {
-        gint64 *other_id = value;
-        if (*other_id == output_id) {
-            return GPOINTER_TO_INT(key);
-        }
-    }
-
-    syslog(LOG_WARNING, "Unable to find a display id for output index %d)", output_index);
-    return display_id;
-}
-
 // gets monitor information about the specified output index and returns true if there was no error
 static bool get_monitor_info_for_output_index(struct vdagent_x11 *x11, int output_index, int *x, int *y, int *width, int *height)
 {
@@ -1093,7 +1064,7 @@ exit:
 
 void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
 {
-    struct vdagentd_guest_xorg_resolution *res = NULL;
+    GArray *res_array = g_array_new(FALSE, FALSE, sizeof(struct vdagentd_guest_xorg_resolution));
     int i, width = 0, height = 0, screen_count = 0;
 
     if (x11->has_xrandr) {
@@ -1101,17 +1072,39 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
             update_randr_res(x11, 0);
 
         screen_count = x11->randr.res->noutput;
-        res = g_new(struct vdagentd_guest_xorg_resolution, screen_count);
 
         for (i = 0; i < screen_count; i++) {
-            struct vdagentd_guest_xorg_resolution *curr = &res[i];
-            if (!get_monitor_info_for_output_index(x11, i, &curr->x, &curr->y,
-                                                   &curr->width, &curr->height))
+            struct vdagentd_guest_xorg_resolution curr;
+            if (!get_monitor_info_for_output_index(x11, i, &curr.x, &curr.y,
+                        &curr.width, &curr.height))
             {
-                g_free(res);
+                g_array_free(res_array, TRUE);
                 goto no_info;
             }
-            curr->display_id = get_display_id_for_output_index(x11, i);
+            if (g_hash_table_size(x11->guest_output_map) == 0) {
+                syslog(LOG_DEBUG, "No guest output map, using output index as display id");
+                curr.display_id = i;
+                g_array_append_val(res_array, curr);
+            } else {
+                // There may be multiple spice outputs representing a single guest output. Send them
+                // all down.
+                RROutput output_id = x11->randr.res->outputs[i];
+                GHashTableIter iter;
+                gpointer key, value;
+                g_hash_table_iter_init(&iter, x11->guest_output_map);
+                bool found = false;
+                while (g_hash_table_iter_next(&iter, &key, &value)) {
+                    gint64 *other_id = value;
+                    if (*other_id == output_id) {
+                        curr.display_id = GPOINTER_TO_INT(key);
+                        g_array_append_val(res_array, curr);
+                        found = true;
+                    }
+                }
+                if (!found) {
+                    syslog(LOG_WARNING, "Unable to find a display id for output index %d)", i);
+                }
+            }
         }
         width  = x11->width[0];
         height = x11->height[0];
@@ -1121,54 +1114,61 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
         screen_info = XineramaQueryScreens(x11->display, &screen_count);
         if (!screen_info)
             goto no_info;
-        res = g_new(struct vdagentd_guest_xorg_resolution, screen_count);
+        g_array_set_size(res_array, screen_count);
         for (i = 0; i < screen_count; i++) {
             if (screen_info[i].screen_number >= screen_count) {
                 syslog(LOG_ERR, "Invalid screen number in xinerama screen info (%d >= %d)",
                        screen_info[i].screen_number, screen_count);
                 XFree(screen_info);
-                g_free(res);
+                g_array_free(res_array, true);
                 return;
             }
-            res[screen_info[i].screen_number].width = screen_info[i].width;
-            res[screen_info[i].screen_number].height = screen_info[i].height;
-            res[screen_info[i].screen_number].x = screen_info[i].x_org;
-            res[screen_info[i].screen_number].y = screen_info[i].y_org;
+            struct vdagentd_guest_xorg_resolution *curr = &g_array_index(res_array,
+                                                                         struct vdagentd_guest_xorg_resolution,
+                                                                         screen_info[i].screen_number);
+            curr->width = screen_info[i].width;
+            curr->height = screen_info[i].height;
+            curr->x = screen_info[i].x_org;
+            curr->y = screen_info[i].y_org;
         }
         XFree(screen_info);
         width  = x11->width[0];
         height = x11->height[0];
     } else {
 no_info:
-        screen_count = x11->screen_count;
-        res = g_new(struct vdagentd_guest_xorg_resolution, screen_count);
         for (i = 0; i < screen_count; i++) {
-            res[i].width  = x11->width[i];
-            res[i].height = x11->height[i];
+            struct vdagentd_guest_xorg_resolution res;
+            res.width  = x11->width[i];
+            res.height = x11->height[i];
             /* No way to get screen coordinates, assume rtl order */
-            res[i].x = width;
-            res[i].y = 0;
+            res.x = width;
+            res.y = 0;
             width += x11->width[i];
             if (x11->height[i] > height)
                 height = x11->height[i];
+            g_array_append_val(res_array, res);
         }
     }
 
     if (screen_count == 0) {
         syslog(LOG_DEBUG, "Screen count is zero, are we on wayland?");
-        g_free(res);
+        g_array_free(res_array, TRUE);
         return;
     }
 
     if (x11->debug) {
         syslog(LOG_DEBUG, "Sending guest screen resolutions to vdagentd:");
-        for (i = 0; i < screen_count; i++) {
-            syslog(LOG_DEBUG, "   screen %d %dx%d%+d%+d, id=%d", i,
+        if (res_array->len > screen_count) {
+            syslog(LOG_DEBUG, "(NOTE: list may contain overlapping areas when multiple spice displays show the same guest output)");
+        }
+        for (i = 0; i < res_array->len; i++) {
+            struct vdagentd_guest_xorg_resolution *res = (struct vdagentd_guest_xorg_resolution*)res_array->data;
+            syslog(LOG_DEBUG, "   screen %d %dx%d%+d%+d, display_id=%d", i,
                    res[i].width, res[i].height, res[i].x, res[i].y, res[i].display_id);
         }
     }
 
     udscs_write(x11->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height,
-                (uint8_t *)res, screen_count * sizeof(*res));
-    g_free(res);
+                (uint8_t *)res_array->data, res_array->len * sizeof(struct vdagentd_guest_xorg_resolution));
+    g_array_free(res_array, TRUE);
 }
-- 
2.17.2



More information about the Spice-devel mailing list