[Spice-devel] [PATCH v4 05/14] sound: Convert SndChannel to GObject
Frediano Ziglio
fziglio at redhat.com
Wed Nov 30 16:37:41 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)
>
Merged and tested. Both play & record work.
Frediano
>
> 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,
More information about the Spice-devel
mailing list