[Spice-devel] [PATCH spice-gtk 0/5] Require GStreamer, fix build warnings
Marc-André Lureau
marcandre.lureau at gmail.com
Mon Jan 7 19:30:55 UTC 2019
Hi
On Mon, Jan 7, 2019 at 5:46 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> > Hi
> >
> > On Mon, Jan 7, 2019 at 3:44 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> > >
> > > > 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.
> >
> > It has to be solid to be the core audio stack of Gnome & Fedora, and
> > replace pulseaudio.
> >
>
> It seems you are speaking of the future using a past tense. I opened
> some programs under Fedora 29 and they are using pulseaudio or alsa.
>
> > > > - 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.
> >
> > spice-gtk should use pipewire when available, and the GStreamer
> > backend is the way to to that.
> >
>
> Agree, or any other framework suitable.
>
> > There is no need to keep modularity if we have single backend.
> >
>
> But as we already have it I don't see the reason to remove it.
>
> > It is fairly easy to bring back modularity when we have it again.
> >
>
> Yes, at the present, but I would like to keep it keeping the
> modularity.
>
> > >
> > > >
> > > > 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.
> >
> > yes
> >
> > >
> > > > >
> > > > > 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.
> >
> > Again, it's really not that big of a deal. Let's not have unnecessary
> > fake modularity for a potential future backend.
> >
>
> I disagree. You are already talking about different backend.
>
> > >
> > > > >
> > > > > > 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.
> >
> > Yes, as discussed, we keep the pulse backend for a while and detect
> > broken GStreamer.
> >
> > >
> > > > 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.
> >
> > In the future, yes! For now, use GStreamer by default if possible, and
> > fallback on pulse.
> >
> > See also proposed implementation:
> > "[PATCH spice-gtk 32/34] gst: check pulseaudio plugin version >= 1.15"
> > "[PATCH spice-gtk 33/34] audio: use gstreamer by default"
We have only GStreamer support for video, so let's make it required.
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..).
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?
> >
> >
> > >
> > > > >
> > > > > > >
> > > > > > > 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
> > > >
> >
> >
> >
> > --
> > Marc-André Lureau
> >
--
Marc-André Lureau
More information about the Spice-devel
mailing list