From e184c8c0af8f2608fbaadf0a9f2d855e8b109c42 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Thu, 28 Sep 2017 12:22:09 +0100 Subject: [PATCH] Send real time to client 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 --- Changes since v1: - more deep clean; - remove the RFC intention; - remove some test notes not strictly related. --- server/dcc-private.h | 1 - server/dcc.cpp | 10 -------- server/dcc.h | 2 -- server/main-dispatcher.cpp | 32 ------------------------- server/reds-private.h | 2 -- server/reds.cpp | 24 +++---------------- server/reds.h | 1 - server/sound.cpp | 49 +------------------------------------- server/sound.h | 2 -- server/video-stream.cpp | 42 -------------------------------- server/video-stream.h | 1 - 11 files changed, 4 insertions(+), 162 deletions(-) diff --git a/server/dcc-private.h b/server/dcc-private.h index 842c14b2..0b42ee1e 100644 --- a/server/dcc-private.h +++ b/server/dcc-private.h @@ -64,7 +64,6 @@ struct DisplayChannelClientPrivate std::array surface_client_lossy_region; std::array stream_agents; - uint32_t streams_max_latency; uint64_t streams_max_bit_rate; bool gl_draw_ongoing; }; diff --git a/server/dcc.cpp b/server/dcc.cpp index 0031e66b..58170681 100644 --- a/server/dcc.cpp +++ b/server/dcc.cpp @@ -1173,16 +1173,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 47f082fe..4e7150d5 100644 --- a/server/dcc.h +++ b/server/dcc.h @@ -157,8 +157,6 @@ VideoStreamAgent *dcc_get_video_stream_agent(DisplayChannelClient *dcc, int stre 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.cpp b/server/main-dispatcher.cpp index 9112c908..5a6db5aa 100644 --- a/server/main-dispatcher.cpp +++ b/server/main-dispatcher.cpp @@ -53,7 +53,6 @@ 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 @@ -68,11 +67,6 @@ struct MainDispatcherMigrateSeamlessDstCompleteMessage { RedClient *client; }; -struct MainDispatcherMmTimeLatencyMessage { - RedClient *client; - uint32_t latency; -}; - struct MainDispatcherClientDisconnectMessage { RedClient *client; }; @@ -111,15 +105,6 @@ static void main_dispatcher_handle_migrate_complete(void *opaque, mig_complete->client->unref(); } -static void main_dispatcher_handle_mm_time_latency(void *opaque, - void *payload) -{ - auto reds = static_cast(opaque); - auto msg = static_cast(payload); - reds_set_client_mm_time_latency(reds, msg->client, msg->latency); - msg->client->unref(); -} - static void main_dispatcher_handle_client_disconnect(void *opaque, void *payload) { @@ -144,20 +129,6 @@ void MainDispatcher::seamless_migrate_dst_complete(RedClient *client) send_message(MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE, &msg); } -void MainDispatcher::set_mm_time_latency(RedClient *client, uint32_t latency) -{ - MainDispatcherMmTimeLatencyMessage msg; - - if (pthread_self() == thread_id) { - reds_set_client_mm_time_latency(reds, client, latency); - return; - } - - msg.client = red::add_ref(client); - msg.latency = latency; - send_message(MAIN_DISPATCHER_SET_MM_TIME_LATENCY, &msg); -} - void MainDispatcher::client_disconnect(RedClient *client) { MainDispatcherClientDisconnectMessage msg; @@ -190,9 +161,6 @@ MainDispatcher::MainDispatcher(RedsState *init_reds): register_handler(MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE, main_dispatcher_handle_migrate_complete, sizeof(MainDispatcherMigrateSeamlessDstCompleteMessage), false); - register_handler(MAIN_DISPATCHER_SET_MM_TIME_LATENCY, - main_dispatcher_handle_mm_time_latency, - sizeof(MainDispatcherMmTimeLatencyMessage), false); register_handler(MAIN_DISPATCHER_CLIENT_DISCONNECT, main_dispatcher_handle_client_disconnect, sizeof(MainDispatcherClientDisconnectMessage), false); diff --git a/server/reds-private.h b/server/reds-private.h index 50e38486..2edc3199 100644 --- a/server/reds-private.h +++ b/server/reds-private.h @@ -30,7 +30,6 @@ #include "safe-list.hpp" #define MIGRATE_TIMEOUT (MSEC_PER_SEC * 10) -#define MM_TIME_DELTA 400 /*ms*/ struct TicketAuthentication { char password[SPICE_MAX_PASSWORD_LENGTH]; @@ -128,7 +127,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.cpp b/server/reds.cpp index 4f45bd26..48700382 100644 --- a/server/reds.cpp +++ b/server/reds.cpp @@ -1852,7 +1852,7 @@ static void reds_handle_main_link(RedsState *reds, RedLinkInfo *link) if (!mig_target) { mcc->push_init(reds->qxl_instances.size(), 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) mcc->push_name(reds->config->spice_name); @@ -1974,7 +1974,7 @@ void reds_on_client_semi_seamless_migrate_complete(RedsState *reds, RedClient *c // TODO: not doing net test. consider doing it on client_migrate_info mcc->push_init(reds->qxl_instances.size(), 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); mcc->migrate_dst_complete(); @@ -2601,24 +2601,7 @@ static void reds_send_mm_time(RedsState *reds) return; } spice_debug("trace"); - reds->main_channel->push_multi_media_time(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); - } + reds->main_channel->push_multi_media_time(reds_get_mm_time()); } static void reds_cleanup_net(SpiceServer *reds) @@ -3059,7 +3042,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 2dd1be82..81b0fc1e 100644 --- a/server/reds.h +++ b/server/reds.h @@ -95,7 +95,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.cpp b/server/sound.cpp index 4909d158..acc80008 100644 --- a/server/sound.cpp +++ b/server/sound.cpp @@ -56,7 +56,6 @@ enum SndCommand { enum PlaybackCommand { SND_PLAYBACK_MODE = SND_END_COMMAND, SND_PLAYBACK_PCM, - SND_PLAYBACK_LATENCY, }; #define SND_MIGRATE_MASK (1 << SND_MIGRATE) @@ -67,7 +66,6 @@ enum PlaybackCommand { #define SND_PLAYBACK_MODE_MASK (1 << SND_PLAYBACK_MODE) #define SND_PLAYBACK_PCM_MASK (1 << SND_PLAYBACK_PCM) -#define SND_PLAYBACK_LATENCY_MASK ( 1 << SND_PLAYBACK_LATENCY) class SndChannelClient; struct SndChannel; @@ -154,7 +152,6 @@ public: AudioFrame *in_progress = nullptr; /* Frame being sent to the client */ AudioFrame *pending_frame = nullptr; /* Next frame to send to the client */ SpiceAudioDataMode mode = SPICE_AUDIO_DATA_MODE_RAW; - uint32_t latency = 0; SndCodec codec = nullptr; uint8_t encode_buf[SND_CODEC_MAX_COMPRESSED_BYTES]; @@ -453,21 +450,6 @@ static bool snd_playback_send_mute(PlaybackChannelClient *playback_client) SPICE_MSG_PLAYBACK_MUTE); } -static bool snd_playback_send_latency(PlaybackChannelClient *playback_client) -{ - RedChannelClient *rcc = playback_client; - SpiceMarshaller *m = rcc->get_marshaller(); - SpiceMsgPlaybackLatency latency_msg; - - spice_debug("latency %u", playback_client->latency); - rcc->init_send_data(SPICE_MSG_PLAYBACK_LATENCY); - latency_msg.latency_ms = playback_client->latency; - spice_marshall_msg_playback_latency(m, &latency_msg); - - rcc->begin_send_message(); - return true; -} - static bool snd_playback_send_start(PlaybackChannelClient *playback_client) { SpiceMarshaller *m = playback_client->get_marshaller(); @@ -642,7 +624,7 @@ void PlaybackChannelClient::send_item(G_GNUC_UNUSED RedPipeItem *item) { command &= SND_PLAYBACK_MODE_MASK|SND_PLAYBACK_PCM_MASK| SND_CTRL_MASK|SND_VOLUME_MUTE_MASK| - SND_MIGRATE_MASK|SND_PLAYBACK_LATENCY_MASK; + SND_MIGRATE_MASK; while (command) { if (command & SND_PLAYBACK_MODE_MASK) { command &= ~SND_PLAYBACK_MODE_MASK; @@ -685,12 +667,6 @@ void PlaybackChannelClient::send_item(G_GNUC_UNUSED RedPipeItem *item) break; } } - if (command & SND_PLAYBACK_LATENCY_MASK) { - command &= ~SND_PLAYBACK_LATENCY_MASK; - if (snd_playback_send_latency(this)) { - break; - } - } } snd_send(this); } @@ -941,29 +917,6 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance snd_send(playback_client); } -void snd_set_playback_latency(RedClient *client, uint32_t latency) -{ - GList *l; - - for (l = snd_channels; l != nullptr; l = l->next) { - auto now = static_cast(l->data); - SndChannelClient *scc = snd_channel_get_client(now); - if (now->type() == SPICE_CHANNEL_PLAYBACK && scc && - scc->get_client() == client) { - - if (scc->test_remote_cap(SPICE_PLAYBACK_CAP_LATENCY)) { - auto 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 SpiceAudioDataMode snd_desired_audio_mode(bool playback_compression, int frequency, bool client_can_opus) { diff --git a/server/sound.h b/server/sound.h index 19df2c2c..a1073f3a 100644 --- a/server/sound.h +++ b/server/sound.h @@ -32,8 +32,6 @@ void snd_detach_record(SpiceRecordInstance *sin); void snd_set_playback_compression(bool on); -void snd_set_playback_latency(struct RedClient *client, uint32_t latency); - #include "pop-visibility.h" #endif /* SOUND_H_ */ diff --git a/server/video-stream.cpp b/server/video-stream.cpp index 72a926cc..8418eed3 100644 --- a/server/video-stream.cpp +++ b/server/video-stream.cpp @@ -706,32 +706,6 @@ void video_stream_maintenance(DisplayChannel *display, } } -static void dcc_update_streams_max_latency(DisplayChannelClient *dcc, - VideoStreamAgent *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++) { - VideoStreamAgent *other_agent = dcc_get_video_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, VideoStream *stream) { char *env_bit_rate_str; @@ -809,19 +783,6 @@ static uint32_t get_source_fps(void *opaque) static void update_client_playback_delay(void *opaque, uint32_t delay_ms) { - auto agent = static_cast(opaque); - DisplayChannelClient *dcc = agent->dcc; - RedClient *client = dcc->get_client(); - RedsState *reds = client->get_server(); - - 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)); - reds_get_main_dispatcher(reds)->set_mm_time_latency(client, dcc_get_max_stream_latency(dcc)); } static void bitmap_ref(gpointer data) @@ -915,9 +876,6 @@ void dcc_create_stream(DisplayChannelClient *dcc, VideoStream *stream) void video_stream_agent_stop(VideoStreamAgent *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 = nullptr; diff --git a/server/video-stream.h b/server/video-stream.h index 6d9b9479..d3f25bb3 100644 --- a/server/video-stream.h +++ b/server/video-stream.h @@ -75,7 +75,6 @@ struct VideoStreamAgent { DisplayChannelClient *dcc; uint32_t report_id; - uint32_t client_required_latency; #ifdef STREAM_STATS StreamStats stats; #endif