[Spice-devel] [PATCH v4 05/14] sound: Convert SndChannel to GObject

Christophe Fergeau cfergeau at redhat.com
Wed Nov 30 14:58:47 UTC 2016


On Wed, Nov 30, 2016 at 12:34:49PM +0000, Frediano Ziglio wrote:
> NOTE: this is not supposed to run!
> 
> Just make easy the review, should be squashed to following 2
> patches.

This works reasonably well (I get sound in a VM) after adding the patch below.
I'd keep it separate, and add a commit log (this adds the GObject boilerplate,
and also stops using the dummy channel, however sending of data still goes
through DummyChannelClient which is why are good with no implementation of some
of the required vfuncs)


diff --git a/server/sound.c b/server/sound.c
index a7622be..e02e9dc 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1017,6 +1017,30 @@ static void snd_disconnect_channel_client(RedChannelClient *rcc)
     }
 }

+static int snd_channel_config_socket(RedChannelClient *rcc)
+{
+    g_assert_not_reached();
+}
+
+static void snd_channel_on_disconnect(RedChannelClient *rcc)
+{
+    g_assert_not_reached();
+}
+
+static uint8_t*
+snd_channel_client_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size)
+{
+    g_assert_not_reached();
+}
+
+static void
+snd_channel_client_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size,
+                                    uint8_t *msg)
+{
+    g_assert_not_reached();
+}
+
+
 static void snd_set_command(SndChannelClient *client, uint32_t command)
 {
     if (!client) {
@@ -1543,6 +1567,12 @@ snd_channel_init(SndChannel *self)
 static void
 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;
+    channel_class->on_disconnect = snd_channel_on_disconnect;
 }

 static void




> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/sound.c | 195 +++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 130 insertions(+), 65 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 5b66f3a..de46be5 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -144,9 +144,14 @@ typedef struct SpiceVolumeState {
>      int mute;
>  } SpiceVolumeState;
>  
> +#define TYPE_SND_CHANNEL snd_channel_get_type()
> +#define SND_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_SND_CHANNEL, SndChannel))
> +GType snd_channel_get_type(void) G_GNUC_CONST;
> +
>  /* Base class for SpicePlaybackState and SpiceRecordState */
>  struct SndChannel {
> -    RedChannel *base_channel;
> +    RedChannel parent;
> +
>      SndChannelClient *connection; /* Only one client is supported */
>      SndChannel *next; /* For the global SndChannel list */
>  
> @@ -155,14 +160,40 @@ struct SndChannel {
>      uint32_t frequency;
>  };
>  
> +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;
> +
>  struct SpicePlaybackState {
> -    struct SndChannel channel;
> +    SndChannel channel;
>  };
>  
> +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;
> +
>  struct SpiceRecordState {
> -    struct SndChannel channel;
> +    SndChannel channel;
>  };
>  
> +G_DEFINE_TYPE(RecordChannel, record_channel, TYPE_SND_CHANNEL)
> +
> +
>  typedef struct RecordChannelClient {
>      SndChannelClient base;
>      uint32_t samples[RECORD_SAMPLES_SIZE];
> @@ -201,7 +232,7 @@ static SndChannelClient *snd_channel_unref(SndChannelClient *client)
>  static RedsState* snd_channel_get_server(SndChannelClient *client)
>  {
>      g_return_val_if_fail(client != NULL, NULL);
> -    return red_channel_get_server(client->channel->base_channel);
> +    return red_channel_get_server(RED_CHANNEL(client->channel));
>  }
>  
>  static void snd_disconnect_channel(SndChannelClient *client)
> @@ -892,7 +923,7 @@ static SndChannelClient *__new_channel(SndChannel *channel, int size, uint32_t c
>  #endif
>      int tos;
>      MainChannelClient *mcc = red_client_get_main(red_client);
> -    RedsState *reds = red_channel_get_server(channel->base_channel);
> +    RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
>  
>      spice_assert(stream);
>      if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
> @@ -953,7 +984,7 @@ static SndChannelClient *__new_channel(SndChannel *channel, int size, uint32_t c
>      client->cleanup = cleanup;
>  
>      client->channel_client =
> -        dummy_channel_client_create(channel->base_channel, red_client,
> +        dummy_channel_client_create(RED_CHANNEL(channel), red_client,
>                                      num_common_caps, common_caps, num_caps, caps);
>      if (!client->channel_client) {
>          goto error2;
> @@ -975,7 +1006,7 @@ static void snd_disconnect_channel_client(RedChannelClient *rcc)
>      uint32_t type;
>  
>      spice_assert(red_channel);
> -    channel = (SndChannel *)g_object_get_data(G_OBJECT(red_channel), "sound-channel");
> +    channel = SND_CHANNEL(red_channel);
>      spice_assert(channel);
>      g_object_get(red_channel, "channel-type", &type, NULL);
>  
> @@ -1126,7 +1157,7 @@ void snd_set_playback_latency(RedClient *client, uint32_t latency)
>  
>      for (; now; now = now->next) {
>          uint32_t type;
> -        g_object_get(now->base_channel, "channel-type", &type, NULL);
> +        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) {
>  
> @@ -1161,7 +1192,7 @@ static int snd_desired_audio_mode(int playback_compression, int frequency,
>  
>  static void on_new_playback_channel(SndChannel *channel, SndChannelClient *snd_channel)
>  {
> -    RedsState *reds = red_channel_get_server(channel->base_channel);
> +    RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
>  
>      spice_assert(snd_channel);
>  
> @@ -1193,7 +1224,7 @@ static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, Re
>                                    int migration, int num_common_caps, uint32_t *common_caps,
>                                    int num_caps, uint32_t *caps)
>  {
> -    SndChannel *channel = g_object_get_data(G_OBJECT(red_channel), "sound-channel");
> +    SndChannel *channel = SND_CHANNEL(red_channel);
>      PlaybackChannelClient *playback_client;
>  
>      snd_disconnect_channel(channel->connection);
> @@ -1249,9 +1280,8 @@ static void snd_record_migrate_channel_client(RedChannelClient *rcc)
>      SndChannel *channel;
>      RedChannel *red_channel = red_channel_client_get_channel(rcc);
>  
> -    spice_debug(NULL);
>      spice_assert(red_channel);
> -    channel = (SndChannel *)g_object_get_data(G_OBJECT(red_channel), "sound-channel");
> +    channel = SND_CHANNEL(red_channel);
>      spice_assert(channel);
>  
>      if (channel->connection) {
> @@ -1389,7 +1419,7 @@ static uint32_t snd_get_best_rate(SndChannelClient *client, uint32_t cap_opus)
>  
>  static void snd_set_rate(SndChannel *channel, uint32_t frequency, uint32_t cap_opus)
>  {
> -    RedChannel *red_channel = channel->base_channel;
> +    RedChannel *red_channel = RED_CHANNEL(channel);
>      channel->frequency = frequency;
>      if (red_channel && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS, frequency)) {
>          red_channel_set_cap(red_channel, cap_opus);
> @@ -1439,7 +1469,7 @@ static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, Reds
>                                  int migration, int num_common_caps, uint32_t *common_caps,
>                                  int num_caps, uint32_t *caps)
>  {
> -    SndChannel *channel = g_object_get_data(G_OBJECT(red_channel), "sound-channel");
> +    SndChannel *channel = SND_CHANNEL(red_channel);
>      RecordChannelClient *record_client;
>  
>      snd_disconnect_channel(channel->connection);
> @@ -1474,7 +1504,7 @@ static void snd_playback_migrate_channel_client(RedChannelClient *rcc)
>      RedChannel *red_channel = red_channel_client_get_channel(rcc);
>  
>      spice_assert(red_channel);
> -    channel = (SndChannel *)g_object_get_data(G_OBJECT(red_channel), "sound-channel");
> +    channel = SND_CHANNEL(red_channel);
>      spice_assert(channel);
>      spice_debug(NULL);
>  
> @@ -1504,60 +1534,107 @@ static void remove_channel(SndChannel *channel)
>      spice_printerr("not found");
>  }
>  
> -void snd_attach_playback(RedsState *reds, SpicePlaybackInstance *sin)
> +static void
> +snd_channel_init(SndChannel *self)
>  {
> -    SndChannel *playback;
> -    RedChannel *red_channel;
> -    ClientCbs client_cbs = { NULL, };
> +    self->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the legacy rate */
> +}
> +
> +static void
> +snd_channel_class_init(SndChannelClass *klass)
> +{
> +}
>  
> -    sin->st = spice_new0(SpicePlaybackState, 1);
> -    playback = &sin->st->channel;
> -    playback->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the legacy rate */
> +static void
> +playback_channel_init(PlaybackChannel *self)
> +{
> +}
>  
> -    // TODO: Make RedChannel base of channel? instead of assigning it to channel->data
> -    red_channel = dummy_channel_new(reds, SPICE_CHANNEL_PLAYBACK, 0);
> +static void
> +playback_channel_constructed(GObject *object)
> +{
> +    ClientCbs client_cbs = { NULL, };
> +    SndChannel *self = SND_CHANNEL(object);
> +    RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
> +
> +    G_OBJECT_CLASS(playback_channel_parent_class)->constructed(object);
>  
> -    g_object_set_data(G_OBJECT(red_channel), "sound-channel", playback);
>      client_cbs.connect = snd_set_playback_peer;
>      client_cbs.disconnect = snd_disconnect_channel_client;
>      client_cbs.migrate = snd_playback_migrate_channel_client;
> -    red_channel_register_client_cbs(red_channel, &client_cbs, playback);
> +    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs, self);
> +
> +    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1, SND_CODEC_ANY_FREQUENCY)) {
> +        red_channel_set_cap(RED_CHANNEL(self), SPICE_PLAYBACK_CAP_CELT_0_5_1);
> +    }
> +    red_channel_set_cap(RED_CHANNEL(self), SPICE_PLAYBACK_CAP_VOLUME);
>  
> -    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1, SND_CODEC_ANY_FREQUENCY))
> -        red_channel_set_cap(red_channel, SPICE_PLAYBACK_CAP_CELT_0_5_1);
> +    add_channel(self);
> +    reds_register_channel(reds, RED_CHANNEL(self));
> +}
>  
> -    red_channel_set_cap(red_channel, SPICE_PLAYBACK_CAP_VOLUME);
> +static void
> +playback_channel_class_init(SndChannelClass *klass)
> +{
> +    GObjectClass *object_class = G_OBJECT_CLASS(klass);
>  
> -    playback->base_channel = red_channel;
> -    add_channel(playback);
> -    reds_register_channel(reds, red_channel);
> +    object_class->constructed = playback_channel_constructed;
>  }
>  
> -void snd_attach_record(RedsState *reds, SpiceRecordInstance *sin)
> +void snd_attach_playback(RedsState *reds, SpicePlaybackInstance *sin)
>  {
> -    SndChannel *record;
> -    RedChannel *red_channel;
> -    ClientCbs client_cbs = { NULL, };
> +    sin->st = g_object_new(TYPE_PLAYBACK_CHANNEL,
> +                           "spice-server", reds,
> +                           "core-interface", reds_get_core_interface(reds),
> +                           "channel-type", SPICE_CHANNEL_PLAYBACK,
> +                           "id", 0,
> +                           NULL);
> +}
>  
> -    sin->st = spice_new0(SpiceRecordState, 1);
> -    record = &sin->st->channel;
> -    record->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the legacy rate */
> +static void
> +record_channel_init(RecordChannel *self)
> +{
> +}
>  
> -    // TODO: Make RedChannel base of channel? instead of assigning it to channel->data
> -    red_channel = dummy_channel_new(reds, SPICE_CHANNEL_RECORD, 0);
> +static void
> +record_channel_constructed(GObject *object)
> +{
> +    ClientCbs client_cbs = { NULL, };
> +    SndChannel *self = SND_CHANNEL(object);
> +    RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
> +
> +    G_OBJECT_CLASS(record_channel_parent_class)->constructed(object);
>  
> -    g_object_set_data(G_OBJECT(red_channel), "sound-channel", record);
>      client_cbs.connect = snd_set_record_peer;
>      client_cbs.disconnect = snd_disconnect_channel_client;
>      client_cbs.migrate = snd_record_migrate_channel_client;
> -    red_channel_register_client_cbs(red_channel, &client_cbs, record);
> -    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1, SND_CODEC_ANY_FREQUENCY))
> -        red_channel_set_cap(red_channel, SPICE_RECORD_CAP_CELT_0_5_1);
> -    red_channel_set_cap(red_channel, SPICE_RECORD_CAP_VOLUME);
> +    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs, self);
> +
> +    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1, SND_CODEC_ANY_FREQUENCY)) {
> +        red_channel_set_cap(RED_CHANNEL(self), SPICE_RECORD_CAP_CELT_0_5_1);
> +    }
> +    red_channel_set_cap(RED_CHANNEL(self), SPICE_RECORD_CAP_VOLUME);
>  
> -    record->base_channel = red_channel;
> -    add_channel(record);
> -    reds_register_channel(reds, red_channel);
> +    add_channel(self);
> +    reds_register_channel(reds, RED_CHANNEL(self));
> +}
> +
> +static void
> +record_channel_class_init(SndChannelClass *klass)
> +{
> +    GObjectClass *object_class = G_OBJECT_CLASS(klass);
> +
> +    object_class->constructed = record_channel_constructed;
> +}
> +
> +void snd_attach_record(RedsState *reds, SpiceRecordInstance *sin)
> +{
> +    sin->st = g_object_new(TYPE_RECORD_CHANNEL,
> +                           "spice-server", reds,
> +                           "core-interface", reds_get_core_interface(reds),
> +                           "channel-type", SPICE_CHANNEL_RECORD,
> +                           "id", 0,
> +                           NULL);
>  }
>  
>  static void snd_detach_common(SndChannel *channel)
> @@ -1565,36 +1642,24 @@ static void snd_detach_common(SndChannel *channel)
>      if (!channel) {
>          return;
>      }
> -    RedsState *reds = red_channel_get_server(channel->base_channel);
> +    RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
>  
>      remove_channel(channel);
>      snd_disconnect_channel(channel->connection);
> -    reds_unregister_channel(reds, channel->base_channel);
> -    red_channel_destroy(channel->base_channel);
> +    reds_unregister_channel(reds, RED_CHANNEL(channel));
>      free(channel->volume.volume);
>      channel->volume.volume = NULL;
> -}
> -
> -static void spice_playback_state_free(SpicePlaybackState *st)
> -{
> -    free(st);
> +    red_channel_destroy(RED_CHANNEL(channel));
>  }
>  
>  void snd_detach_playback(SpicePlaybackInstance *sin)
>  {
>      snd_detach_common(&sin->st->channel);
> -    spice_playback_state_free(sin->st);
> -}
> -
> -static void spice_record_state_free(SpiceRecordState *st)
> -{
> -    free(st);
>  }
>  
>  void snd_detach_record(SpiceRecordInstance *sin)
>  {
>      snd_detach_common(&sin->st->channel);
> -    spice_record_state_free(sin->st);
>  }
>  
>  void snd_set_playback_compression(int on)
> @@ -1603,7 +1668,7 @@ void snd_set_playback_compression(int on)
>  
>      for (; now; now = now->next) {
>          uint32_t type;
> -        g_object_get(now->base_channel, "channel-type", &type, NULL);
> +        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,
> -- 
> git-series 0.9.1
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161130/05d5fb3b/attachment.sig>


More information about the Spice-devel mailing list