On Thu, 2016-09-15 at 11:22 -0500, Jonathon Jongsma wrote:
> 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(displ
> ay, 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(displ
> ay, 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;

should be changed to gboolean values (it is in the current code, so
not issue of this patch)

> +}
> +
> +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_REPOR
> T);
> -        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) {
> 

Acked-by: Pavel Grunt <pgr...@redhat.com>


_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to