When we have overall notion of available bandwidth and the bandwidth is constrained, we should be able to adjust Opus settings accordingly: https://opus-codec.org/examples/#bitrate-scalability The QoS may be as important for uplinks as is for downlinks because many last-mile connections have asymmetrical bandwidth/latency/jitter. David On Fri, 2017-04-07 at 15:19 +0200, Victor Toso wrote: > From: Victor Toso <me@xxxxxxxxxxxxxx> > > The QOS over spice channels is splitted in a few patches in order to > easy review. > > This patch introduces 4 new functions that are only internally > exposed > to SpiceChannels, they are: > > * spice_session_qos_can_channel_write() > * spice_session_qos_can_channel_read() > * spice_session_qos_channel_has_write_nbytes() > * spice_session_qos_channel_has_read_nbytes() > > Prior to each read or write, the SpiceChannel should query if it is > allowed to perform the IO with spice_session_qos_can_channel_write > (or > read). > > After each successful read or write, the SpiceChannel should inform > the QOS system how many bytes it has write/read. > > These functions should perform as fast as possible to not introduce > delays in each IO. > > The logic behind QOS will be introduced in follow up patches. > > Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598 > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > --- > src/spice-channel.c | 28 ++++++++++++ > src/spice-option.c | 5 ++ > src/spice-session-priv.h | 5 ++ > src/spice-session.c | 117 > +++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 155 insertions(+) > > diff --git a/src/spice-channel.c b/src/spice-channel.c > index b3fd521..f49950d 100644 > --- a/src/spice-channel.c > +++ b/src/spice-channel.c > @@ -837,6 +837,13 @@ static gint > spice_channel_flush_wire_nonblocking(SpiceChannel *channel, > return ret; > } > > +static gboolean wait_qos_can_flush(gpointer data) > +{ > + SpiceChannel *channel = SPICE_CHANNEL(data); > + SpiceChannelPrivate *c = channel->priv; > + return spice_session_qos_can_channel_write(c->session, channel); > +} > + > /* > * Write all 'data' of length 'datalen' bytes out to > * the wire > @@ -856,6 +863,12 @@ static void > spice_channel_flush_wire(SpiceChannel *channel, > > if (c->has_error) return; > > + if (!g_coroutine_condition_wait(&c->coroutine, > wait_qos_can_flush, channel)) { > + CHANNEL_DEBUG(channel, "Closing the channel: > spice_channel_flush failed qos"); > + c->has_error = TRUE; > + return; > + } > + > ret = spice_channel_flush_wire_nonblocking(channel, > ptr+offset, datalen-offset, &cond); > if (ret == -1) { > if (cond != 0) { > @@ -875,6 +888,7 @@ static void spice_channel_flush_wire(SpiceChannel > *channel, > } > offset += ret; > c->total_write_bytes += ret; > + spice_session_qos_channel_has_write_nbytes(c->session, > channel, ret); > } > } > > @@ -1063,6 +1077,13 @@ static int > spice_channel_read_wire_nonblocking(SpiceChannel *channel, > return ret; > } > > +static gboolean wait_qos_can_read(gpointer data) > +{ > + SpiceChannel *channel = SPICE_CHANNEL(data); > + SpiceChannelPrivate *c = channel->priv; > + return spice_session_qos_can_channel_read(c->session, channel); > +} > + > /* > * Read at least 1 more byte of data straight off the wire > * into the requested buffer. > @@ -1081,6 +1102,12 @@ static int > spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len > return 0; > } > > + if (!g_coroutine_condition_wait(&c->coroutine, > wait_qos_can_read, channel)) { > + CHANNEL_DEBUG(channel, "Closing the channel: > spice_channel_read failed qos"); > + c->has_error = TRUE; > + return 0; > + } > + > ret = spice_channel_read_wire_nonblocking(channel, data, > len, &cond); > > if (ret == -1) { > @@ -1099,6 +1126,7 @@ static int spice_channel_read_wire(SpiceChannel > *channel, void *data, size_t len > return 0; > } > > + spice_session_qos_channel_has_read_nbytes(c->session, > channel, ret); > return ret; > } > } > diff --git a/src/spice-option.c b/src/spice-option.c > index c04e978..bf81dea 100644 > --- a/src/spice-option.c > +++ b/src/spice-option.c > @@ -36,6 +36,7 @@ static char *usbredir_redirect_on_connect = NULL; > static gboolean smartcard = FALSE; > static gboolean disable_audio = FALSE; > static gboolean disable_usbredir = FALSE; > +static gboolean disable_qos = FALSE; > static gint cache_size = 0; > static gint glz_window_size = 0; > static gchar *secure_channels = NULL; > @@ -203,6 +204,8 @@ GOptionGroup* spice_get_option_group(void) > N_("Subject of the host certificate (field=value pairs > separated by commas)"), N_("<host-subject>") }, > { "spice-disable-audio", '\0', 0, G_OPTION_ARG_NONE, > &disable_audio, > N_("Disable audio support"), NULL }, > + { "spice-disable-qos", '\0', 0, G_OPTION_ARG_NONE, > &disable_qos, > + N_("Disable quality of service"), NULL }, > { "spice-smartcard", '\0', 0, G_OPTION_ARG_NONE, &smartcard, > N_("Enable smartcard support"), NULL }, > { "spice-smartcard-certificates", '\0', 0, > G_OPTION_ARG_STRING, &smartcard_certificates, > @@ -312,6 +315,8 @@ void spice_set_session_option(SpiceSession > *session) > g_object_set(session, "enable-usbredir", FALSE, NULL); > if (disable_audio) > g_object_set(session, "enable-audio", FALSE, NULL); > + if (disable_qos) > + g_object_set(session, "enable-qos", FALSE, NULL); > if (cache_size) > g_object_set(session, "cache-size", cache_size, NULL); > if (glz_window_size) > diff --git a/src/spice-session-priv.h b/src/spice-session-priv.h > index 03005aa..a65a7c0 100644 > --- a/src/spice-session-priv.h > +++ b/src/spice-session-priv.h > @@ -100,6 +100,11 @@ void spice_session_set_main_channel(SpiceSession > *session, SpiceChannel *channel > gboolean spice_session_set_migration_session(SpiceSession *session, > SpiceSession *mig_session); > SpiceAudio *spice_audio_get(SpiceSession *session, GMainContext > *context); > const gchar* spice_audio_data_mode_to_string(gint mode); > + > +gboolean spice_session_qos_can_channel_write(SpiceSession *session, > SpiceChannel *channel); > +gboolean spice_session_qos_can_channel_read(SpiceSession *session, > SpiceChannel *channel); > +void spice_session_qos_channel_has_write_nbytes(SpiceSession > *session, SpiceChannel *channel, gssize nbytes); > +void spice_session_qos_channel_has_read_nbytes(SpiceSession > *session, SpiceChannel *channel, gssize nbytes); > G_END_DECLS > > #endif /* __SPICE_CLIENT_SESSION_PRIV_H__ */ > diff --git a/src/spice-session.c b/src/spice-session.c > index 6f8cf5e..f7c35b4 100644 > --- a/src/spice-session.c > +++ b/src/spice-session.c > @@ -68,6 +68,9 @@ struct _SpiceSessionPrivate { > /* whether to enable smartcard event forwarding to the server */ > gboolean smartcard; > > + /* whether to enable quality of service */ > + gboolean qos_enabled; > + > /* list of certificates to use for the software smartcard reader > if > * enabled. For now, it has to contain exactly 3 certificates > for > * the software reader to be functional > @@ -123,8 +126,15 @@ struct _SpiceSessionPrivate { > SpiceUsbDeviceManager *usb_manager; > SpicePlaybackChannel *playback_channel; > PhodavServer *webdav; > + > + /* QoS */ > + GHashTable *qos_table; > }; > > +typedef struct session_channel_qos { > + bool throttling_write_enabled; > + bool throttling_read_enabled; > +} session_channel_qos; > > /** > * SECTION:spice-session > @@ -204,6 +214,7 @@ enum { > PROP_USERNAME, > PROP_UNIX_PATH, > PROP_PREF_COMPRESSION, > + PROP_QOS, > }; > > /* signals */ > @@ -295,6 +306,7 @@ static void spice_session_init(SpiceSession > *session) > s->glz_window = glz_decoder_window_new(); > update_proxy(session, NULL); > > + s->qos_table = g_hash_table_new_full(g_direct_hash, > g_direct_equal, NULL, g_free); > s->usb_manager = spice_usb_device_manager_get(session, &err); > if (err != NULL) { > SPICE_DEBUG("Could not initialize SpiceUsbDeviceManager - > %s", err->message); > @@ -382,6 +394,7 @@ spice_session_finalize(GObject *gobject) > > g_clear_pointer(&s->pubkey, g_byte_array_unref); > g_clear_pointer(&s->ca, g_byte_array_unref); > + g_clear_pointer(&s->qos_table, g_hash_table_unref); > > /* Chain up to the parent class */ > if (G_OBJECT_CLASS(spice_session_parent_class)->finalize) > @@ -696,6 +709,9 @@ static void > spice_session_get_property(GObject *gobject, > case PROP_PREF_COMPRESSION: > g_value_set_enum(value, s->preferred_compression); > break; > + case PROP_QOS: > + g_value_set_boolean(value, s->qos_enabled); > + break; > default: > G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec); > break; > @@ -835,6 +851,9 @@ static void > spice_session_set_property(GObject *gobject, > case PROP_PREF_COMPRESSION: > s->preferred_compression = g_value_get_enum(value); > break; > + case PROP_QOS: > + s->qos_enabled = g_value_get_boolean(value); > + break; > default: > G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec); > break; > @@ -1260,6 +1279,22 @@ static void > spice_session_class_init(SpiceSessionClass *klass) > G_PARAM_READWRITE | > G_PARAM_STATIC_STRINGS)); > > + /** > + * SpiceSession:enable-qos: > + * > + * If set to TRUE, the quality of service for channels will be > enabled. > + * > + * Since: 0.34 > + **/ > + g_object_class_install_property > + (gobject_class, PROP_QOS, > + g_param_spec_boolean("enable-qos", > + "Enable quality of service", > + "Enable quality of service", > + TRUE, > + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | > + G_PARAM_STATIC_STRINGS)); > + > > /** > * SpiceSession::channel-new: > @@ -1529,6 +1564,7 @@ SpiceSession > *spice_session_new_from_session(SpiceSession *session) > "enable-smartcard", &c->smartcard, > "enable-audio", &c->audio, > "enable-usbredir", &c->usbredir, > + "enable-qos", &c->qos_enabled, > "ca", &c->ca, > NULL); > > @@ -2226,12 +2262,17 @@ void spice_session_channel_new(SpiceSession > *session, SpiceChannel *channel) > > SpiceSessionPrivate *s = session->priv; > struct channel *item; > + session_channel_qos *qos; > > > item = g_new0(struct channel, 1); > item->channel = channel; > ring_add(&s->channels, &item->link); > > + qos = g_new0(session_channel_qos, 1); > + qos->throttling_write_enabled = qos->throttling_read_enabled = > false; > + g_hash_table_insert(s->qos_table, channel, qos); > + > if (SPICE_IS_MAIN_CHANNEL(channel)) { > gboolean all = spice_strv_contains(s->disable_effects, > "all"); > > @@ -2805,3 +2846,79 @@ gboolean > spice_session_set_migration_session(SpiceSession *session, > SpiceSession > > return TRUE; > } > + > +G_GNUC_INTERNAL > +gboolean spice_session_qos_can_channel_write(SpiceSession *session, > + SpiceChannel *channel) > +{ > + session_channel_qos *qos; > + SpiceSessionPrivate *s; > + > + g_return_val_if_fail(SPICE_IS_SESSION(session), FALSE); > + g_return_val_if_fail(SPICE_IS_CHANNEL(channel), FALSE); > + > + s = session->priv; > + if (!s->qos_enabled) > + return true; > + > + qos = g_hash_table_lookup(s->qos_table, channel); > + g_return_val_if_fail(qos != NULL, FALSE); > + > + return !qos->throttling_write_enabled; > +} > + > +G_GNUC_INTERNAL > +gboolean spice_session_qos_can_channel_read(SpiceSession *session, > + SpiceChannel *channel) > +{ > + session_channel_qos *qos; > + SpiceSessionPrivate *s; > + > + g_return_val_if_fail(SPICE_IS_SESSION(session), FALSE); > + g_return_val_if_fail(SPICE_IS_CHANNEL(channel), FALSE); > + > + s = session->priv; > + if (!s->qos_enabled) > + return true; > + > + qos = g_hash_table_lookup(s->qos_table, channel); > + g_return_val_if_fail(qos != NULL, FALSE); > + > + return !qos->throttling_read_enabled; > +} > + > +G_GNUC_INTERNAL > +void spice_session_qos_channel_has_read_nbytes(SpiceSession > *session, > + SpiceChannel > *channel, > + gssize nbytes) > +{ > + SpiceSessionPrivate *s; > + > + g_return_if_fail(SPICE_IS_SESSION(session)); > + g_return_if_fail(SPICE_IS_CHANNEL(channel)); > + > + s = session->priv; > + if (!s->qos_enabled) > + return; > + > + /* TODO */ > + return; > +} > + > +G_GNUC_INTERNAL > +void spice_session_qos_channel_has_write_nbytes(SpiceSession > *session, > + SpiceChannel > *channel, > + gssize nbytes) > +{ > + SpiceSessionPrivate *s; > + > + g_return_if_fail(SPICE_IS_SESSION(session)); > + g_return_if_fail(SPICE_IS_CHANNEL(channel)); > + > + s = session->priv; > + if (!s->qos_enabled) > + return; > + > + /* TODO */ > + return; > +} _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel