Related problem with audio. Look at that video https://www.youtube.com/watch?v=bn5iczqLBmk basically for a short while while playing a video with audio network is disconnected. This create a delay (see the delay when moving the mouse, I had 2 cursor so show the delay). The server cursor follow with a huge delay which disappears when audio is disabled. This is caused by some reasons: - communication is using tcp so packets are retransmitted, not discarded; - audio packets are never discarded; - video is synchronized on audio. You can understand that the delay can became so high that is hard to control the VM. Usually for communications (like mobile phones or similar) when a network problem happens we ears noises or silences, just packets are discarded (or corrupted). I think this should be the behaviour instead of introduce long buffering and delays. Basically means that in the client we should limit the queue to 1/2 audio packets and a new packet will replace the queued one. Frediano > > Do not offset the time attempting to fix client latency. > Client should handle it by itself. > > This patch is not designed to be merged but more for discussion. > > 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. I think only one end should handle this delay in an attempt to > synchronize audio and video instead that doing it in both ends. > > I'm currently trying it and the responsiveness is much better. > > One think I noted is however what happens if I block the connection. > Think about a momentary disconnection like when you disconnect the a > cable between the client and the server or when you drop every packets > between them with a firewall for a short while (this can happens in > many cases like a restart of a network device, a small fault of one or > simply you loose signal to your wifi network). The audio restart when > the connection is fixed (TCP take care of it) but everything then is > delayed. Looks like instead of catching up the delay is maintained. > Actually stopping the audio/video remove the delay. > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > --- > 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 | 7 ------- > 8 files changed, 3 insertions(+), 89 deletions(-) > > diff --git a/server/main-dispatcher.c b/server/main-dispatcher.c > index 43f8b79a7..7ecb53f4b 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 088a5c216..c49d40677 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 259496c64..056264c22 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 97f9219fe..633fdcba4 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -1838,7 +1838,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); > @@ -1971,7 +1971,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); > @@ -2670,25 +2670,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) > @@ -3083,7 +3065,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 264ef205c..9b6c1e6ae 100644 > --- a/server/reds.h > +++ b/server/reds.h > @@ -94,7 +94,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 9073626cd..996bb5208 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 2f0a2b93d..75d6c78cd 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 71755ea1f..ca0b49170 100644 > --- a/server/stream.c > +++ b/server/stream.c > @@ -650,9 +650,6 @@ 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); > > @@ -660,10 +657,6 @@ static void update_client_playback_delay(void *opaque, > uint32_t 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) _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel