[spice-server,v2] Send real time to client

Submitted by Frediano Ziglio on Nov. 20, 2017, 4:02 p.m.

Details

Message ID 20171120160249.30507-1-fziglio@redhat.com
State New
Headers show
Series "Send real time to client" ( rev: 2 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio Nov. 20, 2017, 4:02 p.m.
Do not offset the time attempting to fix client latency.
Client should handle it by itself.

This remove entirely the delay introduced by the server.
This avoids surely possible time drifts in the client.
The server just sends it's concept of time without trying to force any
delay. Only one end should handle this delay in an attempt to
synchronize audio and video instead that doing it in both ends.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
Changes since v1:
- more deep clean;
- remove the RFC intention;
- remove some test notes not strictly related.
---
 server/dcc-private.h     |  1 -
 server/dcc.c             | 10 ----------
 server/dcc.h             |  2 --
 server/main-dispatcher.c | 28 ----------------------------
 server/main-dispatcher.h |  1 -
 server/reds-private.h    |  2 --
 server/reds.c            | 25 +++----------------------
 server/reds.h            |  1 -
 server/sound.c           | 26 --------------------------
 server/sound.h           |  2 --
 server/stream.c          | 44 --------------------------------------------
 server/stream.h          |  1 -
 12 files changed, 3 insertions(+), 140 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/dcc-private.h b/server/dcc-private.h
index c4662202..efbcefc3 100644
--- a/server/dcc-private.h
+++ b/server/dcc-private.h
@@ -62,7 +62,6 @@  struct DisplayChannelClientPrivate
     QRegion surface_client_lossy_region[NUM_SURFACES];
 
     StreamAgent stream_agents[NUM_STREAMS];
-    uint32_t streams_max_latency;
     uint64_t streams_max_bit_rate;
     bool gl_draw_ongoing;
 };
diff --git a/server/dcc.c b/server/dcc.c
index c2fdd8c2..4b534707 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -1403,16 +1403,6 @@  spice_wan_compression_t dcc_get_zlib_glz_state(DisplayChannelClient *dcc)
     return dcc->priv->zlib_glz_state;
 }
 
-uint32_t dcc_get_max_stream_latency(DisplayChannelClient *dcc)
-{
-    return dcc->priv->streams_max_latency;
-}
-
-void dcc_set_max_stream_latency(DisplayChannelClient *dcc, uint32_t latency)
-{
-    dcc->priv->streams_max_latency = latency;
-}
-
 uint64_t dcc_get_max_stream_bit_rate(DisplayChannelClient *dcc)
 {
     return dcc->priv->streams_max_bit_rate;
diff --git a/server/dcc.h b/server/dcc.h
index 4de49457..38842e24 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -193,8 +193,6 @@  StreamAgent *              dcc_get_stream_agent                      (DisplayCha
 ImageEncoders *dcc_get_encoders(DisplayChannelClient *dcc);
 spice_wan_compression_t    dcc_get_jpeg_state                        (DisplayChannelClient *dcc);
 spice_wan_compression_t    dcc_get_zlib_glz_state                    (DisplayChannelClient *dcc);
-uint32_t dcc_get_max_stream_latency(DisplayChannelClient *dcc);
-void dcc_set_max_stream_latency(DisplayChannelClient *dcc, uint32_t latency);
 uint64_t dcc_get_max_stream_bit_rate(DisplayChannelClient *dcc);
 void dcc_set_max_stream_bit_rate(DisplayChannelClient *dcc, uint64_t rate);
 gboolean dcc_is_low_bandwidth(DisplayChannelClient *dcc);
diff --git a/server/main-dispatcher.c b/server/main-dispatcher.c
index 43f8b79a..7ecb53f4 100644
--- a/server/main-dispatcher.c
+++ b/server/main-dispatcher.c
@@ -146,7 +146,6 @@  main_dispatcher_init(MainDispatcher *self)
 enum {
     MAIN_DISPATCHER_CHANNEL_EVENT = 0,
     MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE,
-    MAIN_DISPATCHER_SET_MM_TIME_LATENCY,
     MAIN_DISPATCHER_CLIENT_DISCONNECT,
 
     MAIN_DISPATCHER_NUM_MESSAGES
@@ -214,15 +213,6 @@  static void main_dispatcher_handle_migrate_complete(void *opaque,
     g_object_unref(mig_complete->client);
 }
 
-static void main_dispatcher_handle_mm_time_latency(void *opaque,
-                                                   void *payload)
-{
-    MainDispatcher *self = opaque;
-    MainDispatcherMmTimeLatencyMessage *msg = payload;
-    reds_set_client_mm_time_latency(self->priv->reds, msg->client, msg->latency);
-    g_object_unref(msg->client);
-}
-
 static void main_dispatcher_handle_client_disconnect(void *opaque,
                                                      void *payload)
 {
@@ -249,21 +239,6 @@  void main_dispatcher_seamless_migrate_dst_complete(MainDispatcher *self,
                             &msg);
 }
 
-void main_dispatcher_set_mm_time_latency(MainDispatcher *self, RedClient *client, uint32_t latency)
-{
-    MainDispatcherMmTimeLatencyMessage msg;
-
-    if (pthread_self() == dispatcher_get_thread_id(DISPATCHER(self))) {
-        reds_set_client_mm_time_latency(self->priv->reds, client, latency);
-        return;
-    }
-
-    msg.client = g_object_ref(client);
-    msg.latency = latency;
-    dispatcher_send_message(DISPATCHER(self), MAIN_DISPATCHER_SET_MM_TIME_LATENCY,
-                            &msg);
-}
-
 void main_dispatcher_client_disconnect(MainDispatcher *self, RedClient *client)
 {
     MainDispatcherClientDisconnectMessage msg;
@@ -318,9 +293,6 @@  void main_dispatcher_constructed(GObject *object)
     dispatcher_register_handler(DISPATCHER(self), MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE,
                                 main_dispatcher_handle_migrate_complete,
                                 sizeof(MainDispatcherMigrateSeamlessDstCompleteMessage), false);
-    dispatcher_register_handler(DISPATCHER(self), MAIN_DISPATCHER_SET_MM_TIME_LATENCY,
-                                main_dispatcher_handle_mm_time_latency,
-                                sizeof(MainDispatcherMmTimeLatencyMessage), false);
     dispatcher_register_handler(DISPATCHER(self), MAIN_DISPATCHER_CLIENT_DISCONNECT,
                                 main_dispatcher_handle_client_disconnect,
                                 sizeof(MainDispatcherClientDisconnectMessage), false);
diff --git a/server/main-dispatcher.h b/server/main-dispatcher.h
index 088a5c21..c49d4067 100644
--- a/server/main-dispatcher.h
+++ b/server/main-dispatcher.h
@@ -51,7 +51,6 @@  GType main_dispatcher_get_type(void) G_GNUC_CONST;
 
 void main_dispatcher_channel_event(MainDispatcher *self, int event, SpiceChannelEventInfo *info);
 void main_dispatcher_seamless_migrate_dst_complete(MainDispatcher *self, RedClient *client);
-void main_dispatcher_set_mm_time_latency(MainDispatcher *self, RedClient *client, uint32_t latency);
 /*
  * Disconnecting the client is always executed asynchronously,
  * in order to protect from expired references in the routines
diff --git a/server/reds-private.h b/server/reds-private.h
index adc48ba5..619ca621 100644
--- a/server/reds-private.h
+++ b/server/reds-private.h
@@ -29,7 +29,6 @@ 
 #include "red-record-qxl.h"
 
 #define MIGRATE_TIMEOUT (MSEC_PER_SEC * 10)
-#define MM_TIME_DELTA 400 /*ms*/
 
 typedef struct TicketAuthentication {
     char password[SPICE_MAX_PASSWORD_LENGTH];
@@ -123,7 +122,6 @@  struct RedsState {
     SpiceBuffer client_monitors_config;
 
     int mm_time_enabled;
-    uint32_t mm_time_latency;
 
     SpiceCharDeviceInstance *vdagent;
     SpiceMigrateInstance *migration_interface;
diff --git a/server/reds.c b/server/reds.c
index 0d0af05f..b32ee9d2 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1835,7 +1835,7 @@  static void reds_handle_main_link(RedsState *reds, RedLinkInfo *link)
     if (!mig_target) {
         main_channel_client_push_init(mcc, g_list_length(reds->qxl_instances),
             reds->mouse_mode, reds->is_client_mouse_allowed,
-            reds_get_mm_time() - MM_TIME_DELTA,
+            reds_get_mm_time(),
             reds_qxl_ram_size(reds));
         if (reds->config->spice_name)
             main_channel_client_push_name(mcc, reds->config->spice_name);
@@ -1968,7 +1968,7 @@  void reds_on_client_semi_seamless_migrate_complete(RedsState *reds, RedClient *c
     main_channel_client_push_init(mcc, g_list_length(reds->qxl_instances),
                                   reds->mouse_mode,
                                   reds->is_client_mouse_allowed,
-                                  reds_get_mm_time() - MM_TIME_DELTA,
+                                  reds_get_mm_time(),
                                   reds_qxl_ram_size(reds));
     reds_link_mig_target_channels(reds, client);
     main_channel_client_migrate_dst_complete(mcc);
@@ -2667,25 +2667,7 @@  static void reds_send_mm_time(RedsState *reds)
         return;
     }
     spice_debug("trace");
-    main_channel_push_multi_media_time(reds->main_channel,
-                                       reds_get_mm_time() - reds->mm_time_latency);
-}
-
-void reds_set_client_mm_time_latency(RedsState *reds, RedClient *client, uint32_t latency)
-{
-    // TODO: multi-client support for mm_time
-    if (reds->mm_time_enabled) {
-        // TODO: consider network latency
-        if (latency > reds->mm_time_latency) {
-            reds->mm_time_latency = latency;
-            reds_send_mm_time(reds);
-        } else {
-            spice_debug("new latency %u is smaller than existing %u",
-                        latency, reds->mm_time_latency);
-        }
-    } else {
-        snd_set_playback_latency(client, latency);
-    }
+    main_channel_push_multi_media_time(reds->main_channel, reds_get_mm_time());
 }
 
 static int reds_init_net(RedsState *reds)
@@ -3080,7 +3062,6 @@  uint32_t reds_get_mm_time(void)
 void reds_enable_mm_time(RedsState *reds)
 {
     reds->mm_time_enabled = TRUE;
-    reds->mm_time_latency = MM_TIME_DELTA;
     reds_send_mm_time(reds);
 }
 
diff --git a/server/reds.h b/server/reds.h
index cea002c5..51d3b37e 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -90,7 +90,6 @@  void reds_on_client_semi_seamless_migrate_complete(RedsState *reds, RedClient *c
 void reds_on_client_seamless_migrate_complete(RedsState *reds, RedClient *client);
 void reds_on_main_channel_migrate(RedsState *reds, MainChannelClient *mcc);
 
-void reds_set_client_mm_time_latency(RedsState *reds, RedClient *client, uint32_t latency);
 uint32_t reds_get_streaming_video(const RedsState *reds);
 GArray* reds_get_video_codecs(const RedsState *reds);
 spice_wan_compression_t reds_get_jpeg_state(const RedsState *reds);
diff --git a/server/sound.c b/server/sound.c
index b1bfaaaa..e0206366 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -972,32 +972,6 @@  SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
     snd_send(SND_CHANNEL_CLIENT(playback_client));
 }
 
-void snd_set_playback_latency(RedClient *client, uint32_t latency)
-{
-    GList *l;
-
-    for (l = snd_channels; l != NULL; l = l->next) {
-        SndChannel *now = l->data;
-        SndChannelClient *scc = snd_channel_get_client(now);
-        uint32_t type;
-        g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
-        if (type == SPICE_CHANNEL_PLAYBACK && scc &&
-            red_channel_client_get_client(RED_CHANNEL_CLIENT(scc)) == client) {
-
-            if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(scc),
-                SPICE_PLAYBACK_CAP_LATENCY)) {
-                PlaybackChannelClient* playback = (PlaybackChannelClient*)scc;
-
-                playback->latency = latency;
-                snd_set_command(scc, SND_PLAYBACK_LATENCY_MASK);
-                snd_send(scc);
-            } else {
-                spice_debug("client doesn't not support SPICE_PLAYBACK_CAP_LATENCY");
-            }
-        }
-    }
-}
-
 static int snd_desired_audio_mode(bool playback_compression, int frequency,
                                   bool client_can_celt, bool client_can_opus)
 {
diff --git a/server/sound.h b/server/sound.h
index 2f0a2b93..75d6c78c 100644
--- a/server/sound.h
+++ b/server/sound.h
@@ -30,6 +30,4 @@  void snd_detach_record(SpiceRecordInstance *sin);
 
 void snd_set_playback_compression(bool on);
 
-void snd_set_playback_latency(struct RedClient *client, uint32_t latency);
-
 #endif /* SOUND_H_ */
diff --git a/server/stream.c b/server/stream.c
index 846c921a..f16111cf 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -546,31 +546,6 @@  void stream_maintenance(DisplayChannel *display,
     }
 }
 
-static void dcc_update_streams_max_latency(DisplayChannelClient *dcc, StreamAgent *remove_agent)
-{
-    uint32_t new_max_latency = 0;
-    int i;
-
-    if (dcc_get_max_stream_latency(dcc) != remove_agent->client_required_latency) {
-        return;
-    }
-
-    dcc_set_max_stream_latency(dcc, 0);
-    if (DCC_TO_DC(dcc)->priv->stream_count == 1) {
-        return;
-    }
-    for (i = 0; i < NUM_STREAMS; i++) {
-        StreamAgent *other_agent = dcc_get_stream_agent(dcc, i);
-        if (other_agent == remove_agent || !other_agent->video_encoder) {
-            continue;
-        }
-        if (other_agent->client_required_latency > new_max_latency) {
-            new_max_latency = other_agent->client_required_latency;
-        }
-    }
-    dcc_set_max_stream_latency(dcc, new_max_latency);
-}
-
 static uint64_t get_initial_bit_rate(DisplayChannelClient *dcc, Stream *stream)
 {
     char *env_bit_rate_str;
@@ -648,22 +623,6 @@  static uint32_t get_source_fps(void *opaque)
 
 static void update_client_playback_delay(void *opaque, uint32_t delay_ms)
 {
-    StreamAgent *agent = opaque;
-    DisplayChannelClient *dcc = agent->dcc;
-    RedChannel *channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(dcc));
-    RedClient *client = red_channel_client_get_client(RED_CHANNEL_CLIENT(dcc));
-    RedsState *reds = red_channel_get_server(channel);
-
-    dcc_update_streams_max_latency(dcc, agent);
-
-    agent->client_required_latency = delay_ms;
-    if (delay_ms > dcc_get_max_stream_latency(dcc)) {
-        dcc_set_max_stream_latency(dcc, delay_ms);
-    }
-    spice_debug("resetting client latency: %u", dcc_get_max_stream_latency(dcc));
-    main_dispatcher_set_mm_time_latency(reds_get_main_dispatcher(reds),
-                                        client,
-                                        dcc_get_max_stream_latency(agent->dcc));
 }
 
 static void bitmap_ref(gpointer data)
@@ -759,9 +718,6 @@  void dcc_create_stream(DisplayChannelClient *dcc, Stream *stream)
 
 void stream_agent_stop(StreamAgent *agent)
 {
-    DisplayChannelClient *dcc = agent->dcc;
-
-    dcc_update_streams_max_latency(dcc, agent);
     if (agent->video_encoder) {
         agent->video_encoder->destroy(agent->video_encoder);
         agent->video_encoder = NULL;
diff --git a/server/stream.h b/server/stream.h
index 6f194618..4d626162 100644
--- a/server/stream.h
+++ b/server/stream.h
@@ -80,7 +80,6 @@  typedef struct StreamAgent {
     int fps;
 
     uint32_t report_id;
-    uint32_t client_required_latency;
 #ifdef STREAM_STATS
     StreamStats stats;
 #endif