From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Fri, 25 May 2018 11:12:40 +0200 Subject: [spice-server] sound: Don't mute recording when client reconnects When a new record channel is added, the code relies on a snd_send() call in record_channel_client_constructed() to send RECORD_START to the client. However, at this point, snd_send() is non-functional because the red_channel_client_pipe_add() call it makes is a no-op because prepare_pipe_add() makes a connection check through red_channel_client_is_connected() queueing the item. This connection check returns FALSE at ::constructed() time as the channel client will only become connected towards the end of red_channel_client_initable_init() which runs after the object instantiation is complete. This commit solves this issue by making PlaybackChannelClient and RecordChannelClient implement GInitable, and move the code interacting with the client in their _initable_init() function, as at this point the objects will be able to send data. This causes a bug where starting recording and then disconnecting/reconnecting the client does not successfully reenable recording. This is a regression introduced by commit d8dc09 'sound: Convert SndChannelClient to RedChannelClient' https://bugzilla.redhat.com/show_bug.cgi?id=1549132 Signed-off-by: Christophe Fergeau Acked-by: Victor Toso --- server/sound.c | 120 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 83 insertions(+), 37 deletions(-) diff --git a/server/sound.c b/server/sound.c index 9073626..8c6cf8a 100644 --- a/server/sound.c +++ b/server/sound.c @@ -103,6 +103,11 @@ typedef struct SndChannelClientClass { RedChannelClientClass parent_class; } SndChannelClientClass; +static void playback_channel_client_initable_interface_init(GInitableIface *iface); +static void record_channel_client_initable_interface_init(GInitableIface *iface); +static GInitableIface *playback_channel_client_parent_initable_iface; +static GInitableIface *record_channel_client_parent_initable_iface; + G_DEFINE_TYPE(SndChannelClient, snd_channel_client, RED_TYPE_CHANNEL_CLIENT) @@ -149,7 +154,9 @@ typedef struct PlaybackChannelClientClass { SndChannelClientClass parent_class; } PlaybackChannelClientClass; -G_DEFINE_TYPE(PlaybackChannelClient, playback_channel_client, TYPE_SND_CHANNEL_CLIENT) +G_DEFINE_TYPE_WITH_CODE(PlaybackChannelClient, playback_channel_client, TYPE_SND_CHANNEL_CLIENT, + G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE, playback_channel_client_initable_interface_init)) + typedef struct SpiceVolumeState { @@ -207,6 +214,7 @@ typedef struct RecordChannelClass { } RecordChannelClass; G_DEFINE_TYPE(RecordChannel, record_channel, TYPE_SND_CHANNEL) + #define TYPE_RECORD_CHANNEL_CLIENT record_channel_client_get_type() @@ -230,7 +238,8 @@ typedef struct RecordChannelClientClass { SndChannelClientClass parent_class; } RecordChannelClientClass; -G_DEFINE_TYPE(RecordChannelClient, record_channel_client, TYPE_SND_CHANNEL_CLIENT) +G_DEFINE_TYPE_WITH_CODE(RecordChannelClient, record_channel_client, TYPE_SND_CHANNEL_CLIENT, + G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE, record_channel_client_initable_interface_init)) /* A list of all Spice{Playback,Record}State objects */ @@ -1044,7 +1053,6 @@ playback_channel_client_constructed(GObject *object) RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback_client); RedChannel *red_channel = red_channel_client_get_channel(rcc); SndChannel *channel = SND_CHANNEL(red_channel); - RedClient *red_client = red_channel_client_get_client(rcc); SndChannelClient *scc = SND_CHANNEL_CLIENT(playback_client); G_OBJECT_CLASS(playback_channel_client_parent_class)->constructed(object); @@ -1070,18 +1078,6 @@ playback_channel_client_constructed(GObject *object) spice_debug("playback client %p using mode %s", playback_client, spice_audio_data_mode_to_string(playback_client->mode)); - - if (!red_client_during_migrate_at_target(red_client)) { - snd_set_command(scc, SND_PLAYBACK_MODE_MASK); - if (channel->volume.volume_nchannels) { - snd_set_command(scc, SND_VOLUME_MUTE_MASK); - } - } - - if (channel->active) { - playback_channel_client_start(scc); - } - snd_send(scc); } static void snd_set_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream, @@ -1250,27 +1246,6 @@ record_channel_client_finalize(GObject *object) 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); - SndChannelClient *scc = SND_CHANNEL_CLIENT(record_client); - - G_OBJECT_CLASS(record_channel_client_parent_class)->constructed(object); - - if (channel->volume.volume_nchannels) { - snd_set_command(scc, SND_VOLUME_MUTE_MASK); - } - - if (channel->active) { - record_channel_client_start(scc); - } - snd_send(scc); -} - - static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, RedsStream *stream, G_GNUC_UNUSED int migration, RedChannelCapabilities *caps) @@ -1478,6 +1453,43 @@ snd_channel_client_init(SndChannelClient *self) { } +static gboolean playback_channel_client_initable_init(GInitable *initable, + GCancellable *cancellable, + GError **error) +{ + gboolean success; + RedClient *red_client = red_channel_client_get_client(RED_CHANNEL_CLIENT(initable)); + SndChannelClient *scc = SND_CHANNEL_CLIENT(initable); + RedChannel *red_channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(initable)); + SndChannel *channel = SND_CHANNEL(red_channel); + + success = playback_channel_client_parent_initable_iface->init(initable, cancellable, error); + if (!success) { + return FALSE; + } + + if (!red_client_during_migrate_at_target(red_client)) { + snd_set_command(scc, SND_PLAYBACK_MODE_MASK); + if (channel->volume.volume_nchannels) { + snd_set_command(scc, SND_VOLUME_MUTE_MASK); + } + } + + if (channel->active) { + playback_channel_client_start(scc); + } + snd_send(SND_CHANNEL_CLIENT(initable)); + + return TRUE; +} + +static void playback_channel_client_initable_interface_init(GInitableIface *iface) +{ + playback_channel_client_parent_initable_iface = g_type_interface_peek_parent (iface); + + iface->init = playback_channel_client_initable_init; +} + static void playback_channel_client_class_init(PlaybackChannelClientClass *klass) { @@ -1505,11 +1517,45 @@ playback_channel_client_init(PlaybackChannelClient *playback) snd_playback_alloc_frames(playback); } +static gboolean record_channel_client_initable_init(GInitable *initable, + GCancellable *cancellable, + GError **error) +{ + gboolean success; + RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(initable); + RedChannel *red_channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(record_client)); + SndChannel *channel = SND_CHANNEL(red_channel); + SndChannelClient *scc = SND_CHANNEL_CLIENT(record_client); + + success = record_channel_client_parent_initable_iface->init(initable, cancellable, error); + if (!success) { + return FALSE; + } + + if (channel->volume.volume_nchannels) { + snd_set_command(scc, SND_VOLUME_MUTE_MASK); + } + + if (channel->active) { + record_channel_client_start(scc); + } + snd_send(SND_CHANNEL_CLIENT(initable)); + + return TRUE; +} + +static void record_channel_client_initable_interface_init(GInitableIface *iface) +{ + record_channel_client_parent_initable_iface = g_type_interface_peek_parent (iface); + + iface->init = record_channel_client_initable_init; +} + + 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; }