[Spice-devel] [client v2 11/12] spice-session: Keep track of the global streams lag

Francois Gouget fgouget at codeweavers.com
Mon Jun 17 17:36:04 UTC 2019


Each video and audio stream has its own lag: for video streams it is
the decoding time and for audio ones buffering by the audio subsystem.
The only way to keep them all in sync is to synchronize to the most
laggy stream.

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

There may be a thread conetxt issue with the spice_session_update_lag() 
call in display_handle_stream_data(). I have not noticed this causing 
any problem though.

 src/channel-display-gst.c   |  9 ++++++++
 src/channel-display-mjpeg.c |  6 ++++++
 src/channel-display-priv.h  |  4 ++++
 src/channel-display.c       | 33 +++++++++++++++++++++++++++--
 src/channel-display.h       |  2 ++
 src/channel-playback-priv.h |  3 ++-
 src/channel-playback.c      | 22 ++++++++++++++++---
 src/spice-session-priv.h    |  6 +++++-
 src/spice-session.c         | 42 +++++++++++++++++++++++++++++++++++--
 9 files changed, 118 insertions(+), 9 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 460d0ab9..d5a5d431 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -454,6 +454,15 @@ sink_event_probe(GstPad *pad, GstPadProbeInfo *info, gpointer data)
                    frame->time, frame->size, frame->creation_time, duration,
                    decoder->decoding_queue->length, gstframe->queue_len);
 
+            /* And the average decoding time. Only take into account frames
+             * that were not delayed (see above), or that prove the current
+             * average is clearly overestimated.
+             */
+            if (gstframe->queue_len < MAX_DECODED_FRAMES ||
+                duration / 1000 < decoder->base.decoding_time) {
+                decoder->base.decoding_time = (decoder->base.decoding_time * 15 + duration / 1000) / 16;
+            }
+
             if (!decoder->appsink) {
                 /* The sink will display the frame directly so this
                  * SpiceGstFrame and those of any dropped frame are no longer
diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index b058424e..20e10d9b 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -86,6 +86,7 @@ static gboolean mjpeg_decoder_decode_frame(gpointer video_decoder)
     JDIMENSION width, height;
     uint8_t *dest;
     uint8_t *lines[4];
+    int64_t start = g_get_monotonic_time();
 
     jpeg_read_header(&decoder->mjpeg_cinfo, 1);
     width = decoder->mjpeg_cinfo.image_width;
@@ -156,6 +157,11 @@ static gboolean mjpeg_decoder_decode_frame(gpointer video_decoder)
     }
     jpeg_finish_decompress(&decoder->mjpeg_cinfo);
 
+    uint32_t duration = (g_get_monotonic_time() - start) / 1000;
+    decoder->base.decoding_time = decoder->base.decoding_time ?
+        (decoder->base.decoding_time * 15 + duration) / 16 :
+        duration;
+
     /* Display the frame and dispose of it */
     stream_display_frame(decoder->base.stream, decoder->cur_frame,
                          width, height, SPICE_UNKNOWN_STRIDE, decoder->out_frame);
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 7c978780..03f0c992 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -72,6 +72,9 @@ struct VideoDecoder {
 
     /* The associated display stream. */
     display_stream *stream;
+
+    /* The average time it takes to decode a frame. */
+    float decoding_time;
 };
 
 
@@ -118,6 +121,7 @@ struct display_stream {
     int                         have_region;
 
     VideoDecoder                *video_decoder;
+    guint                       decoder_lag;
 
     SpiceChannel                *channel;
 
diff --git a/src/channel-display.c b/src/channel-display.c
index 03110df6..b6569d04 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1411,6 +1411,27 @@ void stream_display_frame(display_stream *st, SpiceFrame *frame,
     }
 }
 
+G_GNUC_INTERNAL
+guint32 spice_display_channel_get_lag(SpiceDisplayChannel *channel)
+{
+    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
+    int i;
+    guint lag;
+
+    lag = 0;
+    if (c->streams) {
+        for (i = 0; i < c->nstreams; i++) {
+            display_stream *st = c->streams[i];
+            if (st != NULL && st->video_decoder) {
+                st->decoder_lag = st->video_decoder->decoding_time;
+                lag = MAX(lag, st->decoder_lag);
+            }
+        }
+    }
+
+    return lag;
+}
+
 G_GNUC_INTERNAL
 gboolean hand_pipeline_to_widget(display_stream *st, GstPipeline *pipeline)
 {
@@ -1467,7 +1488,7 @@ static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
         report.num_drops = st-> report_num_drops;
         report.last_frame_delay = margin;
         if (spice_session_is_playback_active(session)) {
-            report.audio_delay = spice_session_get_playback_latency(session);
+            report.audio_delay = spice_session_get_playback_lag(session);
         } else {
             report.audio_delay = UINT_MAX;
         }
@@ -1684,7 +1705,7 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
     /* Compute the local frame mmtime */
     stream_time = stream_get_time(st);
     frame_time = spice_session_get_client_time(session, op->multi_media_time);
-    guint32 target_lag = spice_session_get_playback_latency(session);
+    guint32 target_lag = spice_session_get_lag(session);
     if (st->last_frame_time == 0) {
         margin = spice_mmtime_diff(frame_time, stream_time);
         st->delay = MAX(target_lag - margin, INITIAL_DELAY);
@@ -1793,6 +1814,14 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
         return;
     }
 
+    if (st->video_decoder->decoding_time != 0 &&
+        (st->decoder_lag == 0 ||
+         spice_session_lag_needs_update(st->decoder_lag, st->video_decoder->decoding_time))) {
+        SpiceSession *session = spice_channel_get_session(SPICE_CHANNEL(channel));
+        /* FIXME This may be the wrong context for this call */
+        spice_session_update_lag(session, st->video_decoder->decoding_time);
+    }
+
     if (c->enable_adaptive_streaming) {
         display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op->id,
                                      op->multi_media_time, mmtime_margin);
diff --git a/src/channel-display.h b/src/channel-display.h
index 5b48d2ff..57e5e3dd 100644
--- a/src/channel-display.h
+++ b/src/channel-display.h
@@ -157,6 +157,8 @@ void            spice_gl_scanout_free         (SpiceGlScanout *scanout);
 const SpiceGlScanout* spice_display_channel_get_gl_scanout(SpiceDisplayChannel *channel);
 void spice_display_channel_gl_draw_done(SpiceDisplayChannel *channel);
 
+guint32 spice_display_channel_get_lag(SpiceDisplayChannel *channel);
+
 #ifndef SPICE_DISABLE_DEPRECATED
 G_DEPRECATED_FOR(spice_display_channel_change_preferred_compression)
 void spice_display_change_preferred_compression(SpiceChannel *channel, gint compression);
diff --git a/src/channel-playback-priv.h b/src/channel-playback-priv.h
index dc89e2d8..7f0ec33c 100644
--- a/src/channel-playback-priv.h
+++ b/src/channel-playback-priv.h
@@ -19,5 +19,6 @@
 #define __SPICE_CLIENT_PLAYBACK_CHANNEL_PRIV_H__
 
 gboolean spice_playback_channel_is_active(SpicePlaybackChannel *channel);
-guint32 spice_playback_channel_get_latency(SpicePlaybackChannel *channel);
+guint32 spice_playback_channel_get_lag(SpicePlaybackChannel *channel);
+void spice_playback_channel_update_lag(SpicePlaybackChannel *channel, guint32 lag);
 #endif
diff --git a/src/channel-playback.c b/src/channel-playback.c
index 052e3036..21d517af 100644
--- a/src/channel-playback.c
+++ b/src/channel-playback.c
@@ -457,25 +457,30 @@ static void channel_set_handlers(SpiceChannelClass *klass)
  * @channel: a #SpicePlaybackChannel
  * @delay_ms: the delay in ms
  *
- * Adjust the multimedia time according to the delay.
+ * Adjust the multimedia time and global lag according to the delay.
  **/
 void spice_playback_channel_set_delay(SpicePlaybackChannel *channel, guint32 delay_ms)
 {
     SpicePlaybackChannelPrivate *c;
     SpiceSession *session;
+    guint32 old_latency;
 
     g_return_if_fail(SPICE_IS_PLAYBACK_CHANNEL(channel));
 
     CHANNEL_DEBUG(channel, "playback set_delay %u ms", delay_ms);
 
     c = channel->priv;
+    old_latency = c->latency;
     c->latency = delay_ms;
 
     session = spice_channel_get_session(SPICE_CHANNEL(channel));
     if (session) {
         spice_session_set_mm_time(session, c->last_time - delay_ms);
+        if (spice_session_lag_needs_update(old_latency, delay_ms)) {
+            spice_session_update_lag(session, delay_ms);
+        }
     } else {
-        CHANNEL_DEBUG(channel, "channel detached from session, mm time skipped");
+        CHANNEL_DEBUG(channel, "channel detached from session, mm time & global lag skipped");
     }
 }
 
@@ -487,7 +492,7 @@ gboolean spice_playback_channel_is_active(SpicePlaybackChannel *channel)
 }
 
 G_GNUC_INTERNAL
-guint32 spice_playback_channel_get_latency(SpicePlaybackChannel *channel)
+guint32 spice_playback_channel_get_lag(SpicePlaybackChannel *channel)
 {
     g_return_val_if_fail(SPICE_IS_PLAYBACK_CHANNEL(channel), 0);
     if (!channel->priv->is_active) {
@@ -495,3 +500,14 @@ guint32 spice_playback_channel_get_latency(SpicePlaybackChannel *channel)
     }
     return channel->priv->latency;
 }
+
+G_GNUC_INTERNAL
+void spice_playback_channel_update_lag(SpicePlaybackChannel *channel, guint32 lag)
+{
+    g_return_if_fail(SPICE_IS_PLAYBACK_CHANNEL(channel));
+    channel->priv->min_latency = lag;
+    if (channel->priv->is_active) {
+        SPICE_DEBUG("%s: notify lag update %u", __FUNCTION__, channel->priv->min_latency);
+        g_coroutine_object_notify(G_OBJECT(SPICE_CHANNEL(channel)), "min-latency");
+    }
+}
diff --git a/src/spice-session-priv.h b/src/spice-session-priv.h
index 1e91f627..1b3f658c 100644
--- a/src/spice-session-priv.h
+++ b/src/spice-session-priv.h
@@ -85,7 +85,11 @@ SpiceChannel* spice_session_lookup_channel(SpiceSession *session, gint id, gint
 void spice_session_set_uuid(SpiceSession *session, guint8 uuid[16]);
 void spice_session_set_name(SpiceSession *session, const gchar *name);
 gboolean spice_session_is_playback_active(SpiceSession *session);
-guint32 spice_session_get_playback_latency(SpiceSession *session);
+guint32 spice_session_get_playback_lag(SpiceSession *session);
+#define spice_session_lag_needs_update(old_lag, new_lag) \
+    (old_lag == 0 || (new_lag) + 10 < (old_lag) || (new_lag) > (old_lag) + 10)
+guint32 spice_session_get_lag(SpiceSession *session);
+void spice_session_update_lag(SpiceSession *session, guint32 lag);
 gboolean spice_session_get_audio_enabled(SpiceSession *session);
 gboolean spice_session_get_smartcard_enabled(SpiceSession *session);
 gboolean spice_session_get_usbredir_enabled(SpiceSession *session);
diff --git a/src/spice-session.c b/src/spice-session.c
index a1a96829..57b13ae9 100644
--- a/src/spice-session.c
+++ b/src/spice-session.c
@@ -103,6 +103,7 @@ struct _SpiceSessionPrivate {
     gboolean          client_provided_sockets;
     guint64           mm_time_offset;
     gint64            client_time_offset;
+    guint32           lag;
     SpiceSession      *migration;
     GList             *migration_left;
     SpiceSessionMigration migration_state;
@@ -2672,7 +2673,7 @@ gboolean spice_session_is_playback_active(SpiceSession *session)
 }
 
 G_GNUC_INTERNAL
-guint32 spice_session_get_playback_latency(SpiceSession *session)
+guint32 spice_session_get_playback_lag(SpiceSession *session)
 {
     g_return_val_if_fail(SPICE_IS_SESSION(session), 0);
 
@@ -2680,13 +2681,50 @@ guint32 spice_session_get_playback_latency(SpiceSession *session)
 
     if (s->playback_channel &&
         spice_playback_channel_is_active(s->playback_channel)) {
-        return spice_playback_channel_get_latency(s->playback_channel);
+        return spice_playback_channel_get_lag(s->playback_channel);
     } else {
         SPICE_DEBUG("%s: not implemented when there isn't audio playback", __FUNCTION__);
         return 0;
     }
 }
 
+G_GNUC_INTERNAL
+guint32 spice_session_get_lag(SpiceSession *session)
+{
+    g_return_val_if_fail(SPICE_IS_SESSION(session), 0);
+
+    return session->priv->lag;
+}
+
+G_GNUC_INTERNAL
+void spice_session_update_lag(SpiceSession *session, guint32 lag)
+{
+    g_return_if_fail(SPICE_IS_SESSION(session));
+    SpiceSessionPrivate *s = session->priv;
+
+    if (spice_session_lag_needs_update(s->lag, lag)) {
+        guint32 new_lag = 0;
+        for (GList *l = s->channels; l != NULL; ) {
+            SpiceChannel *channel = l->data;
+            l = l->next;
+            switch (spice_channel_get_channel_type(channel)) {
+            case SPICE_CHANNEL_DISPLAY:
+                new_lag = MAX(new_lag, spice_display_channel_get_lag(SPICE_DISPLAY_CHANNEL(channel)));
+                break;
+            case SPICE_CHANNEL_PLAYBACK:
+                new_lag = MAX(new_lag, spice_playback_channel_get_lag(SPICE_PLAYBACK_CHANNEL(channel)));
+                break;
+            }
+        }
+        if (new_lag != s->lag) {
+            s->lag = new_lag;
+            if (s->playback_channel) {
+                spice_playback_channel_update_lag(s->playback_channel, s->lag);
+            }
+        }
+    }
+}
+
 static const gchar* spice_session_get_shared_dir(SpiceSession *session)
 {
     g_return_val_if_fail(SPICE_IS_SESSION(session), NULL);
-- 
2.20.1



More information about the Spice-devel mailing list