> Subject: [PATCH spice-server v2 7/8] Convert RedChannel heirarchy to GObject
>
Small spell, it's "hierarchy".
> From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>
> FIXME: this commit appears to introduce a vdagent-related crash in
> vdi_port_read_one_msg_from_device(). sin->st is NULL.
If this is still true I would fix it.
> ---
> server/Makefile.am | 2 +
> server/common-graphics-channel.c | 109 ++++--
> server/common-graphics-channel.h | 39 ++-
> server/cursor-channel.c | 80 +++--
> server/cursor-channel.h | 31 +-
> server/dcc-send.c | 11 +-
> server/dcc.c | 1 +
> server/dcc.h | 2 +-
> server/display-channel-private.h | 76 +++++
> server/display-channel.c | 261 +++++++++++----
> server/display-channel.h | 127 +++----
> server/dummy-channel-client.c | 17 +-
> server/dummy-channel.c | 58 ++++
> server/dummy-channel.h | 61 ++++
> server/inputs-channel.c | 263 +++++++++------
> server/inputs-channel.h | 30 +-
> server/main-channel-client.c | 65 ++--
> server/main-channel-client.h | 5 +-
> server/main-channel.c | 196 +++++++----
> server/main-channel.h | 44 ++-
> server/red-channel-client-private.h | 19 ++
> server/red-channel-client.c | 179 ++++++----
> server/red-channel-client.h | 6 +-
> server/red-channel.c | 645
> ++++++++++++++++++++++++------------
> server/red-channel.h | 180 +++++-----
> server/red-parse-qxl.h | 2 +
> server/red-qxl.c | 21 +-
> server/red-replay-qxl.c | 2 +-
> server/red-worker.c | 27 +-
> server/red-worker.h | 2 -
> server/reds-private.h | 3 +-
> server/reds.c | 66 ++--
> server/smartcard.c | 127 +++++--
> server/sound.c | 43 ++-
> server/spicevmc.c | 429 ++++++++++++++++++------
> server/stream.c | 4 +-
> server/stream.h | 3 -
> 37 files changed, 2192 insertions(+), 1044 deletions(-)
> create mode 100644 server/display-channel-private.h
> create mode 100644 server/dummy-channel.c
> create mode 100644 server/dummy-channel.h
I imagined this DummyChannel* would be viral... already in
my TODO list.
>
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 036abcd..c809330 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -102,6 +102,8 @@ libserver_la_SOURCES = \
> red-channel-client.c \
> red-channel-client.h \
> red-channel-client-private.h \
> + dummy-channel.c \
> + dummy-channel.h \
> dummy-channel-client.c \
> dummy-channel-client.h \
> red-common.h \
> diff --git a/server/common-graphics-channel.c
> b/server/common-graphics-channel.c
> index 6871540..d8ee9dd 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -29,6 +29,10 @@
>
> #define CHANNEL_RECEIVE_BUF_SIZE 1024
>
> +G_DEFINE_ABSTRACT_TYPE(CommonGraphicsChannel, common_graphics_channel,
> RED_TYPE_CHANNEL)
> +
> +#define GRAPHICS_CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o),
> TYPE_COMMON_GRAPHICS_CHANNEL, CommonGraphicsChannelPrivate))
> +
> struct CommonGraphicsChannelPrivate
> {
> QXLInstance *qxl;
> @@ -44,7 +48,7 @@ struct CommonGraphicsChannelPrivate
> static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type,
> uint32_t size)
> {
> RedChannel *channel = red_channel_client_get_channel(rcc);
> - CommonGraphicsChannel *common = SPICE_CONTAINEROF(channel,
> CommonGraphicsChannel, base);
> + CommonGraphicsChannel *common = COMMON_GRAPHICS_CHANNEL(channel);
>
> /* SPICE_MSGC_MIGRATE_DATA is the only client message whose size is
> dynamic */
> if (type == SPICE_MSGC_MIGRATE_DATA) {
> @@ -66,6 +70,48 @@ static void common_release_recv_buf(RedChannelClient *rcc,
> uint16_t type, uint32
> }
> }
>
> +
> +enum {
> + PROP0,
> + PROP_QXL
> +};
> +
I remember I had some patches to remove this field... in my TODO list...
> +static void
> +common_graphics_channel_get_property(GObject *object,
> + guint property_id,
> + GValue *value,
> + GParamSpec *pspec)
> +{
> + CommonGraphicsChannel *self = COMMON_GRAPHICS_CHANNEL(object);
> +
> + switch (property_id)
> + {
> + case PROP_QXL:
> + g_value_set_pointer(value, self->priv->qxl);
> + break;
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> + }
> +}
> +
> +static void
> +common_graphics_channel_set_property(GObject *object,
> + guint property_id,
> + const GValue *value,
> + GParamSpec *pspec)
> +{
> + CommonGraphicsChannel *self = COMMON_GRAPHICS_CHANNEL(object);
> +
> + switch (property_id)
> + {
> + case PROP_QXL:
> + self->priv->qxl = g_value_get_pointer(value);
> + break;
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> + }
> +}
> +
> int common_channel_config_socket(RedChannelClient *rcc)
> {
> RedClient *client = red_channel_client_get_client(rcc);
> @@ -107,39 +153,36 @@ int common_channel_config_socket(RedChannelClient *rcc)
> return TRUE;
> }
>
> -CommonGraphicsChannel* common_graphics_channel_new(RedsState *server,
> - QXLInstance *qxl,
> - const
> SpiceCoreInterfaceInternal *core,
> - int size, uint32_t
> channel_type,
> - int migration_flags,
> - ChannelCbs *channel_cbs,
> -
> channel_handle_parsed_proc
> handle_parsed)
> +
> +static void
> +common_graphics_channel_class_init(CommonGraphicsChannelClass *klass)
> +{
> + GObjectClass *object_class = G_OBJECT_CLASS(klass);
> + RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
> +
> + g_type_class_add_private(klass, sizeof(CommonGraphicsChannelPrivate));
> +
> + object_class->get_property = common_graphics_channel_get_property;
> + object_class->set_property = common_graphics_channel_set_property;
> +
> + channel_class->config_socket = common_channel_config_socket;
> + channel_class->alloc_recv_buf = common_alloc_recv_buf;
> + channel_class->release_recv_buf = common_release_recv_buf;
> +
> + g_object_class_install_property(object_class,
> + PROP_QXL,
> + g_param_spec_pointer("qxl",
> + "qxl",
> + "QXLInstance for
> this channel",
> + G_PARAM_READWRITE |
> +
> G_PARAM_CONSTRUCT_ONLY
> |
> +
> G_PARAM_STATIC_STRINGS));
> +}
> +
> +static void
> +common_graphics_channel_init(CommonGraphicsChannel *self)
> {
> - RedChannel *channel = NULL;
> - CommonGraphicsChannel *common;
> -
> - spice_return_val_if_fail(channel_cbs, NULL);
> - spice_return_val_if_fail(!channel_cbs->alloc_recv_buf, NULL);
> - spice_return_val_if_fail(!channel_cbs->release_recv_buf, NULL);
> -
> - if (!channel_cbs->config_socket)
> - channel_cbs->config_socket = common_channel_config_socket;
> - channel_cbs->alloc_recv_buf = common_alloc_recv_buf;
> - channel_cbs->release_recv_buf = common_release_recv_buf;
> -
> - channel = red_channel_create_parser(size, server,
> - core, channel_type,
> - qxl->id, TRUE /* handle_acks */,
> -
> spice_get_client_channel_parser(channel_type,
> NULL),
> - handle_parsed,
> - channel_cbs,
> - migration_flags);
> - spice_return_val_if_fail(channel, NULL);
> -
> - common = COMMON_GRAPHICS_CHANNEL(channel);
> - common->priv = g_new0(CommonGraphicsChannelPrivate, 1);
> - common->priv->qxl = qxl;
> - return common;
> + self->priv = GRAPHICS_CHANNEL_PRIVATE(self);
> }
>
> void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value)
> diff --git a/server/common-graphics-channel.h
> b/server/common-graphics-channel.h
> index c6c3f48..6961375 100644
> --- a/server/common-graphics-channel.h
> +++ b/server/common-graphics-channel.h
> @@ -18,21 +18,42 @@
> #ifndef _COMMON_GRAPHICS_CHANNEL_H
> #define _COMMON_GRAPHICS_CHANNEL_H
>
> +#include <glib-object.h>
> +
> #include "red-channel.h"
> -#include "red-worker.h"
> +#include "red-channel-client.h"
> +
> +G_BEGIN_DECLS
>
> int common_channel_config_socket(RedChannelClient *rcc);
>
> #define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
>
> +#define TYPE_COMMON_GRAPHICS_CHANNEL common_graphics_channel_get_type()
> +
> +#define COMMON_GRAPHICS_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj),
> TYPE_COMMON_GRAPHICS_CHANNEL, CommonGraphicsChannel))
> +#define COMMON_GRAPHICS_CHANNEL_CLASS(klass)
> (G_TYPE_CHECK_CLASS_CAST((klass), TYPE_COMMON_GRAPHICS_CHANNEL,
> CommonGraphicsChannelClass))
> +#define COMMON_IS_GRAPHICS_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj),
> TYPE_COMMON_GRAPHICS_CHANNEL))
> +#define COMMON_IS_GRAPHICS_CHANNEL_CLASS(klass)
> (G_TYPE_CHECK_CLASS_TYPE((klass), TYPE_COMMON_GRAPHICS_CHANNEL))
> +#define COMMON_GRAPHICS_CHANNEL_GET_CLASS(obj)
> (G_TYPE_INSTANCE_GET_CLASS((obj), TYPE_COMMON_GRAPHICS_CHANNEL,
> CommonGraphicsChannelClass))
> +
> +typedef struct CommonGraphicsChannel CommonGraphicsChannel;
> +typedef struct CommonGraphicsChannelClass CommonGraphicsChannelClass;
> typedef struct CommonGraphicsChannelPrivate CommonGraphicsChannelPrivate;
> -typedef struct CommonGraphicsChannel {
> - RedChannel base; // Must be the first thing
> +
> +struct CommonGraphicsChannel
> +{
> + RedChannel parent;
>
> CommonGraphicsChannelPrivate *priv;
> -} CommonGraphicsChannel;
> +};
> +
> +struct CommonGraphicsChannelClass
> +{
> + RedChannelClass parent_class;
> +};
>
> -#define COMMON_GRAPHICS_CHANNEL(Channel) ((CommonGraphicsChannel*)(Channel))
> +GType common_graphics_channel_get_type(void) G_GNUC_CONST;
>
> void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value);
> gboolean
> common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel
> *self);
> @@ -76,12 +97,6 @@ static inline void red_pipes_add_verb(RedChannel *channel,
> uint16_t verb)
> red_channel_apply_clients_data(channel, red_pipe_add_verb_proxy,
> GUINT_TO_POINTER(verb));
> }
>
> -CommonGraphicsChannel* common_graphics_channel_new(RedsState *server,
> - QXLInstance *qxl,
> - const
> SpiceCoreInterfaceInternal *core,
> - int size, uint32_t
> channel_type,
> - int migration_flags,
> - ChannelCbs *channel_cbs,
> -
> channel_handle_parsed_proc
> handle_parsed);
> +G_END_DECLS
>
> #endif /* _COMMON_GRAPHICS_CHANNEL_H */
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index f796c8c..cf19c7f 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -26,8 +26,7 @@
> #include "cursor-channel.h"
> #include "cursor-channel-client.h"
> #include "reds.h"
> -
> -#define CURSOR_CHANNEL(channel) ((CursorChannel*)(channel))
> +#include "red-qxl.h"
>
> typedef struct CursorChannelClient CursorChannelClient;
>
> @@ -50,8 +49,12 @@ typedef struct RedCursorPipeItem {
> CursorItem *cursor_item;
> } RedCursorPipeItem;
>
> -typedef struct CursorChannelPrivate CursorChannelPrivate;
> -struct CursorChannelPrivate {
> +G_DEFINE_TYPE(CursorChannel, cursor_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
> +
> +#define CURSOR_CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o),
> TYPE_CURSOR_CHANNEL, CursorChannelPrivate))
> +
> +struct CursorChannelPrivate
> +{
> CursorItem *item;
> int cursor_visible;
> SpicePoint16 cursor_position;
> @@ -60,12 +63,6 @@ struct CursorChannelPrivate {
> uint32_t mouse_mode;
> };
>
> -struct CursorChannel {
> - CommonGraphicsChannel common; // Must be the first thing
> -
> - CursorChannelPrivate priv[1];
> -};
> -
> static void cursor_pipe_item_free(RedPipeItem *pipe_item);
>
> static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
> @@ -219,8 +216,7 @@ static void cursor_marshall(CursorChannelClient *ccc,
> RedCursorPipeItem *cursor_pipe_item)
> {
> RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
> - CursorChannel *cursor_channel =
> SPICE_CONTAINEROF(red_channel_client_get_channel(rcc),
> - CursorChannel,
> common.base);
> + CursorChannel *cursor_channel =
> CURSOR_CHANNEL(red_channel_client_get_channel(rcc));
> CursorItem *item = cursor_pipe_item->cursor_item;
> RedPipeItem *pipe_item = &cursor_pipe_item->base;
> RedCursorCmd *cmd;
> @@ -314,24 +310,14 @@ static void cursor_channel_send_item(RedChannelClient
> *rcc, RedPipeItem *pipe_it
> CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
> const SpiceCoreInterfaceInternal *core)
> {
> - CursorChannel *cursor_channel;
> - CommonGraphicsChannel *channel = NULL;
> - ChannelCbs cbs = {
> - .on_disconnect = cursor_channel_client_on_disconnect,
> - .send_item = cursor_channel_send_item,
> - };
> -
> spice_info("create cursor channel");
> - channel = common_graphics_channel_new(server, qxl, core,
> - sizeof(CursorChannel),
> - SPICE_CHANNEL_CURSOR, 0,
> - &cbs,
> red_channel_client_handle_message);
> -
> - cursor_channel = CURSOR_CHANNEL(channel);
> - cursor_channel->priv->cursor_visible = TRUE;
> - cursor_channel->priv->mouse_mode = SPICE_MOUSE_MODE_SERVER;
> -
> - return cursor_channel;
> + return g_object_new(TYPE_CURSOR_CHANNEL,
> + "spice-server", server,
> + "core-interface", core,
> + "channel-type", SPICE_CHANNEL_CURSOR,
> + "migration-flags", 0,
> + "qxl", qxl,
> + NULL);
> }
>
> void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd
> *cursor_cmd)
> @@ -368,11 +354,11 @@ void cursor_channel_process_cmd(CursorChannel *cursor,
> RedCursorCmd *cursor_cmd)
> return;
> }
>
> - if (red_channel_is_connected(&cursor->common.base) &&
> + if (red_channel_is_connected(RED_CHANNEL(cursor)) &&
> (cursor->priv->mouse_mode == SPICE_MOUSE_MODE_SERVER
> || cursor_cmd->type != QXL_CURSOR_MOVE
> || cursor_show)) {
> - red_channel_pipes_new_add(&cursor->common.base,
> + red_channel_pipes_new_add(RED_CHANNEL(cursor),
> new_cursor_pipe_item, cursor_item);
> }
>
> @@ -381,7 +367,7 @@ void cursor_channel_process_cmd(CursorChannel *cursor,
> RedCursorCmd *cursor_cmd)
>
> void cursor_channel_reset(CursorChannel *cursor)
> {
> - RedChannel *channel = &cursor->common.base;
> + RedChannel *channel = RED_CHANNEL(cursor);
>
> spice_return_if_fail(cursor);
>
> @@ -395,9 +381,9 @@ void cursor_channel_reset(CursorChannel *cursor)
> if
> (!common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor)))
> {
> red_pipes_add_verb(channel, SPICE_MSG_CURSOR_RESET);
> }
> - if (!red_channel_wait_all_sent(&cursor->common.base,
> + if (!red_channel_wait_all_sent(RED_CHANNEL(cursor),
> COMMON_CLIENT_TIMEOUT)) {
> - red_channel_apply_clients(channel,
> + red_channel_apply_clients(RED_CHANNEL(cursor),
Why not using channel variable ??
> red_channel_client_disconnect_if_pending_send);
> }
> }
> @@ -407,7 +393,7 @@ static void cursor_channel_init_client(CursorChannel
> *cursor, CursorChannelClien
> {
> spice_return_if_fail(cursor);
>
> - if (!red_channel_is_connected(&cursor->common.base)
> + if (!red_channel_is_connected(RED_CHANNEL(cursor))
> || common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor)))
> || {
> spice_debug("during_target_migrate: skip init");
> return;
> @@ -420,7 +406,7 @@ static void cursor_channel_init_client(CursorChannel
> *cursor, CursorChannelClien
> red_channel_pipes_add_type(RED_CHANNEL(cursor),
> RED_PIPE_ITEM_TYPE_CURSOR_INIT);
> }
>
> -void cursor_channel_init(CursorChannel *cursor)
> +void cursor_channel_do_init(CursorChannel *cursor)
> {
> cursor_channel_init_client(cursor, NULL);
> }
> @@ -454,3 +440,25 @@ void cursor_channel_connect(CursorChannel *cursor,
> RedClient *client, RedsStream
>
> cursor_channel_init_client(cursor, ccc);
> }
> +
> +static void
> +cursor_channel_class_init(CursorChannelClass *klass)
> +{
> + RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
> +
> + g_type_class_add_private(klass, sizeof(CursorChannelPrivate));
> +
> + channel_class->parser =
> spice_get_client_channel_parser(SPICE_CHANNEL_CURSOR, NULL);
> + channel_class->handle_parsed = red_channel_client_handle_message;
> +
> + channel_class->on_disconnect = cursor_channel_client_on_disconnect;
> + channel_class->send_item = cursor_channel_send_item;
> +}
> +
> +static void
> +cursor_channel_init(CursorChannel *self)
> +{
> + self->priv = CURSOR_CHANNEL_PRIVATE(self);
> + self->priv->cursor_visible = TRUE;
> + self->priv->mouse_mode = SPICE_MOUSE_MODE_SERVER;
> +}
> diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> index a3ddaa3..8b3bc17 100644
> --- a/server/cursor-channel.h
> +++ b/server/cursor-channel.h
> @@ -19,6 +19,17 @@
> # define CURSOR_CHANNEL_H_
>
> #include "common-graphics-channel.h"
> +#include "red-parse-qxl.h"
> +
> +G_BEGIN_DECLS
> +
> +#define TYPE_CURSOR_CHANNEL cursor_channel_get_type()
> +
> +#define CURSOR_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj),
> TYPE_CURSOR_CHANNEL, CursorChannel))
> +#define CURSOR_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass),
> TYPE_CURSOR_CHANNEL, CursorChannelClass))
> +#define IS_CURSOR_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj),
> TYPE_CURSOR_CHANNEL))
> +#define IS_CURSOR_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass),
> TYPE_CURSOR_CHANNEL))
> +#define CURSOR_CHANNEL_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj),
> TYPE_CURSOR_CHANNEL, CursorChannelClass))
>
> /**
> * This type it's a RedChannel class which implement cursor (mouse)
> @@ -26,6 +37,22 @@
> * A pointer to CursorChannel can be converted to a RedChannel.
> */
> typedef struct CursorChannel CursorChannel;
> +typedef struct CursorChannelClass CursorChannelClass;
> +typedef struct CursorChannelPrivate CursorChannelPrivate;
> +
> +struct CursorChannel
> +{
> + CommonGraphicsChannel parent;
> +
> + CursorChannelPrivate *priv;
> +};
> +
> +struct CursorChannelClass
> +{
> + CommonGraphicsChannelClass parent_class;
> +};
> +
> +GType cursor_channel_get_type(void) G_GNUC_CONST;
>
Why we need to expose this here ? On the C file is not enough ?
> /**
> * Create CursorChannel.
> @@ -47,7 +74,7 @@ CursorChannel* cursor_channel_new (RedsState
> *server, QXLInstance
> */
> void cursor_channel_disconnect (CursorChannel *cursor);
> void cursor_channel_reset (CursorChannel *cursor);
> -void cursor_channel_init (CursorChannel *cursor);
> +void cursor_channel_do_init (CursorChannel *cursor);
> void cursor_channel_process_cmd (CursorChannel *cursor,
> RedCursorCmd *cursor_cmd);
> void cursor_channel_set_mouse_mode(CursorChannel *cursor,
> uint32_t mode);
>
> @@ -69,4 +96,6 @@ void cursor_channel_connect
> (CursorChannel *cursor, RedClien
> */
> void cursor_channel_client_migrate(RedChannelClient
> *client);
>
> +G_END_DECLS
> +
> #endif /* CURSOR_CHANNEL_H_ */
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index e33f428..ef67f97 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -20,7 +20,7 @@
> #endif
>
> #include "dcc-private.h"
> -#include "display-channel.h"
> +#include "display-channel-private.h"
> #include "red-channel-client-private.h"
>
> #include <common/marshaller.h>
> @@ -185,8 +185,9 @@ static void
> red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
> SpiceImage *image,
> SpiceImage *io_image,
> int is_lossy)
> {
> + DisplayChannel *display_channel =
> + DISPLAY_CHANNEL(red_channel_client_get_channel(rcc));
> DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(rcc);
> - DisplayChannel *display_channel = DCC_TO_DC(dcc);
>
I prefer the old version, DCC_TO_DC is still present and used.
> if ((image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
> spice_assert(image->descriptor.width * image->descriptor.height >
> 0);
> @@ -1817,7 +1818,7 @@ static void
> display_channel_marshall_migrate_data(RedChannelClient *rcc,
> ImageEncoders *encoders = dcc_get_encoders(dcc);
> SpiceMigrateDataDisplay display_data = {0,};
>
> - display_channel = DCC_TO_DC(dcc);
> + display_channel = DISPLAY_CHANNEL(red_channel_client_get_channel(rcc));
>
same here
> red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, NULL);
> spice_marshaller_add_uint32(base_marshaller,
> SPICE_MIGRATE_DATA_DISPLAY_MAGIC);
> @@ -2120,8 +2121,8 @@ static void marshall_qxl_drawable(RedChannelClient
> *rcc,
> spice_return_if_fail(rcc);
>
> Drawable *item = dpi->drawable;
> - DisplayChannel *display =
> SPICE_CONTAINEROF(red_channel_client_get_channel(rcc),
> - DisplayChannel,
> common.base);
> + DisplayChannel *display =
> + DISPLAY_CHANNEL(red_channel_client_get_channel(rcc));
>
> spice_return_if_fail(display);
> /* allow sized frames to be streamed, even if they where replaced by
> another frame, since
> diff --git a/server/dcc.c b/server/dcc.c
> index 3519d2e..ea6305c 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -21,6 +21,7 @@
>
> #include "dcc-private.h"
> #include "display-channel.h"
> +#include "display-channel-private.h"
> #include "red-channel-client-private.h"
> #include "spice-server-enums.h"
>
> diff --git a/server/dcc.h b/server/dcc.h
> index 2456f09..e4fe788 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -23,8 +23,8 @@
> #include "image-encoders.h"
> #include "image-cache.h"
> #include "pixmap-cache.h"
> -#include "red-worker.h"
> #include "display-limits.h"
> +#include "red-channel-client.h"
>
> G_BEGIN_DECLS
>
> diff --git a/server/display-channel-private.h
> b/server/display-channel-private.h
> new file mode 100644
> index 0000000..38330da
> --- /dev/null
> +++ b/server/display-channel-private.h
> @@ -0,0 +1,76 @@
> +/*
> + 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 DISPLAY_CHANNEL_PRIVATE_H_
> +#define DISPLAY_CHANNEL_PRIVATE_H_
> +
> +#include "display-channel.h"
> +
> +struct DisplayChannelPrivate
> +{
> + DisplayChannel *pub;
> +
> + uint32_t bits_unique;
> +
> + MonitorsConfig *monitors_config;
> +
> + uint32_t renderer;
> + int enable_jpeg;
> + int enable_zlib_glz_wrap;
> +
> + Ring current_list; // of TreeItem
> + uint32_t current_size;
> +
> + uint32_t drawable_count;
> + _Drawable drawables[NUM_DRAWABLES];
> + _Drawable *free_drawables;
> +
> + int stream_video;
> + GArray *video_codecs;
> + uint32_t stream_count;
> + Stream streams_buf[NUM_STREAMS];
> + Stream *free_streams;
> + Ring streams;
> + ItemTrace items_trace[NUM_TRACE_ITEMS];
> + uint32_t next_item_trace;
> + uint64_t streams_size_total;
> +
> + RedSurface surfaces[NUM_SURFACES];
> + uint32_t n_surfaces;
> + SpiceImageSurfaces image_surfaces;
> +
> + ImageCache image_cache;
> +
> + int gl_draw_async_count;
> +
> +/* TODO: some day unify this, make it more runtime.. */
> + stat_info_t add_stat;
> + stat_info_t exclude_stat;
> + stat_info_t __exclude_stat;
> +#ifdef RED_WORKER_STAT
> + uint32_t add_count;
> + uint32_t add_with_shadow_count;
> +#endif
> +#ifdef RED_STATISTICS
> + uint64_t *cache_hits_counter;
> + uint64_t *add_to_cache_counter;
> + uint64_t *non_cache_counter;
> +#endif
> + ImageEncoderSharedData encoder_shared_data;
> +};
> +
> +#endif /* DISPLAY_CHANNEL_PRIVATE_H_ */
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 69edd35..19f0aca 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -20,7 +20,131 @@
>
> #include <common/sw_canvas.h>
>
> -#include "display-channel.h"
> +#include "display-channel-private.h"
> +
> +G_DEFINE_TYPE(DisplayChannel, display_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
> +
> +enum {
> + PROP0,
> + PROP_N_SURFACES,
> + PROP_VIDEO_CODECS
> +};
> +
> +static void
> +display_channel_get_property(GObject *object,
> + guint property_id,
> + GValue *value,
> + GParamSpec *pspec)
> +{
> + DisplayChannel *self = DISPLAY_CHANNEL(object);
> +
> + switch (property_id)
> + {
> + case PROP_N_SURFACES:
> + g_value_set_uint(value, self->priv->n_surfaces);
> + break;
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> + }
> +}
> +
> +static void
> +display_channel_set_property(GObject *object,
> + guint property_id,
> + const GValue *value,
> + GParamSpec *pspec)
> +{
> + DisplayChannel *self = DISPLAY_CHANNEL(object);
> +
> + switch (property_id)
> + {
> + case PROP_N_SURFACES:
> + self->priv->n_surfaces = g_value_get_uint(value);
> + break;
Sure we want this writable ?
Shouldn't we reallocate the array ?
> + case PROP_VIDEO_CODECS:
> + if (self->priv->video_codecs) {
> + g_array_unref(self->priv->video_codecs);
> + }
> + self->priv->video_codecs =
> g_array_ref(g_value_get_boxed(value));
> + break;
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> + }
> +}
> +
> +static void
> +display_channel_finalize(GObject *object)
> +{
> + DisplayChannel *self = DISPLAY_CHANNEL(object);
> +
> + G_OBJECT_CLASS(display_channel_parent_class)->finalize(object);
> +
> + g_free(self->priv);
> + g_array_unref(self->priv->video_codecs);
> +}
> +
> +static void
> +display_channel_constructed(GObject *object)
> +{
> + DisplayChannel *self = DISPLAY_CHANNEL(object);
> +
> + G_OBJECT_CLASS(display_channel_parent_class)->constructed(object);
> +
> + spice_assert(self->priv->video_codecs);
> +
> + self->priv->renderer = RED_RENDERER_INVALID;
> +
> + stat_init(&self->priv->add_stat, "add", CLOCK_THREAD_CPUTIME_ID);
> + stat_init(&self->priv->exclude_stat, "exclude",
> CLOCK_THREAD_CPUTIME_ID);
> + stat_init(&self->priv->__exclude_stat, "__exclude",
> CLOCK_THREAD_CPUTIME_ID);
> +#ifdef RED_STATISTICS
> + QXLInstance *qxl = common_graphics_channel_get_qxl(&self->parent);
> + RedsState *reds = red_qxl_get_server(qxl->st);
> + RedChannel *channel = RED_CHANNEL(self);
> + self->priv->cache_hits_counter =
> + stat_add_counter(reds, red_channel_get_stat_node(channel),
> + "cache_hits", TRUE);
> + self->priv->add_to_cache_counter =
> + stat_add_counter(reds, red_channel_get_stat_node(channel),
> + "add_to_cache", TRUE);
> + self->priv->non_cache_counter =
> + stat_add_counter(reds, red_channel_get_stat_node(channel),
> + "non_cache", TRUE);
> +#endif
> + image_cache_init(&self->priv->image_cache);
> + self->priv->stream_video = SPICE_STREAM_VIDEO_OFF;
> + display_channel_init_streams(self);
> +}
> +
> +static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces,
> uint32_t surface_id)
> +{
> + DisplayChannelPrivate *p = SPICE_CONTAINEROF(surfaces,
> DisplayChannelPrivate, image_surfaces);
> + DisplayChannel *display = p->pub;
> +
> + spice_return_val_if_fail(display_channel_validate_surface(display,
> surface_id), NULL);
> +
> + return p->surfaces[surface_id].context.canvas;
> +}
> +
> +static void drawables_init(DisplayChannel *display);
> +static void
> +display_channel_init(DisplayChannel *self)
> +{
> + static SpiceImageSurfacesOps image_surfaces_ops = {
> + image_surfaces_get,
> + };
> +
> + /* must be manually allocated here since g_type_class_add_private() only
> + * supports structs smaller than 64k */
> + self->priv = g_new0(DisplayChannelPrivate, 1);
> + self->priv->pub = self;
> +
> + image_encoder_shared_init(&self->priv->encoder_shared_data);
> +
> + ring_init(&self->priv->current_list);
> + drawables_init(self);
> + self->priv->image_surfaces.ops = &image_surfaces_ops;
> +}
>
> static void drawable_draw(DisplayChannel *display, Drawable *drawable);
> static Drawable *display_channel_drawable_try_new(DisplayChannel *display,
> @@ -43,12 +167,16 @@ void display_channel_compress_stats_reset(DisplayChannel
> *display)
> image_encoder_shared_stat_reset(&display->priv->encoder_shared_data);
> }
>
> -void display_channel_compress_stats_print(const DisplayChannel
> *display_channel)
> +void display_channel_compress_stats_print(DisplayChannel *display_channel)
> {
> #ifdef COMPRESS_STAT
> + uint32_t id;
> +
> spice_return_if_fail(display_channel);
>
> - spice_info("==> Compression stats for display %u",
> display_channel->common.base.id);
> + g_object_get(display_channel, "id", &id, NULL);
> +
> + spice_info("==> Compression stats for display %u", id);
> image_encoder_shared_stat_print(&display_channel->priv->encoder_shared_data);
> #endif
> }
> @@ -150,6 +278,11 @@ void display_channel_set_video_codecs(DisplayChannel
> *display, GArray *video_cod
> display->priv->video_codecs = g_array_ref(video_codecs);
> }
>
> +int display_channel_get_stream_video(DisplayChannel *display)
> +{
> + return display->priv->stream_video;
> +}
> +
> static void stop_streams(DisplayChannel *display)
> {
> Ring *ring = &display->priv->streams;
> @@ -280,7 +413,7 @@ static void pipes_add_drawable_after(DisplayChannel
> *display,
> pipes_add_drawable(display, drawable);
> return;
> }
> - if (num_other_linked != g_list_length(display->common.base.clients)) {
> + if (num_other_linked != red_channel_get_n_clients(RED_CHANNEL(display)))
> {
> GListIter iter;
> spice_debug("TODO: not O(n^2)");
> FOREACH_DCC(display, iter, dcc) {
> @@ -1115,18 +1248,18 @@ void display_channel_process_draw(DisplayChannel
> *display, RedDrawable *red_draw
> int display_channel_wait_for_migrate_data(DisplayChannel *display)
> {
> uint64_t end_time = spice_get_monotonic_time_ns() +
> DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT;
> - RedChannel *channel = &display->common.base;
> RedChannelClient *rcc;
> int ret = FALSE;
> + GList *clients = red_channel_get_clients(RED_CHANNEL(display));;
>
> - if (!red_channel_is_waiting_for_migrate_data(&display->common.base)) {
> + if (!red_channel_is_waiting_for_migrate_data(RED_CHANNEL(display))) {
> return FALSE;
> }
>
> spice_debug(NULL);
> - spice_warn_if_fail(g_list_length(channel->clients) == 1);
> + spice_warn_if_fail(g_list_length(clients) == 1);
>
> - rcc = g_list_nth_data(channel->clients, 0);
> + rcc = g_list_nth_data(clients, 0);
>
> g_object_ref(rcc);
> for (;;) {
> @@ -1893,16 +2026,6 @@ static int handle_migrate_data(RedChannelClient *rcc,
> uint32_t size, void *messa
> return dcc_handle_migrate_data(DISPLAY_CHANNEL_CLIENT(rcc), size,
> message);
> }
>
> -static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces,
> uint32_t surface_id)
> -{
> - DisplayChannelPrivate *p = SPICE_CONTAINEROF(surfaces,
> DisplayChannelPrivate, image_surfaces);
> - DisplayChannel *display = p->pub;
> -
> - spice_return_val_if_fail(display_channel_validate_surface(display,
> surface_id), NULL);
> -
> - return display->priv->surfaces[surface_id].context.canvas;
> -}
> -
> DisplayChannel* display_channel_new(RedsState *reds,
> QXLInstance *qxl,
> const SpiceCoreInterfaceInternal *core,
> @@ -1911,52 +2034,21 @@ DisplayChannel* display_channel_new(RedsState *reds,
> uint32_t n_surfaces)
> {
> DisplayChannel *display;
> - ChannelCbs cbs = {
> - .on_disconnect = on_disconnect,
> - .send_item = dcc_send_item,
> - .handle_migrate_flush_mark = handle_migrate_flush_mark,
> - .handle_migrate_data = handle_migrate_data,
> - .handle_migrate_data_get_serial = handle_migrate_data_get_serial,
> - .config_socket = dcc_config_socket
> - };
> - static SpiceImageSurfacesOps image_surfaces_ops = {
> - image_surfaces_get,
> - };
>
> + /* FIXME: migrate is not used...? */
> spice_info("create display channel");
> - display = DISPLAY_CHANNEL(common_graphics_channel_new(
> - reds, qxl, core, sizeof(*display), SPICE_CHANNEL_DISPLAY,
> - SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER,
> - &cbs, dcc_handle_message));
> - spice_return_val_if_fail(display, NULL);
> - display->priv->pub = display;
> -
> - clockid_t stat_clock = CLOCK_THREAD_CPUTIME_ID;
> - stat_init(&display->priv->add_stat, "add", stat_clock);
> - stat_init(&display->priv->exclude_stat, "exclude", stat_clock);
> - stat_init(&display->priv->__exclude_stat, "__exclude", stat_clock);
> -#ifdef RED_STATISTICS
> - RedChannel *channel = RED_CHANNEL(display);
> - display->priv->cache_hits_counter = stat_add_counter(reds,
> channel->stat,
> - "cache_hits",
> TRUE);
> - display->priv->add_to_cache_counter = stat_add_counter(reds,
> channel->stat,
> - "add_to_cache",
> TRUE);
> - display->priv->non_cache_counter = stat_add_counter(reds, channel->stat,
> - "non_cache", TRUE);
> -#endif
> - image_encoder_shared_init(&display->priv->encoder_shared_data);
> -
> - display->priv->n_surfaces = n_surfaces;
> - display->priv->renderer = RED_RENDERER_INVALID;
> -
> - ring_init(&display->priv->current_list);
> - display->priv->image_surfaces.ops = &image_surfaces_ops;
> - drawables_init(display);
> - image_cache_init(&display->priv->image_cache);
> - display->priv->stream_video = stream_video;
> - display->priv->video_codecs = g_array_ref(video_codecs);
> - display_channel_init_streams(display);
> -
> + display = g_object_new(TYPE_DISPLAY_CHANNEL,
> + "spice-server", reds,
> + "core-interface", core,
> + "channel-type", SPICE_CHANNEL_DISPLAY,
> + "migration-flags", (SPICE_MIGRATE_NEED_FLUSH |
> SPICE_MIGRATE_NEED_DATA_TRANSFER),
> + "qxl", qxl,
> + "n-surfaces", n_surfaces,
> + "video-codecs", video_codecs,
> + NULL);
> + if (display) {
> + display_channel_set_stream_video(display, stream_video);
> + }
> return display;
> }
>
> @@ -2085,7 +2177,6 @@ void
> display_channel_update_monitors_config(DisplayChannel *display,
> QXLMonitorsConfig *config,
> uint16_t count, uint16_t
> max_allowed)
> {
> -
> if (display->priv->monitors_config)
> monitors_config_unref(display->priv->monitors_config);
>
> @@ -2113,6 +2204,50 @@ void display_channel_reset_image_cache(DisplayChannel
> *self)
> image_cache_reset(&self->priv->image_cache);
> }
>
> +static void
> +display_channel_class_init(DisplayChannelClass *klass)
> +{
> + GObjectClass *object_class = G_OBJECT_CLASS(klass);
> + RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
> +
> + g_type_class_add_private(klass, sizeof(DisplayChannelPrivate));
> +
> + object_class->get_property = display_channel_get_property;
> + object_class->set_property = display_channel_set_property;
> + object_class->constructed = display_channel_constructed;
> + object_class->finalize = display_channel_finalize;
> +
> + channel_class->parser =
> spice_get_client_channel_parser(SPICE_CHANNEL_DISPLAY, NULL);
> + channel_class->handle_parsed = dcc_handle_message;
> +
> + channel_class->on_disconnect = on_disconnect;
> + channel_class->send_item = dcc_send_item;
> + channel_class->handle_migrate_flush_mark = handle_migrate_flush_mark;
> + channel_class->handle_migrate_data = handle_migrate_data;
> + channel_class->handle_migrate_data_get_serial =
> handle_migrate_data_get_serial;
> + channel_class->config_socket = dcc_config_socket;
> +
> + g_object_class_install_property(object_class,
> + PROP_N_SURFACES,
> + g_param_spec_uint("n-surfaces",
> + "number of surfaces",
> + "Number of surfaces
> for this channel",
> + 0, G_MAXUINT,
> + 0,
> + G_PARAM_CONSTRUCT_ONLY
> |
> + G_PARAM_READWRITE |
> +
Imo should be write only.
> G_PARAM_STATIC_STRINGS));
> + g_object_class_install_property(object_class,
> + PROP_VIDEO_CODECS,
> + g_param_spec_boxed("video-codecs",
> + "video codecs",
> + "Video Codecs",
> + G_TYPE_ARRAY,
> +
> G_PARAM_CONSTRUCT_ONLY
> |
> + G_PARAM_WRITABLE |
> +
> G_PARAM_STATIC_STRINGS));
> +}
> +
> void display_channel_debug_oom(DisplayChannel *display, const char *msg)
> {
> RedChannel *channel = RED_CHANNEL(display);
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 3762e54..9ac9046 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -20,32 +20,59 @@
>
> #include <setjmp.h>
> #include <common/rect.h>
> +#include <common/sw_canvas.h>
>
> -#include "reds-stream.h"
> #include "cache-item.h"
> -#include "pixmap-cache.h"
> -#include "stat.h"
> -#include "reds.h"
> -#include "memslot.h"
> -#include "red-parse-qxl.h"
> -#include "red-record-qxl.h"
> +#include "image-encoders.h"
> +#include "dcc.h"
> #include "demarshallers.h"
> -#include "red-channel.h"
> -#include "red-qxl.h"
> #include "dispatcher.h"
> #include "main-channel.h"
> -#include "migration-protocol.h"
> #include "main-dispatcher.h"
> +#include "memslot.h"
> +#include "migration-protocol.h"
> +#include "video-encoder.h"
> +#include "pixmap-cache.h"
> +#include "red-channel.h"
> +#include "red-qxl.h"
> +#include "red-parse-qxl.h"
> +#include "red-record-qxl.h"
> +#include "reds-stream.h"
> +#include "reds.h"
> #include "spice-bitmap-utils.h"
> -#include "image-cache.h"
> -#include "utils.h"
> -#include "tree.h"
> +#include "stat.h"
> #include "stream.h"
> -#include "dcc.h"
> -#include "image-encoders.h"
> #include "common-graphics-channel.h"
> +#include "tree.h"
> +#include "utils.h"
> +
May headers are just included in a different order.
Is there any reason to change the order?
> +G_BEGIN_DECLS
> +
> +#define TYPE_DISPLAY_CHANNEL display_channel_get_type()
> +
> +#define DISPLAY_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj),
> TYPE_DISPLAY_CHANNEL, DisplayChannel))
> +#define DISPLAY_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass),
> TYPE_DISPLAY_CHANNEL, DisplayChannelClass))
> +#define IS_DISPLAY_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj),
> TYPE_DISPLAY_CHANNEL))
> +#define IS_DISPLAY_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass),
> TYPE_DISPLAY_CHANNEL))
> +#define DISPLAY_CHANNEL_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj),
> TYPE_DISPLAY_CHANNEL, DisplayChannelClass))
> +
> +typedef struct DisplayChannel DisplayChannel;
> +typedef struct DisplayChannelClass DisplayChannelClass;
> +typedef struct DisplayChannelPrivate DisplayChannelPrivate;
> +
> +struct DisplayChannel
> +{
> + CommonGraphicsChannel parent;
> +
> + DisplayChannelPrivate *priv;
> +};
> +
> +struct DisplayChannelClass
> +{
> + CommonGraphicsChannelClass parent_class;
> +};
>
> -#define DISPLAY_CHANNEL(channel) ((DisplayChannel*)(channel))
> +GType display_channel_get_type(void) G_GNUC_CONST;
>
> typedef struct DependItem {
> Drawable *drawable;
> @@ -154,69 +181,8 @@ struct _Drawable {
> } u;
> };
>
> -typedef struct DisplayChannelPrivate DisplayChannelPrivate;
> -/* FIXME: move to separate file */
> -struct DisplayChannelPrivate
> -{
> - DisplayChannel *pub;
> -
> - uint32_t bits_unique;
> -
> - MonitorsConfig *monitors_config;
> -
> - uint32_t renderer;
> - int enable_jpeg;
> - int enable_zlib_glz_wrap;
> -
> - Ring current_list; // of TreeItem
> - uint32_t current_size;
> -
> - uint32_t drawable_count;
> - _Drawable drawables[NUM_DRAWABLES];
> - _Drawable *free_drawables;
> -
> - int stream_video;
> - GArray *video_codecs;
> - uint32_t stream_count;
> - Stream streams_buf[NUM_STREAMS];
> - Stream *free_streams;
> - Ring streams;
> - ItemTrace items_trace[NUM_TRACE_ITEMS];
> - uint32_t next_item_trace;
> - uint64_t streams_size_total;
> -
> - RedSurface surfaces[NUM_SURFACES];
> - uint32_t n_surfaces;
> - SpiceImageSurfaces image_surfaces;
> -
> - ImageCache image_cache;
> -
> - int gl_draw_async_count;
> -
> -/* TODO: some day unify this, make it more runtime.. */
> - stat_info_t add_stat;
> - stat_info_t exclude_stat;
> - stat_info_t __exclude_stat;
> -#ifdef RED_WORKER_STAT
> - uint32_t add_count;
> - uint32_t add_with_shadow_count;
> -#endif
> -#ifdef RED_STATISTICS
> - uint64_t *cache_hits_counter;
> - uint64_t *add_to_cache_counter;
> - uint64_t *non_cache_counter;
> -#endif
> - ImageEncoderSharedData encoder_shared_data;
> -};
> -
> -struct DisplayChannel {
> - CommonGraphicsChannel common; // Must be the first thing
> -
> - DisplayChannelPrivate priv[1];
> -};
> -
> #define FOREACH_DCC(_channel, _iter, _data) \
> - GLIST_FOREACH((_channel ? RED_CHANNEL(_channel)->clients : NULL), \
> + GLIST_FOREACH((_channel ? red_channel_get_clients(RED_CHANNEL(_channel))
> : NULL), \
> _iter, DisplayChannelClient, _data)
>
> int display_channel_get_stream_id(DisplayChannel *display, Stream *stream);
> @@ -262,8 +228,9 @@ void
> display_channel_set_stream_video (DisplayCha
> int
> stream_video);
> void display_channel_set_video_codecs
> (DisplayChannel *display,
> GArray
> *video_codecs);
> +int display_channel_get_stream_video
> (DisplayChannel *display);
> int display_channel_get_streams_timeout
> (DisplayChannel *display);
> -void display_channel_compress_stats_print (const
> DisplayChannel *display);
> +void display_channel_compress_stats_print
> (DisplayChannel *display);
> void display_channel_compress_stats_reset
> (DisplayChannel *display);
> void display_channel_surface_unref
> (DisplayChannel *display,
> uint32_t
> surface_id);
> @@ -415,4 +382,6 @@ static inline void region_add_clip_rects(QRegion *rgn,
> SpiceClipRects *data)
> }
> }
>
> +G_END_DECLS
> +
> #endif /* DISPLAY_CHANNEL_H_ */
> diff --git a/server/dummy-channel-client.c b/server/dummy-channel-client.c
> index 1b72137..b7fee6f 100644
> --- a/server/dummy-channel-client.c
> +++ b/server/dummy-channel-client.c
> @@ -37,9 +37,11 @@ struct DummyChannelClientPrivate
>
> static int dummy_channel_client_pre_create_validate(RedChannel *channel,
> RedClient *client)
> {
> - if (red_client_get_channel(client, channel->type, channel->id)) {
> + uint32_t type, id;
> + g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> + if (red_client_get_channel(client, type, id)) {
> spice_printerr("Error client %p: duplicate channel type %d id %d",
> - client, channel->type, channel->id);
> + client, type, id);
> return FALSE;
> }
> return TRUE;
> @@ -54,6 +56,9 @@ static gboolean
> dummy_channel_client_initable_init(GInitable *initable,
> RedChannelClient *rcc = RED_CHANNEL_CLIENT(self);
> RedClient *client = red_channel_client_get_client(rcc);
> RedChannel *channel = red_channel_client_get_channel(rcc);
> + uint32_t type, id;
> +
> + g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> pthread_mutex_lock(&client->lock);
> if (!dummy_channel_client_pre_create_validate(channel,
> client)) {
> @@ -61,7 +66,7 @@ static gboolean
> dummy_channel_client_initable_init(GInitable *initable,
> SPICE_SERVER_ERROR,
> SPICE_SERVER_ERROR_FAILED,
> "Client %p: duplicate channel type %d id %d",
> - client, channel->type, channel->id);
> + client, type, id);
> goto cleanup;
> }
>
> @@ -94,10 +99,12 @@ static void
> dummy_channel_client_disconnect(RedChannelClient *rcc)
> DummyChannelClient *self = DUMMY_CHANNEL_CLIENT(rcc);
> RedChannel *channel = red_channel_client_get_channel(rcc);
> GList *link;
> + uint32_t type, id;
>
> - if (channel && (link = g_list_find(channel->clients, rcc))) {
> + if (channel && (link = g_list_find(red_channel_get_clients(channel),
> rcc))) {
> + g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc, channel,
> - channel->type, channel->id);
> + type, id);
> red_channel_remove_client(channel, link->data);
> }
> self->priv->connected = FALSE;
> diff --git a/server/dummy-channel.c b/server/dummy-channel.c
> new file mode 100644
> index 0000000..6ec7842
> --- /dev/null
> +++ b/server/dummy-channel.c
> @@ -0,0 +1,58 @@
> +/* dummy-channel.c */
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include "dummy-channel.h"
> +
> +G_DEFINE_TYPE(DummyChannel, dummy_channel, RED_TYPE_CHANNEL)
> +
> +#define DUMMY_CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o),
> TYPE_DUMMY_CHANNEL, DummyChannelPrivate))
> +
> +struct DummyChannelPrivate
> +{
> + gpointer padding;
> +};
> +
Why just not using a private ? This padding field is not used.
And you can just keep the priv field to retain ABI.
This dummy stuff is moving from ugly to disgusting...
> +static void
> +dummy_channel_class_init(DummyChannelClass *klass)
> +{
> + g_type_class_add_private(klass, sizeof(DummyChannelPrivate));
> +}
> +
> +static void
> +dummy_channel_init(DummyChannel *self)
> +{
> + self->priv = DUMMY_CHANNEL_PRIVATE(self);
> +}
> +
> +// TODO: red_worker can use this one
> +static void dummy_watch_update_mask(SpiceWatch *watch, int event_mask)
> +{
> +}
> +
> +static SpiceWatch *dummy_watch_add(int fd, int event_mask, SpiceWatchFunc
> func, void *opaque)
> +{
> + return NULL; // apparently allowed?
> +}
> +
> +static void dummy_watch_remove(SpiceWatch *watch)
> +{
> +}
> +
> +// TODO: actually, since I also use channel_client_dummym, no need for core.
> Can be NULL
> +static const SpiceCoreInterface dummy_core = {
> + .watch_update_mask = dummy_watch_update_mask,
> + .watch_add = dummy_watch_add,
> + .watch_remove = dummy_watch_remove,
> +};
> +
> +RedChannel *dummy_channel_new(RedsState *reds, uint32_t type, uint32_t id)
> +{
> + return g_object_new(TYPE_DUMMY_CHANNEL,
> + "spice-server", reds,
> + "core-interface", &dummy_core,
> + "channel-type", type,
> + "id", id,
> + NULL);
> +}
> diff --git a/server/dummy-channel.h b/server/dummy-channel.h
> new file mode 100644
> index 0000000..dd2f005
> --- /dev/null
> +++ b/server/dummy-channel.h
> @@ -0,0 +1,61 @@
> +/*
> + 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 __DUMMY_CHANNEL_H__
> +#define __DUMMY_CHANNEL_H__
> +
> +#include <glib-object.h>
> +
> +#include "red-channel.h"
> +
> +G_BEGIN_DECLS
> +
> +// TODO: tmp, for channels that don't use RedChannel yet (e.g., snd
> channel), but
> +// do use the client callbacks. So the channel clients are not connected
> (the channel doesn't
> +// have list of them, but they do have a link to the channel, and the client
> has a list of them)
> +
> +#define TYPE_DUMMY_CHANNEL dummy_channel_get_type()
> +
> +#define DUMMY_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj),
> TYPE_DUMMY_CHANNEL, DummyChannel))
> +#define DUMMY_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass),
> TYPE_DUMMY_CHANNEL, DummyChannelClass))
> +#define _IS_DUMMY_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj),
> TYPE_DUMMY_CHANNEL))
> +#define _IS_DUMMY_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass),
> TYPE_DUMMY_CHANNEL))
> +#define DUMMY_CHANNEL_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj),
> TYPE_DUMMY_CHANNEL, DummyChannelClass))
> +
> +typedef struct DummyChannel DummyChannel;
> +typedef struct DummyChannelClass DummyChannelClass;
> +typedef struct DummyChannelPrivate DummyChannelPrivate;
> +
> +struct DummyChannel
> +{
> + RedChannel parent;
> +
> + DummyChannelPrivate *priv;
> +};
> +
> +struct DummyChannelClass
> +{
> + RedChannelClass parent_class;
> +};
> +
> +GType dummy_channel_get_type(void) G_GNUC_CONST;
> +
> +RedChannel *dummy_channel_new(RedsState *reds, uint32_t type, uint32_t id);
> +
> +G_END_DECLS
> +
> +#endif /* __DUMMY_CHANNEL_H__ */
> diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> index 83c1360..c351dad 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -57,6 +57,83 @@
> #define RECEIVE_BUF_SIZE \
> (4096 + (REDS_AGENT_WINDOW_SIZE + REDS_NUM_INTERNAL_AGENT_MESSAGES) *
> SPICE_AGENT_MAX_DATA_SIZE)
>
> +G_DEFINE_TYPE(InputsChannel, inputs_channel, RED_TYPE_CHANNEL)
> +
> +#define CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o),
> TYPE_INPUTS_CHANNEL, InputsChannelPrivate))
> +
> +struct InputsChannelPrivate
> +{
> + uint8_t recv_buf[RECEIVE_BUF_SIZE];
> + VDAgentMouseState mouse_state;
> + int src_during_migrate;
> + SpiceTimer *key_modifiers_timer;
> +
> + SpiceKbdInstance *keyboard;
> + SpiceMouseInstance *mouse;
> + SpiceTabletInstance *tablet;
> +};
> +
> +
> +static void
> +inputs_channel_get_property(GObject *object,
> + guint property_id,
> + GValue *value,
> + GParamSpec *pspec)
> +{
> + switch (property_id)
> + {
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> + }
> +}
> +
> +static void
> +inputs_channel_set_property(GObject *object,
> + guint property_id,
> + const GValue *value,
> + GParamSpec *pspec)
> +{
> + switch (property_id)
> + {
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> + }
> +}
> +
I noted there are these empty function here and in different file.
Don't GObject provide some default functions like these?
> +static void inputs_connect(RedChannel *channel, RedClient *client,
> + RedsStream *stream, int migration,
> + int num_common_caps, uint32_t *common_caps,
> + int num_caps, uint32_t *caps);
> +static void inputs_migrate(RedChannelClient *rcc);
> +static void key_modifiers_sender(void *opaque);
> +
> +static void
> +inputs_channel_constructed(GObject *object)
> +{
> + ClientCbs client_cbs = { NULL, };
> + InputsChannel *self = INPUTS_CHANNEL(object);
> + RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
> +
> + G_OBJECT_CLASS(inputs_channel_parent_class)->constructed(object);
> +
> + client_cbs.connect = inputs_connect;
> + client_cbs.migrate = inputs_migrate;
> + red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs, NULL);
> +
> + red_channel_set_cap(RED_CHANNEL(self), SPICE_INPUTS_CAP_KEY_SCANCODE);
> + reds_register_channel(reds, RED_CHANNEL(self));
> +
> + if (!(self->priv->key_modifiers_timer = reds_core_timer_add(reds,
> key_modifiers_sender, self))) {
> + spice_error("key modifiers timer create failed");
> + }
> +}
> +
> +static void
> +inputs_channel_init(InputsChannel *self)
> +{
> + self->priv = CHANNEL_PRIVATE(self);
> +}
> +
> struct SpiceKbdState {
> uint8_t push_ext_type;
>
> @@ -105,18 +182,6 @@ RedsState*
> spice_tablet_state_get_server(SpiceTabletState *st)
> return st->reds;
> }
>
> -struct InputsChannel {
> - RedChannel base;
> - uint8_t recv_buf[RECEIVE_BUF_SIZE];
> - VDAgentMouseState mouse_state;
> - int src_during_migrate;
> - SpiceTimer *key_modifiers_timer;
> -
> - SpiceKbdInstance *keyboard;
> - SpiceMouseInstance *mouse;
> - SpiceTabletInstance *tablet;
> -};
> -
Couldn't we move new function after this declaration to reduce patch size?
> typedef struct RedKeyModifiersPipeItem {
> RedPipeItem base;
> uint8_t modifiers;
... omissis ...
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel