<div dir="ltr">ack<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 8, 2013 at 4:06 PM, Yonit Halperin <span dir="ltr"><<a href="mailto:yhalperi@redhat.com" target="_blank">yhalperi@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Replace the mixed calls to display_channel_client_is_low_bandwidth<br>
and to main_channel_client_is_low_bandwidth, with one flag in<br>
CommonChannelClient that is set upon channel creation.<br>
---<br>
server/red_worker.c | 29 +++++++++++++++--------------<br>
1 file changed, 15 insertions(+), 14 deletions(-)<br>
<br>
diff --git a/server/red_worker.c b/server/red_worker.c<br>
index decbfbb..be53c1d 100644<br>
--- a/server/red_worker.c<br>
+++ b/server/red_worker.c<br>
@@ -665,6 +665,7 @@ typedef struct CommonChannelClient {<br>
RedChannelClient base;<br>
uint32_t id;<br>
struct RedWorker *worker;<br>
+ int is_low_bandwidth;<br>
} CommonChannelClient;<br>
<br>
/* Each drawable can refer to at most 3 images: src, brush and mask */<br>
@@ -3287,12 +3288,6 @@ static inline int red_is_next_stream_frame(RedWorker *worker, const Drawable *ca<br>
FALSE);<br>
}<br>
<br>
-static int display_channel_client_is_low_bandwidth(DisplayChannelClient *dcc)<br>
-{<br>
- return main_channel_client_is_low_bandwidth(<br>
- red_client_get_main(red_channel_client_get_client(&dcc->common.base)));<br>
-}<br>
-<br>
static inline void pre_stream_item_swap(RedWorker *worker, Stream *stream, Drawable *new_frame)<br>
{<br>
DrawablePipeItem *dpi;<br>
@@ -3318,7 +3313,7 @@ static inline void pre_stream_item_swap(RedWorker *worker, Stream *stream, Drawa<br>
agent = &dcc->stream_agents[index];<br>
<br>
if (!dcc->use_mjpeg_encoder_rate_control &&<br>
- !display_channel_client_is_low_bandwidth(dcc)) {<br>
+ !dcc->common.is_low_bandwidth) {<br>
continue;<br>
}<br>
<br>
@@ -8768,7 +8763,7 @@ static void display_channel_marshall_migrate_data(RedChannelClient *rcc,<br>
MIGRATE_DATA_DISPLAY_MAX_CACHE_CLIENTS == MAX_CACHE_CLIENTS);<br>
<br>
display_data.message_serial = red_channel_client_get_message_serial(rcc);<br>
- display_data.low_bandwidth_setting = display_channel_client_is_low_bandwidth(dcc);<br>
+ display_data.low_bandwidth_setting = dcc->common.is_low_bandwidth;<br>
<br>
display_data.pixmap_cache_freezer = pixmap_cache_freeze(dcc->pixmap_cache);<br>
display_data.pixmap_cache_id = dcc->pixmap_cache->id;<br>
@@ -10286,6 +10281,7 @@ static int common_channel_config_socket(RedChannelClient *rcc)<br>
RedClient *client = red_channel_client_get_client(rcc);<br>
MainChannelClient *mcc = red_client_get_main(client);<br>
RedsStream *stream = red_channel_client_get_stream(rcc);<br>
+ CommonChannelClient *ccc = SPICE_CONTAINEROF(rcc, CommonChannelClient, base);<br>
int flags;<br>
int delay_val;<br>
<br>
@@ -10300,7 +10296,14 @@ static int common_channel_config_socket(RedChannelClient *rcc)<br>
}<br>
<br>
// TODO - this should be dynamic, not one time at channel creation<br>
- delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;<br>
+ ccc->is_low_bandwidth = main_channel_client_is_low_bandwidth(mcc);<br>
+ delay_val = ccc->is_low_bandwidth ? 0 : 1;<br>
+ /* FIXME: Using Nagle's Algorithm can lead to apparent delays, depending<br>
+ * on the delayed ack timeout on the other side.<br>
+ * Instead of using Nagle's, we need to implement message buffering on<br>
+ * the application level.<br>
+ * see: <a href="http://www.stuartcheshire.org/papers/NagleDelayedAck/" target="_blank">http://www.stuartcheshire.org/papers/NagleDelayedAck/</a><br>
+ */<br>
if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,<br>
sizeof(delay_val)) == -1) {<br>
if (errno != ENOTSUP) {<br>
@@ -10402,7 +10405,6 @@ static CommonChannelClient *common_channel_client_create(int size,<br>
uint32_t *caps,<br>
int num_caps)<br>
{<br>
- MainChannelClient *mcc = red_client_get_main(client);<br>
RedChannelClient *rcc =<br>
red_channel_client_create(size, &common->base, client, stream, monitor_latency,<br>
num_common_caps, common_caps, num_caps, caps);<br>
@@ -10416,7 +10418,7 @@ static CommonChannelClient *common_channel_client_create(int size,<br>
<br>
// TODO: move wide/narrow ack setting to red_channel.<br>
red_channel_client_ack_set_client_window(rcc,<br>
- main_channel_client_is_low_bandwidth(mcc) ?<br>
+ common_cc->is_low_bandwidth ?<br>
WIDE_CLIENT_ACK_WINDOW : NARROW_CLIENT_ACK_WINDOW);<br>
return common_cc;<br>
}<br>
@@ -10749,7 +10751,6 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red<br>
DisplayChannel *display_channel;<br>
DisplayChannelClient *dcc;<br>
size_t stream_buf_size;<br>
- int is_low_bandwidth = main_channel_client_is_low_bandwidth(red_client_get_main(client));<br>
<br>
if (!worker->display_channel) {<br>
spice_warning("Display channel was not created");<br>
@@ -10776,7 +10777,7 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red<br>
dcc->send_data.free_list.res_size = DISPLAY_FREE_LIST_DEFAULT_SIZE;<br>
<br>
if (worker->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {<br>
- display_channel->enable_jpeg = is_low_bandwidth;<br>
+ display_channel->enable_jpeg = dcc->common.is_low_bandwidth;<br>
} else {<br>
display_channel->enable_jpeg = (worker->jpeg_state == SPICE_WAN_COMPRESSION_ALWAYS);<br>
}<br>
@@ -10785,7 +10786,7 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red<br>
display_channel->jpeg_quality = 85;<br>
<br>
if (worker->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {<br>
- display_channel->enable_zlib_glz_wrap = is_low_bandwidth;<br>
+ display_channel->enable_zlib_glz_wrap = dcc->common.is_low_bandwidth;<br>
} else {<br>
display_channel->enable_zlib_glz_wrap = (worker->zlib_glz_state ==<br>
SPICE_WAN_COMPRESSION_ALWAYS);<br>
<span class="HOEnZb"><font color="#888888">--<br>
1.8.1.4<br>
<br>
_______________________________________________<br>
Spice-devel mailing list<br>
<a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/spice-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/spice-devel</a><br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br>Marc-André Lureau
</div>