[Spice-devel] [PATCH 07/23] handle STREAM_CREATE and STREAM_DESTROY pipe items more coherently

Jonathon Jongsma jjongsma at redhat.com
Tue May 17 19:59:15 UTC 2016


Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


On Mon, 2016-05-16 at 14:31 +0100, Frediano Ziglio wrote:
> The items of these pipe items were allocated staticaly inside the
> StreamAgent structure. All others RedPipeItem are allocated dynamically.
> This could solve possible future maintenance as the life of these
> item is more easier to understand.
> It's more easier to understand why reference where incremented.
> It also make the StreamAgent structure a bit smaller.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/dcc-send.c |  8 ++++----
>  server/dcc.c      | 26 --------------------------
>  server/stream.c   | 37 ++++++++++++++++++++++++++++++++-----
>  server/stream.h   |  7 +++++--
>  4 files changed, 41 insertions(+), 37 deletions(-)
> 
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 5f967cc..0f645a9 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -2383,8 +2383,8 @@ void dcc_send_item(DisplayChannelClient *dcc,
> RedPipeItem *pipe_item)
>          marshall_inval_palette(rcc, m, (RedCacheItem *)pipe_item);
>          break;
>      case RED_PIPE_ITEM_TYPE_STREAM_CREATE: {
> -        StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent,
> create_item);
> -        marshall_stream_start(rcc, m, agent);
> +        StreamCreateDestroyItem *item = SPICE_CONTAINEROF(pipe_item,
> StreamCreateDestroyItem, base);
> +        marshall_stream_start(rcc, m, item->agent);
>          break;
>      }
>      case RED_PIPE_ITEM_TYPE_STREAM_CLIP: {
> @@ -2393,8 +2393,8 @@ void dcc_send_item(DisplayChannelClient *dcc,
> RedPipeItem *pipe_item)
>          break;
>      }
>      case RED_PIPE_ITEM_TYPE_STREAM_DESTROY: {
> -        StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent,
> destroy_item);
> -        marshall_stream_end(rcc, m, agent);
> +        StreamCreateDestroyItem *item = SPICE_CONTAINEROF(pipe_item,
> StreamCreateDestroyItem, base);
> +        marshall_stream_end(rcc, m, item->agent);
>          break;
>      }
>      case RED_PIPE_ITEM_TYPE_UPGRADE:
> diff --git a/server/dcc.c b/server/dcc.c
> index 75f5120..286514c 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -335,28 +335,6 @@ void dcc_add_drawable_after(DisplayChannelClient *dcc,
> Drawable *drawable, RedPi
>      red_channel_client_pipe_add_after(RED_CHANNEL_CLIENT(dcc), &dpi
> ->dpi_pipe_item, pos);
>  }
>  
> -static void dcc_release_stream_create(RedPipeItem *item)
> -{
> -    StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
> -    DisplayChannel *display = (DisplayChannel*)agent->dcc
> ->common.base.channel;
> -    stream_agent_unref(display, agent);
> -
> -    /* this RedPipeItem it's quite weird, basically it's always present */
> -    red_pipe_item_init_full(&agent->create_item,
> RED_PIPE_ITEM_TYPE_STREAM_CREATE,
> -                            (GDestroyNotify)dcc_release_stream_create);
> -}
> -
> -static void dcc_release_stream_destroy(RedPipeItem *item)
> -{
> -    StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item);
> -    DisplayChannel *display = (DisplayChannel*)agent->dcc
> ->common.base.channel;
> -    stream_agent_unref(display, agent);
> -
> -    /* this RedPipeItem it's quite weird, basically it's always present */
> -    red_pipe_item_init_full(&agent->destroy_item,
> RED_PIPE_ITEM_TYPE_STREAM_DESTROY,
> -                            (GDestroyNotify)dcc_release_stream_destroy);
> -}
> -
>  static void dcc_init_stream_agents(DisplayChannelClient *dcc)
>  {
>      int i;
> @@ -367,10 +345,6 @@ static void dcc_init_stream_agents(DisplayChannelClient
> *dcc)
>          agent->stream = &display->streams_buf[i];
>          region_init(&agent->vis_region);
>          region_init(&agent->clip);
> -        red_pipe_item_init_full(&agent->create_item,
> RED_PIPE_ITEM_TYPE_STREAM_CREATE,
> -                                (GDestroyNotify)dcc_release_stream_create);
> -        red_pipe_item_init_full(&agent->destroy_item,
> RED_PIPE_ITEM_TYPE_STREAM_DESTROY,
> -                                (GDestroyNotify)dcc_release_stream_destroy);
>      }
>      dcc->use_video_encoder_rate_control =
>          red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(dcc),
> SPICE_DISPLAY_CAP_STREAM_REPORT);
> diff --git a/server/stream.c b/server/stream.c
> index 35eb7c2..4c733de 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -63,6 +63,36 @@ void stream_agent_stats_print(StreamAgent *agent)
>  #endif
>  }
>  
> +static void stream_create_destroy_item_release(RedPipeItem *base)
> +{
> +    StreamCreateDestroyItem *item = SPICE_CONTAINEROF(base,
> StreamCreateDestroyItem, base);
> +    DisplayChannel *display = (DisplayChannel*)item->agent->dcc
> ->common.base.channel;
> +    stream_agent_unref(display, item->agent);
> +    free(item);
> +}
> +
> +static RedPipeItem *stream_create_destroy_item_new(StreamAgent *agent, gint
> type)
> +{
> +    StreamCreateDestroyItem *item = spice_new0(StreamCreateDestroyItem, 1);
> +
> +    red_pipe_item_init_full(&item->base, type,
> +                           
>  (GDestroyNotify)stream_create_destroy_item_release);
> +    agent->stream->refs++;
> +    item->agent = agent;
> +    return &item->base;
> +}
> +
> +static RedPipeItem *stream_create_item_new(StreamAgent *agent)
> +{
> +    return stream_create_destroy_item_new(agent,
> RED_PIPE_ITEM_TYPE_STREAM_CREATE);
> +}
> +
> +static RedPipeItem *stream_destroy_item_new(StreamAgent *agent)
> +{
> +    return stream_create_destroy_item_new(agent,
> RED_PIPE_ITEM_TYPE_STREAM_DESTROY);
> +}
> +
> +
>  void stream_stop(DisplayChannel *display, Stream *stream)
>  {
>      DisplayChannelClient *dcc;
> @@ -78,7 +108,6 @@ void stream_stop(DisplayChannel *display, Stream *stream)
>          stream_agent = &dcc->stream_agents[get_stream_id(display, stream)];
>          region_clear(&stream_agent->vis_region);
>          region_clear(&stream_agent->clip);
> -        spice_assert(!red_pipe_item_is_linked(&stream_agent->destroy_item));
>          if (stream_agent->video_encoder && dcc
> ->use_video_encoder_rate_control) {
>              uint64_t stream_bit_rate = stream_agent->video_encoder
> ->get_bit_rate(stream_agent->video_encoder);
>  
> @@ -89,8 +118,7 @@ void stream_stop(DisplayChannel *display, Stream *stream)
>                  dcc->streams_max_bit_rate = stream_bit_rate;
>              }
>          }
> -        stream->refs++;
> -        red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &stream_agent
> ->destroy_item);
> +        red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc),
> stream_destroy_item_new(stream_agent));
>          stream_agent_stats_print(stream_agent);
>      }
>      display->streams_size_total -= stream->width * stream->height;
> @@ -702,7 +730,6 @@ void dcc_create_stream(DisplayChannelClient *dcc, Stream
> *stream)
>  
>      spice_return_if_fail(region_is_empty(&agent->vis_region));
>  
> -    stream->refs++;
>      if (stream->current) {
>          agent->frames = 1;
>          region_clone(&agent->vis_region, &stream->current
> ->tree_item.base.rgn);
> @@ -728,7 +755,7 @@ void dcc_create_stream(DisplayChannelClient *dcc, Stream
> *stream)
>      } else {
>          agent->video_encoder = mjpeg_encoder_new(0, NULL);
>      }
> -    red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &agent
> ->create_item);
> +    red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc),
> stream_create_item_new(agent));
>  
>      if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(dcc),
> SPICE_DISPLAY_CAP_STREAM_REPORT)) {
>          RedStreamActivateReportItem *report_pipe_item =
> spice_malloc0(sizeof(*report_pipe_item));
> diff --git a/server/stream.h b/server/stream.h
> index 3c8d761..511d752 100644
> --- a/server/stream.h
> +++ b/server/stream.h
> @@ -82,8 +82,6 @@ typedef struct StreamAgent {
>                             vis_region will contain c2 and also the part of
> c1/c2 that still
>                             displays fragments of the video */
>  
> -    RedPipeItem create_item;
> -    RedPipeItem destroy_item;
>      Stream *stream;
>      uint64_t last_send_time;
>      VideoEncoder *video_encoder;
> @@ -109,6 +107,11 @@ typedef struct RedStreamClipItem {
>  
>  RedStreamClipItem *   red_stream_clip_item_new                     
>  (StreamAgent *agent);
>  
> +typedef struct StreamCreateDestroyItem {
> +    RedPipeItem base;
> +    StreamAgent *agent;
> +} StreamCreateDestroyItem;
> +
>  typedef struct ItemTrace {
>      red_time_t time;
>      red_time_t first_frame_time;


More information about the Spice-devel mailing list