Re: [Spice-devel] [PATCH spice-server] dcc: Remove unused channel parameter

2018-03-11 Thread Frediano Ziglio 
> Hi,
> 
> 
> On 03/08/2018 12:35 PM, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio 
> > ---
> >   server/dcc.c | 7 ++-
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/server/dcc.c b/server/dcc.c
> > index d457989b..15b65978 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -676,8 +676,7 @@ void dcc_push_monitors_config(DisplayChannelClient
> > *dcc)
> >   red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc),
> >   >pipe_item);
> >   }
> >   
> > -static RedSurfaceDestroyItem *red_surface_destroy_item_new(RedChannel
> > *channel,
> > -   uint32_t
> > surface_id)
> > +static RedSurfaceDestroyItem *red_surface_destroy_item_new(uint32_t
> > surface_id)
> >   {
> >   RedSurfaceDestroyItem *destroy;
> >   
> > @@ -730,7 +729,6 @@ RedPipeItem *dcc_gl_draw_item_new(RedChannelClient
> > *rcc, void *data, int num)
> >   void dcc_destroy_surface(DisplayChannelClient *dcc, uint32_t surface_id)
> >   {
> >   DisplayChannel *display;
> > -RedChannel *channel;
> >   RedSurfaceDestroyItem *destroy;
> >   
> >   if (!dcc) {
> > @@ -738,7 +736,6 @@ void dcc_destroy_surface(DisplayChannelClient *dcc,
> > uint32_t surface_id)
> >   }
> >   
> >   display = DCC_TO_DC(dcc);
> > -channel = RED_CHANNEL(display);
> >   
> >   if
> >   
> > (common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
> >   ||
> >   !dcc->priv->surface_client_created[surface_id]) {
> > @@ -746,7 +743,7 @@ void dcc_destroy_surface(DisplayChannelClient *dcc,
> > uint32_t surface_id)
> >   }
> >   
> >   dcc->priv->surface_client_created[surface_id] = FALSE;
> > -destroy = red_surface_destroy_item_new(channel, surface_id);
> > +destroy = red_surface_destroy_item_new(surface_id);
> >   red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc),
> >   >pipe_item);
> >   }
> >   
> 
> 
> Isn't it some kind of convention for "OOP" in c?

No, if you refer to the this/self pointer this function is not a red_channel_*
which would mean a RedChannel method.


> Anyway i prefer it that way so i would ack if there is no objection.
> 
> Snir.
> 

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2018-03-11 Thread Snir Sheriber 

BTW

It is also recommended to apply this hack to avoid the ~400ms latency



diff --git a/src/channel-display.c b/src/channel-display.c
index 45fe38c..b41093b 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1534,6 +1534,7 @@ static void 
display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)

 }
 st->num_input_frames++;

+    op->multi_media_time = mmtime;
 latency = op->multi_media_time - mmtime;
 if (latency < 0) {
 CHANNEL_DEBUG(channel, "stream data too late by %u ms (ts: %u, 
mmtime: %u), dropping",





On 03/11/2018 11:44 AM, Snir Sheriber wrote:

This patch utilize the GstVideoOverlay interface when the
client is running under x window system and it receives a
full-screen stream that is decoded using gstreamer (gst =>
1.9.0)

Some notes
- It currently checks for full screen but probably a msg
   says it is streaming mode would make more sense (is it?
   i think frediano has a patch for that)
- Would it be possible to avoid spice rendering if it is not
   a full screen?
- Does rendering directly from the decoder (channel-element)
   breaks spice architecture?
- This currently seems stable and working well, it decreases
   cpu (no pixman-cairo stuff) mainly in high resolutions but
   needs to be tested with different plugins, windows...
- 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)

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

Thanks, Snir.


Snir Sheriber (1):
   Gstreamer: Use GstVideoOverlay if possible

  src/channel-display-gst.c | 71 +--
  src/channel-display.c | 54 +++
  src/spice-widget.c| 46 ++
  3 files changed, 156 insertions(+), 15 deletions(-)



___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2018-03-11 Thread Snir Sheriber 
Currently when gstreamer is used to decode a full-screen
stream sent from the server, the decoding frames are being
forced to RBGA format and pushed using appsink to be scaled
and rendered to screen.

Today most of the gstreamer sinks supports the GstVideoOverlay
interface which allows to render directly from the pipeline to
a window by a given windowing system id (i.e. xid). This patch
makes playbin to use this feature if possible.
---
 src/channel-display-gst.c | 71 +--
 src/channel-display.c | 54 +++
 src/spice-widget.c| 46 ++
 3 files changed, 156 insertions(+), 15 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index c6280d3..6fa19e0 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -265,7 +265,8 @@ static void free_pipeline(SpiceGstDecoder *decoder)
 
 gst_element_set_state(decoder->pipeline, GST_STATE_NULL);
 gst_object_unref(decoder->appsrc);
-gst_object_unref(decoder->appsink);
+if (decoder->appsink)
+gst_object_unref(decoder->appsink);
 gst_object_unref(decoder->pipeline);
 gst_object_unref(decoder->clock);
 decoder->pipeline = NULL;
@@ -308,7 +309,18 @@ static gboolean handle_pipeline_message(GstBus *bus, 
GstMessage *msg, gpointer v
 break;
 }
 default:
-/* not being handled */
+if (gst_is_video_overlay_prepare_window_handle_message(msg)) {
+GstVideoOverlay *overlay;
+guintptr handle = 0;
+
+g_object_get(decoder->base.stream->channel, "handle", , 
NULL);
+SPICE_DEBUG("prepare-window-handle msg received (handle: %lu)", 
handle);
+if (handle != 0) {
+overlay = GST_VIDEO_OVERLAY(GST_MESSAGE_SRC(msg));
+gst_video_overlay_set_window_handle(overlay, handle);
+}
+}
+/* else- not being handled */
 break;
 }
 return TRUE;
@@ -348,6 +360,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
 #if GST_CHECK_VERSION(1,9,0)
 GstElement *playbin, *sink;
 SpiceGstPlayFlags flags;
+guintptr handle;
 GstCaps *caps;
 
 playbin = gst_element_factory_make("playbin", "playbin");
@@ -356,26 +369,53 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
 return FALSE;
 }
 
-sink = gst_element_factory_make("appsink", "sink");
-if (sink == NULL) {
-spice_warning("error upon creation of 'appsink' element");
-gst_object_unref(playbin);
-return FALSE;
-}
-
-caps = gst_caps_from_string("video/x-raw,format=BGRx");
-g_object_set(sink,
+g_object_get(decoder->base.stream->channel, "handle", , NULL);
+SPICE_DEBUG("Creating Gstreamer pipline (handle for overlay %s)\n",
+handle ? "received" : "not received");
+if (handle == 0) {
+sink = gst_element_factory_make("appsink", "sink");
+if (sink == NULL) {
+spice_warning("error upon creation of 'appsink' element");
+gst_object_unref(playbin);
+return FALSE;
+}
+caps = gst_caps_from_string("video/x-raw,format=BGRx");
+g_object_set(sink,
  "caps", caps,
  "sync", FALSE,
  "drop", FALSE,
  NULL);
-gst_caps_unref(caps);
+gst_caps_unref(caps);
+g_object_set(playbin,
+ "video-sink", gst_object_ref(sink),
+ NULL);
+
+decoder->appsink = GST_APP_SINK(sink);
+} else {
+ /*
+  * handle has received, it means playbin will render directly into
+  * widget using the gstoverlay interface instead of app-sink.
+  * Also avoid using vaapisink if exist since vaapisink could be
+  * buggy when it is combined with playbin. changing its rank to
+  * none will make playbin to avoid of using it.
+  */
+GstRegistry *registry = NULL;
+GstPluginFeature *vaapisink = NULL;
+
+registry = gst_registry_get ();
+if (registry) {
+vaapisink = gst_registry_lookup_feature(registry, "vaapisink");
+}
+if (vaapisink) {
+gst_plugin_feature_set_rank(vaapisink, GST_RANK_NONE);
+gst_object_unref(vaapisink);
+}
+}
 
 g_signal_connect(playbin, "source-setup", G_CALLBACK(app_source_setup), 
decoder);
 
 g_object_set(playbin,
  "uri", "appsrc://",
- "video-sink", gst_object_ref(sink),
  NULL);
 
 /* Disable audio in playbin */
@@ -384,7 +424,6 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
 g_object_set(playbin, "flags", flags, NULL);
 
 g_warn_if_fail(decoder->appsrc == NULL);
-decoder->appsink = GST_APP_SINK(sink);
 decoder->pipeline = playbin;
 #else
 gchar *desc;
@@ -417,7 +456,9 @@ static gboolean

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

2018-03-11 Thread Snir Sheriber 
This patch utilize the GstVideoOverlay interface when the
client is running under x window system and it receives a
full-screen stream that is decoded using gstreamer (gst =>
1.9.0)

Some notes
- It currently checks for full screen but probably a msg
  says it is streaming mode would make more sense (is it?
  i think frediano has a patch for that)
- Would it be possible to avoid spice rendering if it is not
  a full screen?
- Does rendering directly from the decoder (channel-element)
  breaks spice architecture?
- This currently seems stable and working well, it decreases
  cpu (no pixman-cairo stuff) mainly in high resolutions but
  needs to be tested with different plugins, windows...
- 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)

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

Thanks, Snir.


Snir Sheriber (1):
  Gstreamer: Use GstVideoOverlay if possible

 src/channel-display-gst.c | 71 +--
 src/channel-display.c | 54 +++
 src/spice-widget.c| 46 ++
 3 files changed, 156 insertions(+), 15 deletions(-)

-- 
2.14.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] dcc: Remove unused channel parameter

2018-03-11 Thread Snir Sheriber 

Hi,


On 03/08/2018 12:35 PM, Frediano Ziglio wrote:

Signed-off-by: Frediano Ziglio 
---
  server/dcc.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index d457989b..15b65978 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -676,8 +676,7 @@ void dcc_push_monitors_config(DisplayChannelClient *dcc)
  red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), >pipe_item);
  }
  
-static RedSurfaceDestroyItem *red_surface_destroy_item_new(RedChannel *channel,

-   uint32_t surface_id)
+static RedSurfaceDestroyItem *red_surface_destroy_item_new(uint32_t surface_id)
  {
  RedSurfaceDestroyItem *destroy;
  
@@ -730,7 +729,6 @@ RedPipeItem *dcc_gl_draw_item_new(RedChannelClient *rcc, void *data, int num)

  void dcc_destroy_surface(DisplayChannelClient *dcc, uint32_t surface_id)
  {
  DisplayChannel *display;
-RedChannel *channel;
  RedSurfaceDestroyItem *destroy;
  
  if (!dcc) {

@@ -738,7 +736,6 @@ void dcc_destroy_surface(DisplayChannelClient *dcc, 
uint32_t surface_id)
  }
  
  display = DCC_TO_DC(dcc);

-channel = RED_CHANNEL(display);
  
  if (common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display)) ||

  !dcc->priv->surface_client_created[surface_id]) {
@@ -746,7 +743,7 @@ void dcc_destroy_surface(DisplayChannelClient *dcc, 
uint32_t surface_id)
  }
  
  dcc->priv->surface_client_created[surface_id] = FALSE;

-destroy = red_surface_destroy_item_new(channel, surface_id);
+destroy = red_surface_destroy_item_new(surface_id);
  red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), >pipe_item);
  }
  



Isn't it some kind of convention for "OOP" in c?
Anyway i prefer it that way so i would ack if there is no objection.

Snir.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel