[Spice-devel] [RFC spice-gtk v3 1/1] Gstreamer: Use GstVideoOverlay if possible

[Christophe Fergeau]

On Wed, May 16, 2018 at 10:23:01AM +0300, Snir Sheriber wrote:
> > > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > > index 73db593..251b29c 100644
> > > --- a/src/spice-widget.c
> > > +++ b/src/spice-widget.c
> > > +static void* prepare_streaming_mode(SpiceChannel *channel, bool streaming_mode, gpointer data)
> > > +{
> > > +#ifdef GDK_WINDOWING_X11
> > > +    SpiceDisplay *display = data;
> > > +    SpiceDisplayPrivate *d = display->priv;
> > > +
> > > +    /* GstVideoOverlay will currently be used only under x */
> > > +    if (!g_getenv("DISABLE_GSTVIDEOOVERLAY") &&
> > > +        streaming_mode &&
> > > +        GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
> > > +        GdkWindow *window;
> > > +
> > > +        window = gtk_widget_get_window(GTK_WIDGET(display));
> > > +        if (window) {
> > > +#if GTK_CHECK_VERSION(2,18,0)
> > > +            if (gdk_window_ensure_native (window))
> > > +#endif
> > > +            {
> > > +                gtk_stack_set_visible_child_name(d->stack, "gst-area");
> > > +                return (void*)GDK_WINDOW_XID(window);
> > > +            }
> > > +        }
> > > +    }
> > > +#endif
> > > +    return NULL;
> > > +}
> > > +
> > >   static void invalidate(SpiceChannel *channel,
> > >                          gint x, gint y, gint w, gint h, gpointer data)
> > >   {
> > > @@ -2962,6 +2993,8 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
> > >           spice_g_signal_connect_object(channel, "notify::monitors",
> > >                                         G_CALLBACK(spice_display_widget_update_monitor_area),
> > >                                         display, G_CONNECT_AFTER | G_CONNECT_SWAPPED);
> > > +        spice_g_signal_connect_object(channel, "streaming-mode",
> > > +                                      G_CALLBACK(prepare_streaming_mode), display, G_CONNECT_AFTER);
> > Rather than having this signal which is used by ChannelDisplay to call a
> > method on a GtkWidget it should not have any access to, have you
> > considered adding code to spice-widget.c::realize() and
> > spice-widget.c::unrealize() which would call
> > channel_display_set_window_handle(display->priv->display, handle);
> > ?
> 
> Sound a bit similar to a previous version I've tried, iiuc your
> suggestion is to make widget call a function in channel_display
> to set the handle (are we doing something like that already?).

Yes, indeed, this is similar to what you were doing in earlier
iterations.

> I personally really like the current flow of the request for handle
> using the signal and getting it as a response, avoiding of setting
> and getting an handle from different components.

Using signals as some generic way of calling a getter on some unknown
class is rather unusual, and feels like something you should not really
be using signals for.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180516/b70202bd/attachment.sig>