[Spice-devel] [RFC spice-gtk v2 0/1] Direct rendering

[Frediano Ziglio]

> 
> Differences from v1
> -Recognize streaming mode by the streaming-mode surface flag

The propagation of this property (as English word, not gobject) is weird.
Seems that is a SpiceDisplay property but in reality is a surface propery
(which is stored in display_surface structure in spice-gtk), the previous
patch version stored the property on display_surface which seems more
correct.
Storing the "handle" (currently xid) in channel seems more of an hack
than the proper way, a channel in theory can have multiple primary
surfaces (which was a request to implement). Also the handle keep
to be set even if the streaming surface is reset (this can happen
for Virgl, although not implemented currently by the code). Similar,
what happen if the "Display" (in client terminology, the client window)
is closed? Does the xid is still valid? Is there a way to reduce the
CPU/GPU usage in this case (not strict requirement, mainly OT).

> -Modifying the streaming mode signal

Feels more confusing that previous version.
Looking at current code the code (master) calls stream_display_frame
which emit a "display-invalidate" signal, captured by SpiceDisplay
(OT: why SpiceDisplay is implemented in spice-widget.c and not
in a spice-display.c ??). I imagine all SpiceDisplay(s) attached
to the same channel will handle the update filtering when the
rectangle does not intersect.
Maybe a signal like that emitted from channel-display-gst.c to
get the xid handle would make more sense and would support multiple
monitors?

> -Applying patches from Frediano (sent on v1 thread)
> -Applying Uri's patch fixing a memory leak
> -This feature can forced to be disabled now by setting the
>  DISABLE_GSTVIDEOOVERLAY environment variable

Small note: I would avoid to call g_getenv every time but
cache the value.

> -Does not create a new drawing area
> 
> Some issues
> - Canvas is allocated although it's not always used

I won't consider this an issue, is not even a regression and
not strictly related to direct rendering.

> - Needs to be tested with different plugins, environments...

I tested with Intel card with and without vaapi drivers and
with Xorg and Wayland.
Maybe would be useful to get a matrix?

> - Not sure what is needed in order to make it to support
>   multi-monitor in the future.
> - Currently works only with x (xid is transferred from
>   spice-widget to spice-gst-decoder which sets the overlay)
> - There is no synchronization with audio! (decodes and
>    renders AFAP)
> 

On my tests the PTS is taken into account so seems to
sync audio and video... weird!

> 
> I'd be happy to hear more comments, ideas, patches :)
> 

The results with this patch are great! With drivers installed
I'm getting 10% CPU usage using full HD!

Hope to see this patch in master soon!

> Thanks, Snir.
> 
> 
> Snir Sheriber (1):
>   Gstreamer: Use GstVideoOverlay if possible
> 
>  src/channel-display-gst.c | 99
>  ++++++++++++++++++++++++++++++++++++++---------
>  src/channel-display.c     | 55 ++++++++++++++++++++++++++
>  src/channel-display.h     |  3 ++
>  src/spice-widget-priv.h   |  1 +
>  src/spice-widget.c        | 40 ++++++++++++++++++-
>  5 files changed, 179 insertions(+), 19 deletions(-)
> 

Frediano