[Spice-devel] [PATCH spice-gtk 0/5] Require GStreamer, fix build warnings

[Frediano Ziglio]

> 
> Hi,
> 
> On Tue, Jan 08, 2019 at 08:25:09AM -0500, Frediano Ziglio wrote:
> > > We have only GStreamer support for video, so let's make it
> > > required.
> > 
> > There's also mjpeg for video.
> 
> I've tested mjpeg from GStreamer and works well too 0:-)
> 
> > > There is no modularity needed at this point, it can be added when
> > > needed (although I would rather have less modularity in general,
> > > because of testing matrix, maintainance..).
> > > 
> > 
> > Modularity is already present and you are removing it.
> > Usually modularity is used to reduce test matrix making sure all
> > implementations of a given interface behave as expected.
> 
> I don't follow how _having_ modularity _reduces_ the test matrix.
> 

If you have multiple implementation it transform products to sums.
And interfaces are used also for Mocking which is quite a fundamental
technique for code testing.
The problem is that our software is not well designed for testing
and we don't take much this in consideration so testing does not
improve much.

> > > We would like to remove PulseAudio audio backend in the future, as it
> > > will be replaced by pipewire, and unnecessarily make us maintain
> > > different audio backends: GStreamer handles various audio backends for
> > > us, and we know it is quite solid since it is being used with the
> > > Windows build for a long while.
> > > 
> > > Since we would like to release v0.36 soonish, and there are valid
> > > concerns that such a change now is a bit too risky, I propose to:
> > > 
> > > - make GStreamer a blessed dependency in 0.36 for audio & video (since
> > > it is required for video decoding/overlay etc)
> > > - keep pulse audio backend as default, but add a warning at compile
> > > time that it will be deprecated in future releases
> > > - make GStreamer audio backend lower the rank of pulsesrc < 1.15 so
> > > alsa or other backends are chosen with higher priority
> > > - after 0.36, make GStreamer audo backend the default, and disable the
> > > pulse backend (unless explicetly --enable-pulse).
> > > - later on, remove the pulse audio backend
> > > 
> > > Any comments?
> > > 
> > 
> > It sounds fine.
> > I'm noting however that this is the forth version in less then a week,
> > hope you are not changing your mind again tomorrow.
> 
> AFAICS, the changes were from feedback/interaction from me, you
> and teuf. If someone raises a valid point, what's the problem to
> have a new version tomorrow?
> 
> Cheers,
> 

It's true but this category of patches always pass for distro checking,
some of the versions were made because this was not done at the beginning.
Also if you a patch change the design is a good idea to think about a
bit more.

Frediano