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

[Frediano Ziglio]

> Hi,

> On 07/24/2018 06:47 PM, Marc-André Lureau wrote:

> > Hi
> 

> > On Sun, Mar 11, 2018 at 10:44 AM, Snir Sheriber <ssheribe at redhat.com>
> > wrote:
> 

> > > Currently when gstreamer is used to decode a full-screen
> > 
> 

> > > ...
> > 
> 

> > > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > 
> 
> > > index 1e7add4..73a77b7 100644
> > 
> 
> > > --- a/src/spice-widget.c
> > 
> 
> > > +++ b/src/spice-widget.c
> > 
> 
> > > @@ -612,6 +612,29 @@ G_GNUC_END_IGNORE_DEPRECATIONS
> > 
> 
> > > #endif
> > 
> 
> > > #endif
> > 
> 

> > > +static void
> > 
> 
> > > +gst_area_realize(GtkGLArea *area, gpointer user_data)
> > 
> 
> > > +{
> > 
> 
> > > +//TODO: needs rework, currently works only under X
> > 
> 
> > > +#ifdef GDK_WINDOWING_X11
> > 
> 
> > > +    SpiceDisplay *display = SPICE_DISPLAY(user_data);
> > 
> 
> > > +    SpiceDisplayPrivate *d = display->priv;
> > 
> 
> > > +    GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(display));
> > 
> 
> > > +
> > 
> 
> > > +    if (window) {
> > 
> 
> > > +#if GTK_CHECK_VERSION(2,18,0)
> > 
> 
> > > +        if (!gdk_window_ensure_native (window)) {
> > 
> 
> > > +            g_warning("Couldn't create native window needed for
> > > GstVideoOverlay!");
> > 
> 
> > > +            return;
> > 
> 
> > > +        }
> > 
> 
> > > +#endif
> > 
> 
> > > +        g_object_set(G_OBJECT (d->display),
> > 
> 
> > > +                    "handle", GDK_WINDOW_XID(window),
> > 
> 
> > > +                    NULL);
> > 
> 
> > > +    }
> > 
> 
> > > +#endif
> > 
> 
> > > +}
> > 
> 
> > > +
> > 
> 
> > > static void
> > 
> 
> > > drawing_area_realize(GtkWidget *area, gpointer user_data)
> > 
> 
> > > {
> > 
> 
> > > @@ -660,6 +683,13 @@ G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> > 
> 
> > > G_GNUC_END_IGNORE_DEPRECATIONS
> > 
> 
> > > #endif
> > 
> 
> > > #endif
> > 
> 
> > > +
> > 
> 
> > > +    area = gtk_drawing_area_new();
> > 
> 
> > > +    g_object_connect(area,
> > 
> 
> > > +                     "signal::realize", gst_area_realize, display,
> > 
> 
> > > +                     NULL);
> > 
> 
> > > +    gtk_stack_add_named(d->stack, area, "gst-area");
> > 
> 

> > Was there a good reason to use a seperate drawing area rather than the
> 
> > existing one?
> 

> There were some reasons for this, iirc there was an issue with the signals
> that was needed to be disabled to avoid interference with the drawings (i
> had a version where it uses the same drawing area but eventually it was
> dropped) and also at first i was trying both gl-area and drawing-area so it
> was
> easier to switch.
> (Also please notice that this patch is not the recent version which was
> pushed)

I think the "there were some reasons" is an indication that a comment in 
the code with the reason would be good. 

> > Is there a simple way to test the "streaming mode"? A test case like
> 
> > server/tests/test-display-streaming would be great.
> 

> Could be tested by using spice-streaming-agent that streams mjpeg to a client
> that was
> built with disabled builtin_mjpeg (so it will use the spice-gst-decoder)
> or
> By using spice-streaming-agent that streams h264/vp8 video stream using the
> gst-plugin
> which was recently sent to ML and it is still not pushed upstream.

> Currently I'm not aware to any similar\other test option that is available
> upstream.

> Snir.

Tests are always great. I don't think we really need all these system settings. 
But that would be the target of this/these tests? Run into a CI? Manually testing? 
I think gstreamer and video output are used here, the patch is supposed to optimize 
the rendering part and the full pipeline. Also depends on codec and plugins installed 
(although in a CI we can force to have some plugins installed, but still hard to get 
a display server... or a vncserver is enough?) 

Frediano 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180726/13079fb0/attachment.html>