[Spice-devel] [PATCH spice-server 02/10] red_worker: cleanup: add is_low_bandwidth flag to CommonChannelClient

Marc-André Lureau marcandre.lureau at gmail.com
Wed May 8 07:46:54 PDT 2013


ack


On Wed, May 8, 2013 at 4:06 PM, Yonit Halperin <yhalperi at redhat.com> wrote:

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



-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20130508/9bcf2b63/attachment.html>


More information about the Spice-devel mailing list