[Spice-devel] [RFC v4 14/62] server/main_channel: move latency and bitrate to channel client

Alon Levy alevy at redhat.com
Tue Apr 26 03:54:39 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 |   58 +++++++++++++++++++++++++++++------------------
 server/main_channel.h |    3 ++
 server/red_channel.c  |    5 ++++
 server/red_channel.h  |    1 +
 server/red_worker.c   |   60 +++++++++++++++++++++++++++++++++++--------------
 server/reds.h         |    4 +--
 server/snd_worker.c   |   12 +++++++++-
 7 files changed, 100 insertions(+), 43 deletions(-)

diff --git a/server/main_channel.c b/server/main_channel.c
index c2e2465..121bdaa 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -114,12 +114,14 @@ typedef struct MultiMediaTimePipeItem {
     int time;
 } MultiMediaTimePipeItem;
 
-typedef struct MainChannelClient {
+struct MainChannelClient {
     RedChannelClient base;
     uint32_t ping_id;
     uint32_t net_test_id;
     int net_test_stage;
-} MainChannelClient;
+    uint64_t latency;
+    uint64_t bitrate_per_sec;
+};
 
 struct MainChannel {
     RedChannel base;
@@ -133,9 +135,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);
@@ -144,9 +143,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);
@@ -752,23 +748,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:
@@ -829,18 +825,27 @@ static int main_channel_handle_migrate_flush_mark_proc(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)
@@ -857,10 +862,8 @@ MainChannelClient *main_channel_link(Channel *channel, RedClient *client,
             ,main_channel_handle_migrate_data_get_serial_proc);
         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;
 }
 
@@ -883,6 +886,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 9d3aaab..9cd4c4e 100644
--- a/server/main_channel.h
+++ b/server/main_channel.h
@@ -77,6 +77,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 4c66f9a..abb6b4d 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -933,6 +933,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 14f097a..2255f7d 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -343,6 +343,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 950715e..ebeb4af 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -59,6 +59,7 @@
 #include "zlib_encoder.h"
 #include "red_channel.h"
 #include "red_dispatcher.h"
+#include "main_channel.h"
 
 //#define COMPRESS_STAT
 //#define DUMP_BITMAP
@@ -97,7 +98,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)
 
@@ -934,7 +935,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);
@@ -2378,12 +2379,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);
+    uint64_t bit_rate = width * height * BEST_BIT_RATE_PER_PIXEL; 
+    MainChannelClient *mcc;
+    int is_low_bandwidth = 0;
+    
+    if (display_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;
 }
@@ -2401,7 +2411,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);
 }
 
@@ -2432,7 +2442,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;
@@ -2541,12 +2551,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;
     }
@@ -2554,11 +2564,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_connected(worker) || !IS_LOW_BANDWIDTH()) {
+    if (!display_connected(worker) || !display_channel_is_low_bandwidth(worker->display_channel)) {
         return;
     }
 
@@ -8989,9 +9010,11 @@ static int display_channel_handle_message(RedChannelClient *rcc, uint32_t size,
 
 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));
@@ -9004,7 +9027,7 @@ 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));
     }
@@ -9054,6 +9077,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 */,
@@ -9084,7 +9108,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;
@@ -9215,6 +9240,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);
 
@@ -9263,7 +9289,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);
     }
@@ -9272,7 +9298,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 b95decd..8eea164 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -36,6 +36,7 @@
 
 typedef struct RedsStream RedsStream;
 typedef struct RedClient RedClient;
+typedef struct MainChannelClient MainChannelClient;
 
 #if HAVE_SASL
 typedef struct RedsSASL {
@@ -134,9 +135,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 6bf1edd..04d7b52 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -32,6 +32,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)
@@ -736,6 +742,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,
@@ -747,6 +754,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));
@@ -764,7 +772,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));
     }
@@ -961,6 +969,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,
@@ -1127,6 +1136,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.4.4



More information about the Spice-devel mailing list