> > 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 The patch looks surely good, but why red_channel_client_is_connected returns a wrong value? The channel is connected but asking if is connected returns false... Is currently implemented as: is in the channel list so is connected, otherwise is not which during construction is wrong as still not in the list. Won't be better to check if really connected? > 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. > For me a RCC is connected if there is a client connected at socket level, being into a list of RCCs is not a really a great definition for connected. Maybe removing the check for red_channel_client_is_connected on prepare_pipe_add would solve the issue? > 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@xxxxxxxxxx> > --- > server/sound.c | 37 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 34 insertions(+), 3 deletions(-) > > diff --git a/server/sound.c b/server/sound.c > index e3891d2cc..d461b9272 100644 > --- a/server/sound.c > +++ b/server/sound.c > @@ -105,7 +105,11 @@ typedef struct SndChannelClientClass { > RedChannelClientClass parent_class; > } SndChannelClientClass; > > -G_DEFINE_TYPE(SndChannelClient, snd_channel_client, RED_TYPE_CHANNEL_CLIENT) > +static void snd_channel_client_initable_interface_init(GInitableIface > *iface); > +static GInitableIface *snd_channel_client_parent_initable_iface; > + > +G_DEFINE_TYPE_WITH_CODE(SndChannelClient, snd_channel_client, > RED_TYPE_CHANNEL_CLIENT, > + G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE, > snd_channel_client_initable_interface_init)) > > > enum { > @@ -1083,7 +1087,6 @@ playback_channel_client_constructed(GObject *object) > if (channel->active) { > playback_channel_client_start(scc); > } > - snd_send(scc); > } > > static void snd_set_peer(RedChannel *red_channel, RedClient *client, > RedStream *stream, > @@ -1269,7 +1272,6 @@ record_channel_client_constructed(GObject *object) > if (channel->active) { > record_channel_client_start(scc); > } > - snd_send(scc); > } > > > @@ -1480,6 +1482,35 @@ snd_channel_client_init(SndChannelClient *self) > { > } > > +static gboolean snd_channel_client_initable_init(GInitable *initable, > + GCancellable *cancellable, > + GError **error) > +{ > + gboolean success; > + > + success = snd_channel_client_parent_initable_iface->init(initable, > cancellable, error); > + if (!success) { > + return FALSE; > + } > + /* Wait until the very end of the initialization as the channel client > + * becomes connected (red_channel_client_is_connected() starts returning > + * TRUE) only very late in red_channel_client_initable_init() > + * Before that, red_channel_client_pipe_add() is a no-op because a > connection check is done in > + * prepare_pipe_add() > + */ > + snd_send(SND_CHANNEL_CLIENT(initable)); > + > + return TRUE; > +} > + > +static void snd_channel_client_initable_interface_init(GInitableIface > *iface) > +{ > + snd_channel_client_parent_initable_iface = g_type_interface_peek_parent > (iface); > + > + iface->init = snd_channel_client_initable_init; > +} > + > + > static void > playback_channel_client_class_init(PlaybackChannelClientClass *klass) > { Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel