[Spice-devel] [PATCH 01/11] worker: Move stream functions to stream.c
Fabiano Fidêncio
fidencio at redhat.com
Wed Nov 11 05:09:27 PST 2015
On Wed, Nov 11, 2015 at 1:20 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> From: Jonathon Jongsma <jjongsma at redhat.com>
>
> ---
> server/display-channel.h | 17 +++++++-
> server/red_worker.c | 103 ++++-------------------------------------------
> server/stream.c | 70 ++++++++++++++++++++++++++++++++
> server/stream.h | 12 +++++-
> 4 files changed, 103 insertions(+), 99 deletions(-)
>
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 42574a1..f8a1c27 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -57,8 +57,6 @@
> #include "tree.h"
> #include "stream.h"
>
> -typedef struct DisplayChannel DisplayChannel;
> -
> #define PALETTE_CACHE_HASH_SHIFT 8
> #define PALETTE_CACHE_HASH_SIZE (1 << PALETTE_CACHE_HASH_SHIFT)
> #define PALETTE_CACHE_HASH_MASK (PALETTE_CACHE_HASH_SIZE - 1)
> @@ -330,6 +328,21 @@ struct DisplayChannel {
> stat_info_t lz4_stat;
> #endif
> };
> +
> +#define LINK_TO_DCC(ptr) SPICE_CONTAINEROF(ptr, DisplayChannelClient, \
> + common.base.channel_link)
> +#define DCC_FOREACH_SAFE(link, next, dcc, channel) \
> + SAFE_FOREACH(link, next, channel, &(channel)->clients, dcc, LINK_TO_DCC(link))
> +
> +
> +#define FOREACH_DCC(display_channel, link, next, dcc) \
> + DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display_channel))
> +
> +static inline int get_stream_id(DisplayChannel *display, Stream *stream)
> +{
> + return (int)(stream - display->streams_buf);
> +}
> +
> typedef struct SurfaceDestroyItem {
> SpiceMsgSurfaceDestroy surface_destroy;
> PipeItem pipe_item;
> diff --git a/server/red_worker.c b/server/red_worker.c
> index ec1edb1..4d430ad 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -444,22 +444,6 @@ static void display_channel_client_release_item_before_push(DisplayChannelClient
> static void display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
> PipeItem *item);
>
> -/*
> - * Macros to make iterating over stuff easier
> - * The two collections we iterate over:
> - * given a channel, iterate over it's clients
> - */
> -
> -#define LINK_TO_DCC(ptr) SPICE_CONTAINEROF(ptr, DisplayChannelClient, \
> - common.base.channel_link)
> -#define DCC_FOREACH_SAFE(link, next, dcc, channel) \
> - SAFE_FOREACH(link, next, channel, &(channel)->clients, dcc, LINK_TO_DCC(link))
> -
> -
> -#define FOREACH_DCC(display_channel, link, next, dcc) \
> - DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display_channel))
> -
> -
> #define LINK_TO_DPI(ptr) SPICE_CONTAINEROF((ptr), DrawablePipeItem, base)
> #define DRAWABLE_FOREACH_DPI_SAFE(drawable, link, next, dpi) \
> SAFE_FOREACH(link, next, drawable, &(drawable)->pipes, dpi, LINK_TO_DPI(link))
> @@ -471,38 +455,12 @@ static void display_channel_client_release_item_after_push(DisplayChannelClient
> SAFE_FOREACH(link, next, drawable, &(drawable)->glz_ring, glz, LINK_TO_GLZ(link))
>
>
> -static int get_stream_id(DisplayChannel *display, Stream *stream)
> -{
> - return (int)(stream - display->streams_buf);
> -}
> -
> -static void display_stream_free(DisplayChannel *display, Stream *stream)
> -{
> - stream->next = display->free_streams;
> - display->free_streams = stream;
> -}
> -
> -static void display_stream_unref(DisplayChannel *display, Stream *stream)
> -{
> - if (--stream->refs != 0)
> - return;
> -
> - spice_warn_if_fail(!ring_item_is_linked(&stream->link));
> - display_stream_free(display, stream);
> - display->stream_count--;
> -}
> -
> -static void display_stream_agent_unref(DisplayChannel *display, StreamAgent *agent)
> -{
> - display_stream_unref(display, agent->stream);
> -}
> -
> static void display_stream_clip_unref(DisplayChannel *display, StreamClipItem *item)
> {
> if (--item->refs != 0)
> return;
>
> - display_stream_agent_unref(display, item->stream_agent);
> + stream_agent_unref(display, item->stream_agent);
> free(item->rects);
> free(item);
> }
> @@ -567,40 +525,6 @@ void attach_stream(DisplayChannel *display, Drawable *drawable, Stream *stream)
> }
> }
>
> -static void stop_stream(DisplayChannel *display, Stream *stream)
> -{
> - DisplayChannelClient *dcc;
> - RingItem *item, *next;
> -
> - spice_assert(ring_item_is_linked(&stream->link));
> - spice_assert(!stream->current);
> - spice_debug("stream %d", get_stream_id(display, stream));
> - FOREACH_DCC(display, item, next, dcc) {
> - StreamAgent *stream_agent;
> -
> - stream_agent = &dcc->stream_agents[get_stream_id(display, stream)];
> - region_clear(&stream_agent->vis_region);
> - region_clear(&stream_agent->clip);
> - spice_assert(!pipe_item_is_linked(&stream_agent->destroy_item));
> - if (stream_agent->mjpeg_encoder && dcc->use_mjpeg_encoder_rate_control) {
> - uint64_t stream_bit_rate = mjpeg_encoder_get_bit_rate(stream_agent->mjpeg_encoder);
> -
> - if (stream_bit_rate > dcc->streams_max_bit_rate) {
> - spice_debug("old max-bit-rate=%.2f new=%.2f",
> - dcc->streams_max_bit_rate / 8.0 / 1024.0 / 1024.0,
> - stream_bit_rate / 8.0 / 1024.0 / 1024.0);
> - dcc->streams_max_bit_rate = stream_bit_rate;
> - }
> - }
> - stream->refs++;
> - red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &stream_agent->destroy_item);
> - stream_agent_stats_print(stream_agent);
> - }
> - display->streams_size_total -= stream->width * stream->height;
> - ring_remove(&stream->link);
> - display_stream_unref(display, stream);
> -}
> -
> /* fixme: move to display channel */
> DrawablePipeItem *drawable_pipe_item_new(DisplayChannelClient *dcc,
> Drawable *drawable)
> @@ -936,7 +860,7 @@ static void stop_streams(DisplayChannel *display)
> Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
> item = ring_next(ring, item);
> if (!stream->current) {
> - stop_stream(display, stream);
> + stream_stop(display, stream);
> } else {
> spice_info("attached stream");
> }
> @@ -1693,7 +1617,7 @@ static void display_channel_streams_timeout(DisplayChannel *display)
> item = ring_next(ring, item);
> if (now >= (stream->last_time + RED_STREAM_TIMEOUT)) {
> detach_stream_gracefully(display, stream, NULL);
> - stop_stream(display, stream);
> + stream_stop(display, stream);
> }
> }
> }
> @@ -7535,7 +7459,7 @@ static void detach_and_stop_streams(DisplayChannel *display)
> Stream *stream = SPICE_CONTAINEROF(stream_item, Stream, link);
>
> detach_stream_gracefully(display, stream, NULL);
> - stop_stream(display, stream);
> + stream_stop(display, stream);
> }
> }
>
> @@ -8598,7 +8522,7 @@ static void display_channel_client_release_item_before_push(DisplayChannelClient
> }
> case PIPE_ITEM_TYPE_STREAM_CREATE: {
> StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
> - display_stream_agent_unref(display, agent);
> + stream_agent_unref(display, agent);
> break;
> }
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> @@ -8606,7 +8530,7 @@ static void display_channel_client_release_item_before_push(DisplayChannelClient
> break;
> case PIPE_ITEM_TYPE_STREAM_DESTROY: {
> StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item);
> - display_stream_agent_unref(display, agent);
> + stream_agent_unref(display, agent);
> break;
> }
> case PIPE_ITEM_TYPE_UPGRADE:
> @@ -8661,19 +8585,6 @@ static void display_channel_release_item(RedChannelClient *rcc, PipeItem *item,
> }
> }
>
> -static void init_streams(DisplayChannel *display)
> -{
> - int i;
> -
> - ring_init(&display->streams);
> - display->free_streams = NULL;
> - for (i = 0; i < NUM_STREAMS; i++) {
> - Stream *stream = &display->streams_buf[i];
> - ring_item_init(&stream->link);
> - display_stream_free(display, stream);
> - }
> -}
> -
> static void display_channel_create(RedWorker *worker, int migrate)
> {
> DisplayChannel *display_channel;
> @@ -8719,10 +8630,10 @@ static void display_channel_create(RedWorker *worker, int migrate)
> display_channel->num_renderers = num_renderers;
> memcpy(display_channel->renderers, renderers, sizeof(display_channel->renderers));
> display_channel->renderer = RED_RENDERER_INVALID;
> - init_streams(display_channel);
> image_cache_init(&display_channel->image_cache);
> ring_init(&display_channel->current_list);
> drawables_init(display_channel);
> + stream_init(display_channel);
> }
>
> static void guest_set_client_capabilities(RedWorker *worker)
> diff --git a/server/stream.c b/server/stream.c
> index 7ef7876..d61cec1 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -57,6 +57,76 @@ void stream_agent_stats_print(StreamAgent *agent)
> #endif
> }
>
> +void stream_stop(DisplayChannel *display, Stream *stream)
> +{
> + DisplayChannelClient *dcc;
> + RingItem *item, *next;
> +
> + 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));
> + FOREACH_DCC(display, item, next, dcc) {
> + StreamAgent *stream_agent;
> +
> + stream_agent = &dcc->stream_agents[get_stream_id(display, stream)];
> + region_clear(&stream_agent->vis_region);
> + region_clear(&stream_agent->clip);
> + spice_assert(!pipe_item_is_linked(&stream_agent->destroy_item));
> + if (stream_agent->mjpeg_encoder && dcc->use_mjpeg_encoder_rate_control) {
> + uint64_t stream_bit_rate = mjpeg_encoder_get_bit_rate(stream_agent->mjpeg_encoder);
> +
> + if (stream_bit_rate > dcc->streams_max_bit_rate) {
> + spice_debug("old max-bit-rate=%.2f new=%.2f",
> + dcc->streams_max_bit_rate / 8.0 / 1024.0 / 1024.0,
> + stream_bit_rate / 8.0 / 1024.0 / 1024.0);
> + dcc->streams_max_bit_rate = stream_bit_rate;
> + }
> + }
> + stream->refs++;
> + red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &stream_agent->destroy_item);
> + stream_agent_stats_print(stream_agent);
> + }
> + display->streams_size_total -= stream->width * stream->height;
> + ring_remove(&stream->link);
> + stream_unref(display, stream);
> +}
> +
> +static void stream_free(DisplayChannel *display, Stream *stream)
> +{
> + stream->next = display->free_streams;
> + display->free_streams = stream;
> +}
> +
> +void stream_init(DisplayChannel *display)
> +{
> + int i;
> +
> + ring_init(&display->streams);
> + display->free_streams = NULL;
> + for (i = 0; i < NUM_STREAMS; i++) {
> + Stream *stream = &display->streams_buf[i];
> + ring_item_init(&stream->link);
> + stream_free(display, stream);
> + }
> +}
> +
> +void stream_unref(DisplayChannel *display, Stream *stream)
> +{
> + if (--stream->refs != 0)
> + return;
> +
> + spice_warn_if_fail(!ring_item_is_linked(&stream->link));
> +
> + stream_free(display, stream);
> + display->stream_count--;
> +}
> +
> +void stream_agent_unref(DisplayChannel *display, StreamAgent *agent)
> +{
> + stream_unref(display, agent->stream);
> +}
> +
> StreamClipItem *stream_clip_item_new(DisplayChannelClient* dcc, StreamAgent *agent)
> {
> StreamClipItem *item = spice_new(StreamClipItem, 1);
> diff --git a/server/stream.h b/server/stream.h
> index f77fa96..4704937 100644
> --- a/server/stream.h
> +++ b/server/stream.h
> @@ -40,6 +40,9 @@
> #define RED_STREAM_DEFAULT_HIGH_START_BIT_RATE (10 * 1024 * 1024) // 10Mbps
> #define RED_STREAM_DEFAULT_LOW_START_BIT_RATE (2.5 * 1024 * 1024) // 2.5Mbps
>
> +/* move back to display_channel once struct private */
> +typedef struct DisplayChannel DisplayChannel;
> +
> typedef struct Stream Stream;
>
> typedef struct StreamActivateReportItem {
> @@ -132,6 +135,13 @@ struct Stream {
> uint32_t input_fps;
> };
>
> -void stream_agent_stats_print(StreamAgent *agent);
> +void stream_init (DisplayChannel *display);
> +void stream_stop (DisplayChannel *display,
> + Stream *stream);
> +void stream_unref (DisplayChannel *display,
> + Stream *stream);
> +void stream_agent_unref (DisplayChannel *display,
> + StreamAgent *agent);
> +void stream_agent_stats_print (StreamAgent *agent);
>
> #endif /* STREAM_H */
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
I've made a few comments on the first version:
http://lists.freedesktop.org/archives/spice-devel/2015-November/023445.html
The comments are not specifically for Jonathon, who just split the
patch, but for everyone with knowledge in this code.
The patch looks good, but the questions still stand.
More information about the Spice-devel
mailing list