>
> On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > ---
> > server/dcc.c | 124 +++++++++++++++++++++++++++++++++++++
> > server/dcc.h | 3 +
> > server/display-channel.h | 7 +++
> > server/red_worker.c | 158
> > ++++-------------------------------------------
> > 4 files changed, 147 insertions(+), 145 deletions(-)
> >
> > diff --git a/server/dcc.c b/server/dcc.c
> > index fbaf386..eccaa77 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -1390,3 +1390,127 @@ int dcc_handle_migrate_data(DisplayChannelClient
> > *dcc, uint32_t size, void *mess
> > red_channel_client_ack_zero_messages_window(RED_CHANNEL_CLIENT(dcc));
> > return TRUE;
> > }
> > +
> > +static void image_item_unref(ImageItem *item)
> > +{
> > + if (--item->refs != 0)
> > + return;
> > +
> > + free(item);
> > +}
> > +
> > +static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item)
> > +{
> > + if (--item->refs != 0)
> > + return;
> > +
> > + display_channel_drawable_unref(display, item->drawable);
> > + free(item->rects);
> > + free(item);
> > +}
> > +
> > +static void release_item_after_push(DisplayChannelClient *dcc, PipeItem
> > *item)
> > +{
> > + DisplayChannel *display = DCC_TO_DC(dcc);
> > +
> > + switch (item->type) {
> > + case PIPE_ITEM_TYPE_DRAW:
> > + drawable_pipe_item_unref(SPICE_CONTAINEROF(item, DrawablePipeItem,
> > dpi_pipe_item));
> > + break;
> > + case PIPE_ITEM_TYPE_STREAM_CLIP:
> > + stream_clip_item_unref(dcc, (StreamClipItem *)item);
> > + break;
> > + case PIPE_ITEM_TYPE_UPGRADE:
> > + upgrade_item_unref(display, (UpgradeItem *)item);
> > + break;
> > + case PIPE_ITEM_TYPE_IMAGE:
> > + image_item_unref((ImageItem *)item);
> > + break;
> > + case PIPE_ITEM_TYPE_VERB:
> > + free(item);
> > + break;
> > + case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> > + MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> > +
> > MonitorsConfigItem,
> > pipe_item);
> > + monitors_config_unref(monconf_item->monitors_config);
> > + free(item);
> > + break;
> > + }
> > + default:
> > + spice_critical("invalid item type");
> > + }
> > +}
> > +
> > +// TODO: share code between before/after_push since most of the items need
> > the same
> > +// release
> > +static void release_item_before_push(DisplayChannelClient *dcc, PipeItem
> > *item)
> > +{
> > + DisplayChannel *display = DCC_TO_DC(dcc);
> > +
> > + spice_debug("item.type: %d", item->type);
> > + switch (item->type) {
> > + case PIPE_ITEM_TYPE_DRAW: {
> > + DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem,
> > dpi_pipe_item);
> > + ring_remove(&dpi->base);
> > + drawable_pipe_item_unref(dpi);
> > + break;
> > + }
> > + case PIPE_ITEM_TYPE_STREAM_CREATE: {
> > + StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent,
> > create_item);
> > + stream_agent_unref(display, agent);
> > + break;
> > + }
> > + case PIPE_ITEM_TYPE_STREAM_CLIP:
> > + stream_clip_item_unref(dcc, (StreamClipItem *)item);
> > + break;
> > + case PIPE_ITEM_TYPE_STREAM_DESTROY: {
> > + StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent,
> > destroy_item);
> > + stream_agent_unref(display, agent);
> > + break;
> > + }
> > + case PIPE_ITEM_TYPE_UPGRADE:
> > + upgrade_item_unref(display, (UpgradeItem *)item);
> > + break;
> > + case PIPE_ITEM_TYPE_IMAGE:
> > + image_item_unref((ImageItem *)item);
> > + break;
> > + case PIPE_ITEM_TYPE_CREATE_SURFACE: {
> > + SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item,
> > SurfaceCreateItem,
> > + pipe_item);
> > + free(surface_create);
> > + break;
> > + }
> > + case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
> > + SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item,
> > SurfaceDestroyItem,
> > +
> > pipe_item);
> > + free(surface_destroy);
> > + break;
> > + }
> > + case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> > + MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> > +
> > MonitorsConfigItem,
> > pipe_item);
> > + monitors_config_unref(monconf_item->monitors_config);
> > + free(item);
> > + break;
> > + }
> > + case PIPE_ITEM_TYPE_INVAL_ONE:
> > + case PIPE_ITEM_TYPE_VERB:
> > + case PIPE_ITEM_TYPE_MIGRATE_DATA:
> > + case PIPE_ITEM_TYPE_PIXMAP_SYNC:
> > + case PIPE_ITEM_TYPE_PIXMAP_RESET:
> > + case PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE:
> > + case PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT:
> > + free(item);
> > + break;
> > + default:
> > + spice_critical("invalid item type");
> > + }
> > +}
> > +
> > +void dcc_release_item(DisplayChannelClient *dcc, PipeItem *item, int
> > item_pushed)
> > +{
> > + if (item_pushed)
> > + release_item_after_push(dcc, item);
> > + else
> > + release_item_before_push(dcc, item);
> > +}
> > diff --git a/server/dcc.h b/server/dcc.h
> > index 9da85ab..13d8c82 100644
> > --- a/server/dcc.h
> > +++ b/server/dcc.h
> > @@ -194,6 +194,9 @@ void dcc_add_drawable
> > (DisplayCha
> > void dcc_add_drawable_after
> > (DisplayChannelClient *dcc,
> > Drawable
> > *drawable,
> > PipeItem
> > *pos);
> > +void dcc_release_item
> > (DisplayChannelClient *dcc,
> > +
> > PipeItem
> > *item,
> > + int
> > item_pushed);
> >
> > typedef struct compress_send_data_t {
> > void* comp_buf;
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index 4a50912..b49cec9 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -242,6 +242,13 @@ typedef struct SurfaceDestroyItem {
> > PipeItem pipe_item;
> > } SurfaceDestroyItem;
> >
> > +typedef struct UpgradeItem {
> > + PipeItem base;
> > + int refs;
> > + Drawable *drawable;
> > + SpiceClipRects *rects;
> > +} UpgradeItem;
> > +
> >
> > void display_channel_set_stream_video
> > (DisplayChannel *display,
> > int
> > stream_video);
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 10b6c00..3873da4 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -85,13 +85,6 @@ struct SpiceWatch {
> >
> > #define MAX_PIPE_SIZE 50
> >
> > -typedef struct UpgradeItem {
> > - PipeItem base;
> > - int refs;
> > - Drawable *drawable;
> > - SpiceClipRects *rects;
> > -} UpgradeItem;
> > -
> > struct RedWorker {
> > pthread_t thread;
> > clockid_t clockid;
> > @@ -151,10 +144,6 @@ static void red_update_area_till(DisplayChannel
> > *display, const SpiceRect *area,
> > Drawable *last);
> > static inline void display_begin_send_message(RedChannelClient *rcc);
> > static int
> > red_display_free_some_independent_glz_drawables(DisplayChannelClient
> > *dcc);
> > -static void
> > display_channel_client_release_item_before_push(DisplayChannelClient *dcc,
> > - PipeItem
> > *item);
> > -static void
> > display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
> > - PipeItem
> > *item);
> > static void red_create_surface(DisplayChannel *display, uint32_t
> > surface_id, uint32_t width,
> > uint32_t height, int32_t stride, uint32_t
> > format,
> > void *line_0, int data_is_valid, int
> > send_client);
> > @@ -213,23 +202,6 @@ void red_pipes_remove_drawable(Drawable *drawable)
> > }
> > }
> >
> > -static void release_image_item(ImageItem *item)
> > -{
> > - if (!--item->refs) {
> > - free(item);
> > - }
> > -}
> > -
> > -static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item)
> > -{
> > - if (--item->refs != 0)
> > - return;
> > -
> > - display_channel_drawable_unref(display, item->drawable);
> > - free(item->rects);
> > - free(item);
> > -}
> > -
> > static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t
> > type, uint32_t size)
> > {
> > CommonChannel *common = SPICE_CONTAINEROF(rcc->channel, CommonChannel,
> > base);
> > @@ -3960,7 +3932,7 @@ static void
> > red_marshall_stream_activate_report(RedChannelClient *rcc,
> > spice_marshall_msg_display_stream_activate_report(base_marshaller,
> > &msg);
> > }
> >
> > -static void display_channel_send_item(RedChannelClient *rcc, PipeItem
> > *pipe_item)
> > +static void send_item(RedChannelClient *rcc, PipeItem *pipe_item)
>
> This rename is not relevant for this patch ...
>
> > {
> > SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> > DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> > @@ -4041,7 +4013,7 @@ static void
> > display_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item
> > spice_error("invalid pipe item type");
> > }
> >
> > - display_channel_client_release_item_before_push(dcc, pipe_item);
> > + dcc_release_item(dcc, pipe_item, FALSE);
> >
> > // a message is pending
> > if (red_channel_client_send_message_pending(rcc)) {
> > @@ -4059,7 +4031,7 @@ static inline void red_push(RedWorker *worker)
> > }
> > }
> >
> > -static void display_channel_client_on_disconnect(RedChannelClient *rcc)
> > +static void on_disconnect(RedChannelClient *rcc)
>
> Same here ...
>
> > {
> > DisplayChannel *display;
> > DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> > @@ -4335,7 +4307,7 @@ static inline void flush_all_qxl_commands(RedWorker
> > *worker)
> > flush_cursor_commands(worker);
> > }
> >
> > -static int display_channel_handle_migrate_mark(RedChannelClient *rcc)
> > +static int handle_migrate_flush_mark(RedChannelClient *rcc)
>
> Same here ...
>
> > {
> > DisplayChannel *display_channel = SPICE_CONTAINEROF(rcc->channel,
> > DisplayChannel, common.base);
> > RedChannel *channel = RED_CHANNEL(display_channel);
> > @@ -4540,7 +4512,7 @@ RedChannel *red_worker_new_channel(RedWorker *worker,
> > int size,
> > return channel;
> > }
> >
> > -static void display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem
> > *item)
> > +static void hold_item(RedChannelClient *rcc, PipeItem *item)
>
> Same here ...
>
> > {
> > spice_assert(item);
> > switch (item->type) {
> > @@ -4561,116 +4533,12 @@ static void
> > display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item
> > }
> > }
> >
> > -static void
> > display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
> > - PipeItem *item)
> > -{
> > - DisplayChannel *display = DCC_TO_DC(dcc);
> > -
> > - switch (item->type) {
> > - case PIPE_ITEM_TYPE_DRAW:
> > - drawable_pipe_item_unref(SPICE_CONTAINEROF(item, DrawablePipeItem,
> > dpi_pipe_item));
> > - break;
> > - case PIPE_ITEM_TYPE_STREAM_CLIP:
> > - stream_clip_item_unref(dcc, (StreamClipItem *)item);
> > - break;
> > - case PIPE_ITEM_TYPE_UPGRADE:
> > - upgrade_item_unref(display, (UpgradeItem *)item);
> > - break;
> > - case PIPE_ITEM_TYPE_IMAGE:
> > - release_image_item((ImageItem *)item);
> > - break;
> > - case PIPE_ITEM_TYPE_VERB:
> > - free(item);
> > - break;
> > - case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> > - MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> > -
> > MonitorsConfigItem,
> > pipe_item);
> > - monitors_config_unref(monconf_item->monitors_config);
> > - free(item);
> > - break;
> > - }
> > - default:
> > - spice_critical("invalid item type");
> > - }
> > -}
> > -
> > -// TODO: share code between before/after_push since most of the items need
> > the same
> > -// release
> > -static void
> > display_channel_client_release_item_before_push(DisplayChannelClient *dcc,
> > - PipeItem
> > *item)
> > -{
> > - DisplayChannel *display = DCC_TO_DC(dcc);
> > -
> > - switch (item->type) {
> > - case PIPE_ITEM_TYPE_DRAW: {
> > - DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem,
> > dpi_pipe_item);
> > - ring_remove(&dpi->base);
> > - drawable_pipe_item_unref(dpi);
> > - break;
> > - }
> > - case PIPE_ITEM_TYPE_STREAM_CREATE: {
> > - StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent,
> > create_item);
> > - stream_agent_unref(display, agent);
> > - break;
> > - }
> > - case PIPE_ITEM_TYPE_STREAM_CLIP:
> > - stream_clip_item_unref(dcc, (StreamClipItem *)item);
> > - break;
> > - case PIPE_ITEM_TYPE_STREAM_DESTROY: {
> > - StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent,
> > destroy_item);
> > - stream_agent_unref(display, agent);
> > - break;
> > - }
> > - case PIPE_ITEM_TYPE_UPGRADE:
> > - upgrade_item_unref(display, (UpgradeItem *)item);
> > - break;
> > - case PIPE_ITEM_TYPE_IMAGE:
> > - release_image_item((ImageItem *)item);
> > - break;
> > - case PIPE_ITEM_TYPE_CREATE_SURFACE: {
> > - SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item,
> > SurfaceCreateItem,
> > - pipe_item);
> > - free(surface_create);
> > - break;
> > - }
> > - case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
> > - SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item,
> > SurfaceDestroyItem,
> > -
> > pipe_item);
> > - free(surface_destroy);
> > - break;
> > - }
> > - case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> > - MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> > -
> > MonitorsConfigItem,
> > pipe_item);
> > - monitors_config_unref(monconf_item->monitors_config);
> > - free(item);
> > - break;
> > - }
> > - case PIPE_ITEM_TYPE_INVAL_ONE:
> > - case PIPE_ITEM_TYPE_VERB:
> > - case PIPE_ITEM_TYPE_MIGRATE_DATA:
> > - case PIPE_ITEM_TYPE_PIXMAP_SYNC:
> > - case PIPE_ITEM_TYPE_PIXMAP_RESET:
> > - case PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE:
> > - case PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT:
> > - free(item);
> > - break;
> > - default:
> > - spice_critical("invalid item type");
> > - }
> > -}
> > -
> > -static void display_channel_release_item(RedChannelClient *rcc, PipeItem
> > *item, int item_pushed)
> > +static void release_item(RedChannelClient *rcc, PipeItem *item, int
> > item_pushed)
>
> Same here ...
>
> > {
> > DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> >
> > - spice_assert(item);
> > - if (item_pushed) {
> > - display_channel_client_release_item_after_push(dcc, item);
> > - } else {
> > - spice_debug("not pushed (%d)", item->type);
> > - display_channel_client_release_item_before_push(dcc, item);
> > - }
> > + spice_return_if_fail(item != NULL);
> > + dcc_release_item(dcc, item, item_pushed);
> > }
> >
> > static void image_surface_init(DisplayChannel *display)
> > @@ -4687,11 +4555,11 @@ static void display_channel_create(RedWorker
> > *worker, int migrate, int stream_vi
> > {
> > DisplayChannel *display_channel;
> > ChannelCbs cbs = {
> > - .on_disconnect = display_channel_client_on_disconnect,
> > - .send_item = display_channel_send_item,
> > - .hold_item = display_channel_hold_pipe_item,
> > - .release_item = display_channel_release_item,
> > - .handle_migrate_flush_mark = display_channel_handle_migrate_mark,
> > + .on_disconnect = on_disconnect,
> > + .send_item = send_item,
> > + .hold_item = hold_item,
> > + .release_item = release_item,
> > + .handle_migrate_flush_mark = handle_migrate_flush_mark,
>
> Same here ...
>
> > .handle_migrate_data = handle_migrate_data,
> > .handle_migrate_data_get_serial = handle_migrate_data_get_serial
> > };
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
> ACK with these changes ...
>
> --
> Fabiano Fidêncio
>
Could you post an updated patch?
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel