[Spice-devel,client,v2,2/3] streaming: Send a special stream report to signal streaming errors

Submitted by Francois Gouget on Aug. 18, 2016, 3:57 p.m.

Details

Message ID 1f46245ff13e721f93d6cc11cabbcce740c0df88.1471535534.git.fgouget@free.fr
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Francois Gouget Aug. 18, 2016, 3:57 p.m.
Servers that recognize this special report then stop streaming (sending
regular screen updates instead) while older ones essentially ignore it.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
---

This round sends the error report as soon as the stream is created, and 
whenever a message with an invalid stream id is received.

 src/channel-display.c | 68 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 30 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/channel-display.c b/src/channel-display.c
index 54df9c0..68f0a2a 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1077,6 +1077,38 @@  static void display_update_stream_region(display_stream *st)
     }
 }
 
+static display_stream *get_stream_by_id(SpiceChannel *channel, uint32_t id)
+{
+    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
+
+    if (c != NULL && c->streams != NULL && id < c->nstreams &&
+        c->streams[id] != NULL) {
+        return c->streams[id];
+    }
+
+    if (spice_channel_test_capability(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION)) {
+        SpiceMsgcDisplayStreamReport report;
+        SpiceMsgOut *msg;
+
+        /* Send a special stream report to indicate there is no such stream */
+        spice_printerr("notify the server that stream %u does not exist", id);
+        report.stream_id = id;
+        report.unique_id = 0;
+        report.start_frame_mm_time = 0;
+        report.end_frame_mm_time = 0;
+        report.num_frames = 0;
+        report.num_drops = UINT_MAX;
+        report.last_frame_delay = 0;
+        report.audio_delay = 0;
+
+        msg = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_DISPLAY_STREAM_REPORT);
+        msg->marshallers->msgc_display_stream_report(msg->marshaller, &report);
+        spice_msg_out_send(msg);
+    }
+
+    return NULL;
+}
+
 /* coroutine context */
 static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
 {
@@ -1127,6 +1159,8 @@  static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
     if (st->video_decoder == NULL) {
         spice_printerr("could not create a video decoder for codec %u", op->codec_type);
         destroy_stream(channel, op->id);
+        /* Trigger the error reporting code */
+        get_stream_by_id(channel, op->id);
     }
 }
 
@@ -1224,17 +1258,10 @@  void stream_display_frame(display_stream *st, SpiceMsgIn *frame_msg,
 static void display_update_stream_report(SpiceChannel *channel, uint32_t stream_id,
                                          uint32_t frame_time, int32_t latency)
 {
-    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
-    display_stream *st;
+    display_stream *st = get_stream_by_id(channel, stream_id);
     guint64 now;
 
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > stream_id);
-
-    st = c->streams[stream_id];
     g_return_if_fail(st != NULL);
-
     if (!st->report_is_active) {
         return;
     }
@@ -1347,15 +1374,10 @@  static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
 {
     SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
     SpiceStreamDataHeader *op = spice_msg_in_parsed(in);
-    display_stream *st;
+    display_stream *st = get_stream_by_id(channel, op->id);
     guint32 mmtime;
     int32_t latency;
 
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > op->id);
-
-    st =  c->streams[op->id];
     g_return_if_fail(st != NULL);
     mmtime = stream_get_time(st);
 
@@ -1415,17 +1437,10 @@  static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
 /* coroutine context */
 static void display_handle_stream_clip(SpiceChannel *channel, SpiceMsgIn *in)
 {
-    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
     SpiceMsgDisplayStreamClip *op = spice_msg_in_parsed(in);
-    display_stream *st;
-
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > op->id);
+    display_stream *st = get_stream_by_id(channel, op->id);
 
-    st = c->streams[op->id];
     g_return_if_fail(st != NULL);
-
     if (st->msg_clip) {
         spice_msg_in_unref(st->msg_clip);
     }
@@ -1524,17 +1539,10 @@  static void display_handle_stream_destroy_all(SpiceChannel *channel, SpiceMsgIn
 /* coroutine context */
 static void display_handle_stream_activate_report(SpiceChannel *channel, SpiceMsgIn *in)
 {
-    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
     SpiceMsgDisplayStreamActivateReport *op = spice_msg_in_parsed(in);
-    display_stream *st;
+    display_stream *st = get_stream_by_id(channel, op->stream_id);
 
-    g_return_if_fail(c != NULL);
-    g_return_if_fail(c->streams != NULL);
-    g_return_if_fail(c->nstreams > op->stream_id);
-
-    st = c->streams[op->stream_id];
     g_return_if_fail(st != NULL);
-
     st->report_is_active = TRUE;
     st->report_id = op->unique_id;
     st->report_max_window = op->max_window_size;

Comments

Hi,

I plan to give closer look soon, just one note below

On Thu, Aug 18, 2016 at 05:57:58PM +0200, Francois Gouget wrote:
> Servers that recognize this special report then stop streaming (sending
> regular screen updates instead) while older ones essentially ignore it.
> 
> Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> ---
> 
> This round sends the error report as soon as the stream is created, and 
> whenever a message with an invalid stream id is received.
> 
>  src/channel-display.c | 68 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 54df9c0..68f0a2a 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -1077,6 +1077,38 @@ static void display_update_stream_region(display_stream *st)
>      }
>  }
>  
> +static display_stream *get_stream_by_id(SpiceChannel *channel, uint32_t id)
> +{
> +    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> +
> +    if (c != NULL && c->streams != NULL && id < c->nstreams &&
> +        c->streams[id] != NULL) {
> +        return c->streams[id];
> +    }
> +
> +    if (spice_channel_test_capability(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION)) {

SPICE_DISPLAY_CAP_STREAM_REPORT instead

Cheers,
  toso

> +        SpiceMsgcDisplayStreamReport report;
> +        SpiceMsgOut *msg;
> +
> +        /* Send a special stream report to indicate there is no such stream */
> +        spice_printerr("notify the server that stream %u does not exist", id);
> +        report.stream_id = id;
> +        report.unique_id = 0;
> +        report.start_frame_mm_time = 0;
> +        report.end_frame_mm_time = 0;
> +        report.num_frames = 0;
> +        report.num_drops = UINT_MAX;
> +        report.last_frame_delay = 0;
> +        report.audio_delay = 0;
> +
> +        msg = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_DISPLAY_STREAM_REPORT);
> +        msg->marshallers->msgc_display_stream_report(msg->marshaller, &report);
> +        spice_msg_out_send(msg);
> +    }
> +
> +    return NULL;
> +}
> +
>  /* coroutine context */
>  static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
>  {
> @@ -1127,6 +1159,8 @@ static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
>      if (st->video_decoder == NULL) {
>          spice_printerr("could not create a video decoder for codec %u", op->codec_type);
>          destroy_stream(channel, op->id);
> +        /* Trigger the error reporting code */
> +        get_stream_by_id(channel, op->id);
>      }
>  }
>  
> @@ -1224,17 +1258,10 @@ void stream_display_frame(display_stream *st, SpiceMsgIn *frame_msg,
>  static void display_update_stream_report(SpiceChannel *channel, uint32_t stream_id,
>                                           uint32_t frame_time, int32_t latency)
>  {
> -    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> -    display_stream *st;
> +    display_stream *st = get_stream_by_id(channel, stream_id);
>      guint64 now;
>  
> -    g_return_if_fail(c != NULL);
> -    g_return_if_fail(c->streams != NULL);
> -    g_return_if_fail(c->nstreams > stream_id);
> -
> -    st = c->streams[stream_id];
>      g_return_if_fail(st != NULL);
> -
>      if (!st->report_is_active) {
>          return;
>      }
> @@ -1347,15 +1374,10 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
>  {
>      SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
>      SpiceStreamDataHeader *op = spice_msg_in_parsed(in);
> -    display_stream *st;
> +    display_stream *st = get_stream_by_id(channel, op->id);
>      guint32 mmtime;
>      int32_t latency;
>  
> -    g_return_if_fail(c != NULL);
> -    g_return_if_fail(c->streams != NULL);
> -    g_return_if_fail(c->nstreams > op->id);
> -
> -    st =  c->streams[op->id];
>      g_return_if_fail(st != NULL);
>      mmtime = stream_get_time(st);
>  
> @@ -1415,17 +1437,10 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
>  /* coroutine context */
>  static void display_handle_stream_clip(SpiceChannel *channel, SpiceMsgIn *in)
>  {
> -    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
>      SpiceMsgDisplayStreamClip *op = spice_msg_in_parsed(in);
> -    display_stream *st;
> -
> -    g_return_if_fail(c != NULL);
> -    g_return_if_fail(c->streams != NULL);
> -    g_return_if_fail(c->nstreams > op->id);
> +    display_stream *st = get_stream_by_id(channel, op->id);
>  
> -    st = c->streams[op->id];
>      g_return_if_fail(st != NULL);
> -
>      if (st->msg_clip) {
>          spice_msg_in_unref(st->msg_clip);
>      }
> @@ -1524,17 +1539,10 @@ static void display_handle_stream_destroy_all(SpiceChannel *channel, SpiceMsgIn
>  /* coroutine context */
>  static void display_handle_stream_activate_report(SpiceChannel *channel, SpiceMsgIn *in)
>  {
> -    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
>      SpiceMsgDisplayStreamActivateReport *op = spice_msg_in_parsed(in);
> -    display_stream *st;
> +    display_stream *st = get_stream_by_id(channel, op->stream_id);
>  
> -    g_return_if_fail(c != NULL);
> -    g_return_if_fail(c->streams != NULL);
> -    g_return_if_fail(c->nstreams > op->stream_id);
> -
> -    st = c->streams[op->stream_id];
>      g_return_if_fail(st != NULL);
> -
>      st->report_is_active = TRUE;
>      st->report_id = op->unique_id;
>      st->report_max_window = op->max_window_size;
> -- 
> 2.8.1
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel