[Spice-devel] [PATCH linux vdagent v4 9/9] Make cursor work with mjpeg streaming
Lukáš Hrázký
lhrazky at redhat.com
Fri Jan 18 13:19:24 UTC 2019
I'd also squash this up to 7/9. It just reworks what was not a complete
solution.
Cheers,
Lukas
On Thu, 2019-01-17 at 16:14 -0600, Jonathon Jongsma wrote:
> 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);
> }
More information about the Spice-devel
mailing list