Re: [PATCH 02/18] worker: move attach_stream

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Okay, the patch that I have applied (git-pw apply 994) seems a bit
different from this one as it doesn't have the changes on dcc.c, which
are unrelated anyway.
The commit message should be clear about moving detach_streams function as well.


On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>
> ---
>  server/dcc.c        |  5 ++--
>  server/red_worker.c | 74 +++++++----------------------------------------------
>  server/stream.c     | 65 ++++++++++++++++++++++++++++++++++++++++++++--
>  server/stream.h     |  1 -
>  4 files changed, 74 insertions(+), 71 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index 9af6d84..8cc2c0a 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -654,13 +654,12 @@ int dcc_compress_image_lz4(DisplayChannelClient *dcc, SpiceImage *dest,
>                             SpiceBitmap *src, compress_send_data_t* o_comp_data,
>                             uint32_t group_id)
>  {
> -    DisplayChannel *display_channel = DCC_TO_DC(dcc);
>      Lz4Data *lz4_data = &dcc->lz4_data;
>      Lz4EncoderContext *lz4 = dcc->lz4;
>      int lz4_size = 0;
>
>  #ifdef COMPRESS_STAT
> -    stat_time_t start_time = stat_now(display_channel->lz4_stat.clock);
> +    stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->lz4_stat.clock);
>  #endif
>
>      lz4_data->data.bufs_tail = compress_buf_new();
> @@ -707,7 +706,7 @@ int dcc_compress_image_lz4(DisplayChannelClient *dcc, SpiceImage *dest,
>      o_comp_data->comp_buf = lz4_data->data.bufs_head;
>      o_comp_data->comp_buf_size = lz4_size;
>
> -    stat_compress_add(&display_channel->lz4_stat, start_time, src->stride * src->y,
> +    stat_compress_add(&DCC_TO_DC(dcc)->lz4_stat, start_time, src->stride * src->y,
>                        o_comp_data->comp_buf_size);
>      return TRUE;
>  }
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 1963b0c..6e859fe 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -191,50 +191,6 @@ static void red_create_surface(DisplayChannel *display, uint32_t surface_id, uin
>                                 uint32_t height, int32_t stride, uint32_t format,
>                                 void *line_0, int data_is_valid, int send_client);
>
> -void attach_stream(DisplayChannel *display, Drawable *drawable, Stream *stream)
> -{
> -    DisplayChannelClient *dcc;
> -    RingItem *item, *next;
> -
> -    spice_assert(!drawable->stream && !stream->current);
> -    spice_assert(drawable && stream);
> -    stream->current = drawable;
> -    drawable->stream = stream;
> -    stream->last_time = drawable->creation_time;
> -
> -    uint64_t duration = drawable->creation_time - stream->input_fps_start_time;
> -    if (duration >= RED_STREAM_INPUT_FPS_TIMEOUT) {
> -        /* Round to the nearest integer, for instance 24 for 23.976 */
> -        stream->input_fps = ((uint64_t)stream->num_input_frames * 1000 * 1000 * 1000 + duration / 2) / duration;
> -        spice_debug("input-fps=%u", stream->input_fps);
> -        stream->num_input_frames = 0;
> -        stream->input_fps_start_time = drawable->creation_time;
> -    } else {
> -        stream->num_input_frames++;
> -    }
> -
> -    FOREACH_DCC(display, item, next, dcc) {
> -        StreamAgent *agent;
> -        QRegion clip_in_draw_dest;
> -
> -        agent = &dcc->stream_agents[get_stream_id(display, stream)];
> -        region_or(&agent->vis_region, &drawable->tree_item.base.rgn);
> -
> -        region_init(&clip_in_draw_dest);
> -        region_add(&clip_in_draw_dest, &drawable->red_drawable->bbox);
> -        region_and(&clip_in_draw_dest, &agent->clip);
> -
> -        if (!region_is_equal(&clip_in_draw_dest, &drawable->tree_item.base.rgn)) {
> -            region_remove(&agent->clip, &drawable->red_drawable->bbox);
> -            region_or(&agent->clip, &drawable->tree_item.base.rgn);
> -            dcc_stream_agent_clip(dcc, agent);
> -        }
> -#ifdef STREAM_STATS
> -        agent->stats.num_input_frames++;
> -#endif
> -    }
> -}
> -
>  /* fixme: move to display channel */
>  DrawablePipeItem *drawable_pipe_item_new(DisplayChannelClient *dcc,
>                                           Drawable *drawable)
> @@ -829,18 +785,6 @@ static void red_clear_surface_drawables_from_pipes(DisplayChannel *display,
>      }
>  }
>
> -void detach_stream(DisplayChannel *display, Stream *stream,
> -                   int detach_sized)
> -{
> -    spice_assert(stream->current && stream->current->stream);
> -    spice_assert(stream->current->stream == stream);
> -    stream->current->stream = NULL;
> -    if (detach_sized) {
> -        stream->current->sized_stream = NULL;
> -    }
> -    stream->current = NULL;
> -}
> -
>  static int red_display_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *drawable)
>  {
>      DrawablePipeItem *dpi;
> @@ -1391,15 +1335,6 @@ static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces,
>      return display->surfaces[surface_id].context.canvas;
>  }
>
> -static void image_surface_init(DisplayChannel *display)
> -{
> -    static SpiceImageSurfacesOps image_surfaces_ops = {
> -        image_surfaces_get,
> -    };
> -
> -    display->image_surfaces.ops = &image_surfaces_ops;
> -}
> -
>  static void drawable_draw(DisplayChannel *display, Drawable *drawable)
>  {
>      RedSurface *surface;
> @@ -5500,6 +5435,15 @@ static void display_channel_release_item(RedChannelClient *rcc, PipeItem *item,
>      }
>  }
>
> +static void image_surface_init(DisplayChannel *display)
> +{
> +    static SpiceImageSurfacesOps image_surfaces_ops = {
> +        image_surfaces_get,
> +    };
> +
> +    display->image_surfaces.ops = &image_surfaces_ops;
> +}
> +

Moving image_surface_init() around doesn't seem part of this patch.

>  static void display_channel_create(RedWorker *worker, int migrate, int stream_video,
>                                     uint32_t n_surfaces)
>  {
> diff --git a/server/stream.c b/server/stream.c
> index a2acd3a..357c632 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -258,6 +258,67 @@ static int is_next_stream_frame(DisplayChannel *display,
>      }
>  }
>
> +static void attach_stream(DisplayChannel *display, Drawable *drawable, Stream *stream)
> +{
> +    DisplayChannelClient *dcc;
> +    RingItem *item, *next;
> +
> +    spice_return_if_fail(!drawable->stream);
> +    spice_return_if_fail(!stream->current);
> +    spice_return_if_fail(drawable);
> +    spice_return_if_fail(stream);

AFAIU we would not change the spice_assert() to
spice_return_if_fail(), would we?

> +
> +    stream->current = drawable;
> +    drawable->stream = stream;
> +    stream->last_time = drawable->creation_time;
> +
> +    uint64_t duration = drawable->creation_time - stream->input_fps_start_time;
> +    if (duration >= RED_STREAM_INPUT_FPS_TIMEOUT) {
> +        /* Round to the nearest integer, for instance 24 for 23.976 */
> +        stream->input_fps = ((uint64_t)stream->num_input_frames * 1000 * 1000 * 1000 + duration / 2) / duration;
> +        spice_debug("input-fps=%u", stream->input_fps);
> +        stream->num_input_frames = 0;
> +        stream->input_fps_start_time = drawable->creation_time;
> +    } else {
> +        stream->num_input_frames++;
> +    }
> +
> +    FOREACH_DCC(display, item, next, dcc) {
> +        StreamAgent *agent;
> +        QRegion clip_in_draw_dest;
> +
> +        agent = &dcc->stream_agents[get_stream_id(display, stream)];
> +        region_or(&agent->vis_region, &drawable->tree_item.base.rgn);
> +
> +        region_init(&clip_in_draw_dest);
> +        region_add(&clip_in_draw_dest, &drawable->red_drawable->bbox);
> +        region_and(&clip_in_draw_dest, &agent->clip);
> +
> +        if (!region_is_equal(&clip_in_draw_dest, &drawable->tree_item.base.rgn)) {
> +            region_remove(&agent->clip, &drawable->red_drawable->bbox);
> +            region_or(&agent->clip, &drawable->tree_item.base.rgn);
> +            dcc_stream_agent_clip(dcc, agent);
> +        }
> +#ifdef STREAM_STATS
> +        agent->stats.num_input_frames++;
> +#endif
> +    }
> +}
> +
> +void detach_stream(DisplayChannel *display, Stream *stream,
> +                   int detach_sized)
> +{
> +    spice_return_if_fail(stream->current);
> +    spice_return_if_fail(stream->current->stream);
> +    spice_return_if_fail(stream->current->stream == stream);
> +

Same here ...

> +    stream->current->stream = NULL;
> +    if (detach_sized) {
> +        stream->current->sized_stream = NULL;
> +    }
> +    stream->current = NULL;
> +}
> +
>  static void before_reattach_stream(DisplayChannel *display,
>                                     Stream *stream, Drawable *new_frame)
>  {
> @@ -350,13 +411,13 @@ static void display_channel_create_stream(DisplayChannel *display, Drawable *dra
>      Stream *stream;
>      SpiceRect* src_rect;
>
> -    spice_assert(!drawable->stream);
> +    spice_return_if_fail(!drawable->stream);

Same here ...

>
>      if (!(stream = display_channel_stream_try_new(display))) {
>          return;
>      }
>
> -    spice_assert(drawable->red_drawable->type == QXL_DRAW_COPY);
> +    spice_return_if_fail(drawable->red_drawable->type == QXL_DRAW_COPY);

Same here ...

>      src_rect = &drawable->red_drawable->u.copy.src_area;
>
>      ring_add(&display->streams, &stream->link);
> diff --git a/server/stream.h b/server/stream.h
> index 3627b0f..efe7ceb 100644
> --- a/server/stream.h
> +++ b/server/stream.h
> @@ -155,7 +155,6 @@ void                  stream_agent_stats_print                      (StreamAgent
>  void                  stream_agent_stop                             (DisplayChannelClient *dcc,
>                                                                       StreamAgent *agent);
>
> -void attach_stream(DisplayChannel *display, Drawable *drawable, Stream *stream);
>  void detach_stream(DisplayChannel *display, Stream *stream, int detach_sized);
>
>  #endif /* STREAM_H */
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

Apart from the comments, patch looks good.
-- 
Fabiano Fidêncio
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]