[Spice-devel] [spice-server v2] sound: Don't mute recording when client reconnects

[Frediano Ziglio]

> 
> On Fri, May 25, 2018 at 07:33:56AM -0400, Frediano Ziglio wrote:
> > > +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) {
> > 
> > I think is short and easier to read doing the if directly.
> 
> Probably a matter of preference, I prefer to have functions with side
> effects separate from their if (success) test
> > > +    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;
> > >  }
> > >  
> > 
> > Is it only me or che code looks a lot duplicated?
> 
> A lot duplicated? It's a bit messy between ::init(), ::constructed() and
> ::initable_init(), but if I did not mess things up, they should be doing
> different things (ie no actual code duplication).
> 

Forgot to add OT, looks like the 2 _init/_constructed are mostly
doing the same thing. But is not a regression.

> > Reusing constructed function instead of removing and create new
> > function in different places won't reduce the diff?
> 
> RedChannelClient::initable_init() is needed as this is the only place
> where you can handle and report init failures, which is why some code is
> there. Since snd_send() needs to run after the code in
> RedChannelClient::initable_init(), that code is moved there.
> 
> Or where you pointing out a different issue?
> 

Was more a patch style thing.
The new _init are doing more or less the same thing as the old
_constructed but as the new functions are in different lines
in the final code the diff results huge. Probably if you put
the new _init where the old _constructed were the diff will be
smaller and is easier to understand the real change.

> Christophe
> 

Frediano