[PATCH fullscreen-shell v5 18/18] Add a screen sharing plugin

[Andrew Wedgbury]

Hi Jason,

A few comments I have from trying out your fullscreen-shell changes:

On Tue, 18 Mar 2014, Jason Ekstrand wrote:

> +static struct ss_seat *
> +ss_seat_create(struct shared_output *so, uint32_t id)
> +{
> +     struct ss_seat *seat;
> +
> +     seat = zalloc(sizeof *seat);
> +     if (seat == NULL)
> +             return NULL;
> +
> +     weston_seat_init(&seat->base, so->output->compositor, "default");

It would be great if we could transfer the seat name from the parent seat. 
In the case of VNC or RDP this could identify the remote user.

> +     seat->output = so;
> +     seat->parent.seat = wl_registry_bind(so->parent.registry, id,
> +                                          &wl_seat_interface, 1);
> +     wl_list_insert(so->seat_list.prev, &seat->link);
> +
> +     wl_seat_add_listener(seat->parent.seat, &ss_seat_listener, seat);
> +     wl_seat_set_user_data(seat->parent.seat, seat);
> +
> +     return seat;
> +}

> +static struct shared_output *
> +shared_output_create(struct weston_output *output, int parent_fd)
> +{
> +	struct shared_output *so;
> +	struct wl_event_loop *loop;
> +	struct ss_seat *seat;
> +	int epoll_fd;
> +
> +	so = zalloc(sizeof *so);
> +	if (so == NULL)
> +		goto err_close;
> +
> +	wl_list_init(&so->seat_list);
> + 
> +	so->parent.display = wl_display_connect_to_fd(parent_fd);
> +	if (!so->parent.display)
> +		goto err_alloc;
> + 
> +	so->parent.registry = wl_display_get_registry(so->parent.display);
> +	if (!so->parent.registry)
> +		goto err_display;
> +	wl_registry_add_listener(so->parent.registry,
> +				 &registry_listener, so);
> +	wl_display_roundtrip(so->parent.display);
> +	if (so->parent.shm == NULL) {
> +		weston_log("Screen share failed: No wl_shm found\n");
> +		goto err_display;
> +	}
> +	if (so->parent.fshell == NULL) {
> +		weston_log("Screen share failed: "
> +			   "Parent does not support wl_fullscreen_shell\n");
> +		goto err_display;
> +	}
> +	if (so->parent.compositor == NULL) {
> +		weston_log("Screen share failed: No wl_compositor found\n");
> +		goto err_display;
> +	}
> +
> +	/* Get SHM formats */
> +	wl_display_roundtrip(so->parent.display);
> +	if (!(so->parent.shm_formats & (1 << WL_SHM_FORMAT_XRGB8888))) {
> +		weston_log("Screen share failed: "
> +			   "WL_SHM_FORMAT_XRGB8888 not available\n");
> +		goto err_display;
> +	}
> +
> +	so->parent.surface =
> +		wl_compositor_create_surface(so->parent.compositor);
> +	if (!so->parent.surface) {
> +		weston_log("Screen share failed: %m");
> +		goto err_display;
> +	}
> +
> +	so->parent.mode_feedback =
> +		_wl_fullscreen_shell_present_surface_for_mode(so->parent.fshell,
> +							      so->parent.surface,
> +							      so->parent.output,
> +							      output->current_mode->refresh);
> +	if (!so->parent.mode_feedback) {
> +		weston_log("Screen share failed: %m");
> +		goto err_display;
> +	}
> +	_wl_fullscreen_shell_mode_feedback_add_listener(so->parent.mode_feedback,
> +						        &mode_feedback_listener,
> +						        so);
> +
> +	loop = wl_display_get_event_loop(output->compositor->wl_display);
> +
> +	epoll_fd = wl_display_get_fd(so->parent.display);
> +	so->event_source =
> +		wl_event_loop_add_fd(loop, epoll_fd, WL_EVENT_READABLE,
> +				     shared_output_handle_event, so);
> +	if (!so->event_source) {
> +		weston_log("Screen share failed: %m");
> +		goto err_display;
> +	}
> + 
> +	/* Ok, everything's created.  We should be good to go */
> +	wl_list_init(&so->shm.buffers);
> +	wl_list_init(&so->shm.free_buffers);
> +
> +	so->output = output;
> +	so->output_destroyed.notify = output_destroyed;
> +	wl_signal_add(&so->output->destroy_signal, &so->output_destroyed);
> +
> +	so->frame_listener.notify = shared_output_repainted;
> +	wl_signal_add(&output->frame_signal, &so->frame_listener);
> +	output->disable_planes++;

I think you should only disable planes if the fullscreen_shell doesn't 
have the cursor_plane capability. In that case, there also needs to be a 
way to update the pointer surface on the parent seat. Although I think 
the difficulty may be in detecting when the pointer image changes - at 
least I remember that being a problem in my previous attempt at doing 
this.

I guess you also need a corresponding disable_planes-- when screen sharing
stops?

> +	weston_output_damage(output);
> + 
> +	return so;
> +
> +err_display:
> +	wl_list_for_each(seat, &so->seat_list, link)
> +		ss_seat_destroy(seat);
> +	wl_display_disconnect(so->parent.display);
> +err_alloc:
> +	free(so);
> +err_close:
> +	close(parent_fd);
> +	return NULL;
> +}

---
Andrew Wedgbury <andrew.wedgbury at realvnc.com>