[Spice-devel] [PATCH spice-gtk 4/8] channel-display: video stream quality report

Alon Levy alevy at redhat.com
Wed Apr 17 08:24:11 PDT 2013


> handle MSG_STREAM_ACTIVIATE_REPORT and send MSGC_STREAM_REPORT
> periodically, or when the playback is out of sync.

ACK with one comment

> ---
>  gtk/channel-display-priv.h |  10 ++++
>  gtk/channel-display.c      | 116
>  ++++++++++++++++++++++++++++++++++++++++-----
>  spice-common               |   2 +-
>  3 files changed, 114 insertions(+), 14 deletions(-)
> 
> diff --git a/gtk/channel-display-priv.h b/gtk/channel-display-priv.h
> index f57dc6e..49f82fe 100644
> --- a/gtk/channel-display-priv.h
> +++ b/gtk/channel-display-priv.h
> @@ -88,6 +88,16 @@ typedef struct display_stream {
>      GArray               *drops_seqs_stats_arr;
>      uint32_t             num_drops_seqs;
>  
> +    /* playback quality report to server */
> +    gboolean report_is_active;
> +    uint32_t report_id;
> +    uint32_t report_max_window;
> +    uint32_t report_timeout;
> +    uint64_t report_start_time;
> +    uint32_t report_start_frame_time;
> +    uint32_t report_num_frames;
> +    uint32_t report_num_drops;
> +    uint32_t report_drops_seq_len;
>  } display_stream;
>  
>  void stream_get_dimensions(display_stream *st, int *width, int *height);
> diff --git a/gtk/channel-display.c b/gtk/channel-display.c
> index ab4d3bd..efb9aec 100644
> --- a/gtk/channel-display.c
> +++ b/gtk/channel-display.c
> @@ -687,6 +687,7 @@ static void
> spice_display_channel_reset_capabilities(SpiceChannel *channel)
>      spice_channel_set_capability(SPICE_CHANNEL(channel),
>      SPICE_DISPLAY_CAP_MONITORS_CONFIG);
>      spice_channel_set_capability(SPICE_CHANNEL(channel),
>      SPICE_DISPLAY_CAP_COMPOSITE);
>      spice_channel_set_capability(SPICE_CHANNEL(channel),
>      SPICE_DISPLAY_CAP_A8_SURFACE);
> +    spice_channel_set_capability(SPICE_CHANNEL(channel),
> SPICE_DISPLAY_CAP_STREAM_REPORT);
>  }
>  
>  static void spice_display_channel_init(SpiceDisplayChannel *channel)
> @@ -1238,6 +1239,65 @@ static gboolean display_stream_render(display_stream
> *st)
>      return FALSE;
>  }
>  
> +#define STREAM_REPORT_DROP_SEQ_LEN_LIMIT 3 /* after a sequence of 3 drops,
> push a report to the server, even
> +                                              if the report window is bigger
> */

Put the comment before and then you can keep the 80 chars width (not sure if it is actually in the spice-gtk coding standard but it looks better).

> +
> +static void display_update_stream_report(SpiceDisplayChannel *channel,
> uint32_t stream_id,
> +                                         uint32_t frame_time, int32_t
> latency)
> +{
> +    display_stream *st = channel->priv->streams[stream_id];
> +    guint64 now;
> +
> +    if (!st->report_is_active) {
> +        return;
> +    }
> +    now = g_get_monotonic_time();
> +
> +    if (st->report_num_frames == 0) {
> +        st->report_start_frame_time = frame_time;
> +        st->report_start_time = now;
> +    }
> +    st->report_num_frames++;
> +
> +    if (latency < 0) { // drop
> +        st->report_num_drops++;
> +        st->report_drops_seq_len++;
> +    } else {
> +        st->report_drops_seq_len = 0;
> +    }
> +
> +    if (st->report_num_frames >= st->report_max_window ||
> +        now - st->report_start_time >= st->report_timeout ||
> +        st->report_drops_seq_len >= STREAM_REPORT_DROP_SEQ_LEN_LIMIT) {
> +        SpiceMsgcDisplayStreamReport report;
> +        SpiceSession *session =
> spice_channel_get_session(SPICE_CHANNEL(channel));
> +        SpiceMsgOut *msg;
> +
> +        report.stream_id = stream_id;
> +        report.unique_id = st->report_id;
> +        report.start_frame_mm_time = st->report_start_frame_time;
> +        report.end_frame_mm_time = frame_time;
> +        report.num_frames = st->report_num_frames;
> +        report.num_drops = st-> report_num_drops;
> +        report.last_frame_delay = latency;
> +        if (spice_session_is_playback_active(session)) {
> +            report.audio_delay =
> spice_session_get_playback_latency(session);
> +        } else {
> +            report.audio_delay = UINT_MAX;
> +        }
> +
> +        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);
> +
> +        st->report_start_time = 0;
> +        st->report_start_frame_time = 0;
> +        st->report_num_frames = 0;
> +        st->report_num_drops = 0;
> +        st->report_drops_seq_len = 0;
> +    }
> +}
> +
>  /* coroutine context */
>  static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn
>  *in)
>  {
> @@ -1245,6 +1305,7 @@ static void display_handle_stream_data(SpiceChannel
> *channel, SpiceMsgIn *in)
>      SpiceStreamDataHeader *op = spice_msg_in_parsed(in);
>      display_stream *st;
>      guint32 mmtime;
> +    int32_t latency;
>  
>      g_return_if_fail(c != NULL);
>      g_return_if_fail(c->streams != NULL);
> @@ -1266,7 +1327,9 @@ static void display_handle_stream_data(SpiceChannel
> *channel, SpiceMsgIn *in)
>          st->first_frame_mm_time = op->multi_media_time;
>      }
>      st->num_input_frames++;
> -    if (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), dropin",
>                        mmtime - op->multi_media_time, op->multi_media_time,
>                        mmtime);
>          st->arrive_late_time += mmtime - op->multi_media_time;
> @@ -1275,19 +1338,20 @@ static void display_handle_stream_data(SpiceChannel
> *channel, SpiceMsgIn *in)
>              st->cur_drops_seq_stats.start_mm_time = op->multi_media_time;
>          }
>          st->cur_drops_seq_stats.len++;
> -        return;
> -    }
> -
> -    spice_msg_in_ref(in);
> -    g_queue_push_tail(st->msgq, in);
> -    display_stream_schedule(st);
> -    if (st->cur_drops_seq_stats.len) {
> -        st->cur_drops_seq_stats.duration = op->multi_media_time -
> -
> st->cur_drops_seq_stats.start_mm_time;
> -        g_array_append_val(st->drops_seqs_stats_arr,
> st->cur_drops_seq_stats);
> -        memset(&st->cur_drops_seq_stats, 0,
> sizeof(st->cur_drops_seq_stats));
> -        st->num_drops_seqs++;
> +    } else {
> +        spice_msg_in_ref(in);
> +        g_queue_push_tail(st->msgq, in);
> +        display_stream_schedule(st);
> +        if (st->cur_drops_seq_stats.len) {
> +            st->cur_drops_seq_stats.duration = op->multi_media_time -
> +
> st->cur_drops_seq_stats.start_mm_time;
> +            g_array_append_val(st->drops_seqs_stats_arr,
> st->cur_drops_seq_stats);
> +            memset(&st->cur_drops_seq_stats, 0,
> sizeof(st->cur_drops_seq_stats));
> +            st->num_drops_seqs++;
> +        }
>      }
> +    display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op->id,
> +                                 op->multi_media_time, latency);
>  }
>  
>  /* coroutine context */
> @@ -1406,6 +1470,31 @@ static void
> display_handle_stream_destroy_all(SpiceChannel *channel, SpiceMsgIn
>      clear_streams(channel);
>  }
>  
> +/* 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;
> +
> +    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;
> +    st->report_timeout = op->timeout_ms * 1000;
> +    st->report_start_time = 0;
> +    st->report_start_frame_time = 0;
> +    st->report_num_frames = 0;
> +    st->report_num_drops = 0;
> +    st->report_drops_seq_len = 0;
> +}
> +
>  /* ------------------------------------------------------------------ */
>  
>  /* coroutine context */
> @@ -1627,6 +1716,7 @@ static const spice_msg_handler display_handlers[] = {
>      [ SPICE_MSG_DISPLAY_STREAM_DESTROY ]     =
>      display_handle_stream_destroy,
>      [ SPICE_MSG_DISPLAY_STREAM_DESTROY_ALL ] =
>      display_handle_stream_destroy_all,
>      [ SPICE_MSG_DISPLAY_STREAM_DATA_SIZED ]  = display_handle_stream_data,
> +    [ SPICE_MSG_DISPLAY_STREAM_ACTIVATE_REPORT ] =
> display_handle_stream_activate_report,
>  
>      [ SPICE_MSG_DISPLAY_DRAW_FILL ]          = display_handle_draw_fill,
>      [ SPICE_MSG_DISPLAY_DRAW_OPAQUE ]        = display_handle_draw_opaque,
> diff --git a/spice-common b/spice-common
> index 149bb89..09f88f4 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit 149bb89adb0d7676c41085b3e41f07113e05c880
> +Subproject commit 09f88f4a688a156b48c2058dac7a8c0f35e96abd
> --
> 1.8.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 


More information about the Spice-devel mailing list