[Spice-devel] [PATCH v2 1/2] Improve encapsulation of DisplayChannel
Jonathon Jongsma
jjongsma at redhat.com
Thu Sep 15 16:22:58 UTC 2016
Add a couple new functions to the header so that they can be called by
other objects rather than poking into the internals of the struct.
---
server/dcc-send.c | 16 +++++------
server/display-channel.c | 71 ++++++++++++++++++++++++++++++++++++++++++++----
server/display-channel.h | 37 +++++++++++++------------
server/red-worker.c | 44 ++++++------------------------
server/stream.c | 18 ++++++------
5 files changed, 109 insertions(+), 77 deletions(-)
diff --git a/server/dcc-send.c b/server/dcc-send.c
index 521e6a2..0afa25c 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -94,7 +94,7 @@ static int is_surface_area_lossy(DisplayChannelClient *dcc, uint32_t surface_id,
QRegion lossy_region;
DisplayChannel *display = DCC_TO_DC(dcc);
- spice_return_val_if_fail(validate_surface(display, surface_id), FALSE);
+ spice_return_val_if_fail(display_channel_validate_surface(display, surface_id), FALSE);
surface = &display->surfaces[surface_id];
surface_lossy_region = &dcc->priv->surface_client_lossy_region[surface_id];
@@ -414,7 +414,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
RedSurface *surface;
surface_id = simage->u.surface.surface_id;
- if (!validate_surface(display, surface_id)) {
+ if (!display_channel_validate_surface(display, surface_id)) {
spice_warning("Invalid surface in SPICE_IMAGE_TYPE_SURFACE");
pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
return FILL_BITS_TYPE_SURFACE;
@@ -1706,7 +1706,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
return FALSE;
}
- StreamAgent *agent = &dcc->priv->stream_agents[get_stream_id(display, stream)];
+ StreamAgent *agent = &dcc->priv->stream_agents[display_channel_get_stream_id(display, stream)];
uint64_t time_now = spice_get_monotonic_time_ns();
if (!dcc->priv->use_video_encoder_rate_control) {
@@ -1752,7 +1752,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DATA, NULL);
- stream_data.base.id = get_stream_id(display, stream);
+ stream_data.base.id = display_channel_get_stream_id(display, stream);
stream_data.base.multi_media_time = frame_mm_time;
stream_data.data_size = outbuf->size;
@@ -1762,7 +1762,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DATA_SIZED, NULL);
- stream_data.base.id = get_stream_id(display, stream);
+ stream_data.base.id = display_channel_get_stream_id(display, stream);
stream_data.base.multi_media_time = frame_mm_time;
stream_data.data_size = outbuf->size;
stream_data.width = copy->src_area.right - copy->src_area.left;
@@ -2171,7 +2171,7 @@ static void marshall_stream_start(RedChannelClient *rcc,
SpiceClipRects clip_rects;
stream_create.surface_id = 0;
- stream_create.id = get_stream_id(DCC_TO_DC(dcc), stream);
+ stream_create.id = display_channel_get_stream_id(DCC_TO_DC(dcc), stream);
stream_create.flags = stream->top_down ? SPICE_STREAM_FLAGS_TOP_DOWN : 0;
stream_create.codec_type = agent->video_encoder->codec_type;
@@ -2207,7 +2207,7 @@ static void marshall_stream_clip(RedChannelClient *rcc,
red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_CLIP, &item->base);
SpiceMsgDisplayStreamClip stream_clip;
- stream_clip.id = get_stream_id(DCC_TO_DC(dcc), agent->stream);
+ stream_clip.id = display_channel_get_stream_id(DCC_TO_DC(dcc), agent->stream);
stream_clip.clip.type = item->clip_type;
stream_clip.clip.rects = item->rects;
@@ -2221,7 +2221,7 @@ static void marshall_stream_end(RedChannelClient *rcc,
SpiceMsgDisplayStreamDestroy destroy;
red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DESTROY, NULL);
- destroy.id = get_stream_id(DCC_TO_DC(dcc), agent->stream);
+ destroy.id = display_channel_get_stream_id(DCC_TO_DC(dcc), agent->stream);
stream_agent_stop(agent);
spice_marshall_msg_display_stream_destroy(base_marshaller, &destroy);
}
diff --git a/server/display-channel.c b/server/display-channel.c
index 108e69b..56bb029 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -203,6 +203,13 @@ void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id)
spice_warn_if_fail(ring_is_empty(&surface->depend_on_me));
}
+/* TODO: perhaps rename to "ready" or "realized" ? */
+bool display_channel_surface_has_canvas(DisplayChannel *display,
+ uint32_t surface_id)
+{
+ return display->surfaces[surface_id].context.canvas != NULL;
+}
+
static void streams_update_visible_region(DisplayChannel *display, Drawable *drawable)
{
Ring *ring;
@@ -232,7 +239,7 @@ static void streams_update_visible_region(DisplayChannel *display, Drawable *dra
}
FOREACH_CLIENT(display, link, next, dcc) {
- agent = dcc_get_stream_agent(dcc, get_stream_id(display, stream));
+ agent = dcc_get_stream_agent(dcc, display_channel_get_stream_id(display, stream));
if (region_intersects(&agent->vis_region, &drawable->tree_item.base.rgn)) {
region_exclude(&agent->vis_region, &drawable->tree_item.base.rgn);
@@ -955,7 +962,7 @@ static int validate_drawable_bbox(DisplayChannel *display, RedDrawable *drawable
/* surface_id must be validated before calling into
* validate_drawable_bbox
*/
- if (!validate_surface(display, drawable->surface_id)) {
+ if (!display_channel_validate_surface(display, drawable->surface_id)) {
return FALSE;
}
context = &display->surfaces[surface_id].context;
@@ -998,7 +1005,7 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e
}
for (x = 0; x < 3; ++x) {
if (red_drawable->surface_deps[x] != -1
- && !validate_surface(display, red_drawable->surface_deps[x])) {
+ && !display_channel_validate_surface(display, red_drawable->surface_deps[x])) {
return NULL;
}
}
@@ -1669,7 +1676,7 @@ void display_channel_update(DisplayChannel *display,
SpiceRect rect;
RedSurface *surface;
- spice_return_if_fail(validate_surface(display, surface_id));
+ spice_return_if_fail(display_channel_validate_surface(display, surface_id));
red_get_rect_ptr(&rect, area);
display_channel_draw(display, &rect, surface_id);
@@ -1712,7 +1719,7 @@ static void display_channel_destroy_surface(DisplayChannel *display, uint32_t su
void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surface_id)
{
- if (!validate_surface(display, surface_id))
+ if (!display_channel_validate_surface(display, surface_id))
return;
if (!display->surfaces[surface_id].context.canvas)
return;
@@ -1882,7 +1889,7 @@ static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces, uint32_t su
{
DisplayChannel *display = SPICE_CONTAINEROF(surfaces, DisplayChannel, image_surfaces);
- spice_return_val_if_fail(validate_surface(display, surface_id), NULL);
+ spice_return_val_if_fail(display_channel_validate_surface(display, surface_id), NULL);
return display->surfaces[surface_id].context.canvas;
}
@@ -2038,3 +2045,55 @@ void display_channel_gl_draw_done(DisplayChannel *display)
{
set_gl_draw_async_count(display, display->gl_draw_async_count - 1);
}
+
+int display_channel_get_stream_id(DisplayChannel *display, Stream *stream)
+{
+ return (int)(stream - display->streams_buf);
+}
+
+gboolean display_channel_validate_surface(DisplayChannel *display, uint32_t surface_id)
+{
+ if SPICE_UNLIKELY(surface_id >= display->n_surfaces) {
+ spice_warning("invalid surface_id %u", surface_id);
+ return 0;
+ }
+ if (!display->surfaces[surface_id].context.canvas) {
+ spice_warning("canvas address is %p for %d (and is NULL)\n",
+ &(display->surfaces[surface_id].context.canvas), surface_id);
+ spice_warning("failed on %d", surface_id);
+ return 0;
+ }
+ return 1;
+}
+
+void display_channel_update_monitors_config(DisplayChannel *display,
+ QXLMonitorsConfig *config,
+ uint16_t count, uint16_t max_allowed)
+{
+
+ if (display->monitors_config)
+ monitors_config_unref(display->monitors_config);
+
+ display->monitors_config =
+ monitors_config_new(config->heads, count, max_allowed);
+}
+
+void set_monitors_config_to_primary(DisplayChannel *display)
+{
+ DrawContext *context = &display->surfaces[0].context;
+ QXLHead head = { 0, };
+
+ spice_return_if_fail(display->surfaces[0].context.canvas);
+
+ if (display->monitors_config)
+ monitors_config_unref(display->monitors_config);
+
+ head.width = context->width;
+ head.height = context->height;
+ display->monitors_config = monitors_config_new(&head, 1, 1);
+}
+
+void display_channel_reset_image_cache(DisplayChannel *self)
+{
+ image_cache_reset(&self->image_cache);
+}
diff --git a/server/display-channel.h b/server/display-channel.h
index 7b71480..022cd93 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -208,10 +208,17 @@ struct DisplayChannel {
ImageEncoderSharedData encoder_shared_data;
};
-static inline int get_stream_id(DisplayChannel *display, Stream *stream)
-{
- return (int)(stream - display->streams_buf);
-}
+
+#define FOREACH_DCC(channel, _link, _next, _data) \
+ for (_link = (channel ? RED_CHANNEL(channel)->clients : NULL), \
+ _next = (_link ? _link->next : NULL), \
+ _data = (_link ? _link->data : NULL); \
+ _link; \
+ _link = _next, \
+ _next = (_link ? _link->next : NULL), \
+ _data = (_link ? _link->data : NULL))
+
+int display_channel_get_stream_id(DisplayChannel *display, Stream *stream);
typedef struct RedSurfaceDestroyItem {
RedPipeItem pipe_item;
@@ -258,6 +265,8 @@ void display_channel_compress_stats_print (const Disp
void display_channel_compress_stats_reset (DisplayChannel *display);
void display_channel_surface_unref (DisplayChannel *display,
uint32_t surface_id);
+bool display_channel_surface_has_canvas (DisplayChannel *display,
+ uint32_t surface_id);
void display_channel_current_flush (DisplayChannel *display,
int surface_id);
int display_channel_wait_for_migrate_data (DisplayChannel *display);
@@ -281,20 +290,12 @@ void display_channel_gl_draw (DisplayCha
SpiceMsgDisplayGlDraw *draw);
void display_channel_gl_draw_done (DisplayChannel *display);
-static inline int validate_surface(DisplayChannel *display, uint32_t surface_id)
-{
- if SPICE_UNLIKELY(surface_id >= display->n_surfaces) {
- spice_warning("invalid surface_id %u", surface_id);
- return 0;
- }
- if (!display->surfaces[surface_id].context.canvas) {
- spice_warning("canvas address is %p for %d (and is NULL)\n",
- &(display->surfaces[surface_id].context.canvas), surface_id);
- spice_warning("failed on %d", surface_id);
- return 0;
- }
- return 1;
-}
+void display_channel_update_monitors_config(DisplayChannel *display, QXLMonitorsConfig *config,
+ uint16_t count, uint16_t max_allowed);
+void set_monitors_config_to_primary(DisplayChannel *display);
+
+gboolean display_channel_validate_surface(DisplayChannel *display, uint32_t surface_id);
+void display_channel_reset_image_cache(DisplayChannel *self);
static inline int is_equal_path(SpicePath *path1, SpicePath *path2)
{
diff --git a/server/red-worker.c b/server/red-worker.c
index 590412b..6df6420 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -247,7 +247,7 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
&update, ext_cmd.cmd.data)) {
break;
}
- if (!validate_surface(worker->display_channel, update.surface_id)) {
+ if (!display_channel_validate_surface(worker->display_channel, update.surface_id)) {
spice_warning("Invalid surface in QXL_CMD_UPDATE");
} else {
display_channel_draw(worker->display_channel, &update.area, update.surface_id);
@@ -587,18 +587,6 @@ static void handle_dev_destroy_surfaces(void *opaque, void *payload)
cursor_channel_reset(worker->cursor_channel);
}
-static void display_update_monitors_config(DisplayChannel *display,
- QXLMonitorsConfig *config,
- uint16_t count, uint16_t max_allowed)
-{
-
- if (display->monitors_config)
- monitors_config_unref(display->monitors_config);
-
- display->monitors_config =
- monitors_config_new(config->heads, count, max_allowed);
-}
-
static void red_worker_push_monitors_config(RedWorker *worker)
{
DisplayChannelClient *dcc;
@@ -609,21 +597,6 @@ static void red_worker_push_monitors_config(RedWorker *worker)
}
}
-static void set_monitors_config_to_primary(DisplayChannel *display)
-{
- DrawContext *context = &display->surfaces[0].context;
- QXLHead head = { 0, };
-
- spice_return_if_fail(display->surfaces[0].context.canvas);
-
- if (display->monitors_config)
- monitors_config_unref(display->monitors_config);
-
- head.width = context->width;
- head.height = context->height;
- display->monitors_config = monitors_config_new(&head, 1, 1);
-}
-
static void dev_create_primary_surface(RedWorker *worker, uint32_t surface_id,
QXLDevSurfaceCreate surface)
{
@@ -689,12 +662,12 @@ static void destroy_primary_surface(RedWorker *worker, uint32_t surface_id)
{
DisplayChannel *display = worker->display_channel;
- if (!validate_surface(display, surface_id))
+ if (!display_channel_validate_surface(display, surface_id))
return;
spice_warn_if_fail(surface_id == 0);
spice_debug(NULL);
- if (!display->surfaces[surface_id].context.canvas) {
+ if (!display_channel_surface_has_canvas(display, surface_id)) {
spice_warning("double destroy of primary surface");
return;
}
@@ -704,7 +677,7 @@ static void destroy_primary_surface(RedWorker *worker, uint32_t surface_id)
display_channel_surface_unref(display, 0);
spice_warn_if_fail(ring_is_empty(&display->streams));
- spice_warn_if_fail(!display->surfaces[surface_id].context.canvas);
+ spice_warn_if_fail(!display_channel_surface_has_canvas(display, surface_id));
cursor_channel_reset(worker->cursor_channel);
}
@@ -828,9 +801,8 @@ static void handle_dev_reset_cursor(void *opaque, void *payload)
static void handle_dev_reset_image_cache(void *opaque, void *payload)
{
RedWorker *worker = opaque;
- DisplayChannel *display = worker->display_channel;
- image_cache_reset(&display->image_cache);
+ display_channel_reset_image_cache(worker->display_channel);
}
static void handle_dev_destroy_surface_wait_async(void *opaque, void *payload)
@@ -950,9 +922,9 @@ static void handle_dev_monitors_config_async(void *opaque, void *payload)
/* TODO: raise guest bug (requires added QXL interface) */
return;
}
- display_update_monitors_config(worker->display_channel, dev_monitors_config,
- MIN(count, msg->max_monitors),
- MIN(max_allowed, msg->max_monitors));
+ display_channel_update_monitors_config(worker->display_channel, dev_monitors_config,
+ MIN(count, msg->max_monitors),
+ MIN(max_allowed, msg->max_monitors));
red_worker_push_monitors_config(worker);
}
diff --git a/server/stream.c b/server/stream.c
index 4819723..8997c10 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -101,11 +101,11 @@ void stream_stop(DisplayChannel *display, Stream *stream)
spice_return_if_fail(ring_item_is_linked(&stream->link));
spice_return_if_fail(!stream->current);
- spice_debug("stream %d", get_stream_id(display, stream));
+ spice_debug("stream %d", display_channel_get_stream_id(display, stream));
FOREACH_CLIENT(display, link, next, dcc) {
StreamAgent *stream_agent;
- stream_agent = dcc_get_stream_agent(dcc, get_stream_id(display, stream));
+ stream_agent = dcc_get_stream_agent(dcc, display_channel_get_stream_id(display, stream));
region_clear(&stream_agent->vis_region);
region_clear(&stream_agent->clip);
if (stream_agent->video_encoder && dcc_use_video_encoder_rate_control(dcc)) {
@@ -302,7 +302,7 @@ static void attach_stream(DisplayChannel *display, Drawable *drawable, Stream *s
StreamAgent *agent;
QRegion clip_in_draw_dest;
- agent = dcc_get_stream_agent(dcc, get_stream_id(display, stream));
+ agent = dcc_get_stream_agent(dcc, display_channel_get_stream_id(display, stream));
region_or(&agent->vis_region, &drawable->tree_item.base.rgn);
region_init(&clip_in_draw_dest);
@@ -349,7 +349,7 @@ static void before_reattach_stream(DisplayChannel *display,
return;
}
- index = get_stream_id(display, stream);
+ index = display_channel_get_stream_id(display, stream);
DRAWABLE_FOREACH_DPI_SAFE(stream->current, ring_item, next, dpi) {
dcc = dpi->dcc;
agent = dcc_get_stream_agent(dcc, index);
@@ -759,7 +759,7 @@ static VideoEncoder* dcc_create_video_encoder(DisplayChannelClient *dcc,
void dcc_create_stream(DisplayChannelClient *dcc, Stream *stream)
{
- StreamAgent *agent = dcc_get_stream_agent(dcc, get_stream_id(DCC_TO_DC(dcc), stream));
+ StreamAgent *agent = dcc_get_stream_agent(dcc, display_channel_get_stream_id(DCC_TO_DC(dcc), stream));
spice_return_if_fail(region_is_empty(&agent->vis_region));
@@ -796,7 +796,7 @@ void dcc_create_stream(DisplayChannelClient *dcc, Stream *stream)
agent->report_id = rand();
red_pipe_item_init(&report_pipe_item->pipe_item,
RED_PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT);
- report_pipe_item->stream_id = get_stream_id(DCC_TO_DC(dcc), stream);
+ report_pipe_item->stream_id = display_channel_get_stream_id(DCC_TO_DC(dcc), stream);
red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &report_pipe_item->pipe_item);
}
#ifdef STREAM_STATS
@@ -839,7 +839,7 @@ static void dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
Drawable *update_area_limit)
{
DisplayChannel *display = DCC_TO_DC(dcc);
- int stream_id = get_stream_id(display, stream);
+ int stream_id = display_channel_get_stream_id(display, stream);
StreamAgent *agent = dcc_get_stream_agent(dcc, stream_id);
/* stopping the client from playing older frames at once*/
@@ -936,12 +936,12 @@ void stream_detach_behind(DisplayChannel *display, QRegion *region, Drawable *dr
item = ring_next(ring, item);
FOREACH_CLIENT(display, link, next, dcc) {
- StreamAgent *agent = dcc_get_stream_agent(dcc, get_stream_id(display, stream));
+ StreamAgent *agent = dcc_get_stream_agent(dcc, display_channel_get_stream_id(display, stream));
if (region_intersects(&agent->vis_region, region)) {
dcc_detach_stream_gracefully(dcc, stream, drawable);
detach = 1;
- spice_debug("stream %d", get_stream_id(display, stream));
+ spice_debug("stream %d", display_channel_get_stream_id(display, stream));
}
}
if (detach && stream->current) {
--
2.7.4
More information about the Spice-devel
mailing list