Limit direct access to DisplayChannelClient

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.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

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)

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_ */

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>

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;
+}

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,10 @@
 
 #define MAX_PIPE_SIZE 50
 
+typedef struct DisplayChannel DisplayChannel;
+typedef struct Stream Stream;
+typedef struct StreamAgent StreamAgent;
+
 typedef struct WaitForChannels {
     SpiceMsgWaitForChannels header;
     SpiceWaitForChannel buf[MAX_CACHE_CLIENTS];
@@ -54,45 +57,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;
+typedef struct DisplayChannelClient DisplayChannelClient;
 
-    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;
-};
-
-#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 +161,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_ */

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");

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;

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;

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));
@@ -302,7 +302,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);
@@ -352,10 +352,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;
         }
 
@@ -363,7 +363,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;
@@ -375,9 +375,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) {
@@ -592,16 +592,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;
         }
@@ -609,7 +609,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)
@@ -638,7 +638,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,
@@ -646,7 +646,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;
         }
@@ -690,18 +690,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(reds),
                                         RED_CHANNEL_CLIENT(agent->dcc)->client,
-                                        agent->dcc->streams_max_latency);
+                                        dcc_get_max_stream_latency(agent->dcc));
 }
 
 static void bitmap_ref(gpointer data)
@@ -756,7 +756,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));
 
@@ -771,7 +771,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;
 
@@ -837,7 +837,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);
@@ -933,7 +933,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);

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_DETECTION_MAX_DELTA (NSEC_PER_SEC / 5)
 #define RED_STREAM_CONTINUOUS_MAX_DELTA NSEC_PER_SEC