> > 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel