[Spice-devel,spice-server,11/18] sound: Convert SndChannelClient to GObject

Submitted by Frediano Ziglio on Nov. 23, 2016, 6:07 p.m.

Details

Message ID 1479924452-15913-12-git-send-email-fziglio@redhat.com
State Superseded
Headers show
Series "Remove DummyChannel* objects" ( rev: 3 2 1 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio Nov. 23, 2016, 6:07 p.m.
This patch is quite huge but have some benefits:
- reduce dependency (DummyChannel* and some RedChannelClient internals);
- reuse RedChannelClient instead of reading from the RedsStream
  directly.

You can see that SndChannelClient has much less field
as the code to read/write from/to client is reused from
RedChannelClient instead of creating a fake RedChannelClient
just to make the system happy.

One of the different between the old sound code and all other
RedChannelClient objects was that the sound channel don't use
a queue while RedChannelClient use RedPipeItem object. This was
the main reason why RedChannelClient was not used. To implement
the old behaviour a "persistent_pipe_item" is used. This RedPipeItem
will be queued to RedChannelClient (only one!) so signal code we
have data to send. The {playback,record}_channel_send_item will
then send the messages to the client using RedChannelClient functions.
For this reason snd_reset_send_data is replaced by a call to
red_channel_client_init_send_data and snd_begin_send_message is
replaced by red_channel_client_begin_send_message.

Another difference is the AudioFrame allocation. Previously these
frames were allocated inside PlaybackChannelClient while now they
are allocated in a different structure. This allows the samples
to outlive the PlaybackChannelClient. This was possible even before
and the client was kept alive. However having RedChannelClient as
as base class can be an issue as this will cause the entire
RedChannel and RedChannelClient to stay alive. To avoid this
potential problem allocate the frames in a different structure
keeping a reference from PlaybackChannelClient. When the client
is freed the AudioFrameContainer is just unreferences.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/sound.c | 1100 +++++++++++++++++++++++++-------------------------------
 1 file changed, 487 insertions(+), 613 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/sound.c b/server/sound.c
index ac11106..0c2a3ae 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -32,14 +32,10 @@ 
 
 #include "spice.h"
 #include "red-common.h"
-#include "dummy-channel.h"
-#include "dummy-channel-client.h"
 #include "main-channel.h"
 #include "reds.h"
 #include "red-qxl.h"
 #include "red-channel-client.h"
-/* FIXME: for now, allow sound channel access to private RedChannelClient data */
-#include "red-channel-client-private.h"
 #include "red-client.h"
 #include "sound.h"
 #include "demarshallers.h"
@@ -56,6 +52,7 @@  enum SndCommand {
     SND_MIGRATE,
     SND_CTRL,
     SND_VOLUME,
+    SND_MUTE,
     SND_END_COMMAND,
 };
 
@@ -68,67 +65,79 @@  enum PlaybackCommand {
 #define SND_MIGRATE_MASK (1 << SND_MIGRATE)
 #define SND_CTRL_MASK (1 << SND_CTRL)
 #define SND_VOLUME_MASK (1 << SND_VOLUME)
+#define SND_MUTE_MASK (1 << SND_MUTE)
+#define SND_VOLUME_MUTE_MASK (SND_VOLUME_MASK|SND_MUTE_MASK)
 
 #define SND_PLAYBACK_MODE_MASK (1 << SND_PLAYBACK_MODE)
 #define SND_PLAYBACK_PCM_MASK (1 << SND_PLAYBACK_PCM)
 #define SND_PLAYBACK_LATENCY_MASK ( 1 << SND_PLAYBACK_LATENCY)
 
 typedef struct SndChannelClient SndChannelClient;
-typedef void (*snd_channel_send_messages_proc)(void *in_channel);
-typedef int (*snd_channel_handle_message_proc)(SndChannelClient *client, size_t size, uint32_t type, void *message);
+typedef struct SndChannel SndChannel;
+typedef struct PlaybackChannelClient PlaybackChannelClient;
+typedef struct RecordChannelClient RecordChannelClient;
+typedef struct AudioFrame AudioFrame;
+typedef struct AudioFrameContainer AudioFrameContainer;
+typedef struct SpicePlaybackState PlaybackChannel;
+typedef struct SpiceRecordState RecordChannel;
+
 typedef void (*snd_channel_on_message_done_proc)(SndChannelClient *client);
-typedef void (*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
 
-typedef struct SndChannel SndChannel;
+
+#define TYPE_SND_CHANNEL_CLIENT snd_channel_client_get_type()
+#define SND_CHANNEL_CLIENT(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_SND_CHANNEL_CLIENT, SndChannelClient))
+GType snd_channel_client_get_type(void) G_GNUC_CONST;
 
 /* Connects an audio client to a Spice client */
 struct SndChannelClient {
-    RedsStream *stream;
-    SndChannel *channel;
-    spice_parse_channel_func_t parser;
-    int refs;
-
-    RedChannelClient *channel_client;
+    RedChannelClient parent;
 
     int active;
     int client_active;
-    int blocked;
 
     uint32_t command;
 
-    struct {
-        uint64_t serial;
-        SpiceMarshaller *marshaller;
-        uint32_t size;
-        uint32_t pos;
-    } send_data;
-
-    struct {
-        uint8_t buf[SND_RECEIVE_BUF_SIZE];
-        uint8_t *message_start;
-        uint8_t *now;
-        uint8_t *end;
-    } receive_data;
-
-    snd_channel_send_messages_proc send_messages;
-    snd_channel_handle_message_proc handle_message;
+    uint8_t receive_buf[SND_RECEIVE_BUF_SIZE];
+    RedPipeItem persistent_pipe_item;
+
     snd_channel_on_message_done_proc on_message_done;
-    snd_channel_cleanup_channel_proc cleanup;
 };
 
-typedef struct PlaybackChannelClient PlaybackChannelClient;
+typedef RedChannelClientClass SndChannelClientClass;
+
+G_DEFINE_TYPE(SndChannelClient, snd_channel_client, RED_TYPE_CHANNEL_CLIENT)
+
+
+enum {
+    RED_PIPE_ITEM_PERSISTENT = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
+};
+
 
-typedef struct AudioFrame AudioFrame;
 struct AudioFrame {
     uint32_t time;
     uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
     PlaybackChannelClient *client;
     AudioFrame *next;
+    AudioFrameContainer *container;
+    gboolean allocated;
+};
+
+#define NUM_AUDIO_FRAMES 3
+struct AudioFrameContainer
+{
+    int refs;
+    AudioFrame items[NUM_AUDIO_FRAMES];
 };
 
+#define TYPE_PLAYBACK_CHANNEL_CLIENT playback_channel_client_get_type()
+#define PLAYBACK_CHANNEL_CLIENT(obj) \
+    (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_PLAYBACK_CHANNEL_CLIENT, PlaybackChannelClient))
+GType playback_channel_client_get_type(void) G_GNUC_CONST;
+
 struct PlaybackChannelClient {
-    SndChannelClient base;
-    AudioFrame frames[3];
+    SndChannelClient parent;
+
+    AudioFrameContainer *frames;
     AudioFrame *free_frames;
     AudioFrame *in_progress;   /* Frame being sent to the client */
     AudioFrame *pending_frame; /* Next frame to send to the client */
@@ -138,6 +147,11 @@  struct PlaybackChannelClient {
     uint8_t  encode_buf[SND_CODEC_MAX_COMPRESSED_BYTES];
 };
 
+typedef SndChannelClientClass PlaybackChannelClientClass;
+
+G_DEFINE_TYPE(PlaybackChannelClient, playback_channel_client, TYPE_SND_CHANNEL_CLIENT)
+
+
 typedef struct SpiceVolumeState {
     uint16_t *volume;
     uint8_t volume_nchannels;
@@ -165,9 +179,6 @@  typedef RedChannelClass SndChannelClass;
 G_DEFINE_TYPE(SndChannel, snd_channel, RED_TYPE_CHANNEL)
 
 
-typedef struct SpicePlaybackState PlaybackChannel;
-typedef SndChannelClass PlaybackChannelClass;
-
 #define TYPE_PLAYBACK_CHANNEL playback_channel_get_type()
 #define PLAYBACK_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_PLAYBACK_CHANNEL, PlaybackChannel))
 GType playback_channel_get_type(void) G_GNUC_CONST;
@@ -176,12 +187,11 @@  struct SpicePlaybackState {
     SndChannel channel;
 };
 
+typedef SndChannelClass PlaybackChannelClass;
+
 G_DEFINE_TYPE(PlaybackChannel, playback_channel, TYPE_SND_CHANNEL)
 
 
-typedef struct SpiceRecordState RecordChannel;
-typedef SndChannelClass RecordChannelClass;
-
 #define TYPE_RECORD_CHANNEL record_channel_get_type()
 #define RECORD_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_RECORD_CHANNEL, RecordChannel))
 GType record_channel_get_type(void) G_GNUC_CONST;
@@ -190,11 +200,18 @@  struct SpiceRecordState {
     SndChannel channel;
 };
 
+typedef SndChannelClass RecordChannelClass;
+
 G_DEFINE_TYPE(RecordChannel, record_channel, TYPE_SND_CHANNEL)
 
 
-typedef struct RecordChannelClient {
-    SndChannelClient base;
+#define TYPE_RECORD_CHANNEL_CLIENT record_channel_client_get_type()
+#define RECORD_CHANNEL_CLIENT(obj) \
+    (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_RECORD_CHANNEL_CLIENT, RecordChannelClient))
+GType record_channel_client_get_type(void) G_GNUC_CONST;
+
+struct RecordChannelClient {
+    SndChannelClient parent;
     uint32_t samples[RECORD_SAMPLES_SIZE];
     uint32_t write_pos;
     uint32_t read_pos;
@@ -203,64 +220,23 @@  typedef struct RecordChannelClient {
     uint32_t start_time;
     SndCodec codec;
     uint8_t  decode_buf[SND_CODEC_MAX_FRAME_BYTES];
-} RecordChannelClient;
+};
+
+typedef SndChannelClientClass RecordChannelClientClass;
+
+G_DEFINE_TYPE(RecordChannelClient, record_channel_client, TYPE_SND_CHANNEL_CLIENT)
+
 
 /* A list of all Spice{Playback,Record}State objects */
 static SndChannel *snd_channels;
 
-static void snd_receive(SndChannelClient *client);
 static void snd_playback_start(SndChannel *channel);
 static void snd_record_start(SndChannel *channel);
 
-static SndChannelClient *snd_channel_ref(SndChannelClient *client)
-{
-    client->refs++;
-    return client;
-}
-
-static SndChannelClient *snd_channel_unref(SndChannelClient *client)
-{
-    if (!--client->refs) {
-        spice_printerr("SndChannelClient=%p freed", client);
-        free(client);
-        return NULL;
-    }
-    return client;
-}
-
 static RedsState* snd_channel_get_server(SndChannelClient *client)
 {
     g_return_val_if_fail(client != NULL, NULL);
-    return red_channel_get_server(RED_CHANNEL(client->channel));
-}
-
-static void snd_disconnect_channel(SndChannelClient *client)
-{
-    SndChannel *channel;
-    RedsState *reds;
-    RedChannel *red_channel;
-    uint32_t type;
-
-    if (!client || !client->stream) {
-        spice_debug("not connected");
-        return;
-    }
-    red_channel = red_channel_client_get_channel(client->channel_client);
-    reds = snd_channel_get_server(client);
-    g_object_get(red_channel, "channel-type", &type, NULL);
-    spice_debug("SndChannelClient=%p rcc=%p type=%d",
-                 client, client->channel_client, type);
-    channel = client->channel;
-    client->cleanup(client);
-    red_channel_client_disconnect(channel->connection->channel_client);
-    channel->connection->channel_client = NULL;
-    reds_core_watch_remove(reds, client->stream->watch);
-    client->stream->watch = NULL;
-    reds_stream_free(client->stream);
-    client->stream = NULL;
-    spice_marshaller_destroy(client->send_data.marshaller);
-    snd_channel_unref(client);
-    channel->connection = NULL;
+    return red_channel_get_server(red_channel_client_get_channel(RED_CHANNEL_CLIENT(client)));
 }
 
 static void snd_playback_free_frame(PlaybackChannelClient *playback_client, AudioFrame *frame)
@@ -282,65 +258,6 @@  static void snd_playback_on_message_done(SndChannelClient *client)
     }
 }
 
-static void snd_record_on_message_done(SndChannelClient *client)
-{
-}
-
-static int snd_send_data(SndChannelClient *client)
-{
-    uint32_t n;
-
-    if (!client) {
-        return FALSE;
-    }
-
-    if (!(n = client->send_data.size - client->send_data.pos)) {
-        return TRUE;
-    }
-
-    RedsState *reds = snd_channel_get_server(client);
-    for (;;) {
-        struct iovec vec[IOV_MAX];
-        int vec_size;
-
-        if (!n) {
-            client->on_message_done(client);
-
-            if (client->blocked) {
-                client->blocked = FALSE;
-                reds_core_watch_update_mask(reds, client->stream->watch, SPICE_WATCH_EVENT_READ);
-            }
-            break;
-        }
-
-        vec_size = spice_marshaller_fill_iovec(client->send_data.marshaller,
-                                               vec, IOV_MAX, client->send_data.pos);
-        n = reds_stream_writev(client->stream, vec, vec_size);
-        if (n == -1) {
-            switch (errno) {
-            case EAGAIN:
-                client->blocked = TRUE;
-                reds_core_watch_update_mask(reds, client->stream->watch, SPICE_WATCH_EVENT_READ |
-                                                                 SPICE_WATCH_EVENT_WRITE);
-                return FALSE;
-            case EINTR:
-                break;
-            case EPIPE:
-                snd_disconnect_channel(client);
-                return FALSE;
-            default:
-                spice_printerr("%s", strerror(errno));
-                snd_disconnect_channel(client);
-                return FALSE;
-            }
-        } else {
-            client->send_data.pos += n;
-        }
-        n = client->send_data.size - client->send_data.pos;
-    }
-    return TRUE;
-}
-
 static int snd_record_handle_write(RecordChannelClient *record_client, size_t size, void *message)
 {
     SpiceMsgcRecordPacket *packet;
@@ -386,35 +303,29 @@  static int snd_record_handle_write(RecordChannelClient *record_client, size_t si
     return TRUE;
 }
 
-static int snd_playback_handle_message(SndChannelClient *client, size_t size, uint32_t type, void *message)
+static int
+playback_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type, void *message)
 {
-    if (!client) {
-        return FALSE;
-    }
-
     switch (type) {
     case SPICE_MSGC_DISCONNECTING:
         break;
     default:
-        spice_printerr("invalid message type %u", type);
-        return FALSE;
+        return red_channel_client_handle_message(rcc, size, type, message);
     }
     return TRUE;
 }
 
-static int snd_record_handle_message(SndChannelClient *client, size_t size, uint32_t type, void *message)
+static int
+record_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type, void *message)
 {
-    RecordChannelClient *record_client = (RecordChannelClient *)client;
+    RecordChannelClient *record_client = (RecordChannelClient *)rcc; // FIXME
 
-    if (!client) {
-        return FALSE;
-    }
     switch (type) {
     case SPICE_MSGC_RECORD_DATA:
-        return snd_record_handle_write((RecordChannelClient *)client, size, message);
+        return snd_record_handle_write(record_client, size, message);
     case SPICE_MSGC_RECORD_MODE: {
         SpiceMsgcRecordMode *mode = (SpiceMsgcRecordMode *)message;
-        SndChannel *channel = client->channel;
+        SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
         record_client->mode_time = mode->time;
         if (mode->mode != SPICE_AUDIO_DATA_MODE_RAW) {
             if (snd_codec_is_capable(mode->mode, channel->frequency)) {
@@ -444,260 +355,134 @@  static int snd_record_handle_message(SndChannelClient *client, size_t size, uint
     case SPICE_MSGC_DISCONNECTING:
         break;
     default:
-        spice_printerr("invalid message type %u", type);
-        return FALSE;
-    }
-    return TRUE;
-}
-
-static void snd_receive(SndChannelClient *client)
-{
-    SpiceDataHeaderOpaque *header;
-
-    if (!client) {
-        return;
-    }
-
-    header = &client->channel_client->incoming.header;
-
-    for (;;) {
-        ssize_t n;
-        n = client->receive_data.end - client->receive_data.now;
-        spice_warn_if_fail(n > 0);
-        n = reds_stream_read(client->stream, client->receive_data.now, n);
-        if (n <= 0) {
-            if (n == 0) {
-                snd_disconnect_channel(client);
-                return;
-            }
-            spice_assert(n == -1);
-            switch (errno) {
-            case EAGAIN:
-                return;
-            case EINTR:
-                break;
-            case EPIPE:
-                snd_disconnect_channel(client);
-                return;
-            default:
-                spice_printerr("%s", strerror(errno));
-                snd_disconnect_channel(client);
-                return;
-            }
-        } else {
-            client->receive_data.now += n;
-            for (;;) {
-                uint8_t *msg_start = client->receive_data.message_start;
-                uint8_t *data = msg_start + header->header_size;
-                size_t parsed_size;
-                uint8_t *parsed;
-                message_destructor_t parsed_free;
-
-                header->data = msg_start;
-                n = client->receive_data.now - msg_start;
-
-                if (n < header->header_size ||
-                    n < header->header_size + header->get_msg_size(header)) {
-                    break;
-                }
-                parsed = client->parser((void *)data, data + header->get_msg_size(header),
-                                         header->get_msg_type(header),
-                                         SPICE_VERSION_MINOR, &parsed_size, &parsed_free);
-                if (parsed == NULL) {
-                    spice_printerr("failed to parse message type %d", header->get_msg_type(header));
-                    snd_disconnect_channel(client);
-                    return;
-                }
-                if (!client->handle_message(client, parsed_size,
-                                             header->get_msg_type(header), parsed)) {
-                    free(parsed);
-                    snd_disconnect_channel(client);
-                    return;
-                }
-                parsed_free(parsed);
-                client->receive_data.message_start = msg_start + header->header_size +
-                                                     header->get_msg_size(header);
-            }
-            if (client->receive_data.now == client->receive_data.message_start) {
-                client->receive_data.now = client->receive_data.buf;
-                client->receive_data.message_start = client->receive_data.buf;
-            } else if (client->receive_data.now == client->receive_data.end) {
-                memcpy(client->receive_data.buf, client->receive_data.message_start, n);
-                client->receive_data.now = client->receive_data.buf + n;
-                client->receive_data.message_start = client->receive_data.buf;
-            }
-        }
-    }
-}
-
-static void snd_event(int fd, int event, void *data)
-{
-    SndChannelClient *client = data;
-
-    if (event & SPICE_WATCH_EVENT_READ) {
-        snd_receive(client);
-    }
-    if (event & SPICE_WATCH_EVENT_WRITE) {
-        client->send_messages(client);
+        return red_channel_client_handle_message(rcc, size, type, message);
     }
-}
-
-static inline int snd_reset_send_data(SndChannelClient *client, uint16_t verb)
-{
-    SpiceDataHeaderOpaque *header;
-
-    if (!client) {
-        return FALSE;
-    }
-
-    header = &client->channel_client->priv->send_data.header;
-    spice_marshaller_reset(client->send_data.marshaller);
-    header->data = spice_marshaller_reserve_space(client->send_data.marshaller,
-                                                  header->header_size);
-    spice_marshaller_set_base(client->send_data.marshaller,
-                              header->header_size);
-    client->send_data.pos = 0;
-    header->set_msg_size(header, 0);
-    header->set_msg_type(header, verb);
-    client->send_data.serial++;
-    if (!client->channel_client->priv->is_mini_header) {
-        header->set_msg_serial(header, client->send_data.serial);
-        header->set_msg_sub_list(header, 0);
-    }
-
     return TRUE;
 }
 
-static int snd_begin_send_message(SndChannelClient *client)
-{
-    SpiceDataHeaderOpaque *header = &client->channel_client->priv->send_data.header;
-
-    spice_marshaller_flush(client->send_data.marshaller);
-    client->send_data.size = spice_marshaller_get_total_size(client->send_data.marshaller);
-    header->set_msg_size(header, client->send_data.size - header->header_size);
-    return snd_send_data(client);
-}
-
 static int snd_channel_send_migrate(SndChannelClient *client)
 {
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
     SpiceMsgMigrate migrate;
 
-    if (!snd_reset_send_data(client, SPICE_MSG_MIGRATE)) {
-        return FALSE;
-    }
-    spice_debug(NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE, NULL);
     migrate.flags = 0;
-    spice_marshall_msg_migrate(client->send_data.marshaller, &migrate);
+    spice_marshall_msg_migrate(m, &migrate);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int snd_playback_send_migrate(PlaybackChannelClient *client)
 {
-    return snd_channel_send_migrate(&client->base);
+    return snd_channel_send_migrate(SND_CHANNEL_CLIENT(client));
 }
 
 static int snd_send_volume(SndChannelClient *client, uint32_t cap, int msg)
 {
     SpiceMsgAudioVolume *vol;
     uint8_t c;
-    SpiceVolumeState *st = &client->channel->volume;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
+    SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
+    SpiceVolumeState *st = &channel->volume;
 
-    if (!red_channel_client_test_remote_cap(client->channel_client, cap)) {
+    if (!red_channel_client_test_remote_cap(rcc, cap)) {
         return TRUE;
     }
 
     vol = alloca(sizeof (SpiceMsgAudioVolume) +
                  st->volume_nchannels * sizeof (uint16_t));
-    if (!snd_reset_send_data(client, msg)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, msg, NULL);
     vol->nchannels = st->volume_nchannels;
     for (c = 0; c < st->volume_nchannels; ++c) {
         vol->volume[c] = st->volume[c];
     }
-    spice_marshall_SpiceMsgAudioVolume(client->send_data.marshaller, vol);
+    spice_marshall_SpiceMsgAudioVolume(m, vol);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int snd_playback_send_volume(PlaybackChannelClient *playback_client)
 {
-    return snd_send_volume(&playback_client->base, SPICE_PLAYBACK_CAP_VOLUME,
+    return snd_send_volume(SND_CHANNEL_CLIENT(playback_client), SPICE_PLAYBACK_CAP_VOLUME,
                            SPICE_MSG_PLAYBACK_VOLUME);
 }
 
 static int snd_send_mute(SndChannelClient *client, uint32_t cap, int msg)
 {
     SpiceMsgAudioMute mute;
-    SpiceVolumeState *st = &client->channel->volume;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
+    SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
+    SpiceVolumeState *st = &channel->volume;
 
-    if (!red_channel_client_test_remote_cap(client->channel_client, cap)) {
+    if (!red_channel_client_test_remote_cap(rcc, cap)) {
         return TRUE;
     }
 
-    if (!snd_reset_send_data(client, msg)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, msg, NULL);
     mute.mute = st->mute;
-    spice_marshall_SpiceMsgAudioMute(client->send_data.marshaller, &mute);
+    spice_marshall_SpiceMsgAudioMute(m, &mute);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int snd_playback_send_mute(PlaybackChannelClient *playback_client)
 {
-    return snd_send_mute(&playback_client->base, SPICE_PLAYBACK_CAP_VOLUME,
+    return snd_send_mute(SND_CHANNEL_CLIENT(playback_client), SPICE_PLAYBACK_CAP_VOLUME,
                          SPICE_MSG_PLAYBACK_MUTE);
 }
 
 static int snd_playback_send_latency(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = &playback_client->base;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
     SpiceMsgPlaybackLatency latency_msg;
 
     spice_debug("latency %u", playback_client->latency);
-    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_LATENCY)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_LATENCY, NULL);
     latency_msg.latency_ms = playback_client->latency;
-    spice_marshall_msg_playback_latency(client->send_data.marshaller, &latency_msg);
+    spice_marshall_msg_playback_latency(m, &latency_msg);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
+
 static int snd_playback_send_start(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = (SndChannelClient *)playback_client;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
     SpiceMsgPlaybackStart start;
 
-    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_START)) {
-        return FALSE;
-    }
-
+    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_START, NULL);
     start.channels = SPICE_INTERFACE_PLAYBACK_CHAN;
-    start.frequency = client->channel->frequency;
+    start.frequency = SND_CHANNEL(red_channel_client_get_channel(rcc))->frequency;
     spice_assert(SPICE_INTERFACE_PLAYBACK_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
     start.format = SPICE_AUDIO_FMT_S16;
     start.time = reds_get_mm_time();
-    spice_marshall_msg_playback_start(client->send_data.marshaller, &start);
+    spice_marshall_msg_playback_start(m, &start);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int snd_playback_send_stop(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = (SndChannelClient *)playback_client;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
 
-    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_STOP)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_STOP, NULL);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int snd_playback_send_ctl(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = (SndChannelClient *)playback_client;
+    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
 
     if ((client->client_active = client->active)) {
         return snd_playback_send_start(playback_client);
@@ -708,36 +493,35 @@  static int snd_playback_send_ctl(PlaybackChannelClient *playback_client)
 
 static int snd_record_send_start(RecordChannelClient *record_client)
 {
-    SndChannelClient *client = (SndChannelClient *)record_client;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(record_client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
     SpiceMsgRecordStart start;
 
-    if (!snd_reset_send_data(client, SPICE_MSG_RECORD_START)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, SPICE_MSG_RECORD_START, NULL);
 
     start.channels = SPICE_INTERFACE_RECORD_CHAN;
-    start.frequency = client->channel->frequency;
+    start.frequency = SND_CHANNEL(red_channel_client_get_channel(rcc))->frequency;
     spice_assert(SPICE_INTERFACE_RECORD_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
     start.format = SPICE_AUDIO_FMT_S16;
-    spice_marshall_msg_record_start(client->send_data.marshaller, &start);
+    spice_marshall_msg_record_start(m, &start);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int snd_record_send_stop(RecordChannelClient *record_client)
 {
-    SndChannelClient *client = (SndChannelClient *)record_client;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(record_client);
 
-    if (!snd_reset_send_data(client, SPICE_MSG_RECORD_STOP)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, SPICE_MSG_RECORD_STOP, NULL);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int snd_record_send_ctl(RecordChannelClient *record_client)
 {
-    SndChannelClient *client = (SndChannelClient *)record_client;
+    SndChannelClient *client = SND_CHANNEL_CLIENT(record_client);
 
     if ((client->client_active = client->active)) {
         return snd_record_send_start(record_client);
@@ -748,13 +532,13 @@  static int snd_record_send_ctl(RecordChannelClient *record_client)
 
 static int snd_record_send_volume(RecordChannelClient *record_client)
 {
-    return snd_send_volume(&record_client->base, SPICE_RECORD_CAP_VOLUME,
+    return snd_send_volume(SND_CHANNEL_CLIENT(record_client), SPICE_RECORD_CAP_VOLUME,
                            SPICE_MSG_RECORD_VOLUME);
 }
 
 static int snd_record_send_mute(RecordChannelClient *record_client)
 {
-    return snd_send_mute(&record_client->base, SPICE_RECORD_CAP_VOLUME,
+    return snd_send_mute(SND_CHANNEL_CLIENT(record_client), SPICE_RECORD_CAP_VOLUME,
                          SPICE_MSG_RECORD_MUTE);
 }
 
@@ -764,27 +548,25 @@  static int snd_record_send_migrate(RecordChannelClient *record_client)
      * the client receives RECORD_STOP from the src before the migration completion
      * notification (when the vm is stopped).
      * Afterwards, when the vm starts on the dest, the client receives RECORD_START. */
-    return snd_channel_send_migrate(&record_client->base);
+    return snd_channel_send_migrate(SND_CHANNEL_CLIENT(record_client));
 }
 
 static int snd_playback_send_write(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = (SndChannelClient *)playback_client;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
     AudioFrame *frame;
     SpiceMsgPlaybackPacket msg;
 
-    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_DATA)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_DATA, NULL);
 
     frame = playback_client->in_progress;
     msg.time = frame->time;
 
-    spice_marshall_msg_playback_data(client->send_data.marshaller, &msg);
+    spice_marshall_msg_playback_data(m, &msg);
 
     if (playback_client->mode == SPICE_AUDIO_DATA_MODE_RAW) {
-        spice_marshaller_add_ref(client->send_data.marshaller,
-                                 (uint8_t *)frame->samples,
+        spice_marshaller_add_ref(m, (uint8_t *)frame->samples,
                                  snd_codec_frame_size(playback_client->codec) * sizeof(frame->samples[0]));
     }
     else {
@@ -793,45 +575,70 @@  static int snd_playback_send_write(PlaybackChannelClient *playback_client)
                                     snd_codec_frame_size(playback_client->codec) * sizeof(frame->samples[0]),
                                     playback_client->encode_buf, &n) != SND_CODEC_OK) {
             spice_printerr("encode failed");
-            snd_disconnect_channel(client);
-            return FALSE;
+            red_channel_client_disconnect(rcc);
+            return TRUE;
         }
-        spice_marshaller_add_ref(client->send_data.marshaller, playback_client->encode_buf, n);
+        spice_marshaller_add_ref(m, playback_client->encode_buf, n);
     }
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
 static int playback_send_mode(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = (SndChannelClient *)playback_client;
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
+    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
     SpiceMsgPlaybackMode mode;
 
-    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_MODE)) {
-        return FALSE;
-    }
+    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_MODE, NULL);
     mode.time = reds_get_mm_time();
     mode.mode = playback_client->mode;
-    spice_marshall_msg_playback_mode(client->send_data.marshaller, &mode);
+    spice_marshall_msg_playback_mode(m, &mode);
 
-    return snd_begin_send_message(client);
+    red_channel_client_begin_send_message(rcc);
+    return FALSE;
 }
 
-static void snd_playback_send(void* data)
+static void snd_persistent_pipe_item_free(struct RedPipeItem *item)
 {
-    PlaybackChannelClient *playback_client = (PlaybackChannelClient*)data;
-    SndChannelClient *client = (SndChannelClient*)playback_client;
+    SndChannelClient *client = SPICE_CONTAINEROF(item, SndChannelClient, persistent_pipe_item);
+
+    red_pipe_item_init_full(item, RED_PIPE_ITEM_PERSISTENT,
+                            snd_persistent_pipe_item_free);
+
+    if (client->on_message_done) {
+        client->on_message_done(client);
+    }
+}
 
-    if (!playback_client || !snd_send_data(data)) {
+static void snd_send(SndChannelClient * client)
+{
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
+
+    if (!client || !red_channel_client_pipe_is_empty(rcc) || !client->command) {
         return;
     }
+    // just append a dummy item and push!
+    red_pipe_item_init_full(&client->persistent_pipe_item, RED_PIPE_ITEM_PERSISTENT,
+                            snd_persistent_pipe_item_free);
+    red_channel_client_pipe_add_push(rcc, &client->persistent_pipe_item);
+}
 
+static void playback_channel_send_item(RedChannelClient *rcc, RedPipeItem *base)
+{
+    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(rcc);
+    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
+
+    client->command &= SND_PLAYBACK_MODE_MASK|SND_PLAYBACK_PCM_MASK|
+                       SND_CTRL_MASK|SND_VOLUME_MUTE_MASK|
+                       SND_MIGRATE_MASK|SND_PLAYBACK_LATENCY_MASK;
     while (client->command) {
         if (client->command & SND_PLAYBACK_MODE_MASK) {
+            client->command &= ~SND_PLAYBACK_MODE_MASK;
             if (!playback_send_mode(playback_client)) {
-                return;
+                break;
             }
-            client->command &= ~SND_PLAYBACK_MODE_MASK;
         }
         if (client->command & SND_PLAYBACK_PCM_MASK) {
             spice_assert(!playback_client->in_progress && playback_client->pending_frame);
@@ -839,163 +646,77 @@  static void snd_playback_send(void* data)
             playback_client->pending_frame = NULL;
             client->command &= ~SND_PLAYBACK_PCM_MASK;
             if (!snd_playback_send_write(playback_client)) {
-                spice_printerr("snd_send_playback_write failed");
-                return;
+                break;
             }
+            spice_printerr("snd_send_playback_write failed");
         }
         if (client->command & SND_CTRL_MASK) {
+            client->command &= ~SND_CTRL_MASK;
             if (!snd_playback_send_ctl(playback_client)) {
-                return;
+                break;
             }
-            client->command &= ~SND_CTRL_MASK;
         }
         if (client->command & SND_VOLUME_MASK) {
-            if (!snd_playback_send_volume(playback_client) ||
-                !snd_playback_send_mute(playback_client)) {
-                return;
-            }
             client->command &= ~SND_VOLUME_MASK;
+            if (!snd_playback_send_volume(playback_client)) {
+                break;
+            }
+        }
+        if (client->command & SND_MUTE_MASK) {
+            client->command &= ~SND_MUTE_MASK;
+            if (!snd_playback_send_mute(playback_client)) {
+                break;
+            }
         }
         if (client->command & SND_MIGRATE_MASK) {
+            client->command &= ~SND_MIGRATE_MASK;
             if (!snd_playback_send_migrate(playback_client)) {
-                return;
+                break;
             }
-            client->command &= ~SND_MIGRATE_MASK;
         }
         if (client->command & SND_PLAYBACK_LATENCY_MASK) {
+            client->command &= ~SND_PLAYBACK_LATENCY_MASK;
             if (!snd_playback_send_latency(playback_client)) {
-                return;
+                break;
             }
-            client->command &= ~SND_PLAYBACK_LATENCY_MASK;
         }
     }
+    snd_send(client);
 }
 
-static void snd_record_send(void* data)
+static void record_channel_send_item(RedChannelClient *rcc, RedPipeItem *base)
 {
-    RecordChannelClient *record_client = (RecordChannelClient*)data;
-    SndChannelClient *client = (SndChannelClient*)record_client;
-
-    if (!record_client || !snd_send_data(data)) {
-        return;
-    }
+    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(rcc);
+    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
 
+    client->command &= SND_CTRL_MASK|SND_VOLUME_MUTE_MASK|SND_MIGRATE_MASK;
     while (client->command) {
         if (client->command & SND_CTRL_MASK) {
+            client->command &= ~SND_CTRL_MASK;
             if (!snd_record_send_ctl(record_client)) {
-                return;
+                break;
             }
-            client->command &= ~SND_CTRL_MASK;
         }
         if (client->command & SND_VOLUME_MASK) {
-            if (!snd_record_send_volume(record_client) ||
-                !snd_record_send_mute(record_client)) {
-                return;
-            }
             client->command &= ~SND_VOLUME_MASK;
+            if (!snd_record_send_volume(record_client)) {
+                break;
+            }
+        }
+        if (client->command & SND_MUTE_MASK) {
+            client->command &= ~SND_MUTE_MASK;
+            if (!snd_record_send_mute(record_client)) {
+                break;
+            }
         }
         if (client->command & SND_MIGRATE_MASK) {
+            client->command &= ~SND_MIGRATE_MASK;
             if (!snd_record_send_migrate(record_client)) {
-                return;
+                break;
             }
-            client->command &= ~SND_MIGRATE_MASK;
-        }
-    }
-}
-
-static SndChannelClient *__new_channel(SndChannel *channel, int size, uint32_t channel_id,
-                                 RedClient *red_client,
-                                 RedsStream *stream,
-                                 int migrate,
-                                 snd_channel_send_messages_proc send_messages,
-                                 snd_channel_handle_message_proc handle_message,
-                                 snd_channel_on_message_done_proc on_message_done,
-                                 snd_channel_cleanup_channel_proc cleanup,
-                                 uint32_t *common_caps, int num_common_caps,
-                                 uint32_t *caps, int num_caps)
-{
-    SndChannelClient *client;
-    int delay_val;
-    int flags;
-#ifdef SO_PRIORITY
-    int priority;
-#endif
-    int tos;
-    MainChannelClient *mcc = red_client_get_main(red_client);
-    RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
-
-    spice_assert(stream);
-    if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
-        spice_printerr("accept failed, %s", strerror(errno));
-        goto error1;
-    }
-
-#ifdef SO_PRIORITY
-    priority = 6;
-    if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY, (void*)&priority,
-                   sizeof(priority)) == -1) {
-        if (errno != ENOTSUP) {
-            spice_printerr("setsockopt failed, %s", strerror(errno));
         }
     }
-#endif
-
-    tos = IPTOS_LOWDELAY;
-    if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*)&tos, sizeof(tos)) == -1) {
-        if (errno != ENOTSUP) {
-            spice_printerr("setsockopt failed, %s", strerror(errno));
-        }
-    }
-
-    delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
-    if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val, sizeof(delay_val)) == -1) {
-        if (errno != ENOTSUP) {
-            spice_printerr("setsockopt failed, %s", strerror(errno));
-        }
-    }
-
-    if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
-        spice_printerr("accept failed, %s", strerror(errno));
-        goto error1;
-    }
-
-    spice_assert(size >= sizeof(*client));
-    client = spice_malloc0(size);
-    client->refs = 1;
-    client->parser = spice_get_client_channel_parser(channel_id, NULL);
-    client->stream = stream;
-    client->channel = channel;
-    client->receive_data.message_start = client->receive_data.buf;
-    client->receive_data.now = client->receive_data.buf;
-    client->receive_data.end = client->receive_data.buf + sizeof(client->receive_data.buf);
-    client->send_data.marshaller = spice_marshaller_new();
-
-    stream->watch = reds_core_watch_add(reds, stream->socket, SPICE_WATCH_EVENT_READ,
-                                        snd_event, client);
-    if (stream->watch == NULL) {
-        spice_printerr("watch_add failed, %s", strerror(errno));
-        goto error2;
-    }
-
-    client->send_messages = send_messages;
-    client->handle_message = handle_message;
-    client->on_message_done = on_message_done;
-    client->cleanup = cleanup;
-
-    client->channel_client =
-        dummy_channel_client_create(RED_CHANNEL(channel), red_client,
-                                    num_common_caps, common_caps, num_caps, caps);
-    if (!client->channel_client) {
-        goto error2;
-    }
-    return client;
-
-error2:
-    free(client);
-
-error1:
-    reds_stream_free(stream);
-    return NULL;
+    snd_send(client);
 }
 
 static int snd_channel_config_socket(RedChannelClient *rcc)
@@ -1047,6 +768,22 @@  static int snd_channel_config_socket(RedChannelClient *rcc)
     return TRUE;
 }
 
+static uint8_t*
+snd_channel_client_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size)
+{
+    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
+    // TODO isn't too much the buffer ?? return NULL ??
+    spice_assert(size < sizeof(client->receive_buf));
+    return client->receive_buf;
+}
+
+static void
+snd_channel_client_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size,
+                                    uint8_t *msg)
+{
+}
+
+// TODO remove and just use red_channel_client_disconnect ??
 static void snd_disconnect_channel_client(RedChannelClient *rcc)
 {
     SndChannel *channel;
@@ -1060,8 +797,8 @@  static void snd_disconnect_channel_client(RedChannelClient *rcc)
 
     spice_debug("channel-type=%d", type);
     if (channel->connection) {
-        spice_assert(channel->connection->channel_client == rcc);
-        snd_disconnect_channel(channel->connection);
+        spice_assert(RED_CHANNEL_CLIENT(channel->connection) == rcc);
+        red_channel_client_disconnect(rcc);
     }
 }
 
@@ -1079,7 +816,6 @@  SPICE_GNUC_VISIBLE void spice_server_playback_set_volume(SpicePlaybackInstance *
 {
     SpiceVolumeState *st = &sin->st->channel.volume;
     SndChannelClient *client = sin->st->channel.connection;
-    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
 
     st->volume_nchannels = nchannels;
     free(st->volume);
@@ -1088,21 +824,22 @@  SPICE_GNUC_VISIBLE void spice_server_playback_set_volume(SpicePlaybackInstance *
     if (!client || nchannels == 0)
         return;
 
-    snd_playback_send_volume(playback_client);
+    snd_set_command(client, SND_VOLUME_MUTE_MASK);
+    snd_send(client);
 }
 
 SPICE_GNUC_VISIBLE void spice_server_playback_set_mute(SpicePlaybackInstance *sin, uint8_t mute)
 {
     SpiceVolumeState *st = &sin->st->channel.volume;
     SndChannelClient *client = sin->st->channel.connection;
-    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
 
     st->mute = mute;
 
     if (!client)
         return;
 
-    snd_playback_send_mute(playback_client);
+    snd_set_command(client, SND_VOLUME_MUTE_MASK);
+    snd_send(client);
 }
 
 static void snd_playback_start(SndChannel *channel)
@@ -1117,7 +854,7 @@  static void snd_playback_start(SndChannel *channel)
     client->active = TRUE;
     if (!client->client_active) {
         snd_set_command(client, SND_CTRL_MASK);
-        snd_playback_send(client);
+        snd_send(client);
     } else {
         client->command &= ~SND_CTRL_MASK;
     }
@@ -1131,20 +868,20 @@  SPICE_GNUC_VISIBLE void spice_server_playback_start(SpicePlaybackInstance *sin)
 SPICE_GNUC_VISIBLE void spice_server_playback_stop(SpicePlaybackInstance *sin)
 {
     SndChannelClient *client = sin->st->channel.connection;
-    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
 
     sin->st->channel.active = 0;
     if (!client)
         return;
-    spice_assert(playback_client->base.active);
+    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(client);
+    spice_assert(client->active);
     reds_enable_mm_time(snd_channel_get_server(client));
-    playback_client->base.active = FALSE;
-    if (playback_client->base.client_active) {
-        snd_set_command(&playback_client->base, SND_CTRL_MASK);
-        snd_playback_send(&playback_client->base);
+    client->active = FALSE;
+    if (client->client_active) {
+        snd_set_command(client, SND_CTRL_MASK);
+        snd_send(client);
     } else {
-        playback_client->base.command &= ~SND_CTRL_MASK;
-        playback_client->base.command &= ~SND_PLAYBACK_PCM_MASK;
+        client->command &= ~SND_CTRL_MASK;
+        client->command &= ~SND_PLAYBACK_PCM_MASK;
 
         if (playback_client->pending_frame) {
             spice_assert(!playback_client->in_progress);
@@ -1159,15 +896,21 @@  SPICE_GNUC_VISIBLE void spice_server_playback_get_buffer(SpicePlaybackInstance *
                                                          uint32_t **frame, uint32_t *num_samples)
 {
     SndChannelClient *client = sin->st->channel.connection;
-    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
 
-    if (!client || !playback_client->free_frames) {
-        *frame = NULL;
-        *num_samples = 0;
+    *frame = NULL;
+    *num_samples = 0;
+    if (!client) {
+        return;
+    }
+    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(client);
+    if (!playback_client->free_frames) {
         return;
     }
-    spice_assert(playback_client->base.active);
-    snd_channel_ref(client);
+    spice_assert(client->active);
+    if (!playback_client->free_frames->allocated) {
+        playback_client->free_frames->allocated = TRUE;
+        ++playback_client->frames->refs;
+    }
 
     *frame = playback_client->free_frames->samples;
     playback_client->free_frames = playback_client->free_frames->next;
@@ -1180,23 +923,28 @@  SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
     AudioFrame *frame;
 
     frame = SPICE_CONTAINEROF(samples, AudioFrame, samples[0]);
+    if (frame->allocated) {
+        frame->allocated = FALSE;
+        if (--frame->container->refs == 0) {
+            free(frame->container);
+            return;
+        }
+    }
     playback_client = frame->client;
-    spice_assert(playback_client);
-    if (!snd_channel_unref(&playback_client->base) ||
-        sin->st->channel.connection != &playback_client->base) {
+    if (!playback_client || sin->st->channel.connection != SND_CHANNEL_CLIENT(playback_client)) {
         /* lost last reference, client has been destroyed previously */
         spice_info("audio samples belong to a disconnected client");
         return;
     }
-    spice_assert(playback_client->base.active);
+    spice_assert(SND_CHANNEL_CLIENT(playback_client)->active);
 
     if (playback_client->pending_frame) {
         snd_playback_free_frame(playback_client, playback_client->pending_frame);
     }
     frame->time = reds_get_mm_time();
     playback_client->pending_frame = frame;
-    snd_set_command(&playback_client->base, SND_PLAYBACK_PCM_MASK);
-    snd_playback_send(&playback_client->base);
+    snd_set_command(SND_CHANNEL_CLIENT(playback_client), SND_PLAYBACK_PCM_MASK);
+    snd_send(SND_CHANNEL_CLIENT(playback_client));
 }
 
 void snd_set_playback_latency(RedClient *client, uint32_t latency)
@@ -1207,15 +955,15 @@  void snd_set_playback_latency(RedClient *client, uint32_t latency)
         uint32_t type;
         g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
         if (type == SPICE_CHANNEL_PLAYBACK && now->connection &&
-            red_channel_client_get_client(now->connection->channel_client) == client) {
+            red_channel_client_get_client(RED_CHANNEL_CLIENT(now->connection)) == client) {
 
-            if (red_channel_client_test_remote_cap(now->connection->channel_client,
+            if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(now->connection),
                 SPICE_PLAYBACK_CAP_LATENCY)) {
                 PlaybackChannelClient* playback = (PlaybackChannelClient*)now->connection;
 
                 playback->latency = latency;
                 snd_set_command(now->connection, SND_PLAYBACK_LATENCY_MASK);
-                snd_playback_send(now->connection);
+                snd_send(now->connection);
             } else {
                 spice_debug("client doesn't not support SPICE_PLAYBACK_CAP_LATENCY");
             }
@@ -1250,60 +998,58 @@  static void on_new_playback_channel(SndChannel *channel, SndChannelClient *snd_c
         snd_set_command(snd_channel, SND_CTRL_MASK);
     }
     if (channel->volume.volume_nchannels) {
-        snd_set_command(snd_channel, SND_VOLUME_MASK);
+        snd_set_command(snd_channel, SND_VOLUME_MUTE_MASK);
     }
     if (snd_channel->active) {
         reds_disable_mm_time(reds);
     }
 }
 
-static void snd_playback_cleanup(SndChannelClient *client)
+static void
+playback_channel_client_finalize(GObject *object)
 {
-    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
+    int i;
+    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(object);
+    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
 
-    if (playback_client->base.active) {
+    // free frames, unref them
+    for (i = 0; i < NUM_AUDIO_FRAMES; ++i) {
+        playback_client->frames->items[i].client = NULL;
+    }
+    if (--playback_client->frames->refs == 0) {
+        free(playback_client->frames);
+    }
+
+    if (client->active) {
         reds_enable_mm_time(snd_channel_get_server(client));
     }
 
     snd_codec_destroy(&playback_client->codec);
+
+    G_OBJECT_CLASS(playback_channel_client_parent_class)->finalize(object);
 }
 
-static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream,
-                                  int migration, int num_common_caps, uint32_t *common_caps,
-                                  int num_caps, uint32_t *caps)
+static void
+playback_channel_client_constructed(GObject *object)
 {
+    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(object);
+    RedChannel *red_channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(playback_client));
+    RedClient *client = red_channel_client_get_client(RED_CHANNEL_CLIENT(playback_client));
     SndChannel *channel = SND_CHANNEL(red_channel);
-    PlaybackChannelClient *playback_client;
 
-    snd_disconnect_channel(channel->connection);
-
-    if (!(playback_client = (PlaybackChannelClient *)__new_channel(channel,
-                                                              sizeof(*playback_client),
-                                                              SPICE_CHANNEL_PLAYBACK,
-                                                              client,
-                                                              stream,
-                                                              migration,
-                                                              snd_playback_send,
-                                                              snd_playback_handle_message,
-                                                              snd_playback_on_message_done,
-                                                              snd_playback_cleanup,
-                                                              common_caps, num_common_caps,
-                                                              caps, num_caps))) {
-        return;
-    }
-    snd_playback_free_frame(playback_client, &playback_client->frames[0]);
-    snd_playback_free_frame(playback_client, &playback_client->frames[1]);
-    snd_playback_free_frame(playback_client, &playback_client->frames[2]);
+    G_OBJECT_CLASS(playback_channel_client_parent_class)->constructed(object);
 
-    int client_can_celt = red_channel_client_test_remote_cap(playback_client->base.channel_client,
+    SND_CHANNEL_CLIENT(playback_client)->on_message_done = snd_playback_on_message_done;
+
+    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
+    int client_can_celt = red_channel_client_test_remote_cap(rcc,
                                           SPICE_PLAYBACK_CAP_CELT_0_5_1);
-    int client_can_opus = red_channel_client_test_remote_cap(playback_client->base.channel_client,
+    int client_can_opus = red_channel_client_test_remote_cap(rcc,
                                           SPICE_PLAYBACK_CAP_OPUS);
     int playback_compression =
         reds_config_get_playback_compression(red_channel_get_server(red_channel));
     int desired_mode = snd_desired_audio_mode(playback_compression, channel->frequency,
                                               client_can_celt, client_can_opus);
-    playback_client->mode = SPICE_AUDIO_DATA_MODE_RAW;
     if (desired_mode != SPICE_AUDIO_DATA_MODE_RAW) {
         if (snd_codec_create(&playback_client->codec, desired_mode, channel->frequency,
                              SND_CODEC_ENCODE) == SND_CODEC_OK) {
@@ -1314,13 +1060,52 @@  static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, Re
     }
 
     if (!red_client_during_migrate_at_target(client)) {
-        on_new_playback_channel(channel, &playback_client->base);
+        on_new_playback_channel(channel, SND_CHANNEL_CLIENT(playback_client));
     }
 
     if (channel->active) {
         snd_playback_start(channel);
     }
-    snd_playback_send(channel->connection);
+    snd_send(SND_CHANNEL_CLIENT(playback_client));
+}
+
+static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream,
+                                  int migration, int num_common_caps, uint32_t *common_caps,
+                                  int num_caps, uint32_t *caps)
+{
+    SndChannel *channel = SND_CHANNEL(red_channel);
+    GArray *common_caps_array = NULL, *caps_array = NULL;
+
+    if (channel->connection) {
+        red_channel_client_disconnect(RED_CHANNEL_CLIENT(channel->connection));
+        channel->connection = NULL;
+    }
+
+    if (common_caps) {
+        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
+                                              num_common_caps);
+        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
+    }
+    if (caps) {
+        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
+        g_array_append_vals(caps_array, caps, num_caps);
+    }
+
+    g_initable_new(TYPE_PLAYBACK_CHANNEL_CLIENT,
+                   NULL, NULL,
+                   "channel", channel,
+                   "client", client,
+                   "stream", stream,
+                   "caps", caps_array,
+                   "common-caps", common_caps_array,
+                   NULL);
+
+    if (caps_array) {
+        g_array_unref(caps_array);
+    }
+    if (common_caps_array) {
+        g_array_unref(common_caps_array);
+    }
 }
 
 static void snd_record_migrate_channel_client(RedChannelClient *rcc)
@@ -1333,9 +1118,9 @@  static void snd_record_migrate_channel_client(RedChannelClient *rcc)
     spice_assert(channel);
 
     if (channel->connection) {
-        spice_assert(channel->connection->channel_client == rcc);
+        spice_assert(RED_CHANNEL_CLIENT(channel->connection) == rcc);
         snd_set_command(channel->connection, SND_MIGRATE_MASK);
-        snd_record_send(channel->connection);
+        snd_send(channel->connection);
     }
 }
 
@@ -1345,7 +1130,6 @@  SPICE_GNUC_VISIBLE void spice_server_record_set_volume(SpiceRecordInstance *sin,
 {
     SpiceVolumeState *st = &sin->st->channel.volume;
     SndChannelClient *client = sin->st->channel.connection;
-    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
 
     st->volume_nchannels = nchannels;
     free(st->volume);
@@ -1354,38 +1138,40 @@  SPICE_GNUC_VISIBLE void spice_server_record_set_volume(SpiceRecordInstance *sin,
     if (!client || nchannels == 0)
         return;
 
-    snd_record_send_volume(record_client);
+    snd_set_command(client, SND_VOLUME_MUTE_MASK);
+    snd_send(client);
 }
 
 SPICE_GNUC_VISIBLE void spice_server_record_set_mute(SpiceRecordInstance *sin, uint8_t mute)
 {
     SpiceVolumeState *st = &sin->st->channel.volume;
     SndChannelClient *client = sin->st->channel.connection;
-    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
 
     st->mute = mute;
 
     if (!client)
         return;
 
-    snd_record_send_mute(record_client);
+    snd_set_command(client, SND_VOLUME_MUTE_MASK);
+    snd_send(client);
 }
 
 static void snd_record_start(SndChannel *channel)
 {
     SndChannelClient *client = channel->connection;
-    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
 
     channel->active = 1;
-    if (!client)
+    if (!client) {
         return;
+    }
+    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(client);
     spice_assert(!client->active);
     record_client->read_pos = record_client->write_pos = 0;   //todo: improve by
                                                                 //stream generation
     client->active = TRUE;
     if (!client->client_active) {
         snd_set_command(client, SND_CTRL_MASK);
-        snd_record_send(client);
+        snd_send(client);
     } else {
         client->command &= ~SND_CTRL_MASK;
     }
@@ -1399,18 +1185,17 @@  SPICE_GNUC_VISIBLE void spice_server_record_start(SpiceRecordInstance *sin)
 SPICE_GNUC_VISIBLE void spice_server_record_stop(SpiceRecordInstance *sin)
 {
     SndChannelClient *client = sin->st->channel.connection;
-    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
 
     sin->st->channel.active = 0;
     if (!client)
         return;
-    spice_assert(record_client->base.active);
-    record_client->base.active = FALSE;
-    if (record_client->base.client_active) {
-        snd_set_command(&record_client->base, SND_CTRL_MASK);
-        snd_record_send(&record_client->base);
+    spice_assert(client->active);
+    client->active = FALSE;
+    if (client->client_active) {
+        snd_set_command(client, SND_CTRL_MASK);
+        snd_send(client);
     } else {
-        record_client->base.command &= ~SND_CTRL_MASK;
+        client->command &= ~SND_CTRL_MASK;
     }
 }
 
@@ -1418,14 +1203,14 @@  SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
                                                             uint32_t *samples, uint32_t bufsize)
 {
     SndChannelClient *client = sin->st->channel.connection;
-    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
     uint32_t read_pos;
     uint32_t now;
     uint32_t len;
 
     if (!client)
         return 0;
-    spice_assert(record_client->base.active);
+    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(client);
+    spice_assert(client->active);
 
     if (record_client->write_pos < RECORD_SAMPLES_SIZE / 2) {
         return 0;
@@ -1433,14 +1218,17 @@  SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
 
     len = MIN(record_client->write_pos - record_client->read_pos, bufsize);
 
+    // TODO why ?? stream already call if got data
+#if 0
     if (len < bufsize) {
-        SndChannel *channel = record_client->base.channel;
-        snd_receive(&record_client->base);
+        SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(RED_CHANNEL_CLIENT(client)));
+        snd_receive(client);
         if (!channel->connection) {
             return 0;
         }
         len = MIN(record_client->write_pos - record_client->read_pos, bufsize);
     }
+#endif
 
     read_pos = record_client->read_pos % RECORD_SAMPLES_SIZE;
     record_client->read_pos += len;
@@ -1456,7 +1244,7 @@  static uint32_t snd_get_best_rate(SndChannelClient *client, uint32_t cap_opus)
 {
     int client_can_opus = TRUE;
     if (client) {
-        client_can_opus = red_channel_client_test_remote_cap(client->channel_client, cap_opus);
+        client_can_opus = red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(client), cap_opus);
     }
 
     if (client_can_opus && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS, SND_CODEC_ANY_FREQUENCY))
@@ -1498,52 +1286,79 @@  static void on_new_record_channel(SndChannel *channel, SndChannelClient *snd_cha
 {
     spice_assert(snd_channel);
 
-    channel->connection = snd_channel ;
+    channel->connection = snd_channel;
     if (channel->volume.volume_nchannels) {
-        snd_set_command(snd_channel, SND_VOLUME_MASK);
+        snd_set_command(snd_channel, SND_VOLUME_MUTE_MASK);
     }
     if (snd_channel->active) {
         snd_set_command(snd_channel, SND_CTRL_MASK);
     }
 }
 
-static void snd_record_cleanup(SndChannelClient *client)
+static void
+record_channel_client_finalize(GObject *object)
 {
-    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
+    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(object);
+
     snd_codec_destroy(&record_client->codec);
+
+    G_OBJECT_CLASS(record_channel_client_parent_class)->finalize(object);
 }
 
+static void
+record_channel_client_constructed(GObject *object)
+{
+    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(object);
+    RedChannel *red_channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(record_client));
+    SndChannel *channel = SND_CHANNEL(red_channel);
+
+    G_OBJECT_CLASS(record_channel_client_parent_class)->constructed(object);
+
+    on_new_record_channel(channel, SND_CHANNEL_CLIENT(record_client));
+    if (channel->active) {
+        snd_record_start(channel);
+    }
+    snd_send(SND_CHANNEL_CLIENT(record_client));
+}
+
+
 static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream,
                                 int migration, int num_common_caps, uint32_t *common_caps,
                                 int num_caps, uint32_t *caps)
 {
     SndChannel *channel = SND_CHANNEL(red_channel);
-    RecordChannelClient *record_client;
-
-    snd_disconnect_channel(channel->connection);
-
-    if (!(record_client = (RecordChannelClient *)__new_channel(channel,
-                                                          sizeof(*record_client),
-                                                          SPICE_CHANNEL_RECORD,
-                                                          client,
-                                                          stream,
-                                                          migration,
-                                                          snd_record_send,
-                                                          snd_record_handle_message,
-                                                          snd_record_on_message_done,
-                                                          snd_record_cleanup,
-                                                          common_caps, num_common_caps,
-                                                          caps, num_caps))) {
-        return;
+    GArray *common_caps_array = NULL, *caps_array = NULL;
+
+    if (channel->connection) {
+        red_channel_client_disconnect(RED_CHANNEL_CLIENT(channel->connection));
+        channel->connection = NULL;
     }
 
-    record_client->mode = SPICE_AUDIO_DATA_MODE_RAW;
+    if (common_caps) {
+        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
+                                              num_common_caps);
+        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
+    }
+    if (caps) {
+        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
+        g_array_append_vals(caps_array, caps, num_caps);
+    }
 
-    on_new_record_channel(channel, &record_client->base);
-    if (channel->active) {
-        snd_record_start(channel);
+    g_initable_new(TYPE_RECORD_CHANNEL_CLIENT,
+                   NULL, NULL,
+                   "channel", channel,
+                   "client", client,
+                   "stream", stream,
+                   "caps", caps_array,
+                   "common-caps", common_caps_array,
+                   NULL);
+
+    if (caps_array) {
+        g_array_unref(caps_array);
+    }
+    if (common_caps_array) {
+        g_array_unref(common_caps_array);
     }
-    snd_record_send(channel->connection);
 }
 
 static void snd_playback_migrate_channel_client(RedChannelClient *rcc)
@@ -1557,9 +1372,9 @@  static void snd_playback_migrate_channel_client(RedChannelClient *rcc)
     spice_debug(NULL);
 
     if (channel->connection) {
-        spice_assert(channel->connection->channel_client == rcc);
+        spice_assert(RED_CHANNEL_CLIENT(channel->connection) == rcc);
         snd_set_command(channel->connection, SND_MIGRATE_MASK);
-        snd_playback_send(channel->connection);
+        snd_send(channel->connection);
     }
 }
 
@@ -1593,6 +1408,8 @@  snd_channel_class_init(SndChannelClass *klass)
 {
     RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
     channel_class->config_socket = snd_channel_config_socket;
+    channel_class->alloc_recv_buf = snd_channel_client_alloc_recv_buf;
+    channel_class->release_recv_buf = snd_channel_client_release_recv_buf;
 }
 
 static void
@@ -1627,8 +1444,13 @@  static void
 playback_channel_class_init(SndChannelClass *klass)
 {
     GObjectClass *object_class = G_OBJECT_CLASS(klass);
+    RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
 
     object_class->constructed = playback_channel_constructed;
+
+    channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_PLAYBACK, NULL);
+    channel_class->handle_parsed = playback_channel_handle_parsed;
+    channel_class->send_item = playback_channel_send_item;
 }
 
 void snd_attach_playback(RedsState *reds, SpicePlaybackInstance *sin)
@@ -1673,8 +1495,13 @@  static void
 record_channel_class_init(SndChannelClass *klass)
 {
     GObjectClass *object_class = G_OBJECT_CLASS(klass);
+    RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
 
     object_class->constructed = record_channel_constructed;
+
+    channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_RECORD, NULL);
+    channel_class->handle_parsed = record_channel_handle_parsed;
+    channel_class->send_item = record_channel_send_item;
 }
 
 void snd_attach_record(RedsState *reds, SpiceRecordInstance *sin)
@@ -1695,7 +1522,6 @@  static void snd_detach_common(SndChannel *channel)
     RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
 
     remove_channel(channel);
-    snd_disconnect_channel(channel->connection);
     reds_unregister_channel(reds, RED_CHANNEL(channel));
     free(channel->volume.volume);
     channel->volume.volume = NULL;
@@ -1721,9 +1547,10 @@  void snd_set_playback_compression(int on)
         g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
         if (type == SPICE_CHANNEL_PLAYBACK && now->connection) {
             PlaybackChannelClient* playback = (PlaybackChannelClient*)now->connection;
-            int client_can_celt = red_channel_client_test_remote_cap(playback->base.channel_client,
+            RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback);
+            int client_can_celt = red_channel_client_test_remote_cap(rcc,
                                     SPICE_PLAYBACK_CAP_CELT_0_5_1);
-            int client_can_opus = red_channel_client_test_remote_cap(playback->base.channel_client,
+            int client_can_opus = red_channel_client_test_remote_cap(rcc,
                                     SPICE_PLAYBACK_CAP_OPUS);
             int desired_mode = snd_desired_audio_mode(on, now->frequency,
                                                       client_can_opus, client_can_celt);
@@ -1734,3 +1561,50 @@  void snd_set_playback_compression(int on)
         }
     }
 }
+
+static void
+snd_channel_client_class_init(SndChannelClientClass *self)
+{
+}
+
+static void
+snd_channel_client_init(SndChannelClient *self)
+{
+}
+
+static void
+playback_channel_client_class_init(PlaybackChannelClientClass *klass)
+{
+    GObjectClass *object_class = G_OBJECT_CLASS(klass);
+    object_class->constructed = playback_channel_client_constructed;
+    object_class->finalize = playback_channel_client_finalize;
+}
+
+static void
+playback_channel_client_init(PlaybackChannelClient *playback)
+{
+    int i;
+
+    playback->mode = SPICE_AUDIO_DATA_MODE_RAW;
+
+    playback->frames = spice_new0(AudioFrameContainer, 1);
+    playback->frames->refs = 1;
+    for (i = 0; i < NUM_AUDIO_FRAMES; ++i) {
+        playback->frames->items[i].container = playback->frames;
+        snd_playback_free_frame(playback, &playback->frames->items[i]);
+    }
+}
+
+static void
+record_channel_client_class_init(RecordChannelClientClass *klass)
+{
+    GObjectClass *object_class = G_OBJECT_CLASS(klass);
+    object_class->constructed = record_channel_client_constructed;
+    object_class->finalize = record_channel_client_finalize;
+}
+
+static void
+record_channel_client_init(RecordChannelClient *record)
+{
+    record->mode = SPICE_AUDIO_DATA_MODE_RAW;
+}

Comments

Hey,

On Wed, Nov 23, 2016 at 06:07:25PM +0000, Frediano Ziglio wrote:
> This patch is quite huge but have some benefits:
> - reduce dependency (DummyChannel* and some RedChannelClient internals);
> - reuse RedChannelClient instead of reading from the RedsStream
>   directly.
> 
> You can see that SndChannelClient has much less field
> as the code to read/write from/to client is reused from
> RedChannelClient instead of creating a fake RedChannelClient
> just to make the system happy.
> 
I started looking at this, and it's really big/hard to review with all
these changes :(
Wondering if the AudioFrame allocation change could be done before, if
we could have a few cosmetic commits first introducing
SpiceMarshaller *m = client->send_data.marshaller; in snd_send_xxx()
Maybe separate the GObjectification by itself from switching to using
facilities provided by RedChannelClient, ...
Dunno if the snd_set_command/snd_send API can be introduced before the
gobjectification

Just a few random thoughts after reading this patch in the hope that it
can be made simpler, some of them are probably not practical at all :(

Christophe



> One of the different between the old sound code and all other
> RedChannelClient objects was that the sound channel don't use
> a queue while RedChannelClient use RedPipeItem object. This was
> the main reason why RedChannelClient was not used. To implement
> the old behaviour a "persistent_pipe_item" is used. This RedPipeItem
> will be queued to RedChannelClient (only one!) so signal code we
> have data to send. The {playback,record}_channel_send_item will
> then send the messages to the client using RedChannelClient functions.
> For this reason snd_reset_send_data is replaced by a call to
> red_channel_client_init_send_data and snd_begin_send_message is
> replaced by red_channel_client_begin_send_message.
> 
> Another difference is the AudioFrame allocation. Previously these
> frames were allocated inside PlaybackChannelClient while now they
> are allocated in a different structure. This allows the samples
> to outlive the PlaybackChannelClient. This was possible even before
> and the client was kept alive. However having RedChannelClient as
> as base class can be an issue as this will cause the entire
> RedChannel and RedChannelClient to stay alive. To avoid this
> potential problem allocate the frames in a different structure
> keeping a reference from PlaybackChannelClient. When the client
> is freed the AudioFrameContainer is just unreferences.
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  server/sound.c | 1100 +++++++++++++++++++++++++-------------------------------
>  1 file changed, 487 insertions(+), 613 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index ac11106..0c2a3ae 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -32,14 +32,10 @@
>  
>  #include "spice.h"
>  #include "red-common.h"
> -#include "dummy-channel.h"
> -#include "dummy-channel-client.h"
>  #include "main-channel.h"
>  #include "reds.h"
>  #include "red-qxl.h"
>  #include "red-channel-client.h"
> -/* FIXME: for now, allow sound channel access to private RedChannelClient data */
> -#include "red-channel-client-private.h"
>  #include "red-client.h"
>  #include "sound.h"
>  #include "demarshallers.h"
> @@ -56,6 +52,7 @@ enum SndCommand {
>      SND_MIGRATE,
>      SND_CTRL,
>      SND_VOLUME,
> +    SND_MUTE,
>      SND_END_COMMAND,
>  };
>  
> @@ -68,67 +65,79 @@ enum PlaybackCommand {
>  #define SND_MIGRATE_MASK (1 << SND_MIGRATE)
>  #define SND_CTRL_MASK (1 << SND_CTRL)
>  #define SND_VOLUME_MASK (1 << SND_VOLUME)
> +#define SND_MUTE_MASK (1 << SND_MUTE)
> +#define SND_VOLUME_MUTE_MASK (SND_VOLUME_MASK|SND_MUTE_MASK)
>  
>  #define SND_PLAYBACK_MODE_MASK (1 << SND_PLAYBACK_MODE)
>  #define SND_PLAYBACK_PCM_MASK (1 << SND_PLAYBACK_PCM)
>  #define SND_PLAYBACK_LATENCY_MASK ( 1 << SND_PLAYBACK_LATENCY)
>  
>  typedef struct SndChannelClient SndChannelClient;
> -typedef void (*snd_channel_send_messages_proc)(void *in_channel);
> -typedef int (*snd_channel_handle_message_proc)(SndChannelClient *client, size_t size, uint32_t type, void *message);
> +typedef struct SndChannel SndChannel;
> +typedef struct PlaybackChannelClient PlaybackChannelClient;
> +typedef struct RecordChannelClient RecordChannelClient;
> +typedef struct AudioFrame AudioFrame;
> +typedef struct AudioFrameContainer AudioFrameContainer;
> +typedef struct SpicePlaybackState PlaybackChannel;
> +typedef struct SpiceRecordState RecordChannel;
> +
>  typedef void (*snd_channel_on_message_done_proc)(SndChannelClient *client);
> -typedef void (*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
>  
> -typedef struct SndChannel SndChannel;
> +
> +#define TYPE_SND_CHANNEL_CLIENT snd_channel_client_get_type()
> +#define SND_CHANNEL_CLIENT(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_SND_CHANNEL_CLIENT, SndChannelClient))
> +GType snd_channel_client_get_type(void) G_GNUC_CONST;
>  
>  /* Connects an audio client to a Spice client */
>  struct SndChannelClient {
> -    RedsStream *stream;
> -    SndChannel *channel;
> -    spice_parse_channel_func_t parser;
> -    int refs;
> -
> -    RedChannelClient *channel_client;
> +    RedChannelClient parent;
>  
>      int active;
>      int client_active;
> -    int blocked;
>  
>      uint32_t command;
>  
> -    struct {
> -        uint64_t serial;
> -        SpiceMarshaller *marshaller;
> -        uint32_t size;
> -        uint32_t pos;
> -    } send_data;
> -
> -    struct {
> -        uint8_t buf[SND_RECEIVE_BUF_SIZE];
> -        uint8_t *message_start;
> -        uint8_t *now;
> -        uint8_t *end;
> -    } receive_data;
> -
> -    snd_channel_send_messages_proc send_messages;
> -    snd_channel_handle_message_proc handle_message;
> +    uint8_t receive_buf[SND_RECEIVE_BUF_SIZE];
> +    RedPipeItem persistent_pipe_item;
> +
>      snd_channel_on_message_done_proc on_message_done;
> -    snd_channel_cleanup_channel_proc cleanup;
>  };
>  
> -typedef struct PlaybackChannelClient PlaybackChannelClient;
> +typedef RedChannelClientClass SndChannelClientClass;
> +
> +G_DEFINE_TYPE(SndChannelClient, snd_channel_client, RED_TYPE_CHANNEL_CLIENT)
> +
> +
> +enum {
> +    RED_PIPE_ITEM_PERSISTENT = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
> +};
> +
>  
> -typedef struct AudioFrame AudioFrame;
>  struct AudioFrame {
>      uint32_t time;
>      uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
>      PlaybackChannelClient *client;
>      AudioFrame *next;
> +    AudioFrameContainer *container;
> +    gboolean allocated;
> +};
> +
> +#define NUM_AUDIO_FRAMES 3
> +struct AudioFrameContainer
> +{
> +    int refs;
> +    AudioFrame items[NUM_AUDIO_FRAMES];
>  };
>  
> +#define TYPE_PLAYBACK_CHANNEL_CLIENT playback_channel_client_get_type()
> +#define PLAYBACK_CHANNEL_CLIENT(obj) \
> +    (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_PLAYBACK_CHANNEL_CLIENT, PlaybackChannelClient))
> +GType playback_channel_client_get_type(void) G_GNUC_CONST;
> +
>  struct PlaybackChannelClient {
> -    SndChannelClient base;
> -    AudioFrame frames[3];
> +    SndChannelClient parent;
> +
> +    AudioFrameContainer *frames;
>      AudioFrame *free_frames;
>      AudioFrame *in_progress;   /* Frame being sent to the client */
>      AudioFrame *pending_frame; /* Next frame to send to the client */
> @@ -138,6 +147,11 @@ struct PlaybackChannelClient {
>      uint8_t  encode_buf[SND_CODEC_MAX_COMPRESSED_BYTES];
>  };
>  
> +typedef SndChannelClientClass PlaybackChannelClientClass;
> +
> +G_DEFINE_TYPE(PlaybackChannelClient, playback_channel_client, TYPE_SND_CHANNEL_CLIENT)
> +
> +
>  typedef struct SpiceVolumeState {
>      uint16_t *volume;
>      uint8_t volume_nchannels;
> @@ -165,9 +179,6 @@ typedef RedChannelClass SndChannelClass;
>  G_DEFINE_TYPE(SndChannel, snd_channel, RED_TYPE_CHANNEL)
>  
>  
> -typedef struct SpicePlaybackState PlaybackChannel;
> -typedef SndChannelClass PlaybackChannelClass;
> -
>  #define TYPE_PLAYBACK_CHANNEL playback_channel_get_type()
>  #define PLAYBACK_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_PLAYBACK_CHANNEL, PlaybackChannel))
>  GType playback_channel_get_type(void) G_GNUC_CONST;
> @@ -176,12 +187,11 @@ struct SpicePlaybackState {
>      SndChannel channel;
>  };
>  
> +typedef SndChannelClass PlaybackChannelClass;
> +
>  G_DEFINE_TYPE(PlaybackChannel, playback_channel, TYPE_SND_CHANNEL)
>  
>  
> -typedef struct SpiceRecordState RecordChannel;
> -typedef SndChannelClass RecordChannelClass;
> -
>  #define TYPE_RECORD_CHANNEL record_channel_get_type()
>  #define RECORD_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_RECORD_CHANNEL, RecordChannel))
>  GType record_channel_get_type(void) G_GNUC_CONST;
> @@ -190,11 +200,18 @@ struct SpiceRecordState {
>      SndChannel channel;
>  };
>  
> +typedef SndChannelClass RecordChannelClass;
> +
>  G_DEFINE_TYPE(RecordChannel, record_channel, TYPE_SND_CHANNEL)
>  
>  
> -typedef struct RecordChannelClient {
> -    SndChannelClient base;
> +#define TYPE_RECORD_CHANNEL_CLIENT record_channel_client_get_type()
> +#define RECORD_CHANNEL_CLIENT(obj) \
> +    (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_RECORD_CHANNEL_CLIENT, RecordChannelClient))
> +GType record_channel_client_get_type(void) G_GNUC_CONST;
> +
> +struct RecordChannelClient {
> +    SndChannelClient parent;
>      uint32_t samples[RECORD_SAMPLES_SIZE];
>      uint32_t write_pos;
>      uint32_t read_pos;
> @@ -203,64 +220,23 @@ typedef struct RecordChannelClient {
>      uint32_t start_time;
>      SndCodec codec;
>      uint8_t  decode_buf[SND_CODEC_MAX_FRAME_BYTES];
> -} RecordChannelClient;
> +};
> +
> +typedef SndChannelClientClass RecordChannelClientClass;
> +
> +G_DEFINE_TYPE(RecordChannelClient, record_channel_client, TYPE_SND_CHANNEL_CLIENT)
> +
>  
>  /* A list of all Spice{Playback,Record}State objects */
>  static SndChannel *snd_channels;
>  
> -static void snd_receive(SndChannelClient *client);
>  static void snd_playback_start(SndChannel *channel);
>  static void snd_record_start(SndChannel *channel);
>  
> -static SndChannelClient *snd_channel_ref(SndChannelClient *client)
> -{
> -    client->refs++;
> -    return client;
> -}
> -
> -static SndChannelClient *snd_channel_unref(SndChannelClient *client)
> -{
> -    if (!--client->refs) {
> -        spice_printerr("SndChannelClient=%p freed", client);
> -        free(client);
> -        return NULL;
> -    }
> -    return client;
> -}
> -
>  static RedsState* snd_channel_get_server(SndChannelClient *client)
>  {
>      g_return_val_if_fail(client != NULL, NULL);
> -    return red_channel_get_server(RED_CHANNEL(client->channel));
> -}
> -
> -static void snd_disconnect_channel(SndChannelClient *client)
> -{
> -    SndChannel *channel;
> -    RedsState *reds;
> -    RedChannel *red_channel;
> -    uint32_t type;
> -
> -    if (!client || !client->stream) {
> -        spice_debug("not connected");
> -        return;
> -    }
> -    red_channel = red_channel_client_get_channel(client->channel_client);
> -    reds = snd_channel_get_server(client);
> -    g_object_get(red_channel, "channel-type", &type, NULL);
> -    spice_debug("SndChannelClient=%p rcc=%p type=%d",
> -                 client, client->channel_client, type);
> -    channel = client->channel;
> -    client->cleanup(client);
> -    red_channel_client_disconnect(channel->connection->channel_client);
> -    channel->connection->channel_client = NULL;
> -    reds_core_watch_remove(reds, client->stream->watch);
> -    client->stream->watch = NULL;
> -    reds_stream_free(client->stream);
> -    client->stream = NULL;
> -    spice_marshaller_destroy(client->send_data.marshaller);
> -    snd_channel_unref(client);
> -    channel->connection = NULL;
> +    return red_channel_get_server(red_channel_client_get_channel(RED_CHANNEL_CLIENT(client)));
>  }
>  
>  static void snd_playback_free_frame(PlaybackChannelClient *playback_client, AudioFrame *frame)
> @@ -282,65 +258,6 @@ static void snd_playback_on_message_done(SndChannelClient *client)
>      }
>  }
>  
> -static void snd_record_on_message_done(SndChannelClient *client)
> -{
> -}
> -
> -static int snd_send_data(SndChannelClient *client)
> -{
> -    uint32_t n;
> -
> -    if (!client) {
> -        return FALSE;
> -    }
> -
> -    if (!(n = client->send_data.size - client->send_data.pos)) {
> -        return TRUE;
> -    }
> -
> -    RedsState *reds = snd_channel_get_server(client);
> -    for (;;) {
> -        struct iovec vec[IOV_MAX];
> -        int vec_size;
> -
> -        if (!n) {
> -            client->on_message_done(client);
> -
> -            if (client->blocked) {
> -                client->blocked = FALSE;
> -                reds_core_watch_update_mask(reds, client->stream->watch, SPICE_WATCH_EVENT_READ);
> -            }
> -            break;
> -        }
> -
> -        vec_size = spice_marshaller_fill_iovec(client->send_data.marshaller,
> -                                               vec, IOV_MAX, client->send_data.pos);
> -        n = reds_stream_writev(client->stream, vec, vec_size);
> -        if (n == -1) {
> -            switch (errno) {
> -            case EAGAIN:
> -                client->blocked = TRUE;
> -                reds_core_watch_update_mask(reds, client->stream->watch, SPICE_WATCH_EVENT_READ |
> -                                                                 SPICE_WATCH_EVENT_WRITE);
> -                return FALSE;
> -            case EINTR:
> -                break;
> -            case EPIPE:
> -                snd_disconnect_channel(client);
> -                return FALSE;
> -            default:
> -                spice_printerr("%s", strerror(errno));
> -                snd_disconnect_channel(client);
> -                return FALSE;
> -            }
> -        } else {
> -            client->send_data.pos += n;
> -        }
> -        n = client->send_data.size - client->send_data.pos;
> -    }
> -    return TRUE;
> -}
> -
>  static int snd_record_handle_write(RecordChannelClient *record_client, size_t size, void *message)
>  {
>      SpiceMsgcRecordPacket *packet;
> @@ -386,35 +303,29 @@ static int snd_record_handle_write(RecordChannelClient *record_client, size_t si
>      return TRUE;
>  }
>  
> -static int snd_playback_handle_message(SndChannelClient *client, size_t size, uint32_t type, void *message)
> +static int
> +playback_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type, void *message)
>  {
> -    if (!client) {
> -        return FALSE;
> -    }
> -
>      switch (type) {
>      case SPICE_MSGC_DISCONNECTING:
>          break;
>      default:
> -        spice_printerr("invalid message type %u", type);
> -        return FALSE;
> +        return red_channel_client_handle_message(rcc, size, type, message);
>      }
>      return TRUE;
>  }
>  
> -static int snd_record_handle_message(SndChannelClient *client, size_t size, uint32_t type, void *message)
> +static int
> +record_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type, void *message)
>  {
> -    RecordChannelClient *record_client = (RecordChannelClient *)client;
> +    RecordChannelClient *record_client = (RecordChannelClient *)rcc; // FIXME
>  
> -    if (!client) {
> -        return FALSE;
> -    }
>      switch (type) {
>      case SPICE_MSGC_RECORD_DATA:
> -        return snd_record_handle_write((RecordChannelClient *)client, size, message);
> +        return snd_record_handle_write(record_client, size, message);
>      case SPICE_MSGC_RECORD_MODE: {
>          SpiceMsgcRecordMode *mode = (SpiceMsgcRecordMode *)message;
> -        SndChannel *channel = client->channel;
> +        SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
>          record_client->mode_time = mode->time;
>          if (mode->mode != SPICE_AUDIO_DATA_MODE_RAW) {
>              if (snd_codec_is_capable(mode->mode, channel->frequency)) {
> @@ -444,260 +355,134 @@ static int snd_record_handle_message(SndChannelClient *client, size_t size, uint
>      case SPICE_MSGC_DISCONNECTING:
>          break;
>      default:
> -        spice_printerr("invalid message type %u", type);
> -        return FALSE;
> -    }
> -    return TRUE;
> -}
> -
> -static void snd_receive(SndChannelClient *client)
> -{
> -    SpiceDataHeaderOpaque *header;
> -
> -    if (!client) {
> -        return;
> -    }
> -
> -    header = &client->channel_client->incoming.header;
> -
> -    for (;;) {
> -        ssize_t n;
> -        n = client->receive_data.end - client->receive_data.now;
> -        spice_warn_if_fail(n > 0);
> -        n = reds_stream_read(client->stream, client->receive_data.now, n);
> -        if (n <= 0) {
> -            if (n == 0) {
> -                snd_disconnect_channel(client);
> -                return;
> -            }
> -            spice_assert(n == -1);
> -            switch (errno) {
> -            case EAGAIN:
> -                return;
> -            case EINTR:
> -                break;
> -            case EPIPE:
> -                snd_disconnect_channel(client);
> -                return;
> -            default:
> -                spice_printerr("%s", strerror(errno));
> -                snd_disconnect_channel(client);
> -                return;
> -            }
> -        } else {
> -            client->receive_data.now += n;
> -            for (;;) {
> -                uint8_t *msg_start = client->receive_data.message_start;
> -                uint8_t *data = msg_start + header->header_size;
> -                size_t parsed_size;
> -                uint8_t *parsed;
> -                message_destructor_t parsed_free;
> -
> -                header->data = msg_start;
> -                n = client->receive_data.now - msg_start;
> -
> -                if (n < header->header_size ||
> -                    n < header->header_size + header->get_msg_size(header)) {
> -                    break;
> -                }
> -                parsed = client->parser((void *)data, data + header->get_msg_size(header),
> -                                         header->get_msg_type(header),
> -                                         SPICE_VERSION_MINOR, &parsed_size, &parsed_free);
> -                if (parsed == NULL) {
> -                    spice_printerr("failed to parse message type %d", header->get_msg_type(header));
> -                    snd_disconnect_channel(client);
> -                    return;
> -                }
> -                if (!client->handle_message(client, parsed_size,
> -                                             header->get_msg_type(header), parsed)) {
> -                    free(parsed);
> -                    snd_disconnect_channel(client);
> -                    return;
> -                }
> -                parsed_free(parsed);
> -                client->receive_data.message_start = msg_start + header->header_size +
> -                                                     header->get_msg_size(header);
> -            }
> -            if (client->receive_data.now == client->receive_data.message_start) {
> -                client->receive_data.now = client->receive_data.buf;
> -                client->receive_data.message_start = client->receive_data.buf;
> -            } else if (client->receive_data.now == client->receive_data.end) {
> -                memcpy(client->receive_data.buf, client->receive_data.message_start, n);
> -                client->receive_data.now = client->receive_data.buf + n;
> -                client->receive_data.message_start = client->receive_data.buf;
> -            }
> -        }
> -    }
> -}
> -
> -static void snd_event(int fd, int event, void *data)
> -{
> -    SndChannelClient *client = data;
> -
> -    if (event & SPICE_WATCH_EVENT_READ) {
> -        snd_receive(client);
> -    }
> -    if (event & SPICE_WATCH_EVENT_WRITE) {
> -        client->send_messages(client);
> +        return red_channel_client_handle_message(rcc, size, type, message);
>      }
> -}
> -
> -static inline int snd_reset_send_data(SndChannelClient *client, uint16_t verb)
> -{
> -    SpiceDataHeaderOpaque *header;
> -
> -    if (!client) {
> -        return FALSE;
> -    }
> -
> -    header = &client->channel_client->priv->send_data.header;
> -    spice_marshaller_reset(client->send_data.marshaller);
> -    header->data = spice_marshaller_reserve_space(client->send_data.marshaller,
> -                                                  header->header_size);
> -    spice_marshaller_set_base(client->send_data.marshaller,
> -                              header->header_size);
> -    client->send_data.pos = 0;
> -    header->set_msg_size(header, 0);
> -    header->set_msg_type(header, verb);
> -    client->send_data.serial++;
> -    if (!client->channel_client->priv->is_mini_header) {
> -        header->set_msg_serial(header, client->send_data.serial);
> -        header->set_msg_sub_list(header, 0);
> -    }
> -
>      return TRUE;
>  }
>  
> -static int snd_begin_send_message(SndChannelClient *client)
> -{
> -    SpiceDataHeaderOpaque *header = &client->channel_client->priv->send_data.header;
> -
> -    spice_marshaller_flush(client->send_data.marshaller);
> -    client->send_data.size = spice_marshaller_get_total_size(client->send_data.marshaller);
> -    header->set_msg_size(header, client->send_data.size - header->header_size);
> -    return snd_send_data(client);
> -}
> -
>  static int snd_channel_send_migrate(SndChannelClient *client)
>  {
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>      SpiceMsgMigrate migrate;
>  
> -    if (!snd_reset_send_data(client, SPICE_MSG_MIGRATE)) {
> -        return FALSE;
> -    }
> -    spice_debug(NULL);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE, NULL);
>      migrate.flags = 0;
> -    spice_marshall_msg_migrate(client->send_data.marshaller, &migrate);
> +    spice_marshall_msg_migrate(m, &migrate);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int snd_playback_send_migrate(PlaybackChannelClient *client)
>  {
> -    return snd_channel_send_migrate(&client->base);
> +    return snd_channel_send_migrate(SND_CHANNEL_CLIENT(client));
>  }
>  
>  static int snd_send_volume(SndChannelClient *client, uint32_t cap, int msg)
>  {
>      SpiceMsgAudioVolume *vol;
>      uint8_t c;
> -    SpiceVolumeState *st = &client->channel->volume;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> +    SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
> +    SpiceVolumeState *st = &channel->volume;
>  
> -    if (!red_channel_client_test_remote_cap(client->channel_client, cap)) {
> +    if (!red_channel_client_test_remote_cap(rcc, cap)) {
>          return TRUE;
>      }
>  
>      vol = alloca(sizeof (SpiceMsgAudioVolume) +
>                   st->volume_nchannels * sizeof (uint16_t));
> -    if (!snd_reset_send_data(client, msg)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, msg, NULL);
>      vol->nchannels = st->volume_nchannels;
>      for (c = 0; c < st->volume_nchannels; ++c) {
>          vol->volume[c] = st->volume[c];
>      }
> -    spice_marshall_SpiceMsgAudioVolume(client->send_data.marshaller, vol);
> +    spice_marshall_SpiceMsgAudioVolume(m, vol);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int snd_playback_send_volume(PlaybackChannelClient *playback_client)
>  {
> -    return snd_send_volume(&playback_client->base, SPICE_PLAYBACK_CAP_VOLUME,
> +    return snd_send_volume(SND_CHANNEL_CLIENT(playback_client), SPICE_PLAYBACK_CAP_VOLUME,
>                             SPICE_MSG_PLAYBACK_VOLUME);
>  }
>  
>  static int snd_send_mute(SndChannelClient *client, uint32_t cap, int msg)
>  {
>      SpiceMsgAudioMute mute;
> -    SpiceVolumeState *st = &client->channel->volume;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> +    SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
> +    SpiceVolumeState *st = &channel->volume;
>  
> -    if (!red_channel_client_test_remote_cap(client->channel_client, cap)) {
> +    if (!red_channel_client_test_remote_cap(rcc, cap)) {
>          return TRUE;
>      }
>  
> -    if (!snd_reset_send_data(client, msg)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, msg, NULL);
>      mute.mute = st->mute;
> -    spice_marshall_SpiceMsgAudioMute(client->send_data.marshaller, &mute);
> +    spice_marshall_SpiceMsgAudioMute(m, &mute);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int snd_playback_send_mute(PlaybackChannelClient *playback_client)
>  {
> -    return snd_send_mute(&playback_client->base, SPICE_PLAYBACK_CAP_VOLUME,
> +    return snd_send_mute(SND_CHANNEL_CLIENT(playback_client), SPICE_PLAYBACK_CAP_VOLUME,
>                           SPICE_MSG_PLAYBACK_MUTE);
>  }
>  
>  static int snd_playback_send_latency(PlaybackChannelClient *playback_client)
>  {
> -    SndChannelClient *client = &playback_client->base;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>      SpiceMsgPlaybackLatency latency_msg;
>  
>      spice_debug("latency %u", playback_client->latency);
> -    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_LATENCY)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_LATENCY, NULL);
>      latency_msg.latency_ms = playback_client->latency;
> -    spice_marshall_msg_playback_latency(client->send_data.marshaller, &latency_msg);
> +    spice_marshall_msg_playback_latency(m, &latency_msg);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
> +
>  static int snd_playback_send_start(PlaybackChannelClient *playback_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)playback_client;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>      SpiceMsgPlaybackStart start;
>  
> -    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_START)) {
> -        return FALSE;
> -    }
> -
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_START, NULL);
>      start.channels = SPICE_INTERFACE_PLAYBACK_CHAN;
> -    start.frequency = client->channel->frequency;
> +    start.frequency = SND_CHANNEL(red_channel_client_get_channel(rcc))->frequency;
>      spice_assert(SPICE_INTERFACE_PLAYBACK_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
>      start.format = SPICE_AUDIO_FMT_S16;
>      start.time = reds_get_mm_time();
> -    spice_marshall_msg_playback_start(client->send_data.marshaller, &start);
> +    spice_marshall_msg_playback_start(m, &start);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int snd_playback_send_stop(PlaybackChannelClient *playback_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)playback_client;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
>  
> -    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_STOP)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_STOP, NULL);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int snd_playback_send_ctl(PlaybackChannelClient *playback_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)playback_client;
> +    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
>  
>      if ((client->client_active = client->active)) {
>          return snd_playback_send_start(playback_client);
> @@ -708,36 +493,35 @@ static int snd_playback_send_ctl(PlaybackChannelClient *playback_client)
>  
>  static int snd_record_send_start(RecordChannelClient *record_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)record_client;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(record_client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>      SpiceMsgRecordStart start;
>  
> -    if (!snd_reset_send_data(client, SPICE_MSG_RECORD_START)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_RECORD_START, NULL);
>  
>      start.channels = SPICE_INTERFACE_RECORD_CHAN;
> -    start.frequency = client->channel->frequency;
> +    start.frequency = SND_CHANNEL(red_channel_client_get_channel(rcc))->frequency;
>      spice_assert(SPICE_INTERFACE_RECORD_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
>      start.format = SPICE_AUDIO_FMT_S16;
> -    spice_marshall_msg_record_start(client->send_data.marshaller, &start);
> +    spice_marshall_msg_record_start(m, &start);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int snd_record_send_stop(RecordChannelClient *record_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)record_client;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(record_client);
>  
> -    if (!snd_reset_send_data(client, SPICE_MSG_RECORD_STOP)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_RECORD_STOP, NULL);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int snd_record_send_ctl(RecordChannelClient *record_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)record_client;
> +    SndChannelClient *client = SND_CHANNEL_CLIENT(record_client);
>  
>      if ((client->client_active = client->active)) {
>          return snd_record_send_start(record_client);
> @@ -748,13 +532,13 @@ static int snd_record_send_ctl(RecordChannelClient *record_client)
>  
>  static int snd_record_send_volume(RecordChannelClient *record_client)
>  {
> -    return snd_send_volume(&record_client->base, SPICE_RECORD_CAP_VOLUME,
> +    return snd_send_volume(SND_CHANNEL_CLIENT(record_client), SPICE_RECORD_CAP_VOLUME,
>                             SPICE_MSG_RECORD_VOLUME);
>  }
>  
>  static int snd_record_send_mute(RecordChannelClient *record_client)
>  {
> -    return snd_send_mute(&record_client->base, SPICE_RECORD_CAP_VOLUME,
> +    return snd_send_mute(SND_CHANNEL_CLIENT(record_client), SPICE_RECORD_CAP_VOLUME,
>                           SPICE_MSG_RECORD_MUTE);
>  }
>  
> @@ -764,27 +548,25 @@ static int snd_record_send_migrate(RecordChannelClient *record_client)
>       * the client receives RECORD_STOP from the src before the migration completion
>       * notification (when the vm is stopped).
>       * Afterwards, when the vm starts on the dest, the client receives RECORD_START. */
> -    return snd_channel_send_migrate(&record_client->base);
> +    return snd_channel_send_migrate(SND_CHANNEL_CLIENT(record_client));
>  }
>  
>  static int snd_playback_send_write(PlaybackChannelClient *playback_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)playback_client;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>      AudioFrame *frame;
>      SpiceMsgPlaybackPacket msg;
>  
> -    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_DATA)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_DATA, NULL);
>  
>      frame = playback_client->in_progress;
>      msg.time = frame->time;
>  
> -    spice_marshall_msg_playback_data(client->send_data.marshaller, &msg);
> +    spice_marshall_msg_playback_data(m, &msg);
>  
>      if (playback_client->mode == SPICE_AUDIO_DATA_MODE_RAW) {
> -        spice_marshaller_add_ref(client->send_data.marshaller,
> -                                 (uint8_t *)frame->samples,
> +        spice_marshaller_add_ref(m, (uint8_t *)frame->samples,
>                                   snd_codec_frame_size(playback_client->codec) * sizeof(frame->samples[0]));
>      }
>      else {
> @@ -793,45 +575,70 @@ static int snd_playback_send_write(PlaybackChannelClient *playback_client)
>                                      snd_codec_frame_size(playback_client->codec) * sizeof(frame->samples[0]),
>                                      playback_client->encode_buf, &n) != SND_CODEC_OK) {
>              spice_printerr("encode failed");
> -            snd_disconnect_channel(client);
> -            return FALSE;
> +            red_channel_client_disconnect(rcc);
> +            return TRUE;
>          }
> -        spice_marshaller_add_ref(client->send_data.marshaller, playback_client->encode_buf, n);
> +        spice_marshaller_add_ref(m, playback_client->encode_buf, n);
>      }
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
>  static int playback_send_mode(PlaybackChannelClient *playback_client)
>  {
> -    SndChannelClient *client = (SndChannelClient *)playback_client;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
> +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>      SpiceMsgPlaybackMode mode;
>  
> -    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_MODE)) {
> -        return FALSE;
> -    }
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_MODE, NULL);
>      mode.time = reds_get_mm_time();
>      mode.mode = playback_client->mode;
> -    spice_marshall_msg_playback_mode(client->send_data.marshaller, &mode);
> +    spice_marshall_msg_playback_mode(m, &mode);
>  
> -    return snd_begin_send_message(client);
> +    red_channel_client_begin_send_message(rcc);
> +    return FALSE;
>  }
>  
> -static void snd_playback_send(void* data)
> +static void snd_persistent_pipe_item_free(struct RedPipeItem *item)
>  {
> -    PlaybackChannelClient *playback_client = (PlaybackChannelClient*)data;
> -    SndChannelClient *client = (SndChannelClient*)playback_client;
> +    SndChannelClient *client = SPICE_CONTAINEROF(item, SndChannelClient, persistent_pipe_item);
> +
> +    red_pipe_item_init_full(item, RED_PIPE_ITEM_PERSISTENT,
> +                            snd_persistent_pipe_item_free);
> +
> +    if (client->on_message_done) {
> +        client->on_message_done(client);
> +    }
> +}
>  
> -    if (!playback_client || !snd_send_data(data)) {
> +static void snd_send(SndChannelClient * client)
> +{
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
> +
> +    if (!client || !red_channel_client_pipe_is_empty(rcc) || !client->command) {
>          return;
>      }
> +    // just append a dummy item and push!
> +    red_pipe_item_init_full(&client->persistent_pipe_item, RED_PIPE_ITEM_PERSISTENT,
> +                            snd_persistent_pipe_item_free);
> +    red_channel_client_pipe_add_push(rcc, &client->persistent_pipe_item);
> +}
>  
> +static void playback_channel_send_item(RedChannelClient *rcc, RedPipeItem *base)
> +{
> +    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(rcc);
> +    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
> +
> +    client->command &= SND_PLAYBACK_MODE_MASK|SND_PLAYBACK_PCM_MASK|
> +                       SND_CTRL_MASK|SND_VOLUME_MUTE_MASK|
> +                       SND_MIGRATE_MASK|SND_PLAYBACK_LATENCY_MASK;
>      while (client->command) {
>          if (client->command & SND_PLAYBACK_MODE_MASK) {
> +            client->command &= ~SND_PLAYBACK_MODE_MASK;
>              if (!playback_send_mode(playback_client)) {
> -                return;
> +                break;
>              }
> -            client->command &= ~SND_PLAYBACK_MODE_MASK;
>          }
>          if (client->command & SND_PLAYBACK_PCM_MASK) {
>              spice_assert(!playback_client->in_progress && playback_client->pending_frame);
> @@ -839,163 +646,77 @@ static void snd_playback_send(void* data)
>              playback_client->pending_frame = NULL;
>              client->command &= ~SND_PLAYBACK_PCM_MASK;
>              if (!snd_playback_send_write(playback_client)) {
> -                spice_printerr("snd_send_playback_write failed");
> -                return;
> +                break;
>              }
> +            spice_printerr("snd_send_playback_write failed");
>          }
>          if (client->command & SND_CTRL_MASK) {
> +            client->command &= ~SND_CTRL_MASK;
>              if (!snd_playback_send_ctl(playback_client)) {
> -                return;
> +                break;
>              }
> -            client->command &= ~SND_CTRL_MASK;
>          }
>          if (client->command & SND_VOLUME_MASK) {
> -            if (!snd_playback_send_volume(playback_client) ||
> -                !snd_playback_send_mute(playback_client)) {
> -                return;
> -            }
>              client->command &= ~SND_VOLUME_MASK;
> +            if (!snd_playback_send_volume(playback_client)) {
> +                break;
> +            }
> +        }
> +        if (client->command & SND_MUTE_MASK) {
> +            client->command &= ~SND_MUTE_MASK;
> +            if (!snd_playback_send_mute(playback_client)) {
> +                break;
> +            }
>          }
>          if (client->command & SND_MIGRATE_MASK) {
> +            client->command &= ~SND_MIGRATE_MASK;
>              if (!snd_playback_send_migrate(playback_client)) {
> -                return;
> +                break;
>              }
> -            client->command &= ~SND_MIGRATE_MASK;
>          }
>          if (client->command & SND_PLAYBACK_LATENCY_MASK) {
> +            client->command &= ~SND_PLAYBACK_LATENCY_MASK;
>              if (!snd_playback_send_latency(playback_client)) {
> -                return;
> +                break;
>              }
> -            client->command &= ~SND_PLAYBACK_LATENCY_MASK;
>          }
>      }
> +    snd_send(client);
>  }
>  
> -static void snd_record_send(void* data)
> +static void record_channel_send_item(RedChannelClient *rcc, RedPipeItem *base)
>  {
> -    RecordChannelClient *record_client = (RecordChannelClient*)data;
> -    SndChannelClient *client = (SndChannelClient*)record_client;
> -
> -    if (!record_client || !snd_send_data(data)) {
> -        return;
> -    }
> +    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(rcc);
> +    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
>  
> +    client->command &= SND_CTRL_MASK|SND_VOLUME_MUTE_MASK|SND_MIGRATE_MASK;
>      while (client->command) {
>          if (client->command & SND_CTRL_MASK) {
> +            client->command &= ~SND_CTRL_MASK;
>              if (!snd_record_send_ctl(record_client)) {
> -                return;
> +                break;
>              }
> -            client->command &= ~SND_CTRL_MASK;
>          }
>          if (client->command & SND_VOLUME_MASK) {
> -            if (!snd_record_send_volume(record_client) ||
> -                !snd_record_send_mute(record_client)) {
> -                return;
> -            }
>              client->command &= ~SND_VOLUME_MASK;
> +            if (!snd_record_send_volume(record_client)) {
> +                break;
> +            }
> +        }
> +        if (client->command & SND_MUTE_MASK) {
> +            client->command &= ~SND_MUTE_MASK;
> +            if (!snd_record_send_mute(record_client)) {
> +                break;
> +            }
>          }
>          if (client->command & SND_MIGRATE_MASK) {
> +            client->command &= ~SND_MIGRATE_MASK;
>              if (!snd_record_send_migrate(record_client)) {
> -                return;
> +                break;
>              }
> -            client->command &= ~SND_MIGRATE_MASK;
> -        }
> -    }
> -}
> -
> -static SndChannelClient *__new_channel(SndChannel *channel, int size, uint32_t channel_id,
> -                                 RedClient *red_client,
> -                                 RedsStream *stream,
> -                                 int migrate,
> -                                 snd_channel_send_messages_proc send_messages,
> -                                 snd_channel_handle_message_proc handle_message,
> -                                 snd_channel_on_message_done_proc on_message_done,
> -                                 snd_channel_cleanup_channel_proc cleanup,
> -                                 uint32_t *common_caps, int num_common_caps,
> -                                 uint32_t *caps, int num_caps)
> -{
> -    SndChannelClient *client;
> -    int delay_val;
> -    int flags;
> -#ifdef SO_PRIORITY
> -    int priority;
> -#endif
> -    int tos;
> -    MainChannelClient *mcc = red_client_get_main(red_client);
> -    RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
> -
> -    spice_assert(stream);
> -    if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
> -        spice_printerr("accept failed, %s", strerror(errno));
> -        goto error1;
> -    }
> -
> -#ifdef SO_PRIORITY
> -    priority = 6;
> -    if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY, (void*)&priority,
> -                   sizeof(priority)) == -1) {
> -        if (errno != ENOTSUP) {
> -            spice_printerr("setsockopt failed, %s", strerror(errno));
>          }
>      }
> -#endif
> -
> -    tos = IPTOS_LOWDELAY;
> -    if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*)&tos, sizeof(tos)) == -1) {
> -        if (errno != ENOTSUP) {
> -            spice_printerr("setsockopt failed, %s", strerror(errno));
> -        }
> -    }
> -
> -    delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
> -    if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val, sizeof(delay_val)) == -1) {
> -        if (errno != ENOTSUP) {
> -            spice_printerr("setsockopt failed, %s", strerror(errno));
> -        }
> -    }
> -
> -    if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> -        spice_printerr("accept failed, %s", strerror(errno));
> -        goto error1;
> -    }
> -
> -    spice_assert(size >= sizeof(*client));
> -    client = spice_malloc0(size);
> -    client->refs = 1;
> -    client->parser = spice_get_client_channel_parser(channel_id, NULL);
> -    client->stream = stream;
> -    client->channel = channel;
> -    client->receive_data.message_start = client->receive_data.buf;
> -    client->receive_data.now = client->receive_data.buf;
> -    client->receive_data.end = client->receive_data.buf + sizeof(client->receive_data.buf);
> -    client->send_data.marshaller = spice_marshaller_new();
> -
> -    stream->watch = reds_core_watch_add(reds, stream->socket, SPICE_WATCH_EVENT_READ,
> -                                        snd_event, client);
> -    if (stream->watch == NULL) {
> -        spice_printerr("watch_add failed, %s", strerror(errno));
> -        goto error2;
> -    }
> -
> -    client->send_messages = send_messages;
> -    client->handle_message = handle_message;
> -    client->on_message_done = on_message_done;
> -    client->cleanup = cleanup;
> -
> -    client->channel_client =
> -        dummy_channel_client_create(RED_CHANNEL(channel), red_client,
> -                                    num_common_caps, common_caps, num_caps, caps);
> -    if (!client->channel_client) {
> -        goto error2;
> -    }
> -    return client;
> -
> -error2:
> -    free(client);
> -
> -error1:
> -    reds_stream_free(stream);
> -    return NULL;
> +    snd_send(client);
>  }
>  
>  static int snd_channel_config_socket(RedChannelClient *rcc)
> @@ -1047,6 +768,22 @@ static int snd_channel_config_socket(RedChannelClient *rcc)
>      return TRUE;
>  }
>  
> +static uint8_t*
> +snd_channel_client_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size)
> +{
> +    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
> +    // TODO isn't too much the buffer ?? return NULL ??
> +    spice_assert(size < sizeof(client->receive_buf));
> +    return client->receive_buf;
> +}
> +
> +static void
> +snd_channel_client_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size,
> +                                    uint8_t *msg)
> +{
> +}
> +
> +// TODO remove and just use red_channel_client_disconnect ??
>  static void snd_disconnect_channel_client(RedChannelClient *rcc)
>  {
>      SndChannel *channel;
> @@ -1060,8 +797,8 @@ static void snd_disconnect_channel_client(RedChannelClient *rcc)
>  
>      spice_debug("channel-type=%d", type);
>      if (channel->connection) {
> -        spice_assert(channel->connection->channel_client == rcc);
> -        snd_disconnect_channel(channel->connection);
> +        spice_assert(RED_CHANNEL_CLIENT(channel->connection) == rcc);
> +        red_channel_client_disconnect(rcc);
>      }
>  }
>  
> @@ -1079,7 +816,6 @@ SPICE_GNUC_VISIBLE void spice_server_playback_set_volume(SpicePlaybackInstance *
>  {
>      SpiceVolumeState *st = &sin->st->channel.volume;
>      SndChannelClient *client = sin->st->channel.connection;
> -    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
>  
>      st->volume_nchannels = nchannels;
>      free(st->volume);
> @@ -1088,21 +824,22 @@ SPICE_GNUC_VISIBLE void spice_server_playback_set_volume(SpicePlaybackInstance *
>      if (!client || nchannels == 0)
>          return;
>  
> -    snd_playback_send_volume(playback_client);
> +    snd_set_command(client, SND_VOLUME_MUTE_MASK);
> +    snd_send(client);
>  }
>  
>  SPICE_GNUC_VISIBLE void spice_server_playback_set_mute(SpicePlaybackInstance *sin, uint8_t mute)
>  {
>      SpiceVolumeState *st = &sin->st->channel.volume;
>      SndChannelClient *client = sin->st->channel.connection;
> -    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
>  
>      st->mute = mute;
>  
>      if (!client)
>          return;
>  
> -    snd_playback_send_mute(playback_client);
> +    snd_set_command(client, SND_VOLUME_MUTE_MASK);
> +    snd_send(client);
>  }
>  
>  static void snd_playback_start(SndChannel *channel)
> @@ -1117,7 +854,7 @@ static void snd_playback_start(SndChannel *channel)
>      client->active = TRUE;
>      if (!client->client_active) {
>          snd_set_command(client, SND_CTRL_MASK);
> -        snd_playback_send(client);
> +        snd_send(client);
>      } else {
>          client->command &= ~SND_CTRL_MASK;
>      }
> @@ -1131,20 +868,20 @@ SPICE_GNUC_VISIBLE void spice_server_playback_start(SpicePlaybackInstance *sin)
>  SPICE_GNUC_VISIBLE void spice_server_playback_stop(SpicePlaybackInstance *sin)
>  {
>      SndChannelClient *client = sin->st->channel.connection;
> -    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
>  
>      sin->st->channel.active = 0;
>      if (!client)
>          return;
> -    spice_assert(playback_client->base.active);
> +    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(client);
> +    spice_assert(client->active);
>      reds_enable_mm_time(snd_channel_get_server(client));
> -    playback_client->base.active = FALSE;
> -    if (playback_client->base.client_active) {
> -        snd_set_command(&playback_client->base, SND_CTRL_MASK);
> -        snd_playback_send(&playback_client->base);
> +    client->active = FALSE;
> +    if (client->client_active) {
> +        snd_set_command(client, SND_CTRL_MASK);
> +        snd_send(client);
>      } else {
> -        playback_client->base.command &= ~SND_CTRL_MASK;
> -        playback_client->base.command &= ~SND_PLAYBACK_PCM_MASK;
> +        client->command &= ~SND_CTRL_MASK;
> +        client->command &= ~SND_PLAYBACK_PCM_MASK;
>  
>          if (playback_client->pending_frame) {
>              spice_assert(!playback_client->in_progress);
> @@ -1159,15 +896,21 @@ SPICE_GNUC_VISIBLE void spice_server_playback_get_buffer(SpicePlaybackInstance *
>                                                           uint32_t **frame, uint32_t *num_samples)
>  {
>      SndChannelClient *client = sin->st->channel.connection;
> -    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
>  
> -    if (!client || !playback_client->free_frames) {
> -        *frame = NULL;
> -        *num_samples = 0;
> +    *frame = NULL;
> +    *num_samples = 0;
> +    if (!client) {
> +        return;
> +    }
> +    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(client);
> +    if (!playback_client->free_frames) {
>          return;
>      }
> -    spice_assert(playback_client->base.active);
> -    snd_channel_ref(client);
> +    spice_assert(client->active);
> +    if (!playback_client->free_frames->allocated) {
> +        playback_client->free_frames->allocated = TRUE;
> +        ++playback_client->frames->refs;
> +    }
>  
>      *frame = playback_client->free_frames->samples;
>      playback_client->free_frames = playback_client->free_frames->next;
> @@ -1180,23 +923,28 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
>      AudioFrame *frame;
>  
>      frame = SPICE_CONTAINEROF(samples, AudioFrame, samples[0]);
> +    if (frame->allocated) {
> +        frame->allocated = FALSE;
> +        if (--frame->container->refs == 0) {
> +            free(frame->container);
> +            return;
> +        }
> +    }
>      playback_client = frame->client;
> -    spice_assert(playback_client);
> -    if (!snd_channel_unref(&playback_client->base) ||
> -        sin->st->channel.connection != &playback_client->base) {
> +    if (!playback_client || sin->st->channel.connection != SND_CHANNEL_CLIENT(playback_client)) {
>          /* lost last reference, client has been destroyed previously */
>          spice_info("audio samples belong to a disconnected client");
>          return;
>      }
> -    spice_assert(playback_client->base.active);
> +    spice_assert(SND_CHANNEL_CLIENT(playback_client)->active);
>  
>      if (playback_client->pending_frame) {
>          snd_playback_free_frame(playback_client, playback_client->pending_frame);
>      }
>      frame->time = reds_get_mm_time();
>      playback_client->pending_frame = frame;
> -    snd_set_command(&playback_client->base, SND_PLAYBACK_PCM_MASK);
> -    snd_playback_send(&playback_client->base);
> +    snd_set_command(SND_CHANNEL_CLIENT(playback_client), SND_PLAYBACK_PCM_MASK);
> +    snd_send(SND_CHANNEL_CLIENT(playback_client));
>  }
>  
>  void snd_set_playback_latency(RedClient *client, uint32_t latency)
> @@ -1207,15 +955,15 @@ void snd_set_playback_latency(RedClient *client, uint32_t latency)
>          uint32_t type;
>          g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
>          if (type == SPICE_CHANNEL_PLAYBACK && now->connection &&
> -            red_channel_client_get_client(now->connection->channel_client) == client) {
> +            red_channel_client_get_client(RED_CHANNEL_CLIENT(now->connection)) == client) {
>  
> -            if (red_channel_client_test_remote_cap(now->connection->channel_client,
> +            if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(now->connection),
>                  SPICE_PLAYBACK_CAP_LATENCY)) {
>                  PlaybackChannelClient* playback = (PlaybackChannelClient*)now->connection;
>  
>                  playback->latency = latency;
>                  snd_set_command(now->connection, SND_PLAYBACK_LATENCY_MASK);
> -                snd_playback_send(now->connection);
> +                snd_send(now->connection);
>              } else {
>                  spice_debug("client doesn't not support SPICE_PLAYBACK_CAP_LATENCY");
>              }
> @@ -1250,60 +998,58 @@ static void on_new_playback_channel(SndChannel *channel, SndChannelClient *snd_c
>          snd_set_command(snd_channel, SND_CTRL_MASK);
>      }
>      if (channel->volume.volume_nchannels) {
> -        snd_set_command(snd_channel, SND_VOLUME_MASK);
> +        snd_set_command(snd_channel, SND_VOLUME_MUTE_MASK);
>      }
>      if (snd_channel->active) {
>          reds_disable_mm_time(reds);
>      }
>  }
>  
> -static void snd_playback_cleanup(SndChannelClient *client)
> +static void
> +playback_channel_client_finalize(GObject *object)
>  {
> -    PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
> +    int i;
> +    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(object);
> +    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
>  
> -    if (playback_client->base.active) {
> +    // free frames, unref them
> +    for (i = 0; i < NUM_AUDIO_FRAMES; ++i) {
> +        playback_client->frames->items[i].client = NULL;
> +    }
> +    if (--playback_client->frames->refs == 0) {
> +        free(playback_client->frames);
> +    }
> +
> +    if (client->active) {
>          reds_enable_mm_time(snd_channel_get_server(client));
>      }
>  
>      snd_codec_destroy(&playback_client->codec);
> +
> +    G_OBJECT_CLASS(playback_channel_client_parent_class)->finalize(object);
>  }
>  
> -static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream,
> -                                  int migration, int num_common_caps, uint32_t *common_caps,
> -                                  int num_caps, uint32_t *caps)
> +static void
> +playback_channel_client_constructed(GObject *object)
>  {
> +    PlaybackChannelClient *playback_client = PLAYBACK_CHANNEL_CLIENT(object);
> +    RedChannel *red_channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(playback_client));
> +    RedClient *client = red_channel_client_get_client(RED_CHANNEL_CLIENT(playback_client));
>      SndChannel *channel = SND_CHANNEL(red_channel);
> -    PlaybackChannelClient *playback_client;
>  
> -    snd_disconnect_channel(channel->connection);
> -
> -    if (!(playback_client = (PlaybackChannelClient *)__new_channel(channel,
> -                                                              sizeof(*playback_client),
> -                                                              SPICE_CHANNEL_PLAYBACK,
> -                                                              client,
> -                                                              stream,
> -                                                              migration,
> -                                                              snd_playback_send,
> -                                                              snd_playback_handle_message,
> -                                                              snd_playback_on_message_done,
> -                                                              snd_playback_cleanup,
> -                                                              common_caps, num_common_caps,
> -                                                              caps, num_caps))) {
> -        return;
> -    }
> -    snd_playback_free_frame(playback_client, &playback_client->frames[0]);
> -    snd_playback_free_frame(playback_client, &playback_client->frames[1]);
> -    snd_playback_free_frame(playback_client, &playback_client->frames[2]);
> +    G_OBJECT_CLASS(playback_channel_client_parent_class)->constructed(object);
>  
> -    int client_can_celt = red_channel_client_test_remote_cap(playback_client->base.channel_client,
> +    SND_CHANNEL_CLIENT(playback_client)->on_message_done = snd_playback_on_message_done;
> +
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
> +    int client_can_celt = red_channel_client_test_remote_cap(rcc,
>                                            SPICE_PLAYBACK_CAP_CELT_0_5_1);
> -    int client_can_opus = red_channel_client_test_remote_cap(playback_client->base.channel_client,
> +    int client_can_opus = red_channel_client_test_remote_cap(rcc,
>                                            SPICE_PLAYBACK_CAP_OPUS);
>      int playback_compression =
>          reds_config_get_playback_compression(red_channel_get_server(red_channel));
>      int desired_mode = snd_desired_audio_mode(playback_compression, channel->frequency,
>                                                client_can_celt, client_can_opus);
> -    playback_client->mode = SPICE_AUDIO_DATA_MODE_RAW;
>      if (desired_mode != SPICE_AUDIO_DATA_MODE_RAW) {
>          if (snd_codec_create(&playback_client->codec, desired_mode, channel->frequency,
>                               SND_CODEC_ENCODE) == SND_CODEC_OK) {
> @@ -1314,13 +1060,52 @@ static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, Re
>      }
>  
>      if (!red_client_during_migrate_at_target(client)) {
> -        on_new_playback_channel(channel, &playback_client->base);
> +        on_new_playback_channel(channel, SND_CHANNEL_CLIENT(playback_client));
>      }
>  
>      if (channel->active) {
>          snd_playback_start(channel);
>      }
> -    snd_playback_send(channel->connection);
> +    snd_send(SND_CHANNEL_CLIENT(playback_client));
> +}
> +
> +static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream,
> +                                  int migration, int num_common_caps, uint32_t *common_caps,
> +                                  int num_caps, uint32_t *caps)
> +{
> +    SndChannel *channel = SND_CHANNEL(red_channel);
> +    GArray *common_caps_array = NULL, *caps_array = NULL;
> +
> +    if (channel->connection) {
> +        red_channel_client_disconnect(RED_CHANNEL_CLIENT(channel->connection));
> +        channel->connection = NULL;
> +    }
> +
> +    if (common_caps) {
> +        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
> +                                              num_common_caps);
> +        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
> +    }
> +    if (caps) {
> +        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
> +        g_array_append_vals(caps_array, caps, num_caps);
> +    }
> +
> +    g_initable_new(TYPE_PLAYBACK_CHANNEL_CLIENT,
> +                   NULL, NULL,
> +                   "channel", channel,
> +                   "client", client,
> +                   "stream", stream,
> +                   "caps", caps_array,
> +                   "common-caps", common_caps_array,
> +                   NULL);
> +
> +    if (caps_array) {
> +        g_array_unref(caps_array);
> +    }
> +    if (common_caps_array) {
> +        g_array_unref(common_caps_array);
> +    }
>  }
>  
>  static void snd_record_migrate_channel_client(RedChannelClient *rcc)
> @@ -1333,9 +1118,9 @@ static void snd_record_migrate_channel_client(RedChannelClient *rcc)
>      spice_assert(channel);
>  
>      if (channel->connection) {
> -        spice_assert(channel->connection->channel_client == rcc);
> +        spice_assert(RED_CHANNEL_CLIENT(channel->connection) == rcc);
>          snd_set_command(channel->connection, SND_MIGRATE_MASK);
> -        snd_record_send(channel->connection);
> +        snd_send(channel->connection);
>      }
>  }
>  
> @@ -1345,7 +1130,6 @@ SPICE_GNUC_VISIBLE void spice_server_record_set_volume(SpiceRecordInstance *sin,
>  {
>      SpiceVolumeState *st = &sin->st->channel.volume;
>      SndChannelClient *client = sin->st->channel.connection;
> -    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
>  
>      st->volume_nchannels = nchannels;
>      free(st->volume);
> @@ -1354,38 +1138,40 @@ SPICE_GNUC_VISIBLE void spice_server_record_set_volume(SpiceRecordInstance *sin,
>      if (!client || nchannels == 0)
>          return;
>  
> -    snd_record_send_volume(record_client);
> +    snd_set_command(client, SND_VOLUME_MUTE_MASK);
> +    snd_send(client);
>  }
>  
>  SPICE_GNUC_VISIBLE void spice_server_record_set_mute(SpiceRecordInstance *sin, uint8_t mute)
>  {
>      SpiceVolumeState *st = &sin->st->channel.volume;
>      SndChannelClient *client = sin->st->channel.connection;
> -    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
>  
>      st->mute = mute;
>  
>      if (!client)
>          return;
>  
> -    snd_record_send_mute(record_client);
> +    snd_set_command(client, SND_VOLUME_MUTE_MASK);
> +    snd_send(client);
>  }
>  
>  static void snd_record_start(SndChannel *channel)
>  {
>      SndChannelClient *client = channel->connection;
> -    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
>  
>      channel->active = 1;
> -    if (!client)
> +    if (!client) {
>          return;
> +    }
> +    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(client);
>      spice_assert(!client->active);
>      record_client->read_pos = record_client->write_pos = 0;   //todo: improve by
>                                                                  //stream generation
>      client->active = TRUE;
>      if (!client->client_active) {
>          snd_set_command(client, SND_CTRL_MASK);
> -        snd_record_send(client);
> +        snd_send(client);
>      } else {
>          client->command &= ~SND_CTRL_MASK;
>      }
> @@ -1399,18 +1185,17 @@ SPICE_GNUC_VISIBLE void spice_server_record_start(SpiceRecordInstance *sin)
>  SPICE_GNUC_VISIBLE void spice_server_record_stop(SpiceRecordInstance *sin)
>  {
>      SndChannelClient *client = sin->st->channel.connection;
> -    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
>  
>      sin->st->channel.active = 0;
>      if (!client)
>          return;
> -    spice_assert(record_client->base.active);
> -    record_client->base.active = FALSE;
> -    if (record_client->base.client_active) {
> -        snd_set_command(&record_client->base, SND_CTRL_MASK);
> -        snd_record_send(&record_client->base);
> +    spice_assert(client->active);
> +    client->active = FALSE;
> +    if (client->client_active) {
> +        snd_set_command(client, SND_CTRL_MASK);
> +        snd_send(client);
>      } else {
> -        record_client->base.command &= ~SND_CTRL_MASK;
> +        client->command &= ~SND_CTRL_MASK;
>      }
>  }
>  
> @@ -1418,14 +1203,14 @@ SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
>                                                              uint32_t *samples, uint32_t bufsize)
>  {
>      SndChannelClient *client = sin->st->channel.connection;
> -    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
>      uint32_t read_pos;
>      uint32_t now;
>      uint32_t len;
>  
>      if (!client)
>          return 0;
> -    spice_assert(record_client->base.active);
> +    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(client);
> +    spice_assert(client->active);
>  
>      if (record_client->write_pos < RECORD_SAMPLES_SIZE / 2) {
>          return 0;
> @@ -1433,14 +1218,17 @@ SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
>  
>      len = MIN(record_client->write_pos - record_client->read_pos, bufsize);
>  
> +    // TODO why ?? stream already call if got data
> +#if 0
>      if (len < bufsize) {
> -        SndChannel *channel = record_client->base.channel;
> -        snd_receive(&record_client->base);
> +        SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(RED_CHANNEL_CLIENT(client)));
> +        snd_receive(client);
>          if (!channel->connection) {
>              return 0;
>          }
>          len = MIN(record_client->write_pos - record_client->read_pos, bufsize);
>      }
> +#endif
>  
>      read_pos = record_client->read_pos % RECORD_SAMPLES_SIZE;
>      record_client->read_pos += len;
> @@ -1456,7 +1244,7 @@ static uint32_t snd_get_best_rate(SndChannelClient *client, uint32_t cap_opus)
>  {
>      int client_can_opus = TRUE;
>      if (client) {
> -        client_can_opus = red_channel_client_test_remote_cap(client->channel_client, cap_opus);
> +        client_can_opus = red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(client), cap_opus);
>      }
>  
>      if (client_can_opus && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS, SND_CODEC_ANY_FREQUENCY))
> @@ -1498,52 +1286,79 @@ static void on_new_record_channel(SndChannel *channel, SndChannelClient *snd_cha
>  {
>      spice_assert(snd_channel);
>  
> -    channel->connection = snd_channel ;
> +    channel->connection = snd_channel;
>      if (channel->volume.volume_nchannels) {
> -        snd_set_command(snd_channel, SND_VOLUME_MASK);
> +        snd_set_command(snd_channel, SND_VOLUME_MUTE_MASK);
>      }
>      if (snd_channel->active) {
>          snd_set_command(snd_channel, SND_CTRL_MASK);
>      }
>  }
>  
> -static void snd_record_cleanup(SndChannelClient *client)
> +static void
> +record_channel_client_finalize(GObject *object)
>  {
> -    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
> +    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(object);
> +
>      snd_codec_destroy(&record_client->codec);
> +
> +    G_OBJECT_CLASS(record_channel_client_parent_class)->finalize(object);
>  }
>  
> +static void
> +record_channel_client_constructed(GObject *object)
> +{
> +    RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(object);
> +    RedChannel *red_channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(record_client));
> +    SndChannel *channel = SND_CHANNEL(red_channel);
> +
> +    G_OBJECT_CLASS(record_channel_client_parent_class)->constructed(object);
> +
> +    on_new_record_channel(channel, SND_CHANNEL_CLIENT(record_client));
> +    if (channel->active) {
> +        snd_record_start(channel);
> +    }
> +    snd_send(SND_CHANNEL_CLIENT(record_client));
> +}
> +
> +
>  static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream,
>                                  int migration, int num_common_caps, uint32_t *common_caps,
>                                  int num_caps, uint32_t *caps)
>  {
>      SndChannel *channel = SND_CHANNEL(red_channel);
> -    RecordChannelClient *record_client;
> -
> -    snd_disconnect_channel(channel->connection);
> -
> -    if (!(record_client = (RecordChannelClient *)__new_channel(channel,
> -                                                          sizeof(*record_client),
> -                                                          SPICE_CHANNEL_RECORD,
> -                                                          client,
> -                                                          stream,
> -                                                          migration,
> -                                                          snd_record_send,
> -                                                          snd_record_handle_message,
> -                                                          snd_record_on_message_done,
> -                                                          snd_record_cleanup,
> -                                                          common_caps, num_common_caps,
> -                                                          caps, num_caps))) {
> -        return;
> +    GArray *common_caps_array = NULL, *caps_array = NULL;
> +
> +    if (channel->connection) {
> +        red_channel_client_disconnect(RED_CHANNEL_CLIENT(channel->connection));
> +        channel->connection = NULL;
>      }
>  
> -    record_client->mode = SPICE_AUDIO_DATA_MODE_RAW;
> +    if (common_caps) {
> +        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
> +                                              num_common_caps);
> +        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
> +    }
> +    if (caps) {
> +        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
> +        g_array_append_vals(caps_array, caps, num_caps);
> +    }
>  
> -    on_new_record_channel(channel, &record_client->base);
> -    if (channel->active) {
> -        snd_record_start(channel);
> +    g_initable_new(TYPE_RECORD_CHANNEL_CLIENT,
> +                   NULL, NULL,
> +                   "channel", channel,
> +                   "client", client,
> +                   "stream", stream,
> +                   "caps", caps_array,
> +                   "common-caps", common_caps_array,
> +                   NULL);
> +
> +    if (caps_array) {
> +        g_array_unref(caps_array);
> +    }
> +    if (common_caps_array) {
> +        g_array_unref(common_caps_array);
>      }
> -    snd_record_send(channel->connection);
>  }
>  
>  static void snd_playback_migrate_channel_client(RedChannelClient *rcc)
> @@ -1557,9 +1372,9 @@ static void snd_playback_migrate_channel_client(RedChannelClient *rcc)
>      spice_debug(NULL);
>  
>      if (channel->connection) {
> -        spice_assert(channel->connection->channel_client == rcc);
> +        spice_assert(RED_CHANNEL_CLIENT(channel->connection) == rcc);
>          snd_set_command(channel->connection, SND_MIGRATE_MASK);
> -        snd_playback_send(channel->connection);
> +        snd_send(channel->connection);
>      }
>  }
>  
> @@ -1593,6 +1408,8 @@ snd_channel_class_init(SndChannelClass *klass)
>  {
>      RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>      channel_class->config_socket = snd_channel_config_socket;
> +    channel_class->alloc_recv_buf = snd_channel_client_alloc_recv_buf;
> +    channel_class->release_recv_buf = snd_channel_client_release_recv_buf;
>  }
>  
>  static void
> @@ -1627,8 +1444,13 @@ static void
>  playback_channel_class_init(SndChannelClass *klass)
>  {
>      GObjectClass *object_class = G_OBJECT_CLASS(klass);
> +    RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>  
>      object_class->constructed = playback_channel_constructed;
> +
> +    channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_PLAYBACK, NULL);
> +    channel_class->handle_parsed = playback_channel_handle_parsed;
> +    channel_class->send_item = playback_channel_send_item;
>  }
>  
>  void snd_attach_playback(RedsState *reds, SpicePlaybackInstance *sin)
> @@ -1673,8 +1495,13 @@ static void
>  record_channel_class_init(SndChannelClass *klass)
>  {
>      GObjectClass *object_class = G_OBJECT_CLASS(klass);
> +    RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>  
>      object_class->constructed = record_channel_constructed;
> +
> +    channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_RECORD, NULL);
> +    channel_class->handle_parsed = record_channel_handle_parsed;
> +    channel_class->send_item = record_channel_send_item;
>  }
>  
>  void snd_attach_record(RedsState *reds, SpiceRecordInstance *sin)
> @@ -1695,7 +1522,6 @@ static void snd_detach_common(SndChannel *channel)
>      RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
>  
>      remove_channel(channel);
> -    snd_disconnect_channel(channel->connection);
>      reds_unregister_channel(reds, RED_CHANNEL(channel));
>      free(channel->volume.volume);
>      channel->volume.volume = NULL;
> @@ -1721,9 +1547,10 @@ void snd_set_playback_compression(int on)
>          g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
>          if (type == SPICE_CHANNEL_PLAYBACK && now->connection) {
>              PlaybackChannelClient* playback = (PlaybackChannelClient*)now->connection;
> -            int client_can_celt = red_channel_client_test_remote_cap(playback->base.channel_client,
> +            RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback);
> +            int client_can_celt = red_channel_client_test_remote_cap(rcc,
>                                      SPICE_PLAYBACK_CAP_CELT_0_5_1);
> -            int client_can_opus = red_channel_client_test_remote_cap(playback->base.channel_client,
> +            int client_can_opus = red_channel_client_test_remote_cap(rcc,
>                                      SPICE_PLAYBACK_CAP_OPUS);
>              int desired_mode = snd_desired_audio_mode(on, now->frequency,
>                                                        client_can_opus, client_can_celt);
> @@ -1734,3 +1561,50 @@ void snd_set_playback_compression(int on)
>          }
>      }
>  }
> +
> +static void
> +snd_channel_client_class_init(SndChannelClientClass *self)
> +{
> +}
> +
> +static void
> +snd_channel_client_init(SndChannelClient *self)
> +{
> +}
> +
> +static void
> +playback_channel_client_class_init(PlaybackChannelClientClass *klass)
> +{
> +    GObjectClass *object_class = G_OBJECT_CLASS(klass);
> +    object_class->constructed = playback_channel_client_constructed;
> +    object_class->finalize = playback_channel_client_finalize;
> +}
> +
> +static void
> +playback_channel_client_init(PlaybackChannelClient *playback)
> +{
> +    int i;
> +
> +    playback->mode = SPICE_AUDIO_DATA_MODE_RAW;
> +
> +    playback->frames = spice_new0(AudioFrameContainer, 1);
> +    playback->frames->refs = 1;
> +    for (i = 0; i < NUM_AUDIO_FRAMES; ++i) {
> +        playback->frames->items[i].container = playback->frames;
> +        snd_playback_free_frame(playback, &playback->frames->items[i]);
> +    }
> +}
> +
> +static void
> +record_channel_client_class_init(RecordChannelClientClass *klass)
> +{
> +    GObjectClass *object_class = G_OBJECT_CLASS(klass);
> +    object_class->constructed = record_channel_client_constructed;
> +    object_class->finalize = record_channel_client_finalize;
> +}
> +
> +static void
> +record_channel_client_init(RecordChannelClient *record)
> +{
> +    record->mode = SPICE_AUDIO_DATA_MODE_RAW;
> +}
> -- 
> 2.7.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> Hey,
> 
> On Wed, Nov 23, 2016 at 06:07:25PM +0000, Frediano Ziglio wrote:
> > This patch is quite huge but have some benefits:
> > - reduce dependency (DummyChannel* and some RedChannelClient internals);
> > - reuse RedChannelClient instead of reading from the RedsStream
> >   directly.
> > 
> > You can see that SndChannelClient has much less field
> > as the code to read/write from/to client is reused from
> > RedChannelClient instead of creating a fake RedChannelClient
> > just to make the system happy.
> > 
> I started looking at this, and it's really big/hard to review with all
> these changes :(
> Wondering if the AudioFrame allocation change could be done before, if
> we could have a few cosmetic commits first introducing
> SpiceMarshaller *m = client->send_data.marshaller; in snd_send_xxx()
> Maybe separate the GObjectification by itself from switching to using
> facilities provided by RedChannelClient, ...
> Dunno if the snd_set_command/snd_send API can be introduced before the
> gobjectification
> 
> Just a few random thoughts after reading this patch in the hope that it
> can be made simpler, some of them are probably not practical at all :(
> 
> Christophe
> 

Could be.
AudioFrame and possibly SpiceMarshaller and some other could be
split, snd_send and snd_set_command I don't think so, they rely to
reusing RedChannelClient base.
Have you considered that some patches are quite intermediate and not
expected to run?
Would be quite frustrating if I spend some time to split them to
finally squash to have all commits working.

Frediano
On Tue, Nov 29, 2016 at 10:16:04AM -0500, Frediano Ziglio wrote:
> > 
> > Hey,
> > 
> > On Wed, Nov 23, 2016 at 06:07:25PM +0000, Frediano Ziglio wrote:
> > > This patch is quite huge but have some benefits:
> > > - reduce dependency (DummyChannel* and some RedChannelClient internals);
> > > - reuse RedChannelClient instead of reading from the RedsStream
> > >   directly.
> > > 
> > > You can see that SndChannelClient has much less field
> > > as the code to read/write from/to client is reused from
> > > RedChannelClient instead of creating a fake RedChannelClient
> > > just to make the system happy.
> > > 
> > I started looking at this, and it's really big/hard to review with all
> > these changes :(
> > Wondering if the AudioFrame allocation change could be done before, if
> > we could have a few cosmetic commits first introducing
> > SpiceMarshaller *m = client->send_data.marshaller; in snd_send_xxx()
> > Maybe separate the GObjectification by itself from switching to using
> > facilities provided by RedChannelClient, ...
> > Dunno if the snd_set_command/snd_send API can be introduced before the
> > gobjectification
> > 
> > Just a few random thoughts after reading this patch in the hope that it
> > can be made simpler, some of them are probably not practical at all :(
> > 
> > Christophe
> > 
> 
> Could be.
> AudioFrame and possibly SpiceMarshaller and some other could be
> split, snd_send and snd_set_command I don't think so, they rely to
> reusing RedChannelClient base.
> Have you considered that some patches are quite intermediate and not
> expected to run?

Yes, these suggestions are patches that seems to be possible _before_
these gobject patches, I guess anything before that is not going to need
to be squashed?

Christophe
> 
> On Tue, Nov 29, 2016 at 10:16:04AM -0500, Frediano Ziglio wrote:
> > > 
> > > Hey,
> > > 
> > > On Wed, Nov 23, 2016 at 06:07:25PM +0000, Frediano Ziglio wrote:
> > > > This patch is quite huge but have some benefits:
> > > > - reduce dependency (DummyChannel* and some RedChannelClient
> > > > internals);
> > > > - reuse RedChannelClient instead of reading from the RedsStream
> > > >   directly.
> > > > 
> > > > You can see that SndChannelClient has much less field
> > > > as the code to read/write from/to client is reused from
> > > > RedChannelClient instead of creating a fake RedChannelClient
> > > > just to make the system happy.
> > > > 
> > > I started looking at this, and it's really big/hard to review with all
> > > these changes :(
> > > Wondering if the AudioFrame allocation change could be done before, if
> > > we could have a few cosmetic commits first introducing
> > > SpiceMarshaller *m = client->send_data.marshaller; in snd_send_xxx()
> > > Maybe separate the GObjectification by itself from switching to using
> > > facilities provided by RedChannelClient, ...
> > > Dunno if the snd_set_command/snd_send API can be introduced before the
> > > gobjectification
> > > 
> > > Just a few random thoughts after reading this patch in the hope that it
> > > can be made simpler, some of them are probably not practical at all :(
> > > 
> > > Christophe
> > > 
> > 
> > Could be.
> > AudioFrame and possibly SpiceMarshaller and some other could be
> > split, snd_send and snd_set_command I don't think so, they rely to
> > reusing RedChannelClient base.
> > Have you considered that some patches are quite intermediate and not
> > expected to run?
> 
> Yes, these suggestions are patches that seems to be possible _before_
> these gobject patches, I guess anything before that is not going to need
> to be squashed?
> 
> Christophe
> 

If you want all commit to run correctly 8, 9, 10, 11 and 13 of version 3
should be squashed, that would make a really big patch.

Frediano