[Spice-devel] [PATCH v9 01/11] sound: Convert SndChannelClient to GObject

Frediano Ziglio fziglio at redhat.com
Tue Jan 3 10:37:42 UTC 2017


> 
> I've been trying to review this for quite a while now. It's rather
> dense and difficult to make sure I understand all of the potential
> minor changes, etc.
> 
> In general, it looks good, but I have a couple of questions/comments
> below.
> 
> On Tue, 2016-12-20 at 17:44 +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.
> > 
> > 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
> 
> Perhaps it's just my brain turning to mush as I try to fully understand
> all of these changes, but can you explain *why* we can't just use a
> queue like the other channels?
> 

A simple answer to this is what's happen on a digital (like a mobile)
phone when the line is disturbed? You just ear holes on the conversation
you don't get delay trying to recovery everything.
This is the behavior, messages are sent if possible, if not possible
just discarded. At least this was the design with some settings on
tcp side however tcp (that is the internet streaming socket) by default
have a queue and retransmission so the results is not that effective.

> 
> > 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.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/sound.c | 944 +++++++++++++++++++++------------------------
> > -----
> >  1 file changed, 413 insertions(+), 531 deletions(-)
> > 
> > diff --git a/server/sound.c b/server/sound.c
> > index 310ff6e..a645b60 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,6 +65,8 @@ 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)
> > @@ -82,49 +81,43 @@ typedef struct AudioFrameContainer
> > AudioFrameContainer;
> >  typedef struct SpicePlaybackState PlaybackChannel;
> >  typedef struct SpiceRecordState RecordChannel;
> >  
> > -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 void (*snd_channel_on_message_done_proc)(SndChannelClient
> > *client);
> > -typedef void (*snd_channel_cleanup_channel_proc)(SndChannelClient
> > *client);
> >  
> > -#define SND_CHANNEL_CLIENT(obj) (&(obj)->base)
> > +
> > +#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;
> > +    /* we don't expect very big messages so don't allocate too much
> > +     * bytes, data will be cached in RecordChannelClient::samples */
> > +    uint8_t receive_buf[SND_CODEC_MAX_FRAME_BYTES + 64];
> > +    RedPipeItem persistent_pipe_item;
> > +
> >      snd_channel_on_message_done_proc on_message_done;
> > -    snd_channel_cleanup_channel_proc cleanup;
> 
> In theory, on_message_done() seems like more of a class-level (virtual)
> function, no?
> 

Yes, sure. At this point however I would prefer a follow up patch.

> 
> >  };
> >  
> > -typedef struct AudioFrame AudioFrame;
> > +typedef struct SndChannelClientClass {
> > +    RedChannelClientClass parent_class;
> > +} SndChannelClientClass;
> > +
> > +G_DEFINE_TYPE(SndChannelClient, snd_channel_client,
> > RED_TYPE_CHANNEL_CLIENT)
> > +
> > +
> > +enum {
> > +    RED_PIPE_ITEM_PERSISTENT = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
> > +};
> > +
> > +
> >  struct AudioFrame {
> >      uint32_t time;
> >      uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
> > @@ -141,8 +134,13 @@ struct AudioFrameContainer
> >      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;
> > +    SndChannelClient parent;
> >  
> >      AudioFrameContainer *frames;
> >      AudioFrame *free_frames;
> > @@ -154,6 +152,13 @@ struct PlaybackChannelClient {
> >      uint8_t  encode_buf[SND_CODEC_MAX_COMPRESSED_BYTES];
> >  };
> >  
> > +typedef struct PlaybackChannelClientClass {
> > +    SndChannelClientClass parent_class;
> > +} PlaybackChannelClientClass;
> > +
> > +G_DEFINE_TYPE(PlaybackChannelClient, playback_channel_client,
> > TYPE_SND_CHANNEL_CLIENT)
> > +
> > +
> >  typedef struct SpiceVolumeState {
> >      uint16_t *volume;
> >      uint8_t volume_nchannels;
> > @@ -214,8 +219,13 @@ typedef struct RecordChannelClass {
> >  G_DEFINE_TYPE(RecordChannel, record_channel, TYPE_SND_CHANNEL)
> >  
> >  
> > +#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 base;
> > +    SndChannelClient parent;
> >      uint32_t samples[RECORD_SAMPLES_SIZE];
> >      uint32_t write_pos;
> >      uint32_t read_pos;
> > @@ -226,57 +236,24 @@ struct RecordChannelClient {
> >      uint8_t  decode_buf[SND_CODEC_MAX_FRAME_BYTES];
> >  };
> >  
> > +typedef struct RecordChannelClientClass {
> > +    SndChannelClientClass parent_class;
> > +} 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 void snd_playback_alloc_frames(PlaybackChannelClient
> > *playback);
> > -
> > -static SndChannelClient *snd_channel_unref(SndChannelClient *client)
> > -{
> > -    if (!--client->refs) {
> > -        spice_printerr("SndChannelClient=%p freed", client);
> > -        free(client);
> > -        return NULL;
> > -    }
> > -    return client;
> > -}
> > +static void snd_send(SndChannelClient * 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_CLI
> > ENT(client)));
> >  }
> >  
> >  static void snd_playback_free_frame(PlaybackChannelClient
> > *playback_client, AudioFrame *frame)
> > @@ -294,69 +271,11 @@ static void
> > snd_playback_on_message_done(SndChannelClient *client)
> >          playback_client->in_progress = NULL;
> >          if (playback_client->pending_frame) {
> >              client->command |= SND_PLAYBACK_PCM_MASK;
> > +            snd_send(client);
> >          }
> 
> I'm afraid that it's not entirely clear to me why this change was
> necessary.
> 

It's not so easy to understand as before there was an explicit while
loop while now the RedChannelClient queue is used (but forced to be
1 element).
Basically the old code was "send as much changes/data as possible".
This check respect this after sending a single item.

> >      }
> >  }
> >  
> > -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 |
> > -                                                                 SPI
> > CE_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;
> > @@ -402,35 +321,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 = RECORD_CHANNEL_CLIENT(rcc);
> >  
> > -    if (!client) {
> > -        return FALSE;
> > -    }
> >      switch (type) {
> >      case SPICE_MSGC_RECORD_DATA:
> >          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)) {
> > @@ -460,153 +373,23 @@ 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 red_channel_client_handle_message(rcc, size, type,
> > message);
> >      }
> >      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);
> > -    }
> > -}
> > -
> > -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)
> >  {
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    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);
> >      migrate.flags = 0;
> >      spice_marshall_msg_migrate(m, &migrate);
> >  
> > -    return snd_begin_send_message(client);
> > +    red_channel_client_begin_send_message(rcc);
> > +    return FALSE;
> 
> Why return FALSE here (and the following functions)? The previous code
> returned FALSE to indicate an error. I would think you'd want to return
> TRUE from all of these functions, no?
> 

FALSE was returned if wasn't possible to send a message (so basically
is message was not send).
But yes, looking at the code looks like for some reason the TRUE/FALSE
is reversed and would make more sense to return FALSE on failure which
actually is a bit different failure as now there is no check for
network queue directly.

> >  }
> >  
> >  static int snd_playback_send_migrate(PlaybackChannelClient *client)
> > @@ -618,25 +401,26 @@ static int snd_send_volume(SndChannelClient
> > *client, uint32_t cap, int msg)
> >  {
> >      SpiceMsgAudioVolume *vol;
> >      uint8_t c;
> > -    SpiceVolumeState *st = &client->channel->volume;
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    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);
> >      vol->nchannels = st->volume_nchannels;
> >      for (c = 0; c < st->volume_nchannels; ++c) {
> >          vol->volume[c] = st->volume[c];
> >      }
> >      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)
> > @@ -648,20 +432,21 @@ static int
> > snd_playback_send_volume(PlaybackChannelClient *playback_client)
> >  static int snd_send_mute(SndChannelClient *client, uint32_t cap, int
> > msg)
> >  {
> >      SpiceMsgAudioMute mute;
> > -    SpiceVolumeState *st = &client->channel->volume;
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    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);
> >      mute.mute = st->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)
> > @@ -672,48 +457,45 @@ static int
> > snd_playback_send_mute(PlaybackChannelClient *playback_client)
> >  
> >  static int snd_playback_send_latency(PlaybackChannelClient
> > *playback_client)
> >  {
> > -    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    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);
> >      latency_msg.latency_ms = playback_client->latency;
> >      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;
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    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);
> >      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(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);
> >  
> > -    return snd_begin_send_message(client);
> > +    red_channel_client_begin_send_message(rcc);
> > +    return FALSE;
> >  }
> >  
> >  static int snd_playback_send_ctl(PlaybackChannelClient
> > *playback_client)
> > @@ -729,32 +511,30 @@ static int
> > snd_playback_send_ctl(PlaybackChannelClient *playback_client)
> >  
> >  static int snd_record_send_start(RecordChannelClient *record_client)
> >  {
> > -    SndChannelClient *client = (SndChannelClient *)record_client;
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    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);
> >  
> >      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(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);
> >  
> > -    return snd_begin_send_message(client);
> > +    red_channel_client_begin_send_message(rcc);
> > +    return FALSE;
> >  }
> >  
> >  static int snd_record_send_ctl(RecordChannelClient *record_client)
> > @@ -791,14 +571,13 @@ static int
> > snd_record_send_migrate(RecordChannelClient *record_client)
> >  
> >  static int snd_playback_send_write(PlaybackChannelClient
> > *playback_client)
> >  {
> > -    SndChannelClient *client = (SndChannelClient *)playback_client;
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client);
> > +    SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> >      AudioFrame *frame;
> >      SpiceMsgPlaybackPacket msg;
> > +    RedPipeItem *pipe_item = &SND_CHANNEL_CLIENT(playback_client)-
> > >persistent_pipe_item;
> >  
> > -    if (!snd_reset_send_data(client, SPICE_MSG_PLAYBACK_DATA)) {
> > -        return FALSE;
> > -    }
> > +    red_channel_client_init_send_data(rcc, SPICE_MSG_PLAYBACK_DATA);
> >  
> >      frame = playback_client->in_progress;
> >      msg.time = frame->time;
> > @@ -806,9 +585,10 @@ static int
> > snd_playback_send_write(PlaybackChannelClient *playback_client)
> >      spice_marshall_msg_playback_data(m, &msg);
> >  
> >      if (playback_client->mode == SPICE_AUDIO_DATA_MODE_RAW) {
> > -        spice_marshaller_add_by_ref(m, (uint8_t *)frame->samples,
> > -                                    snd_codec_frame_size(playback_cl
> > ient->codec) *
> > -                                    sizeof(frame->samples[0]));
> > +        spice_marshaller_add_by_ref_full(m, (uint8_t *)frame-
> > >samples,
> > +                                         snd_codec_frame_size(playba
> > ck_client->codec) *
> > +                                         sizeof(frame->samples[0]),
> > +                                         marshaller_unref_pipe_item,
> > pipe_item);
> >      }
> >      else {
> >          int n = sizeof(playback_client->encode_buf);
> > @@ -816,46 +596,71 @@ static int
> > snd_playback_send_write(PlaybackChannelClient *playback_client)
> >                                      snd_codec_frame_size(playback_cl
> > ient->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;
> 
> Hmm, I see here that switching success from TRUE to FALSE was
> apparently intentional? Why?
> 

Still I think the value was just inverted, I'll revert it back.

> >          }
> > -        spice_marshaller_add_by_ref(m, playback_client->encode_buf,
> > n);
> > +        spice_marshaller_add_by_ref_full(m, playback_client-
> > >encode_buf, n,
> > +                                         marshaller_unref_pipe_item,
> > pipe_item);
> >      }
> >  
> > -    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;
> > -    SpiceMarshaller *m = client->send_data.marshaller;
> > +    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);
> >      mode.time = reds_get_mm_time();
> >      mode.mode = playback_client->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 = SND_CHANNEL_CLIENT(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 (!playback_client || !snd_send_data(data)) {
> > +    if (client->on_message_done) {
> > +        client->on_message_done(client);
> > +    }
> > +}
> 
> This is a pretty odd function and I think it deserves some explanation.
> 
> So, essentially, we're using the "freeing" of the pipe item to signal
> that a message is done being sent. Then we (potentially) kick off
> another message if there is an audio frame pending when
> on_message_done() is called.
> 
> But the first thing that came to my mind was: is there any way to
> simply tie into the existing channel callbacks (on_out_msg_done())
> instead of using this 'fake' free function to signal that the message
> is done? Maybe that would be even more 'odd'
> 

There are different such usages to RedPipeItem. The RedChannelClient
callback looks like some attempt to make RedChannelClient independent
to RedChannel but looking more closely looks like than the design was
abandoned quite earlier. If you looks more closely there is only a
single callback and it must be called so if you'll ever would like to
change it you must call the previous one. Quite prone to mistakes
which results in inconsistent states.

> > +
> > +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)
> 
> Why is the RedPipeItem argument named "base"? Seems something like
> 'item' would be more appropriate
> 

I think the reply is some copy&paste. item would do.

> 
> > +{
> > +    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;
> 
> So, previously, if playback_send_mode() failed, we didn't clear the
> MODE_MASK bit and presumably tried to send this message again next
> time. Now we clear it regardless of success or failure. In addition,
> when a send succeeded, it continued on to send the next command. In
> this version, you break out of the loop and only send a single command.
> 

As said the failure now is only due to serious errors or message not
supported. For this reason you need to clear the flag in all cases
to avoid infinite loop. Note that although this function looks similar
to the previous one we are sending a single message, not a loop till
we don't have nothing to send or the queue is full.

> Not sure that it matters much, but it is a behavior change.
> 

The final result is not changing the behavior.

> >          }
> >          if (client->command & SND_PLAYBACK_PCM_MASK) {
> >              spice_assert(!playback_client->in_progress &&
> > playback_client->pending_frame);
> > @@ -863,95 +668,94 @@ 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;
> > +            }
> >          }

Here not that the flags are a bit more bound to the messages
sent.

> >          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)
> 
> same comments as above
> 
> >  {
> > -    RecordChannelClient *record_client = (RecordChannelClient*)data;
> > -    SndChannelClient *client = SND_CHANNEL_CLIENT(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;
> >          }
> >      }
> > +    snd_send(client);
> >  }
> >  
> > -static SndChannelClient *__new_channel(SndChannel *channel, int
> > size, uint32_t channel_id,
> > -                                       RedClient *red_client,
> > -                                       RedsStream *stream,
> > -                                       int migrate,
> > -                                       snd_channel_send_messages_pro
> > c send_messages,
> > -                                       snd_channel_handle_message_pr
> > oc handle_message,
> > -                                       snd_channel_on_message_done_p
> > roc on_message_done,
> > -                                       snd_channel_cleanup_channel_p
> > roc cleanup,
> > -                                       uint32_t *common_caps, int
> > num_common_caps,
> > -                                       uint32_t *caps, int num_caps)
> > +static int snd_channel_config_socket(RedChannelClient *rcc)
> >  {
> > -    SndChannelClient *client;
> >      int delay_val;
> >      int flags;
> >  #ifdef SO_PRIORITY
> >      int priority;
> >  #endif
> >      int tos;
> > +    RedsStream *stream = red_channel_client_get_stream(rcc);
> > +    RedClient *red_client = red_channel_client_get_client(rcc);
> >      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;
> > +        return FALSE;
> >      }
> >  
> >  #ifdef SO_PRIORITY
> > @@ -980,69 +784,39 @@ static SndChannelClient
> > *__new_channel(SndChannel *channel, int size, uint32_t c
> >  
> >      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 FALSE;
> >      }
> > -    return client;
> > -
> > -error2:
> > -    free(client);
> > -
> > -error1:
> > -    reds_stream_free(stream);
> > -    return NULL;
> > -}
> >  
> > -static int snd_channel_config_socket(RedChannelClient *rcc)
> > -{
> > -    g_assert_not_reached();
> > +    return TRUE;
> >  }
> >  
> >  static void snd_channel_on_disconnect(RedChannelClient *rcc)
> >  {
> > -    g_assert_not_reached();
> > +    SndChannel *channel =
> > SND_CHANNEL(red_channel_client_get_channel(rcc));
> > +    if (channel->connection && rcc == RED_CHANNEL_CLIENT(channel-
> > >connection)) {
> > +        channel->connection = NULL;
> > +    }
> >  }
> >  
> >  static uint8_t*
> >  snd_channel_client_alloc_recv_buf(RedChannelClient *rcc, uint16_t
> > type, uint32_t size)
> >  {
> > -    g_assert_not_reached();
> > +    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
> > +    // If message is to big allocate one, this should never happen
> 
> to -> too?
> 

yes.

> > +    if (size > sizeof(client->receive_buf)) {
> > +        return spice_malloc(size);
> > +    }
> > +    return client->receive_buf;
> >  }
> >  
> >  static void
> >  snd_channel_client_release_recv_buf(RedChannelClient *rcc, uint16_t
> > type, uint32_t size,
> >                                      uint8_t *msg)
> >  {
> > -    g_assert_not_reached();
> > +    SndChannelClient *client = SND_CHANNEL_CLIENT(rcc);
> > +    if (msg != client->receive_buf) {
> > +        free(msg);
> > +    }
> >  }
> >  
> >  static void snd_disconnect_channel_client(RedChannelClient *rcc)
> > @@ -1057,8 +831,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);
> >      }
> >  }
> >  
> > @@ -1076,7 +850,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);
> > @@ -1085,21 +858,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)
> > @@ -1114,7 +888,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;
> >      }
> > @@ -1128,17 +902,17 @@ 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;
> > +    PlaybackChannelClient *playback_client =
> > PLAYBACK_CHANNEL_CLIENT(client);
> >      spice_assert(client->active);
> >      reds_enable_mm_time(snd_channel_get_server(client));
> >      client->active = FALSE;
> >      if (client->client_active) {
> >          snd_set_command(client, SND_CTRL_MASK);
> > -        snd_playback_send(client);
> > +        snd_send(client);
> >      } else {
> >          client->command &= ~SND_CTRL_MASK;
> >          client->command &= ~SND_PLAYBACK_PCM_MASK;
> > @@ -1156,11 +930,14 @@ 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(client->active);
> > @@ -1201,7 +978,7 @@ SPICE_GNUC_VISIBLE void
> > spice_server_playback_put_samples(SpicePlaybackInstance
> >      frame->time = reds_get_mm_time();
> >      playback_client->pending_frame = frame;
> >      snd_set_command(SND_CHANNEL_CLIENT(playback_client),
> > SND_PLAYBACK_PCM_MASK);
> > -    snd_playback_send(SND_CHANNEL_CLIENT(playback_client));
> > +    snd_send(SND_CHANNEL_CLIENT(playback_client));
> >  }
> >  
> >  void snd_set_playback_latency(RedClient *client, uint32_t latency)
> > @@ -1212,15 +989,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");
> >              }
> > @@ -1255,17 +1032,19 @@ 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);
> >  
> >      // free frames, unref them
> >      for (i = 0; i < NUM_AUDIO_FRAMES; ++i) {
> > @@ -1280,43 +1059,31 @@ static void
> > snd_playback_cleanup(SndChannelClient *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,
> > -                                                                   s
> > izeof(*playback_client),
> > -                                                                   S
> > PICE_CHANNEL_PLAYBACK,
> > -                                                                   c
> > lient,
> > -                                                                   s
> > tream,
> > -                                                                   m
> > igration,
> > -                                                                   s
> > nd_playback_send,
> > -                                                                   s
> > nd_playback_handle_message,
> > -                                                                   s
> > nd_playback_on_message_done,
> > -                                                                   s
> > nd_playback_cleanup,
> > -                                                                   c
> > ommon_caps, num_common_caps,
> > -                                                                   c
> > aps, num_caps))) {
> > -        return;
> > -    }
> > +    G_OBJECT_CLASS(playback_channel_client_parent_class)-
> > >constructed(object);
> >  
> > -    snd_playback_alloc_frames(playback_client);
> > +    SND_CHANNEL_CLIENT(playback_client)->on_message_done =
> > snd_playback_on_message_done;
> >  
> > -    int client_can_celt =
> > red_channel_client_test_remote_cap(playback_client-
> > >base.channel_client,
> > +    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) {
> > @@ -1333,7 +1100,46 @@ static void snd_set_playback_peer(RedChannel
> > *red_channel, RedClient *client, Re
> >      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)
> > @@ -1345,9 +1151,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);
> >      }
> >  }
> >  
> > @@ -1357,7 +1163,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);
> > @@ -1366,38 +1171,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
> >                                                                //stre
> > am 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;
> >      }
> > @@ -1419,7 +1226,7 @@ SPICE_GNUC_VISIBLE void
> > spice_server_record_stop(SpiceRecordInstance *sin)
> >      client->active = FALSE;
> >      if (client->client_active) {
> >          snd_set_command(client, SND_CTRL_MASK);
> > -        snd_record_send(client);
> > +        snd_send(client);
> >      } else {
> >          client->command &= ~SND_CTRL_MASK;
> >      }
> > @@ -1429,13 +1236,13 @@ 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;
> > +    RecordChannelClient *record_client =
> > RECORD_CHANNEL_CLIENT(client);
> >      spice_assert(client->active);
> >  
> >      if (record_client->write_pos < RECORD_SAMPLES_SIZE / 2) {
> > @@ -1444,14 +1251,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
> 
> ???
> 

I think this code should just be removed. Data should be readed
as soon as available. I'll remove it.

> >      if (len < bufsize) {
> > -        SndChannel *channel = client->channel;
> > +        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;
> > @@ -1467,7 +1277,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))
> > @@ -1509,52 +1319,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,
> > -                                                               sizeo
> > f(*record_client),
> > -                                                               SPICE
> > _CHANNEL_RECORD,
> > -                                                               clien
> > t,
> > -                                                               strea
> > m,
> > -                                                               migra
> > tion,
> > -                                                               snd_r
> > ecord_send,
> > -                                                               snd_r
> > ecord_handle_message,
> > -                                                               snd_r
> > ecord_on_message_done,
> > -                                                               snd_r
> > ecord_cleanup,
> > -                                                               commo
> > n_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,
> > SND_CHANNEL_CLIENT(record_client));
> > -    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)
> > @@ -1567,9 +1404,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);
> >      }
> >  }
> >  
> > @@ -1641,8 +1478,13 @@ static void
> >  playback_channel_class_init(PlaybackChannelClass *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)
> > @@ -1687,8 +1529,13 @@ static void
> >  record_channel_class_init(RecordChannelClass *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)
> > @@ -1709,7 +1556,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;
> > @@ -1735,9 +1581,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_opu
> > s, client_can_celt);
> > @@ -1749,10 +1596,31 @@ void snd_set_playback_compression(int on)
> >      }
> >  }
> >  
> > -static void snd_playback_alloc_frames(PlaybackChannelClient
> > *playback)
> > +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) {
> > @@ -1760,3 +1628,17 @@ static void
> > snd_playback_alloc_frames(PlaybackChannelClient *playback)
> >          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;
> > +}
> 

Frediano


More information about the Spice-devel mailing list