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

[Frediano Ziglio]

> Hi
> 
> On Sat, Jan 5, 2019 at 9:00 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> >
> > >
> > > Hi
> > >
> > > On Sat, Jan 5, 2019 at 1:59 PM Victor Toso <victortoso at redhat.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Fri, Jan 04, 2019 at 04:48:21PM +0400, marcandre.lureau at redhat.com
> > > > wrote:
> > > > > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > > >
> > > > > Hi,
> > > > >
> > > > > The series drops support for optional GStreamer dependency. Spice
> > > > > increasingly require GStreamer for audio and video support. GStreamer
> > > > > is widely available, including in embedded space.
> > > >
> > > > I think this is fine indeed, not 100% sure if we shouldn't do a
> > > > last release without this first. Another question is about
> > >
> > > As long as we don't require a too recent GStreamer (only 1.0
> > > apparently, the rest is checked with GST_CHECK_VERSION), I think it
> > > should be fine.
> > >
> > > > pulseaudio backend. If gstreamer is always enabled, I don't see
> > > > why we need pulseaudio..
> > >
> > > Good point, I thought the the gst backend was inferior to the pulse
> > > backend wrt volume handling, but you pointed out in commit message
> > > 808ac1d9b3fd926f660231776d5c66946fda5664 that gst now implements it
> > > for various elements.
> > >
> > > And with pipewire plan to replace pulseaudio (and using the gstreamer
> > > elements), I think this is the path forward.
> > >
> >
> > Is pipewire stable and cross platform? From the website I cannot find these
> > information. Before removing some support we should make sure we have a
> > good
> > replacement.
> 
> There are several reasons I mentionned pipewire:
> - it uses the gstreamer audio elements, which means they have to be quite
> solid

It's not true, plugins are written because is important to interact, it's
not a proof that one technology us solid, just that is used.

> - spice-gtk audio should use pipewire, and GStreamer backend is a good
> to do that

I don't understand what you mean here. Your patch is embedding gstreamer
as a requirement removing the possibility to switch multimedia framework
in the future (will require to undo part of your patches).
The current code allow multiple implementation, I would like to keep this
possibility. I'm not saying that we cannot require gstreamer, just that
should not became a design requirement.

> 
> Don't get me wrong, pipewire is not going to be a spice-gtk dependency.
> 

Than what do you mean with "spice-gtk audio should use pipewire"?
If it should use pipewire through gstreamer should be up to gstreamer
to choose the best output.

> >
> > Honestly I'm not really fun of needing GStreamer, if any platform have
> > a different streaming library I'd like to be able to use it, making
> > GStreamer a need is going the other direction.
> 
> As long as we support only GStreamer for audio/video, I don't see the
> point in having a switch to disable it.
> 

Not saying to have a switch to disable, but is good to keep the
possibility to switch implementation in the future.

> >
> > > I'll work on a patch to remove pulse.
> > >
> >
> > It seems too soon.
> 
> yes, we haven't done enough testing of the gstreamer backend on Linux.
> The recording path at least fails very often for me due to a race:
> https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/merge_requests/69
> 
> After fixing the pulsesrc element, I added a check in spice-gtk for
> pulseaudio plugin version. I don't see what else I could do.
> 

We can keep pulseaudio support. We have to consider also the various
distro. RHEL 7, for instance is using gstreamer 1.10.

> I propose to make GStreamer audio backend the default, and deprecate
> the pulse backend.
> 

"I'll work on a patch to remove pulse" is not deprecating, is removing it.

> If the gst pulseaudio plugin is too old (<1.15), then fallback on
> spice-gtk pulse backend for now.
> 

So it's not clear if you want to remove or not.

> >
> > > >
> > > > Thanks,
> > > >
> > > > > Marc-André Lureau (5):
> > > > >   build-sys: remove autoconf --with-audio=..
> > > > >   build-sys: drop gstaudio option, make GStreamer a requirement
> > > > >   build-sys: drop gstvideo option, make it required
> > > > >   widget: gst_size_allocate() is static
> > > > >   build-sys: fix gir/vapi warnings with GstPipeline
> > > > >
> > > > >  .gitlab-ci.yml                  |  4 --
> > > > >  configure.ac                    | 85
> > > > >  ++++++++++-----------------------
> > > > >  meson.build                     | 33 ++-----------
> > > > >  meson_options.txt               | 10 ----
> > > > >  src/Makefile.am                 | 18 ++-----
> > > > >  src/channel-display-priv.h      | 10 +---
> > > > >  src/channel-display.c           |  7 ---
> > > > >  src/meson.build                 | 14 ++----
> > > > >  src/spice-audio.c               |  4 --
> > > > >  src/spice-widget-priv.h         |  4 --
> > > > >  src/spice-widget.c              | 14 ++----
> > > > >  tools/spicy.c                   |  8 ----
> > > > >  vapi/Makefile.am                |  2 +
> > > > >  vapi/meson.build                |  4 +-
> > > > >  vapi/spice-client-glib-2.0.deps |  1 +
> > > > >  15 files changed, 47 insertions(+), 171 deletions(-)
> > > > >
> >
> > Frediano
> 
> 
> 
> --
> Marc-André Lureau
>