[Spice-devel] [PATCH spice-server 4/5] server: add support for SPICE_COMMON_CAP_MINI_HEADER

Alon Levy alevy at redhat.com
Mon Jan 9 02:04:47 PST 2012


On Sun, Jan 08, 2012 at 10:53:32AM +0200, Yonit Halperin wrote:
> Support for a header without a serial and without sub list.
> red_channel: Support the two types of headers.
>              Keep a consistent consecutive messages serial.
> red_worker: use urgent marshaller instead of sub list.
> snd_worker: Sound channels need special support since they still don't use
>             red_channel for sending & receiving.
> ---
>  server/red_channel.c |  209 +++++++++++++++++++++++++++++++++++++------------
>  server/red_channel.h |   35 ++++++++-
>  server/red_worker.c  |   88 +++++++++++++++++-----
>  server/snd_worker.c  |   93 ++++++++++++++---------
>  4 files changed, 317 insertions(+), 108 deletions(-)
> 
> diff --git a/server/red_channel.c b/server/red_channel.c
> index 3a9f254..f5b99b0 100644
> --- a/server/red_channel.c
> +++ b/server/red_channel.c
> @@ -40,6 +40,81 @@ static void red_client_add_channel(RedClient *client, RedChannelClient *rcc);
>  static void red_client_remove_channel(RedChannelClient *rcc);
>  static void red_channel_client_restore_main_sender(RedChannelClient *rcc);
>  
> +static uint32_t full_header_get_msg_size(SpiceDataHeaderOpaque *header)
> +{
> +    return ((SpiceDataHeader *)header->data)->size;
> +}
> +
> +static uint32_t mini_header_get_msg_size(SpiceDataHeaderOpaque *header)
> +{
> +    return ((SpiceMiniDataHeader *)header->data)->size;
> +}
> +
> +static uint16_t full_header_get_msg_type(SpiceDataHeaderOpaque *header)
> +{
> +    return ((SpiceDataHeader *)header->data)->type;
> +}
> +
> +static uint16_t mini_header_get_msg_type(SpiceDataHeaderOpaque *header)
> +{
> +    return ((SpiceMiniDataHeader *)header->data)->type;
> +}
> +
> +void full_header_set_msg_type(SpiceDataHeaderOpaque *header, uint16_t type)
> +{
> +    ((SpiceDataHeader *)header->data)->type = type;
> +}
> +
> +void mini_header_set_msg_type(SpiceDataHeaderOpaque *header, uint16_t type)
> +{
> +    ((SpiceMiniDataHeader *)header->data)->type = type;
> +}
> +
> +void full_header_set_msg_size(SpiceDataHeaderOpaque *header, uint32_t size)
> +{
> +    ((SpiceDataHeader *)header->data)->size = size;
> +}
> +
> +void mini_header_set_msg_size(SpiceDataHeaderOpaque *header, uint32_t size)
> +{
> +    ((SpiceMiniDataHeader *)header->data)->size = size;
> +}
> +
> +void full_header_set_msg_serial(SpiceDataHeaderOpaque *header, uint64_t serial)
> +{
> +    ((SpiceDataHeader *)header->data)->serial = serial;
> +}
> +
> +void mini_header_set_msg_serial(SpiceDataHeaderOpaque *header, uint64_t serial)
> +{

red_error ?

> +}
> +
> +void full_header_set_msg_sub_list(SpiceDataHeaderOpaque *header, uint32_t sub_list)
> +{
> +    ((SpiceDataHeader *)header->data)->sub_list = sub_list;
> +}
> +
> +void mini_header_set_msg_sub_list(SpiceDataHeaderOpaque *header, uint32_t sub_list)
> +{
> +    red_error("attempt to set header sub list on mini header");
> +}
> +
> +static SpiceDataHeaderOpaque full_header_wrapper = {NULL, sizeof(SpiceDataHeader),
> +                                                    full_header_set_msg_type,
> +                                                    full_header_set_msg_size,
> +                                                    full_header_set_msg_serial,
> +                                                    full_header_set_msg_sub_list,
> +                                                    full_header_get_msg_type,
> +                                                    full_header_get_msg_size};
> +
> +static SpiceDataHeaderOpaque mini_header_wrapper = {NULL, sizeof(SpiceMiniDataHeader),
> +                                                    mini_header_set_msg_type,
> +                                                    mini_header_set_msg_size,
> +                                                    mini_header_set_msg_serial,
> +                                                    mini_header_set_msg_sub_list,
> +                                                    mini_header_get_msg_type,
> +                                                    mini_header_get_msg_size};
> +
>  /* return the number of bytes read. -1 in case of error */
>  static int red_peer_receive(RedsStream *stream, uint8_t *buf, uint32_t size)
>  {
> @@ -83,29 +158,31 @@ static void red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
>      uint8_t *parsed;
>      size_t parsed_size;
>      message_destructor_t parsed_free;
> +    uint16_t msg_type;
> +    uint32_t msg_size;
>  
>      for (;;) {
>          int ret_handle;
> -        if (handler->header_pos < sizeof(SpiceDataHeader)) {
> +        if (handler->header_pos < handler->header.header_size) {
>              bytes_read = red_peer_receive(stream,
> -                                          ((uint8_t *)&handler->header) + handler->header_pos,
> -                                          sizeof(SpiceDataHeader) - handler->header_pos);
> +                                          handler->header.data + handler->header_pos,
> +                                          handler->header.header_size - handler->header_pos);
>              if (bytes_read == -1) {
>                  handler->cb->on_error(handler->opaque);
>                  return;
>              }
>              handler->header_pos += bytes_read;
>  
> -            if (handler->header_pos != sizeof(SpiceDataHeader)) {
> +            if (handler->header_pos != handler->header.header_size) {
>                  return;
>              }
>          }
>  
> -        if (handler->msg_pos < handler->header.size) {
> +        msg_size = handler->header.get_msg_size(&handler->header);
> +        msg_type =  handler->header.get_msg_type(&handler->header);
> +        if (handler->msg_pos < msg_size) {
>              if (!handler->msg) {
> -                handler->msg = handler->cb->alloc_msg_buf(handler->opaque,
> -                                                          handler->header.type,
> -                                                          handler->header.size);
> +                handler->msg = handler->cb->alloc_msg_buf(handler->opaque, msg_type, msg_size);
>                  if (handler->msg == NULL) {
>                      red_printf("ERROR: channel refused to allocate buffer.");
>                      handler->cb->on_error(handler->opaque);
> @@ -115,47 +192,37 @@ static void red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
>  
>              bytes_read = red_peer_receive(stream,
>                                            handler->msg + handler->msg_pos,
> -                                          handler->header.size - handler->msg_pos);
> +                                          msg_size - handler->msg_pos);
>              if (bytes_read == -1) {
> -                handler->cb->release_msg_buf(handler->opaque,
> -                                             handler->header.type,
> -                                             handler->header.size,
> -                                             handler->msg);
> +                handler->cb->release_msg_buf(handler->opaque, msg_type, msg_size, handler->msg);
>                  handler->cb->on_error(handler->opaque);
>                  return;
>              }
>              handler->msg_pos += bytes_read;
> -            if (handler->msg_pos != handler->header.size) {
> +            if (handler->msg_pos != msg_size) {
>                  return;
>              }
>          }
>  
>          if (handler->cb->parser) {
>              parsed = handler->cb->parser(handler->msg,
> -                handler->msg + handler->header.size, handler->header.type,
> +                handler->msg + msg_size, msg_type,
>                  SPICE_VERSION_MINOR, &parsed_size, &parsed_free);
>              if (parsed == NULL) {
> -                red_printf("failed to parse message type %d", handler->header.type);
> -                handler->cb->release_msg_buf(handler->opaque, handler->header.type,
> -                                             handler->header.size,
> -                                             handler->msg);
> +                red_printf("failed to parse message type %d", msg_type);
> +                handler->cb->release_msg_buf(handler->opaque, msg_type, msg_size, handler->msg);
>                  handler->cb->on_error(handler->opaque);
>                  return;
>              }
>              ret_handle = handler->cb->handle_parsed(handler->opaque, parsed_size,
> -                                    handler->header.type, parsed);
> +                                                    msg_type, parsed);
>              parsed_free(parsed);
>          } else {
> -            ret_handle = handler->cb->handle_message(handler->opaque,
> -                                                     handler->header.type,
> -                                                     handler->header.size,
> +            ret_handle = handler->cb->handle_message(handler->opaque, msg_type, msg_size,
>                                                       handler->msg);
>          }
>          handler->msg_pos = 0;
> -        handler->cb->release_msg_buf(handler->opaque,
> -                                     handler->header.type,
> -                                     handler->header.size,
> -                                     handler->msg);
> +        handler->cb->release_msg_buf(handler->opaque, msg_type, msg_size, handler->msg);
>          handler->msg = NULL;
>          handler->header_pos = 0;
>  
> @@ -271,17 +338,27 @@ static void red_channel_client_peer_on_out_block(void *opaque)
>  static void red_channel_client_reset_send_data(RedChannelClient *rcc)
>  {
>      spice_marshaller_reset(rcc->send_data.marshaller);
> -    rcc->send_data.header = (SpiceDataHeader *)
> -        spice_marshaller_reserve_space(rcc->send_data.marshaller, sizeof(SpiceDataHeader));
> -    spice_marshaller_set_base(rcc->send_data.marshaller, sizeof(SpiceDataHeader));
> -    rcc->send_data.header->type = 0;
> -    rcc->send_data.header->size = 0;
> -    rcc->send_data.header->sub_list = 0;
> -
> -    if (rcc->send_data.marshaller != rcc->send_data.urgent.marshaller) {
> -        rcc->send_data.header->serial = ++rcc->send_data.serial;
> -    } else {
> -        rcc->send_data.header->serial = rcc->send_data.serial++;
> +    rcc->send_data.header.data = spice_marshaller_reserve_space(rcc->send_data.marshaller,
> +                                                                rcc->send_data.header.header_size);
> +    spice_marshaller_set_base(rcc->send_data.marshaller, rcc->send_data.header.header_size);
> +    rcc->send_data.header.set_msg_type(&rcc->send_data.header, 0);
> +    rcc->send_data.header.set_msg_size(&rcc->send_data.header, 0);
> +
> +    /* Keeping the serial consecutive: reseting it if reset_send_data
> +     * has been called before, but no message has been sent since then.
> +     */
> +    if (rcc->send_data.last_sent_serial != rcc->send_data.serial) {
> +        ASSERT(rcc->send_data.serial - rcc->send_data.last_sent_serial == 1);
> +        if (rcc->send_data.marshaller != rcc->send_data.urgent.marshaller) {
> +            rcc->send_data.serial = rcc->send_data.last_sent_serial;
> +        }
> +    }
> +    rcc->send_data.serial++;
> +
> +    if (!rcc->is_mini_header) {
> +        ASSERT(rcc->send_data.marshaller != rcc->send_data.urgent.marshaller);
> +        rcc->send_data.header.set_msg_sub_list(&rcc->send_data.header, 0);
> +        rcc->send_data.header.set_msg_serial(&rcc->send_data.header, rcc->send_data.serial);
>      }
>  }
>  
> @@ -367,7 +444,7 @@ static void red_channel_peer_on_out_msg_done(void *opaque)
>  
>      if (rcc->send_data.marshaller == rcc->send_data.urgent.marshaller) {
>          red_channel_client_restore_main_sender(rcc);
> -        ASSERT(rcc->send_data.header != NULL);
> +        ASSERT(rcc->send_data.header.data != NULL);
>          red_channel_client_begin_send_message(rcc);
>      }
>  }
> @@ -448,6 +525,18 @@ RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedCl
>      rcc->outgoing.size = 0;
>  
>      red_channel_client_set_remote_caps(rcc, num_common_caps, common_caps, num_caps, caps);
> +    if (red_channel_client_test_remote_common_cap(rcc, SPICE_COMMON_CAP_MINI_HEADER)) {
> +        rcc->incoming.header = mini_header_wrapper;
> +        rcc->send_data.header = mini_header_wrapper;
> +        rcc->is_mini_header = TRUE;
> +    } else {
> +        rcc->incoming.header = full_header_wrapper;
> +        rcc->send_data.header = full_header_wrapper;
> +        rcc->is_mini_header = FALSE;
> +    }
> +
> +    rcc->incoming.header.data = rcc->incoming.header_buf;
> +    rcc->incoming.serial = 1;
>  
>      if (!channel->channel_cbs.config_socket(rcc)) {
>          goto error;
> @@ -536,6 +625,7 @@ RedChannel *red_channel_create(int size,
>      client_cbs.migrate = red_channel_client_default_migrate;
>  
>      red_channel_register_client_cbs(channel, &client_cbs);
> +    red_channel_set_common_cap(channel, SPICE_COMMON_CAP_MINI_HEADER);
>  
>      channel->thread_id = pthread_self();
>  
> @@ -581,6 +671,7 @@ RedChannel *red_channel_create_dummy(int size, uint32_t type, uint32_t id)
>      client_cbs.migrate = red_channel_client_default_migrate;
>  
>      red_channel_register_client_cbs(channel, &client_cbs);
> +    red_channel_set_common_cap(channel, SPICE_COMMON_CAP_MINI_HEADER);
>  
>      channel->thread_id = pthread_self();
>  
> @@ -841,7 +932,7 @@ static void red_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t size
>  }
>  
>  int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
> -                               uint16_t type, void *message)
> +                                      uint16_t type, void *message)
>  {
>      switch (type) {
>      case SPICE_MSGC_ACK_SYNC:
> @@ -888,7 +979,7 @@ void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type,
>  {
>      ASSERT(red_channel_client_no_item_being_sent(rcc));
>      ASSERT(msg_type != 0);
> -    rcc->send_data.header->type = msg_type;
> +    rcc->send_data.header.set_msg_type(&rcc->send_data.header, msg_type);
>      rcc->send_data.item = item;
>      if (item) {
>          rcc->channel->channel_cbs.hold_item(rcc, item);
> @@ -900,23 +991,25 @@ void red_channel_client_begin_send_message(RedChannelClient *rcc)
>      SpiceMarshaller *m = rcc->send_data.marshaller;
>  
>      // TODO - better check: type in channel_allowed_types. Better: type in channel_allowed_types(channel_state)
> -    if (rcc->send_data.header->type == 0) {
> +    if (rcc->send_data.header.get_msg_type(&rcc->send_data.header) == 0) {
>          red_printf("BUG: header->type == 0");
>          return;
>      }
>      spice_marshaller_flush(m);
>      rcc->send_data.size = spice_marshaller_get_total_size(m);
> -    rcc->send_data.header->size = rcc->send_data.size - sizeof(SpiceDataHeader);
> +    rcc->send_data.header.set_msg_size(&rcc->send_data.header,
> +                                       rcc->send_data.size - rcc->send_data.header.header_size);
>      rcc->ack_data.messages_window++;
> -    rcc->send_data.header = NULL; /* avoid writing to this until we have a new message */
> +    rcc->send_data.last_sent_serial = rcc->send_data.serial;
> +    rcc->send_data.header.data = NULL; /* avoid writing to this until we have a new message */
>      red_channel_client_send(rcc);
>  }
>  
>  SpiceMarshaller *red_channel_client_switch_to_urgent_sender(RedChannelClient *rcc)
>  {
>      ASSERT(red_channel_client_no_item_being_sent(rcc));
> -    ASSERT(rcc->send_data.header != NULL);
> -    rcc->send_data.main.header = rcc->send_data.header;
> +    ASSERT(rcc->send_data.header.data != NULL);
> +    rcc->send_data.main.header_data = rcc->send_data.header.data;
>      rcc->send_data.main.item = rcc->send_data.item;
>  
>      rcc->send_data.marshaller = rcc->send_data.urgent.marshaller;
> @@ -929,8 +1022,10 @@ static void red_channel_client_restore_main_sender(RedChannelClient *rcc)
>  {
>      spice_marshaller_reset(rcc->send_data.urgent.marshaller);
>      rcc->send_data.marshaller = rcc->send_data.main.marshaller;
> -    rcc->send_data.header = rcc->send_data.main.header;
> -    rcc->send_data.header->serial = rcc->send_data.serial;
> +    rcc->send_data.header.data = rcc->send_data.main.header_data;
> +    if (!rcc->is_mini_header) {
> +        rcc->send_data.header.set_msg_serial(&rcc->send_data.header, rcc->send_data.serial);
> +    }
>      rcc->send_data.item = rcc->send_data.main.item;
>  }
>  
> @@ -1060,7 +1155,6 @@ void red_channel_client_ack_set_client_window(RedChannelClient *rcc, int client_
>      rcc->ack_data.client_window = client_window;
>  }
>  
> -
>  static void red_channel_remove_client(RedChannelClient *rcc)
>  {
>      ASSERT(pthread_equal(pthread_self(), rcc->channel->thread_id));
> @@ -1118,6 +1212,19 @@ RedChannelClient *red_channel_client_create_dummy(int size,
>      rcc->client = client;
>      rcc->channel = channel;
>      red_channel_client_set_remote_caps(rcc, num_common_caps, common_caps, num_caps, caps);
> +    if (red_channel_client_test_remote_common_cap(rcc, SPICE_COMMON_CAP_MINI_HEADER)) {
> +        rcc->incoming.header = mini_header_wrapper;
> +        rcc->send_data.header = mini_header_wrapper;
> +        rcc->is_mini_header = TRUE;
> +    } else {
> +        rcc->incoming.header = full_header_wrapper;
> +        rcc->send_data.header = full_header_wrapper;
> +        rcc->is_mini_header = FALSE;
> +    }
> +
> +    rcc->incoming.header.data = rcc->incoming.header_buf;
> +    rcc->incoming.serial = 1;
> +
>      red_channel_add_client(channel, rcc);
>      return rcc;
>  }
> @@ -1191,7 +1298,7 @@ int red_channel_client_blocked(RedChannelClient *rcc)
>  
>  int red_channel_client_send_message_pending(RedChannelClient *rcc)
>  {
> -    return rcc->send_data.header->type != 0;
> +    return rcc->send_data.header.get_msg_type(&rcc->send_data.header) != 0;
>  }
>  
>  /* accessors for RedChannelClient */
> @@ -1212,7 +1319,7 @@ RedClient *red_channel_client_get_client(RedChannelClient *rcc)
>  
>  void red_channel_client_set_header_sub_list(RedChannelClient *rcc, uint32_t sub_list)
>  {
> -    rcc->send_data.header->sub_list = sub_list;
> +    rcc->send_data.header.set_msg_sub_list(&rcc->send_data.header, sub_list);
>  }
>  
>  /* end of accessors */
> diff --git a/server/red_channel.h b/server/red_channel.h
> index 40792c1..045bfd4 100644
> --- a/server/red_channel.h
> +++ b/server/red_channel.h
> @@ -33,10 +33,34 @@
>  #define MAX_SEND_VEC 100
>  #define CLIENT_ACK_WINDOW 20
>  
> +#define MAX_HEADER_SIZE sizeof(SpiceDataHeader)
> +
>  /* Basic interface for channels, without using the RedChannel interface.
>     The intention is to move towards one channel interface gradually.
>     At the final stage, this interface shouldn't be exposed. Only RedChannel will use it. */
>  
> +typedef struct SpiceDataHeaderOpaque SpiceDataHeaderOpaque;
> +
> +typedef uint16_t (*get_msg_type_proc)(SpiceDataHeaderOpaque *header);
> +typedef uint32_t (*get_msg_size_proc)(SpiceDataHeaderOpaque *header);
> +typedef void (*set_msg_type_proc)(SpiceDataHeaderOpaque *header, uint16_t type);
> +typedef void (*set_msg_size_proc)(SpiceDataHeaderOpaque *header, uint32_t size);
> +typedef void (*set_msg_serial_proc)(SpiceDataHeaderOpaque *header, uint64_t serial);
> +typedef void (*set_msg_sub_list_proc)(SpiceDataHeaderOpaque *header, uint32_t sub_list);
> +
> +struct SpiceDataHeaderOpaque {
> +    uint8_t *data;
> +    uint16_t header_size;
> +
> +    set_msg_type_proc set_msg_type;
> +    set_msg_size_proc set_msg_size;
> +    set_msg_serial_proc set_msg_serial;
> +    set_msg_sub_list_proc set_msg_sub_list;
> +
> +    get_msg_type_proc get_msg_type;
> +    get_msg_size_proc get_msg_size;
> +};
> +
>  typedef int (*handle_message_proc)(void *opaque,
>                                     uint16_t type, uint32_t size, uint8_t *msg);
>  typedef int (*handle_parsed_proc)(void *opaque, uint32_t size, uint16_t type, void *message);
> @@ -58,10 +82,12 @@ typedef struct IncomingHandlerInterface {
>  typedef struct IncomingHandler {
>      IncomingHandlerInterface *cb;
>      void *opaque;
> -    SpiceDataHeader header;
> +    uint8_t header_buf[MAX_HEADER_SIZE];
> +    SpiceDataHeaderOpaque header;
>      uint32_t header_pos;
>      uint8_t *msg; // data of the msg following the header. allocated by alloc_msg_buf.
>      uint32_t msg_pos;
> +    uint64_t serial;
>  } IncomingHandler;
>  
>  typedef int (*get_outgoing_msg_size_proc)(void *opaque);
> @@ -202,21 +228,21 @@ struct RedChannelClient {
>  
>      struct {
>          SpiceMarshaller *marshaller;
> -        SpiceDataHeader *header;
> +        SpiceDataHeaderOpaque header;
>          uint32_t size;
>          PipeItem *item;
>          int blocked;
>          uint64_t serial;
> +        uint64_t last_sent_serial;
>  
>          struct {
>              SpiceMarshaller *marshaller;
> -            SpiceDataHeader *header;
> +            uint8_t *header_data;
>              PipeItem *item;
>          } main;
>  
>          struct {
>              SpiceMarshaller *marshaller;
> -            SpiceDataHeader *header;
>          } urgent;
>      } send_data;
>  
> @@ -228,6 +254,7 @@ struct RedChannelClient {
>      uint32_t pipe_size;
>  
>      RedChannelCapabilities remote_caps;
> +    int is_mini_header;
>  };
>  
>  struct RedChannel {
> diff --git a/server/red_worker.c b/server/red_worker.c
> index f454302..7b719de 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -593,6 +593,8 @@ typedef struct CommonChannelClient {
>      struct RedWorker *worker;
>  } CommonChannelClient;
>  
> +#define MAX_DRAWABLE_PIXMAP_CACHE_ITEMS 3
> +

Can you add a comment explaining why 3? Is it because of src, mask
and brush?

>  struct DisplayChannelClient {
>      CommonChannelClient common;
>  
> @@ -616,6 +618,8 @@ struct DisplayChannelClient {
>          RedCompressBuf *used_compress_bufs;
>  
>          FreeList free_list;
> +        uint64_t pixmap_cache_items[MAX_DRAWABLE_PIXMAP_CACHE_ITEMS];
> +        int num_pixmap_cache_items;

Can you explain the usage of this a little more? is it actually related
to this patch? You say nothing about this in the summary.
It looks like you are calling pixmap_cache_hit with this for every
message if there were any sent pixmaps (and there are a maximum of
MAX_DRAWABLE_PIXMAP_CACHE_ITEMS per drawable). 

Also, I don't understand what it actually does - pixmap_cache_hit is
being called once, and then you store the id to call pixmap_cache_hit
*again* when sending the message?

>      } send_data;
>  
>      /* global lz encoding entities */
> @@ -986,8 +990,7 @@ static void red_display_release_stream(RedWorker *worker, StreamAgent *agent);
>  static inline void red_detach_stream(RedWorker *worker, Stream *stream);
>  static void red_stop_stream(RedWorker *worker, Stream *stream);
>  static inline void red_stream_maintenance(RedWorker *worker, Drawable *candidate, Drawable *sect);
> -static inline void display_begin_send_message(RedChannelClient *rcc,
> -                                              SpiceMarshaller *base_marshaller);
> +static inline void display_begin_send_message(RedChannelClient *rcc);
>  static void red_release_pixmap_cache(DisplayChannelClient *dcc);
>  static void red_release_glz(DisplayChannelClient *dcc);
>  static void red_freeze_glz(DisplayChannelClient *dcc);
> @@ -6248,6 +6251,8 @@ static inline void red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
>                                   image->descriptor.width * image->descriptor.height, is_lossy,
>                                   dcc)) {
>                  io_image->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
> +                dcc->send_data.pixmap_cache_items[dcc->send_data.num_pixmap_cache_items++] =
> +                                                                               image->descriptor.id;
>                  stat_inc_counter(display_channel->add_to_cache_counter, 1);
>              }
>          }
> @@ -6290,6 +6295,8 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
>          int lossy_cache_item;
>          if (pixmap_cache_hit(dcc->pixmap_cache, image.descriptor.id,
>                               &lossy_cache_item, dcc)) {
> +            dcc->send_data.pixmap_cache_items[dcc->send_data.num_pixmap_cache_items++] =
> +                                                                               image.descriptor.id;
>              if (can_lossy || !lossy_cache_item) {
>                  if (!display_channel->enable_jpeg || lossy_cache_item) {
>                      image.descriptor.type = SPICE_IMAGE_TYPE_FROM_CACHE;
> @@ -6463,6 +6470,7 @@ static inline void red_display_reset_send_data(DisplayChannelClient *dcc)
>  {
>      red_display_reset_compress_buf(dcc);
>      dcc->send_data.free_list.res->count = 0;
> +    dcc->send_data.num_pixmap_cache_items = 0;
>      memset(dcc->send_data.free_list.sync, 0, sizeof(dcc->send_data.free_list.sync));
>  }
>  
> @@ -7780,27 +7788,23 @@ static void display_channel_push_release(DisplayChannelClient *dcc, uint8_t type
>      free_list->res->resources[free_list->res->count++].id = id;
>  }
>  
> -static inline void display_begin_send_message(RedChannelClient *rcc,
> -                                              SpiceMarshaller *base_marshaller)
> +static inline void display_begin_send_message(RedChannelClient *rcc)

Not sure if you would like this comment, but perhaps breaking this
function into:
 display_begin_send_message_mini_header
and
 display_begin_send_message_legacy

Would be more readable?

>  {
>      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
>      FreeList *free_list = &dcc->send_data.free_list;
>  
>      if (free_list->res->count) {
>          int sub_list_len = 1;
> +        SpiceMarshaller *urgent_marshaller;
>          SpiceMarshaller *wait_m = NULL;
>          SpiceMarshaller *inval_m;
> +        SpiceMarshaller *sub_list_m;
> +        uint32_t sub_arr_offset;
> +        uint32_t wait_offset = 0;
> +        uint32_t inval_offset = 0;
>          int sync_count = 0;
>          int i;
>  
> -        inval_m = spice_marshaller_get_submarshaller(base_marshaller);
> -
> -        /* type + size + submessage */
> -        spice_marshaller_add_uint16(inval_m, SPICE_MSG_DISPLAY_INVAL_LIST);
> -        spice_marshaller_add_uint32(inval_m, sizeof(*free_list->res) +
> -                        free_list->res->count * sizeof(free_list->res->resources[0]));
> -        spice_marshall_msg_display_inval_list(inval_m, free_list->res);
> -
>          for (i = 0; i < MAX_CACHE_CLIENTS; i++) {
>              if (i != dcc->common.id && free_list->sync[i] != 0) {
>                  free_list->wait.header.wait_list[sync_count].channel_type = SPICE_CHANNEL_DISPLAY;
> @@ -7810,8 +7814,32 @@ static inline void display_begin_send_message(RedChannelClient *rcc,
>          }
>          free_list->wait.header.wait_count = sync_count;
>  
> +        if (!rcc->is_mini_header) {
> +            urgent_marshaller = red_channel_client_get_marshaller(rcc);
> +        } else {
> +            urgent_marshaller = red_channel_client_switch_to_urgent_sender(rcc);
> +            if (sync_count) {
> +                red_channel_client_init_send_data(rcc, SPICE_MSG_LIST, NULL);
> +            } else { /* only one message, no need for a list */
> +                red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_INVAL_LIST, NULL);
> +                spice_marshall_msg_display_inval_list(urgent_marshaller, free_list->res);
> +                goto end;
> +            }
> +        }
> +
> +        inval_m = spice_marshaller_get_submarshaller(urgent_marshaller);
> +
> +        /* type + size + submessage */
> +        spice_marshaller_add_uint16(inval_m, SPICE_MSG_DISPLAY_INVAL_LIST);
> +        spice_marshaller_add_uint32(inval_m, sizeof(*free_list->res) +
> +                        free_list->res->count * sizeof(free_list->res->resources[0]));
> +        spice_marshall_msg_display_inval_list(inval_m, free_list->res);
> +
> +
>          if (sync_count) {
> -            wait_m = spice_marshaller_get_submarshaller(base_marshaller);
> +            int j;
> +
> +            wait_m = spice_marshaller_get_submarshaller(urgent_marshaller);
>  
>              /* type + size + submessage */
>              spice_marshaller_add_uint16(wait_m, SPICE_MSG_WAIT_FOR_CHANNELS);
> @@ -7819,16 +7847,40 @@ static inline void display_begin_send_message(RedChannelClient *rcc,
>                                                  sync_count * sizeof(free_list->wait.buf[0]));
>              spice_marshall_msg_wait_for_channels(wait_m, &free_list->wait.header);
>              sub_list_len++;
> +
> +            if (rcc->is_mini_header) {
> +                for (j = 0; j < dcc->send_data.num_pixmap_cache_items; j++) {
> +                    int dummy;
> +                    // refresshing the serial value
> +                    pixmap_cache_hit(dcc->pixmap_cache, dcc->send_data.pixmap_cache_items[j],
> +                                    &dummy, dcc);
> +                }
> +            }
> +        }
> +
> +        if (!rcc->is_mini_header) {
> +            sub_list_m = spice_marshaller_get_submarshaller(urgent_marshaller);
> +            sub_arr_offset = 0;
> +        } else {
> +            sub_list_m = urgent_marshaller;
> +            sub_arr_offset = sub_list_len * sizeof(uint32_t);
>          }
>  
> -        SpiceMarshaller *sub_list_m = spice_marshaller_get_submarshaller(base_marshaller);
>          spice_marshaller_add_uint16(sub_list_m, sub_list_len);
> +        inval_offset = spice_marshaller_get_offset(inval_m); // calc the offset before
> +                                                             // adding the sub list
> +                                                             // offsets array to the marshaller
>          if (wait_m) {
> -            spice_marshaller_add_uint32(sub_list_m, spice_marshaller_get_offset(wait_m));
> +            wait_offset = spice_marshaller_get_offset(wait_m);
> +            spice_marshaller_add_uint32(sub_list_m, wait_offset + sub_arr_offset);
> +        }
> +        spice_marshaller_add_uint32(sub_list_m, inval_offset + sub_arr_offset);
> +
> +        if (!rcc->is_mini_header) {
> +            red_channel_client_set_header_sub_list(rcc, spice_marshaller_get_offset(sub_list_m));
>          }
> -        spice_marshaller_add_uint32(sub_list_m, spice_marshaller_get_offset(inval_m));
> -        red_channel_client_set_header_sub_list(rcc, spice_marshaller_get_offset(sub_list_m));
>      }
> +end:
>      red_channel_client_begin_send_message(rcc);
>  }
>  
> @@ -8495,7 +8547,7 @@ static void display_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item
>  
>      // a message is pending
>      if (red_channel_client_send_message_pending(rcc)) {
> -        display_begin_send_message(rcc, m);
> +        display_begin_send_message(rcc);
>      }
>  }
>  
> diff --git a/server/snd_worker.c b/server/snd_worker.c
> index 048da34..5d58077 100644
> --- a/server/snd_worker.c
> +++ b/server/snd_worker.c
> @@ -101,7 +101,6 @@ struct SndChannel {
>  
>      struct {
>          uint64_t serial;
> -        SpiceDataHeader *header;
>          SpiceMarshaller *marshaller;
>          uint32_t size;
>          uint32_t pos;
> @@ -109,7 +108,7 @@ struct SndChannel {
>  
>      struct {
>          uint8_t buf[RECIVE_BUF_SIZE];
> -        SpiceDataHeader *message;
> +        uint8_t *message_start;
>          uint8_t *now;
>          uint8_t *end;
>      } recive_data;
> @@ -417,10 +416,14 @@ static int snd_record_handle_message(SndChannel *channel, size_t size, uint32_t
>  static void snd_receive(void* data)
>  {
>      SndChannel *channel = (SndChannel*)data;
> +    SpiceDataHeaderOpaque *header;
> +
>      if (!channel) {
>          return;
>      }
>  
> +    header = &channel->channel_client->incoming.header;
> +
>      for (;;) {
>          ssize_t n;
>          n = channel->recive_data.end - channel->recive_data.now;
> @@ -448,40 +451,44 @@ static void snd_receive(void* data)
>          } else {
>              channel->recive_data.now += n;
>              for (;;) {
> -                SpiceDataHeader *header = channel->recive_data.message;
> -                uint8_t *data = (uint8_t *)(header+1);
> +                uint8_t *msg_start = channel->recive_data.message_start;
> +                uint8_t *data = msg_start + header->header_size;
>                  size_t parsed_size;
>                  uint8_t *parsed;
>                  message_destructor_t parsed_free;
>  
> -                n = channel->recive_data.now - (uint8_t *)header;
> -                if (n < sizeof(SpiceDataHeader) || n < sizeof(SpiceDataHeader) + header->size) {
> +                header->data = msg_start;
> +                n = channel->recive_data.now - msg_start;
> +
> +                if (n < header->header_size ||
> +                    n < header->header_size + header->get_msg_size(header)) {
>                      break;
>                  }
> -                parsed = channel->parser((void *)data, data + header->size, header->type,
> +                parsed = channel->parser((void *)data, data + header->get_msg_size(header),
> +                                         header->get_msg_type(header),
>                                           SPICE_VERSION_MINOR, &parsed_size, &parsed_free);
>                  if (parsed == NULL) {
> -                    red_printf("failed to parse message type %d", header->type);
> +                    red_printf("failed to parse message type %d", header->get_msg_type(header));
>                      snd_disconnect_channel(channel);
>                      return;
>                  }
> -                if (!channel->handle_message(channel, parsed_size, header->type, parsed)) {
> +                if (!channel->handle_message(channel, parsed_size,
> +                                             header->get_msg_type(header), parsed)) {
>                      free(parsed);
>                      snd_disconnect_channel(channel);
>                      return;
>                  }
>                  parsed_free(parsed);
> -                channel->recive_data.message = (SpiceDataHeader *)((uint8_t *)header +
> -                                                                 sizeof(SpiceDataHeader) +
> -                                                                 header->size);
> +                channel->recive_data.message_start = msg_start + header->header_size +
> +                                                     header->get_msg_size(header);
>              }
> -            if (channel->recive_data.now == (uint8_t *)channel->recive_data.message) {
> +            if (channel->recive_data.now == channel->recive_data.message_start) {
>                  channel->recive_data.now = channel->recive_data.buf;
> -                channel->recive_data.message = (SpiceDataHeader *)channel->recive_data.buf;
> +                channel->recive_data.message_start = channel->recive_data.buf;
>              } else if (channel->recive_data.now == channel->recive_data.end) {
> -                memcpy(channel->recive_data.buf, channel->recive_data.message, n);
> +                memcpy(channel->recive_data.buf, channel->recive_data.message_start, n);
>                  channel->recive_data.now = channel->recive_data.buf + n;
> -                channel->recive_data.message = (SpiceDataHeader *)channel->recive_data.buf;
> +                channel->recive_data.message_start = channel->recive_data.buf;
>              }
>          }
>      }
> @@ -501,28 +508,37 @@ static void snd_event(int fd, int event, void *data)
>  
>  static inline int snd_reset_send_data(SndChannel *channel, uint16_t verb)
>  {
> +    SpiceDataHeaderOpaque *header;
> +
>      if (!channel) {
>          return FALSE;
>      }
>  
> +    header = &channel->channel_client->send_data.header;
>      spice_marshaller_reset(channel->send_data.marshaller);
> -    channel->send_data.header = (SpiceDataHeader *)
> -        spice_marshaller_reserve_space(channel->send_data.marshaller, sizeof(SpiceDataHeader));
> -    spice_marshaller_set_base(channel->send_data.marshaller, sizeof(SpiceDataHeader));
> +    header->data = spice_marshaller_reserve_space(channel->send_data.marshaller,
> +                                                  header->header_size);
> +    spice_marshaller_set_base(channel->send_data.marshaller,
> +                              header->header_size);
>      channel->send_data.pos = 0;
> -    channel->send_data.header->sub_list = 0;
> -    channel->send_data.header->size = 0;
> -    channel->send_data.header->type = verb;
> -    channel->send_data.header->serial = ++channel->send_data.serial;
> +    header->set_msg_size(header, 0);
> +    header->set_msg_type(header, verb);
> +    channel->send_data.serial++;
> +    if (!channel->channel_client->is_mini_header) {
> +        header->set_msg_serial(header, channel->send_data.serial);
> +        header->set_msg_sub_list(header, 0);
> +    }
> +
>      return TRUE;
>  }
>  
>  static int snd_begin_send_message(SndChannel *channel)
>  {
> +    SpiceDataHeaderOpaque *header = &channel->channel_client->send_data.header;
> +
>      spice_marshaller_flush(channel->send_data.marshaller);
>      channel->send_data.size = spice_marshaller_get_total_size(channel->send_data.marshaller);
> -    channel->send_data.header->size = channel->send_data.size - sizeof(SpiceDataHeader);
> -    channel->send_data.header = NULL; /* avoid writing to this until we have a new message */
> +    header->set_msg_size(header, channel->send_data.size - header->header_size);
>      return snd_send_data(channel);
>  }
>  
> @@ -709,22 +725,25 @@ static int snd_record_send_migrate(RecordChannel *record_channel)
>  {
>      SndChannel *channel = (SndChannel *)record_channel;
>      SpiceMsgMigrate migrate;
> -    SpiceDataHeader *header;
> +    SpiceDataHeaderOpaque *header;
>      RecordMigrateData *data;
>  
>      if (!snd_reset_send_data(channel, SPICE_MSG_MIGRATE)) {
>          return FALSE;
>      }
>  
> +    header = &channel->channel_client->send_data.header;
>      migrate.flags = SPICE_MIGRATE_NEED_DATA_TRANSFER;
>      spice_marshall_msg_migrate(channel->send_data.marshaller, &migrate);
>  
> -    header = (SpiceDataHeader *)spice_marshaller_reserve_space(channel->send_data.marshaller,
> -                                                               sizeof(SpiceDataHeader));
> -    header->type = SPICE_MSG_MIGRATE_DATA;
> -    header->size = sizeof(RecordMigrateData);
> -    header->serial = ++channel->send_data.serial;
> -    header->sub_list = 0;
> +    header->data = spice_marshaller_reserve_space(channel->send_data.marshaller, header->header_size);
> +    header->set_msg_size(header, sizeof(RecordMigrateData));
> +    header->set_msg_type(header, SPICE_MSG_MIGRATE_DATA);
> +    ++channel->send_data.serial;
> +    if (!channel->channel_client->is_mini_header) {
> +        header->set_msg_serial(header, channel->send_data.serial);
> +        header->set_msg_sub_list(header, 0);
> +    }
>  
>      data = (RecordMigrateData *)spice_marshaller_reserve_space(channel->send_data.marshaller,
>                                                                 sizeof(RecordMigrateData));
> @@ -735,7 +754,8 @@ static int snd_record_send_migrate(RecordChannel *record_channel)
>      data->mode_time = record_channel->mode_time;
>  
>      channel->send_data.size = spice_marshaller_get_total_size(channel->send_data.marshaller);
> -    channel->send_data.header->size = channel->send_data.size - sizeof(SpiceDataHeader) - sizeof(SpiceDataHeader) - sizeof(*data);
> +    header->set_msg_size(header, channel->send_data.size - header->header_size -
> +                         header->header_size - sizeof(*data));
>  
>      return snd_send_data(channel);
>  }
> @@ -876,6 +896,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
>                                   snd_channel_handle_message_proc handle_message,
>                                   snd_channel_on_message_done_proc on_message_done,
>                                   snd_channel_cleanup_channel_proc cleanup,
> +                                 uint32_t *common_caps, int num_common_caps,
>                                   uint32_t *caps, int num_caps)
>  {
>      SndChannel *channel;
> @@ -917,7 +938,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
>      channel->parser = spice_get_client_channel_parser(channel_id, NULL);
>      channel->stream = stream;
>      channel->worker = worker;
> -    channel->recive_data.message = (SpiceDataHeader *)channel->recive_data.buf;
> +    channel->recive_data.message_start = channel->recive_data.buf;
>      channel->recive_data.now = channel->recive_data.buf;
>      channel->recive_data.end = channel->recive_data.buf + sizeof(channel->recive_data.buf);
>      channel->send_data.marshaller = spice_marshaller_new();
> @@ -938,7 +959,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
>      channel->channel_client = red_channel_client_create_dummy(sizeof(RedChannelClient),
>                                                                worker->base_channel,
>                                                                client,
> -                                                              0, NULL,
> +                                                              num_common_caps, common_caps,
>                                                                num_caps, caps);
>      return channel;
>  
> @@ -1159,6 +1180,7 @@ static void snd_set_playback_peer(RedChannel *channel, RedClient *client, RedsSt
>                                                                snd_playback_handle_message,
>                                                                snd_playback_on_message_done,
>                                                                snd_playback_cleanup,
> +                                                              common_caps, num_common_caps,
>                                                                caps, num_caps))) {
>          goto error_2;
>      }
> @@ -1367,6 +1389,7 @@ static void snd_set_record_peer(RedChannel *channel, RedClient *client, RedsStre
>                                                            snd_record_handle_message,
>                                                            snd_record_on_message_done,
>                                                            snd_record_cleanup,
> +                                                          common_caps, num_common_caps,
>                                                            caps, num_caps))) {
>          goto error_2;
>      }
> -- 
> 1.7.6.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list