[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