[Spice-devel] [PATCH 07/14] Limit direct access to DisplayChannelClient

Pavel Grunt pgrunt at redhat.com
Thu May 5 12:28:48 UTC 2016


Hi Jonathon,

there are some unrelated changes which should go in separate patch.

On Tue, 2016-05-03 at 15:00 -0500, Jonathon Jongsma wrote:
> 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-encoders.c    | 11 +++---
>  server/dcc-encoders.h    |  8 +++--
>  server/dcc-private.h     | 87 ++++++++++++++++++++++++++++++++++++++++++++
>  server/dcc-send.c        |  2 +-
>  server/dcc.c             | 60 +++++++++++++++++++++++++++----
>  server/dcc.h             | 93 ++++++++++++-----------------------------------
> -
>  server/display-channel.c | 18 +++++-----
>  server/display-channel.h |  3 +-
>  server/image-cache.h     |  1 -
>  server/stream.c          | 54 ++++++++++++++--------------
>  server/stream.h          |  2 +-
>  12 files changed, 214 insertions(+), 126 deletions(-)
>  create mode 100644 server/dcc-private.h
> 
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 66b7f73..5f098cb 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -147,6 +147,7 @@ libserver_la_SOURCES =				\
>  	dcc-send.c					\
>  	dcc.h					\
>  	display-limits.h			\
> +	dcc-private.h				\
>  	dcc-encoders.c					\
>  	dcc-encoders.h					\
>  	$(NULL)
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index f1dd1bb..0433ebf 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -21,6 +21,7 @@
>  
>  #include <glib.h>
>  
> +#include "dcc-private.h"
>  #include "dcc-encoders.h"
>  #include "display-channel.h"
>  
> @@ -647,8 +648,8 @@ static GlzSharedDictionary
> *create_glz_dictionary(DisplayChannelClient *dcc,
>      return glz_shared_dictionary_new(RED_CHANNEL_CLIENT(dcc)->client, id,
> glz_dict);
>  }
>  
> -GlzSharedDictionary *dcc_get_glz_dictionary(DisplayChannelClient *dcc,
> -                                            uint8_t id, int window_size)
> +GlzSharedDictionary *get_glz_dictionary_for_dcc(DisplayChannelClient *dcc,
> +                                                uint8_t id, int window_size)
>  {
>      GlzSharedDictionary *shared_dict;
>  
> @@ -676,9 +677,9 @@ static GlzSharedDictionary
> *restore_glz_dictionary(DisplayChannelClient *dcc,
>      return glz_shared_dictionary_new(RED_CHANNEL_CLIENT(dcc)->client, id,
> glz_dict);
>  }
>  
> -GlzSharedDictionary *dcc_restore_glz_dictionary(DisplayChannelClient *dcc,
> -                                                uint8_t id,
> -                                                GlzEncDictRestoreData
> *restore_data)
> +GlzSharedDictionary *restore_glz_dictionary_for_dcc(DisplayChannelClient
> *dcc,
> +                                                    uint8_t id,
> +                                                    GlzEncDictRestoreData
> *restore_data)
>  {
>      GlzSharedDictionary *shared_dict = NULL;
>  
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 1cfc4f7..2c133ea 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -23,7 +23,7 @@
>  #include "common/quic.h"
>  #include "red-channel.h"
>  #include "red-parse-qxl.h"
> -#include "image-cache.h"
> +#include "dcc.h"
unrelated changes (although dcc.h should be included to bring the
DisplayChannelClient type)
>  #include "glz-encoder.h"
>  #include "jpeg-encoder.h"
>  #ifdef USE_LZ4
> @@ -34,6 +34,8 @@
>  typedef struct RedCompressBuf RedCompressBuf;
>  typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
>  typedef struct RedGlzDrawable RedGlzDrawable;
> +/* FIXME: remove */
> +typedef struct DisplayChannelClient DisplayChannelClient;

Do not introduce it, there are compile errors due to multiple typedefs.

>  
>  void             dcc_encoders_init                           (DisplayChannelC
> lient *dcc);
>  void             dcc_encoders_free                           (DisplayChannelC
> lient *dcc);
> @@ -74,9 +76,9 @@ typedef struct GlzSharedDictionary {
>      RedClient *client; // channel clients of the same client share the dict
>  } GlzSharedDictionary;
>  
> -GlzSharedDictionary*
> dcc_get_glz_dictionary                  (DisplayChannelClient *dcc,
> +GlzSharedDictionary*
> get_glz_dictionary_for_dcc              (DisplayChannelClient *dcc,
>                                                                uint8_t id, int
> window_size);
> -GlzSharedDictionary*
> dcc_restore_glz_dictionary              (DisplayChannelClient *dcc,
> +GlzSharedDictionary*
> restore_glz_dictionary_for_dcc          (DisplayChannelClient *dcc,
>                                                                uint8_t id,
>                                                                GlzEncDictResto
> reData *restore_data);
>  
> diff --git a/server/dcc-private.h b/server/dcc-private.h
> new file mode 100644
> index 0000000..6f6bbf9
> --- /dev/null
> +++ b/server/dcc-private.h
> @@ -0,0 +1,87 @@
> +/* -*- 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 "dcc-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;
> +    int jpeg_quality;
> +    int zlib_level;
> +
> +    QuicData quic_data;
> +    QuicContext *quic;
> +    LzData lz_data;
> +    LzContext  *lz;
> +    JpegData jpeg_data;
> +    JpegEncoderContext *jpeg;
> +#ifdef USE_LZ4
> +    Lz4Data lz4_data;
> +    Lz4EncoderContext *lz4;
> +#endif
> +    ZlibData zlib_data;
> +    ZlibEncoder *zlib;
> +
> +    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 {
> +        uint32_t stream_outbuf_size;
> +        uint8_t *stream_outbuf; // caution stream buffer is also used as
> compress bufs!!!
> +
> +        FreeList free_list;
> +        uint64_t pixmap_cache_items[MAX_DRAWABLE_PIXMAP_CACHE_ITEMS];
> +        int num_pixmap_cache_items;
> +    } send_data;
> +
> +    /* global lz encoding entities */
> +    GlzSharedDictionary *glz_dict;
> +    GlzEncoderContext   *glz;
> +    GlzData glz_data;
> +
> +    Ring glz_drawables;               // all the living lz drawable, ordered
> by encoding time
> +    Ring glz_drawables_inst_to_free;               // list of instances to be
> freed
> +    pthread_mutex_t glz_drawables_inst_to_free_lock;
> +
> +    uint8_t surface_client_created[NUM_SURFACES];
> +    QRegion surface_client_lossy_region[NUM_SURFACES];
> +
> +    StreamAgent stream_agents[NUM_STREAMS];
> +    int use_mjpeg_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 efbc454..d17413a 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -19,7 +19,7 @@
>  #include <config.h>
>  #endif
>  
> -#include "dcc.h"
I would keep the include
> +#include "dcc-private.h"
>  #include "display-channel.h"
>  
>  #include "common/marshaller.h"
> diff --git a/server/dcc.c b/server/dcc.c
> index 038d74d..1c7efda 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -19,7 +19,7 @@
>  #include <config.h>
>  #endif
>  
> -#include "dcc.h"
same here
> +#include "dcc-private.h"
>  #include "display-channel.h"
>  
>  #define DISPLAY_CLIENT_SHORT_TIMEOUT 15000000000ULL //nano
> @@ -1373,9 +1373,9 @@ static int dcc_handle_init(DisplayChannelClient *dcc,
> SpiceMsgcDisplayInit *init
>      ring_init(&dcc->glz_drawables);
>      ring_init(&dcc->glz_drawables_inst_to_free);
>      pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
> -    dcc->glz_dict = dcc_get_glz_dictionary(dcc,
> -                                           init->glz_dictionary_id,
> -                                           init->glz_dictionary_window_size);
> +    dcc->glz_dict = get_glz_dictionary_for_dcc(dcc,
> +                                               init->glz_dictionary_id,
> +                                               init-
> >glz_dictionary_window_size);
>      spice_return_val_if_fail(dcc->glz_dict, FALSE);
>  
>      return TRUE;
> @@ -1471,9 +1471,9 @@ static int
> dcc_handle_migrate_glz_dictionary(DisplayChannelClient *dcc,
>      ring_init(&dcc->glz_drawables);
>      ring_init(&dcc->glz_drawables_inst_to_free);
>      pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
> -    dcc->glz_dict = dcc_restore_glz_dictionary(dcc,
> -                                               migrate->glz_dict_id,
> -                                               &migrate->glz_dict_data);
> +    dcc->glz_dict = restore_glz_dictionary_for_dcc(dcc,
> +                                                   migrate->glz_dict_id,
> +                                                   &migrate->glz_dict_data);
>      return dcc->glz_dict != NULL;
>  }
>  
> @@ -1683,3 +1683,49 @@ void dcc_release_item(DisplayChannelClient *dcc,
> RedPipeItem *item, int item_pus
>      else
>          release_item_before_push(dcc, item);
>  }
> +
> +StreamAgent* dcc_get_stream_agent(DisplayChannelClient *dcc, int stream_id)
> +{
> +    return &dcc->stream_agents[stream_id];
> +}
> +
> +GlzSharedDictionary* dcc_get_glz_dictionary(DisplayChannelClient *dcc)
> +{
> +    return dcc->glz_dict;
> +}
> +
> +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_mjpeg_encoder_rate_control(DisplayChannelClient *dcc)
> +{
> +    return dcc->use_mjpeg_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 509a6c6..a56c4e2 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -18,11 +18,9 @@
>  #ifndef DCC_H_
>  # define DCC_H_
>  
> -#include "red-worker.h"
> +#include "image-cache.h"
>  #include "pixmap-cache.h"
> -#include "cache-item.h"
> -#include "dcc-encoders.h"
> -#include "stream.h"
> +#include "red-worker.h"
>  #include "display-limits.h"
>  
>  #define PALETTE_CACHE_HASH_SHIFT 8
> @@ -42,6 +40,12 @@
>  
>  #define MAX_PIPE_SIZE 50
>  
> +/* FIXME: remove */
> +typedef struct DisplayChannel DisplayChannel;
> +typedef struct Stream Stream;
> +typedef struct StreamAgent StreamAgent;
> +typedef struct GlzSharedDictionary GlzSharedDictionary;
> +
Duplicated typedefs

>  typedef struct WaitForChannels {
>      SpiceMsgWaitForChannels header;
>      SpiceWaitForChannel buf[MAX_CACHE_CLIENTS];
> @@ -54,72 +58,11 @@ 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;
> -    int jpeg_quality;
> -    int zlib_level;
> -
> -    QuicData quic_data;
> -    QuicContext *quic;
> -    LzData lz_data;
> -    LzContext  *lz;
> -    JpegData jpeg_data;
> -    JpegEncoderContext *jpeg;
> -#ifdef USE_LZ4
> -    Lz4Data lz4_data;
> -    Lz4EncoderContext *lz4;
> -#endif
> -    ZlibData zlib_data;
> -    ZlibEncoder *zlib;
> -
> -    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 {
> -        uint32_t stream_outbuf_size;
> -        uint8_t *stream_outbuf; // caution stream buffer is also used as
> compress bufs!!!
> -
> -        FreeList free_list;
> -        uint64_t pixmap_cache_items[MAX_DRAWABLE_PIXMAP_CACHE_ITEMS];
> -        int num_pixmap_cache_items;
> -    } send_data;
> -
> -    /* global lz encoding entities */
> -    GlzSharedDictionary *glz_dict;
> -    GlzEncoderContext   *glz;
> -    GlzData glz_data;
> -
> -    Ring glz_drawables;               // all the living lz drawable, ordered
> by encoding time
> -    Ring glz_drawables_inst_to_free;               // list of instances to be
> freed
> -    pthread_mutex_t glz_drawables_inst_to_free_lock;
> -
> -    uint8_t surface_client_created[NUM_SURFACES];
> -    QRegion surface_client_lossy_region[NUM_SURFACES];
> -
> -    StreamAgent stream_agents[NUM_STREAMS];
> -    int use_mjpeg_encoder_rate_control;
> -    uint32_t streams_max_latency;
> -    uint64_t streams_max_bit_rate;
> -    bool gl_draw_ongoing;
> -};
> -
> -#define DCC_TO_WORKER(dcc)                                              \
> -    (SPICE_CONTAINEROF((dcc)->common.base.channel, CommonGraphicsChannel,
> base)->worker)
> -#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)
> +typedef struct DisplayChannelClient DisplayChannelClient;
> +
> +#define DCC_TO_WORKER(dcc)
> ((RedWorker*)((CommonGraphicsChannel*)((RedChannelClient*)dcc)->channel)-
> >worker)
> +#define DCC_TO_DC(dcc) ((DisplayChannel*)((RedChannelClient*)dcc)->channel)
> +#define RCC_TO_DCC(rcc) ((DisplayChannelClient*)rcc)
>  
>  typedef struct RedSurfaceCreateItem {
>      SpiceMsgSurfaceCreate surface_create;
> @@ -231,4 +174,14 @@
> int                        dcc_compress_image                        (DisplayC
> ha
>                                                                        int
> can_lossy,
>                                                                        compres
> s_send_data_t* o_comp_data);
>  
> +StreamAgent
> *              dcc_get_stream_agent                      (DisplayChannelClient
> *dcc, int stream_id);
> +GlzSharedDictionary
> *      dcc_get_glz_dictionary                    (DisplayChannelClient *dcc);
> +spice_wan_compression_t    dcc_get_jpeg_state                        (Display
> ChannelClient *dcc);
> +spice_wan_compression_t    dcc_get_zlib_glz_state                    (Display
> ChannelClient *dcc);
> +gboolean                   dcc_use_mjpeg_encoder_rate_control        (Display
> ChannelClient *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 8c040d7..e5c239a 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -312,7 +312,7 @@ static void streams_update_visible_region(DisplayChannel
> *display, Drawable *dra
>          }
>  
>          FOREACH_DCC(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);
> @@ -1302,7 +1302,7 @@ void display_channel_free_some(DisplayChannel *display)
>      spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
>                  display->glz_drawable_count);
>      FOREACH_DCC(display, link, next, dcc) {
> -        GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> +        GlzSharedDictionary *glz_dict = dcc_get_glz_dictionary(dcc);

Before it was ok to have dcc == NULL, but now it would cause problems.

>  
>          if (glz_dict) {
>              // encoding using the dictionary is prevented since the following
> operations might
> @@ -1317,7 +1317,7 @@ void display_channel_free_some(DisplayChannel *display)
>      }
>  
>      FOREACH_DCC(display, link, next, dcc) {
> -        GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> +        GlzSharedDictionary *glz_dict = dcc_get_glz_dictionary(dcc);
>  
Same here

>          if (glz_dict) {
>              pthread_rwlock_unlock(&glz_dict->encode_lock);
> @@ -2128,16 +2128,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 3bb77cd..d87b222 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 "dcc-encoders.h"
>  
>  typedef struct DependItem {
>      Drawable *drawable;
> diff --git a/server/image-cache.h b/server/image-cache.h
> index 014a45e..43e3637 100644
> --- a/server/image-cache.h
> +++ b/server/image-cache.h
> @@ -26,7 +26,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 d467503..131e347 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -76,18 +76,18 @@ void stream_stop(DisplayChannel *display, Stream *stream)
>      FOREACH_DCC(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);
>          spice_assert(!red_pipe_item_is_linked(&stream_agent->destroy_item));
> -        if (stream_agent->mjpeg_encoder && dcc-
> >use_mjpeg_encoder_rate_control) {
> +        if (stream_agent->mjpeg_encoder &&
> dcc_use_mjpeg_encoder_rate_control(dcc)) {
>              uint64_t stream_bit_rate =
> mjpeg_encoder_get_bit_rate(stream_agent->mjpeg_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);
>              }
>          }
>          stream->refs++;
> @@ -284,7 +284,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);
> @@ -338,10 +338,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_mjpeg_encoder_rate_control &&
> -            !dcc->common.is_low_bandwidth) {
> +        if (!dcc_use_mjpeg_encoder_rate_control(dcc) &&
> +            !((CommonGraphicsChannelClient*)dcc)->is_low_bandwidth) {
>              continue;
>          }
>  
> @@ -349,7 +349,7 @@ static void before_reattach_stream(DisplayChannel
> *display,
>  #ifdef STREAM_STATS
>              agent->stats.num_drops_pipe++;
>  #endif
> -            if (dcc->use_mjpeg_encoder_rate_control) {
> +            if (dcc_use_mjpeg_encoder_rate_control(dcc)) {
>                  mjpeg_encoder_notify_server_frame_drop(agent->mjpeg_encoder);
>              } else {
>                  ++agent->drops;
> @@ -361,9 +361,9 @@ static void before_reattach_stream(DisplayChannel
> *display,
>      FOREACH_DCC(display, link, link_next, dcc) {
>          double drop_factor;
>  
> -        agent = &dcc->stream_agents[index];
> +        agent = dcc_get_stream_agent(dcc, index);
>  
> -        if (dcc->use_mjpeg_encoder_rate_control) {
> +        if (dcc_use_mjpeg_encoder_rate_control(dcc)) {
>              continue;
>          }
>          if (agent->frames / agent->fps < FPS_TEST_INTERVAL) {
> @@ -586,16 +586,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->mjpeg_encoder) {
>              continue;
>          }
> @@ -603,7 +603,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)
> @@ -632,7 +632,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_per_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,
> @@ -640,7 +640,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;
>          }
> @@ -684,23 +684,23 @@ 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(reds),
>                                          RED_CHANNEL_CLIENT(agent->dcc)-
> >client,
> -                                        agent->dcc->streams_max_latency);
> +                                        dcc_get_max_stream_latency(agent-
> >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));
>  
> @@ -716,7 +716,7 @@ void dcc_create_stream(DisplayChannelClient *dcc, Stream
> *stream)
>      agent->fps = MAX_FPS;
>      agent->dcc = dcc;
>  
> -    if (dcc->use_mjpeg_encoder_rate_control) {
> +    if (dcc_use_mjpeg_encoder_rate_control(dcc)) {
>          MJpegEncoderRateControlCbs mjpeg_cbs;
>          uint64_t initial_bit_rate;
>  
> @@ -780,7 +780,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);
> @@ -877,7 +877,7 @@ void stream_detach_behind(DisplayChannel *display, QRegion
> *region, Drawable *dr
>          item = ring_next(ring, item);
>  
>          FOREACH_DCC(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 bff7ca7..29071f5 100644
> --- a/server/stream.h
> +++ b/server/stream.h
> @@ -23,7 +23,7 @@
>  #include "mjpeg-encoder.h"
>  #include "common/region.h"
>  #include "red-channel.h"
> -#include "image-cache.h"
> +#include "dcc.h"

unrelated change

Without the unrelated changes the patch is good.

Reviwed-by: Pavel Grunt <pgrunt at redhat.com>

Pavel



More information about the Spice-devel mailing list