[Spice-devel] [RFCv5 14/47] server/main_channel: move latency and bitrate to channel client

Alon Levy alevy at redhat.com
Sun May 8 06:11:10 PDT 2011


They were globals before. This introduces api for other channels
to query the low bandwidth status. The queries themselves are still done
from the wrong context (channel and not channel client) but that's because
the decoupling of channel and channel client will be done in the following
patches.

Note that snd_worker.c got two copied function declarations that belong to
main_channel.h but can't be easily dragged into snd_worker.c since it still
uses it's own RedChannel struct.
---
 server/main_channel.c |   54 ++++++++++++++++++++++++++++-----------------
 server/main_channel.h |    3 ++
 server/red_channel.c  |    5 ++++
 server/red_channel.h  |    1 +
 server/red_worker.c   |   58 +++++++++++++++++++++++++++++++++++-------------
 server/reds.h         |    3 --
 server/snd_worker.c   |   12 +++++++++-
 7 files changed, 96 insertions(+), 40 deletions(-)

diff --git a/server/main_channel.c b/server/main_channel.c
index 9c07abd..3f6149c 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -122,6 +122,8 @@ struct MainChannelClient {
     uint32_t ping_id;
     uint32_t net_test_id;
     int net_test_stage;
+    uint64_t latency;
+    uint64_t bitrate_per_sec;
 };
 
 struct MainChannel {
@@ -136,9 +138,6 @@ enum NetTestStage {
     NET_TEST_STAGE_RATE,
 };
 
-static uint64_t latency = 0;
-uint64_t bitrate_per_sec = ~0;
-
 static void main_channel_client_disconnect(RedChannelClient *rcc)
 {
     red_channel_client_disconnect(rcc);
@@ -147,9 +146,6 @@ static void main_channel_client_disconnect(RedChannelClient *rcc)
 static void main_disconnect(MainChannel *main_chan)
 {
     red_channel_destroy(&main_chan->base);
-
-    latency = 0;
-    bitrate_per_sec = ~0;
 }
 
 static int main_channel_client_push_ping(RedChannelClient *rcc, int size);
@@ -747,23 +743,23 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
             case NET_TEST_STAGE_LATENCY:
                 mcc->net_test_id++;
                 mcc->net_test_stage = NET_TEST_STAGE_RATE;
-                latency = roundtrip;
+                mcc->latency = roundtrip;
                 break;
             case NET_TEST_STAGE_RATE:
                 mcc->net_test_id = 0;
-                if (roundtrip <= latency) {
+                if (roundtrip <= mcc->latency) {
                     // probably high load on client or server result with incorrect values
-                    latency = 0;
+                    mcc->latency = 0;
                     red_printf("net test: invalid values, latency %lu roundtrip %lu. assuming high"
-                               "bandwidth", latency, roundtrip);
+                               "bandwidth", mcc->latency, roundtrip);
                     break;
                 }
-                bitrate_per_sec = (uint64_t)(NET_TEST_BYTES * 8) * 1000000 / (roundtrip - latency);
+                mcc->bitrate_per_sec = (uint64_t)(NET_TEST_BYTES * 8) * 1000000 / (roundtrip - mcc->latency);
                 red_printf("net test: latency %f ms, bitrate %lu bps (%f Mbps)%s",
-                           (double)latency / 1000,
-                           bitrate_per_sec,
-                           (double)bitrate_per_sec / 1024 / 1024,
-                           IS_LOW_BANDWIDTH() ? " LOW BANDWIDTH" : "");
+                           (double)mcc->latency / 1000,
+                           mcc->bitrate_per_sec,
+                           (double)mcc->bitrate_per_sec / 1024 / 1024,
+                           main_channel_client_is_low_bandwidth(mcc) ? " LOW BANDWIDTH" : "");
                 mcc->net_test_stage = NET_TEST_STAGE_INVALID;
                 break;
             default:
@@ -824,18 +820,27 @@ static int main_channel_handle_migrate_flush_mark(RedChannelClient *rcc)
     return TRUE;
 }
 
+MainChannelClient *main_channel_client_create(MainChannel *main_chan,
+    RedClient *client, RedsStream *stream)
+{
+    MainChannelClient *mcc = (MainChannelClient*)red_channel_client_create(
+        sizeof(MainChannelClient), &main_chan->base, client, stream);
+
+    mcc->bitrate_per_sec = ~0;
+    return mcc;
+}
+
 MainChannelClient *main_channel_link(Channel *channel, RedClient *client,
                         RedsStream *stream, int migration,
                         int num_common_caps, uint32_t *common_caps, int num_caps,
                         uint32_t *caps)
 {
-    MainChannel *main_chan;
     MainChannelClient *mcc;
 
     if (channel->data == NULL) {
         red_printf("create main channel");
         channel->data = red_channel_create_parser(
-            sizeof(*main_chan), core, migration, FALSE /* handle_acks */
+            sizeof(MainChannel), core, migration, FALSE /* handle_acks */
             ,main_channel_config_socket
             ,main_channel_client_disconnect
             ,spice_get_client_channel_parser(SPICE_CHANNEL_MAIN, NULL)
@@ -852,10 +857,8 @@ MainChannelClient *main_channel_link(Channel *channel, RedClient *client,
             ,main_channel_handle_migrate_data_get_serial);
         ASSERT(channel->data);
     }
-    main_chan = (MainChannel*)channel->data;
     red_printf("add main channel client");
-    mcc = (MainChannelClient*)
-            red_channel_client_create(sizeof(MainChannelClient), &main_chan->base, client, stream);
+    mcc = main_channel_client_create(channel->data, client, stream);
     return mcc;
 }
 
@@ -878,6 +881,17 @@ void main_channel_close(MainChannel *main_chan)
     }
 }
 
+int main_channel_client_is_low_bandwidth(MainChannelClient *mcc)
+{
+    // TODO: configurable?
+    return mcc->bitrate_per_sec < 10 * 1024 * 1024;
+}
+
+uint64_t main_channel_client_get_bitrate_per_sec(MainChannelClient *mcc)
+{
+    return mcc->bitrate_per_sec;
+}
+
 static void main_channel_shutdown(Channel *channel)
 {
     MainChannel *main_chan = channel->data;
diff --git a/server/main_channel.h b/server/main_channel.h
index ed17d53..651334d 100644
--- a/server/main_channel.h
+++ b/server/main_channel.h
@@ -76,6 +76,9 @@ void main_channel_push_multi_media_time(MainChannel *main_chan, int time);
 int main_channel_getsockname(MainChannel *main_chan, struct sockaddr *sa, socklen_t *salen);
 int main_channel_getpeername(MainChannel *main_chan, struct sockaddr *sa, socklen_t *salen);
 
+int main_channel_client_is_low_bandwidth(MainChannelClient *mcc);
+uint64_t main_channel_client_get_bitrate_per_sec(MainChannelClient *mcc);
+
 // TODO: Defines used to calculate receive buffer size, and also by reds.c
 // other options: is to make a reds_main_consts.h, to duplicate defines.
 #define REDS_AGENT_WINDOW_SIZE 10
diff --git a/server/red_channel.c b/server/red_channel.c
index d950e59..8606e14 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -953,6 +953,11 @@ RedsStream *red_channel_client_get_stream(RedChannelClient *rcc)
     return rcc->stream;
 }
 
+RedClient *red_channel_client_get_client(RedChannelClient *rcc)
+{
+    return rcc->client;
+}
+
 SpiceDataHeader *red_channel_client_get_header(RedChannelClient *rcc)
 {
     return rcc->send_data.header;
diff --git a/server/red_channel.h b/server/red_channel.h
index 874fdf0..60dcf69 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -345,6 +345,7 @@ void red_channel_client_disconnect(RedChannelClient *rcc);
 /* Note: the valid times to call red_channel_get_marshaller are just during send_item callback. */
 SpiceMarshaller *red_channel_client_get_marshaller(RedChannelClient *rcc);
 RedsStream *red_channel_client_get_stream(RedChannelClient *rcc);
+RedClient *red_channel_client_get_client(RedChannelClient *rcc);
 
 /* this is a convenience function for sending messages, sometimes (migration only?)
  * the serial from the header needs to be available for sending. Note that the header
diff --git a/server/red_worker.c b/server/red_worker.c
index 5a86f0c..dfc934c 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -62,6 +62,7 @@
 #include "zlib_encoder.h"
 #include "red_channel.h"
 #include "red_dispatcher.h"
+#include "main_channel.h"
 
 //#define COMPRESS_STAT
 //#define DUMP_BITMAP
@@ -100,7 +101,7 @@
 
 //best bit rate per pixel base on 13000000 bps for frame size 720x576 pixels and 25 fps
 #define BEST_BIT_RATE_PER_PIXEL 38
-#define WARST_BIT_RATE_PER_PIXEL 4
+#define WORST_BIT_RATE_PER_PIXEL 4
 
 #define RED_COMPRESS_BUF_SIZE (1024 * 64)
 
@@ -937,7 +938,7 @@ static void display_channel_push_release(DisplayChannel *channel, uint8_t type,
 static void red_display_release_stream_clip(DisplayChannel* channel, StreamClipItem *item);
 static int red_display_free_some_independent_glz_drawables(DisplayChannel *channel);
 static void red_display_free_glz_drawable(DisplayChannel *channel, RedGlzDrawable *drawable);
-static void reset_rate(StreamAgent *stream_agent);
+static void reset_rate(RedWorker *worker, StreamAgent *stream_agent);
 static BitmapGradualType _get_bitmap_graduality_level(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
 static inline int _stride_is_extra(SpiceBitmap *bitmap);
 static void red_disconnect_cursor(RedChannel *channel);
@@ -2389,12 +2390,21 @@ static inline Stream *red_alloc_stream(RedWorker *worker)
     return stream;
 }
 
-static int get_bit_rate(int width, int height)
+static int get_bit_rate(RedWorker *worker,
+    int width, int height)
 {
     uint64_t bit_rate = width * height * BEST_BIT_RATE_PER_PIXEL;
-    if (IS_LOW_BANDWIDTH()) {
-        bit_rate = MIN(bitrate_per_sec * 70 / 100, bit_rate);
-        bit_rate = MAX(bit_rate, width * height * WARST_BIT_RATE_PER_PIXEL);
+    MainChannelClient *mcc;
+    int is_low_bandwidth = 0;
+
+    if (display_is_connected(worker)) {
+        mcc = red_client_get_main(worker->display_channel->common.base.rcc->client);
+        is_low_bandwidth = main_channel_client_is_low_bandwidth(mcc);
+    }
+
+    if (is_low_bandwidth) {
+        bit_rate = MIN(main_channel_client_get_bitrate_per_sec(mcc) * 70 / 100, bit_rate);
+        bit_rate = MAX(bit_rate, width * height * WORST_BIT_RATE_PER_PIXEL);
     }
     return bit_rate;
 }
@@ -2412,7 +2422,7 @@ static void red_display_create_stream(DisplayChannel *display, Stream *stream)
     }
     agent->drops = 0;
     agent->fps = MAX_FPS;
-    reset_rate(agent);
+    reset_rate(display->common.worker, agent);
     red_channel_client_pipe_add(display->common.base.rcc, &agent->create_item);
 }
 
@@ -2443,7 +2453,7 @@ static void red_create_stream(RedWorker *worker, Drawable *drawable)
     stream->height = src_rect->bottom - src_rect->top;
     stream->dest_area = drawable->red_drawable->bbox;
     stream->refs = 1;
-    stream->bit_rate = get_bit_rate(stream_width, stream_height);
+    stream->bit_rate = get_bit_rate(worker, stream_width, stream_height);
     SpiceBitmap *bitmap = &drawable->red_drawable->u.copy.src_bitmap->u.bitmap;
     stream->top_down = !!(bitmap->flags & SPICE_BITMAP_FLAGS_TOP_DOWN);
     drawable->stream = stream;
@@ -2552,12 +2562,12 @@ static inline int red_is_next_stream_frame(RedWorker *worker, const Drawable *ca
                                       prev->stream);
 }
 
-static void reset_rate(StreamAgent *stream_agent)
+static void reset_rate(RedWorker *worker, StreamAgent *stream_agent)
 {
     Stream *stream = stream_agent->stream;
     int rate;
 
-    rate = get_bit_rate(stream->width, stream->height);
+    rate = get_bit_rate(worker, stream->width, stream->height);
     if (rate == stream->bit_rate) {
         return;
     }
@@ -2565,11 +2575,22 @@ static void reset_rate(StreamAgent *stream_agent)
     /* MJpeg has no rate limiting anyway, so do nothing */
 }
 
+static int display_channel_is_low_bandwidth(DisplayChannel *display_channel)
+{
+    if (display_channel->common.base.rcc) {
+        if (main_channel_client_is_low_bandwidth(
+                red_client_get_main(display_channel->common.base.rcc->client))) {
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+
 static inline void pre_stream_item_swap(RedWorker *worker, Stream *stream)
 {
     ASSERT(stream->current);
 
-    if (!display_is_connected(worker) || !IS_LOW_BANDWIDTH()) {
+    if (!display_is_connected(worker) || !display_channel_is_low_bandwidth(worker->display_channel)) {
         return;
     }
 
@@ -8999,9 +9020,11 @@ static int display_channel_handle_message(RedChannelClient *rcc, uint32_t size,
 
 static int common_channel_config_socket(RedChannelClient *rcc)
 {
+    RedClient *client = red_channel_client_get_client(rcc);
+    MainChannelClient *mcc = red_client_get_main(client);
+    RedsStream *stream = red_channel_client_get_stream(rcc);
     int flags;
     int delay_val;
-    RedsStream *stream = red_channel_client_get_stream(rcc);
 
     if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
         red_printf("accept failed, %s", strerror(errno));
@@ -9014,7 +9037,7 @@ static int common_channel_config_socket(RedChannelClient *rcc)
     }
 
     // TODO - this should be dynamic, not one time at channel creation
-    delay_val = IS_LOW_BANDWIDTH() ? 0 : 1;
+    delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
     if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val, sizeof(delay_val)) == -1) {
         red_printf("setsockopt failed, %s", strerror(errno));
     }
@@ -9064,6 +9087,7 @@ static RedChannel *__new_channel(RedWorker *worker, int size, uint32_t channel_i
     struct epoll_event event;
     RedChannel *channel;
     CommonChannel *common;
+    MainChannelClient *mcc = red_client_get_main(client);
 
     channel = red_channel_create_parser(size, &worker_core, migrate,
                                         TRUE /* handle_acks */,
@@ -9094,7 +9118,8 @@ static RedChannel *__new_channel(RedWorker *worker, int size, uint32_t channel_i
     // TODO: Should this be distinctive for the Display/Cursor channels? doesn't
     // make sense, does it?
     red_channel_ack_set_client_window(channel,
-        IS_LOW_BANDWIDTH() ? WIDE_CLIENT_ACK_WINDOW : NARROW_CLIENT_ACK_WINDOW);
+        main_channel_client_is_low_bandwidth(mcc) ?
+        WIDE_CLIENT_ACK_WINDOW : NARROW_CLIENT_ACK_WINDOW);
 
     event.events = EPOLLIN | EPOLLOUT | EPOLLET;
     event.data.ptr = &common->listener;
@@ -9225,6 +9250,7 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
 {
     DisplayChannel *display_channel;
     size_t stream_buf_size;
+    int is_low_bandwidth = main_channel_client_is_low_bandwidth(red_client_get_main(client));
 
     red_disconnect_all_display_TODO_remove_me((RedChannel *)worker->display_channel);
 
@@ -9273,7 +9299,7 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
     display_channel->send_data.free_list.res_size = DISPLAY_FREE_LIST_DEFAULT_SIZE;
 
     if (worker->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
-        display_channel->enable_jpeg = IS_LOW_BANDWIDTH();
+        display_channel->enable_jpeg = is_low_bandwidth;
     } else {
         display_channel->enable_jpeg = (worker->jpeg_state == SPICE_WAN_COMPRESSION_ALWAYS);
     }
@@ -9282,7 +9308,7 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
     display_channel->jpeg_quality = 85;
 
     if (worker->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
-        display_channel->enable_zlib_glz_wrap = IS_LOW_BANDWIDTH();
+        display_channel->enable_zlib_glz_wrap = is_low_bandwidth;
     } else {
         display_channel->enable_zlib_glz_wrap = (worker->zlib_glz_state == 
                                                  SPICE_WAN_COMPRESSION_ALWAYS);
diff --git a/server/reds.h b/server/reds.h
index 29ce15f..eca3712 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -133,9 +133,6 @@ int reds_has_vdagent(void); // used by inputs channel
 void reds_handle_agent_mouse_event(const VDAgentMouseState *mouse_state); // used by inputs_channel
 
 extern struct SpiceCoreInterface *core;
-extern uint64_t bitrate_per_sec;
-
-#define IS_LOW_BANDWIDTH() (bitrate_per_sec < 10 * 1024 * 1024)
 
 // Temporary measures to make splitting reds.c to inputs_channel.c easier
 void reds_client_disconnect(RedClient *client);
diff --git a/server/snd_worker.c b/server/snd_worker.c
index 71b7f19..157e1b8 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -35,6 +35,12 @@
 #include "generated_marshallers.h"
 #include "demarshallers.h"
 
+/* main_channel.h inclusion drags red_channel.h which has conflicting types.
+ * until the channels here are defined in terms of red_channel.h we have some
+ * duplicate declarations */
+MainChannelClient *red_client_get_main(RedClient *client);
+int main_channel_client_is_low_bandwidth(MainChannelClient *mcc);
+
 #define MAX_SEND_VEC 100
 
 #define RECIVE_BUF_SIZE (16 * 1024 * 2)
@@ -739,6 +745,7 @@ static void snd_record_send(void* data)
 }
 
 static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_id,
+                                 RedClient *client,
                                  RedsStream *stream,
                                  int migrate, send_messages_proc send_messages,
                                  handle_message_proc handle_message,
@@ -750,6 +757,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
     int flags;
     int priority;
     int tos;
+    MainChannelClient *mcc = red_client_get_main(client);
 
     if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
         red_printf("accept failed, %s", strerror(errno));
@@ -767,7 +775,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
         red_printf("setsockopt failed, %s", strerror(errno));
     }
 
-    delay_val = IS_LOW_BANDWIDTH() ? 0 : 1;
+    delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
     if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val, sizeof(delay_val)) == -1) {
         red_printf("setsockopt failed, %s", strerror(errno));
     }
@@ -964,6 +972,7 @@ static void snd_set_playback_peer(Channel *channel, RedClient *client, RedsStrea
     if (!(playback_channel = (PlaybackChannel *)__new_channel(worker,
                                                               sizeof(*playback_channel),
                                                               SPICE_CHANNEL_PLAYBACK,
+                                                              client,
                                                               stream,
                                                               migration,
                                                               snd_playback_send,
@@ -1130,6 +1139,7 @@ static void snd_set_record_peer(Channel *channel, RedClient *client, RedsStream
     if (!(record_channel = (RecordChannel *)__new_channel(worker,
                                                           sizeof(*record_channel),
                                                           SPICE_CHANNEL_RECORD,
+                                                          client,
                                                           stream,
                                                           migration,
                                                           snd_record_send,
-- 
1.7.5.1



More information about the Spice-devel mailing list