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

[Jason Ekstrand]

Andrew,
Thanks for reviewing.  Comments follow.

On Mar 27, 2014 7:02 AM, "Andrew Wedgbury" <andrew.wedgbury at realvnc.com>
wrote:
>
> 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.

That's not a bad idea.  I'll look into it.

>
>
>> +     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.

Actually, it's more subtle than that.  Weston uses more than just the
cursor plane.  It can also throw entire windows into planes to avoid
unnecisary re-rendering.  In order to do a screen capture, we have to do a
full composite.  We could, in theory, only use a plane for the one cursor,
but I'm not sure whether or not that would work with screen sharing.  I
should definitely look into it for the case where the compositor is running
stand-alone on top of the VNC/RDP server.

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

Yes, I should.  Do I not have one?  I should add one to
shared_output_destroy.

Thanks for reviewing,
--Jason Ekstrand

>
>
>> +       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>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140327/25101b0d/attachment-0001.html>