[Spice-devel] [PATCH 02/18] worker: move attach_stream
Fabiano Fidêncio
fabiano at fidencio.org
Fri Nov 20 06:20:27 PST 2015
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 at redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> ---
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
Apart from the comments, patch looks good.
--
Fabiano Fidêncio
More information about the Spice-devel
mailing list