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

[Victor Toso]

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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel