[Spice-devel] [PATCH 12/18] worker: move display_channel_new
Frediano Ziglio
fziglio at redhat.com
Wed Nov 25 02:23:24 PST 2015
>
> Reduced diff:
>
>
> --- before.c 2015-11-25 10:09:19.413943460 +0000
> +++ after.c 2015-11-25 09:56:05.709537277 +0000
> @@ -1700,20 +1700,21 @@
> RedsStream
> *stream,
> int
> mig_target,
> uint32_t
> *common_caps,
> int
> num_common_caps,
> uint32_t
> *caps,
> int
> num_caps,
> SpiceImageCompression
> image_compression,
> spice_wan_compression_t
> jpeg_state,
> spice_wan_compression_t
> zlib_glz_state);
> void dcc_start
> (DisplayChannelClient *dcc);
> +void dcc_stop
> (DisplayChannelClient *dcc);
> int dcc_handle_message
> (RedChannelClient *rcc,
> uint32_t
> size,
> uint16_t
> type,
> void
> *msg);
> int dcc_handle_migrate_data
> (DisplayChannelClient *dcc,
> uint32_t
> size,
> void
> *message);
> void dcc_push_monitors_config
> (DisplayChannelClient *dcc);
> void dcc_destroy_surface
> (DisplayChannelClient *dcc,
> uint32_t
> surface_id);
> void dcc_stream_agent_clip
> (DisplayChannelClient* dcc,
> StreamAgent
> *agent);
> @@ -1737,20 +1738,22 @@
> void dcc_prepend_drawable
> (DisplayChannelClient *dcc,
> Drawable
> *drawable);
> void dcc_append_drawable
> (DisplayChannelClient *dcc,
> Drawable
> *drawable);
> void dcc_add_drawable_after
> (DisplayChannelClient *dcc,
> Drawable
> *drawable,
> PipeItem
> *pos);
> void dcc_release_item
> (DisplayChannelClient *dcc,
> PipeItem
> *item,
> int
> item_pushed);
> +void dcc_send_item
> (DisplayChannelClient *dcc,
> +
> PipeItem
> *item);
>
> typedef struct compress_send_data_t {
> void* comp_buf;
> uint32_t comp_buf_size;
> SpicePalette *lzplt_palette;
> int is_lossy;
> } compress_send_data_t;
>
> int dcc_compress_image
> (DisplayChannelClient *dcc,
> SpiceImage
> *dest,
> SpiceBitmap
> *src,
> Drawable
> *drawable,
> @@ -3175,58 +3178,62 @@
> return;
>
> draw_until(display, surface, last);
> surface_update_dest(surface, area);
> }
>
> static void on_disconnect(RedChannelClient *rcc)
> {
> DisplayChannel *display;
> DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> - CommonChannel *common;
> RedWorker *worker;
>
> - if (!rcc) {
> - return;
> - }
> spice_info(NULL);
> - common = SPICE_CONTAINEROF(rcc->channel, CommonChannel, base);
> - worker = common->worker;
> - display = (DisplayChannel *)rcc->channel;
> - spice_assert(display == worker->display_channel);
> - display_channel_compress_stats_print(display);
> + spice_return_if_fail(rcc != NULL);
The spice_info(NULL) is moved before the check for rcc NULL.
Not change much as rcc should be not NULL here.
> +
> + display = DCC_TO_DC(dcc);
> + worker = COMMON_CHANNEL(display)->worker;
> + spice_return_if_fail(rcc->channel ==
> red_worker_get_display_channel(worker));
the worker is used only for the check.
I suspect this was changed when we moved some fields from worker to display.
The two lines above (and worker variable declaration) can be removed.
> +
> dcc_stop(dcc); // TODO: start/stop -> connect/disconnect?
> + display_channel_compress_stats_print(display);
>
Here compression statistics are printed after stopping the dcc.
However is not a problem, they just access data from display.
> // this was the last channel client
> spice_debug("#draw=%d, #red_draw=%d, #glz_draw=%d",
> display->drawable_count, display->red_drawable_count,
> display->glz_drawable_count);
> }
>
> +static void send_item(RedChannelClient *rcc, PipeItem *item)
> +{
> + dcc_send_item(RCC_TO_DCC(rcc), item);
> +}
> +
This is required as send_item was renamed as dcc_send_item and first parameter
changed to DisplayChannelClient.
Perhaps adding an inline could help the compiler but I don't think that
much.
> static void hold_item(RedChannelClient *rcc, PipeItem *item)
> {
> - spice_assert(item);
> + spice_return_if_fail(item);
> +
Still prefer the spice_assert but in this case does not harm
> switch (item->type) {
> case PIPE_ITEM_TYPE_DRAW:
> drawable_pipe_item_ref(SPICE_CONTAINEROF(item, DrawablePipeItem,
> dpi_pipe_item));
> break;
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> ((StreamClipItem *)item)->refs++;
> break;
> case PIPE_ITEM_TYPE_UPGRADE:
> ((UpgradeItem *)item)->refs++;
> break;
> case PIPE_ITEM_TYPE_IMAGE:
> ((ImageItem *)item)->refs++;
> break;
> default:
> - spice_critical("invalid item type");
> + spice_warn_if_reached();
> }
> }
>
> static void release_item(RedChannelClient *rcc, PipeItem *item, int
> item_pushed)
> {
> DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
>
> spice_return_if_fail(item != NULL);
> dcc_release_item(dcc, item, item_pushed);
> }
> @@ -3256,52 +3263,50 @@
>
> static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces,
> uint32_t surface_id)
> {
> DisplayChannel *display = SPICE_CONTAINEROF(surfaces, DisplayChannel,
> image_surfaces);
>
> spice_return_val_if_fail(validate_surface(display, surface_id), NULL);
>
> return display->surfaces[surface_id].context.canvas;
> }
>
> -static void display_channel_create(RedWorker *worker, int migrate, int
> stream_video,
> +DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int
> stream_video,
> uint32_t n_surfaces)
> {
> DisplayChannel *display;
> ChannelCbs cbs = {
> .on_disconnect = on_disconnect,
> .send_item = send_item,
> .hold_item = hold_item,
> .release_item = release_item,
> .handle_migrate_flush_mark = handle_migrate_flush_mark,
> .handle_migrate_data = handle_migrate_data,
> .handle_migrate_data_get_serial = handle_migrate_data_get_serial
> };
> static SpiceImageSurfacesOps image_surfaces_ops = {
> image_surfaces_get,
> };
>
> - spice_return_if_fail(num_renderers > 0);
> + spice_return_val_if_fail(num_renderers > 0, NULL);
>
> spice_info("create display channel");
> - if (!(display = (DisplayChannel *)red_worker_new_channel(
> + display = (DisplayChannel *)red_worker_new_channel(
> worker, sizeof(*display), "display_channel",
> SPICE_CHANNEL_DISPLAY,
> SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER,
> - &cbs, dcc_handle_message))) {
> - spice_warning("failed to create display channel");
> - return;
> - }
> - worker->display_channel = display;
> - stat_init(&display->add_stat, "add", worker->clockid);
> - stat_init(&display->exclude_stat, "exclude", worker->clockid);
> - stat_init(&display->__exclude_stat, "__exclude", worker->clockid);
> + &cbs, dcc_handle_message);
> + spice_return_val_if_fail(display, NULL);
> +
> + stat_init(&display->add_stat, "add", red_worker_get_clockid(worker));
> + stat_init(&display->exclude_stat, "exclude",
> red_worker_get_clockid(worker));
> + stat_init(&display->__exclude_stat, "__exclude",
> red_worker_get_clockid(worker));
> #ifdef RED_STATISTICS
> RedChannel *channel = RED_CHANNEL(display);
> display->cache_hits_counter = stat_add_counter(channel->stat,
> "cache_hits",
> TRUE);
> display->add_to_cache_counter = stat_add_counter(channel->stat,
> "add_to_cache",
> TRUE);
> display->non_cache_counter = stat_add_counter(channel->stat,
> "non_cache",
> TRUE);
> #endif
> stat_compress_init(&display->lz_stat, "lz");
> @@ -3316,22 +3321,23 @@
> display->num_renderers = num_renderers;
> memcpy(display->renderers, renderers, sizeof(display->renderers));
> display->renderer = RED_RENDERER_INVALID;
>
> ring_init(&display->current_list);
> display->image_surfaces.ops = &image_surfaces_ops;
> drawables_init(display);
> image_cache_init(&display->image_cache);
> display->stream_video = stream_video;
> display_channel_init_streams(display);
> -}
>
> + return display;
> +}
> /* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> /*
> Copyright (C) 2009-2015 Red Hat, Inc.
>
> This library is free software; you can redistribute it and/or
> modify it under the terms of the GNU Lesser General Public
> License as published by the Free Software Foundation; either
> version 2.1 of the License, or (at your option) any later version.
>
> This library is distributed in the hope that it will be useful,
> @@ -3571,20 +3577,24 @@
> } SurfaceDestroyItem;
>
> typedef struct UpgradeItem {
> PipeItem base;
> int refs;
> Drawable *drawable;
> SpiceClipRects *rects;
> } UpgradeItem;
>
>
> +DisplayChannel* display_channel_new
> (RedWorker *worker,
> + int
> migrate,
> + int
> stream_video,
> +
> uint32_t
> n_surfaces);
> void display_channel_draw
> (DisplayChannel *display,
> const
> SpiceRect
> *area,
> int
> surface_id);
> void display_channel_draw_until
> (DisplayChannel *display,
> const
> SpiceRect
> *area,
> int
> surface_id,
> Drawable
> *last);
> void display_channel_free_some
> (DisplayChannel *display);
> void display_channel_set_stream_video
> (DisplayChannel *display,
> int
> stream_video);
> @@ -3877,22 +3887,20 @@
> BITMAP_DATA_TYPE_BITMAP,
> BITMAP_DATA_TYPE_BITMAP_TO_CACHE,
> } BitmapDataType;
>
> typedef struct BitmapData {
> BitmapDataType type;
> uint64_t id; // surface id or cache item id
> SpiceRect lossy_rect;
> } BitmapData;
>
> -static inline int validate_surface(DisplayChannel *display, uint32_t
> surface_id);
> -
> static inline void display_begin_send_message(RedChannelClient *rcc);
> 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);
>
> QXLInstance* red_worker_get_qxl(RedWorker *worker)
> {
> spice_return_val_if_fail(worker != NULL, NULL);
>
> return worker->qxl;
> @@ -7133,33 +7141,33 @@
> SpiceMsgDisplayStreamActivateReport msg;
>
> red_channel_client_init_send_data(rcc,
> SPICE_MSG_DISPLAY_STREAM_ACTIVATE_REPORT, NULL);
> msg.stream_id = stream_id;
> msg.unique_id = agent->report_id;
> msg.max_window_size = RED_STREAM_CLIENT_REPORT_WINDOW;
> msg.timeout_ms = RED_STREAM_CLIENT_REPORT_TIMEOUT;
> spice_marshall_msg_display_stream_activate_report(base_marshaller,
> &msg);
> }
>
> -static inline void red_display_reset_send_data(DisplayChannelClient *dcc)
> +static void reset_send_data(DisplayChannelClient *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));
> }
>
> -static void send_item(RedChannelClient *rcc, PipeItem *pipe_item)
> +void dcc_send_item(DisplayChannelClient *dcc, PipeItem *pipe_item)
> {
> + RedChannelClient *rcc = RED_CHANNEL_CLIENT(dcc);
> SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> - DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
>
> - red_display_reset_send_data(dcc);
> + reset_send_data(dcc);
> switch (pipe_item->type) {
> case PIPE_ITEM_TYPE_DRAW: {
> DrawablePipeItem *dpi = SPICE_CONTAINEROF(pipe_item,
> DrawablePipeItem, dpi_pipe_item);
> marshall_qxl_drawable(rcc, m, dpi);
> break;
> }
> case PIPE_ITEM_TYPE_INVAL_ONE:
> red_marshall_inval_palette(rcc, m, (CacheItem *)pipe_item);
> break;
> case PIPE_ITEM_TYPE_STREAM_CREATE: {
> @@ -8724,21 +8732,22 @@
> init_info.memslot_gen_bits,
> init_info.memslot_id_bits,
> init_info.internal_groupslot_id);
>
> spice_warn_if(init_info.n_surfaces > NUM_SURFACES);
>
> worker->event_timeout = INF_EVENT_WAIT;
>
> worker->cursor_channel = cursor_channel_new(worker);
> // TODO: handle seemless migration. Temp, setting migrate to FALSE
> - display_channel_create(worker, FALSE, streaming_video,
> init_info.n_surfaces);
> + worker->display_channel = display_channel_new(worker, FALSE,
> streaming_video,
> + init_info.n_surfaces);
>
> return worker;
> }
>
> SPICE_GNUC_NORETURN static void *red_worker_main(void *arg)
> {
> RedWorker *worker = arg;
>
> spice_info("begin");
> spice_assert(MAX_PIPE_SIZE > WIDE_CLIENT_ACK_WINDOW &&
I'll remove the 2 useless lines above.
Frediano
More information about the Spice-devel
mailing list