[Spice-devel] [PATCH 1/9] Limit direct access to DisplayChannelClient

Frediano Ziglio fziglio at redhat.com
Mon Aug 8 06:54:43 UTC 2016


> 
> On Fri, 2016-08-05 at 07:51 -0400, Frediano Ziglio wrote:
> > > 
> > > 
> > > From: Frediano Ziglio <fziglio at redhat.com>
> > > 
> > 
> > I didn't remember to wrote this patch.
> > From https://cgit.freedesktop.org/~fziglio/spice-server/commit/?h=ref
> > actory&id=c211e0292e0fbf8754a185f8724d42768e4cdab8
> > the original author is 	Jonathon Jongsma so should be changed
> > back and possibly
> > added 2 signed off (me and Jonathon)
> > 
> > > 
> > > Add a few more methods and accessors so that other files don't need
> > > to
> > > manipulate the struct members directly. Move the struct definition
> > > to a
> > > private header which only the dcc-* files will include.
> > > ---
> > >  server/Makefile.am       |  1 +
> > >  server/dcc-private.h     | 62
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > >  server/dcc-send.c        |  2 +-
> > >  server/dcc.c             | 47 ++++++++++++++++++++++++++++++++++-
> > >  server/dcc.h             | 64
> > >  +++++++++++++++++-------------------------------
> > >  server/display-channel.c | 28 ++++++++++++---------
> > >  server/display-channel.h |  3 +--
> > >  server/image-cache.h     |  1 -
> > >  server/stream.c          | 56 +++++++++++++++++++++---------------
> > > ------
> > >  server/stream.h          |  2 +-
> > >  10 files changed, 178 insertions(+), 88 deletions(-)
> > >  create mode 100644 server/dcc-private.h
> > > 
> > > diff --git a/server/Makefile.am b/server/Makefile.am
> > > index 921b082..24e6e21 100644
> > > --- a/server/Makefile.am
> > > +++ b/server/Makefile.am
> > > @@ -148,6 +148,7 @@ libserver_la_SOURCES =
> > > \
> > >  	dcc-send.c					\
> > >  	dcc.h					\
> > >  	display-limits.h			\
> > > +	dcc-private.h				\
> > >  	image-encoders.c					\
> > >  	image-encoders.h					\
> > >  	$(NULL)
> > > diff --git a/server/dcc-private.h b/server/dcc-private.h
> > > new file mode 100644
> > > index 0000000..6e11163
> > > --- /dev/null
> > > +++ b/server/dcc-private.h
> > > @@ -0,0 +1,62 @@
> > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > +/*
> > > +   Copyright (C) 2009-2015 Red Hat, Inc.
> > > +
> > > +   This library is free software; you can redistribute it and/or
> > > +   modify it under the terms of the GNU Lesser General Public
> > > +   License as published by the Free Software Foundation; either
> > > +   version 2.1 of the License, or (at your option) any later
> > > version.
> > > +
> > > +   This library is distributed in the hope that it will be useful,
> > > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > GNU
> > > +   Lesser General Public License for more details.
> > > +
> > > +   You should have received a copy of the GNU Lesser General
> > > Public
> > > +   License along with this library; if not, see
> > > <http://www.gnu.org/licenses/>.
> > > +*/
> > > +#ifndef DCC_PRIVATE_H_
> > > +#define DCC_PRIVATE_H_
> > > +
> > > +#include "cache-item.h"
> > > +#include "dcc.h"
> > > +#include "image-encoders.h"
> > > +#include "stream.h"
> > > +
> > > +struct DisplayChannelClient {
> > > +    CommonGraphicsChannelClient common;
> > > +    uint32_t id;
> > > +    SpiceImageCompression image_compression;
> > > +    spice_wan_compression_t jpeg_state;
> > > +    spice_wan_compression_t zlib_glz_state;
> > > +
> > > +    ImageEncoders encoders;
> > > +
> > > +    int expect_init;
> > > +
> > > +    PixmapCache *pixmap_cache;
> > > +    uint32_t pixmap_cache_generation;
> > > +    int pending_pixmaps_sync;
> > > +
> > > +    RedCacheItem *palette_cache[PALETTE_CACHE_HASH_SIZE];
> > > +    Ring palette_cache_lru;
> > > +    long palette_cache_available;
> > > +    uint32_t palette_cache_items;
> > > +
> > > +    struct {
> > > +        FreeList free_list;
> > > +        uint64_t
> > > pixmap_cache_items[MAX_DRAWABLE_PIXMAP_CACHE_ITEMS];
> > > +        int num_pixmap_cache_items;
> > > +    } send_data;
> > > +
> > > +    uint8_t surface_client_created[NUM_SURFACES];
> > > +    QRegion surface_client_lossy_region[NUM_SURFACES];
> > > +
> > > +    StreamAgent stream_agents[NUM_STREAMS];
> > > +    int use_video_encoder_rate_control;
> > > +    uint32_t streams_max_latency;
> > > +    uint64_t streams_max_bit_rate;
> > > +    bool gl_draw_ongoing;
> > > +};
> > > +
> > > +#endif /* DCC_PRIVATE_H_ */
> > > diff --git a/server/dcc-send.c b/server/dcc-send.c
> > > index 56bbf23..395129f 100644
> > > --- a/server/dcc-send.c
> > > +++ b/server/dcc-send.c
> > > @@ -19,7 +19,7 @@
> > >  #include <config.h>
> > >  #endif
> > >  
> > > -#include "dcc.h"
> > > +#include "dcc-private.h"
> > >  #include "display-channel.h"
> > >  
> > >  #include <common/marshaller.h>
> > > diff --git a/server/dcc.c b/server/dcc.c
> > > index 45c3ca8..c1c30c6 100644
> > > --- a/server/dcc.c
> > > +++ b/server/dcc.c
> > > @@ -19,7 +19,7 @@
> > >  #include <config.h>
> > >  #endif
> > >  
> > > -#include "dcc.h"
> > > +#include "dcc-private.h"
> > >  #include "display-channel.h"
> > >  
> > >  #define DISPLAY_CLIENT_SHORT_TIMEOUT 15000000000ULL //nano
> > > @@ -1129,3 +1129,48 @@ int
> > > dcc_handle_migrate_data(DisplayChannelClient *dcc,
> > > uint32_t size, void *mess
> > >      red_channel_client_ack_zero_messages_window(RED_CHANNEL_CLIENT
> > > (dcc));
> > >      return TRUE;
> > >  }
> > > +
> > > +StreamAgent* dcc_get_stream_agent(DisplayChannelClient *dcc, int
> > > stream_id)
> > > +{
> > > +    return &dcc->stream_agents[stream_id];
> > > +}
> > > +
> > > +ImageEncoders* dcc_get_encoders(DisplayChannelClient *dcc)
> > > +{
> > > +    return &dcc->encoders;
> > > +}
> > > +
> > > +spice_wan_compression_t dcc_get_jpeg_state(DisplayChannelClient
> > > *dcc)
> > > +{
> > > +    return dcc->jpeg_state;
> > > +}
> > > +
> > > +spice_wan_compression_t
> > > dcc_get_zlib_glz_state(DisplayChannelClient *dcc)
> > > +{
> > > +    return dcc->zlib_glz_state;
> > > +}
> > > +
> > > +gboolean dcc_use_video_encoder_rate_control(DisplayChannelClient
> > > *dcc)
> > > +{
> > > +    return dcc->use_video_encoder_rate_control;
> > > +}
> > > +
> > > +uint32_t dcc_get_max_stream_latency(DisplayChannelClient *dcc)
> > > +{
> > > +    return dcc->streams_max_latency;
> > > +}
> > > +
> > > +void dcc_set_max_stream_latency(DisplayChannelClient *dcc,
> > > uint32_t latency)
> > > +{
> > > +    dcc->streams_max_latency = latency;
> > > +}
> > > +
> > > +uint64_t dcc_get_max_stream_bit_rate(DisplayChannelClient *dcc)
> > > +{
> > > +    return dcc->streams_max_bit_rate;
> > > +}
> > > +
> > > +void dcc_set_max_stream_bit_rate(DisplayChannelClient *dcc,
> > > uint64_t rate)
> > > +{
> > > +    dcc->streams_max_bit_rate = rate;
> > > +}
> > > diff --git a/server/dcc.h b/server/dcc.h
> > > index 5f7b16f..4d9dccb 100644
> > > --- a/server/dcc.h
> > > +++ b/server/dcc.h
> > > @@ -18,11 +18,10 @@
> > >  #ifndef DCC_H_
> > >  # define DCC_H_
> > >  
> > > -#include "red-worker.h"
> > > -#include "pixmap-cache.h"
> > > -#include "cache-item.h"
> > >  #include "image-encoders.h"
> > > -#include "stream.h"
> > > +#include "image-cache.h"
> > > +#include "pixmap-cache.h"
> > > +#include "red-worker.h"
> > >  #include "display-limits.h"
> > >  
> > >  #define PALETTE_CACHE_HASH_SHIFT 8
> > > @@ -42,6 +41,12 @@
> > >  
> > >  #define MAX_PIPE_SIZE 50
> > >  
> > > +/* FIXME: remove */
> > > +typedef struct DisplayChannel DisplayChannel;
> > > +typedef struct Stream Stream;
> > > +typedef struct StreamAgent StreamAgent;
> > > +typedef struct GlzSharedDictionary GlzSharedDictionary;
> > > +
> > 
> > What can I say... let it be!
> 
> What do you mean by this? Remove the FIXME comment?
> 
> It looks like the forward-declaration for GlzSharedDictionary is not
> actually needed here; it still compiles when I comment it out.
> 

Yes, GlzSharedDictionary should be removed, it's now better encapsulated.
I accept the code, there were lot of proposal but no final decision
so I'll go for this code.

> 
> > 
> > > 
> > >  typedef struct WaitForChannels {
> > >      SpiceMsgWaitForChannels header;
> > >      SpiceWaitForChannel buf[MAX_CACHE_CLIENTS];
> > > @@ -54,45 +59,10 @@ typedef struct FreeList {
> > >      WaitForChannels wait;
> > >  } FreeList;
> > >  
> > > -struct DisplayChannelClient {
> > > -    CommonGraphicsChannelClient common;
> > > -    uint32_t id;
> > > -    SpiceImageCompression image_compression;
> > > -    spice_wan_compression_t jpeg_state;
> > > -    spice_wan_compression_t zlib_glz_state;
> > > -
> > > -    ImageEncoders encoders;
> > > -
> > > -    int expect_init;
> > > +typedef struct DisplayChannelClient DisplayChannelClient;
> > >  
> > > -    PixmapCache *pixmap_cache;
> > > -    uint32_t pixmap_cache_generation;
> > > -    int pending_pixmaps_sync;
> > > -
> > > -    RedCacheItem *palette_cache[PALETTE_CACHE_HASH_SIZE];
> > > -    Ring palette_cache_lru;
> > > -    long palette_cache_available;
> > > -    uint32_t palette_cache_items;
> > > -
> > > -    struct {
> > > -        FreeList free_list;
> > > -        uint64_t
> > > pixmap_cache_items[MAX_DRAWABLE_PIXMAP_CACHE_ITEMS];
> > > -        int num_pixmap_cache_items;
> > > -    } send_data;
> > > -
> > > -    uint8_t surface_client_created[NUM_SURFACES];
> > > -    QRegion surface_client_lossy_region[NUM_SURFACES];
> > > -
> > > -    StreamAgent stream_agents[NUM_STREAMS];
> > > -    int use_video_encoder_rate_control;
> > > -    uint32_t streams_max_latency;
> > > -    uint64_t streams_max_bit_rate;
> > > -    bool gl_draw_ongoing;
> > > -};
> > > -
> > > -#define
> > > DCC_TO_DC(dcc)                                                  \
> > > -     SPICE_CONTAINEROF((dcc)->common.base.channel, DisplayChannel,
> > > common.base)
> > > -#define RCC_TO_DCC(rcc) SPICE_CONTAINEROF((rcc),
> > > DisplayChannelClient,
> > > common.base)
> > > +#define DCC_TO_DC(dcc) ((DisplayChannel*)((RedChannelClient*)dcc)-
> > > >channel)
> > > +#define RCC_TO_DCC(rcc) ((DisplayChannelClient*)rcc)
> > >  
> > >  typedef struct RedSurfaceCreateItem {
> > >      RedPipeItem pipe_item;
> > > @@ -193,4 +163,14 @@ int                        dcc_compress_image
> > > (DisplayCha
> > >                                                                    
> > >     int
> > >                                                                    
> > >     can_lossy,
> > >                                                                    
> > >     compress_send_data_t*
> > >                                                                    
> > >     o_comp_data);
> > >  
> > > +StreamAgent *              dcc_get_stream_agent
> > > (DisplayChannelClient *dcc, int stream_id);
> > > +ImageEncoders *dcc_get_encoders(DisplayChannelClient *dcc);
> > > +spice_wan_compression_t    dcc_get_jpeg_state
> > > (DisplayChannelClient *dcc);
> > > +spice_wan_compression_t    dcc_get_zlib_glz_state
> > > (DisplayChannelClient *dcc);
> > > +gboolean                   dcc_use_video_encoder_rate_control
> > > (DisplayChannelClient *dcc);
> > > +uint32_t dcc_get_max_stream_latency(DisplayChannelClient *dcc);
> > > +void dcc_set_max_stream_latency(DisplayChannelClient *dcc,
> > > uint32_t
> > > latency);
> > > +uint64_t dcc_get_max_stream_bit_rate(DisplayChannelClient *dcc);
> > > +void dcc_set_max_stream_bit_rate(DisplayChannelClient *dcc,
> > > uint64_t rate);
> > > +
> > >  #endif /* DCC_H_ */
> > > diff --git a/server/display-channel.c b/server/display-channel.c
> > > index 073d45e..89bb0c6 100644
> > > --- a/server/display-channel.c
> > > +++ b/server/display-channel.c
> > > @@ -230,7 +230,7 @@ static void
> > > streams_update_visible_region(DisplayChannel
> > > *display, Drawable *dra
> > >          }
> > >  
> > >          FOREACH_CLIENT(display, link, next, dcc) {
> > > -            agent = &dcc->stream_agents[get_stream_id(display,
> > > stream)];
> > > +            agent = dcc_get_stream_agent(dcc,
> > > 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);
> > > @@ -1151,7 +1151,7 @@ void
> > > display_channel_free_glz_drawables_to_free(DisplayChannel *display)
> > >      spice_return_if_fail(display);
> > >  
> > >      FOREACH_CLIENT(display, link, next, dcc) {
> > > -        image_encoders_free_glz_drawables_to_free(&dcc->encoders);
> > > +        image_encoders_free_glz_drawables_to_free(dcc_get_encoders
> > > (dcc));
> > >      }
> > >  }
> > >  
> > > @@ -1163,7 +1163,7 @@ void
> > > display_channel_free_glz_drawables(DisplayChannel
> > > *display)
> > >      spice_return_if_fail(display);
> > >  
> > >      FOREACH_CLIENT(display, link, next, dcc) {
> > > -        image_encoders_free_glz_drawables(&dcc->encoders);
> > > +        image_encoders_free_glz_drawables(dcc_get_encoders(dcc));
> > >      }
> > >  }
> > >  
> > > @@ -1206,10 +1206,12 @@ void
> > > display_channel_free_some(DisplayChannel
> > > *display)
> > >      spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
> > >                  display->encoder_shared_data.glz_drawable_count);
> > >      FOREACH_CLIENT(display, link, next, dcc) {
> > > +        ImageEncoders *encoders = dcc_get_encoders(dcc);
> > > +
> > >          // encoding using the dictionary is prevented since the
> > > following
> > >          operations might
> > >          // change the dictionary
> > > -        if (image_encoders_glz_encode_lock(&dcc->encoders)) {
> > > -            n =
> > > image_encoders_free_some_independent_glz_drawables(&dcc->encoders);
> > > +        if (image_encoders_glz_encode_lock(encoders)) {
> > > +            n =
> > > image_encoders_free_some_independent_glz_drawables(encoders);
> > >          }
> > >      }
> > >  
> > > @@ -1218,7 +1220,9 @@ void display_channel_free_some(DisplayChannel
> > > *display)
> > >      }
> > >  
> > >      FOREACH_CLIENT(display, link, next, dcc) {
> > > -        image_encoders_glz_encode_unlock(&dcc->encoders);
> > > +        ImageEncoders *encoders = dcc_get_encoders(dcc);
> > > +
> > > +        image_encoders_glz_encode_unlock(encoders);
> > >      }
> > >  }
> > >  
> > > @@ -1987,16 +1991,16 @@ void
> > > display_channel_process_surface_cmd(DisplayChannel *display,
> > > RedSurfaceCmd
> > >  
> > >  void display_channel_update_compression(DisplayChannel *display,
> > >  DisplayChannelClient *dcc)
> > >  {
> > > -    if (dcc->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
> > > -        display->enable_jpeg = dcc->common.is_low_bandwidth;
> > > +    if (dcc_get_jpeg_state(dcc) == SPICE_WAN_COMPRESSION_AUTO) {
> > > +        display->enable_jpeg =
> > > ((CommonGraphicsChannelClient*)dcc)->is_low_bandwidth;
> > >      } else {
> > > -        display->enable_jpeg = (dcc->jpeg_state ==
> > > SPICE_WAN_COMPRESSION_ALWAYS);
> > > +        display->enable_jpeg = (dcc_get_jpeg_state(dcc) ==
> > > SPICE_WAN_COMPRESSION_ALWAYS);
> > >      }
> > >  
> > > -    if (dcc->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
> > > -        display->enable_zlib_glz_wrap = dcc-
> > > >common.is_low_bandwidth;
> > > +    if (dcc_get_zlib_glz_state(dcc) == SPICE_WAN_COMPRESSION_AUTO)
> > > {
> > > +        display->enable_zlib_glz_wrap =
> > > ((CommonGraphicsChannelClient*)dcc)->is_low_bandwidth;
> > >      } else {
> > > -        display->enable_zlib_glz_wrap = (dcc->zlib_glz_state ==
> > > SPICE_WAN_COMPRESSION_ALWAYS);
> > > +        display->enable_zlib_glz_wrap =
> > > (dcc_get_zlib_glz_state(dcc) ==
> > > SPICE_WAN_COMPRESSION_ALWAYS);
> > >      }
> > >      spice_info("jpeg %s", display->enable_jpeg ? "enabled" :
> > > "disabled");
> > >      spice_info("zlib-over-glz %s", display->enable_zlib_glz_wrap ?
> > > "enabled"
> > >      : "disabled");
> > > diff --git a/server/display-channel.h b/server/display-channel.h
> > > index f090d99..f133c62 100644
> > > --- a/server/display-channel.h
> > > +++ b/server/display-channel.h
> > > @@ -44,8 +44,7 @@
> > >  #include "tree.h"
> > >  #include "stream.h"
> > >  #include "dcc.h"
> > > -#include "display-limits.h"
> > > -
> > > +#include "image-encoders.h"
> > >  
> > >  typedef struct DependItem {
> > >      Drawable *drawable;
> > > diff --git a/server/image-cache.h b/server/image-cache.h
> > > index b90a3b3..d86c8b7 100644
> > > --- a/server/image-cache.h
> > > +++ b/server/image-cache.h
> > > @@ -25,7 +25,6 @@
> > >  
> > >  /* FIXME: move back to display-channel.h (once structs are
> > > private) */
> > >  typedef struct Drawable Drawable;
> > > -typedef struct DisplayChannelClient DisplayChannelClient;
> > >  
> > >  typedef struct ImageCacheItem {
> > >      RingItem lru_link;
> > > diff --git a/server/stream.c b/server/stream.c
> > > index 985bc71..9e7fa3a 100644
> > > --- a/server/stream.c
> > > +++ b/server/stream.c
> > > @@ -66,7 +66,7 @@ static void stream_agent_stats_print(StreamAgent
> > > *agent)
> > >  static void stream_create_destroy_item_release(RedPipeItem *base)
> > >  {
> > >      StreamCreateDestroyItem *item =
> > > SPICE_UPCAST(StreamCreateDestroyItem,
> > >      base);
> > > -    DisplayChannel *display =
> > > (DisplayChannel*)item->agent->dcc->common.base.channel;
> > > +    DisplayChannel *display = DCC_TO_DC(item->agent->dcc);
> > >      stream_agent_unref(display, item->agent);
> > >      free(item);
> > >  }
> > > @@ -105,17 +105,17 @@ void stream_stop(DisplayChannel *display,
> > > Stream
> > > *stream)
> > >      FOREACH_CLIENT(display, link, next, dcc) {
> > >          StreamAgent *stream_agent;
> > >  
> > > -        stream_agent = &dcc->stream_agents[get_stream_id(display,
> > > stream)];
> > > +        stream_agent = dcc_get_stream_agent(dcc,
> > > 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) {
> > > +        if (stream_agent->video_encoder &&
> > > dcc_use_video_encoder_rate_control(dcc)) {
> > >              uint64_t stream_bit_rate =
> > >              stream_agent->video_encoder-
> > > >get_bit_rate(stream_agent->video_encoder);
> > >  
> > > -            if (stream_bit_rate > dcc->streams_max_bit_rate) {
> > > +            if (stream_bit_rate >
> > > dcc_get_max_stream_bit_rate(dcc)) {
> > >                  spice_debug("old max-bit-rate=%.2f new=%.2f",
> > > -                            dcc->streams_max_bit_rate / 8.0 /
> > > 1024.0 /
> > > 1024.0,
> > > +                            dcc_get_max_stream_bit_rate(dcc) / 8.0
> > > / 1024.0
> > > / 1024.0,
> > >                              stream_bit_rate / 8.0 / 1024.0 /
> > > 1024.0);
> > > -                dcc->streams_max_bit_rate = stream_bit_rate;
> > > +                dcc_set_max_stream_bit_rate(dcc, stream_bit_rate);
> > >              }
> > >          }
> > >          red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc),
> > >          stream_destroy_item_new(stream_agent));
> > > @@ -303,7 +303,7 @@ static void attach_stream(DisplayChannel
> > > *display,
> > > Drawable *drawable, Stream *s
> > >          StreamAgent *agent;
> > >          QRegion clip_in_draw_dest;
> > >  
> > > -        agent = &dcc->stream_agents[get_stream_id(display,
> > > stream)];
> > > +        agent = dcc_get_stream_agent(dcc, get_stream_id(display,
> > > stream));
> > >          region_or(&agent->vis_region, &drawable-
> > > >tree_item.base.rgn);
> > >  
> > >          region_init(&clip_in_draw_dest);
> > > @@ -353,10 +353,10 @@ static void
> > > before_reattach_stream(DisplayChannel
> > > *display,
> > >      index = get_stream_id(display, stream);
> > >      DRAWABLE_FOREACH_DPI_SAFE(stream->current, ring_item, next,
> > > dpi) {
> > >          dcc = dpi->dcc;
> > > -        agent = &dcc->stream_agents[index];
> > > +        agent = dcc_get_stream_agent(dcc, index);
> > >  
> > > -        if (!dcc->use_video_encoder_rate_control &&
> > > -            !dcc->common.is_low_bandwidth) {
> > > +        if (!dcc_use_video_encoder_rate_control(dcc) &&
> > > +            !((CommonGraphicsChannelClient*)dcc)-
> > > >is_low_bandwidth) {
> > >              continue;
> > >          }
> > >  
> > > @@ -364,7 +364,7 @@ static void
> > > before_reattach_stream(DisplayChannel
> > > *display,
> > >  #ifdef STREAM_STATS
> > >              agent->stats.num_drops_pipe++;
> > >  #endif
> > > -            if (dcc->use_video_encoder_rate_control) {
> > > +            if (dcc_use_video_encoder_rate_control(dcc)) {
> > >                  agent->video_encoder-
> > > >notify_server_frame_drop(agent->video_encoder);
> > >              } else {
> > >                  ++agent->drops;
> > > @@ -376,9 +376,9 @@ static void
> > > before_reattach_stream(DisplayChannel
> > > *display,
> > >      FOREACH_CLIENT(display, link, link_next, dcc) {
> > >          double drop_factor;
> > >  
> > > -        agent = &dcc->stream_agents[index];
> > > +        agent = dcc_get_stream_agent(dcc, index);
> > >  
> > > -        if (dcc->use_video_encoder_rate_control) {
> > > +        if (dcc_use_video_encoder_rate_control(dcc)) {
> > >              continue;
> > >          }
> > >          if (agent->frames / agent->fps < FPS_TEST_INTERVAL) {
> > > @@ -593,16 +593,16 @@ static void
> > > dcc_update_streams_max_latency(DisplayChannelClient *dcc,
> > > StreamAgen
> > >      uint32_t new_max_latency = 0;
> > >      int i;
> > >  
> > > -    if (dcc->streams_max_latency != remove_agent-
> > > >client_required_latency) {
> > > +    if (dcc_get_max_stream_latency(dcc) !=
> > > remove_agent->client_required_latency) {
> > >          return;
> > >      }
> > >  
> > > -    dcc->streams_max_latency = 0;
> > > +    dcc_set_max_stream_latency(dcc, 0);
> > >      if (DCC_TO_DC(dcc)->stream_count == 1) {
> > >          return;
> > >      }
> > >      for (i = 0; i < NUM_STREAMS; i++) {
> > > -        StreamAgent *other_agent = &dcc->stream_agents[i];
> > > +        StreamAgent *other_agent = dcc_get_stream_agent(dcc, i);
> > >          if (other_agent == remove_agent || !other_agent-
> > > >video_encoder) {
> > >              continue;
> > >          }
> > > @@ -610,7 +610,7 @@ static void
> > > dcc_update_streams_max_latency(DisplayChannelClient *dcc,
> > > StreamAgen
> > >              new_max_latency = other_agent-
> > > >client_required_latency;
> > >          }
> > >      }
> > > -    dcc->streams_max_latency = new_max_latency;
> > > +    dcc_set_max_stream_latency(dcc, new_max_latency);
> > >  }
> > >  
> > >  static uint64_t get_initial_bit_rate(DisplayChannelClient *dcc,
> > > Stream
> > >  *stream)
> > > @@ -639,7 +639,7 @@ static uint64_t
> > > get_initial_bit_rate(DisplayChannelClient
> > > *dcc, Stream *stream)
> > >          net_test_bit_rate =
> > >          main_channel_client_is_network_info_initialized(mcc) ?
> > >                                  main_channel_client_get_bitrate_pe
> > > r_sec(mcc)
> > >                                  :
> > >                                  0;
> > > -        bit_rate = MAX(dcc->streams_max_bit_rate,
> > > net_test_bit_rate);
> > > +        bit_rate = MAX(dcc_get_max_stream_bit_rate(dcc),
> > > net_test_bit_rate);
> > >          if (bit_rate == 0) {
> > >              /*
> > >               * In case we are after a spice session migration,
> > > @@ -647,7 +647,7 @@ static uint64_t
> > > get_initial_bit_rate(DisplayChannelClient
> > > *dcc, Stream *stream)
> > >               * If the network info is not initialized due to
> > > another reason,
> > >               * the low_bandwidth flag is FALSE.
> > >               */
> > > -            bit_rate = dcc->common.is_low_bandwidth ?
> > > +            bit_rate = ((CommonGraphicsChannelClient*)dcc)-
> > > >is_low_bandwidth
> > > ?
> > >                  RED_STREAM_DEFAULT_LOW_START_BIT_RATE :
> > >                  RED_STREAM_DEFAULT_HIGH_START_BIT_RATE;
> > >          }
> > > @@ -691,18 +691,18 @@ static void update_client_playback_delay(void
> > > *opaque,
> > > uint32_t delay_ms)
> > >  {
> > >      StreamAgent *agent = opaque;
> > >      DisplayChannelClient *dcc = agent->dcc;
> > > -    RedsState *reds = red_channel_get_server(dcc-
> > > >common.base.channel);
> > > +    RedsState *reds =
> > > red_channel_get_server(((RedChannelClient*)dcc)->channel);
> > >  
> > >      dcc_update_streams_max_latency(dcc, agent);
> > >  
> > >      agent->client_required_latency = delay_ms;
> > > -    if (delay_ms > agent->dcc->streams_max_latency) {
> > > -        agent->dcc->streams_max_latency = delay_ms;
> > > +    if (delay_ms > dcc_get_max_stream_latency(agent->dcc)) {
> > > +        dcc_set_max_stream_latency(agent->dcc, delay_ms);
> > >      }
> > > -    spice_debug("resetting client latency: %u",
> > > agent->dcc->streams_max_latency);
> > > +    spice_debug("resetting client latency: %u",
> > > dcc_get_max_stream_latency(agent->dcc));
> > >      main_dispatcher_set_mm_time_latency(reds_get_main_dispatcher(r
> > > eds),
> > >                                          RED_CHANNEL_CLIENT(agent-
> > > >dcc)->client,
> > > -                                        agent->dcc-
> > > >streams_max_latency);
> > > +
> > > dcc_get_max_stream_latency(agent->dcc));
> > >  }
> > >  
> > >  static void bitmap_ref(gpointer data)
> > > @@ -757,7 +757,7 @@ static VideoEncoder*
> > > dcc_create_video_encoder(DisplayChannelClient *dcc,
> > >  
> > >  void dcc_create_stream(DisplayChannelClient *dcc, Stream *stream)
> > >  {
> > > -    StreamAgent *agent = &dcc-
> > > >stream_agents[get_stream_id(DCC_TO_DC(dcc),
> > > stream)];
> > > +    StreamAgent *agent = dcc_get_stream_agent(dcc,
> > > get_stream_id(DCC_TO_DC(dcc), stream));
> > >  
> > >      spice_return_if_fail(region_is_empty(&agent->vis_region));
> > >  
> > > @@ -772,7 +772,7 @@ void dcc_create_stream(DisplayChannelClient
> > > *dcc, Stream
> > > *stream)
> > >      agent->fps = MAX_FPS;
> > >      agent->dcc = dcc;
> > >  
> > > -    if (dcc->use_video_encoder_rate_control) {
> > > +    if (dcc_use_video_encoder_rate_control(dcc)) {
> > >          VideoEncoderRateControlCbs video_cbs;
> > >          uint64_t initial_bit_rate;
> > >  
> > > @@ -838,7 +838,7 @@ static void
> > > dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
> > >  {
> > >      DisplayChannel *display = DCC_TO_DC(dcc);
> > >      int stream_id = get_stream_id(display, stream);
> > > -    StreamAgent *agent = &dcc->stream_agents[stream_id];
> > > +    StreamAgent *agent = dcc_get_stream_agent(dcc, stream_id);
> > >  
> > >      /* stopping the client from playing older frames at once*/
> > >      region_clear(&agent->clip);
> > > @@ -934,7 +934,7 @@ void stream_detach_behind(DisplayChannel
> > > *display,
> > > QRegion *region, Drawable *dr
> > >          item = ring_next(ring, item);
> > >  
> > >          FOREACH_CLIENT(display, link, next, dcc) {
> > > -            StreamAgent *agent = &dcc-
> > > >stream_agents[get_stream_id(display,
> > > stream)];
> > > +            StreamAgent *agent = dcc_get_stream_agent(dcc,
> > > get_stream_id(display, stream));
> > >  
> > >              if (region_intersects(&agent->vis_region, region)) {
> > >                  dcc_detach_stream_gracefully(dcc, stream,
> > > drawable);
> > > diff --git a/server/stream.h b/server/stream.h
> > > index b4fe540..26fb9a9 100644
> > > --- a/server/stream.h
> > > +++ b/server/stream.h
> > > @@ -24,7 +24,7 @@
> > >  #include "utils.h"
> > >  #include "video-encoder.h"
> > >  #include "red-channel.h"
> > > -#include "image-cache.h"
> > > +#include "dcc.h"
> > >  
> > >  #define RED_STREAM_DETACTION_MAX_DELTA (NSEC_PER_SEC / 5)
> > >  #define RED_STREAM_CONTINUS_MAX_DELTA NSEC_PER_SEC
> > 
> > I remember I fixed these typos ...
> > Yes, in 777a4c4987cd657c7d0f463e615905e6263b2a2b (3 days ago).
> > 
> > Frediano
> 

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano


More information about the Spice-devel mailing list