[Spice-devel] [PATCH spice-server 11/18] sound: Convert SndChannelClient to GObject

[Christophe Fergeau]

On Tue, Nov 29, 2016 at 10:16:04AM -0500, Frediano Ziglio wrote:
> > 
> > Hey,
> > 
> > On Wed, Nov 23, 2016 at 06:07:25PM +0000, Frediano Ziglio wrote:
> > > This patch is quite huge but have some benefits:
> > > - reduce dependency (DummyChannel* and some RedChannelClient internals);
> > > - reuse RedChannelClient instead of reading from the RedsStream
> > >   directly.
> > > 
> > > You can see that SndChannelClient has much less field
> > > as the code to read/write from/to client is reused from
> > > RedChannelClient instead of creating a fake RedChannelClient
> > > just to make the system happy.
> > > 
> > I started looking at this, and it's really big/hard to review with all
> > these changes :(
> > Wondering if the AudioFrame allocation change could be done before, if
> > we could have a few cosmetic commits first introducing
> > SpiceMarshaller *m = client->send_data.marshaller; in snd_send_xxx()
> > Maybe separate the GObjectification by itself from switching to using
> > facilities provided by RedChannelClient, ...
> > Dunno if the snd_set_command/snd_send API can be introduced before the
> > gobjectification
> > 
> > Just a few random thoughts after reading this patch in the hope that it
> > can be made simpler, some of them are probably not practical at all :(
> > 
> > Christophe
> > 
> 
> Could be.
> AudioFrame and possibly SpiceMarshaller and some other could be
> split, snd_send and snd_set_command I don't think so, they rely to
> reusing RedChannelClient base.
> Have you considered that some patches are quite intermediate and not
> expected to run?

Yes, these suggestions are patches that seems to be possible _before_
these gobject patches, I guess anything before that is not going to need
to be squashed?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161129/2f503f2c/attachment.sig>