From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Christophe Fergeau <cfergeau@redhat.com>

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 <cfergeau@redhat.com>

Acked-by: Victor Toso <victortoso@redhat.com>

---

 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;

 }